[PATCH v4 0/7] Add support for ipq8064 tsens

[PATCH v4 0/7] Add support for ipq8064 tsens

[Ansuel Smith]

Ipq8064 SoCs tsens driver is based on 8960 tsens driver. This patchset 
expand the 8960 unused driver with interrupt support and set_trip point.
Ipq8064 needs to be registered as a gcc child as the tsens regs on
this platform are shared with the controller.

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 (7):
  ipq806x: gcc: add support for child probe
  drivers: thermal: tsens: try load regmap from parent for 8960
  drivers: thermal: tsens: add ipq8064 support
  dt-bindings: thermal: tsens: document ipq8064 bindings
  drivers: thermal: tsens: add interrupt support for 9860 driver
  drivers: thermal: tsens: add support for custom set_trip function
  drivers: thermal: tsens: add set_trip support for 8960

 .../bindings/thermal/qcom-tsens.yaml          |  50 ++-
 drivers/clk/qcom/gcc-ipq806x.c                |   2 +-
 drivers/thermal/qcom/tsens-8960.c             | 287 +++++++++++++++++-
 drivers/thermal/qcom/tsens.c                  |   7 +
 drivers/thermal/qcom/tsens.h                  |   5 +
 5 files changed, 328 insertions(+), 23 deletions(-)

-- 
2.27.0

[PATCH v4 1/7] ipq806x: gcc: add support for child probe

[Ansuel Smith]

Add support for child probing needed for tsens driver that share the
seme regs of gcc for this platform.

Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
---
 drivers/clk/qcom/gcc-ipq806x.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/qcom/gcc-ipq806x.c b/drivers/clk/qcom/gcc-ipq806x.c
index a8456e09c44d..d6b7adb4be38 100644
--- a/drivers/clk/qcom/gcc-ipq806x.c
+++ b/drivers/clk/qcom/gcc-ipq806x.c
@@ -3089,7 +3089,7 @@ static int gcc_ipq806x_probe(struct platform_device *pdev)
 	regmap_write(regmap, 0x3cf8, 8);
 	regmap_write(regmap, 0x3d18, 8);
 
-	return 0;
+	return of_platform_populate(pdev->dev.of_node, NULL, NULL, &pdev->dev);
 }
 
 static struct platform_driver gcc_ipq806x_driver = {
-- 
2.27.0

Re: [PATCH v4 1/7] ipq806x: gcc: add support for child probe

[Amit Kucheria]

On Thu, Jul 16, 2020 at 7:58 AM Ansuel Smith <ansuelsmth@gmail.com> wrote:
>
> Add support for child probing needed for tsens driver that share the
> seme regs of gcc for this platform.

Typo: same


>
> Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
> ---
>  drivers/clk/qcom/gcc-ipq806x.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/clk/qcom/gcc-ipq806x.c b/drivers/clk/qcom/gcc-ipq806x.c
> index a8456e09c44d..d6b7adb4be38 100644
> --- a/drivers/clk/qcom/gcc-ipq806x.c
> +++ b/drivers/clk/qcom/gcc-ipq806x.c
> @@ -3089,7 +3089,7 @@ static int gcc_ipq806x_probe(struct platform_device *pdev)
>         regmap_write(regmap, 0x3cf8, 8);
>         regmap_write(regmap, 0x3d18, 8);
>
> -       return 0;
> +       return of_platform_populate(pdev->dev.of_node, NULL, NULL, &pdev->dev);
>  }
>
>  static struct platform_driver gcc_ipq806x_driver = {
> --
> 2.27.0
>

Re: [PATCH v4 1/7] ipq806x: gcc: add support for child probe

[Stephen Boyd]

Quoting Amit Kucheria (2020-07-20 02:41:44)
> On Thu, Jul 16, 2020 at 7:58 AM Ansuel Smith <ansuelsmth@gmail.com> wrote:
> >
> > Add support for child probing needed for tsens driver that share the
> > seme regs of gcc for this platform.
> 
> Typo: same
> 

Otherwise reviewed-by? Because I can throw this into the clk tree with
the typo fixed.

Re: [PATCH v4 1/7] ipq806x: gcc: add support for child probe

[Amit Kucheria]

On Tue, Jul 21, 2020 at 11:27 AM Amit Kucheria <amit.kucheria@linaro.org> wrote:
>
>
>
> On Tue, 21 Jul, 2020, 05:06 Stephen Boyd, <sboyd@kernel.org> wrote:
>>
>> Quoting Amit Kucheria (2020-07-20 02:41:44)
>> > On Thu, Jul 16, 2020 at 7:58 AM Ansuel Smith <ansuelsmth@gmail.com> wrote:
>> > >
>> > > Add support for child probing needed for tsens driver that share the
>> > > seme regs of gcc for this platform.
>> >
>> > Typo: same
>> >
>>
>> Otherwise reviewed-by? Because I can throw this into the clk tree with
>> the typo fixed.
>
>
> Yes, the rest of the series need work imo, but this patch looks ok to populate the child nodes in OF.
>
> Reviewed-by: Amit Kucheria <amit.kucheria@linaro.org>

Replied earlier from a phone, which resulted in HTML email. Resending.

The rest of the series need work imo, but this patch looks ok to
populate the child nodes in OF.

Reviewed-by: Amit Kucheria <amit.kucheria@linaro.org>

Re: [PATCH v4 1/7] ipq806x: gcc: add support for child probe

[Stephen Boyd]

Quoting Ansuel Smith (2020-07-15 19:28:10)
> Add support for child probing needed for tsens driver that share the
> seme regs of gcc for this platform.
> 
> Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
> ---

Applied to clk-next with the typo fix and tag.

[PATCH v4 2/7] drivers: thermal: tsens: try load regmap from parent for 8960

[Ansuel Smith]

Devices based on 8060 tsens driver (ipq8064) use the reg of the gcc
driver. Try to load the regmap of the parent as they share the same
regs.

Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
---
 drivers/thermal/qcom/tsens-8960.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/thermal/qcom/tsens-8960.c b/drivers/thermal/qcom/tsens-8960.c
index 2a28a5af209e..45788eb3c666 100644
--- a/drivers/thermal/qcom/tsens-8960.c
+++ b/drivers/thermal/qcom/tsens-8960.c
@@ -7,6 +7,7 @@
 #include <linux/delay.h>
 #include <linux/bitops.h>
 #include <linux/regmap.h>
+#include <linux/mfd/syscon.h>
 #include <linux/thermal.h>
 #include "tsens.h"
 
@@ -168,8 +169,17 @@ static int init_8960(struct tsens_priv *priv)
 	u32 reg_cntl;
 
 	priv->tm_map = dev_get_regmap(priv->dev, NULL);
-	if (!priv->tm_map)
+	if (!priv->tm_map) {
+		struct device *parent = priv->dev->parent;
+
+		if (parent)
+			priv->tm_map = syscon_node_to_regmap(parent->of_node);
+	}
+
+	if (!priv->tm_map || IS_ERR(priv->tm_map)) {
+		dev_err(priv->dev, "failed to get tsens regmap\n");
 		return -ENODEV;
+	}
 
 	/*
 	 * The status registers for each sensor are discontiguous
-- 
2.27.0

Re: [PATCH v4 2/7] drivers: thermal: tsens: try load regmap from parent for 8960

[Amit Kucheria]

On Thu, Jul 16, 2020 at 7:58 AM Ansuel Smith <ansuelsmth@gmail.com> wrote:
>
> Devices based on 8060 tsens driver (ipq8064) use the reg of the gcc

typo: 8960

> driver. Try to load the regmap of the parent as they share the same
> regs.
>
> Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
> ---
>  drivers/thermal/qcom/tsens-8960.c | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/thermal/qcom/tsens-8960.c b/drivers/thermal/qcom/tsens-8960.c
> index 2a28a5af209e..45788eb3c666 100644
> --- a/drivers/thermal/qcom/tsens-8960.c
> +++ b/drivers/thermal/qcom/tsens-8960.c
> @@ -7,6 +7,7 @@
>  #include <linux/delay.h>
>  #include <linux/bitops.h>
>  #include <linux/regmap.h>
> +#include <linux/mfd/syscon.h>
>  #include <linux/thermal.h>
>  #include "tsens.h"
>
> @@ -168,8 +169,17 @@ static int init_8960(struct tsens_priv *priv)
>         u32 reg_cntl;
>
>         priv->tm_map = dev_get_regmap(priv->dev, NULL);
> -       if (!priv->tm_map)
> +       if (!priv->tm_map) {
> +               struct device *parent = priv->dev->parent;
> +
> +               if (parent)
> +                       priv->tm_map = syscon_node_to_regmap(parent->of_node);
> +       }
> +
> +       if (!priv->tm_map || IS_ERR(priv->tm_map)) {
> +               dev_err(priv->dev, "failed to get tsens regmap\n");
>                 return -ENODEV;
> +       }
>
>         /*
>          * The status registers for each sensor are discontiguous
> --
> 2.27.0
>

[PATCH v4 3/7] drivers: thermal: tsens: add ipq8064 support

[Ansuel Smith]

Ipq8064 SoCs based use the same 8960 driver.

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

diff --git a/drivers/thermal/qcom/tsens.c b/drivers/thermal/qcom/tsens.c
index 39c4462e38f6..23f63dfbf13d 100644
--- a/drivers/thermal/qcom/tsens.c
+++ b/drivers/thermal/qcom/tsens.c
@@ -906,6 +906,9 @@ static const struct of_device_id tsens_table[] = {
 	}, {
 		.compatible = "qcom,msm8996-tsens",
 		.data = &data_8996,
+	}, {
+		.compatible = "qcom,ipq8064-tsens",
+		.data = &data_8960,
 	}, {
 		.compatible = "qcom,tsens-v1",
 		.data = &data_tsens_v1,
-- 
2.27.0

[PATCH v4 4/7] dt-bindings: thermal: tsens: document ipq8064 bindings

[Ansuel Smith]

Document the use of bindings used for ipq8064 SoCs tsens.
ipq8064 use the same gcc regs and is set as a child of the qcom gcc.

Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
---
 .../bindings/thermal/qcom-tsens.yaml          | 50 ++++++++++++++++---
 1 file changed, 43 insertions(+), 7 deletions(-)

diff --git a/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml b/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml
index d7be931b42d2..9d480e3943a2 100644
--- a/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml
+++ b/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml
@@ -19,6 +19,11 @@ description: |
 properties:
   compatible:
     oneOf:
+      - description: msm9860 TSENS based
+        items:
+          - enum:
+            - qcom,ipq8064-tsens
+
       - description: v0.1 of TSENS
         items:
           - enum:
@@ -85,12 +90,18 @@ properties:
       Number of cells required to uniquely identify the thermal sensors. Since
       we have multiple sensors this is set to 1
 
+required:
+  - compatible
+  - interrupts
+  - "#thermal-sensor-cells"
+
 allOf:
   - if:
       properties:
         compatible:
           contains:
             enum:
+              - qcom,ipq8064-tsens
               - qcom,msm8916-tsens
               - qcom,msm8974-tsens
               - qcom,msm8976-tsens
@@ -111,17 +122,42 @@ allOf:
         interrupt-names:
           minItems: 2
 
-required:
-  - compatible
-  - reg
-  - "#qcom,sensors"
-  - interrupts
-  - interrupt-names
-  - "#thermal-sensor-cells"
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - qcom,tsens-v0_1
+              - qcom,tsens-v1
+              - qcom,tsens-v2
+
+    then:
+      required:
+        - reg
+        - interrupt-names
+        - "#qcom,sensors"
 
 additionalProperties: false
 
 examples:
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    // Example msm9860 based SoC (ipq8064):
+    gcc: clock-controller {
+
+           /* ... */
+
+           tsens: thermal-sensor {
+                compatible = "qcom,ipq8064-tsens";
+
+                 nvmem-cells = <&tsens_calib>, <&tsens_calsel>;
+                 nvmem-cell-names = "calib", "calib_sel";
+                 interrupts = <GIC_SPI 178 IRQ_TYPE_LEVEL_HIGH>;
+
+                 #thermal-sensor-cells = <1>;
+          };
+    };
+
   - |
     #include <dt-bindings/interrupt-controller/arm-gic.h>
     // Example 1 (legacy: for pre v1 IP):
