[RFC PATCH 00/10] thermal/drivers/tsens: specify nvmem cells in DT rather than parsing them manually

[RFC PATCH 00/10] thermal/drivers/tsens: specify nvmem cells in DT rather than parsing them manually

[Dmitry Baryshkov]

Historically the tsens driver fetches the calibration data as a blob and
then parses the blob on its own. This results in semi-duplicated code
spreading over the platform-specific functions.

This patch series changes tsens calibration code to use pre-parsed nvmem
cells rather than parsing the blob in the driver. For backwards
compatibility the old code is left in place for msm8916 and qcs404, two
platforms which have in-tree DT files. For msm8974 the original function
is left intact, since it differs significantly (and I can not test the
code on msm8974). For all other affected platforms the old parsing code
has been dropped as a part of this RFC.

The code was tested on msm8916 and qcs404 only, thus it is being sent as
an RFC.

Dmitry Baryshkov (10):
  dt-bindings: thermal: tsens: support per-sensor calibration cells
  thermal/drivers/tsens: Support using nvmem cells for calibration data
  thermal/drivers/tsens: drop single-cell code for msm8939
  thermal/drivers/tsens: drop single-cell code for mdm9607
  thermal/drivers/tsens: drop msm8976-specific defines
  thermal/drivers/tsens: use generic calibration routine for msm8976
  thermal/drivers/tsens: use tsens_calibrate_nvmem for msm8976
    calibration
  thermal/drivers/tsens: drop single-cell code for msm8976
  arm64: dts: qcom: msm8916: specify per-sensor calibration cells
  arm64: dts: qcom: qcs404: specify per-sensor calibration cells

 .../bindings/thermal/qcom-tsens.yaml          |  64 +++++-
 arch/arm64/boot/dts/qcom/msm8916.dtsi         |  70 +++++-
 arch/arm64/boot/dts/qcom/qcs404.dtsi          | 120 +++++++++-
 drivers/thermal/qcom/tsens-v0_1.c             | 208 +-----------------
 drivers/thermal/qcom/tsens-v1.c               | 154 +------------
 drivers/thermal/qcom/tsens.c                  |  62 ++++++
 drivers/thermal/qcom/tsens.h                  |   4 +
 7 files changed, 314 insertions(+), 368 deletions(-)

-- 
2.35.1

[RFC PATCH 01/10] dt-bindings: thermal: tsens: support per-sensor calibration cells

[Dmitry Baryshkov]

Allow specifing the exact calibration mode and calibration data as nvmem
cells, rather than specifying just a single calibration data blob.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 .../bindings/thermal/qcom-tsens.yaml          | 64 ++++++++++++++++---
 1 file changed, 54 insertions(+), 10 deletions(-)

diff --git a/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml b/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml
index 038d81338fcf..b813f6f19c1d 100644
--- a/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml
+++ b/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml
@@ -77,18 +77,62 @@ properties:
       - const: critical
 
   nvmem-cells:
-    minItems: 1
-    maxItems: 2
-    description:
-      Reference to an nvmem node for the calibration data
+    oneOf:
+      - minItems: 1
+        maxItems: 2
+        description:
+          Reference to an nvmem node for the calibration data
+      - minItems: 5
+        maxItems: 35
+        description: |
+          Reference to an nvmem cells for the calibration mode, two calibration
+          bases and two cells per each sensor
 
   nvmem-cell-names:
-    minItems: 1
-    items:
-      - const: calib
-      - enum:
-          - calib_backup
-          - calib_sel
+    oneOf:
+      - minItems: 1
+        items:
+          - const: calib
+          - enum:
+              - calib_backup
+              - calib_sel
+      - minItems: 5
+        items:
+          - const: mode
+          - const: base1
+          - const: base2
+          - const: s0_p1
+          - const: s0_p2
+          - const: s1_p1
+          - const: s1_p2
+          - const: s2_p1
+          - const: s2_p2
+          - const: s3_p1
+          - const: s3_p2
+          - const: s4_p1
+          - const: s4_p2
+          - const: s5_p1
+          - const: s5_p2
+          - const: s6_p1
+          - const: s6_p2
+          - const: s7_p1
+          - const: s7_p2
+          - const: s8_p1
+          - const: s8_p2
+          - const: s9_p1
+          - const: s9_p2
+          - const: s10_p1
+          - const: s10_p2
+          - const: s11_p1
+          - const: s11_p2
+          - const: s12_p1
+          - const: s12_p2
+          - const: s13_p1
+          - const: s13_p2
+          - const: s14_p1
+          - const: s14_p2
+          - const: s15_p1
+          - const: s15_p2
 
   "#qcom,sensors":
     description:
-- 
2.35.1

Re: [RFC PATCH 01/10] dt-bindings: thermal: tsens: support per-sensor calibration cells

[Krzysztof Kozlowski]

On 10/09/2022 14:46, Dmitry Baryshkov wrote:
> Allow specifing the exact calibration mode and calibration data as nvmem
> cells, rather than specifying just a single calibration data blob.
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
>  .../bindings/thermal/qcom-tsens.yaml          | 64 ++++++++++++++++---
>  1 file changed, 54 insertions(+), 10 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml b/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml
> index 038d81338fcf..b813f6f19c1d 100644
> --- a/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml
> +++ b/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml
> @@ -77,18 +77,62 @@ properties:
>        - const: critical
>  
>    nvmem-cells:
> -    minItems: 1
> -    maxItems: 2
> -    description:
> -      Reference to an nvmem node for the calibration data
> +    oneOf:
> +      - minItems: 1
> +        maxItems: 2
> +        description:
> +          Reference to an nvmem node for the calibration data
> +      - minItems: 5
> +        maxItems: 35
> +        description: |
> +          Reference to an nvmem cells for the calibration mode, two calibration

s/an nvmem/nvmem/

Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>


Best regards,
Krzysztof

Re: [RFC PATCH 01/10] dt-bindings: thermal: tsens: support per-sensor calibration cells

[Amit Kucheria]

On Sat, Sep 10, 2022 at 6:17 PM Dmitry Baryshkov
<dmitry.baryshkov@linaro.org> wrote:
>
> Allow specifing the exact calibration mode and calibration data as nvmem

typo: specifying

> cells, rather than specifying just a single calibration data blob.
>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

Reviewed-by: Amit Kucheria <amitk@kernel.org>


> ---
>  .../bindings/thermal/qcom-tsens.yaml          | 64 ++++++++++++++++---
>  1 file changed, 54 insertions(+), 10 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml b/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml
> index 038d81338fcf..b813f6f19c1d 100644
> --- a/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml
> +++ b/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml
> @@ -77,18 +77,62 @@ properties:
>        - const: critical
>
>    nvmem-cells:
> -    minItems: 1
> -    maxItems: 2
> -    description:
> -      Reference to an nvmem node for the calibration data
> +    oneOf:
> +      - minItems: 1
> +        maxItems: 2
> +        description:
> +          Reference to an nvmem node for the calibration data
> +      - minItems: 5
> +        maxItems: 35
> +        description: |
> +          Reference to an nvmem cells for the calibration mode, two calibration
> +          bases and two cells per each sensor
>
>    nvmem-cell-names:
> -    minItems: 1
> -    items:
> -      - const: calib
> -      - enum:
> -          - calib_backup
> -          - calib_sel
> +    oneOf:
> +      - minItems: 1
> +        items:
> +          - const: calib
> +          - enum:
> +              - calib_backup
> +              - calib_sel
> +      - minItems: 5
> +        items:
> +          - const: mode
> +          - const: base1
> +          - const: base2
> +          - const: s0_p1
> +          - const: s0_p2
> +          - const: s1_p1
> +          - const: s1_p2
> +          - const: s2_p1
> +          - const: s2_p2
> +          - const: s3_p1
> +          - const: s3_p2
> +          - const: s4_p1
> +          - const: s4_p2
> +          - const: s5_p1
> +          - const: s5_p2
> +          - const: s6_p1
> +          - const: s6_p2
> +          - const: s7_p1
> +          - const: s7_p2
> +          - const: s8_p1
> +          - const: s8_p2
> +          - const: s9_p1
> +          - const: s9_p2
> +          - const: s10_p1
> +          - const: s10_p2
> +          - const: s11_p1
> +          - const: s11_p2
> +          - const: s12_p1
> +          - const: s12_p2
> +          - const: s13_p1
> +          - const: s13_p2
> +          - const: s14_p1
> +          - const: s14_p2
> +          - const: s15_p1
> +          - const: s15_p2
>
>    "#qcom,sensors":
>      description:
> --
> 2.35.1
>

[RFC PATCH 02/10] thermal/drivers/tsens: Support using nvmem cells for calibration data

[Dmitry Baryshkov]

Add a unified function using nvmem cells for parsing the calibration
data rather than parsing the calibration blob manually.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/thermal/qcom/tsens-v0_1.c | 15 ++++++++
 drivers/thermal/qcom/tsens-v1.c   |  6 ++-
 drivers/thermal/qcom/tsens.c      | 62 +++++++++++++++++++++++++++++++
 drivers/thermal/qcom/tsens.h      |  4 ++
 4 files changed, 86 insertions(+), 1 deletion(-)

diff --git a/drivers/thermal/qcom/tsens-v0_1.c b/drivers/thermal/qcom/tsens-v0_1.c
index f136cb350238..2974eea578f4 100644
--- a/drivers/thermal/qcom/tsens-v0_1.c
+++ b/drivers/thermal/qcom/tsens-v0_1.c
@@ -229,6 +229,11 @@ static int calibrate_8916(struct tsens_priv *priv)
 	u32 p1[5], p2[5];
 	int mode = 0;
 	u32 *qfprom_cdata, *qfprom_csel;
+	int ret;
+
+	ret = tsens_calibrate_nvmem(priv, 3);
+	if (!ret)
+		return 0;
 
 	qfprom_cdata = (u32 *)qfprom_read(priv->dev, "calib");
 	if (IS_ERR(qfprom_cdata))
@@ -286,6 +291,11 @@ static int calibrate_8939(struct tsens_priv *priv)
 	int mode = 0;
 	u32 *qfprom_cdata;
 	u32 cdata[6];
+	int ret;
+
+	ret = tsens_calibrate_nvmem(priv, 2);
+	if (!ret)
+		return 0;
 
 	qfprom_cdata = (u32 *)qfprom_read(priv->dev, "calib");
 	if (IS_ERR(qfprom_cdata))
@@ -491,6 +501,11 @@ static int calibrate_9607(struct tsens_priv *priv)
 	u32 p1[5], p2[5];
 	int mode = 0;
 	u32 *qfprom_cdata;
+	int ret;
+
+	ret = tsens_calibrate_nvmem(priv, 2);
+	if (!ret)
+		return 0;
 
 	qfprom_cdata = (u32 *)qfprom_read(priv->dev, "calib");
 	if (IS_ERR(qfprom_cdata))
diff --git a/drivers/thermal/qcom/tsens-v1.c b/drivers/thermal/qcom/tsens-v1.c
index 573e261ccca7..868d7b4c9e36 100644
--- a/drivers/thermal/qcom/tsens-v1.c
+++ b/drivers/thermal/qcom/tsens-v1.c
@@ -172,7 +172,11 @@ static int calibrate_v1(struct tsens_priv *priv)
 	u32 p1[10], p2[10];
 	u32 mode = 0, lsb = 0, msb = 0;
 	u32 *qfprom_cdata;
-	int i;
+	int i, ret;
+
+	ret = tsens_calibrate_nvmem(priv, 2);
+	if (!ret)
+		return 0;
 
 	qfprom_cdata = (u32 *)qfprom_read(priv->dev, "calib");
 	if (IS_ERR(qfprom_cdata))
diff --git a/drivers/thermal/qcom/tsens.c b/drivers/thermal/qcom/tsens.c
index e49f58e83513..8331b924325a 100644
--- a/drivers/thermal/qcom/tsens.c
+++ b/drivers/thermal/qcom/tsens.c
@@ -70,6 +70,68 @@ char *qfprom_read(struct device *dev, const char *cname)
 	return ret;
 }
 
