[RFC PATCH v6 0/8] Add support for ipq8064 tsens

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

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

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

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

-- 
2.27.0

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

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

diff --git a/drivers/thermal/qcom/tsens.c b/drivers/thermal/qcom/tsens.c
index 9fe9a2b26705..965c4799918a 100644
--- a/drivers/thermal/qcom/tsens.c
+++ b/drivers/thermal/qcom/tsens.c

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

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

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

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

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

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

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

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

diff --git a/drivers/thermal/qcom/tsens.h b/drivers/thermal/qcom/tsens.h
index 59d01162c66a..f1120791737c 100644
--- a/drivers/thermal/qcom/tsens.h
+++ b/drivers/thermal/qcom/tsens.h

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

@@ -441,6 +442,10 @@ enum regfield_ids {
 	CRIT_THRESH_14,
 	CRIT_THRESH_15,
 
+	/* VER_0 MIN MAX THRESH */
+	MIN_THRESH_0,
+	MAX_THRESH_0,
+
 	/* WATCHDOG */
 	WDOG_BARK_STATUS,
 	WDOG_BARK_CLEAR,

-- 
2.27.0

msm8960 based tsens have an hardcoded slope. Fix the calibrate function
with the new added slope, change code_to_mdegC to use slope and conver
get_temp to use reg_field.

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

diff --git a/drivers/thermal/qcom/tsens-8960.c b/drivers/thermal/qcom/tsens-8960.c
index 93d2c6c7d1bd..ca83c7f838a5 100644
--- a/drivers/thermal/qcom/tsens-8960.c
+++ b/drivers/thermal/qcom/tsens-8960.c

@@ -67,6 +67,14 @@
 #define S9_STATUS_OFF		0x3674
 #define S10_STATUS_OFF		0x3678
 
+#define TSENS_FACTOR		1
+
+u32 tsens_msm8960_slope[] = {
+			1176, 1176, 1154, 1176,
+			1111, 1132, 1132, 1199,
+			1132, 1199, 1132
+			};
+
 static int suspend_8960(struct tsens_priv *priv)
 {
 	int ret;

@@ -187,8 +195,10 @@ static int calibrate_8960(struct tsens_priv *priv)
 	if (IS_ERR(data))
 		return PTR_ERR(data);
 
-	for (i = 0; i < num_read; i++, s++)
-		s->offset = data[i];
+	for (i = 0; i < num_read; i++, s++) {
+		s->slope = tsens_msm8960_slope[i];
+		s->offset = CAL_MDEGC - (data[i] * s->slope);
+	}
 
 	kfree(data);
 

@@ -198,32 +208,43 @@ static int calibrate_8960(struct tsens_priv *priv)
 /* 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;
+	int num, degc;
+
+	num = (adc_code * s->slope) + s->offset;
 
-	slope = thermal_zone_get_slope(s->tzd);
-	offset = CAL_MDEGC - slope * s->offset;
+	if (num == 0)
+		degc = num;
+	else if (num > 0)
+		degc = (num + TSENS_FACTOR / 2)
+			/ TSENS_FACTOR;
+	else
+		degc = (num - TSENS_FACTOR / 2)
+			/ TSENS_FACTOR;
 
-	return adc_code * slope + offset;
+	return degc;
 }
 
 static int get_temp_8960(const struct tsens_sensor *s, int *temp)
 {
 	int ret;
-	u32 code, trdy;
+	u32 last_temp = 0, trdy;
 	struct tsens_priv *priv = s->priv;
 	unsigned long timeout;
 
 	timeout = jiffies + usecs_to_jiffies(TIMEOUT_US);
 	do {
-		ret = regmap_read(priv->tm_map, INT_STATUS_ADDR, &trdy);
+		ret = regmap_field_read(priv->rf[TRDY], &trdy);
 		if (ret)
 			return ret;
-		if (!(trdy & TRDY_MASK))
+		if (!trdy)
 			continue;
-		ret = regmap_read(priv->tm_map, s->status, &code);
+
+		ret = regmap_field_read(priv->rf[LAST_TEMP_0 + s->hw_id], &last_temp);
 		if (ret)
 			return ret;
-		*temp = code_to_mdegC(code, s);
+
+		*temp = code_to_mdegC(last_temp, s);
+
 		return 0;
 	} while (time_before(jiffies, timeout));
 

-- 
2.27.0

Use init_common and drop custom init for msm8960.

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

diff --git a/drivers/thermal/qcom/tsens-8960.c b/drivers/thermal/qcom/tsens-8960.c
index cb3611299e8f..93d2c6c7d1bd 100644
--- a/drivers/thermal/qcom/tsens-8960.c
+++ b/drivers/thermal/qcom/tsens-8960.c

@@ -51,7 +51,6 @@
 #define MIN_LIMIT_TH		0x0
 #define MAX_LIMIT_TH		0xff
 
-#define S0_STATUS_ADDR		0x3628
 #define INT_STATUS_ADDR		0x363c
 #define TRDY_MASK		BIT(7)
 #define TIMEOUT_US		100

@@ -174,56 +173,6 @@ static void disable_8960(struct tsens_priv *priv)
 	regmap_write(priv->tm_map, CNTL_ADDR, reg_cntl);
 }
 
-static int init_8960(struct tsens_priv *priv)
-{
-	int ret, i;
-	u32 reg_cntl;
-
-	priv->tm_map = dev_get_regmap(priv->dev, NULL);
-	if (!priv->tm_map)
-		return -ENODEV;
-
-	/*
-	 * The status registers for each sensor are discontiguous
-	 * because some SoCs have 5 sensors while others have more
-	 * but the control registers stay in the same place, i.e
-	 * directly after the first 5 status registers.
-	 */
-	for (i = 0; i < priv->num_sensors; i++) {
-		if (i >= 5)
-			priv->sensor[i].status = S0_STATUS_ADDR + 40;
-		priv->sensor[i].status += i * 4;
-	}
-
-	reg_cntl = SW_RST;
-	ret = regmap_update_bits(priv->tm_map, CNTL_ADDR, SW_RST, reg_cntl);
-	if (ret)
-		return ret;
-
-	if (priv->num_sensors > 1) {
-		reg_cntl |= SLP_CLK_ENA | (MEASURE_PERIOD << 18);
-		reg_cntl &= ~SW_RST;
-		ret = regmap_update_bits(priv->tm_map, CONFIG_ADDR,
-					 CONFIG_MASK, CONFIG);
-	} else {
-		reg_cntl |= SLP_CLK_ENA_8660 | (MEASURE_PERIOD << 16);
-		reg_cntl &= ~CONFIG_MASK_8660;
-		reg_cntl |= CONFIG_8660 << CONFIG_SHIFT_8660;
-	}
-
-	reg_cntl |= GENMASK(priv->num_sensors - 1, 0) << SENSOR0_SHIFT;
-	ret = regmap_write(priv->tm_map, CNTL_ADDR, reg_cntl);
-	if (ret)
-		return ret;
-
-	reg_cntl |= EN;
-	ret = regmap_write(priv->tm_map, CNTL_ADDR, reg_cntl);
-	if (ret)
-		return ret;
-
-	return 0;
-}
-
 static int calibrate_8960(struct tsens_priv *priv)
 {
 	int i;

@@ -342,7 +291,7 @@ static const struct reg_field tsens_8960_regfields[MAX_REGFIELDS] = {
 };
 
 static const struct tsens_ops ops_8960 = {
-	.init		= init_8960,
+	.init		= init_common,
 	.calibrate	= calibrate_8960,
 	.get_temp	= get_temp_8960,
 	.enable		= enable_8960,

-- 
2.27.0

Follow standard naming for calib secondary rom and change calib_backup
to tsens_calsel.

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

diff --git a/drivers/thermal/qcom/tsens-8960.c b/drivers/thermal/qcom/tsens-8960.c
index ca83c7f838a5..a8c85bd6c71f 100644
--- a/drivers/thermal/qcom/tsens-8960.c
+++ b/drivers/thermal/qcom/tsens-8960.c

@@ -191,7 +191,7 @@ static int calibrate_8960(struct tsens_priv *priv)
 
 	data = qfprom_read(priv->dev, "calib");
 	if (IS_ERR(data))
-		data = qfprom_read(priv->dev, "calib_backup");
+		data = qfprom_read(priv->dev, "calib_sel");
 	if (IS_ERR(data))
 		return PTR_ERR(data);
 

-- 
2.27.0

Add support for tsens present in ipq806x SoCs based on generic msm8960
tsens 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 965c4799918a..d571a6ddd914 100644
--- a/drivers/thermal/qcom/tsens.c
+++ b/drivers/thermal/qcom/tsens.c

@@ -993,6 +993,9 @@ static SIMPLE_DEV_PM_OPS(tsens_pm_ops, tsens_suspend, tsens_resume);
 
 static const struct of_device_id tsens_table[] = {
 	{
+		.compatible = "qcom,ipq8064-tsens",
+		.data = &data_8960,
+	}, {
 		.compatible = "qcom,msm8916-tsens",
 		.data = &data_8916,
 	}, {

-- 
2.27.0

Document the use of bindings used for msm8960 tsens based devices.
msm8960 use the same gcc regs and is set as a child of the qcom gcc.

Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
 .../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

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

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

diff --git a/drivers/thermal/qcom/tsens.c b/drivers/thermal/qcom/tsens.c
index 9af6f71ab640..9fe9a2b26705 100644
--- a/drivers/thermal/qcom/tsens.c
+++ b/drivers/thermal/qcom/tsens.c

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

@@ -600,9 +599,9 @@ int get_temp_tsens_valid(const struct tsens_sensor *s, int *temp)
 	}
 
 	/* Valid bit is set, OK to read the temperature */
-	*temp = tsens_hw_to_mC(s, temp_idx);
+	ret = priv->ops->get_temp(s, temp);
 
-	return 0;
+	return ret;
 }
 
 int get_temp_common(const struct tsens_sensor *s, int *temp)

-- 
2.27.0

Convert msm9860 driver to reg_field to use the init_common
function.

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

diff --git a/drivers/thermal/qcom/tsens-8960.c b/drivers/thermal/qcom/tsens-8960.c
index 2a28a5af209e..cb3611299e8f 100644
--- a/drivers/thermal/qcom/tsens-8960.c
+++ b/drivers/thermal/qcom/tsens-8960.c

@@ -56,6 +56,18 @@
 #define TRDY_MASK		BIT(7)
 #define TIMEOUT_US		100
 
+#define S0_STATUS_OFF		0x3628
+#define S1_STATUS_OFF		0x362c
+#define S2_STATUS_OFF		0x3630
+#define S3_STATUS_OFF		0x3634
+#define S4_STATUS_OFF		0x3638
+#define S5_STATUS_OFF		0x3664  /* Sensors 5-10 found on apq8064/msm8960 */
+#define S6_STATUS_OFF		0x3668
+#define S7_STATUS_OFF		0x366c
+#define S8_STATUS_OFF		0x3670
+#define S9_STATUS_OFF		0x3674
+#define S10_STATUS_OFF		0x3678
+
 static int suspend_8960(struct tsens_priv *priv)
 {
 	int ret;

@@ -269,6 +281,66 @@ static int get_temp_8960(const struct tsens_sensor *s, int *temp)
 	return -ETIMEDOUT;
 }
 
+static struct tsens_features tsens_8960_feat = {
+	.ver_major	= VER_0,
+	.crit_int	= 0,
+	.adc		= 1,
+	.srot_split	= 0,
+	.max_sensors	= 11,
+};
+
+static const struct reg_field tsens_8960_regfields[MAX_REGFIELDS] = {
+	/* ----- SROT ------ */
+	/* No VERSION information */
+
+	/* CNTL */
+	[TSENS_EN]     = REG_FIELD(CNTL_ADDR,  0, 0),
+	[TSENS_SW_RST] = REG_FIELD(CNTL_ADDR,  1, 1),
+	/* 8960 has 5 sensors, 8660 has 11, we only handle 5 */
+	[SENSOR_EN]    = REG_FIELD(CNTL_ADDR,  3, 7),
+
+	/* ----- TM ------ */
+	/* INTERRUPT ENABLE */
+	/* NO INTERRUPT ENABLE */
+
+	/* Single UPPER/LOWER TEMPERATURE THRESHOLD for all sensors */
+	[LOW_THRESH_0]   = REG_FIELD(THRESHOLD_ADDR,  0,  7),
+	[UP_THRESH_0]    = REG_FIELD(THRESHOLD_ADDR,  8, 15),
+	[MIN_THRESH_0]   = REG_FIELD(THRESHOLD_ADDR, 16, 23),
+	[MAX_THRESH_0]   = REG_FIELD(THRESHOLD_ADDR, 24, 31),
+
+	/* UPPER/LOWER INTERRUPT [CLEAR/STATUS] */
+	/* 1 == clear, 0 == normal operation */
+	[LOW_INT_CLEAR_0]   = REG_FIELD(CNTL_ADDR,  9,  9),
+	[UP_INT_CLEAR_0]    = REG_FIELD(CNTL_ADDR, 10, 10),
+
+	/* NO CRITICAL INTERRUPT SUPPORT on 8960 */
+
+	/* Sn_STATUS */
+	[LAST_TEMP_0]  = REG_FIELD(S0_STATUS_OFF,  0,  7),
+	[LAST_TEMP_1]  = REG_FIELD(S1_STATUS_OFF,  0,  7),
+	[LAST_TEMP_2]  = REG_FIELD(S2_STATUS_OFF,  0,  7),
+	[LAST_TEMP_3]  = REG_FIELD(S3_STATUS_OFF,  0,  7),
+	[LAST_TEMP_4]  = REG_FIELD(S4_STATUS_OFF,  0,  7),
+	[LAST_TEMP_5]  = REG_FIELD(S5_STATUS_OFF,  0,  7),
+	[LAST_TEMP_6]  = REG_FIELD(S6_STATUS_OFF,  0,  7),
+	[LAST_TEMP_7]  = REG_FIELD(S7_STATUS_OFF,  0,  7),
+	[LAST_TEMP_8]  = REG_FIELD(S8_STATUS_OFF,  0,  7),
+	[LAST_TEMP_9]  = REG_FIELD(S9_STATUS_OFF,  0,  7),
+	[LAST_TEMP_10] = REG_FIELD(S10_STATUS_OFF, 0,  7),
+
+	/* No VALID field on 8960 */
+	/* TSENS_INT_STATUS bits: 1 == threshold violated */
+	[MIN_STATUS_0] = REG_FIELD(INT_STATUS_ADDR, 0, 0),
+	[LOWER_STATUS_0] = REG_FIELD(INT_STATUS_ADDR, 1, 1),
+	[UPPER_STATUS_0] = REG_FIELD(INT_STATUS_ADDR, 2, 2),
+	/* No CRITICAL field on 8960 */
+	[MAX_STATUS_0] = REG_FIELD(INT_STATUS_ADDR, 3, 3),
+
+	/* TRDY: 1=ready, 0=in progress */
+	[TRDY] = REG_FIELD(INT_STATUS_ADDR, 7, 7),
+};
+
 static const struct tsens_ops ops_8960 = {
 	.init		= init_8960,
 	.calibrate	= calibrate_8960,

@@ -282,4 +354,6 @@ static const struct tsens_ops ops_8960 = {
 struct tsens_plat_data data_8960 = {
 	.num_sensors	= 11,
 	.ops		= &ops_8960,
+	.feat		= &tsens_8960_feat,
+	.fields		= tsens_8960_regfields,
 };

-- 
2.27.0

Hi Ansuel,

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

Regards,
Amit

On Fri, Aug 14, 2020 at 7:12 PM Ansuel Smith [off-list ref] wrote:

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

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

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

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

--
2.27.0

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

Hi Ansuel,

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

Regards,
Amit

Thx a lot. Was thinking of resending this but I will wait.

Hi Ansuel,

My apologies for being tardy in reviewing this series. Career changes...

On Fri, Aug 14, 2020 at 7:12 PM Ansuel Smith [off-list ref] wrote:

This is wrong.

.get_temp is set to get_temp_tsens_valid() for v1 and v2 platforms. So
you've just broken all those platforms by creating a recursive loop.

I assume you were trying to use the common interrupt code which
currently uses get_temp_tsens_valid()? I suggest trying to add 8960
support to tsens_hw_to_mC().

-       return 0;
+       return ret;
 }

 int get_temp_common(const struct tsens_sensor *s, int *temp)
--
2.27.0

Hi Ansuel,

See comments inline.

On Fri, Aug 14, 2020 at 7:12 PM Ansuel Smith [off-list ref] wrote:

Let's revisit this after fixing patch 1.

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

Consider reusing LOW_THRESH_0 and UP_THRESH_0 for these?

        /* WATCHDOG */
        WDOG_BARK_STATUS,
        WDOG_BARK_CLEAR,
--
2.27.0

On Mon, Nov 23, 2020 at 01:35:22AM +0530, Amit Kucheria wrote:

Hi Ansuel,

See comments inline.

On Fri, Aug 14, 2020 at 7:12 PM Ansuel Smith [off-list ref] wrote:

quoted

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

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

diff --git a/drivers/thermal/qcom/tsens.c b/drivers/thermal/qcom/tsens.c
index 9fe9a2b26705..965c4799918a 100644
--- a/drivers/thermal/qcom/tsens.c
+++ b/drivers/thermal/qcom/tsens.c

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

        return IRQ_HANDLED;

@@ -531,6 +540,13 @@ static int tsens_set_trips(void *_sensor, int low, int high)
        int high_val, low_val, cl_high, cl_low;
        u32 hw_id = s->hw_id;

+       if (tsens_version(priv) < VER_0_1) {
+               /* Pre v0.1 IP had a single register for each type of interrupt
+                * and thresholds
+                */
+               hw_id = 0;
+       }
+
        dev_dbg(dev, "[%u] %s: proposed thresholds: (%d:%d)\n",
                hw_id, __func__, low, high);

@@ -584,18 +600,21 @@ int get_temp_tsens_valid(const struct tsens_sensor *s, int *temp)
        u32 valid;
        int ret;

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

Let's revisit this after fixing patch 1.

quoted

        }

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

@@ -763,6 +782,10 @@ int __init init_common(struct tsens_priv *priv)
                goto err_put_device;
        }

+       /* VER_0 have only tm_map */
+       if (!priv->srot_map)
+               priv->srot_map = priv->tm_map;
+
        if (tsens_version(priv) > VER_0_1) {
                for (i = VER_MAJOR; i <= VER_STEP; i++) {
                        priv->rf[i] = devm_regmap_field_alloc(dev, priv->srot_map,

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

@@ -803,6 +830,61 @@ int __init init_common(struct tsens_priv *priv)
                goto err_put_device;
        }

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

@@ -856,7 +938,11 @@ int __init init_common(struct tsens_priv *priv)
        }

        spin_lock_init(&priv->ul_lock);
-       tsens_enable_irq(priv);
+
+       /* VER_0 interrupt doesn't need to be enabled */
+       if (tsens_version(priv) >= VER_0_1)
+               tsens_enable_irq(priv);
+
        tsens_debug_init(op);

 err_put_device:

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

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

quoted

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

diff --git a/drivers/thermal/qcom/tsens.h b/drivers/thermal/qcom/tsens.h
index 59d01162c66a..f1120791737c 100644
--- a/drivers/thermal/qcom/tsens.h
+++ b/drivers/thermal/qcom/tsens.h

@@ -25,7 +25,8 @@ struct tsens_priv;

 /* IP version numbers in ascending order */
 enum tsens_ver {
-       VER_0_1 = 0,
+       VER_0 = 0,
+       VER_0_1,
        VER_1_X,
        VER_2_X,
 };

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

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

Consider reusing LOW_THRESH_0 and UP_THRESH_0 for these?

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

quoted

        /* WATCHDOG */
        WDOG_BARK_STATUS,
        WDOG_BARK_CLEAR,
--
2.27.0

On Thu, Nov 26, 2020 at 2:16 AM Ansuel Smith [off-list ref] wrote:

quoted

quoted

 };

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

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

Consider reusing LOW_THRESH_0 and UP_THRESH_0 for these?

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

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

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

On Sun, Nov 29, 2020 at 06:28:01PM +0530, Amit Kucheria wrote:

On Thu, Nov 26, 2020 at 2:16 AM Ansuel Smith [off-list ref] wrote:

quoted

quoted

quoted

 };

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

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

Consider reusing LOW_THRESH_0 and UP_THRESH_0 for these?

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

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

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

Let's sum up things and take a decision about this. On V_0 the original
driver have a special implementation that has a 4 trips point, one
critical high (that should be MAX_THRESH), one critical low (that should
be MIN_THRESH), one configurabile hi and one configurable low.

This is the regfiled
[LOW_THRESH_0]   = REG_FIELD(THRESHOLD_ADDR,  0,  7),
[UP_THRESH_0]    = REG_FIELD(THRESHOLD_ADDR,  8, 15),
[MIN_THRESH_0]   = REG_FIELD(THRESHOLD_ADDR, 16, 23),
[MAX_THRESH_0]   = REG_FIELD(THRESHOLD_ADDR, 24, 31),
and we have the regfiled to check if the threshold is violated.

Looking at the set trips code, since V_0 doesn't have critical
interrupt, we only set the uplow interrupt. Now the current code only
check the LOW and UP regfield and V_0. The original code also check MIN
and MAX (that are set to 125 C and 0 C, that should be the critical trip
point). Should we:
1. drop the MIN and MAX THRESH and keep them unconfigured (and make the
interrupt set only to the UP/LOW trips) or
2. add the missing code to set_trips

Honestrly I'm more with the first approach. I also sent v7 that should
address all the other request. As always thanks for the attention.