-- 
2.27.0

Re: [PATCH v4 4/7] dt-bindings: thermal: tsens: document ipq8064 bindings

[Rob Herring]

On Thu, 16 Jul 2020 04:28:13 +0200, Ansuel Smith wrote:
> Document the use of bindings used for ipq8064 SoCs tsens.
> ipq8064 use the same gcc regs and is set as a child of the qcom gcc.
> 
> Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
> ---
>  .../bindings/thermal/qcom-tsens.yaml          | 50 ++++++++++++++++---
>  1 file changed, 43 insertions(+), 7 deletions(-)
> 

Reviewed-by: Rob Herring <robh@kernel.org>

Re: [PATCH v4 4/7] dt-bindings: thermal: tsens: document ipq8064 bindings

[Amit Kucheria]

On Thu, Jul 16, 2020 at 7:58 AM Ansuel Smith <ansuelsmth@gmail.com> wrote:
>
> Document the use of bindings used for ipq8064 SoCs tsens.
> ipq8064 use the same gcc regs and is set as a child of the qcom gcc.
>
> Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
> ---
>  .../bindings/thermal/qcom-tsens.yaml          | 50 ++++++++++++++++---
>  1 file changed, 43 insertions(+), 7 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml b/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml
> index d7be931b42d2..9d480e3943a2 100644
> --- a/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml
> +++ b/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml
> @@ -19,6 +19,11 @@ description: |
>  properties:
>    compatible:
>      oneOf:
> +      - description: msm9860 TSENS based

Another variant of the number here: I've seen 8960, 8064 (correct) and
8060, 9860 (wrong) so far.

Just use 8960 throughout this series and then add a new patch at the
end of the series for a compatible for ipq8064.

> +        items:
> +          - enum:
> +            - qcom,ipq8064-tsens
> +
>        - description: v0.1 of TSENS
>          items:
>            - enum:
> @@ -85,12 +90,18 @@ properties:
>        Number of cells required to uniquely identify the thermal sensors. Since
>        we have multiple sensors this is set to 1
>
> +required:
> +  - compatible
> +  - interrupts
> +  - "#thermal-sensor-cells"
> +
>  allOf:
>    - if:
>        properties:
>          compatible:
>            contains:
>              enum:
> +              - qcom,ipq8064-tsens
>                - qcom,msm8916-tsens
>                - qcom,msm8974-tsens
>                - qcom,msm8976-tsens
> @@ -111,17 +122,42 @@ allOf:
>          interrupt-names:
>            minItems: 2
>
> -required:
> -  - compatible
> -  - reg
> -  - "#qcom,sensors"
> -  - interrupts
> -  - interrupt-names
> -  - "#thermal-sensor-cells"
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - qcom,tsens-v0_1
> +              - qcom,tsens-v1
> +              - qcom,tsens-v2
> +
> +    then:
> +      required:
> +        - reg
> +        - interrupt-names
> +        - "#qcom,sensors"
>
>  additionalProperties: false
>
>  examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    // Example msm9860 based SoC (ipq8064):
> +    gcc: clock-controller {
> +
> +           /* ... */
> +
> +           tsens: thermal-sensor {
> +                compatible = "qcom,ipq8064-tsens";
> +
> +                 nvmem-cells = <&tsens_calib>, <&tsens_calsel>;
> +                 nvmem-cell-names = "calib", "calib_sel";
> +                 interrupts = <GIC_SPI 178 IRQ_TYPE_LEVEL_HIGH>;
> +
> +                 #thermal-sensor-cells = <1>;
> +          };
> +    };
> +
>    - |
>      #include <dt-bindings/interrupt-controller/arm-gic.h>
>      // Example 1 (legacy: for pre v1 IP):
> --
> 2.27.0
>

[PATCH v4 5/7] drivers: thermal: tsens: add interrupt support for 9860 driver

[Ansuel Smith]

Add interrupt support for 9860 tsens driver used to set thermal trip
point for the system.

Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
---
 drivers/thermal/qcom/tsens-8960.c | 197 +++++++++++++++++++++++++++---
 drivers/thermal/qcom/tsens.h      |   3 +
 2 files changed, 186 insertions(+), 14 deletions(-)

diff --git a/drivers/thermal/qcom/tsens-8960.c b/drivers/thermal/qcom/tsens-8960.c
index 45788eb3c666..20d0bfb10f1f 100644
--- a/drivers/thermal/qcom/tsens-8960.c
+++ b/drivers/thermal/qcom/tsens-8960.c
@@ -8,6 +8,7 @@
 #include <linux/bitops.h>
 #include <linux/regmap.h>
 #include <linux/mfd/syscon.h>
+#include <linux/interrupt.h>
 #include <linux/thermal.h>
 #include "tsens.h"
 
@@ -27,7 +28,6 @@
 /* CNTL_ADDR bitmasks */
 #define EN			BIT(0)
 #define SW_RST			BIT(1)
-#define SENSOR0_EN		BIT(3)
 #define SLP_CLK_ENA		BIT(26)
 #define SLP_CLK_ENA_8660	BIT(24)
 #define MEASURE_PERIOD		1
@@ -41,14 +41,26 @@
 
 #define THRESHOLD_ADDR		0x3624
 /* THRESHOLD_ADDR bitmasks */
+#define THRESHOLD_MAX_CODE		0x20000
+#define THRESHOLD_MIN_CODE		0
 #define THRESHOLD_MAX_LIMIT_SHIFT	24
 #define THRESHOLD_MIN_LIMIT_SHIFT	16
 #define THRESHOLD_UPPER_LIMIT_SHIFT	8
 #define THRESHOLD_LOWER_LIMIT_SHIFT	0
+#define THRESHOLD_MAX_LIMIT_MASK	(THRESHOLD_MAX_CODE << \
+						THRESHOLD_MAX_LIMIT_SHIFT)
+#define THRESHOLD_MIN_LIMIT_MASK	(THRESHOLD_MAX_CODE << \
+						THRESHOLD_MIN_LIMIT_SHIFT)
+#define THRESHOLD_UPPER_LIMIT_MASK	(THRESHOLD_MAX_CODE << \
+						THRESHOLD_UPPER_LIMIT_SHIFT)
+#define THRESHOLD_LOWER_LIMIT_MASK	(THRESHOLD_MAX_CODE << \
+						THRESHOLD_LOWER_LIMIT_SHIFT)
 
 /* Initial temperature threshold values */
-#define LOWER_LIMIT_TH		0x50
-#define UPPER_LIMIT_TH		0xdf
+#define LOWER_LIMIT_TH_8960	0x50
+#define UPPER_LIMIT_TH_8960	0xdf
+#define LOWER_LIMIT_TH_8064	0x9d /* 95C */
+#define UPPER_LIMIT_TH_8064	0xa6 /* 105C */
 #define MIN_LIMIT_TH		0x0
 #define MAX_LIMIT_TH		0xff
 
@@ -57,6 +69,170 @@
 #define TRDY_MASK		BIT(7)
 #define TIMEOUT_US		100
 