+int tsens_calibrate_nvmem(struct tsens_priv *priv, int shift)
+{
+	u32 mode;
+	u32 base1, base2;
+	u32 p1[MAX_SENSORS], p2[MAX_SENSORS];
+	char name[] = "sX_pY";
+	int i, ret;
+
+	if (priv->num_sensors > MAX_SENSORS)
+		return -EINVAL;
+
+	ret = nvmem_cell_read_variable_le_u32(priv->dev, "mode", &mode);
+	if (ret == -ENOENT)
+		dev_warn(priv->dev, "Please migrate to sepate nvmem cells for calibration data\n");
+	if (ret < 0)
+		return ret;
+
+	dev_dbg(priv->dev, "calibration mode is %d\n", mode);
+
+	ret = nvmem_cell_read_variable_le_u32(priv->dev, "base1", &base1);
+	if (ret < 0)
+		return ret;
+
+	ret = nvmem_cell_read_variable_le_u32(priv->dev, "base2", &base2);
+	if (ret < 0)
+		return ret;
+
+	for (i = 0; i < priv->num_sensors; i++) {
+		ret = snprintf(name, sizeof(name), "s%d_p1", i);
+		if (ret < 0)
+			return ret;
+
+		ret = nvmem_cell_read_variable_le_u32(priv->dev, name, &p1[i]);
+		if (ret)
+			return ret;
+
+		p1[i] = (base1 + p1[i]) << shift;
+
+		ret = snprintf(name, sizeof(name), "s%d_p2", i);
+		if (ret < 0)
+			return ret;
+
+		ret = nvmem_cell_read_variable_le_u32(priv->dev, name, &p2[i]);
+		if (ret)
+			return ret;
+
+		p2[i] = (base2 + p2[i]) << shift;
+	}
+
+	if (mode == NO_PT_CALIB) {
+		dev_dbg(priv->dev, "calibrationless mode\n");
+		for (i = 0; i < priv->num_sensors; i++) {
+			p1[i] = 500;
+			p2[i] = 780;
+		}
+	}
+
+	compute_intercept_slope(priv, p1, p2, mode);
+
+	return 0;
+}
+
 /*
  * Use this function on devices where slope and offset calculations
  * depend on calibration data read from qfprom. On others the slope
diff --git a/drivers/thermal/qcom/tsens.h b/drivers/thermal/qcom/tsens.h
index ba05c8233356..504ed3394a79 100644
--- a/drivers/thermal/qcom/tsens.h
+++ b/drivers/thermal/qcom/tsens.h
@@ -6,6 +6,7 @@
 #ifndef __QCOM_TSENS_H__
 #define __QCOM_TSENS_H__
 
+#define NO_PT_CALIB		0x0
 #define ONE_PT_CALIB		0x1
 #define ONE_PT_CALIB2		0x2
 #define TWO_PT_CALIB		0x3
@@ -17,6 +18,8 @@
 #define THRESHOLD_MAX_ADC_CODE	0x3ff
 #define THRESHOLD_MIN_ADC_CODE	0x0
 
+#define MAX_SENSORS 16
+
 #include <linux/interrupt.h>
 #include <linux/thermal.h>
 #include <linux/regmap.h>
@@ -576,6 +579,7 @@ struct tsens_priv {
 };
 
 char *qfprom_read(struct device *dev, const char *cname);
+int tsens_calibrate_nvmem(struct tsens_priv *priv, int shift);
 void compute_intercept_slope(struct tsens_priv *priv, u32 *pt1, u32 *pt2, u32 mode);
 int init_common(struct tsens_priv *priv);
 int get_temp_tsens_valid(const struct tsens_sensor *s, int *temp);
-- 
2.35.1

Re: [RFC PATCH 02/10] thermal/drivers/tsens: Support using nvmem cells for calibration data

[Amit Kucheria]

Hi Dmitry,

I like this clean up.

On Sat, Sep 10, 2022 at 6:17 PM Dmitry Baryshkov
<dmitry.baryshkov@linaro.org> wrote:
>
> Add a unified function using nvmem cells for parsing the calibration
> data rather than parsing the calibration blob manually.
>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
>  drivers/thermal/qcom/tsens-v0_1.c | 15 ++++++++
>  drivers/thermal/qcom/tsens-v1.c   |  6 ++-
>  drivers/thermal/qcom/tsens.c      | 62 +++++++++++++++++++++++++++++++
>  drivers/thermal/qcom/tsens.h      |  4 ++
>  4 files changed, 86 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/thermal/qcom/tsens-v0_1.c b/drivers/thermal/qcom/tsens-v0_1.c
> index f136cb350238..2974eea578f4 100644
> --- a/drivers/thermal/qcom/tsens-v0_1.c
> +++ b/drivers/thermal/qcom/tsens-v0_1.c
> @@ -229,6 +229,11 @@ static int calibrate_8916(struct tsens_priv *priv)
>         u32 p1[5], p2[5];
>         int mode = 0;
>         u32 *qfprom_cdata, *qfprom_csel;
> +       int ret;
> +
> +       ret = tsens_calibrate_nvmem(priv, 3);
> +       if (!ret)
> +               return 0;
>
>         qfprom_cdata = (u32 *)qfprom_read(priv->dev, "calib");
>         if (IS_ERR(qfprom_cdata))
> @@ -286,6 +291,11 @@ static int calibrate_8939(struct tsens_priv *priv)
>         int mode = 0;
>         u32 *qfprom_cdata;
>         u32 cdata[6];
> +       int ret;
> +
> +       ret = tsens_calibrate_nvmem(priv, 2);
> +       if (!ret)
> +               return 0;
>
>         qfprom_cdata = (u32 *)qfprom_read(priv->dev, "calib");
>         if (IS_ERR(qfprom_cdata))
> @@ -491,6 +501,11 @@ static int calibrate_9607(struct tsens_priv *priv)
>         u32 p1[5], p2[5];
>         int mode = 0;
>         u32 *qfprom_cdata;
> +       int ret;
> +
> +       ret = tsens_calibrate_nvmem(priv, 2);
> +       if (!ret)
> +               return 0;
>
>         qfprom_cdata = (u32 *)qfprom_read(priv->dev, "calib");
>         if (IS_ERR(qfprom_cdata))
> diff --git a/drivers/thermal/qcom/tsens-v1.c b/drivers/thermal/qcom/tsens-v1.c
> index 573e261ccca7..868d7b4c9e36 100644
> --- a/drivers/thermal/qcom/tsens-v1.c
> +++ b/drivers/thermal/qcom/tsens-v1.c
> @@ -172,7 +172,11 @@ static int calibrate_v1(struct tsens_priv *priv)
>         u32 p1[10], p2[10];
>         u32 mode = 0, lsb = 0, msb = 0;
>         u32 *qfprom_cdata;
> -       int i;
> +       int i, ret;
> +
> +       ret = tsens_calibrate_nvmem(priv, 2);
> +       if (!ret)
> +               return 0;
>
>         qfprom_cdata = (u32 *)qfprom_read(priv->dev, "calib");
>         if (IS_ERR(qfprom_cdata))
> diff --git a/drivers/thermal/qcom/tsens.c b/drivers/thermal/qcom/tsens.c
> index e49f58e83513..8331b924325a 100644
> --- a/drivers/thermal/qcom/tsens.c
> +++ b/drivers/thermal/qcom/tsens.c
> @@ -70,6 +70,68 @@ char *qfprom_read(struct device *dev, const char *cname)
>         return ret;
>  }
>
> +int tsens_calibrate_nvmem(struct tsens_priv *priv, int shift)
> +{
> +       u32 mode;
> +       u32 base1, base2;
> +       u32 p1[MAX_SENSORS], p2[MAX_SENSORS];
> +       char name[] = "sX_pY";
> +       int i, ret;
> +
> +       if (priv->num_sensors > MAX_SENSORS)
> +               return -EINVAL;
> +
> +       ret = nvmem_cell_read_variable_le_u32(priv->dev, "mode", &mode);
> +       if (ret == -ENOENT)
> +               dev_warn(priv->dev, "Please migrate to sepate nvmem cells for calibration data\n");

typo: separate

> +       if (ret < 0)
> +               return ret;
> +
> +       dev_dbg(priv->dev, "calibration mode is %d\n", mode);
> +
> +       ret = nvmem_cell_read_variable_le_u32(priv->dev, "base1", &base1);
> +       if (ret < 0)
> +               return ret;
> +
> +       ret = nvmem_cell_read_variable_le_u32(priv->dev, "base2", &base2);
> +       if (ret < 0)
> +               return ret;
> +
> +       for (i = 0; i < priv->num_sensors; i++) {
> +               ret = snprintf(name, sizeof(name), "s%d_p1", i);
> +               if (ret < 0)
> +                       return ret;
> +
> +               ret = nvmem_cell_read_variable_le_u32(priv->dev, name, &p1[i]);
> +               if (ret)
> +                       return ret;
> +
> +               p1[i] = (base1 + p1[i]) << shift;
> +
> +               ret = snprintf(name, sizeof(name), "s%d_p2", i);
> +               if (ret < 0)
> +                       return ret;
> +
> +               ret = nvmem_cell_read_variable_le_u32(priv->dev, name, &p2[i]);
> +               if (ret)
> +                       return ret;
> +
> +               p2[i] = (base2 + p2[i]) << shift;

Have you verified that the p1 and p2 arrays end up with identical
values before and after this conversion?


> +       }
> +
> +       if (mode == NO_PT_CALIB) {
> +               dev_dbg(priv->dev, "calibrationless mode\n");
> +               for (i = 0; i < priv->num_sensors; i++) {
> +                       p1[i] = 500;
> +                       p2[i] = 780;
> +               }
> +       }
> +
> +       compute_intercept_slope(priv, p1, p2, mode);
> +
> +       return 0;
> +}
> +
>  /*

Re: [RFC PATCH 02/10] thermal/drivers/tsens: Support using nvmem cells for calibration data

[Dmitry Baryshkov]

On 13 September 2022 22:18:48 GMT+03:00, Amit Kucheria <amitk@kernel.org> wrote:
>Hi Dmitry,
>
>I like this clean up.
>
>On Sat, Sep 10, 2022 at 6:17 PM Dmitry Baryshkov
><dmitry.baryshkov@linaro.org> wrote:
>>
>> Add a unified function using nvmem cells for parsing the calibration
>> data rather than parsing the calibration blob manually.
>>
>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>> ---
>>  drivers/thermal/qcom/tsens-v0_1.c | 15 ++++++++
>>  drivers/thermal/qcom/tsens-v1.c   |  6 ++-
>>  drivers/thermal/qcom/tsens.c      | 62 +++++++++++++++++++++++++++++++
>>  drivers/thermal/qcom/tsens.h      |  4 ++
>>  4 files changed, 86 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/thermal/qcom/tsens-v0_1.c b/drivers/thermal/qcom/tsens-v0_1.c
>> index f136cb350238..2974eea578f4 100644
>> --- a/drivers/thermal/qcom/tsens-v0_1.c
>> +++ b/drivers/thermal/qcom/tsens-v0_1.c
>> @@ -229,6 +229,11 @@ static int calibrate_8916(struct tsens_priv *priv)
>>         u32 p1[5], p2[5];
>>         int mode = 0;
>>         u32 *qfprom_cdata, *qfprom_csel;
>> +       int ret;
>> +
>> +       ret = tsens_calibrate_nvmem(priv, 3);
>> +       if (!ret)
>> +               return 0;
>>
>>         qfprom_cdata = (u32 *)qfprom_read(priv->dev, "calib");
>>         if (IS_ERR(qfprom_cdata))
>> @@ -286,6 +291,11 @@ static int calibrate_8939(struct tsens_priv *priv)
>>         int mode = 0;
>>         u32 *qfprom_cdata;
>>         u32 cdata[6];
>> +       int ret;
>> +
>> +       ret = tsens_calibrate_nvmem(priv, 2);
>> +       if (!ret)
>> +               return 0;
>>
>>         qfprom_cdata = (u32 *)qfprom_read(priv->dev, "calib");
>>         if (IS_ERR(qfprom_cdata))
>> @@ -491,6 +501,11 @@ static int calibrate_9607(struct tsens_priv *priv)
>>         u32 p1[5], p2[5];
>>         int mode = 0;
>>         u32 *qfprom_cdata;
>> +       int ret;
>> +
>> +       ret = tsens_calibrate_nvmem(priv, 2);
>> +       if (!ret)
>> +               return 0;
>>
>>         qfprom_cdata = (u32 *)qfprom_read(priv->dev, "calib");
>>         if (IS_ERR(qfprom_cdata))
>> diff --git a/drivers/thermal/qcom/tsens-v1.c b/drivers/thermal/qcom/tsens-v1.c
>> index 573e261ccca7..868d7b4c9e36 100644
>> --- a/drivers/thermal/qcom/tsens-v1.c
>> +++ b/drivers/thermal/qcom/tsens-v1.c
>> @@ -172,7 +172,11 @@ static int calibrate_v1(struct tsens_priv *priv)
>>         u32 p1[10], p2[10];
>>         u32 mode = 0, lsb = 0, msb = 0;
>>         u32 *qfprom_cdata;
>> -       int i;
>> +       int i, ret;
>> +
>> +       ret = tsens_calibrate_nvmem(priv, 2);
>> +       if (!ret)
>> +               return 0;
>>
>>         qfprom_cdata = (u32 *)qfprom_read(priv->dev, "calib");
>>         if (IS_ERR(qfprom_cdata))
>> diff --git a/drivers/thermal/qcom/tsens.c b/drivers/thermal/qcom/tsens.c
>> index e49f58e83513..8331b924325a 100644
>> --- a/drivers/thermal/qcom/tsens.c
>> +++ b/drivers/thermal/qcom/tsens.c
>> @@ -70,6 +70,68 @@ char *qfprom_read(struct device *dev, const char *cname)
>>         return ret;
>>  }
>>
>> +int tsens_calibrate_nvmem(struct tsens_priv *priv, int shift)
>> +{
>> +       u32 mode;
>> +       u32 base1, base2;
>> +       u32 p1[MAX_SENSORS], p2[MAX_SENSORS];
>> +       char name[] = "sX_pY";
>> +       int i, ret;
>> +
>> +       if (priv->num_sensors > MAX_SENSORS)
>> +               return -EINVAL;
>> +
>> +       ret = nvmem_cell_read_variable_le_u32(priv->dev, "mode", &mode);
>> +       if (ret == -ENOENT)
>> +               dev_warn(priv->dev, "Please migrate to sepate nvmem cells for calibration data\n");
>
>typo: separate

Ack.


>
>> +       if (ret < 0)
>> +               return ret;
>> +
>> +       dev_dbg(priv->dev, "calibration mode is %d\n", mode);
>> +
>> +       ret = nvmem_cell_read_variable_le_u32(priv->dev, "base1", &base1);
>> +       if (ret < 0)
>> +               return ret;
>> +
>> +       ret = nvmem_cell_read_variable_le_u32(priv->dev, "base2", &base2);
>> +       if (ret < 0)
>> +               return ret;
>> +
>> +       for (i = 0; i < priv->num_sensors; i++) {
>> +               ret = snprintf(name, sizeof(name), "s%d_p1", i);
>> +               if (ret < 0)
>> +                       return ret;
>> +
>> +               ret = nvmem_cell_read_variable_le_u32(priv->dev, name, &p1[i]);
>> +               if (ret)
>> +                       return ret;
>> +
>> +               p1[i] = (base1 + p1[i]) << shift;
>> +
>> +               ret = snprintf(name, sizeof(name), "s%d_p2", i);
>> +               if (ret < 0)
>> +                       return ret;
>> +
>> +               ret = nvmem_cell_read_variable_le_u32(priv->dev, name, &p2[i]);
>> +               if (ret)
>> +                       return ret;
>> +
>> +               p2[i] = (base2 + p2[i]) << shift;
>
>Have you verified that the p1 and p2 arrays end up with identical
>values before and after this conversion?

Yes. I have verified that the calibration values are unchanged on db410c(apq8016, v0_1) and qcs404-evb (v1).


>
>
>> +       }
>> +
>> +       if (mode == NO_PT_CALIB) {
>> +               dev_dbg(priv->dev, "calibrationless mode\n");
>> +               for (i = 0; i < priv->num_sensors; i++) {
>> +                       p1[i] = 500;
>> +                       p2[i] = 780;
>> +               }
>> +       }
>> +
>> +       compute_intercept_slope(priv, p1, p2, mode);
>> +
>> +       return 0;
>> +}
>> +
>>  /*

-- 
With best wishes
Dmitry

[RFC PATCH 03/10] thermal/drivers/tsens: drop single-cell code for msm8939

[Dmitry Baryshkov]

There is no dtsi file for msm8939 in the kernel sources. Drop the
compatibility with unofficial dtsi and remove support for handling the
single-cell calibration data on msm8939.

Cc: Shawn Guo <shawn.guo@linaro.org>
Cc: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/thermal/qcom/tsens-v0_1.c | 129 +-----------------------------
 1 file changed, 1 insertion(+), 128 deletions(-)

diff --git a/drivers/thermal/qcom/tsens-v0_1.c b/drivers/thermal/qcom/tsens-v0_1.c
index 2974eea578f4..add5d9dbc9d4 100644
--- a/drivers/thermal/qcom/tsens-v0_1.c
+++ b/drivers/thermal/qcom/tsens-v0_1.c
@@ -48,63 +48,6 @@
 #define MSM8916_CAL_SEL_MASK	0xe0000000
 #define MSM8916_CAL_SEL_SHIFT	29
 
-/* eeprom layout data for 8939 */
-#define MSM8939_BASE0_MASK	0x000000ff
-#define MSM8939_BASE1_MASK	0xff000000
-#define MSM8939_BASE0_SHIFT	0
-#define MSM8939_BASE1_SHIFT	24
-
-#define MSM8939_S0_P1_MASK	0x000001f8
-#define MSM8939_S1_P1_MASK	0x001f8000
-#define MSM8939_S2_P1_MASK_0_4	0xf8000000
-#define MSM8939_S2_P1_MASK_5	0x00000001
-#define MSM8939_S3_P1_MASK	0x00001f80
-#define MSM8939_S4_P1_MASK	0x01f80000
-#define MSM8939_S5_P1_MASK	0x00003f00
-#define MSM8939_S6_P1_MASK	0x03f00000
-#define MSM8939_S7_P1_MASK	0x0000003f
-#define MSM8939_S8_P1_MASK	0x0003f000
-#define MSM8939_S9_P1_MASK	0x07e00000
-
-#define MSM8939_S0_P2_MASK	0x00007e00
-#define MSM8939_S1_P2_MASK	0x07e00000
-#define MSM8939_S2_P2_MASK	0x0000007e
-#define MSM8939_S3_P2_MASK	0x0007e000
-#define MSM8939_S4_P2_MASK	0x7e000000
-#define MSM8939_S5_P2_MASK	0x000fc000
-#define MSM8939_S6_P2_MASK	0xfc000000
-#define MSM8939_S7_P2_MASK	0x00000fc0
-#define MSM8939_S8_P2_MASK	0x00fc0000
-#define MSM8939_S9_P2_MASK_0_4	0xf8000000
-#define MSM8939_S9_P2_MASK_5	0x00002000
-
-#define MSM8939_S0_P1_SHIFT	3
-#define MSM8939_S1_P1_SHIFT	15
-#define MSM8939_S2_P1_SHIFT_0_4	27
-#define MSM8939_S2_P1_SHIFT_5	0
-#define MSM8939_S3_P1_SHIFT	7
-#define MSM8939_S4_P1_SHIFT	19
-#define MSM8939_S5_P1_SHIFT	8
-#define MSM8939_S6_P1_SHIFT	20
-#define MSM8939_S7_P1_SHIFT	0
-#define MSM8939_S8_P1_SHIFT	12
-#define MSM8939_S9_P1_SHIFT	21
-
-#define MSM8939_S0_P2_SHIFT	9
-#define MSM8939_S1_P2_SHIFT	21
-#define MSM8939_S2_P2_SHIFT	1
-#define MSM8939_S3_P2_SHIFT	13
-#define MSM8939_S4_P2_SHIFT	25
-#define MSM8939_S5_P2_SHIFT	14
-#define MSM8939_S6_P2_SHIFT	26
-#define MSM8939_S7_P2_SHIFT	6
-#define MSM8939_S8_P2_SHIFT	18
-#define MSM8939_S9_P2_SHIFT_0_4	27
-#define MSM8939_S9_P2_SHIFT_5	13
-
-#define MSM8939_CAL_SEL_MASK	0x7
-#define MSM8939_CAL_SEL_SHIFT	0
-
 /* eeprom layout data for 8974 */
 #define BASE1_MASK		0xff
 #define S0_P1_MASK		0x3f00
