Add support for ipq8064 tsens

Use the driver get_temp function instead of force to use the generic get temp function. This is needed as tsens v0 version use a custom function to get the real temperature.

Signed-off-by Ansuel Smith <[email protected]>

---
 drivers/thermal/qcom/tsens.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

drivers/thermal/qcom/tsens.c +0 -1

index 9af6f71ab640..9fe9a2b26705 100644
@@ -580,7 +580,6 @@ int get_temp_tsens_valid(const struct tsens_sensor *s, int *temp)
 {
        struct tsens_priv *priv = s->priv;
        int hw_id = s->hw_id;
-       u32 temp_idx = LAST_TEMP_0 + hw_id;
        u32 valid_idx = VALID_0 + hw_id;
        u32 valid;
        int ret;

drivers/thermal/qcom/tsens.c +1 -2

-       return 0;
+       return ret;
 }
 
 int get_temp_common(const struct tsens_sensor *s, int *temp)
--

2.27.0

VER_0 is used to describe device based on tsens version before v0.1. These device are devices based on msm8960 for example apq8064 or ipq806x.

Signed-off-by Ansuel Smith <[email protected]>

---
 drivers/thermal/qcom/tsens.c | 122 +++++++++++++++++++++++++++++++----
 drivers/thermal/qcom/tsens.h |   7 +-
 2 files changed, 114 insertions(+), 15 deletions(-)

drivers/thermal/qcom/tsens.c +18 -9

index 9fe9a2b26705..965c4799918a 100644
@@ -516,6 +516,15 @@ static irqreturn_t tsens_irq_thread(int irq, void *data)
                        dev_dbg(priv->dev, "[%u] %s: no violation:  %d\n",
                                hw_id, __func__, temp);
                }
+
+               if (tsens_version(priv) < VER_0_1) {
+                       /* Constraint: There is only 1 interrupt control register for all
+                        * 11 temperature sensor. So monitoring more than 1 sensor based
+                        * on interrupts will yield inconsistent result. To overcome this
+                        * issue we will monitor only sensor 0 which is the master sensor.
+                        */
+                       break;
+               }
        }
 
        return IRQ_HANDLED;

@@ -531,6 +540,13 @@ static int tsens_set_trips(void *_sensor, int low, int high)
        int high_val, low_val, cl_high, cl_low;
        u32 hw_id = s->hw_id;
 
+       if (tsens_version(priv) < VER_0_1) {
+               /* Pre v0.1 IP had a single register for each type of interrupt
+                * and thresholds
+                */
+               hw_id = 0;
+       }
+
        dev_dbg(dev, "[%u] %s: proposed thresholds: (%d:%d)\n",
                hw_id, __func__, low, high);
 

@@ -584,18 +600,21 @@ int get_temp_tsens_valid(const struct tsens_sensor *s, int *temp)
        u32 valid;
        int ret;
 
-       ret = regmap_field_read(priv->rf[valid_idx], &valid);
-       if (ret)
-               return ret;
-       while (!valid) {
-               /* Valid bit is 0 for 6 AHB clock cycles.
-                * At 19.2MHz, 1 AHB clock is ~60ns.
-                * We should enter this loop very, very rarely.
-                */
-               ndelay(400);
+       /* VER_0 doesn't have VALID bit */
+       if (tsens_version(priv) >= VER_0_1) {
                ret = regmap_field_read(priv->rf[valid_idx], &valid);
                if (ret)
                        return ret;

drivers/thermal/qcom/tsens.c +70 -5

        }
 
        /* Valid bit is set, OK to read the temperature */

@@ -763,6 +782,10 @@ int __init init_common(struct tsens_priv *priv)
                goto err_put_device;
        }
 