+#define TSENS_EN		BIT(0)
+#define TSENS_SW_RST		BIT(1)
+#define TSENS_ADC_CLK_SEL	BIT(2)
+#define SENSOR0_EN		BIT(3)
+#define SENSOR1_EN		BIT(4)
+#define SENSOR2_EN		BIT(5)
+#define SENSOR3_EN		BIT(6)
+#define SENSOR4_EN		BIT(7)
+#define SENSORS_EN		(SENSOR0_EN | SENSOR1_EN | \
+				SENSOR2_EN | SENSOR3_EN | SENSOR4_EN)
+#define TSENS_8064_SENSOR5_EN				BIT(8)
+#define TSENS_8064_SENSOR6_EN				BIT(9)
+#define TSENS_8064_SENSOR7_EN				BIT(10)
+#define TSENS_8064_SENSOR8_EN				BIT(11)
+#define TSENS_8064_SENSOR9_EN				BIT(12)
+#define TSENS_8064_SENSOR10_EN				BIT(13)
+#define TSENS_8064_SENSORS_EN				(SENSORS_EN | \
+						TSENS_8064_SENSOR5_EN | \
+						TSENS_8064_SENSOR6_EN | \
+						TSENS_8064_SENSOR7_EN | \
+						TSENS_8064_SENSOR8_EN | \
+						TSENS_8064_SENSOR9_EN | \
+						TSENS_8064_SENSOR10_EN)
+
+u32 tsens_8960_slope[] = {
+			1176, 1176, 1154, 1176,
+			1111, 1132, 1132, 1199,
+			1132, 1199, 1132
+			};
+
+/* Temperature on y axis and ADC-code on x-axis */
+static inline int code_to_mdegC(u32 adc_code, const struct tsens_sensor *s)
+{
+	int slope, offset;
+
+	slope = thermal_zone_get_slope(s->tzd);
+	offset = CAL_MDEGC - slope * s->offset;
+
+	return adc_code * slope + offset;
+}
+
+static void notify_uspace_tsens_fn(struct work_struct *work)
+{
+	struct tsens_sensor *s = container_of(work, struct tsens_sensor,
+								notify_work);
+
+	sysfs_notify(&s->tzd->device.kobj, NULL, "type");
+}
+
+static void tsens_scheduler_fn(struct work_struct *work)
+{
+	struct tsens_priv *priv =
+		container_of(work, struct tsens_priv, tsens_work);
+	unsigned int threshold, threshold_low, code, reg, sensor;
+	unsigned long mask;
+	bool upper_th_x, lower_th_x;
+	int ret;
+
+	ret = regmap_read(priv->tm_map, STATUS_CNTL_ADDR_8064, &reg);
+	if (ret)
+		return;
+	reg = reg | LOWER_STATUS_CLR | UPPER_STATUS_CLR;
+	ret = regmap_write(priv->tm_map, STATUS_CNTL_ADDR_8064, reg);
+	if (ret)
+		return;
+
+	mask = ~(LOWER_STATUS_CLR | UPPER_STATUS_CLR);
+	ret = regmap_read(priv->tm_map, THRESHOLD_ADDR, &threshold);
+	if (ret)
+		return;
+	threshold_low = (threshold & THRESHOLD_LOWER_LIMIT_MASK) >>
+			THRESHOLD_LOWER_LIMIT_SHIFT;
+	threshold = (threshold & THRESHOLD_UPPER_LIMIT_MASK) >>
+		    THRESHOLD_UPPER_LIMIT_SHIFT;
+
+	ret = regmap_read(priv->tm_map, STATUS_CNTL_ADDR_8064, &reg);
+	if (ret)
+		return;
+
+	ret = regmap_read(priv->tm_map, CNTL_ADDR, &sensor);
+	if (ret)
+		return;
+	sensor &= (uint32_t)TSENS_8064_SENSORS_EN;
+	sensor >>= SENSOR0_SHIFT;
+
+	/* 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.
+	 */
+
+	/* Skip if the sensor is disabled */
+	if (sensor & 1) {
+		ret = regmap_read(priv->tm_map, priv->sensor[0].status, &code);
+		if (ret)
+			return;
+		upper_th_x = code >= threshold;
+		lower_th_x = code <= threshold_low;
+		if (upper_th_x)
+			mask |= UPPER_STATUS_CLR;
+		if (lower_th_x)
+			mask |= LOWER_STATUS_CLR;
+		if (upper_th_x || lower_th_x) {
+			/* Notify user space */
+			schedule_work(&priv->sensor[0].notify_work);
+			pr_debug("Trigger (%d degrees) for sensor %d\n",
+				 code_to_mdegC(code, &priv->sensor[0]), 0);
+		}
+	}
+	regmap_write(priv->tm_map, STATUS_CNTL_ADDR_8064, reg & mask);
+}
+
+static irqreturn_t tsens_isr(int irq, void *data)
+{
+	struct tsens_priv *priv = data;
+
+	schedule_work(&priv->tsens_work);
+	return IRQ_HANDLED;
+}
+
+static void hw_init(struct tsens_priv *priv)
+{
+	int ret;
+	unsigned int reg_cntl = 0, reg_cfg = 0, reg_thr = 0;
+	unsigned int reg_status_cntl = 0;
+
+	regmap_read(priv->tm_map, CNTL_ADDR, &reg_cntl);
+	regmap_write(priv->tm_map, CNTL_ADDR, reg_cntl | TSENS_SW_RST);
+
+	reg_cntl |= SLP_CLK_ENA | (MEASURE_PERIOD << 18) |
+		    (((1 << priv->num_sensors) - 1) << SENSOR0_SHIFT);
+	regmap_write(priv->tm_map, CNTL_ADDR, reg_cntl);
+	regmap_read(priv->tm_map, STATUS_CNTL_ADDR_8064, &reg_status_cntl);
+	reg_status_cntl |= LOWER_STATUS_CLR | UPPER_STATUS_CLR |
+			   MIN_STATUS_MASK | MAX_STATUS_MASK;
+	regmap_write(priv->tm_map, STATUS_CNTL_ADDR_8064, reg_status_cntl);
+	reg_cntl |= TSENS_EN;
+	regmap_write(priv->tm_map, CNTL_ADDR, reg_cntl);
+
+	regmap_read(priv->tm_map, CONFIG_ADDR, &reg_cfg);
+	if (priv->num_sensors > 1)
+		reg_cfg = (reg_cfg & ~CONFIG_MASK) | CONFIG;
+	else
+		reg_cfg = (reg_cfg & ~CONFIG_MASK) |
+			  (CONFIG << CONFIG_SHIFT_8660);
+	regmap_write(priv->tm_map, CONFIG_ADDR, reg_cfg);
+
+	reg_thr |= (LOWER_LIMIT_TH_8064 << THRESHOLD_LOWER_LIMIT_SHIFT) |
+		   (UPPER_LIMIT_TH_8064 << THRESHOLD_UPPER_LIMIT_SHIFT) |
+		   (MIN_LIMIT_TH << THRESHOLD_MIN_LIMIT_SHIFT) |
+		   (MAX_LIMIT_TH << THRESHOLD_MAX_LIMIT_SHIFT);
+
+	regmap_write(priv->tm_map, THRESHOLD_ADDR, reg_thr);
+
+	ret = devm_request_irq(priv->dev, priv->tsens_irq, tsens_isr,
+			       IRQF_TRIGGER_RISING, "tsens_interrupt", priv);
+	if (ret < 0) {
+		dev_err(priv->dev, "request_irq FAIL: %d", ret);
+		return;
+	}
+
+	INIT_WORK(&priv->tsens_work, tsens_scheduler_fn);
+}
+
 static int suspend_8960(struct tsens_priv *priv)
 {
 	int ret;
@@ -191,6 +367,8 @@ static int init_8960(struct tsens_priv *priv)
 		if (i >= 5)
 			priv->sensor[i].status = S0_STATUS_ADDR + 40;
 		priv->sensor[i].status += i * 4;
+		priv->sensor[i].slope = tsens_8960_slope[i];
+		INIT_WORK(&priv->sensor[i].notify_work, notify_uspace_tsens_fn);
 	}
 
 	reg_cntl = SW_RST;
@@ -241,18 +419,9 @@ static int calibrate_8960(struct tsens_priv *priv)
 
 	kfree(data);
 
-	return 0;
-}
-
-/* Temperature on y axis and ADC-code on x-axis */
-static inline int code_to_mdegC(u32 adc_code, const struct tsens_sensor *s)
-{
-	int slope, offset;
+	hw_init(priv);
 
-	slope = thermal_zone_get_slope(s->tzd);
-	offset = CAL_MDEGC - slope * s->offset;
-
-	return adc_code * slope + offset;
+	return 0;
 }
 
 static int get_temp_8960(const struct tsens_sensor *s, int *temp)