@@ -286,77 +229,7 @@ static int calibrate_8916(struct tsens_priv *priv)
 
 static int calibrate_8939(struct tsens_priv *priv)
 {
-	int base0 = 0, base1 = 0, i;
-	u32 p1[10], p2[10];
-	int mode = 0;
-	u32 *qfprom_cdata;
-	u32 cdata[6];
-	int ret;
-
-	ret = tsens_calibrate_nvmem(priv, 2);
-	if (!ret)
-		return 0;
-
-	qfprom_cdata = (u32 *)qfprom_read(priv->dev, "calib");
-	if (IS_ERR(qfprom_cdata))
-		return PTR_ERR(qfprom_cdata);
-
-	/* Mapping between qfprom nvmem and calibration data */
-	cdata[0] = qfprom_cdata[12];
-	cdata[1] = qfprom_cdata[13];
-	cdata[2] = qfprom_cdata[0];
-	cdata[3] = qfprom_cdata[1];
-	cdata[4] = qfprom_cdata[22];
-	cdata[5] = qfprom_cdata[21];
-
-	mode = (cdata[0] & MSM8939_CAL_SEL_MASK) >> MSM8939_CAL_SEL_SHIFT;
-	dev_dbg(priv->dev, "calibration mode is %d\n", mode);
-
-	switch (mode) {
-	case TWO_PT_CALIB:
-		base1 = (cdata[3] & MSM8939_BASE1_MASK) >> MSM8939_BASE1_SHIFT;
-		p2[0] = (cdata[0] & MSM8939_S0_P2_MASK) >> MSM8939_S0_P2_SHIFT;
-		p2[1] = (cdata[0] & MSM8939_S1_P2_MASK) >> MSM8939_S1_P2_SHIFT;
-		p2[2] = (cdata[1] & MSM8939_S2_P2_MASK) >> MSM8939_S2_P2_SHIFT;
-		p2[3] = (cdata[1] & MSM8939_S3_P2_MASK) >> MSM8939_S3_P2_SHIFT;
-		p2[4] = (cdata[1] & MSM8939_S4_P2_MASK) >> MSM8939_S4_P2_SHIFT;
-		p2[5] = (cdata[2] & MSM8939_S5_P2_MASK) >> MSM8939_S5_P2_SHIFT;
-		p2[6] = (cdata[2] & MSM8939_S6_P2_MASK) >> MSM8939_S6_P2_SHIFT;
-		p2[7] = (cdata[3] & MSM8939_S7_P2_MASK) >> MSM8939_S7_P2_SHIFT;
-		p2[8] = (cdata[3] & MSM8939_S8_P2_MASK) >> MSM8939_S8_P2_SHIFT;
-		p2[9] = (cdata[4] & MSM8939_S9_P2_MASK_0_4) >> MSM8939_S9_P2_SHIFT_0_4;
-		p2[9] |= ((cdata[5] & MSM8939_S9_P2_MASK_5) >> MSM8939_S9_P2_SHIFT_5) << 5;
-		for (i = 0; i < priv->num_sensors; i++)
-			p2[i] = (base1 + p2[i]) << 2;
-		fallthrough;
-	case ONE_PT_CALIB2:
-		base0 = (cdata[2] & MSM8939_BASE0_MASK) >> MSM8939_BASE0_SHIFT;
-		p1[0] = (cdata[0] & MSM8939_S0_P1_MASK) >> MSM8939_S0_P1_SHIFT;
-		p1[1] = (cdata[0] & MSM8939_S1_P1_MASK) >> MSM8939_S1_P1_SHIFT;
-		p1[2] = (cdata[0] & MSM8939_S2_P1_MASK_0_4) >> MSM8939_S2_P1_SHIFT_0_4;
-		p1[2] |= ((cdata[1] & MSM8939_S2_P1_MASK_5) >> MSM8939_S2_P1_SHIFT_5) << 5;
-		p1[3] = (cdata[1] & MSM8939_S3_P1_MASK) >> MSM8939_S3_P1_SHIFT;
-		p1[4] = (cdata[1] & MSM8939_S4_P1_MASK) >> MSM8939_S4_P1_SHIFT;
-		p1[5] = (cdata[2] & MSM8939_S5_P1_MASK) >> MSM8939_S5_P1_SHIFT;
-		p1[6] = (cdata[2] & MSM8939_S6_P1_MASK) >> MSM8939_S6_P1_SHIFT;
-		p1[7] = (cdata[3] & MSM8939_S7_P1_MASK) >> MSM8939_S7_P1_SHIFT;
-		p1[8] = (cdata[3] & MSM8939_S8_P1_MASK) >> MSM8939_S8_P1_SHIFT;
-		p1[9] = (cdata[4] & MSM8939_S9_P1_MASK) >> MSM8939_S9_P1_SHIFT;
-		for (i = 0; i < priv->num_sensors; i++)
-			p1[i] = ((base0) + p1[i]) << 2;
-		break;
-	default:
-		for (i = 0; i < priv->num_sensors; i++) {
-			p1[i] = 500;
-			p2[i] = 780;
-		}
-		break;
-	}
-
-	compute_intercept_slope(priv, p1, p2, mode);
-	kfree(qfprom_cdata);
-
-	return 0;
+	return tsens_calibrate_nvmem(priv, 2);
 }
 
 static int calibrate_8974(struct tsens_priv *priv)