+       /* VER_0 have only tm_map */
+       if (!priv->srot_map)
+               priv->srot_map = priv->tm_map;
+
        if (tsens_version(priv) > VER_0_1) {
                for (i = VER_MAJOR; i <= VER_STEP; i++) {
                        priv->rf[i] = devm_regmap_field_alloc(dev, priv->srot_map,

@@ -781,6 +804,10 @@ int __init init_common(struct tsens_priv *priv)
                ret = PTR_ERR(priv->rf[TSENS_EN]);
                goto err_put_device;
        }
+       /* in VER_0 TSENS need to be explicitly enabled */
+       if (tsens_version(priv) == VER_0)
+               regmap_field_write(priv->rf[TSENS_EN], 1);
+
        ret = regmap_field_read(priv->rf[TSENS_EN], &enabled);
        if (ret)
                goto err_put_device;

@@ -803,6 +830,61 @@ int __init init_common(struct tsens_priv *priv)
                goto err_put_device;
        }
 
+       priv->rf[TSENS_EN] = devm_regmap_field_alloc(dev, priv->tm_map,
+                                                    priv->fields[TSENS_EN]);
+       if (IS_ERR(priv->rf[TSENS_EN])) {
+               ret = PTR_ERR(priv->rf[TSENS_EN]);
+               goto err_put_device;
+       }
+
+       priv->rf[TSENS_SW_RST] = devm_regmap_field_alloc(
+               dev, priv->tm_map, priv->fields[TSENS_EN]);
+       if (IS_ERR(priv->rf[TSENS_EN])) {
+               ret = PTR_ERR(priv->rf[TSENS_EN]);
+               goto err_put_device;
+       }
+
+       priv->rf[LOW_INT_CLEAR_0] = devm_regmap_field_alloc(
+               dev, priv->tm_map, priv->fields[LOW_INT_CLEAR_0]);
+       if (IS_ERR(priv->rf[LOW_INT_CLEAR_0])) {
+               ret = PTR_ERR(priv->rf[LOW_INT_CLEAR_0]);
+               goto err_put_device;
+       }
+
+       priv->rf[UP_INT_CLEAR_0] = devm_regmap_field_alloc(
+               dev, priv->tm_map, priv->fields[UP_INT_CLEAR_0]);
+       if (IS_ERR(priv->rf[UP_INT_CLEAR_0])) {
+               ret = PTR_ERR(priv->rf[UP_INT_CLEAR_0]);
+               goto err_put_device;
+       }
+
+       /* VER_0 require to set MIN and MAX THRESH */
+       if (tsens_version(priv) < VER_0_1) {
+               priv->rf[MIN_THRESH_0] = devm_regmap_field_alloc(
+                       dev, priv->tm_map, priv->fields[MIN_THRESH_0]);
+               if (IS_ERR(priv->rf[MIN_THRESH_0])) {
+                       ret = PTR_ERR(priv->rf[MIN_THRESH_0]);
+                       goto err_put_device;
+               }
+
+               priv->rf[MAX_THRESH_0] = devm_regmap_field_alloc(
+                       dev, priv->tm_map, priv->fields[MAX_THRESH_0]);
+               if (IS_ERR(priv->rf[MAX_THRESH_0])) {
+                       ret = PTR_ERR(priv->rf[MAX_THRESH_0]);
+                       goto err_put_device;
+               }
+
+               regmap_field_write(priv->rf[MIN_THRESH_0], 0);
+               regmap_field_write(priv->rf[MAX_THRESH_0], 120000);
+       }
+
+       priv->rf[TRDY] =
+               devm_regmap_field_alloc(dev, priv->tm_map, priv->fields[TRDY]);
+       if (IS_ERR(priv->rf[TRDY])) {
+               ret = PTR_ERR(priv->rf[TRDY]);
+               goto err_put_device;
+       }
+
        /* This loop might need changes if enum regfield_ids is reordered */
        for (j = LAST_TEMP_0; j <= UP_THRESH_15; j += 16) {
                for (i = 0; i < priv->feat->max_sensors; i++) {

@@ -856,7 +938,11 @@ int __init init_common(struct tsens_priv *priv)
        }
 
        spin_lock_init(&priv->ul_lock);
-       tsens_enable_irq(priv);
+
+       /* VER_0 interrupt doesn't need to be enabled */
+       if (tsens_version(priv) >= VER_0_1)
+               tsens_enable_irq(priv);
+
        tsens_debug_init(op);
 
 err_put_device:

@@ -952,10 +1038,18 @@ static int tsens_register_irq(struct tsens_priv *priv, char *irqname,
                if (irq == -ENXIO)
                        ret = 0;
        } else {
-               ret = devm_request_threaded_irq(&pdev->dev, irq,
-                                               NULL, thread_fn,
-                                               IRQF_ONESHOT,
-                                               dev_name(&pdev->dev), priv);
+               /* VER_0 interrupt is TRIGGER_RISING, VER_0_1 and up is ONESHOT */
+               if (tsens_version(priv) > VER_0)

drivers/thermal/qcom/tsens.c +0 -0

                if (ret)
                        dev_err(&pdev->dev, "%s: failed to get irq\n",
                                __func__);

drivers/thermal/qcom/tsens.h +2 -1

index 59d01162c66a..f1120791737c 100644
@@ -25,7 +25,8 @@ struct tsens_priv;
 
 /* IP version numbers in ascending order */
 enum tsens_ver {
-       VER_0_1 = 0,
+       VER_0 = 0,
+       VER_0_1,
        VER_1_X,
        VER_2_X,
 };

drivers/thermal/qcom/tsens.h +0 -1

        /* WATCHDOG */
        WDOG_BARK_STATUS,
        WDOG_BARK_CLEAR,
--

2.27.0

Hi Ansuel,

See comments inline.

Quoted 63 lines · Ansuel Smith

VER_0 is used to describe device based on tsens version before v0.1. These device are devices based on msm8960 for example apq8064 or ipq806x.

Signed-off-by Ansuel Smith <[email protected]>

---
 drivers/thermal/qcom/tsens.c | 122 +++++++++++++++++++++++++++++++----
 drivers/thermal/qcom/tsens.h |   7 +-
 2 files changed, 114 insertions(+), 15 deletions(-)

drivers/thermal/qcom/tsens.c +18 -9

index 9fe9a2b26705..965c4799918a 100644
@@ -516,6 +516,15 @@ static irqreturn_t tsens_irq_thread(int irq, void *data)
                        dev_dbg(priv->dev, "[%u] %s: no violation:  %d\n",
                                hw_id, __func__, temp);
                }
+
+               if (tsens_version(priv) < VER_0_1) {
+                       /* Constraint: There is only 1 interrupt control register for all
+                        * 11 temperature sensor. So monitoring more than 1 sensor based
+                        * on interrupts will yield inconsistent result. To overcome this
+                        * issue we will monitor only sensor 0 which is the master sensor.
+                        */
+                       break;
+               }
        }
 
        return IRQ_HANDLED;

@@ -531,6 +540,13 @@ static int tsens_set_trips(void *_sensor, int low, int high)
        int high_val, low_val, cl_high, cl_low;
        u32 hw_id = s->hw_id;
 
+       if (tsens_version(priv) < VER_0_1) {
+               /* Pre v0.1 IP had a single register for each type of interrupt
+                * and thresholds
+                */
+               hw_id = 0;
+       }
+
        dev_dbg(dev, "[%u] %s: proposed thresholds: (%d:%d)\n",
                hw_id, __func__, low, high);
 

@@ -584,18 +600,21 @@ int get_temp_tsens_valid(const struct tsens_sensor *s, int *temp)
        u32 valid;
        int ret;
 
-       ret = regmap_field_read(priv->rf[valid_idx], &valid);
-       if (ret)
-               return ret;
-       while (!valid) {
-               /* Valid bit is 0 for 6 AHB clock cycles.
-                * At 19.2MHz, 1 AHB clock is ~60ns.
-                * We should enter this loop very, very rarely.
-                */
-               ndelay(400);
+       /* VER_0 doesn't have VALID bit */
+       if (tsens_version(priv) >= VER_0_1) {
                ret = regmap_field_read(priv->rf[valid_idx], &valid);
                if (ret)
                        return ret;

drivers/thermal/qcom/tsens.c int get_temp_tsens_valid(const struct tsens_sensor *s, int *temp) · line 617

Quoted 10 lines

drivers/thermal/qcom/tsens.c +10 -0

+               while (!valid) {
+                       /* Valid bit is 0 for 6 AHB clock cycles.
+                        * At 19.2MHz, 1 AHB clock is ~60ns.
+                        * We should enter this loop very, very rarely.
+                        */
+                       ndelay(400);
+                       ret = regmap_field_read(priv->rf[valid_idx], &valid);
+                       if (ret)
+                               return ret;
+               }

Let's revisit this after fixing patch 1.

Quoted 110 lines

drivers/thermal/qcom/tsens.c +70 -5

        }
 
        /* Valid bit is set, OK to read the temperature */

@@ -763,6 +782,10 @@ int __init init_common(struct tsens_priv *priv)
                goto err_put_device;
        }
 
+       /* VER_0 have only tm_map */
+       if (!priv->srot_map)
+               priv->srot_map = priv->tm_map;
+
        if (tsens_version(priv) > VER_0_1) {
                for (i = VER_MAJOR; i <= VER_STEP; i++) {
                        priv->rf[i] = devm_regmap_field_alloc(dev, priv->srot_map,

@@ -781,6 +804,10 @@ int __init init_common(struct tsens_priv *priv)
                ret = PTR_ERR(priv->rf[TSENS_EN]);
                goto err_put_device;
        }
+       /* in VER_0 TSENS need to be explicitly enabled */
+       if (tsens_version(priv) == VER_0)
+               regmap_field_write(priv->rf[TSENS_EN], 1);
+
        ret = regmap_field_read(priv->rf[TSENS_EN], &enabled);
        if (ret)
                goto err_put_device;

@@ -803,6 +830,61 @@ int __init init_common(struct tsens_priv *priv)
                goto err_put_device;
        }
 
+       priv->rf[TSENS_EN] = devm_regmap_field_alloc(dev, priv->tm_map,
+                                                    priv->fields[TSENS_EN]);
+       if (IS_ERR(priv->rf[TSENS_EN])) {
+               ret = PTR_ERR(priv->rf[TSENS_EN]);
+               goto err_put_device;
+       }
+
+       priv->rf[TSENS_SW_RST] = devm_regmap_field_alloc(
+               dev, priv->tm_map, priv->fields[TSENS_EN]);
+       if (IS_ERR(priv->rf[TSENS_EN])) {
+               ret = PTR_ERR(priv->rf[TSENS_EN]);
+               goto err_put_device;
+       }
+
+       priv->rf[LOW_INT_CLEAR_0] = devm_regmap_field_alloc(
+               dev, priv->tm_map, priv->fields[LOW_INT_CLEAR_0]);
+       if (IS_ERR(priv->rf[LOW_INT_CLEAR_0])) {
+               ret = PTR_ERR(priv->rf[LOW_INT_CLEAR_0]);
+               goto err_put_device;
+       }
+
+       priv->rf[UP_INT_CLEAR_0] = devm_regmap_field_alloc(
+               dev, priv->tm_map, priv->fields[UP_INT_CLEAR_0]);
+       if (IS_ERR(priv->rf[UP_INT_CLEAR_0])) {
+               ret = PTR_ERR(priv->rf[UP_INT_CLEAR_0]);
+               goto err_put_device;
+       }
+
+       /* VER_0 require to set MIN and MAX THRESH */
+       if (tsens_version(priv) < VER_0_1) {
+               priv->rf[MIN_THRESH_0] = devm_regmap_field_alloc(
+                       dev, priv->tm_map, priv->fields[MIN_THRESH_0]);
+               if (IS_ERR(priv->rf[MIN_THRESH_0])) {
+                       ret = PTR_ERR(priv->rf[MIN_THRESH_0]);
+                       goto err_put_device;
+               }
+
+               priv->rf[MAX_THRESH_0] = devm_regmap_field_alloc(
+                       dev, priv->tm_map, priv->fields[MAX_THRESH_0]);
+               if (IS_ERR(priv->rf[MAX_THRESH_0])) {
+                       ret = PTR_ERR(priv->rf[MAX_THRESH_0]);
+                       goto err_put_device;
+               }
+
+               regmap_field_write(priv->rf[MIN_THRESH_0], 0);
+               regmap_field_write(priv->rf[MAX_THRESH_0], 120000);
+       }
+
+       priv->rf[TRDY] =
+               devm_regmap_field_alloc(dev, priv->tm_map, priv->fields[TRDY]);
+       if (IS_ERR(priv->rf[TRDY])) {
+               ret = PTR_ERR(priv->rf[TRDY]);
+               goto err_put_device;
+       }
+
        /* This loop might need changes if enum regfield_ids is reordered */
        for (j = LAST_TEMP_0; j <= UP_THRESH_15; j += 16) {
                for (i = 0; i < priv->feat->max_sensors; i++) {

@@ -856,7 +938,11 @@ int __init init_common(struct tsens_priv *priv)
        }
 
        spin_lock_init(&priv->ul_lock);
-       tsens_enable_irq(priv);
+
+       /* VER_0 interrupt doesn't need to be enabled */
+       if (tsens_version(priv) >= VER_0_1)
+               tsens_enable_irq(priv);
+
        tsens_debug_init(op);
 
 err_put_device:

@@ -952,10 +1038,18 @@ static int tsens_register_irq(struct tsens_priv *priv, char *irqname,
                if (irq == -ENXIO)
                        ret = 0;
        } else {
-               ret = devm_request_threaded_irq(&pdev->dev, irq,
-                                               NULL, thread_fn,
-                                               IRQF_ONESHOT,
-                                               dev_name(&pdev->dev), priv);
+               /* VER_0 interrupt is TRIGGER_RISING, VER_0_1 and up is ONESHOT */
+               if (tsens_version(priv) > VER_0)

drivers/thermal/qcom/tsens.c static int tsens_register_irq(struct tsens_priv *priv, char *irqname, · line 1052

Quoted 10 lines

drivers/thermal/qcom/tsens.c +10 -0

+                       ret = devm_request_threaded_irq(&pdev->dev, irq, NULL,
+                                                       thread_fn, IRQF_ONESHOT,
+                                                       dev_name(&pdev->dev),
+                                                       priv);
+               else
+                       ret = devm_request_threaded_irq(&pdev->dev, irq,
+                                                       thread_fn, NULL,
+                                                       IRQF_TRIGGER_RISING,
+                                                       dev_name(&pdev->dev),
+                                                       priv);

Just set a flag variable to ONESHOT OR TRIGGER_RISING and use that in the call.

Quoted 21 lines

drivers/thermal/qcom/tsens.c +0 -0

                if (ret)
                        dev_err(&pdev->dev, "%s: failed to get irq\n",
                                __func__);

drivers/thermal/qcom/tsens.h +2 -1

index 59d01162c66a..f1120791737c 100644
@@ -25,7 +25,8 @@ struct tsens_priv;
 
 /* IP version numbers in ascending order */
 enum tsens_ver {
-       VER_0_1 = 0,
+       VER_0 = 0,
+       VER_0_1,
        VER_1_X,
        VER_2_X,
 };

@@ -441,6 +442,10 @@ enum regfield_ids {
        CRIT_THRESH_14,
        CRIT_THRESH_15,
 

drivers/thermal/qcom/tsens.h enum regfield_ids { · line 448

Quoted 4 lines

drivers/thermal/qcom/tsens.h +4 -0

+       /* VER_0 MIN MAX THRESH */
+       MIN_THRESH_0,
+       MAX_THRESH_0,
+

Consider reusing LOW_THRESH_0 and UP_THRESH_0 for these?

Quoted 6 lines

drivers/thermal/qcom/tsens.h +0 -1

        /* WATCHDOG */
        WDOG_BARK_STATUS,
        WDOG_BARK_CLEAR,
--

2.27.0

Quoted 5 lines

Quoted 5 lines

};

drivers/thermal/qcom/tsens.h +0 -0

@@ -441,6 +442,10 @@ enum regfield_ids {
        CRIT_THRESH_14,
        CRIT_THRESH_15,
 

Quoted 7 lines

drivers/thermal/qcom/tsens.h enum regfield_ids { · line 448

Quoted 4 lines

drivers/thermal/qcom/tsens.h +4 -0

+       /* VER_0 MIN MAX THRESH */
+       MIN_THRESH_0,
+       MAX_THRESH_0,
+

Consider reusing LOW_THRESH_0 and UP_THRESH_0 for these?

As we already have defined LOW_THRESH and UP how can we reuse that regfield to define MIN and MAX?

Quoted 13 lines · Ansuel Smith

Quoted 5 lines

Quoted 5 lines

};

drivers/thermal/qcom/tsens.h +0 -0

@@ -441,6 +442,10 @@ enum regfield_ids {
        CRIT_THRESH_14,
        CRIT_THRESH_15,
 

Quoted 7 lines

drivers/thermal/qcom/tsens.h enum regfield_ids { · line 448

Quoted 4 lines

drivers/thermal/qcom/tsens.h +4 -0

+       /* VER_0 MIN MAX THRESH */
+       MIN_THRESH_0,
+       MAX_THRESH_0,
+

Consider reusing LOW_THRESH_0 and UP_THRESH_0 for these?

Quoted 3 lines

As we already have defined LOW_THRESH and UP how can we reuse that regfield to define MIN and MAX?

We are using MIN and MAX THRESH on the apq8064 to mean LOW and UP THRESOLD, isn't it? IIUC, It was just named differently earlier.

When the driver is loaded on the apq8064, only that one field will be use since v0 has a single threshold for all sensors. When the driver is loaded on new IPs, all fields will be used.

This patchset convert msm8960 to reg_filed, use int_common instead of a custom function and fix wrong tsens get_temp function for msm8960. Ipq8064 SoCs tsens driver is based on 8960 tsens driver. Ipq8064 needs to be registered as a gcc child as the tsens regs on this platform are shared with the controller.
This is based on work and code here
https://git.linaro.org/people/amit.kucheria/kernel.git/log/?h=wrk3/tsens-8960-breakage

v6:
* Fix spelling error (can't find the problem with variable misallignment)
* Rework big if-else
* Remove extra comments
* Add description about different interrupts
v5:
* Conver driver to use reg_fiedl
* Use init_common
* Drop custom set_trip and set_interrupt
* Use common set_trip and set_interrupt
* Fix bad get_temp function
* Add missing hardcoded slope
v4:
* Fix compilation error and warning reported by the bot
v3:
* Change driver to register as child instead of use phandle v2:
* Fix dt-bindings problems

Ansuel Smith (8):
drivers: thermal: tsens: use get_temp for tsens_valid
drivers: thermal: tsens: Add VER_0 tsens version
drivers: thermal: tsens: Convert msm8960 to reg_field
drivers: thermal: tsens: Use init_common for msm8960
drivers: thermal: tsens: Fix wrong get_temp for msm8960
drivers: thermal: tsens: Change calib_backup name for msm8960
drivers: thermal: tsens: Add support for ipq8064-tsens
dt-bindings: thermal: tsens: Document ipq8064 bindings

 .../bindings/thermal/qcom-tsens.yaml          |  50 ++++-
 drivers/thermal/qcom/tsens-8960.c             | 172 +++++++++++-------
 drivers/thermal/qcom/tsens.c                  | 130 +++++++++++--
 drivers/thermal/qcom/tsens.h                  |   7 +-
 4 files changed, 270 insertions(+), 89 deletions(-)

--
2.27.0

-----Original Message-----
From: Amit Kucheria <[email protected]>
Sent: Monday, September 28, 2020 1:33 PM
To: Ansuel Smith <[email protected]>
Cc: Andy Gross <[email protected]>; Bjorn Andersson <[email protected]>; Zhang Rui <[email protected]>; Daniel Lezcano <[email protected]>; Rob Herring <[email protected]>; linux-arm-msm <[email protected]>; Linux PM list <linux- [email protected]>; open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS <[email protected]>; LKML <linux- [email protected]>
Subject: Re: [RFC PATCH v6 0/8] Add support for ipq8064 tsens

Hi Ansuel,

Just a quick note to say that I'm not ignoring this, just on vacations. I'll be back to review this in a few days.

Regards,
Amit