diff --git a/drivers/thermal/qcom/tsens.h b/drivers/thermal/qcom/tsens.h
index 59d01162c66a..e66048fabcc7 100644
--- a/drivers/thermal/qcom/tsens.h
+++ b/drivers/thermal/qcom/tsens.h
@@ -48,6 +48,7 @@ enum tsens_irq_type {
 struct tsens_sensor {
 	struct tsens_priv		*priv;
 	struct thermal_zone_device	*tzd;
+	struct work_struct		notify_work;
 	int				offset;
 	unsigned int			hw_id;
 	int				slope;
@@ -559,6 +560,7 @@ struct tsens_priv {
 	struct regmap			*tm_map;
 	struct regmap			*srot_map;
 	u32				tm_offset;
+	u32				tsens_irq;
 
 	/* lock for upper/lower threshold interrupts */
 	spinlock_t			ul_lock;
@@ -568,6 +570,7 @@ struct tsens_priv {
 	struct tsens_features		*feat;
 	const struct reg_field		*fields;
 	const struct tsens_ops		*ops;
+	struct work_struct		tsens_work;
 
 	struct dentry			*debug_root;
 	struct dentry			*debug;
-- 
2.27.0

Re: [PATCH v4 5/7] drivers: thermal: tsens: add interrupt support for 9860 driver

[Amit Kucheria]

Hi Ansuel,

Thanks for this patch.

On Thu, Jul 16, 2020 at 7:58 AM Ansuel Smith <ansuelsmth@gmail.com> wrote:
>
> Add interrupt support for 9860 tsens driver used to set thermal trip
> point for the system.

typo: 8960

You've used the names 8960 and ipq8064 interchangeably throughout the
series. AFAICT, msm8960, ipq8064 and apq8064 use the same IP version
of tsens. Please use 8960 in all patches, descriptions and dt-binding.
to reflect the filename for the driver.
Then add ipq8064 and apq8064 in a comment in the driver like here to
show that the driver also supports these other SoCs:
https://elixir.bootlin.com/linux/v5.8-rc4/source/drivers/thermal/qcom/tsens-v0_1.c#L328

You can also add a new compatible string for ipq8064 as a separate
patch at the end of the series.

> Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
> ---
>  drivers/thermal/qcom/tsens-8960.c | 197 +++++++++++++++++++++++++++---
>  drivers/thermal/qcom/tsens.h      |   3 +
>  2 files changed, 186 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/thermal/qcom/tsens-8960.c b/drivers/thermal/qcom/tsens-8960.c
> index 45788eb3c666..20d0bfb10f1f 100644
> --- a/drivers/thermal/qcom/tsens-8960.c
> +++ b/drivers/thermal/qcom/tsens-8960.c
> @@ -8,6 +8,7 @@
>  #include <linux/bitops.h>
>  #include <linux/regmap.h>
>  #include <linux/mfd/syscon.h>
> +#include <linux/interrupt.h>
>  #include <linux/thermal.h>
>  #include "tsens.h"
>
> @@ -27,7 +28,6 @@
>  /* CNTL_ADDR bitmasks */
>  #define EN                     BIT(0)
>  #define SW_RST                 BIT(1)
> -#define SENSOR0_EN             BIT(3)
>  #define SLP_CLK_ENA            BIT(26)
>  #define SLP_CLK_ENA_8660       BIT(24)
>  #define MEASURE_PERIOD         1
> @@ -41,14 +41,26 @@
>
>  #define THRESHOLD_ADDR         0x3624
>  /* THRESHOLD_ADDR bitmasks */
> +#define THRESHOLD_MAX_CODE             0x20000
> +#define THRESHOLD_MIN_CODE             0
>  #define THRESHOLD_MAX_LIMIT_SHIFT      24
>  #define THRESHOLD_MIN_LIMIT_SHIFT      16
>  #define THRESHOLD_UPPER_LIMIT_SHIFT    8
>  #define THRESHOLD_LOWER_LIMIT_SHIFT    0
> +#define THRESHOLD_MAX_LIMIT_MASK       (THRESHOLD_MAX_CODE << \
> +                                               THRESHOLD_MAX_LIMIT_SHIFT)
> +#define THRESHOLD_MIN_LIMIT_MASK       (THRESHOLD_MAX_CODE << \
> +                                               THRESHOLD_MIN_LIMIT_SHIFT)
> +#define THRESHOLD_UPPER_LIMIT_MASK     (THRESHOLD_MAX_CODE << \
> +                                               THRESHOLD_UPPER_LIMIT_SHIFT)
> +#define THRESHOLD_LOWER_LIMIT_MASK     (THRESHOLD_MAX_CODE << \
> +                                               THRESHOLD_LOWER_LIMIT_SHIFT)
>
>  /* Initial temperature threshold values */
> -#define LOWER_LIMIT_TH         0x50
> -#define UPPER_LIMIT_TH         0xdf
> +#define LOWER_LIMIT_TH_8960    0x50
> +#define UPPER_LIMIT_TH_8960    0xdf
> +#define LOWER_LIMIT_TH_8064    0x9d /* 95C */
> +#define UPPER_LIMIT_TH_8064    0xa6 /* 105C */
>  #define MIN_LIMIT_TH           0x0
>  #define MAX_LIMIT_TH           0xff
>
> @@ -57,6 +69,170 @@
>  #define TRDY_MASK              BIT(7)
>  #define TIMEOUT_US             100
>
> +#define TSENS_EN               BIT(0)
> +#define TSENS_SW_RST           BIT(1)
> +#define TSENS_ADC_CLK_SEL      BIT(2)
> +#define SENSOR0_EN             BIT(3)
> +#define SENSOR1_EN             BIT(4)
> +#define SENSOR2_EN             BIT(5)
> +#define SENSOR3_EN             BIT(6)
> +#define SENSOR4_EN             BIT(7)
> +#define SENSORS_EN             (SENSOR0_EN | SENSOR1_EN | \
> +                               SENSOR2_EN | SENSOR3_EN | SENSOR4_EN)
> +#define TSENS_8064_SENSOR5_EN                          BIT(8)
> +#define TSENS_8064_SENSOR6_EN                          BIT(9)
> +#define TSENS_8064_SENSOR7_EN                          BIT(10)
> +#define TSENS_8064_SENSOR8_EN                          BIT(11)
> +#define TSENS_8064_SENSOR9_EN                          BIT(12)
> +#define TSENS_8064_SENSOR10_EN                         BIT(13)
> +#define TSENS_8064_SENSORS_EN                          (SENSORS_EN | \
> +                                               TSENS_8064_SENSOR5_EN | \
> +                                               TSENS_8064_SENSOR6_EN | \
> +                                               TSENS_8064_SENSOR7_EN | \
> +                                               TSENS_8064_SENSOR8_EN | \
> +                                               TSENS_8064_SENSOR9_EN | \
> +                                               TSENS_8064_SENSOR10_EN)
> +
> +u32 tsens_8960_slope[] = {
> +                       1176, 1176, 1154, 1176,
> +                       1111, 1132, 1132, 1199,
> +                       1132, 1199, 1132
> +                       };
> +
> +/* Temperature on y axis and ADC-code on x-axis */
> +static inline int code_to_mdegC(u32 adc_code, const struct tsens_sensor *s)
> +{
> +       int slope, offset;
> +
> +       slope = thermal_zone_get_slope(s->tzd);
> +       offset = CAL_MDEGC - slope * s->offset;
> +
> +       return adc_code * slope + offset;
> +}
> +
> +static void notify_uspace_tsens_fn(struct work_struct *work)
> +{
> +       struct tsens_sensor *s = container_of(work, struct tsens_sensor,
> +                                                               notify_work);
> +
> +       sysfs_notify(&s->tzd->device.kobj, NULL, "type");
> +}
> +
> +static void tsens_scheduler_fn(struct work_struct *work)
> +{
> +       struct tsens_priv *priv =
> +               container_of(work, struct tsens_priv, tsens_work);
> +       unsigned int threshold, threshold_low, code, reg, sensor;
> +       unsigned long mask;
> +       bool upper_th_x, lower_th_x;
> +       int ret;
> +
> +       ret = regmap_read(priv->tm_map, STATUS_CNTL_ADDR_8064, &reg);
> +       if (ret)
> +               return;
> +       reg = reg | LOWER_STATUS_CLR | UPPER_STATUS_CLR;
> +       ret = regmap_write(priv->tm_map, STATUS_CNTL_ADDR_8064, reg);
> +       if (ret)
> +               return;
> +
> +       mask = ~(LOWER_STATUS_CLR | UPPER_STATUS_CLR);
> +       ret = regmap_read(priv->tm_map, THRESHOLD_ADDR, &threshold);
> +       if (ret)
> +               return;
> +       threshold_low = (threshold & THRESHOLD_LOWER_LIMIT_MASK) >>
> +                       THRESHOLD_LOWER_LIMIT_SHIFT;
> +       threshold = (threshold & THRESHOLD_UPPER_LIMIT_MASK) >>
> +                   THRESHOLD_UPPER_LIMIT_SHIFT;
> +
> +       ret = regmap_read(priv->tm_map, STATUS_CNTL_ADDR_8064, &reg);
> +       if (ret)
> +               return;
> +
> +       ret = regmap_read(priv->tm_map, CNTL_ADDR, &sensor);
> +       if (ret)
> +               return;
> +       sensor &= (uint32_t)TSENS_8064_SENSORS_EN;
> +       sensor >>= SENSOR0_SHIFT;
> +
> +       /* 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.
> +        */
> +
> +       /* Skip if the sensor is disabled */
> +       if (sensor & 1) {
> +               ret = regmap_read(priv->tm_map, priv->sensor[0].status, &code);
> +               if (ret)
> +                       return;
> +               upper_th_x = code >= threshold;
> +               lower_th_x = code <= threshold_low;
> +               if (upper_th_x)
> +                       mask |= UPPER_STATUS_CLR;
> +               if (lower_th_x)
> +                       mask |= LOWER_STATUS_CLR;
> +               if (upper_th_x || lower_th_x) {
> +                       /* Notify user space */
> +                       schedule_work(&priv->sensor[0].notify_work);
> +                       pr_debug("Trigger (%d degrees) for sensor %d\n",
> +                                code_to_mdegC(code, &priv->sensor[0]), 0);
> +               }
> +       }
> +       regmap_write(priv->tm_map, STATUS_CNTL_ADDR_8064, reg & mask);
> +}
> +
> +static irqreturn_t tsens_isr(int irq, void *data)
> +{
> +       struct tsens_priv *priv = data;
> +
> +       schedule_work(&priv->tsens_work);
> +       return IRQ_HANDLED;


Have you considered trying to reuse the regmap and interrupt handling
infrastructure in tsens.c that I used to convert over everything after
IP version 0.1?

I started converting over 8960 but never managed to finish testing
this[1]. I'd be happy for you to take this over and get it working so
the 8960 doesn't end up being a completely separate driver from the
other platforms.

[1] https://git.linaro.org/people/amit.kucheria/kernel.git/log/?h=wrk3/tsens-8960-breakage

> +}
> +
> +static void hw_init(struct tsens_priv *priv)
> +{
> +       int ret;
> +       unsigned int reg_cntl = 0, reg_cfg = 0, reg_thr = 0;
> +       unsigned int reg_status_cntl = 0;
> +
> +       regmap_read(priv->tm_map, CNTL_ADDR, &reg_cntl);
> +       regmap_write(priv->tm_map, CNTL_ADDR, reg_cntl | TSENS_SW_RST);
> +
> +       reg_cntl |= SLP_CLK_ENA | (MEASURE_PERIOD << 18) |
> +                   (((1 << priv->num_sensors) - 1) << SENSOR0_SHIFT);
> +       regmap_write(priv->tm_map, CNTL_ADDR, reg_cntl);
> +       regmap_read(priv->tm_map, STATUS_CNTL_ADDR_8064, &reg_status_cntl);
> +       reg_status_cntl |= LOWER_STATUS_CLR | UPPER_STATUS_CLR |
> +                          MIN_STATUS_MASK | MAX_STATUS_MASK;
> +       regmap_write(priv->tm_map, STATUS_CNTL_ADDR_8064, reg_status_cntl);
> +       reg_cntl |= TSENS_EN;
> +       regmap_write(priv->tm_map, CNTL_ADDR, reg_cntl);
> +
> +       regmap_read(priv->tm_map, CONFIG_ADDR, &reg_cfg);
> +       if (priv->num_sensors > 1)
> +               reg_cfg = (reg_cfg & ~CONFIG_MASK) | CONFIG;
> +       else
> +               reg_cfg = (reg_cfg & ~CONFIG_MASK) |
> +                         (CONFIG << CONFIG_SHIFT_8660);
> +       regmap_write(priv->tm_map, CONFIG_ADDR, reg_cfg);
> +
> +       reg_thr |= (LOWER_LIMIT_TH_8064 << THRESHOLD_LOWER_LIMIT_SHIFT) |
> +                  (UPPER_LIMIT_TH_8064 << THRESHOLD_UPPER_LIMIT_SHIFT) |
> +                  (MIN_LIMIT_TH << THRESHOLD_MIN_LIMIT_SHIFT) |
> +                  (MAX_LIMIT_TH << THRESHOLD_MAX_LIMIT_SHIFT);
> +
> +       regmap_write(priv->tm_map, THRESHOLD_ADDR, reg_thr);
> +
> +       ret = devm_request_irq(priv->dev, priv->tsens_irq, tsens_isr,
> +                              IRQF_TRIGGER_RISING, "tsens_interrupt", priv);
> +       if (ret < 0) {
> +               dev_err(priv->dev, "request_irq FAIL: %d", ret);
> +               return;
> +       }
> +
> +       INIT_WORK(&priv->tsens_work, tsens_scheduler_fn);
> +}
> +
>  static int suspend_8960(struct tsens_priv *priv)
>  {
>         int ret;
> @@ -191,6 +367,8 @@ static int init_8960(struct tsens_priv *priv)
>                 if (i >= 5)
>                         priv->sensor[i].status = S0_STATUS_ADDR + 40;
>                 priv->sensor[i].status += i * 4;
> +               priv->sensor[i].slope = tsens_8960_slope[i];
> +               INIT_WORK(&priv->sensor[i].notify_work, notify_uspace_tsens_fn);
>         }
>
>         reg_cntl = SW_RST;
> @@ -241,18 +419,9 @@ static int calibrate_8960(struct tsens_priv *priv)
>
>         kfree(data);
>
> -       return 0;
> -}
> -
> -/* Temperature on y axis and ADC-code on x-axis */
> -static inline int code_to_mdegC(u32 adc_code, const struct tsens_sensor *s)
> -{
> -       int slope, offset;
> +       hw_init(priv);
>
> -       slope = thermal_zone_get_slope(s->tzd);
> -       offset = CAL_MDEGC - slope * s->offset;
> -
> -       return adc_code * slope + offset;

This code move hunk belongs in a separate patch.

> +       return 0;
>  }
>
>  static int get_temp_8960(const struct tsens_sensor *s, int *temp)
> diff --git a/drivers/thermal/qcom/tsens.h b/drivers/thermal/qcom/tsens.h
> index 59d01162c66a..e66048fabcc7 100644
> --- a/drivers/thermal/qcom/tsens.h
> +++ b/drivers/thermal/qcom/tsens.h
> @@ -48,6 +48,7 @@ enum tsens_irq_type {
>  struct tsens_sensor {
>         struct tsens_priv               *priv;
>         struct thermal_zone_device      *tzd;
> +       struct work_struct              notify_work;
>         int                             offset;
>         unsigned int                    hw_id;
>         int                             slope;
> @@ -559,6 +560,7 @@ struct tsens_priv {
>         struct regmap                   *tm_map;
>         struct regmap                   *srot_map;
>         u32                             tm_offset;
> +       u32                             tsens_irq;
>
>         /* lock for upper/lower threshold interrupts */
>         spinlock_t                      ul_lock;
> @@ -568,6 +570,7 @@ struct tsens_priv {
>         struct tsens_features           *feat;
>         const struct reg_field          *fields;
>         const struct tsens_ops          *ops;
> +       struct work_struct              tsens_work;
>
>         struct dentry                   *debug_root;
>         struct dentry                   *debug;
> --
> 2.27.0
>

R: [PATCH v4 5/7] drivers: thermal: tsens: add interrupt support for 9860 driver

[ansuelsmth]

> -----Messaggio originale-----
> Da: Amit Kucheria <amit.kucheria@linaro.org>
> Inviato: lunedì 20 luglio 2020 11:41
> A: Ansuel Smith <ansuelsmth@gmail.com>
> Cc: Rob Herring <robh+dt@kernel.org>; Andy Gross <agross@kernel.org>;
> Bjorn Andersson <bjorn.andersson@linaro.org>; Zhang Rui
> <rui.zhang@intel.com>; Daniel Lezcano <daniel.lezcano@linaro.org>;
> Michael Turquette <mturquette@baylibre.com>; Stephen Boyd
> <sboyd@kernel.org>; Linux PM list <linux-pm@vger.kernel.org>; linux-arm-
> msm <linux-arm-msm@vger.kernel.org>; DTML
> <devicetree@vger.kernel.org>; Linux Kernel Mailing List <linux-
> kernel@vger.kernel.org>; linux-clk <linux-clk@vger.kernel.org>
> Oggetto: Re: [PATCH v4 5/7] drivers: thermal: tsens: add interrupt support
> for 9860 driver
> 
> Hi Ansuel,
> 
> Thanks for this patch.
> 
> On Thu, Jul 16, 2020 at 7:58 AM Ansuel Smith <ansuelsmth@gmail.com>
> wrote:
> >
> > Add interrupt support for 9860 tsens driver used to set thermal trip
> > point for the system.
> 
> typo: 8960
> 
> You've used the names 8960 and ipq8064 interchangeably throughout the
> series. AFAICT, msm8960, ipq8064 and apq8064 use the same IP version
> of tsens. Please use 8960 in all patches, descriptions and dt-binding.
> to reflect the filename for the driver.
> Then add ipq8064 and apq8064 in a comment in the driver like here to
> show that the driver also supports these other SoCs:
> https://elixir.bootlin.com/linux/v5.8-
> rc4/source/drivers/thermal/qcom/tsens-v0_1.c#L328
> 
> You can also add a new compatible string for ipq8064 as a separate
> patch at the end of the series.
> 
> > Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
> > ---
> >  drivers/thermal/qcom/tsens-8960.c | 197
> +++++++++++++++++++++++++++---
> >  drivers/thermal/qcom/tsens.h      |   3 +
> >  2 files changed, 186 insertions(+), 14 deletions(-)
> >
> > diff --git a/drivers/thermal/qcom/tsens-8960.c
> b/drivers/thermal/qcom/tsens-8960.c
> > index 45788eb3c666..20d0bfb10f1f 100644
> > --- a/drivers/thermal/qcom/tsens-8960.c
> > +++ b/drivers/thermal/qcom/tsens-8960.c
> > @@ -8,6 +8,7 @@
> >  #include <linux/bitops.h>
> >  #include <linux/regmap.h>
> >  #include <linux/mfd/syscon.h>
> > +#include <linux/interrupt.h>
> >  #include <linux/thermal.h>
> >  #include "tsens.h"
> >
> > @@ -27,7 +28,6 @@
> >  /* CNTL_ADDR bitmasks */
> >  #define EN                     BIT(0)
> >  #define SW_RST                 BIT(1)
> > -#define SENSOR0_EN             BIT(3)
> >  #define SLP_CLK_ENA            BIT(26)
> >  #define SLP_CLK_ENA_8660       BIT(24)
> >  #define MEASURE_PERIOD         1
> > @@ -41,14 +41,26 @@
> >
> >  #define THRESHOLD_ADDR         0x3624
> >  /* THRESHOLD_ADDR bitmasks */
> > +#define THRESHOLD_MAX_CODE             0x20000
> > +#define THRESHOLD_MIN_CODE             0
> >  #define THRESHOLD_MAX_LIMIT_SHIFT      24
> >  #define THRESHOLD_MIN_LIMIT_SHIFT      16
> >  #define THRESHOLD_UPPER_LIMIT_SHIFT    8
> >  #define THRESHOLD_LOWER_LIMIT_SHIFT    0
> > +#define THRESHOLD_MAX_LIMIT_MASK       (THRESHOLD_MAX_CODE
> << \
> > +                                               THRESHOLD_MAX_LIMIT_SHIFT)
> > +#define THRESHOLD_MIN_LIMIT_MASK       (THRESHOLD_MAX_CODE <<
> \
> > +                                               THRESHOLD_MIN_LIMIT_SHIFT)
> > +#define THRESHOLD_UPPER_LIMIT_MASK     (THRESHOLD_MAX_CODE
> << \
> > +                                               THRESHOLD_UPPER_LIMIT_SHIFT)
> > +#define THRESHOLD_LOWER_LIMIT_MASK     (THRESHOLD_MAX_CODE
> << \
> > +                                               THRESHOLD_LOWER_LIMIT_SHIFT)
> >
> >  /* Initial temperature threshold values */
> > -#define LOWER_LIMIT_TH         0x50
> > -#define UPPER_LIMIT_TH         0xdf
> > +#define LOWER_LIMIT_TH_8960    0x50
> > +#define UPPER_LIMIT_TH_8960    0xdf
> > +#define LOWER_LIMIT_TH_8064    0x9d /* 95C */
> > +#define UPPER_LIMIT_TH_8064    0xa6 /* 105C */
> >  #define MIN_LIMIT_TH           0x0
> >  #define MAX_LIMIT_TH           0xff
> >
> > @@ -57,6 +69,170 @@
> >  #define TRDY_MASK              BIT(7)
> >  #define TIMEOUT_US             100
> >
> > +#define TSENS_EN               BIT(0)
> > +#define TSENS_SW_RST           BIT(1)
> > +#define TSENS_ADC_CLK_SEL      BIT(2)
> > +#define SENSOR0_EN             BIT(3)
> > +#define SENSOR1_EN             BIT(4)
> > +#define SENSOR2_EN             BIT(5)
> > +#define SENSOR3_EN             BIT(6)
> > +#define SENSOR4_EN             BIT(7)
> > +#define SENSORS_EN             (SENSOR0_EN | SENSOR1_EN | \
> > +                               SENSOR2_EN | SENSOR3_EN | SENSOR4_EN)
> > +#define TSENS_8064_SENSOR5_EN                          BIT(8)
> > +#define TSENS_8064_SENSOR6_EN                          BIT(9)
> > +#define TSENS_8064_SENSOR7_EN                          BIT(10)
> > +#define TSENS_8064_SENSOR8_EN                          BIT(11)
> > +#define TSENS_8064_SENSOR9_EN                          BIT(12)
> > +#define TSENS_8064_SENSOR10_EN                         BIT(13)
> > +#define TSENS_8064_SENSORS_EN                          (SENSORS_EN | \
> > +                                               TSENS_8064_SENSOR5_EN | \
> > +                                               TSENS_8064_SENSOR6_EN | \
> > +                                               TSENS_8064_SENSOR7_EN | \
> > +                                               TSENS_8064_SENSOR8_EN | \
> > +                                               TSENS_8064_SENSOR9_EN | \
> > +                                               TSENS_8064_SENSOR10_EN)
> > +
> > +u32 tsens_8960_slope[] = {
> > +                       1176, 1176, 1154, 1176,
> > +                       1111, 1132, 1132, 1199,
> > +                       1132, 1199, 1132
> > +                       };
> > +
> > +/* Temperature on y axis and ADC-code on x-axis */
> > +static inline int code_to_mdegC(u32 adc_code, const struct
> tsens_sensor *s)
> > +{
> > +       int slope, offset;
> > +
> > +       slope = thermal_zone_get_slope(s->tzd);
> > +       offset = CAL_MDEGC - slope * s->offset;
> > +
> > +       return adc_code * slope + offset;
> > +}
> > +
> > +static void notify_uspace_tsens_fn(struct work_struct *work)
> > +{
> > +       struct tsens_sensor *s = container_of(work, struct tsens_sensor,
> > +                                                               notify_work);
> > +
> > +       sysfs_notify(&s->tzd->device.kobj, NULL, "type");
> > +}
> > +
> > +static void tsens_scheduler_fn(struct work_struct *work)
> > +{
> > +       struct tsens_priv *priv =
> > +               container_of(work, struct tsens_priv, tsens_work);
> > +       unsigned int threshold, threshold_low, code, reg, sensor;
> > +       unsigned long mask;
> > +       bool upper_th_x, lower_th_x;
> > +       int ret;
> > +
> > +       ret = regmap_read(priv->tm_map, STATUS_CNTL_ADDR_8064,
> &reg);
> > +       if (ret)
> > +               return;
> > +       reg = reg | LOWER_STATUS_CLR | UPPER_STATUS_CLR;
> > +       ret = regmap_write(priv->tm_map, STATUS_CNTL_ADDR_8064, reg);
> > +       if (ret)
> > +               return;
> > +
> > +       mask = ~(LOWER_STATUS_CLR | UPPER_STATUS_CLR);
> > +       ret = regmap_read(priv->tm_map, THRESHOLD_ADDR, &threshold);
> > +       if (ret)
> > +               return;
> > +       threshold_low = (threshold & THRESHOLD_LOWER_LIMIT_MASK) >>
> > +                       THRESHOLD_LOWER_LIMIT_SHIFT;
> > +       threshold = (threshold & THRESHOLD_UPPER_LIMIT_MASK) >>
> > +                   THRESHOLD_UPPER_LIMIT_SHIFT;
> > +
> > +       ret = regmap_read(priv->tm_map, STATUS_CNTL_ADDR_8064,
> &reg);
> > +       if (ret)
> > +               return;
> > +
> > +       ret = regmap_read(priv->tm_map, CNTL_ADDR, &sensor);
> > +       if (ret)
> > +               return;
> > +       sensor &= (uint32_t)TSENS_8064_SENSORS_EN;
> > +       sensor >>= SENSOR0_SHIFT;
> > +
> > +       /* 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.
> > +        */
> > +
> > +       /* Skip if the sensor is disabled */
> > +       if (sensor & 1) {
> > +               ret = regmap_read(priv->tm_map, priv->sensor[0].status,
> &code);
> > +               if (ret)
> > +                       return;
> > +               upper_th_x = code >= threshold;
> > +               lower_th_x = code <= threshold_low;
> > +               if (upper_th_x)
> > +                       mask |= UPPER_STATUS_CLR;
> > +               if (lower_th_x)
> > +                       mask |= LOWER_STATUS_CLR;
> > +               if (upper_th_x || lower_th_x) {
> > +                       /* Notify user space */
> > +                       schedule_work(&priv->sensor[0].notify_work);
> > +                       pr_debug("Trigger (%d degrees) for sensor %d\n",
> > +                                code_to_mdegC(code, &priv->sensor[0]), 0);
> > +               }
> > +       }
> > +       regmap_write(priv->tm_map, STATUS_CNTL_ADDR_8064, reg &
> mask);
> > +}
> > +
> > +static irqreturn_t tsens_isr(int irq, void *data)
> > +{
> > +       struct tsens_priv *priv = data;
> > +
> > +       schedule_work(&priv->tsens_work);
> > +       return IRQ_HANDLED;
> 
> 
> Have you considered trying to reuse the regmap and interrupt handling
> infrastructure in tsens.c that I used to convert over everything after
> IP version 0.1?
> 
> I started converting over 8960 but never managed to finish testing
> this[1]. I'd be happy for you to take this over and get it working so
> the 8960 doesn't end up being a completely separate driver from the
> other platforms.
> 
> [1]
> https://git.linaro.org/people/amit.kucheria/kernel.git/log/?h=wrk3/tsens-
> 8960-breakage
> 

Thanks a lot for the link. I started doing some test and I think the only general
code we will be able to use will be the init_common. The get temp and 
the function to convert code to decg are very different from the one used in 
8960.  Do you think keep a custom get temp function is good or not?


> > +}
> > +
> > +static void hw_init(struct tsens_priv *priv)
> > +{
> > +       int ret;
> > +       unsigned int reg_cntl = 0, reg_cfg = 0, reg_thr = 0;
> > +       unsigned int reg_status_cntl = 0;
> > +
> > +       regmap_read(priv->tm_map, CNTL_ADDR, &reg_cntl);
> > +       regmap_write(priv->tm_map, CNTL_ADDR, reg_cntl |
> TSENS_SW_RST);
> > +
> > +       reg_cntl |= SLP_CLK_ENA | (MEASURE_PERIOD << 18) |
> > +                   (((1 << priv->num_sensors) - 1) << SENSOR0_SHIFT);
> > +       regmap_write(priv->tm_map, CNTL_ADDR, reg_cntl);
> > +       regmap_read(priv->tm_map, STATUS_CNTL_ADDR_8064,
> &reg_status_cntl);
> > +       reg_status_cntl |= LOWER_STATUS_CLR | UPPER_STATUS_CLR |
> > +                          MIN_STATUS_MASK | MAX_STATUS_MASK;
> > +       regmap_write(priv->tm_map, STATUS_CNTL_ADDR_8064,
> reg_status_cntl);
> > +       reg_cntl |= TSENS_EN;
> > +       regmap_write(priv->tm_map, CNTL_ADDR, reg_cntl);
> > +
> > +       regmap_read(priv->tm_map, CONFIG_ADDR, &reg_cfg);
> > +       if (priv->num_sensors > 1)
> > +               reg_cfg = (reg_cfg & ~CONFIG_MASK) | CONFIG;
> > +       else
> > +               reg_cfg = (reg_cfg & ~CONFIG_MASK) |
> > +                         (CONFIG << CONFIG_SHIFT_8660);
> > +       regmap_write(priv->tm_map, CONFIG_ADDR, reg_cfg);
> > +
> > +       reg_thr |= (LOWER_LIMIT_TH_8064 <<
> THRESHOLD_LOWER_LIMIT_SHIFT) |
> > +                  (UPPER_LIMIT_TH_8064 << THRESHOLD_UPPER_LIMIT_SHIFT)
> |
> > +                  (MIN_LIMIT_TH << THRESHOLD_MIN_LIMIT_SHIFT) |
> > +                  (MAX_LIMIT_TH << THRESHOLD_MAX_LIMIT_SHIFT);
> > +
> > +       regmap_write(priv->tm_map, THRESHOLD_ADDR, reg_thr);
> > +
> > +       ret = devm_request_irq(priv->dev, priv->tsens_irq, tsens_isr,
> > +                              IRQF_TRIGGER_RISING, "tsens_interrupt", priv);
> > +       if (ret < 0) {
> > +               dev_err(priv->dev, "request_irq FAIL: %d", ret);
> > +               return;
> > +       }
> > +
> > +       INIT_WORK(&priv->tsens_work, tsens_scheduler_fn);
> > +}
> > +
> >  static int suspend_8960(struct tsens_priv *priv)
> >  {
> >         int ret;
> > @@ -191,6 +367,8 @@ static int init_8960(struct tsens_priv *priv)
> >                 if (i >= 5)
> >                         priv->sensor[i].status = S0_STATUS_ADDR + 40;
> >                 priv->sensor[i].status += i * 4;
> > +               priv->sensor[i].slope = tsens_8960_slope[i];
> > +               INIT_WORK(&priv->sensor[i].notify_work,
> notify_uspace_tsens_fn);
> >         }
> >
> >         reg_cntl = SW_RST;
> > @@ -241,18 +419,9 @@ static int calibrate_8960(struct tsens_priv
> *priv)
> >
> >         kfree(data);
> >
> > -       return 0;
> > -}
> > -
> > -/* Temperature on y axis and ADC-code on x-axis */
> > -static inline int code_to_mdegC(u32 adc_code, const struct tsens_sensor
> *s)
> > -{
> > -       int slope, offset;
> > +       hw_init(priv);
> >
> > -       slope = thermal_zone_get_slope(s->tzd);
> > -       offset = CAL_MDEGC - slope * s->offset;
> > -
> > -       return adc_code * slope + offset;
> 
> This code move hunk belongs in a separate patch.
> 
> > +       return 0;
> >  }
> >
> >  static int get_temp_8960(const struct tsens_sensor *s, int *temp)
> > diff --git a/drivers/thermal/qcom/tsens.h
> b/drivers/thermal/qcom/tsens.h
> > index 59d01162c66a..e66048fabcc7 100644
> > --- a/drivers/thermal/qcom/tsens.h
> > +++ b/drivers/thermal/qcom/tsens.h
> > @@ -48,6 +48,7 @@ enum tsens_irq_type {
> >  struct tsens_sensor {
> >         struct tsens_priv               *priv;
> >         struct thermal_zone_device      *tzd;
> > +       struct work_struct              notify_work;
> >         int                             offset;
> >         unsigned int                    hw_id;
> >         int                             slope;
> > @@ -559,6 +560,7 @@ struct tsens_priv {
> >         struct regmap                   *tm_map;
> >         struct regmap                   *srot_map;
> >         u32                             tm_offset;
> > +       u32                             tsens_irq;
> >
> >         /* lock for upper/lower threshold interrupts */
> >         spinlock_t                      ul_lock;
> > @@ -568,6 +570,7 @@ struct tsens_priv {
> >         struct tsens_features           *feat;
> >         const struct reg_field          *fields;
> >         const struct tsens_ops          *ops;
> > +       struct work_struct              tsens_work;
> >
> >         struct dentry                   *debug_root;
> >         struct dentry                   *debug;
> > --
> > 2.27.0
> >

Re: [PATCH v4 5/7] drivers: thermal: tsens: add interrupt support for 9860 driver

[Amit Kucheria]

On Wed, Jul 22, 2020 at 12:52 AM <ansuelsmth@gmail.com> wrote:
>
>
>
> > -----Messaggio originale-----
> > Da: Amit Kucheria <amit.kucheria@linaro.org>
> > Inviato: lunedì 20 luglio 2020 11:41
> > A: Ansuel Smith <ansuelsmth@gmail.com>
> > Cc: Rob Herring <robh+dt@kernel.org>; Andy Gross <agross@kernel.org>;
> > Bjorn Andersson <bjorn.andersson@linaro.org>; Zhang Rui
> > <rui.zhang@intel.com>; Daniel Lezcano <daniel.lezcano@linaro.org>;
> > Michael Turquette <mturquette@baylibre.com>; Stephen Boyd
> > <sboyd@kernel.org>; Linux PM list <linux-pm@vger.kernel.org>; linux-arm-
> > msm <linux-arm-msm@vger.kernel.org>; DTML
> > <devicetree@vger.kernel.org>; Linux Kernel Mailing List <linux-
> > kernel@vger.kernel.org>; linux-clk <linux-clk@vger.kernel.org>
> > Oggetto: Re: [PATCH v4 5/7] drivers: thermal: tsens: add interrupt support
> > for 9860 driver
> >
> > Hi Ansuel,
> >
> > Thanks for this patch.
> >
> > On Thu, Jul 16, 2020 at 7:58 AM Ansuel Smith <ansuelsmth@gmail.com>
> > wrote:
> > >
> > > Add interrupt support for 9860 tsens driver used to set thermal trip
> > > point for the system.
> >
> > typo: 8960
> >
> > You've used the names 8960 and ipq8064 interchangeably throughout the
> > series. AFAICT, msm8960, ipq8064 and apq8064 use the same IP version
> > of tsens. Please use 8960 in all patches, descriptions and dt-binding.
> > to reflect the filename for the driver.
> > Then add ipq8064 and apq8064 in a comment in the driver like here to
> > show that the driver also supports these other SoCs:
> > https://elixir.bootlin.com/linux/v5.8-
> > rc4/source/drivers/thermal/qcom/tsens-v0_1.c#L328
> >
> > You can also add a new compatible string for ipq8064 as a separate
> > patch at the end of the series.
> >
> > > Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
> > > ---
> > >  drivers/thermal/qcom/tsens-8960.c | 197
> > +++++++++++++++++++++++++++---
> > >  drivers/thermal/qcom/tsens.h      |   3 +
> > >  2 files changed, 186 insertions(+), 14 deletions(-)
> > >
> > > diff --git a/drivers/thermal/qcom/tsens-8960.c
> > b/drivers/thermal/qcom/tsens-8960.c
> > > index 45788eb3c666..20d0bfb10f1f 100644
> > > --- a/drivers/thermal/qcom/tsens-8960.c
> > > +++ b/drivers/thermal/qcom/tsens-8960.c
> > > @@ -8,6 +8,7 @@
> > >  #include <linux/bitops.h>
> > >  #include <linux/regmap.h>
> > >  #include <linux/mfd/syscon.h>
> > > +#include <linux/interrupt.h>
> > >  #include <linux/thermal.h>
> > >  #include "tsens.h"
> > >
> > > @@ -27,7 +28,6 @@
> > >  /* CNTL_ADDR bitmasks */
> > >  #define EN                     BIT(0)
> > >  #define SW_RST                 BIT(1)
> > > -#define SENSOR0_EN             BIT(3)
> > >  #define SLP_CLK_ENA            BIT(26)
> > >  #define SLP_CLK_ENA_8660       BIT(24)
> > >  #define MEASURE_PERIOD         1
> > > @@ -41,14 +41,26 @@
> > >
> > >  #define THRESHOLD_ADDR         0x3624
> > >  /* THRESHOLD_ADDR bitmasks */
> > > +#define THRESHOLD_MAX_CODE             0x20000
> > > +#define THRESHOLD_MIN_CODE             0
> > >  #define THRESHOLD_MAX_LIMIT_SHIFT      24
> > >  #define THRESHOLD_MIN_LIMIT_SHIFT      16
> > >  #define THRESHOLD_UPPER_LIMIT_SHIFT    8
> > >  #define THRESHOLD_LOWER_LIMIT_SHIFT    0
> > > +#define THRESHOLD_MAX_LIMIT_MASK       (THRESHOLD_MAX_CODE
> > << \
> > > +                                               THRESHOLD_MAX_LIMIT_SHIFT)
> > > +#define THRESHOLD_MIN_LIMIT_MASK       (THRESHOLD_MAX_CODE <<
> > \
> > > +                                               THRESHOLD_MIN_LIMIT_SHIFT)
> > > +#define THRESHOLD_UPPER_LIMIT_MASK     (THRESHOLD_MAX_CODE
> > << \
> > > +                                               THRESHOLD_UPPER_LIMIT_SHIFT)
> > > +#define THRESHOLD_LOWER_LIMIT_MASK     (THRESHOLD_MAX_CODE
> > << \
> > > +                                               THRESHOLD_LOWER_LIMIT_SHIFT)
> > >
> > >  /* Initial temperature threshold values */
> > > -#define LOWER_LIMIT_TH         0x50
> > > -#define UPPER_LIMIT_TH         0xdf
> > > +#define LOWER_LIMIT_TH_8960    0x50
> > > +#define UPPER_LIMIT_TH_8960    0xdf
> > > +#define LOWER_LIMIT_TH_8064    0x9d /* 95C */
> > > +#define UPPER_LIMIT_TH_8064    0xa6 /* 105C */
> > >  #define MIN_LIMIT_TH           0x0
> > >  #define MAX_LIMIT_TH           0xff
> > >
> > > @@ -57,6 +69,170 @@
> > >  #define TRDY_MASK              BIT(7)
> > >  #define TIMEOUT_US             100
> > >
> > > +#define TSENS_EN               BIT(0)
> > > +#define TSENS_SW_RST           BIT(1)
> > > +#define TSENS_ADC_CLK_SEL      BIT(2)
> > > +#define SENSOR0_EN             BIT(3)
> > > +#define SENSOR1_EN             BIT(4)
> > > +#define SENSOR2_EN             BIT(5)
> > > +#define SENSOR3_EN             BIT(6)
> > > +#define SENSOR4_EN             BIT(7)
> > > +#define SENSORS_EN             (SENSOR0_EN | SENSOR1_EN | \
> > > +                               SENSOR2_EN | SENSOR3_EN | SENSOR4_EN)
> > > +#define TSENS_8064_SENSOR5_EN                          BIT(8)
> > > +#define TSENS_8064_SENSOR6_EN                          BIT(9)
> > > +#define TSENS_8064_SENSOR7_EN                          BIT(10)
> > > +#define TSENS_8064_SENSOR8_EN                          BIT(11)
> > > +#define TSENS_8064_SENSOR9_EN                          BIT(12)
> > > +#define TSENS_8064_SENSOR10_EN                         BIT(13)
> > > +#define TSENS_8064_SENSORS_EN                          (SENSORS_EN | \
> > > +                                               TSENS_8064_SENSOR5_EN | \
> > > +                                               TSENS_8064_SENSOR6_EN | \
> > > +                                               TSENS_8064_SENSOR7_EN | \
> > > +                                               TSENS_8064_SENSOR8_EN | \
> > > +                                               TSENS_8064_SENSOR9_EN | \
> > > +                                               TSENS_8064_SENSOR10_EN)
> > > +
> > > +u32 tsens_8960_slope[] = {
> > > +                       1176, 1176, 1154, 1176,
> > > +                       1111, 1132, 1132, 1199,
> > > +                       1132, 1199, 1132
> > > +                       };
> > > +
> > > +/* Temperature on y axis and ADC-code on x-axis */
> > > +static inline int code_to_mdegC(u32 adc_code, const struct
> > tsens_sensor *s)
> > > +{
> > > +       int slope, offset;
> > > +
> > > +       slope = thermal_zone_get_slope(s->tzd);
> > > +       offset = CAL_MDEGC - slope * s->offset;
> > > +
> > > +       return adc_code * slope + offset;
> > > +}
> > > +
> > > +static void notify_uspace_tsens_fn(struct work_struct *work)
> > > +{
> > > +       struct tsens_sensor *s = container_of(work, struct tsens_sensor,
> > > +                                                               notify_work);
> > > +
> > > +       sysfs_notify(&s->tzd->device.kobj, NULL, "type");
> > > +}
> > > +
> > > +static void tsens_scheduler_fn(struct work_struct *work)
> > > +{
> > > +       struct tsens_priv *priv =
> > > +               container_of(work, struct tsens_priv, tsens_work);
> > > +       unsigned int threshold, threshold_low, code, reg, sensor;
> > > +       unsigned long mask;
> > > +       bool upper_th_x, lower_th_x;
> > > +       int ret;
> > > +
> > > +       ret = regmap_read(priv->tm_map, STATUS_CNTL_ADDR_8064,
> > &reg);
> > > +       if (ret)
> > > +               return;
> > > +       reg = reg | LOWER_STATUS_CLR | UPPER_STATUS_CLR;
> > > +       ret = regmap_write(priv->tm_map, STATUS_CNTL_ADDR_8064, reg);
> > > +       if (ret)
> > > +               return;
> > > +
> > > +       mask = ~(LOWER_STATUS_CLR | UPPER_STATUS_CLR);
> > > +       ret = regmap_read(priv->tm_map, THRESHOLD_ADDR, &threshold);
> > > +       if (ret)
> > > +               return;
> > > +       threshold_low = (threshold & THRESHOLD_LOWER_LIMIT_MASK) >>
> > > +                       THRESHOLD_LOWER_LIMIT_SHIFT;
> > > +       threshold = (threshold & THRESHOLD_UPPER_LIMIT_MASK) >>
> > > +                   THRESHOLD_UPPER_LIMIT_SHIFT;
> > > +
> > > +       ret = regmap_read(priv->tm_map, STATUS_CNTL_ADDR_8064,
> > &reg);
> > > +       if (ret)
> > > +               return;
> > > +
> > > +       ret = regmap_read(priv->tm_map, CNTL_ADDR, &sensor);
> > > +       if (ret)
> > > +               return;
> > > +       sensor &= (uint32_t)TSENS_8064_SENSORS_EN;
> > > +       sensor >>= SENSOR0_SHIFT;
> > > +
> > > +       /* 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.
> > > +        */
> > > +
> > > +       /* Skip if the sensor is disabled */
> > > +       if (sensor & 1) {
> > > +               ret = regmap_read(priv->tm_map, priv->sensor[0].status,
> > &code);
> > > +               if (ret)
> > > +                       return;
> > > +               upper_th_x = code >= threshold;
> > > +               lower_th_x = code <= threshold_low;
> > > +               if (upper_th_x)
> > > +                       mask |= UPPER_STATUS_CLR;
> > > +               if (lower_th_x)
> > > +                       mask |= LOWER_STATUS_CLR;
> > > +               if (upper_th_x || lower_th_x) {
> > > +                       /* Notify user space */
> > > +                       schedule_work(&priv->sensor[0].notify_work);
> > > +                       pr_debug("Trigger (%d degrees) for sensor %d\n",
> > > +                                code_to_mdegC(code, &priv->sensor[0]), 0);
> > > +               }
> > > +       }
> > > +       regmap_write(priv->tm_map, STATUS_CNTL_ADDR_8064, reg &
> > mask);
> > > +}
> > > +
> > > +static irqreturn_t tsens_isr(int irq, void *data)
> > > +{
> > > +       struct tsens_priv *priv = data;
> > > +
> > > +       schedule_work(&priv->tsens_work);
> > > +       return IRQ_HANDLED;
> >
> >
> > Have you considered trying to reuse the regmap and interrupt handling
> > infrastructure in tsens.c that I used to convert over everything after
> > IP version 0.1?
> >
> > I started converting over 8960 but never managed to finish testing
> > this[1]. I'd be happy for you to take this over and get it working so
> > the 8960 doesn't end up being a completely separate driver from the
> > other platforms.
> >
> > [1]
> > https://git.linaro.org/people/amit.kucheria/kernel.git/log/?h=wrk3/tsens-
> > 8960-breakage
> >
>
> Thanks a lot for the link. I started doing some test and I think the only general
> code we will be able to use will be the init_common. The get temp and
> the function to convert code to decg are very different from the one used in
> 8960.  Do you think keep a custom get temp function is good or not?

OK. Let's start common init code and custom get_temp and adc-to-degc functions.

Regards,
Amit

[PATCH v4 6/7] drivers: thermal: tsens: add support for custom set_trip function

[Ansuel Smith]

8960 tsens driver have a custom implementation to set set_trip function.
Permit the generic driver to use the custom function if provided.

Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
---
 drivers/thermal/qcom/tsens.c | 4 ++++
 drivers/thermal/qcom/tsens.h | 2 ++
 2 files changed, 6 insertions(+)

diff --git a/drivers/thermal/qcom/tsens.c b/drivers/thermal/qcom/tsens.c
index 23f63dfbf13d..5f49f4117610 100644
--- a/drivers/thermal/qcom/tsens.c
+++ b/drivers/thermal/qcom/tsens.c
@@ -530,6 +530,10 @@ 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;
 
+	// Use the driver set_trips if present
+	if (priv->ops->set_trip_temp)
+		return priv->ops->set_trip_temp(_sensor, low, high);
+
 	dev_dbg(dev, "[%u] %s: proposed thresholds: (%d:%d)\n",
 		hw_id, __func__, low, high);
 
diff --git a/drivers/thermal/qcom/tsens.h b/drivers/thermal/qcom/tsens.h
index e66048fabcc7..4f0ab4aa5fd1 100644
--- a/drivers/thermal/qcom/tsens.h
+++ b/drivers/thermal/qcom/tsens.h
@@ -65,6 +65,7 @@ struct tsens_sensor {
  * @suspend: Function to suspend the tsens device
  * @resume: Function to resume the tsens device
  * @get_trend: Function to get the thermal/temp trend
+ * @set_trip_temp: Function to get trip temp
  */
 struct tsens_ops {
 	/* mandatory callbacks */
@@ -77,6 +78,7 @@ struct tsens_ops {
 	int (*suspend)(struct tsens_priv *priv);
 	int (*resume)(struct tsens_priv *priv);
 	int (*get_trend)(struct tsens_sensor *s, enum thermal_trend *trend);
+	int (*set_trip_temp)(void *data, int trip, int temp);
 };
 
 #define REG_FIELD_FOR_EACH_SENSOR11(_name, _offset, _startbit, _stopbit) \
-- 
2.27.0

[PATCH v4 7/7] drivers: thermal: tsens: add set_trip support for 8960

[Ansuel Smith]

Add custom set_trip function for 8960 needed to set trip point to the
tsens driver for 8960 driver.

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

diff --git a/drivers/thermal/qcom/tsens-8960.c b/drivers/thermal/qcom/tsens-8960.c
index 20d0bfb10f1f..4ad65ab3fd18 100644
--- a/drivers/thermal/qcom/tsens-8960.c
+++ b/drivers/thermal/qcom/tsens-8960.c
@@ -93,6 +93,15 @@
 						TSENS_8064_SENSOR9_EN | \
 						TSENS_8064_SENSOR10_EN)
 
+/* Trips: from very hot to very cold */
+enum tsens_trip_type {
+	TSENS_TRIP_STAGE3 = 0,
+	TSENS_TRIP_STAGE2,
+	TSENS_TRIP_STAGE1,
+	TSENS_TRIP_STAGE0,
+	TSENS_TRIP_NUM,
+};
+
 u32 tsens_8960_slope[] = {
 			1176, 1176, 1154, 1176,
 			1111, 1132, 1132, 1199,
@@ -110,6 +119,16 @@ static inline int code_to_mdegC(u32 adc_code, const struct tsens_sensor *s)
 	return adc_code * slope + offset;
 }
 
+static int mdegC_to_code(int degC, const struct tsens_sensor *s)
+{
+	int slope, offset;
+
+	slope = thermal_zone_get_slope(s->tzd);
+	offset = CAL_MDEGC - slope * s->offset;
+
+	return degC / slope - offset;
+}
+
 static void notify_uspace_tsens_fn(struct work_struct *work)
 {
 	struct tsens_sensor *s = container_of(work, struct tsens_sensor,
@@ -448,6 +467,64 @@ static int get_temp_8960(const struct tsens_sensor *s, int *temp)
 	return -ETIMEDOUT;
 }
 
+static int set_trip_temp_ipq8064(void *data, int trip, int temp)
+{
+	unsigned int reg_th, reg_cntl;
+	int ret, code, code_chk, hi_code, lo_code;
+	const struct tsens_sensor *s = data;
+	struct tsens_priv *priv = s->priv;
+
+	code = mdegC_to_code(temp, s);
+	code_chk = code;
+
+	if (code < THRESHOLD_MIN_CODE || code > THRESHOLD_MAX_CODE)
+		return -EINVAL;
+
+	ret = regmap_read(priv->tm_map, STATUS_CNTL_ADDR_8064, &reg_cntl);
+	if (ret)
+		return ret;
+
+	ret = regmap_read(priv->tm_map, THRESHOLD_ADDR, &reg_th);
+	if (ret)
+		return ret;
+
+	hi_code = (reg_th & THRESHOLD_UPPER_LIMIT_MASK)
+			>> THRESHOLD_UPPER_LIMIT_SHIFT;
+	lo_code = (reg_th & THRESHOLD_LOWER_LIMIT_MASK)
+			>> THRESHOLD_LOWER_LIMIT_SHIFT;
+
+	switch (trip) {
+	case TSENS_TRIP_STAGE3:
+		code <<= THRESHOLD_MAX_LIMIT_SHIFT;
+		reg_th &= ~THRESHOLD_MAX_LIMIT_MASK;
+		break;
+	case TSENS_TRIP_STAGE2:
+		if (code_chk <= lo_code)
+			return -EINVAL;
+		code <<= THRESHOLD_UPPER_LIMIT_SHIFT;
+		reg_th &= ~THRESHOLD_UPPER_LIMIT_MASK;
+		break;
+	case TSENS_TRIP_STAGE1:
+		if (code_chk >= hi_code)
+			return -EINVAL;
+		code <<= THRESHOLD_LOWER_LIMIT_SHIFT;
+		reg_th &= ~THRESHOLD_LOWER_LIMIT_MASK;
+		break;
+	case TSENS_TRIP_STAGE0:
+		code <<= THRESHOLD_MIN_LIMIT_SHIFT;
+		reg_th &= ~THRESHOLD_MIN_LIMIT_MASK;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	ret = regmap_write(priv->tm_map, THRESHOLD_ADDR, reg_th | code);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
 static const struct tsens_ops ops_8960 = {
 	.init		= init_8960,
 	.calibrate	= calibrate_8960,
@@ -456,6 +533,7 @@ static const struct tsens_ops ops_8960 = {
 	.disable	= disable_8960,
 	.suspend	= suspend_8960,
 	.resume		= resume_8960,
+	.set_trip_temp	= set_trip_temp_ipq8064,
 };
 
 struct tsens_plat_data data_8960 = {
-- 
2.27.0

Re: [PATCH v4 7/7] drivers: thermal: tsens: add set_trip support for 8960

[Amit Kucheria]

Hi Ansuel,

On Thu, Jul 16, 2020 at 7:58 AM Ansuel Smith <ansuelsmth@gmail.com> wrote:
>
> Add custom set_trip function for 8960 needed to set trip point to the
> tsens driver for 8960 driver.

Please describe why it needs a custom function please.

>
> Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
> ---
>  drivers/thermal/qcom/tsens-8960.c | 78 +++++++++++++++++++++++++++++++
>  1 file changed, 78 insertions(+)
>
> diff --git a/drivers/thermal/qcom/tsens-8960.c b/drivers/thermal/qcom/tsens-8960.c
> index 20d0bfb10f1f..4ad65ab3fd18 100644
> --- a/drivers/thermal/qcom/tsens-8960.c
> +++ b/drivers/thermal/qcom/tsens-8960.c
> @@ -93,6 +93,15 @@
>                                                 TSENS_8064_SENSOR9_EN | \
>                                                 TSENS_8064_SENSOR10_EN)
>
> +/* Trips: from very hot to very cold */
> +enum tsens_trip_type {
> +       TSENS_TRIP_STAGE3 = 0,
> +       TSENS_TRIP_STAGE2,
> +       TSENS_TRIP_STAGE1,
> +       TSENS_TRIP_STAGE0,
> +       TSENS_TRIP_NUM,
> +};
> +
>  u32 tsens_8960_slope[] = {
>                         1176, 1176, 1154, 1176,
>                         1111, 1132, 1132, 1199,
> @@ -110,6 +119,16 @@ static inline int code_to_mdegC(u32 adc_code, const struct tsens_sensor *s)
>         return adc_code * slope + offset;
>  }
>
> +static int mdegC_to_code(int degC, const struct tsens_sensor *s)
> +{
> +       int slope, offset;
> +
> +       slope = thermal_zone_get_slope(s->tzd);
> +       offset = CAL_MDEGC - slope * s->offset;
> +
> +       return degC / slope - offset;
> +}
> +
>  static void notify_uspace_tsens_fn(struct work_struct *work)
>  {
>         struct tsens_sensor *s = container_of(work, struct tsens_sensor,
> @@ -448,6 +467,64 @@ static int get_temp_8960(const struct tsens_sensor *s, int *temp)
>         return -ETIMEDOUT;
>  }
>
> +static int set_trip_temp_ipq8064(void *data, int trip, int temp)
> +{
> +       unsigned int reg_th, reg_cntl;
> +       int ret, code, code_chk, hi_code, lo_code;
> +       const struct tsens_sensor *s = data;
> +       struct tsens_priv *priv = s->priv;
> +
> +       code = mdegC_to_code(temp, s);
> +       code_chk = code;
> +
> +       if (code < THRESHOLD_MIN_CODE || code > THRESHOLD_MAX_CODE)
> +               return -EINVAL;
> +
> +       ret = regmap_read(priv->tm_map, STATUS_CNTL_ADDR_8064, &reg_cntl);
> +       if (ret)
> +               return ret;
> +
> +       ret = regmap_read(priv->tm_map, THRESHOLD_ADDR, &reg_th);
> +       if (ret)
> +               return ret;
> +
> +       hi_code = (reg_th & THRESHOLD_UPPER_LIMIT_MASK)
> +                       >> THRESHOLD_UPPER_LIMIT_SHIFT;
> +       lo_code = (reg_th & THRESHOLD_LOWER_LIMIT_MASK)
> +                       >> THRESHOLD_LOWER_LIMIT_SHIFT;
> +
> +       switch (trip) {
> +       case TSENS_TRIP_STAGE3:
> +               code <<= THRESHOLD_MAX_LIMIT_SHIFT;
> +               reg_th &= ~THRESHOLD_MAX_LIMIT_MASK;
> +               break;
> +       case TSENS_TRIP_STAGE2:
> +               if (code_chk <= lo_code)
> +                       return -EINVAL;
> +               code <<= THRESHOLD_UPPER_LIMIT_SHIFT;
> +               reg_th &= ~THRESHOLD_UPPER_LIMIT_MASK;
> +               break;
> +       case TSENS_TRIP_STAGE1:
> +               if (code_chk >= hi_code)
> +                       return -EINVAL;
> +               code <<= THRESHOLD_LOWER_LIMIT_SHIFT;
> +               reg_th &= ~THRESHOLD_LOWER_LIMIT_MASK;
> +               break;
> +       case TSENS_TRIP_STAGE0:
> +               code <<= THRESHOLD_MIN_LIMIT_SHIFT;
> +               reg_th &= ~THRESHOLD_MIN_LIMIT_MASK;
> +               break;
> +       default:
> +               return -EINVAL;
> +       }
> +
> +       ret = regmap_write(priv->tm_map, THRESHOLD_ADDR, reg_th | code);
> +       if (ret)
> +               return ret;
> +
> +       return 0;
> +}
> +
>  static const struct tsens_ops ops_8960 = {
>         .init           = init_8960,
>         .calibrate      = calibrate_8960,
> @@ -456,6 +533,7 @@ static const struct tsens_ops ops_8960 = {
>         .disable        = disable_8960,
>         .suspend        = suspend_8960,
>         .resume         = resume_8960,
> +       .set_trip_temp  = set_trip_temp_ipq8064,
>  };
>
>  struct tsens_plat_data data_8960 = {
> --
> 2.27.0
>