-- 
2.35.1

[RFC PATCH 04/10] thermal/drivers/tsens: drop single-cell code for mdm9607

[Dmitry Baryshkov]

There is no dtsi file for mdm9607 in the kernel sources. Drop the
compatibility with unofficial dtsi and remove support for handling the
single-cell calibration data on mdm9607.

Cc: Konrad Dybcio <konrad.dybcio@somainline.org>
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/thermal/qcom/tsens-v0_1.c | 84 +------------------------------
 1 file changed, 1 insertion(+), 83 deletions(-)

diff --git a/drivers/thermal/qcom/tsens-v0_1.c b/drivers/thermal/qcom/tsens-v0_1.c
index add5d9dbc9d4..c3613b7ccc19 100644
--- a/drivers/thermal/qcom/tsens-v0_1.c
+++ b/drivers/thermal/qcom/tsens-v0_1.c
@@ -133,39 +133,6 @@
 
 #define BIT_APPEND		0x3
 
-/* eeprom layout data for mdm9607 */
-#define MDM9607_BASE0_MASK	0x000000ff
-#define MDM9607_BASE1_MASK	0x000ff000
-#define MDM9607_BASE0_SHIFT	0
-#define MDM9607_BASE1_SHIFT	12
-
-#define MDM9607_S0_P1_MASK	0x00003f00
-#define MDM9607_S1_P1_MASK	0x03f00000
-#define MDM9607_S2_P1_MASK	0x0000003f
-#define MDM9607_S3_P1_MASK	0x0003f000
-#define MDM9607_S4_P1_MASK	0x0000003f
-
-#define MDM9607_S0_P2_MASK	0x000fc000
-#define MDM9607_S1_P2_MASK	0xfc000000
-#define MDM9607_S2_P2_MASK	0x00000fc0
-#define MDM9607_S3_P2_MASK	0x00fc0000
-#define MDM9607_S4_P2_MASK	0x00000fc0
-
-#define MDM9607_S0_P1_SHIFT	8
-#define MDM9607_S1_P1_SHIFT	20
-#define MDM9607_S2_P1_SHIFT	0
-#define MDM9607_S3_P1_SHIFT	12
-#define MDM9607_S4_P1_SHIFT	0
-
-#define MDM9607_S0_P2_SHIFT	14
-#define MDM9607_S1_P2_SHIFT	26
-#define MDM9607_S2_P2_SHIFT	6
-#define MDM9607_S3_P2_SHIFT	18
-#define MDM9607_S4_P2_SHIFT	6
-
-#define MDM9607_CAL_SEL_MASK	0x00700000
-#define MDM9607_CAL_SEL_SHIFT	20
-
 static int calibrate_8916(struct tsens_priv *priv)
 {
 	int base0 = 0, base1 = 0, i;
@@ -370,56 +337,7 @@ static int calibrate_8974(struct tsens_priv *priv)
 
 static int calibrate_9607(struct tsens_priv *priv)
 {
-	int base, i;
-	u32 p1[5], p2[5];
-	int mode = 0;
-	u32 *qfprom_cdata;
-	int ret;
-
-	ret = tsens_calibrate_nvmem(priv, 2);
-	if (!ret)
-		return 0;
-
-	qfprom_cdata = (u32 *)qfprom_read(priv->dev, "calib");
-	if (IS_ERR(qfprom_cdata))
-		return PTR_ERR(qfprom_cdata);
-
-	mode = (qfprom_cdata[2] & MDM9607_CAL_SEL_MASK) >> MDM9607_CAL_SEL_SHIFT;
-	dev_dbg(priv->dev, "calibration mode is %d\n", mode);
-
-	switch (mode) {
-	case TWO_PT_CALIB:
-		base = (qfprom_cdata[2] & MDM9607_BASE1_MASK) >> MDM9607_BASE1_SHIFT;
-		p2[0] = (qfprom_cdata[0] & MDM9607_S0_P2_MASK) >> MDM9607_S0_P2_SHIFT;
-		p2[1] = (qfprom_cdata[0] & MDM9607_S1_P2_MASK) >> MDM9607_S1_P2_SHIFT;
-		p2[2] = (qfprom_cdata[1] & MDM9607_S2_P2_MASK) >> MDM9607_S2_P2_SHIFT;
-		p2[3] = (qfprom_cdata[1] & MDM9607_S3_P2_MASK) >> MDM9607_S3_P2_SHIFT;
-		p2[4] = (qfprom_cdata[2] & MDM9607_S4_P2_MASK) >> MDM9607_S4_P2_SHIFT;
-		for (i = 0; i < priv->num_sensors; i++)
-			p2[i] = ((base + p2[i]) << 2);
-		fallthrough;
-	case ONE_PT_CALIB2:
-		base = (qfprom_cdata[0] & MDM9607_BASE0_MASK);
-		p1[0] = (qfprom_cdata[0] & MDM9607_S0_P1_MASK) >> MDM9607_S0_P1_SHIFT;
-		p1[1] = (qfprom_cdata[0] & MDM9607_S1_P1_MASK) >> MDM9607_S1_P1_SHIFT;
-		p1[2] = (qfprom_cdata[1] & MDM9607_S2_P1_MASK) >> MDM9607_S2_P1_SHIFT;
-		p1[3] = (qfprom_cdata[1] & MDM9607_S3_P1_MASK) >> MDM9607_S3_P1_SHIFT;
-		p1[4] = (qfprom_cdata[2] & MDM9607_S4_P1_MASK) >> MDM9607_S4_P1_SHIFT;
-		for (i = 0; i < priv->num_sensors; i++)
-			p1[i] = ((base + p1[i]) << 2);
-		break;
-	default:
-		for (i = 0; i < priv->num_sensors; i++) {
-			p1[i] = 500;
-			p2[i] = 780;
-		}
-		break;
-	}
-
-	compute_intercept_slope(priv, p1, p2, mode);
-	kfree(qfprom_cdata);
-
-	return 0;
+	return tsens_calibrate_nvmem(priv, 2);
 }
 
 /* v0.1: 8916, 8939, 8974, 9607 */
-- 
2.35.1

[RFC PATCH 05/10] thermal/drivers/tsens: drop msm8976-specific defines

[Dmitry Baryshkov]

Drop msm8976-specific defines, which duplicate generic ones.

Cc: Konrad Dybcio <konrad.dybcio@somainline.org>
Cc: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/thermal/qcom/tsens-v1.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/drivers/thermal/qcom/tsens-v1.c b/drivers/thermal/qcom/tsens-v1.c
index 868d7b4c9e36..e6ef3bacfc39 100644
--- a/drivers/thermal/qcom/tsens-v1.c
+++ b/drivers/thermal/qcom/tsens-v1.c
@@ -78,11 +78,6 @@
 
 #define MSM8976_CAL_SEL_MASK	0x3
 
-#define MSM8976_CAL_DEGC_PT1	30
-#define MSM8976_CAL_DEGC_PT2	120
-#define MSM8976_SLOPE_FACTOR	1000
-#define MSM8976_SLOPE_DEFAULT	3200
-
 /* eeprom layout data for qcs404/405 (v1) */
 #define BASE0_MASK	0x000007f8
 #define BASE1_MASK	0x0007f800
@@ -160,8 +155,8 @@ static void compute_intercept_slope_8976(struct tsens_priv *priv,
 	priv->sensor[10].slope = 3286;
 
 	for (i = 0; i < priv->num_sensors; i++) {
-		priv->sensor[i].offset = (p1[i] * MSM8976_SLOPE_FACTOR) -
-				(MSM8976_CAL_DEGC_PT1 *
+		priv->sensor[i].offset = (p1[i] * SLOPE_FACTOR) -
+				(CAL_DEGC_PT1 *
 				priv->sensor[i].slope);
 	}
 }
-- 
2.35.1

Re: [RFC PATCH 05/10] thermal/drivers/tsens: drop msm8976-specific defines

[AngeloGioacchino Del Regno]

Il 10/09/22 14:46, Dmitry Baryshkov ha scritto:
> Drop msm8976-specific defines, which duplicate generic ones.
> 
> Cc: Konrad Dybcio <konrad.dybcio@somainline.org>
> Cc: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

The duplication is useless indeed... However, should you add a Fixes tag here?

0e580290170d ("thermal: qcom: tsens-v1: Add support for MSM8956 and MSM8976")

Anyway, for the code:

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

Cheers,
Angelo

[RFC PATCH 06/10] thermal/drivers/tsens: use generic calibration routine for msm8976

[Dmitry Baryshkov]

According to msm-3.10, msm8976 uses the same routine for processing
calibration data as other platforms. Drop the msm8976-specific
compute_intercept_slope_8976() and use compute_intercept_slope().

Cc: Konrad Dybcio <konrad.dybcio@somainline.org>
Cc: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/thermal/qcom/tsens-v1.c | 26 +-------------------------
 1 file changed, 1 insertion(+), 25 deletions(-)

diff --git a/drivers/thermal/qcom/tsens-v1.c b/drivers/thermal/qcom/tsens-v1.c
index e6ef3bacfc39..11363a318ae8 100644
--- a/drivers/thermal/qcom/tsens-v1.c
+++ b/drivers/thermal/qcom/tsens-v1.c
@@ -137,30 +137,6 @@
 #define CAL_SEL_MASK	7
 #define CAL_SEL_SHIFT	0
 
-static void compute_intercept_slope_8976(struct tsens_priv *priv,
-			      u32 *p1, u32 *p2, u32 mode)
-{
-	int i;
-
-	priv->sensor[0].slope = 3313;
-	priv->sensor[1].slope = 3275;
-	priv->sensor[2].slope = 3320;
-	priv->sensor[3].slope = 3246;
-	priv->sensor[4].slope = 3279;
-	priv->sensor[5].slope = 3257;
-	priv->sensor[6].slope = 3234;
-	priv->sensor[7].slope = 3269;
-	priv->sensor[8].slope = 3255;
-	priv->sensor[9].slope = 3239;
-	priv->sensor[10].slope = 3286;
-
-	for (i = 0; i < priv->num_sensors; i++) {
-		priv->sensor[i].offset = (p1[i] * SLOPE_FACTOR) -
-				(CAL_DEGC_PT1 *
-				priv->sensor[i].slope);
-	}
-}
-
 static int calibrate_v1(struct tsens_priv *priv)
 {
 	u32 base0 = 0, base1 = 0;
@@ -290,7 +266,7 @@ static int calibrate_8976(struct tsens_priv *priv)
 		break;
 	}
 
-	compute_intercept_slope_8976(priv, p1, p2, mode);
+	compute_intercept_slope(priv, p1, p2, mode);
 	kfree(qfprom_cdata);
 
 	return 0;
-- 
2.35.1

Re: [RFC PATCH 06/10] thermal/drivers/tsens: use generic calibration routine for msm8976

[AngeloGioacchino Del Regno]

Il 10/09/22 14:46, Dmitry Baryshkov ha scritto:
> According to msm-3.10, msm8976 uses the same routine for processing
> calibration data as other platforms. Drop the msm8976-specific
> compute_intercept_slope_8976() and use compute_intercept_slope().
> 
> Cc: Konrad Dybcio <konrad.dybcio@somainline.org>
> Cc: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

As far as I remember, at least some MSM8976 versions don't have the slope
fuses populated and/or the values are bad. This is the reason why slopes
are hardcoded.

This commit would break support for MSM8976.
Cc'ing Marijn, as he should be able to re-test this.

Cheers,
Angelo

Re: [RFC PATCH 06/10] thermal/drivers/tsens: use generic calibration routine for msm8976

[Dmitry Baryshkov]

Hi,

On Mon, 12 Sept 2022 at 11:57, AngeloGioacchino Del Regno
<angelogioacchino.delregno@collabora.com> wrote:
>
> Il 10/09/22 14:46, Dmitry Baryshkov ha scritto:
> > According to msm-3.10, msm8976 uses the same routine for processing
> > calibration data as other platforms. Drop the msm8976-specific
> > compute_intercept_slope_8976() and use compute_intercept_slope().
> >
> > Cc: Konrad Dybcio <konrad.dybcio@somainline.org>
> > Cc: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>
> As far as I remember, at least some MSM8976 versions don't have the slope
> fuses populated and/or the values are bad. This is the reason why slopes
> are hardcoded.
>
> This commit would break support for MSM8976.
> Cc'ing Marijn, as he should be able to re-test this.

Thanks! I was not sure, granted that the downstream seems to use
standard procedure. The details/retesting would be appreciated.


-- 
With best wishes
Dmitry

Re: [RFC PATCH 06/10] thermal/drivers/tsens: use generic calibration routine for msm8976

[Dmitry Baryshkov]

On 12/09/2022 11:57, AngeloGioacchino Del Regno wrote:
> Il 10/09/22 14:46, Dmitry Baryshkov ha scritto:
>> According to msm-3.10, msm8976 uses the same routine for processing
>> calibration data as other platforms. Drop the msm8976-specific
>> compute_intercept_slope_8976() and use compute_intercept_slope().
>>
>> Cc: Konrad Dybcio <konrad.dybcio@somainline.org>
>> Cc: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> 
> As far as I remember, at least some MSM8976 versions don't have the slope
> fuses populated and/or the values are bad. This is the reason why slopes
> are hardcoded.
> 
> This commit would break support for MSM8976.
> Cc'ing Marijn, as he should be able to re-test this.

Angelo, Marijn, any update on testing this?

-- 
With best wishes
Dmitry

[RFC PATCH 07/10] thermal/drivers/tsens: use tsens_calibrate_nvmem for msm8976 calibration

[Dmitry Baryshkov]

Call tsens_calibrate_nvmem() for parsing msm8976 calibration data in
addition to parsing the blob manually.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/thermal/qcom/tsens-v1.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/thermal/qcom/tsens-v1.c b/drivers/thermal/qcom/tsens-v1.c
index 11363a318ae8..2a4440a34735 100644
--- a/drivers/thermal/qcom/tsens-v1.c
+++ b/drivers/thermal/qcom/tsens-v1.c
@@ -213,6 +213,11 @@ static int calibrate_8976(struct tsens_priv *priv)
 	u32 p1[11], p2[11];
 	int mode = 0, tmp = 0;
 	u32 *qfprom_cdata;
+	int ret;
+
+	ret = tsens_calibrate_nvmem(priv, 2);
+	if (!ret)
+		return 0;
 
 	qfprom_cdata = (u32 *)qfprom_read(priv->dev, "calib");
 	if (IS_ERR(qfprom_cdata))
-- 
2.35.1

[RFC PATCH 08/10] thermal/drivers/tsens: drop single-cell code for msm8976

[Dmitry Baryshkov]

There is no dtsi file for msm8976 in the kernel sources. Drop the
compatibility with unofficial dtsi and remove support for handling the
single-cell calibration data on msm8976.

Cc: Konrad Dybcio <konrad.dybcio@somainline.org>
Cc: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/thermal/qcom/tsens-v1.c | 124 +-------------------------------
 1 file changed, 1 insertion(+), 123 deletions(-)

diff --git a/drivers/thermal/qcom/tsens-v1.c b/drivers/thermal/qcom/tsens-v1.c
index 2a4440a34735..b6c2da3371e4 100644
--- a/drivers/thermal/qcom/tsens-v1.c
+++ b/drivers/thermal/qcom/tsens-v1.c
@@ -21,63 +21,6 @@
 #define TM_HIGH_LOW_INT_STATUS_OFF		0x0088
 #define TM_HIGH_LOW_Sn_INT_THRESHOLD_OFF	0x0090
 
-/* eeprom layout data for msm8956/76 (v1) */
-#define MSM8976_BASE0_MASK	0xff
-#define MSM8976_BASE1_MASK	0xff
-#define MSM8976_BASE1_SHIFT	8
-
-#define MSM8976_S0_P1_MASK	0x3f00
-#define MSM8976_S1_P1_MASK	0x3f00000
-#define MSM8976_S2_P1_MASK	0x3f
-#define MSM8976_S3_P1_MASK	0x3f000
-#define MSM8976_S4_P1_MASK	0x3f00
-#define MSM8976_S5_P1_MASK	0x3f00000
-#define MSM8976_S6_P1_MASK	0x3f
-#define MSM8976_S7_P1_MASK	0x3f000
-#define MSM8976_S8_P1_MASK	0x1f8
-#define MSM8976_S9_P1_MASK	0x1f8000
-#define MSM8976_S10_P1_MASK	0xf8000000
-#define MSM8976_S10_P1_MASK_1	0x1
-
-#define MSM8976_S0_P2_MASK	0xfc000
-#define MSM8976_S1_P2_MASK	0xfc000000
-#define MSM8976_S2_P2_MASK	0xfc0
-#define MSM8976_S3_P2_MASK	0xfc0000
-#define MSM8976_S4_P2_MASK	0xfc000
-#define MSM8976_S5_P2_MASK	0xfc000000
-#define MSM8976_S6_P2_MASK	0xfc0
-#define MSM8976_S7_P2_MASK	0xfc0000
-#define MSM8976_S8_P2_MASK	0x7e00
-#define MSM8976_S9_P2_MASK	0x7e00000
-#define MSM8976_S10_P2_MASK	0x7e
-
-#define MSM8976_S0_P1_SHIFT	8
-#define MSM8976_S1_P1_SHIFT	20
-#define MSM8976_S2_P1_SHIFT	0
-#define MSM8976_S3_P1_SHIFT	12
-#define MSM8976_S4_P1_SHIFT	8
-#define MSM8976_S5_P1_SHIFT	20
-#define MSM8976_S6_P1_SHIFT	0
-#define MSM8976_S7_P1_SHIFT	12
-#define MSM8976_S8_P1_SHIFT	3
-#define MSM8976_S9_P1_SHIFT	15
-#define MSM8976_S10_P1_SHIFT	27
-#define MSM8976_S10_P1_SHIFT_1	0
-
-#define MSM8976_S0_P2_SHIFT	14
-#define MSM8976_S1_P2_SHIFT	26
-#define MSM8976_S2_P2_SHIFT	6
-#define MSM8976_S3_P2_SHIFT	18
-#define MSM8976_S4_P2_SHIFT	14
-#define MSM8976_S5_P2_SHIFT	26
-#define MSM8976_S6_P2_SHIFT	6
-#define MSM8976_S7_P2_SHIFT	18
-#define MSM8976_S8_P2_SHIFT	9
-#define MSM8976_S9_P2_SHIFT	21
-#define MSM8976_S10_P2_SHIFT	1
-
-#define MSM8976_CAL_SEL_MASK	0x3
-
 /* eeprom layout data for qcs404/405 (v1) */
 #define BASE0_MASK	0x000007f8
 #define BASE1_MASK	0x0007f800
@@ -209,72 +152,7 @@ static int calibrate_v1(struct tsens_priv *priv)
 
 static int calibrate_8976(struct tsens_priv *priv)
 {
-	int base0 = 0, base1 = 0, i;
-	u32 p1[11], p2[11];
-	int mode = 0, tmp = 0;
-	u32 *qfprom_cdata;
-	int ret;
-
-	ret = tsens_calibrate_nvmem(priv, 2);
-	if (!ret)
-		return 0;
-
-	qfprom_cdata = (u32 *)qfprom_read(priv->dev, "calib");
-	if (IS_ERR(qfprom_cdata))
-		return PTR_ERR(qfprom_cdata);
-
-	mode = (qfprom_cdata[4] & MSM8976_CAL_SEL_MASK);
-	dev_dbg(priv->dev, "calibration mode is %d\n", mode);
-
-	switch (mode) {
-	case TWO_PT_CALIB:
-		base1 = (qfprom_cdata[2] & MSM8976_BASE1_MASK) >> MSM8976_BASE1_SHIFT;
-		p2[0] = (qfprom_cdata[0] & MSM8976_S0_P2_MASK) >> MSM8976_S0_P2_SHIFT;
-		p2[1] = (qfprom_cdata[0] & MSM8976_S1_P2_MASK) >> MSM8976_S1_P2_SHIFT;
-		p2[2] = (qfprom_cdata[1] & MSM8976_S2_P2_MASK) >> MSM8976_S2_P2_SHIFT;
-		p2[3] = (qfprom_cdata[1] & MSM8976_S3_P2_MASK) >> MSM8976_S3_P2_SHIFT;
-		p2[4] = (qfprom_cdata[2] & MSM8976_S4_P2_MASK) >> MSM8976_S4_P2_SHIFT;
-		p2[5] = (qfprom_cdata[2] & MSM8976_S5_P2_MASK) >> MSM8976_S5_P2_SHIFT;
-		p2[6] = (qfprom_cdata[3] & MSM8976_S6_P2_MASK) >> MSM8976_S6_P2_SHIFT;
-		p2[7] = (qfprom_cdata[3] & MSM8976_S7_P2_MASK) >> MSM8976_S7_P2_SHIFT;
-		p2[8] = (qfprom_cdata[4] & MSM8976_S8_P2_MASK) >> MSM8976_S8_P2_SHIFT;
-		p2[9] = (qfprom_cdata[4] & MSM8976_S9_P2_MASK) >> MSM8976_S9_P2_SHIFT;
-		p2[10] = (qfprom_cdata[5] & MSM8976_S10_P2_MASK) >> MSM8976_S10_P2_SHIFT;
-
-		for (i = 0; i < priv->num_sensors; i++)
-			p2[i] = ((base1 + p2[i]) << 2);
-		fallthrough;
-	case ONE_PT_CALIB2:
-		base0 = qfprom_cdata[0] & MSM8976_BASE0_MASK;
-		p1[0] = (qfprom_cdata[0] & MSM8976_S0_P1_MASK) >> MSM8976_S0_P1_SHIFT;
-		p1[1] = (qfprom_cdata[0] & MSM8976_S1_P1_MASK) >> MSM8976_S1_P1_SHIFT;
-		p1[2] = (qfprom_cdata[1] & MSM8976_S2_P1_MASK) >> MSM8976_S2_P1_SHIFT;
-		p1[3] = (qfprom_cdata[1] & MSM8976_S3_P1_MASK) >> MSM8976_S3_P1_SHIFT;
-		p1[4] = (qfprom_cdata[2] & MSM8976_S4_P1_MASK) >> MSM8976_S4_P1_SHIFT;
-		p1[5] = (qfprom_cdata[2] & MSM8976_S5_P1_MASK) >> MSM8976_S5_P1_SHIFT;
-		p1[6] = (qfprom_cdata[3] & MSM8976_S6_P1_MASK) >> MSM8976_S6_P1_SHIFT;
-		p1[7] = (qfprom_cdata[3] & MSM8976_S7_P1_MASK) >> MSM8976_S7_P1_SHIFT;
-		p1[8] = (qfprom_cdata[4] & MSM8976_S8_P1_MASK) >> MSM8976_S8_P1_SHIFT;
-		p1[9] = (qfprom_cdata[4] & MSM8976_S9_P1_MASK) >> MSM8976_S9_P1_SHIFT;
-		p1[10] = (qfprom_cdata[4] & MSM8976_S10_P1_MASK) >> MSM8976_S10_P1_SHIFT;
-		tmp = (qfprom_cdata[5] & MSM8976_S10_P1_MASK_1) << MSM8976_S10_P1_SHIFT_1;
-		p1[10] |= tmp;
-
-		for (i = 0; i < priv->num_sensors; i++)
-			p1[i] = (((base0) + p1[i]) << 2);
-		break;
-	default:
-		for (i = 0; i < priv->num_sensors; i++) {
-			p1[i] = 500;
-			p2[i] = 780;
-		}
-		break;
-	}
-
-	compute_intercept_slope(priv, p1, p2, mode);
-	kfree(qfprom_cdata);
-
-	return 0;
+	return tsens_calibrate_nvmem(priv, 2);
 }
 
 /* v1.x: msm8956,8976,qcs404,405 */
-- 
2.35.1

[RFC PATCH 09/10] arm64: dts: qcom: msm8916: specify per-sensor calibration cells

[Dmitry Baryshkov]

Specify pre-parsed per-sensor calibration nvmem cells in the tsens
device node rather than parsing the whole data blob in the driver.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 arch/arm64/boot/dts/qcom/msm8916.dtsi | 70 ++++++++++++++++++++++++---
 1 file changed, 64 insertions(+), 6 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/msm8916.dtsi b/arch/arm64/boot/dts/qcom/msm8916.dtsi
index 48bc2e09128d..d2cce8f75fb6 100644
--- a/arch/arm64/boot/dts/qcom/msm8916.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8916.dtsi
@@ -442,11 +442,57 @@ qfprom: qfprom@5c000 {
 			reg = <0x0005c000 0x1000>;
 			#address-cells = <1>;
 			#size-cells = <1>;
-			tsens_caldata: caldata@d0 {
-				reg = <0xd0 0x8>;
+			tsens_base1: base1@d0 {
+				reg = <0xd0 0x1>;
+				bits = <0 7>;
 			};
-			tsens_calsel: calsel@ec {
-				reg = <0xec 0x4>;
+			tsens_s0_p1: s0_p1@d0 {
+				reg = <0xd0 0x2>;
+				bits = <7 5>;
+			};
+			tsens_s0_p2: s0_p2@d1 {
+				reg = <0xd1 0x2>;
+				bits = <4 5>;
+			};
+			tsens_s1_p1: s1_p1@d2 {
+				reg = <0xd2 0x1>;
+				bits = <1 5>;
+			};
+			tsens_s1_p2: s1_p2@d2 {
+				reg = <0xd2 0x2>;
+				bits = <6 5>;
+			};
+			tsens_s2_p1: s2_p1@d3 {
+				reg = <0xd3 0x1>;
+				bits = <3 5>;
+			};
+			tsens_s2_p2: s2_p2@d4 {
+				reg = <0xd4 0x1>;
+				bits = <0 5>;
+			};
+			tsens_s3_p1: s3_p1@d4 {
+				reg = <0xd4 0x2>;
+				bits = <5 5>;
+			};
+			tsens_s3_p2: s3_p2@d5 {
+				reg = <0xd5 0x1>;
+				bits = <2 5>;
+			};
+			tsens_s4_p1: s4_p1@d5 {
+				reg = <0xd5 0x2>;
+				bits = <7 5>;
+			};
+			tsens_s4_p2: s4_p2@d6 {
+				reg = <0xd6 0x2>;
+				bits = <4 5>;
+			};
+			tsens_base2: base2@d7 {
+				reg = <0xd7 0x1>;
+				bits = <1 7>;
+			};
+			tsens_mode: mode@ec {
+				reg = <0xef 0x1>;
+				bits = <5 3>;
 			};
 		};
 
@@ -473,8 +519,20 @@ tsens: thermal-sensor@4a9000 {
 			compatible = "qcom,msm8916-tsens", "qcom,tsens-v0_1";
 			reg = <0x004a9000 0x1000>, /* TM */
 			      <0x004a8000 0x1000>; /* SROT */
-			nvmem-cells = <&tsens_caldata>, <&tsens_calsel>;
-			nvmem-cell-names = "calib", "calib_sel";
+			nvmem-cells = <&tsens_mode>,
+				      <&tsens_base1>, <&tsens_base2>,
+				      <&tsens_s0_p1>, <&tsens_s0_p2>,
+				      <&tsens_s1_p1>, <&tsens_s1_p2>,
+				      <&tsens_s2_p1>, <&tsens_s2_p2>,
+				      <&tsens_s3_p1>, <&tsens_s3_p2>,
+				      <&tsens_s4_p1>, <&tsens_s4_p2>;
+			nvmem-cell-names = "mode",
+					   "base1", "base2",
+					   "s0_p1", "s0_p2",
+					   "s1_p1", "s1_p2",
+					   "s2_p1", "s2_p2",
+					   "s3_p1", "s3_p2",
+					   "s4_p1", "s4_p2";
 			#qcom,sensors = <5>;
 			interrupts = <GIC_SPI 184 IRQ_TYPE_LEVEL_HIGH>;
 			interrupt-names = "uplow";
-- 
2.35.1

Re: [RFC PATCH 09/10] arm64: dts: qcom: msm8916: specify per-sensor calibration cells

[Krzysztof Kozlowski]

On 10/09/2022 14:47, Dmitry Baryshkov wrote:
> Specify pre-parsed per-sensor calibration nvmem cells in the tsens
> device node rather than parsing the whole data blob in the driver.
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

You should describe the dependencies - the patchset is non-bisectable as
it depends on the driver changes and DTS cannot go along with DTS in the
same branch.

This must wait for next release.

Best regards,
Krzysztof

[RFC PATCH 10/10] arm64: dts: qcom: qcs404: specify per-sensor calibration cells

[Dmitry Baryshkov]

Specify pre-parsed per-sensor calibration nvmem cells in the tsens
device node rather than parsing the whole data blob in the driver.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 arch/arm64/boot/dts/qcom/qcs404.dtsi | 120 ++++++++++++++++++++++++++-
 1 file changed, 117 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/qcs404.dtsi b/arch/arm64/boot/dts/qcom/qcs404.dtsi
index 9ab990061522..bcbbd5ede2a1 100644
--- a/arch/arm64/boot/dts/qcom/qcs404.dtsi
+++ b/arch/arm64/boot/dts/qcom/qcs404.dtsi
@@ -370,13 +370,105 @@ qfprom: qfprom@a4000 {
 			reg = <0x000a4000 0x1000>;
 			#address-cells = <1>;
 			#size-cells = <1>;
-			tsens_caldata: caldata@d0 {
+			tsens_caldata: caldata@1f8 {
 				reg = <0x1f8 0x14>;
 			};
 			cpr_efuse_speedbin: speedbin@13c {
 				reg = <0x13c 0x4>;
 				bits = <2 3>;
 			};
+			tsens_s0_p1: s0_p1@1f8 {
+				reg = <0x1f8 0x1>;
+				bits = <0 6>;
+			};
+			tsens_s0_p2: s0_p2@1f8 {
+				reg = <0x1f8 0x2>;
+				bits = <6 6>;
+			};
+			tsens_s1_p1: s1_p1@1f9 {
+				reg = <0x1f9 0x2>;
+				bits = <4 6>;
+			};
+			tsens_s1_p2: s1_p2@1fa {
+				reg = <0x1fa 0x1>;
+				bits = <2 6>;
+			};
+			tsens_s2_p1: s2_p1@1fb {
+				reg = <0x1fb 0x1>;
+				bits = <0 6>;
+			};
+			tsens_s2_p2: s2_p2@1fb {
+				reg = <0x1fb 0x2>;
+				bits = <6 6>;
+			};
+			tsens_s3_p1: s3_p1@1fc {
+				reg = <0x1fc 0x2>;
+				bits = <4 6>;
+			};
+			tsens_s3_p2: s3_p2@1fd {
+				reg = <0x1fd 0x1>;
+				bits = <2 6>;
+			};
+			tsens_s4_p1: s4_p1@1fe {
+				reg = <0x1fe 0x1>;
+				bits = <0 6>;
+			};
+			tsens_s4_p2: s4_p2@1fe {
+				reg = <0x1fe 0x2>;
+				bits = <6 6>;
+			};
+			tsens_s5_p1: s5_p1@200 {
+				reg = <0x200 0x1>;
+				bits = <0 6>;
+			};
+			tsens_s5_p2: s5_p2@200 {
+				reg = <0x200 0x2>;
+				bits = <6 6>;
+			};
+			tsens_s6_p1: s6_p1@201 {
+				reg = <0x201 0x2>;
+				bits = <4 6>;
+			};
+			tsens_s6_p2: s6_p2@202 {
+				reg = <0x202 0x1>;
+				bits = <2 6>;
+			};
+			tsens_s7_p1: s7_p1@203 {
+				reg = <0x203 0x1>;
+				bits = <0 6>;
+			};
+			tsens_s7_p2: s7_p2@203 {
+				reg = <0x203 0x2>;
+				bits = <6 6>;
+			};
+			tsens_s8_p1: s8_p1@204 {
+				reg = <0x204 0x2>;
+				bits = <4 6>;
+			};
+			tsens_s8_p2: s8_p2@205 {
+				reg = <0x205 0x1>;
+				bits = <2 6>;
+			};
+			tsens_s9_p1: s9_p1@206 {
+				reg = <0x206 0x1>;
+				bits = <0 6>;
+			};
+			tsens_s9_p2: s9_p2@206 {
+				reg = <0x206 0x2>;
+				bits = <6 6>;
+			};
+			tsens_mode: mode@208 {
+				reg = <0x208 1>;
+				bits = <0 3>;
+			};
+			tsens_base1: base1@208 {
+				reg = <0x208 2>;
+				bits = <3 8>;
+			};
+			tsens_base2: base2@208 {
+				reg = <0x209 2>;
+				bits = <3 8>;
+			};
 			cpr_efuse_quot_offset1: qoffset1@231 {
 				reg = <0x231 0x4>;
 				bits = <4 7>;
@@ -451,8 +543,30 @@ tsens: thermal-sensor@4a9000 {
 			compatible = "qcom,qcs404-tsens", "qcom,tsens-v1";
 			reg = <0x004a9000 0x1000>, /* TM */
 			      <0x004a8000 0x1000>; /* SROT */
-			nvmem-cells = <&tsens_caldata>;
-			nvmem-cell-names = "calib";
+			nvmem-cells = <&tsens_caldata>, <&tsens_mode>,
+				      <&tsens_base1>, <&tsens_base2>,
+				      <&tsens_s0_p1>, <&tsens_s0_p2>,
+				      <&tsens_s1_p1>, <&tsens_s1_p2>,
+				      <&tsens_s2_p1>, <&tsens_s2_p2>,
+				      <&tsens_s3_p1>, <&tsens_s3_p2>,
+				      <&tsens_s4_p1>, <&tsens_s4_p2>,
+				      <&tsens_s5_p1>, <&tsens_s5_p2>,
+				      <&tsens_s6_p1>, <&tsens_s6_p2>,
+				      <&tsens_s7_p1>, <&tsens_s7_p2>,
+				      <&tsens_s8_p1>, <&tsens_s8_p2>,
+				      <&tsens_s9_p1>, <&tsens_s9_p2>;
+			nvmem-cell-names = "calib", "mode",
+					   "base1", "base2",
+					   "s0_p1", "s0_p2",
+					   "s1_p1", "s1_p2",
+					   "s2_p1", "s2_p2",
+					   "s3_p1", "s3_p2",
+					   "s4_p1", "s4_p2",
+					   "s5_p1", "s5_p2",
+					   "s6_p1", "s6_p2",
+					   "s7_p1", "s7_p2",
+					   "s8_p1", "s8_p2",
+					   "s9_p1", "s9_p2";
 			#qcom,sensors = <10>;
 			interrupts = <GIC_SPI 184 IRQ_TYPE_LEVEL_HIGH>;
 			interrupt-names = "uplow";
-- 
2.35.1

Re: [RFC PATCH 00/10] thermal/drivers/tsens: specify nvmem cells in DT rather than parsing them manually

[Stephan Gerhold]

Hi Dmitry,

On Sat, Sep 10, 2022 at 03:46:51PM +0300, Dmitry Baryshkov wrote:
> Historically the tsens driver fetches the calibration data as a blob and
> then parses the blob on its own. This results in semi-duplicated code
> spreading over the platform-specific functions.
> 
> This patch series changes tsens calibration code to use pre-parsed nvmem
> cells rather than parsing the blob in the driver. For backwards
> compatibility the old code is left in place for msm8916 and qcs404, two
> platforms which have in-tree DT files. For msm8974 the original function
> is left intact, since it differs significantly (and I can not test the
> code on msm8974). For all other affected platforms the old parsing code
> has been dropped as a part of this RFC.
> 
> The code was tested on msm8916 and qcs404 only, thus it is being sent as
> an RFC.
> 

Thanks a lot for working on this!

After thinking about this for a while I wonder if we can go even a step
further: Can we drop SoC-specific code entirely for 8939 and 9607 and
match the generic compatible (qcom,tsens-v0_1)? This would allow most
v0.1 plaforms to use generic code like for qcom,tsens-v2.

AFAICT with your changes only the following remains SoC-specific:

  - hw_ids (actually only needed for 8939 since 9607 has standard IDs)

While two other things are already handled:

  - num_sensors (the driver supports "#qcom,sensors" in DT already)
  - tsens_calibrate_nvmem() shift (AFAICT in downstream msm-tsens.c
    everything except 8916 uses shift = 2. 8916 needs special handling
    anyway for the backwards compatibility)

Having the generic compatible would allow me to add MSM8909 without any
code changes at all (just DT schema addition).

For 8939 we could read the hw_ids from the DT with something like:

	qcom,sensors = <0 1 2 3 5 6 7 8 9 10>;

And actually there are two revisions of 8939, the older one has one
sensor less (msm-3.10: msm8939-common.dtsi vs msm8939-v3.0.dtsi).
This could also be easily handled from the DT without any code changes:

	qcom,sensors = <0 1 2 3 5 6 7 8 9>;

The diff could be something like the following (I did not test it yet).

What do you think?

Thanks,
Stephan

 drivers/thermal/qcom/tsens-v0_1.c | 32 +++++---------------------------
 drivers/thermal/qcom/tsens.c      | 27 ++++++++++++++++++---------
 drivers/thermal/qcom/tsens.h      |  2 +-
 3 files changed, 24 insertions(+), 37 deletions(-)

diff --git a/drivers/thermal/qcom/tsens-v0_1.c b/drivers/thermal/qcom/tsens-v0_1.c
index c3613b7ccc19..8bfa85b31146 100644
--- a/drivers/thermal/qcom/tsens-v0_1.c
+++ b/drivers/thermal/qcom/tsens-v0_1.c
@@ -194,11 +194,6 @@ static int calibrate_8916(struct tsens_priv *priv)
 	return 0;
 }
 
-static int calibrate_8939(struct tsens_priv *priv)
-{
-	return tsens_calibrate_nvmem(priv, 2);
-}
-
 static int calibrate_8974(struct tsens_priv *priv)
 {
 	int base1 = 0, base2 = 0, i;
@@ -335,7 +330,7 @@ static int calibrate_8974(struct tsens_priv *priv)
 	return 0;
 }
 
-static int calibrate_9607(struct tsens_priv *priv)
+static int calibrate_dt(struct tsens_priv *priv)
 {
 	return tsens_calibrate_nvmem(priv, 2);
 }
@@ -401,21 +396,6 @@ struct tsens_plat_data data_8916 = {
 	.fields	= tsens_v0_1_regfields,
 };
 
-static const struct tsens_ops ops_8939 = {
-	.init		= init_common,
-	.calibrate	= calibrate_8939,
-	.get_temp	= get_temp_common,
-};
-
-struct tsens_plat_data data_8939 = {
-	.num_sensors	= 10,
-	.ops		= &ops_8939,
-	.hw_ids		= (unsigned int []){ 0, 1, 2, 4, 5, 6, 7, 8, 9, 10 },
-
-	.feat		= &tsens_v0_1_feat,
-	.fields	= tsens_v0_1_regfields,
-};
-
 static const struct tsens_ops ops_8974 = {
 	.init		= init_common,
 	.calibrate	= calibrate_8974,
@@ -429,16 +409,14 @@ struct tsens_plat_data data_8974 = {
 	.fields	= tsens_v0_1_regfields,
 };
 
-static const struct tsens_ops ops_9607 = {
+static const struct tsens_ops ops_generic_v0_1 = {
 	.init		= init_common,
-	.calibrate	= calibrate_9607,
+	.calibrate	= calibrate_dt,
 	.get_temp	= get_temp_common,
 };
 
-struct tsens_plat_data data_9607 = {
-	.num_sensors	= 5,
-	.ops		= &ops_9607,
-	.hw_ids		= (unsigned int []){ 0, 1, 2, 3, 4 },
+struct tsens_plat_data data_tsens_v0_1 = {
+	.ops		= &ops_generic_v0_1
 	.feat		= &tsens_v0_1_feat,
 	.fields	= tsens_v0_1_regfields,
 };
diff --git a/drivers/thermal/qcom/tsens.c b/drivers/thermal/qcom/tsens.c
index e69134a6419a..c4772962fb94 100644
--- a/drivers/thermal/qcom/tsens.c
+++ b/drivers/thermal/qcom/tsens.c
@@ -1021,15 +1021,9 @@ static const struct of_device_id tsens_table[] = {
 	{
 		.compatible = "qcom,ipq8064-tsens",
 		.data = &data_8960,
-	}, {
-		.compatible = "qcom,mdm9607-tsens",
-		.data = &data_9607,
 	}, {
 		.compatible = "qcom,msm8916-tsens",
 		.data = &data_8916,
-	}, {
-		.compatible = "qcom,msm8939-tsens",
-		.data = &data_8939,
 	}, {
 		.compatible = "qcom,msm8960-tsens",
 		.data = &data_8960,
@@ -1042,6 +1036,9 @@ static const struct of_device_id tsens_table[] = {
 	}, {
 		.compatible = "qcom,msm8996-tsens",
 		.data = &data_8996,
+	}, {
+		.compatible = "qcom,tsens-v0_1",
+		.data = &data_tsens_v0_1,
 	}, {
 		.compatible = "qcom,tsens-v1",
 		.data = &data_tsens_v1,
@@ -1152,6 +1149,8 @@ static int tsens_probe(struct platform_device *pdev)
 	struct tsens_priv *priv;
 	const struct tsens_plat_data *data;
 	const struct of_device_id *id;
+	unsigned int *hw_ids;
+	unsigned int dt_hw_ids[MAX_SENSORS];
 	u32 num_sensors;
 
 	if (pdev->dev.of_node)
@@ -1168,10 +1167,20 @@ static int tsens_probe(struct platform_device *pdev)
 		data = &data_8960;
 
 	num_sensors = data->num_sensors;
+	hw_ids = data->hw_ids;
 
-	if (np)
+	if (np) {
 		of_property_read_u32(np, "#qcom,sensors", &num_sensors);
 
+		ret = of_property_read_variable_u32_array(np, "qcom,sensors",
+							  dt_hw_ids,
+							  1, MAX_SENSORS);
+		if (ret > 0) {
+			hw_ids = dt_hw_ids;
+			num_sensors = ret;
+		}
+	}
+
 	if (num_sensors <= 0) {
 		dev_err(dev, "%s: invalid number of sensors\n", __func__);
 		return -EINVAL;
@@ -1187,8 +1196,8 @@ static int tsens_probe(struct platform_device *pdev)
 	priv->num_sensors = num_sensors;
 	priv->ops = data->ops;
 	for (i = 0;  i < priv->num_sensors; i++) {
-		if (data->hw_ids)
-			priv->sensor[i].hw_id = data->hw_ids[i];
+		if (hw_ids)
+			priv->sensor[i].hw_id = hw_ids[i];
 		else
 			priv->sensor[i].hw_id = i;
 	}
diff --git a/drivers/thermal/qcom/tsens.h b/drivers/thermal/qcom/tsens.h
index 504ed3394a79..439f356f0177 100644
--- a/drivers/thermal/qcom/tsens.h
+++ b/drivers/thermal/qcom/tsens.h
@@ -589,7 +589,7 @@ int get_temp_common(const struct tsens_sensor *s, int *temp);
 extern struct tsens_plat_data data_8960;
 
 /* TSENS v0.1 targets */
-extern struct tsens_plat_data data_8916, data_8939, data_8974, data_9607;
+extern struct tsens_plat_data data_8916, data_8974, data_tsens_v0_1;
 
 /* TSENS v1 targets */
 extern struct tsens_plat_data data_tsens_v1, data_8976;

Re: [RFC PATCH 00/10] thermal/drivers/tsens: specify nvmem cells in DT rather than parsing them manually

[Dmitry Baryshkov]

Hi,

On 22/09/2022 20:23, Stephan Gerhold wrote:
> Hi Dmitry,
> 
> On Sat, Sep 10, 2022 at 03:46:51PM +0300, Dmitry Baryshkov wrote:
>> Historically the tsens driver fetches the calibration data as a blob and
>> then parses the blob on its own. This results in semi-duplicated code
>> spreading over the platform-specific functions.
>>
>> This patch series changes tsens calibration code to use pre-parsed nvmem
>> cells rather than parsing the blob in the driver. For backwards
>> compatibility the old code is left in place for msm8916 and qcs404, two
>> platforms which have in-tree DT files. For msm8974 the original function
>> is left intact, since it differs significantly (and I can not test the
>> code on msm8974). For all other affected platforms the old parsing code
>> has been dropped as a part of this RFC.
>>
>> The code was tested on msm8916 and qcs404 only, thus it is being sent as
>> an RFC.
>>
> 
> Thanks a lot for working on this!
> 
> After thinking about this for a while I wonder if we can go even a step
> further: Can we drop SoC-specific code entirely for 8939 and 9607 and
> match the generic compatible (qcom,tsens-v0_1)? This would allow most
> v0.1 plaforms to use generic code like for qcom,tsens-v2.

While this idea looks appealing, I think it's a bit against our custom 
to put hardware details into the driver rather than putting them into 
the DT. So, I think, the 8939 will have to stay as is, while for the 
9607 and maybe several other devices it should be possible to create a 
fallback entry.

> 
> AFAICT with your changes only the following remains SoC-specific:
> 
>    - hw_ids (actually only needed for 8939 since 9607 has standard IDs)

As I wrote, I wouldn't put this into DT.

> 
> While two other things are already handled:
> 
>    - num_sensors (the driver supports "#qcom,sensors" in DT already)
>    - tsens_calibrate_nvmem() shift (AFAICT in downstream msm-tsens.c
>      everything except 8916 uses shift = 2. 8916 needs special handling
>      anyway for the backwards compatibility)
> 
> Having the generic compatible would allow me to add MSM8909 without any
> code changes at all (just DT schema addition).
> 
> For 8939 we could read the hw_ids from the DT with something like:
> 
> 	qcom,sensors = <0 1 2 3 5 6 7 8 9 10>;
> 
> And actually there are two revisions of 8939, the older one has one
> sensor less (msm-3.10: msm8939-common.dtsi vs msm8939-v3.0.dtsi).
> This could also be easily handled from the DT without any code changes:
> 
> 	qcom,sensors = <0 1 2 3 5 6 7 8 9>;

Usually we only care about the latest revision of the chip, earlier 
revisions typically correspond to engineering samples, never hitting the 
actual consumer devices.

> 
> The diff could be something like the following (I did not test it yet).
> 
> What do you think?
I'd like to sort the calibration data for 8976 first. At this moment I'm 
waiting for the 8976 data to be tested. Also it would be nice to be able 
to cleanup the 8976 calibration code.

-- 
With best wishes
Dmitry

Re: [RFC PATCH 00/10] thermal/drivers/tsens: specify nvmem cells in DT rather than parsing them manually

[Stephan Gerhold]

On Sat, Sep 24, 2022 at 09:58:56PM +0300, Dmitry Baryshkov wrote:
> On 22/09/2022 20:23, Stephan Gerhold wrote:
> > On Sat, Sep 10, 2022 at 03:46:51PM +0300, Dmitry Baryshkov wrote:
> > > Historically the tsens driver fetches the calibration data as a blob and
> > > then parses the blob on its own. This results in semi-duplicated code
> > > spreading over the platform-specific functions.
> > > 
> > > This patch series changes tsens calibration code to use pre-parsed nvmem
> > > cells rather than parsing the blob in the driver. For backwards
> > > compatibility the old code is left in place for msm8916 and qcs404, two
> > > platforms which have in-tree DT files. For msm8974 the original function
> > > is left intact, since it differs significantly (and I can not test the
> > > code on msm8974). For all other affected platforms the old parsing code
> > > has been dropped as a part of this RFC.
> > > 
> > > The code was tested on msm8916 and qcs404 only, thus it is being sent as
> > > an RFC.
> > > 
> > 
> > Thanks a lot for working on this!
> > 
> > After thinking about this for a while I wonder if we can go even a step
> > further: Can we drop SoC-specific code entirely for 8939 and 9607 and
> > match the generic compatible (qcom,tsens-v0_1)? This would allow most
> > v0.1 plaforms to use generic code like for qcom,tsens-v2.
> 
> While this idea looks appealing, I think it's a bit against our custom to
> put hardware details into the driver rather than putting them into the DT.
> So, I think, the 8939 will have to stay as is, while for the 9607 and maybe
> several other devices it should be possible to create a fallback entry.
> 

IMHO the existing tsens-v2 support is a good example that it's sometimes
better to have some minor hardware details in the DT so the driver does
not have to be changed for every single platform. Extending from
specifying the number of sensors in the DT to the exact set of sensors
is not a very big step.

Also, aren't you also going against the custom here by moving the fuse
hardware details to the DT? :)

> [...]
> > And actually there are two revisions of 8939, the older one has one
> > sensor less (msm-3.10: msm8939-common.dtsi vs msm8939-v3.0.dtsi).
> > This could also be easily handled from the DT without any code changes:
> > 
> > 	qcom,sensors = <0 1 2 3 5 6 7 8 9>;
> 
> Usually we only care about the latest revision of the chip, earlier
> revisions typically correspond to engineering samples, never hitting the
> actual consumer devices.
> 

I'm afraid we might have to care about both revisions here - I recently
checked a couple of MSM8939 devices in postmarketOS and there are
definitely two different revisions used in production - they are easily
identifiable since they have different CPU revisions in the "lscpu"
output (Cortex-A53 r0p1 vs r0p4).

Thanks,
Stephan

Re: [RFC PATCH 00/10] thermal/drivers/tsens: specify nvmem cells in DT rather than parsing them manually

[Dmitry Baryshkov]

On 25 September 2022 13:20:09 GMT+03:00, Stephan Gerhold <stephan@gerhold.net> wrote:
>On Sat, Sep 24, 2022 at 09:58:56PM +0300, Dmitry Baryshkov wrote:
>> On 22/09/2022 20:23, Stephan Gerhold wrote:
>> > On Sat, Sep 10, 2022 at 03:46:51PM +0300, Dmitry Baryshkov wrote:
>> > > Historically the tsens driver fetches the calibration data as a blob and
>> > > then parses the blob on its own. This results in semi-duplicated code
>> > > spreading over the platform-specific functions.
>> > > 
>> > > This patch series changes tsens calibration code to use pre-parsed nvmem
>> > > cells rather than parsing the blob in the driver. For backwards
>> > > compatibility the old code is left in place for msm8916 and qcs404, two
>> > > platforms which have in-tree DT files. For msm8974 the original function
>> > > is left intact, since it differs significantly (and I can not test the
>> > > code on msm8974). For all other affected platforms the old parsing code
>> > > has been dropped as a part of this RFC.
>> > > 
>> > > The code was tested on msm8916 and qcs404 only, thus it is being sent as
>> > > an RFC.
>> > > 
>> > 
>> > Thanks a lot for working on this!
>> > 
>> > After thinking about this for a while I wonder if we can go even a step
>> > further: Can we drop SoC-specific code entirely for 8939 and 9607 and
>> > match the generic compatible (qcom,tsens-v0_1)? This would allow most
>> > v0.1 plaforms to use generic code like for qcom,tsens-v2.
>> 
>> While this idea looks appealing, I think it's a bit against our custom to
>> put hardware details into the driver rather than putting them into the DT.
>> So, I think, the 8939 will have to stay as is, while for the 9607 and maybe
>> several other devices it should be possible to create a fallback entry.
>> 
>
>IMHO the existing tsens-v2 support is a good example that it's sometimes
>better to have some minor hardware details in the DT so the driver does
>not have to be changed for every single platform. Extending from
>specifying the number of sensors in the DT to the exact set of sensors
>is not a very big step.

Fine, I will take a look.

>
>Also, aren't you also going against the custom here by moving the fuse
>hardware details to the DT? :)

Not quite. Fuses are completely software thing. 

>
>> [...]
>> > And actually there are two revisions of 8939, the older one has one
>> > sensor less (msm-3.10: msm8939-common.dtsi vs msm8939-v3.0.dtsi).
>> > This could also be easily handled from the DT without any code changes:
>> > 
>> > 	qcom,sensors = <0 1 2 3 5 6 7 8 9>;
>> 
>> Usually we only care about the latest revision of the chip, earlier
>> revisions typically correspond to engineering samples, never hitting the
>> actual consumer devices.
>> 
>
>I'm afraid we might have to care about both revisions here - I recently
>checked a couple of MSM8939 devices in postmarketOS and there are
>definitely two different revisions used in production - they are easily
>identifiable since they have different CPU revisions in the "lscpu"
>output (Cortex-A53 r0p1 vs r0p4).

Ugh. 

>
>Thanks,
>Stephan

-- 
With best wishes
Dmitry

Re: [RFC PATCH 00/10] thermal/drivers/tsens: specify nvmem cells in DT rather than parsing them manually

[Stephan Gerhold]

On Sun, Sep 25, 2022 at 02:21:11PM +0300, Dmitry Baryshkov wrote:
> On 25 September 2022 13:20:09 GMT+03:00, Stephan Gerhold <stephan@gerhold.net> wrote:
> >On Sat, Sep 24, 2022 at 09:58:56PM +0300, Dmitry Baryshkov wrote:
> >> On 22/09/2022 20:23, Stephan Gerhold wrote:
> >> > On Sat, Sep 10, 2022 at 03:46:51PM +0300, Dmitry Baryshkov wrote:
> >> > > Historically the tsens driver fetches the calibration data as a blob and
> >> > > then parses the blob on its own. This results in semi-duplicated code
> >> > > spreading over the platform-specific functions.
> >> > > 
> >> > > This patch series changes tsens calibration code to use pre-parsed nvmem
> >> > > cells rather than parsing the blob in the driver. For backwards
> >> > > compatibility the old code is left in place for msm8916 and qcs404, two
> >> > > platforms which have in-tree DT files. For msm8974 the original function
> >> > > is left intact, since it differs significantly (and I can not test the
> >> > > code on msm8974). For all other affected platforms the old parsing code
> >> > > has been dropped as a part of this RFC.
> >> > > 
> >> > > The code was tested on msm8916 and qcs404 only, thus it is being sent as
> >> > > an RFC.
> >> > > 
> >> > 
> >> > Thanks a lot for working on this!
> >> > 
> >> > After thinking about this for a while I wonder if we can go even a step
> >> > further: Can we drop SoC-specific code entirely for 8939 and 9607 and
> >> > match the generic compatible (qcom,tsens-v0_1)? This would allow most
> >> > v0.1 plaforms to use generic code like for qcom,tsens-v2.
> >> 
> >> While this idea looks appealing, I think it's a bit against our custom to
> >> put hardware details into the driver rather than putting them into the DT.
> >> So, I think, the 8939 will have to stay as is, while for the 9607 and maybe
> >> several other devices it should be possible to create a fallback entry.
> >> 
> >
> >IMHO the existing tsens-v2 support is a good example that it's sometimes
> >better to have some minor hardware details in the DT so the driver does
> >not have to be changed for every single platform. Extending from
> >specifying the number of sensors in the DT to the exact set of sensors
> >is not a very big step.
> 
> Fine, I will take a look.
> 

Thanks!

> >
> >Also, aren't you also going against the custom here by moving the fuse
> >hardware details to the DT? :)
> 
> Not quite. Fuses are completely software thing. 
> 

Good point, but this depends on the point of view: Since those fuses are
likely "burned in" at the factory I would consider them to be part of
"hardware" like everything else. Even if I wanted to I probably couldn't
change anything about the layout, at least as a user (and probably even
as a customer). :-)

> >
> >> [...]
> >> > And actually there are two revisions of 8939, the older one has one
> >> > sensor less (msm-3.10: msm8939-common.dtsi vs msm8939-v3.0.dtsi).
> >> > This could also be easily handled from the DT without any code changes:
> >> > 
> >> > 	qcom,sensors = <0 1 2 3 5 6 7 8 9>;
> >> 
> >> Usually we only care about the latest revision of the chip, earlier
> >> revisions typically correspond to engineering samples, never hitting the
> >> actual consumer devices.
> >> 
> >
> >I'm afraid we might have to care about both revisions here - I recently
> >checked a couple of MSM8939 devices in postmarketOS and there are
> >definitely two different revisions used in production - they are easily
> >identifiable since they have different CPU revisions in the "lscpu"
> >output (Cortex-A53 r0p1 vs r0p4).
> 
> Ugh. 
> 

Indeed.

Thanks,
Stephan