Re: [PATCH v11 5/9] drivers: thermal: tsens: Fix bug in sensor enable for msm8960

[Thara Gopinath]

Hi!

On 3/18/21 8:52 PM, Ansuel Smith wrote:

Device based on tsens VER_0 contains a hardware bug that results in some
problem with sensor enablement. Sensor id 6-11 can't be enabled
selectively and all of them must be enabled in one step.

Thanks for rewording!

Signed-off-by: Ansuel Smith <ansuels...@gmail.com>
---
  drivers/thermal/qcom/tsens-8960.c | 24 +++++++++++++++++++++---
  1 file changed, 21 insertions(+), 3 deletions(-)

diff --git a/drivers/thermal/qcom/tsens-8960.c 
b/drivers/thermal/qcom/tsens-8960.c
index 86585f439985..bdc64d4188bf 100644
--- a/drivers/thermal/qcom/tsens-8960.c
+++ b/drivers/thermal/qcom/tsens-8960.c
@@ -27,9 +27,9 @@
  #define EN                    BIT(0)
  #define SW_RST                        BIT(1)
  #define SENSOR0_EN            BIT(3)
+#define MEASURE_PERIOD         BIT(18)
  #define SLP_CLK_ENA           BIT(26)
  #define SLP_CLK_ENA_8660      BIT(24)
-#define MEASURE_PERIOD         1
  #define SENSOR0_SHIFT         3

/* INT_STATUS_ADDR bitmasks */

@@ -126,17 +126,35 @@ static int resume_8960(struct tsens_priv *priv)
  static int enable_8960(struct tsens_priv *priv, int id)
  {
        int ret;
-       u32 reg, mask;
+       u32 reg, mask = BIT(id);

ret = regmap_read(priv->tm_map, CNTL_ADDR, &reg);

        if (ret)
                return ret;

- mask = BIT(id + SENSOR0_SHIFT);

+       /* HARDWARE BUG:
+        * On platform with more than 6 sensors, all the remaining
+        * sensors needs to be enabled all togheder or underfined
+        * results are expected. (Sensor 6-7 disabled, Sensor 3
+        * disabled...) In the original driver, all the sensors
+        * are enabled in one step hence this bug is not triggered.
+        */
+       if (id > 5) {
+               mask = GENMASK(10, 6);
+
+               /* Sensors already enabled. Skip. */
+               if ((reg & mask) == mask)

This is a bug. You have to do mask <<= SENSOR0_SHIFT; before reg & mask.

+                       return 0;
+       }
+
+       mask <<= SENSOR0_SHIFT;
+
        ret = regmap_write(priv->tm_map, CNTL_ADDR, reg | SW_RST);
        if (ret)
                return ret;

+ reg |= MEASURE_PERIOD;

+
        if (priv->num_sensors > 1)
                reg |= mask | SLP_CLK_ENA | EN;
        else

--
Warm Regards
Thara