On 2/17/21 2:40 PM, Ansuel Smith wrote:
It's present a hardware bug in tsens VER_0 where if sensors upper to id
6 are enabled selectively, underfined results are expected. Fix this by
enabling all the remaining sensor in one step.
It took me a while to understand this. It is most likely me! But please
consider rewording.
Signed-off-by: Ansuel Smith <ansuelsmth@xxxxxxxxx>
---
drivers/thermal/qcom/tsens-8960.c | 19 +++++++++++++++++--
1 file changed, 17 insertions(+), 2 deletions(-)
diff --git a/drivers/thermal/qcom/tsens-8960.c b/drivers/thermal/qcom/tsens-8960.c
index 86585f439985..248aaa65b5b0 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 */
@@ -132,11 +132,26 @@ static int enable_8960(struct tsens_priv *priv, int id)
if (ret)
return ret;
- mask = BIT(id + SENSOR0_SHIFT);
+ /* HARDWARE BUG:
+ * On platform with more than 5 sensors, all the remaining
Isn't it 6 ? At least according to code below it is.. You are checking
for id > 5.
+ * 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.
Also with this change, you should add a check in this function to see if
the sensors are already enabled and if yes return back. The enabling
call from tsens.c happens for every single sensor. But at sensor number
6 you are enabling rest of the sensors. There is absolutely no reason to
keep doing this for rest of the sensors.
+ */
+ if (id > 5)
+ mask = GENMASK(10, 6);
+ else
+ mask = BIT(id);
+
+ mask <<= SENSOR0_SHIFT;
+
ret = regmap_write(priv->tm_map, CNTL_ADDR, reg | SW_RST);
I know this is not part of this patch. But you mention above that
earlier you were enabling all sensors one shot. Now that this is being
done one at a time, is it needed to do a SW_RST every time ?
if (ret)
return ret;
+ reg |= MEASURE_PERIOD;
+
if (priv->num_sensors > 1)
reg |= mask | SLP_CLK_ENA | EN;
else
--
Warm Regards
Thara
