All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v11 0/9] Add support for ipq8064 tsens
@ 2021-03-19  0:52 Ansuel Smith
  2021-03-19  0:52 ` [PATCH v11 1/9] drivers: thermal: tsens: Add VER_0 tsens version Ansuel Smith
                   ` (8 more replies)
  0 siblings, 9 replies; 17+ messages in thread
From: Ansuel Smith @ 2021-03-19  0:52 UTC (permalink / raw)
  To: Thara Gopinath
  Cc: Ansuel Smith, Andy Gross, Bjorn Andersson, Amit Kucheria,
	Zhang Rui, Daniel Lezcano, Rob Herring, linux-arm-msm, linux-pm,
	devicetree, linux-kernel

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

v11:
* Address comments from Thara (thx)
v10:
* Fix wrong tsens init for ver_0 (crit_trips needs to be set in tsens_register)
v9:
* Fix warning from Documentation bot
v8:
* Drop MIN and MAX THRESH and use CRIT_THRESH instead
* Fix broken documentation patch
v7:
* Rework calibrate function to use get_temp_common
* Fix wrong required in the Documentation for ipq8064
* Fix hardware bug in sensor enable function
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 (9):
  drivers: thermal: tsens: Add VER_0 tsens version
  drivers: thermal: tsens: Don't hardcode sensor slope
  drivers: thermal: tsens: Convert msm8960 to reg_field
  drivers: thermal: tsens: Use init_common for msm8960
  drivers: thermal: tsens: Fix bug in sensor enable for msm8960
  drivers: thermal: tsens: Replace custom 8960 apis with generic apis
  drivers: thermal: tsens: Drop unused define for msm8960
  drivers: thermal: tsens: Add support for ipq8064-tsens
  dt-bindings: thermal: tsens: Document ipq8064 bindings

 .../bindings/thermal/qcom-tsens.yaml          |  56 ++++-
 drivers/thermal/qcom/tsens-8960.c             | 234 +++++++++---------
 drivers/thermal/qcom/tsens.c                  | 147 ++++++++---
 drivers/thermal/qcom/tsens.h                  |   4 +-
 4 files changed, 285 insertions(+), 156 deletions(-)

-- 
2.30.2


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [PATCH v11 1/9] drivers: thermal: tsens: Add VER_0 tsens version
  2021-03-19  0:52 [PATCH v11 0/9] Add support for ipq8064 tsens Ansuel Smith
@ 2021-03-19  0:52 ` Ansuel Smith
  2021-03-19 13:11   ` Thara Gopinath
  2021-03-19  0:52 ` [PATCH v11 2/9] drivers: thermal: tsens: Don't hardcode sensor slope Ansuel Smith
                   ` (7 subsequent siblings)
  8 siblings, 1 reply; 17+ messages in thread
From: Ansuel Smith @ 2021-03-19  0:52 UTC (permalink / raw)
  To: Thara Gopinath
  Cc: Ansuel Smith, Andy Gross, Bjorn Andersson, Amit Kucheria,
	Zhang Rui, Daniel Lezcano, Rob Herring, linux-arm-msm, linux-pm,
	devicetree, linux-kernel

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 | 141 ++++++++++++++++++++++++++++-------
 drivers/thermal/qcom/tsens.h |   4 +-
 2 files changed, 116 insertions(+), 29 deletions(-)

diff --git a/drivers/thermal/qcom/tsens.c b/drivers/thermal/qcom/tsens.c
index d8ce3a687b80..277d9b17e949 100644
--- a/drivers/thermal/qcom/tsens.c
+++ b/drivers/thermal/qcom/tsens.c
@@ -12,6 +12,7 @@
 #include <linux/of.h>
 #include <linux/of_address.h>
 #include <linux/of_platform.h>
+#include <linux/mfd/syscon.h>
 #include <linux/platform_device.h>
 #include <linux/pm.h>
 #include <linux/regmap.h>
@@ -515,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;
@@ -530,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 +601,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 */
@@ -608,15 +628,29 @@ int get_temp_common(const struct tsens_sensor *s, int *temp)
 {
 	struct tsens_priv *priv = s->priv;
 	int hw_id = s->hw_id;
-	int last_temp = 0, ret;
+	int last_temp = 0, ret, trdy;
+	unsigned long timeout;
 
-	ret = regmap_field_read(priv->rf[LAST_TEMP_0 + hw_id], &last_temp);
-	if (ret)
-		return ret;
+	timeout = jiffies + usecs_to_jiffies(TIMEOUT_US);
+	do {
+		if (tsens_version(priv) == VER_0) {
+			ret = regmap_field_read(priv->rf[TRDY], &trdy);
+			if (ret)
+				return ret;
+			if (!trdy)
+				continue;
+		}
 
-	*temp = code_to_degc(last_temp, s) * 1000;
+		ret = regmap_field_read(priv->rf[LAST_TEMP_0 + hw_id], &last_temp);
+		if (ret)
+			return ret;
 
-	return 0;
+		*temp = code_to_degc(last_temp, s) * 1000;
+
+		return 0;
+	} while (time_before(jiffies, timeout));
+
+	return -ETIMEDOUT;
 }
 
 #ifdef CONFIG_DEBUG_FS
@@ -738,19 +772,31 @@ int __init init_common(struct tsens_priv *priv)
 		priv->tm_offset = 0x1000;
 	}
 
-	res = platform_get_resource(op, IORESOURCE_MEM, 0);
-	tm_base = devm_ioremap_resource(dev, res);
-	if (IS_ERR(tm_base)) {
-		ret = PTR_ERR(tm_base);
-		goto err_put_device;
+	if (tsens_version(priv) >= VER_0_1) {
+		res = platform_get_resource(op, IORESOURCE_MEM, 0);
+		tm_base = devm_ioremap_resource(dev, res);
+		if (IS_ERR(tm_base)) {
+			ret = PTR_ERR(tm_base);
+			goto err_put_device;
+		}
+
+		priv->tm_map = devm_regmap_init_mmio(dev, tm_base, &tsens_config);
+	} else { /* VER_0 share the same gcc regs using a syscon */
+		struct device *parent = priv->dev->parent;
+
+		if (parent)
+			priv->tm_map = syscon_node_to_regmap(parent->of_node);
 	}
 
-	priv->tm_map = devm_regmap_init_mmio(dev, tm_base, &tsens_config);
-	if (IS_ERR(priv->tm_map)) {
+	if (IS_ERR_OR_NULL(priv->tm_map)) {
 		ret = PTR_ERR(priv->tm_map);
 		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,
@@ -769,6 +815,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;
@@ -791,6 +841,20 @@ int __init init_common(struct tsens_priv *priv)
 		goto err_put_device;
 	}
 
+	priv->rf[TSENS_SW_RST] = devm_regmap_field_alloc(
+		dev, priv->srot_map, priv->fields[TSENS_SW_RST]);
+	if (IS_ERR(priv->rf[TSENS_SW_RST])) {
+		ret = PTR_ERR(priv->rf[TSENS_SW_RST]);
+		goto err_put_device;
+	}
+
+	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++) {
@@ -806,7 +870,7 @@ int __init init_common(struct tsens_priv *priv)
 		}
 	}
 
-	if (priv->feat->crit_int) {
+	if (priv->feat->crit_int || tsens_version(priv) < VER_0_1) {
 		/* Loop might need changes if enum regfield_ids is reordered */
 		for (j = CRITICAL_STATUS_0; j <= CRIT_THRESH_15; j += 16) {
 			for (i = 0; i < priv->feat->max_sensors; i++) {
@@ -844,7 +908,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:
@@ -943,10 +1011,14 @@ 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 */
+		ret = devm_request_threaded_irq(
+			&pdev->dev, irq,
+			tsens_version(priv) == VER_0 ? thread_fn : NULL,
+			tsens_version(priv) == VER_0 ? NULL : thread_fn,
+			tsens_version(priv) == VER_0 ? IRQF_TRIGGER_RISING :
+							     IRQF_ONESHOT,
+			dev_name(&pdev->dev), priv);
 		if (ret)
 			dev_err(&pdev->dev, "%s: failed to get irq\n",
 				__func__);
@@ -975,6 +1047,19 @@ static int tsens_register(struct tsens_priv *priv)
 			priv->ops->enable(priv, i);
 	}
 
+	/* VER_0 require to set MIN and MAX THRESH
+	 * These 2 regs are set using the:
+	 * - CRIT_THRESH_0 for MAX THRESH hardcoded to 120°C
+	 * - CRIT_THRESH_1 for MIN THRESH hardcoded to   0°C
+	 */
+	if (tsens_version(priv) < VER_0_1) {
+		regmap_field_write(priv->rf[CRIT_THRESH_0],
+				   tsens_mC_to_hw(priv->sensor, 120000));
+
+		regmap_field_write(priv->rf[CRIT_THRESH_1],
+				   tsens_mC_to_hw(priv->sensor, 0));
+	}
+
 	ret = tsens_register_irq(priv, "uplow", tsens_irq_thread);
 	if (ret < 0)
 		return ret;
diff --git a/drivers/thermal/qcom/tsens.h b/drivers/thermal/qcom/tsens.h
index f40b625f897e..8e6c1fd3ccf5 100644
--- a/drivers/thermal/qcom/tsens.h
+++ b/drivers/thermal/qcom/tsens.h
@@ -13,6 +13,7 @@
 #define CAL_DEGC_PT2		120
 #define SLOPE_FACTOR		1000
 #define SLOPE_DEFAULT		3200
+#define TIMEOUT_US		100
 #define THRESHOLD_MAX_ADC_CODE	0x3ff
 #define THRESHOLD_MIN_ADC_CODE	0x0
 
@@ -25,7 +26,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,
 };
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v11 2/9] drivers: thermal: tsens: Don't hardcode sensor slope
  2021-03-19  0:52 [PATCH v11 0/9] Add support for ipq8064 tsens Ansuel Smith
  2021-03-19  0:52 ` [PATCH v11 1/9] drivers: thermal: tsens: Add VER_0 tsens version Ansuel Smith
@ 2021-03-19  0:52 ` Ansuel Smith
  2021-03-19  0:52 ` [PATCH v11 3/9] drivers: thermal: tsens: Convert msm8960 to reg_field Ansuel Smith
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 17+ messages in thread
From: Ansuel Smith @ 2021-03-19  0:52 UTC (permalink / raw)
  To: Thara Gopinath
  Cc: Ansuel Smith, Andy Gross, Bjorn Andersson, Amit Kucheria,
	Zhang Rui, Daniel Lezcano, Rob Herring, linux-arm-msm, linux-pm,
	devicetree, linux-kernel

Function compute_intercept_slope hardcode the sensor slope to
SLOPE_DEFAULT. Change this and use the default value only if a slope is
not defined. This is needed for tsens VER_0 that has a hardcoded slope
table.

Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
Reviewed-by: Thara Gopinath <thara.gopinath@linaro.org>
---
 drivers/thermal/qcom/tsens.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/thermal/qcom/tsens.c b/drivers/thermal/qcom/tsens.c
index 277d9b17e949..44bce16217db 100644
--- a/drivers/thermal/qcom/tsens.c
+++ b/drivers/thermal/qcom/tsens.c
@@ -86,7 +86,8 @@ void compute_intercept_slope(struct tsens_priv *priv, u32 *p1,
 			"%s: sensor%d - data_point1:%#x data_point2:%#x\n",
 			__func__, i, p1[i], p2[i]);
 
-		priv->sensor[i].slope = SLOPE_DEFAULT;
+		if (!priv->sensor[i].slope)
+			priv->sensor[i].slope = SLOPE_DEFAULT;
 		if (mode == TWO_PT_CALIB) {
 			/*
 			 * slope (m) = adc_code2 - adc_code1 (y2 - y1)/
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v11 3/9] drivers: thermal: tsens: Convert msm8960 to reg_field
  2021-03-19  0:52 [PATCH v11 0/9] Add support for ipq8064 tsens Ansuel Smith
  2021-03-19  0:52 ` [PATCH v11 1/9] drivers: thermal: tsens: Add VER_0 tsens version Ansuel Smith
  2021-03-19  0:52 ` [PATCH v11 2/9] drivers: thermal: tsens: Don't hardcode sensor slope Ansuel Smith
@ 2021-03-19  0:52 ` Ansuel Smith
  2021-03-19  0:52 ` [PATCH v11 4/9] drivers: thermal: tsens: Use init_common for msm8960 Ansuel Smith
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 17+ messages in thread
From: Ansuel Smith @ 2021-03-19  0:52 UTC (permalink / raw)
  To: Thara Gopinath
  Cc: Ansuel Smith, Amit Kucheria, Andy Gross, Bjorn Andersson,
	Zhang Rui, Daniel Lezcano, Rob Herring, linux-pm, linux-arm-msm,
	devicetree, linux-kernel

Convert msm9860 driver to reg_field to use the init_common
function.

Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
Acked-by: Thara Gopinath <thara.gopinath@linaro.org>
---
 drivers/thermal/qcom/tsens-8960.c | 80 ++++++++++++++++++++++++++++++-
 1 file changed, 79 insertions(+), 1 deletion(-)

diff --git a/drivers/thermal/qcom/tsens-8960.c b/drivers/thermal/qcom/tsens-8960.c
index 2a28a5af209e..3f4fc1ffe679 100644
--- a/drivers/thermal/qcom/tsens-8960.c
+++ b/drivers/thermal/qcom/tsens-8960.c
@@ -51,11 +51,22 @@
 #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
 
+#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 +280,71 @@ 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 and MAX_THRESH_0 are not present in the regfield
+	 * Recycle CRIT_THRESH_0 and 1 to set the required regs to hardcoded temp
+	 * MIN_THRESH_0 -> CRIT_THRESH_1
+	 * MAX_THRESH_0 -> CRIT_THRESH_0
+	 */
+	[CRIT_THRESH_1]   = REG_FIELD(THRESHOLD_ADDR, 16, 23),
+	[CRIT_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 +358,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.30.2


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v11 4/9] drivers: thermal: tsens: Use init_common for msm8960
  2021-03-19  0:52 [PATCH v11 0/9] Add support for ipq8064 tsens Ansuel Smith
                   ` (2 preceding siblings ...)
  2021-03-19  0:52 ` [PATCH v11 3/9] drivers: thermal: tsens: Convert msm8960 to reg_field Ansuel Smith
@ 2021-03-19  0:52 ` Ansuel Smith
  2021-03-19  0:52 ` [PATCH v11 5/9] drivers: thermal: tsens: Fix bug in sensor enable " Ansuel Smith
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 17+ messages in thread
From: Ansuel Smith @ 2021-03-19  0:52 UTC (permalink / raw)
  To: Thara Gopinath
  Cc: Ansuel Smith, Amit Kucheria, Andy Gross, Bjorn Andersson,
	Zhang Rui, Daniel Lezcano, Rob Herring, linux-pm, linux-arm-msm,
	devicetree, linux-kernel

Use init_common and drop custom init for msm8960.

Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
Reviewed-by: Thara Gopinath <thara.gopinath@linaro.org>
---
 drivers/thermal/qcom/tsens-8960.c | 52 +------------------------------
 1 file changed, 1 insertion(+), 51 deletions(-)

diff --git a/drivers/thermal/qcom/tsens-8960.c b/drivers/thermal/qcom/tsens-8960.c
index 3f4fc1ffe679..86585f439985 100644
--- a/drivers/thermal/qcom/tsens-8960.c
+++ b/drivers/thermal/qcom/tsens-8960.c
@@ -173,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;
@@ -346,7 +296,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.30.2


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v11 5/9] drivers: thermal: tsens: Fix bug in sensor enable for msm8960
  2021-03-19  0:52 [PATCH v11 0/9] Add support for ipq8064 tsens Ansuel Smith
                   ` (3 preceding siblings ...)
  2021-03-19  0:52 ` [PATCH v11 4/9] drivers: thermal: tsens: Use init_common for msm8960 Ansuel Smith
@ 2021-03-19  0:52 ` Ansuel Smith
  2021-03-19 13:11   ` Thara Gopinath
  2021-03-22 10:47   ` Marc Gonzalez
  2021-03-19  0:52 ` [PATCH v11 6/9] drivers: thermal: tsens: Replace custom 8960 apis with generic apis Ansuel Smith
                   ` (3 subsequent siblings)
  8 siblings, 2 replies; 17+ messages in thread
From: Ansuel Smith @ 2021-03-19  0:52 UTC (permalink / raw)
  To: Thara Gopinath
  Cc: Ansuel Smith, Andy Gross, Bjorn Andersson, Amit Kucheria,
	Zhang Rui, Daniel Lezcano, Rob Herring, linux-arm-msm, linux-pm,
	devicetree, linux-kernel

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

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

diff --git a/drivers/thermal/qcom/tsens-8960.c b/drivers/thermal/qcom/tsens-8960.c
index 86585f439985..bdc64d4188bf 100644
--- a/drivers/thermal/qcom/tsens-8960.c
+++ b/drivers/thermal/qcom/tsens-8960.c
@@ -27,9 +27,9 @@
 #define EN			BIT(0)
 #define SW_RST			BIT(1)
 #define SENSOR0_EN		BIT(3)
+#define MEASURE_PERIOD		BIT(18)
 #define SLP_CLK_ENA		BIT(26)
 #define SLP_CLK_ENA_8660	BIT(24)
-#define MEASURE_PERIOD		1
 #define SENSOR0_SHIFT		3
 
 /* INT_STATUS_ADDR bitmasks */
@@ -126,17 +126,35 @@ static int resume_8960(struct tsens_priv *priv)
 static int enable_8960(struct tsens_priv *priv, int id)
 {
 	int ret;
-	u32 reg, mask;
+	u32 reg, mask = BIT(id);
 
 	ret = regmap_read(priv->tm_map, CNTL_ADDR, &reg);
 	if (ret)
 		return ret;
 
-	mask = BIT(id + SENSOR0_SHIFT);
+	/* HARDWARE BUG:
+	 * On platform with more than 6 sensors, all the remaining
+	 * sensors needs to be enabled all togheder or underfined
+	 * results are expected. (Sensor 6-7 disabled, Sensor 3
+	 * disabled...) In the original driver, all the sensors
+	 * are enabled in one step hence this bug is not triggered.
+	 */
+	if (id > 5) {
+		mask = GENMASK(10, 6);
+
+		/* Sensors already enabled. Skip. */
+		if ((reg & mask) == mask)
+			return 0;
+	}
+
+	mask <<= SENSOR0_SHIFT;
+
 	ret = regmap_write(priv->tm_map, CNTL_ADDR, reg | SW_RST);
 	if (ret)
 		return ret;
 
+	reg |= MEASURE_PERIOD;
+
 	if (priv->num_sensors > 1)
 		reg |= mask | SLP_CLK_ENA | EN;
 	else
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v11 6/9] drivers: thermal: tsens: Replace custom 8960 apis with generic apis
  2021-03-19  0:52 [PATCH v11 0/9] Add support for ipq8064 tsens Ansuel Smith
                   ` (4 preceding siblings ...)
  2021-03-19  0:52 ` [PATCH v11 5/9] drivers: thermal: tsens: Fix bug in sensor enable " Ansuel Smith
@ 2021-03-19  0:52 ` Ansuel Smith
  2021-03-19 13:12   ` Thara Gopinath
  2021-03-19  0:52 ` [PATCH v11 7/9] drivers: thermal: tsens: Drop unused define for msm8960 Ansuel Smith
                   ` (2 subsequent siblings)
  8 siblings, 1 reply; 17+ messages in thread
From: Ansuel Smith @ 2021-03-19  0:52 UTC (permalink / raw)
  To: Thara Gopinath
  Cc: Ansuel Smith, Andy Gross, Bjorn Andersson, Amit Kucheria,
	Zhang Rui, Daniel Lezcano, Rob Herring, linux-arm-msm, linux-pm,
	devicetree, linux-kernel

Rework calibrate function to use common function. Derive the offset from
a missing hardcoded slope table and the data from the nvmem calib
efuses.
Drop custom get_temp function and use generic api.

Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
Acked-by: Thara Gopinath <thara.gopinath@linaro.org>
---
 drivers/thermal/qcom/tsens-8960.c | 56 +++++++++----------------------
 1 file changed, 15 insertions(+), 41 deletions(-)

diff --git a/drivers/thermal/qcom/tsens-8960.c b/drivers/thermal/qcom/tsens-8960.c
index bdc64d4188bf..8c523b764862 100644
--- a/drivers/thermal/qcom/tsens-8960.c
+++ b/drivers/thermal/qcom/tsens-8960.c
@@ -67,6 +67,13 @@
 #define S9_STATUS_OFF		0x3674
 #define S10_STATUS_OFF		0x3678
 
+/* Original slope - 200 to compensate mC to C inaccuracy */
+u32 tsens_msm8960_slope[] = {
+			976, 976, 954, 976,
+			911, 932, 932, 999,
+			932, 999, 932
+			};
+
 static int suspend_8960(struct tsens_priv *priv)
 {
 	int ret;
@@ -195,9 +202,7 @@ static int calibrate_8960(struct tsens_priv *priv)
 {
 	int i;
 	char *data;
-
-	ssize_t num_read = priv->num_sensors;
-	struct tsens_sensor *s = priv->sensor;
+	u32 p1[11];
 
 	data = qfprom_read(priv->dev, "calib");
 	if (IS_ERR(data))
@@ -205,49 +210,18 @@ 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 < priv->num_sensors; i++) {
+		p1[i] = data[i];
+		priv->sensor[i].slope = tsens_msm8960_slope[i];
+	}
+
+	compute_intercept_slope(priv, p1, NULL, ONE_PT_CALIB);
 
 	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;
-
-	slope = thermal_zone_get_slope(s->tzd);
-	offset = CAL_MDEGC - slope * s->offset;
-
-	return adc_code * slope + offset;
-}
-
-static int get_temp_8960(const struct tsens_sensor *s, int *temp)
-{
-	int ret;
-	u32 code, 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);
-		if (ret)
-			return ret;
-		if (!(trdy & TRDY_MASK))
-			continue;
-		ret = regmap_read(priv->tm_map, s->status, &code);
-		if (ret)
-			return ret;
-		*temp = code_to_mdegC(code, s);
-		return 0;
-	} while (time_before(jiffies, timeout));
-
-	return -ETIMEDOUT;
-}
-
 static struct tsens_features tsens_8960_feat = {
 	.ver_major	= VER_0,
 	.crit_int	= 0,
@@ -316,7 +290,7 @@ static const struct reg_field tsens_8960_regfields[MAX_REGFIELDS] = {
 static const struct tsens_ops ops_8960 = {
 	.init		= init_common,
 	.calibrate	= calibrate_8960,
-	.get_temp	= get_temp_8960,
+	.get_temp	= get_temp_common,
 	.enable		= enable_8960,
 	.disable	= disable_8960,
 	.suspend	= suspend_8960,
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v11 7/9] drivers: thermal: tsens: Drop unused define for msm8960
  2021-03-19  0:52 [PATCH v11 0/9] Add support for ipq8064 tsens Ansuel Smith
                   ` (5 preceding siblings ...)
  2021-03-19  0:52 ` [PATCH v11 6/9] drivers: thermal: tsens: Replace custom 8960 apis with generic apis Ansuel Smith
@ 2021-03-19  0:52 ` Ansuel Smith
  2021-03-19 13:12   ` Thara Gopinath
  2021-03-19  0:52 ` [PATCH v11 8/9] drivers: thermal: tsens: Add support for ipq8064-tsens Ansuel Smith
  2021-03-19  0:52 ` [PATCH v11 9/9] dt-bindings: thermal: tsens: Document ipq8064 bindings Ansuel Smith
  8 siblings, 1 reply; 17+ messages in thread
From: Ansuel Smith @ 2021-03-19  0:52 UTC (permalink / raw)
  To: Thara Gopinath
  Cc: Ansuel Smith, Amit Kucheria, Andy Gross, Bjorn Andersson,
	Zhang Rui, Daniel Lezcano, Rob Herring, linux-pm, linux-arm-msm,
	devicetree, linux-kernel

Drop unused define for msm8960 replaced by generic api and reg_field.

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

diff --git a/drivers/thermal/qcom/tsens-8960.c b/drivers/thermal/qcom/tsens-8960.c
index 8c523b764862..31e44d17d484 100644
--- a/drivers/thermal/qcom/tsens-8960.c
+++ b/drivers/thermal/qcom/tsens-8960.c
@@ -10,8 +10,6 @@
 #include <linux/thermal.h>
 #include "tsens.h"
 
-#define CAL_MDEGC		30000
-
 #define CONFIG_ADDR		0x3640
 #define CONFIG_ADDR_8660	0x3620
 /* CONFIG_ADDR bitmasks */
@@ -21,39 +19,19 @@
 #define CONFIG_SHIFT_8660	28
 #define CONFIG_MASK_8660	(3 << CONFIG_SHIFT_8660)
 
-#define STATUS_CNTL_ADDR_8064	0x3660
 #define CNTL_ADDR		0x3620
 /* CNTL_ADDR bitmasks */
 #define EN			BIT(0)
 #define SW_RST			BIT(1)
-#define SENSOR0_EN		BIT(3)
+
 #define MEASURE_PERIOD		BIT(18)
 #define SLP_CLK_ENA		BIT(26)
 #define SLP_CLK_ENA_8660	BIT(24)
 #define SENSOR0_SHIFT		3
 
-/* INT_STATUS_ADDR bitmasks */
-#define MIN_STATUS_MASK		BIT(0)
-#define LOWER_STATUS_CLR	BIT(1)
-#define UPPER_STATUS_CLR	BIT(2)
-#define MAX_STATUS_MASK		BIT(3)
-
 #define THRESHOLD_ADDR		0x3624
-/* THRESHOLD_ADDR bitmasks */
-#define THRESHOLD_MAX_LIMIT_SHIFT	24
-#define THRESHOLD_MIN_LIMIT_SHIFT	16
-#define THRESHOLD_UPPER_LIMIT_SHIFT	8
-#define THRESHOLD_LOWER_LIMIT_SHIFT	0
-
-/* Initial temperature threshold values */
-#define LOWER_LIMIT_TH		0x50
-#define UPPER_LIMIT_TH		0xdf
-#define MIN_LIMIT_TH		0x0
-#define MAX_LIMIT_TH		0xff
 
 #define INT_STATUS_ADDR		0x363c
-#define TRDY_MASK		BIT(7)
-#define TIMEOUT_US		100
 
 #define S0_STATUS_OFF		0x3628
 #define S1_STATUS_OFF		0x362c
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v11 8/9] drivers: thermal: tsens: Add support for ipq8064-tsens
  2021-03-19  0:52 [PATCH v11 0/9] Add support for ipq8064 tsens Ansuel Smith
                   ` (6 preceding siblings ...)
  2021-03-19  0:52 ` [PATCH v11 7/9] drivers: thermal: tsens: Drop unused define for msm8960 Ansuel Smith
@ 2021-03-19  0:52 ` Ansuel Smith
  2021-03-19  0:52 ` [PATCH v11 9/9] dt-bindings: thermal: tsens: Document ipq8064 bindings Ansuel Smith
  8 siblings, 0 replies; 17+ messages in thread
From: Ansuel Smith @ 2021-03-19  0:52 UTC (permalink / raw)
  To: Thara Gopinath
  Cc: Ansuel Smith, Amit Kucheria, Andy Gross, Bjorn Andersson,
	Zhang Rui, Daniel Lezcano, Rob Herring, linux-pm, linux-arm-msm,
	devicetree, linux-kernel

Add support for tsens present in ipq806x SoCs based on generic msm8960
tsens driver.

Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
Reviewed-by: Thara Gopinath <thara.gopinath@linaro.org>
---
 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 44bce16217db..1fd634f8f5d2 100644
--- a/drivers/thermal/qcom/tsens.c
+++ b/drivers/thermal/qcom/tsens.c
@@ -964,6 +964,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.30.2


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [PATCH v11 9/9] dt-bindings: thermal: tsens: Document ipq8064 bindings
  2021-03-19  0:52 [PATCH v11 0/9] Add support for ipq8064 tsens Ansuel Smith
                   ` (7 preceding siblings ...)
  2021-03-19  0:52 ` [PATCH v11 8/9] drivers: thermal: tsens: Add support for ipq8064-tsens Ansuel Smith
@ 2021-03-19  0:52 ` Ansuel Smith
  8 siblings, 0 replies; 17+ messages in thread
From: Ansuel Smith @ 2021-03-19  0:52 UTC (permalink / raw)
  To: Thara Gopinath
  Cc: Ansuel Smith, Rob Herring, Andy Gross, Bjorn Andersson,
	Amit Kucheria, Zhang Rui, Daniel Lezcano, Rob Herring,
	linux-arm-msm, linux-pm, devicetree, linux-kernel

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          | 56 ++++++++++++++++---
 1 file changed, 48 insertions(+), 8 deletions(-)

diff --git a/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml b/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml
index 95462e071ab4..1785b1c75a3c 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:
@@ -73,7 +78,9 @@ properties:
     maxItems: 2
     items:
       - const: calib
-      - const: calib_sel
+      - enum:
+          - calib_backup
+          - calib_sel
 
   "#qcom,sensors":
     description:
@@ -88,12 +95,20 @@ properties:
       Number of cells required to uniquely identify the thermal sensors. Since
       we have multiple sensors this is set to 1
 
+required:
+  - compatible
+  - interrupts
+  - interrupt-names
+  - "#thermal-sensor-cells"
+  - "#qcom,sensors"
+
 allOf:
   - if:
       properties:
         compatible:
           contains:
             enum:
+              - qcom,ipq8064-tsens
               - qcom,msm8916-tsens
               - qcom,msm8974-tsens
               - qcom,msm8976-tsens
@@ -114,17 +129,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
 
 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_calib_backup>;
+                 nvmem-cell-names = "calib", "calib_backup";
+                 interrupts = <GIC_SPI 178 IRQ_TYPE_LEVEL_HIGH>;
+                 interrupt-names = "uplow";
+
+                 #qcom,sensors = <11>;
+                 #thermal-sensor-cells = <1>;
+          };
+    };
+
   - |
     #include <dt-bindings/interrupt-controller/arm-gic.h>
     // Example 1 (legacy: for pre v1 IP):
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [PATCH v11 1/9] drivers: thermal: tsens: Add VER_0 tsens version
  2021-03-19  0:52 ` [PATCH v11 1/9] drivers: thermal: tsens: Add VER_0 tsens version Ansuel Smith
@ 2021-03-19 13:11   ` Thara Gopinath
  2021-03-19 13:28     ` Ansuel Smith
  0 siblings, 1 reply; 17+ messages in thread
From: Thara Gopinath @ 2021-03-19 13:11 UTC (permalink / raw)
  To: Ansuel Smith
  Cc: Andy Gross, Bjorn Andersson, Amit Kucheria, Zhang Rui,
	Daniel Lezcano, Rob Herring, linux-arm-msm, linux-pm, devicetree,
	linux-kernel



On 3/18/21 8:52 PM, Ansuel Smith wrote:
> 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.

Hi Ansuel,

There are still checkpatch check warnings in this patch. Please run 
checkpatch.pl --strict and fix them. Once that is done, you can add

Reviewed-by: Thara Gopinath <thara.gopinath@linaro.org>

Warm Regards
Thara

> 
> Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
> ---
>   drivers/thermal/qcom/tsens.c | 141 ++++++++++++++++++++++++++++-------
>   drivers/thermal/qcom/tsens.h |   4 +-
>   2 files changed, 116 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/thermal/qcom/tsens.c b/drivers/thermal/qcom/tsens.c
> index d8ce3a687b80..277d9b17e949 100644
> --- a/drivers/thermal/qcom/tsens.c
> +++ b/drivers/thermal/qcom/tsens.c
> @@ -12,6 +12,7 @@
>   #include <linux/of.h>
>   #include <linux/of_address.h>
>   #include <linux/of_platform.h>
> +#include <linux/mfd/syscon.h>
>   #include <linux/platform_device.h>
>   #include <linux/pm.h>
>   #include <linux/regmap.h>
> @@ -515,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;
> @@ -530,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 +601,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 */
> @@ -608,15 +628,29 @@ int get_temp_common(const struct tsens_sensor *s, int *temp)
>   {
>   	struct tsens_priv *priv = s->priv;
>   	int hw_id = s->hw_id;
> -	int last_temp = 0, ret;
> +	int last_temp = 0, ret, trdy;
> +	unsigned long timeout;
>   
> -	ret = regmap_field_read(priv->rf[LAST_TEMP_0 + hw_id], &last_temp);
> -	if (ret)
> -		return ret;
> +	timeout = jiffies + usecs_to_jiffies(TIMEOUT_US);
> +	do {
> +		if (tsens_version(priv) == VER_0) {
> +			ret = regmap_field_read(priv->rf[TRDY], &trdy);
> +			if (ret)
> +				return ret;
> +			if (!trdy)
> +				continue;
> +		}
>   
> -	*temp = code_to_degc(last_temp, s) * 1000;
> +		ret = regmap_field_read(priv->rf[LAST_TEMP_0 + hw_id], &last_temp);
> +		if (ret)
> +			return ret;
>   
> -	return 0;
> +		*temp = code_to_degc(last_temp, s) * 1000;
> +
> +		return 0;
> +	} while (time_before(jiffies, timeout));
> +
> +	return -ETIMEDOUT;
>   }
>   
>   #ifdef CONFIG_DEBUG_FS
> @@ -738,19 +772,31 @@ int __init init_common(struct tsens_priv *priv)
>   		priv->tm_offset = 0x1000;
>   	}
>   
> -	res = platform_get_resource(op, IORESOURCE_MEM, 0);
> -	tm_base = devm_ioremap_resource(dev, res);
> -	if (IS_ERR(tm_base)) {
> -		ret = PTR_ERR(tm_base);
> -		goto err_put_device;
> +	if (tsens_version(priv) >= VER_0_1) {
> +		res = platform_get_resource(op, IORESOURCE_MEM, 0);
> +		tm_base = devm_ioremap_resource(dev, res);
> +		if (IS_ERR(tm_base)) {
> +			ret = PTR_ERR(tm_base);
> +			goto err_put_device;
> +		}
> +
> +		priv->tm_map = devm_regmap_init_mmio(dev, tm_base, &tsens_config);
> +	} else { /* VER_0 share the same gcc regs using a syscon */
> +		struct device *parent = priv->dev->parent;
> +
> +		if (parent)
> +			priv->tm_map = syscon_node_to_regmap(parent->of_node);
>   	}
>   
> -	priv->tm_map = devm_regmap_init_mmio(dev, tm_base, &tsens_config);
> -	if (IS_ERR(priv->tm_map)) {
> +	if (IS_ERR_OR_NULL(priv->tm_map)) {
>   		ret = PTR_ERR(priv->tm_map);
>   		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,
> @@ -769,6 +815,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;
> @@ -791,6 +841,20 @@ int __init init_common(struct tsens_priv *priv)
>   		goto err_put_device;
>   	}
>   
> +	priv->rf[TSENS_SW_RST] = devm_regmap_field_alloc(
> +		dev, priv->srot_map, priv->fields[TSENS_SW_RST]);
> +	if (IS_ERR(priv->rf[TSENS_SW_RST])) {
> +		ret = PTR_ERR(priv->rf[TSENS_SW_RST]);
> +		goto err_put_device;
> +	}
> +
> +	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++) {
> @@ -806,7 +870,7 @@ int __init init_common(struct tsens_priv *priv)
>   		}
>   	}
>   
> -	if (priv->feat->crit_int) {
> +	if (priv->feat->crit_int || tsens_version(priv) < VER_0_1) {
>   		/* Loop might need changes if enum regfield_ids is reordered */
>   		for (j = CRITICAL_STATUS_0; j <= CRIT_THRESH_15; j += 16) {
>   			for (i = 0; i < priv->feat->max_sensors; i++) {
> @@ -844,7 +908,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:
> @@ -943,10 +1011,14 @@ 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 */
> +		ret = devm_request_threaded_irq(
> +			&pdev->dev, irq,
> +			tsens_version(priv) == VER_0 ? thread_fn : NULL,
> +			tsens_version(priv) == VER_0 ? NULL : thread_fn,
> +			tsens_version(priv) == VER_0 ? IRQF_TRIGGER_RISING :
> +							     IRQF_ONESHOT,
> +			dev_name(&pdev->dev), priv);
>   		if (ret)
>   			dev_err(&pdev->dev, "%s: failed to get irq\n",
>   				__func__);
> @@ -975,6 +1047,19 @@ static int tsens_register(struct tsens_priv *priv)
>   			priv->ops->enable(priv, i);
>   	}
>   
> +	/* VER_0 require to set MIN and MAX THRESH
> +	 * These 2 regs are set using the:
> +	 * - CRIT_THRESH_0 for MAX THRESH hardcoded to 120°C
> +	 * - CRIT_THRESH_1 for MIN THRESH hardcoded to   0°C
> +	 */
> +	if (tsens_version(priv) < VER_0_1) {
> +		regmap_field_write(priv->rf[CRIT_THRESH_0],
> +				   tsens_mC_to_hw(priv->sensor, 120000));
> +
> +		regmap_field_write(priv->rf[CRIT_THRESH_1],
> +				   tsens_mC_to_hw(priv->sensor, 0));
> +	}
> +
>   	ret = tsens_register_irq(priv, "uplow", tsens_irq_thread);
>   	if (ret < 0)
>   		return ret;
> diff --git a/drivers/thermal/qcom/tsens.h b/drivers/thermal/qcom/tsens.h
> index f40b625f897e..8e6c1fd3ccf5 100644
> --- a/drivers/thermal/qcom/tsens.h
> +++ b/drivers/thermal/qcom/tsens.h
> @@ -13,6 +13,7 @@
>   #define CAL_DEGC_PT2		120
>   #define SLOPE_FACTOR		1000
>   #define SLOPE_DEFAULT		3200
> +#define TIMEOUT_US		100
>   #define THRESHOLD_MAX_ADC_CODE	0x3ff
>   #define THRESHOLD_MIN_ADC_CODE	0x0
>   
> @@ -25,7 +26,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,
>   };
>

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v11 5/9] drivers: thermal: tsens: Fix bug in sensor enable for msm8960
  2021-03-19  0:52 ` [PATCH v11 5/9] drivers: thermal: tsens: Fix bug in sensor enable " Ansuel Smith
@ 2021-03-19 13:11   ` Thara Gopinath
  2021-03-22 10:47   ` Marc Gonzalez
  1 sibling, 0 replies; 17+ messages in thread
From: Thara Gopinath @ 2021-03-19 13:11 UTC (permalink / raw)
  To: Ansuel Smith
  Cc: Andy Gross, Bjorn Andersson, Amit Kucheria, Zhang Rui,
	Daniel Lezcano, Rob Herring, linux-arm-msm, linux-pm, devicetree,
	linux-kernel

Hi!

On 3/18/21 8:52 PM, Ansuel Smith wrote:
> Device based on tsens VER_0 contains a hardware bug that results in some
> problem with sensor enablement. Sensor id 6-11 can't be enabled
> selectively and all of them must be enabled in one step.

Thanks for rewording!

> 
> Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
> ---
>   drivers/thermal/qcom/tsens-8960.c | 24 +++++++++++++++++++++---
>   1 file changed, 21 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/thermal/qcom/tsens-8960.c b/drivers/thermal/qcom/tsens-8960.c
> index 86585f439985..bdc64d4188bf 100644
> --- a/drivers/thermal/qcom/tsens-8960.c
> +++ b/drivers/thermal/qcom/tsens-8960.c
> @@ -27,9 +27,9 @@
>   #define EN			BIT(0)
>   #define SW_RST			BIT(1)
>   #define SENSOR0_EN		BIT(3)
> +#define MEASURE_PERIOD		BIT(18)
>   #define SLP_CLK_ENA		BIT(26)
>   #define SLP_CLK_ENA_8660	BIT(24)
> -#define MEASURE_PERIOD		1
>   #define SENSOR0_SHIFT		3
>   
>   /* INT_STATUS_ADDR bitmasks */
> @@ -126,17 +126,35 @@ static int resume_8960(struct tsens_priv *priv)
>   static int enable_8960(struct tsens_priv *priv, int id)
>   {
>   	int ret;
> -	u32 reg, mask;
> +	u32 reg, mask = BIT(id);
>   
>   	ret = regmap_read(priv->tm_map, CNTL_ADDR, &reg);
>   	if (ret)
>   		return ret;
>   
> -	mask = BIT(id + SENSOR0_SHIFT);
> +	/* HARDWARE BUG:
> +	 * On platform with more than 6 sensors, all the remaining
> +	 * sensors needs to be enabled all togheder or underfined
> +	 * results are expected. (Sensor 6-7 disabled, Sensor 3
> +	 * disabled...) In the original driver, all the sensors
> +	 * are enabled in one step hence this bug is not triggered.
> +	 */
> +	if (id > 5) {
> +		mask = GENMASK(10, 6);
> +
> +		/* Sensors already enabled. Skip. */
> +		if ((reg & mask) == mask)

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

> +			return 0;
> +	}
> +
> +	mask <<= SENSOR0_SHIFT;
> +
>   	ret = regmap_write(priv->tm_map, CNTL_ADDR, reg | SW_RST);
>   	if (ret)
>   		return ret;
>   
> +	reg |= MEASURE_PERIOD;
> +
>   	if (priv->num_sensors > 1)
>   		reg |= mask | SLP_CLK_ENA | EN;
>   	else
> 

-- 
Warm Regards
Thara

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v11 6/9] drivers: thermal: tsens: Replace custom 8960 apis with generic apis
  2021-03-19  0:52 ` [PATCH v11 6/9] drivers: thermal: tsens: Replace custom 8960 apis with generic apis Ansuel Smith
@ 2021-03-19 13:12   ` Thara Gopinath
  0 siblings, 0 replies; 17+ messages in thread
From: Thara Gopinath @ 2021-03-19 13:12 UTC (permalink / raw)
  To: Ansuel Smith
  Cc: Andy Gross, Bjorn Andersson, Amit Kucheria, Zhang Rui,
	Daniel Lezcano, Rob Herring, linux-arm-msm, linux-pm, devicetree,
	linux-kernel



On 3/18/21 8:52 PM, Ansuel Smith wrote:
> Rework calibrate function to use common function. Derive the offset from
> a missing hardcoded slope table and the data from the nvmem calib
> efuses.
> Drop custom get_temp function and use generic api.
> 
> Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
> Acked-by: Thara Gopinath <thara.gopinath@linaro.org>
> ---
>   drivers/thermal/qcom/tsens-8960.c | 56 +++++++++----------------------
>   1 file changed, 15 insertions(+), 41 deletions(-)
> 
> diff --git a/drivers/thermal/qcom/tsens-8960.c b/drivers/thermal/qcom/tsens-8960.c
> index bdc64d4188bf..8c523b764862 100644
> --- a/drivers/thermal/qcom/tsens-8960.c
> +++ b/drivers/thermal/qcom/tsens-8960.c
> @@ -67,6 +67,13 @@
>   #define S9_STATUS_OFF		0x3674
>   #define S10_STATUS_OFF		0x3678
>   
> +/* Original slope - 200 to compensate mC to C inaccuracy */
> +u32 tsens_msm8960_slope[] = {
> +			976, 976, 954, 976,
> +			911, 932, 932, 999,
> +			932, 999, 932
> +			};

make -C1 throws a warning for this. You have to make the table static. 
You can keep my Acked-by once it is fixed.

Warm Regards
Thara

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v11 7/9] drivers: thermal: tsens: Drop unused define for msm8960
  2021-03-19  0:52 ` [PATCH v11 7/9] drivers: thermal: tsens: Drop unused define for msm8960 Ansuel Smith
@ 2021-03-19 13:12   ` Thara Gopinath
  0 siblings, 0 replies; 17+ messages in thread
From: Thara Gopinath @ 2021-03-19 13:12 UTC (permalink / raw)
  To: Ansuel Smith
  Cc: Amit Kucheria, Andy Gross, Bjorn Andersson, Zhang Rui,
	Daniel Lezcano, Rob Herring, linux-pm, linux-arm-msm, devicetree,
	linux-kernel



On 3/18/21 8:52 PM, Ansuel Smith wrote:
> Drop unused define for msm8960 replaced by generic api and reg_field.
> 
> Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>

Thanks for the clean up!

Reviewed-by: Thara Gopinath <thara.gopinath@linaro.org>

Warm Regards
Thara

> ---
>   drivers/thermal/qcom/tsens-8960.c | 24 +-----------------------
>   1 file changed, 1 insertion(+), 23 deletions(-)
> 
> diff --git a/drivers/thermal/qcom/tsens-8960.c b/drivers/thermal/qcom/tsens-8960.c
> index 8c523b764862..31e44d17d484 100644
> --- a/drivers/thermal/qcom/tsens-8960.c
> +++ b/drivers/thermal/qcom/tsens-8960.c
> @@ -10,8 +10,6 @@
>   #include <linux/thermal.h>
>   #include "tsens.h"
>   
> -#define CAL_MDEGC		30000
> -
>   #define CONFIG_ADDR		0x3640
>   #define CONFIG_ADDR_8660	0x3620
>   /* CONFIG_ADDR bitmasks */
> @@ -21,39 +19,19 @@
>   #define CONFIG_SHIFT_8660	28
>   #define CONFIG_MASK_8660	(3 << CONFIG_SHIFT_8660)
>   
> -#define STATUS_CNTL_ADDR_8064	0x3660
>   #define CNTL_ADDR		0x3620
>   /* CNTL_ADDR bitmasks */
>   #define EN			BIT(0)
>   #define SW_RST			BIT(1)
> -#define SENSOR0_EN		BIT(3)
> +
>   #define MEASURE_PERIOD		BIT(18)
>   #define SLP_CLK_ENA		BIT(26)
>   #define SLP_CLK_ENA_8660	BIT(24)
>   #define SENSOR0_SHIFT		3
>   
> -/* INT_STATUS_ADDR bitmasks */
> -#define MIN_STATUS_MASK		BIT(0)
> -#define LOWER_STATUS_CLR	BIT(1)
> -#define UPPER_STATUS_CLR	BIT(2)
> -#define MAX_STATUS_MASK		BIT(3)
> -
>   #define THRESHOLD_ADDR		0x3624
> -/* THRESHOLD_ADDR bitmasks */
> -#define THRESHOLD_MAX_LIMIT_SHIFT	24
> -#define THRESHOLD_MIN_LIMIT_SHIFT	16
> -#define THRESHOLD_UPPER_LIMIT_SHIFT	8
> -#define THRESHOLD_LOWER_LIMIT_SHIFT	0
> -
> -/* Initial temperature threshold values */
> -#define LOWER_LIMIT_TH		0x50
> -#define UPPER_LIMIT_TH		0xdf
> -#define MIN_LIMIT_TH		0x0
> -#define MAX_LIMIT_TH		0xff
>   
>   #define INT_STATUS_ADDR		0x363c
> -#define TRDY_MASK		BIT(7)
> -#define TIMEOUT_US		100
>   
>   #define S0_STATUS_OFF		0x3628
>   #define S1_STATUS_OFF		0x362c
> 

-- 
Warm Regards
Thara

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v11 1/9] drivers: thermal: tsens: Add VER_0 tsens version
  2021-03-19 13:11   ` Thara Gopinath
@ 2021-03-19 13:28     ` Ansuel Smith
  2021-03-19 14:11       ` Thara Gopinath
  0 siblings, 1 reply; 17+ messages in thread
From: Ansuel Smith @ 2021-03-19 13:28 UTC (permalink / raw)
  To: Thara Gopinath
  Cc: Andy Gross, Bjorn Andersson, Amit Kucheria, Zhang Rui,
	Daniel Lezcano, Rob Herring, linux-arm-msm, linux-pm, devicetree,
	linux-kernel

On Fri, Mar 19, 2021 at 09:11:38AM -0400, Thara Gopinath wrote:
> 
> 
> On 3/18/21 8:52 PM, Ansuel Smith wrote:
> > 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.
> 
> Hi Ansuel,
> 
> There are still checkpatch check warnings in this patch. Please run
> checkpatch.pl --strict and fix them. Once that is done, you can add
> 
> Reviewed-by: Thara Gopinath <thara.gopinath@linaro.org>
> 
> Warm Regards
> Thara
>

Hi,
thanks a lot for the review. The only warning I have is a line ending
with ( that i think I can't fix or I will go over the max char for line.
Do you have something more?

> > 
> > Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
> > ---
> >   drivers/thermal/qcom/tsens.c | 141 ++++++++++++++++++++++++++++-------
> >   drivers/thermal/qcom/tsens.h |   4 +-
> >   2 files changed, 116 insertions(+), 29 deletions(-)
> > 
> > diff --git a/drivers/thermal/qcom/tsens.c b/drivers/thermal/qcom/tsens.c
> > index d8ce3a687b80..277d9b17e949 100644
> > --- a/drivers/thermal/qcom/tsens.c
> > +++ b/drivers/thermal/qcom/tsens.c
> > @@ -12,6 +12,7 @@
> >   #include <linux/of.h>
> >   #include <linux/of_address.h>
> >   #include <linux/of_platform.h>
> > +#include <linux/mfd/syscon.h>
> >   #include <linux/platform_device.h>
> >   #include <linux/pm.h>
> >   #include <linux/regmap.h>
> > @@ -515,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;
> > @@ -530,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 +601,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 */
> > @@ -608,15 +628,29 @@ int get_temp_common(const struct tsens_sensor *s, int *temp)
> >   {
> >   	struct tsens_priv *priv = s->priv;
> >   	int hw_id = s->hw_id;
> > -	int last_temp = 0, ret;
> > +	int last_temp = 0, ret, trdy;
> > +	unsigned long timeout;
> > -	ret = regmap_field_read(priv->rf[LAST_TEMP_0 + hw_id], &last_temp);
> > -	if (ret)
> > -		return ret;
> > +	timeout = jiffies + usecs_to_jiffies(TIMEOUT_US);
> > +	do {
> > +		if (tsens_version(priv) == VER_0) {
> > +			ret = regmap_field_read(priv->rf[TRDY], &trdy);
> > +			if (ret)
> > +				return ret;
> > +			if (!trdy)
> > +				continue;
> > +		}
> > -	*temp = code_to_degc(last_temp, s) * 1000;
> > +		ret = regmap_field_read(priv->rf[LAST_TEMP_0 + hw_id], &last_temp);
> > +		if (ret)
> > +			return ret;
> > -	return 0;
> > +		*temp = code_to_degc(last_temp, s) * 1000;
> > +
> > +		return 0;
> > +	} while (time_before(jiffies, timeout));
> > +
> > +	return -ETIMEDOUT;
> >   }
> >   #ifdef CONFIG_DEBUG_FS
> > @@ -738,19 +772,31 @@ int __init init_common(struct tsens_priv *priv)
> >   		priv->tm_offset = 0x1000;
> >   	}
> > -	res = platform_get_resource(op, IORESOURCE_MEM, 0);
> > -	tm_base = devm_ioremap_resource(dev, res);
> > -	if (IS_ERR(tm_base)) {
> > -		ret = PTR_ERR(tm_base);
> > -		goto err_put_device;
> > +	if (tsens_version(priv) >= VER_0_1) {
> > +		res = platform_get_resource(op, IORESOURCE_MEM, 0);
> > +		tm_base = devm_ioremap_resource(dev, res);
> > +		if (IS_ERR(tm_base)) {
> > +			ret = PTR_ERR(tm_base);
> > +			goto err_put_device;
> > +		}
> > +
> > +		priv->tm_map = devm_regmap_init_mmio(dev, tm_base, &tsens_config);
> > +	} else { /* VER_0 share the same gcc regs using a syscon */
> > +		struct device *parent = priv->dev->parent;
> > +
> > +		if (parent)
> > +			priv->tm_map = syscon_node_to_regmap(parent->of_node);
> >   	}
> > -	priv->tm_map = devm_regmap_init_mmio(dev, tm_base, &tsens_config);
> > -	if (IS_ERR(priv->tm_map)) {
> > +	if (IS_ERR_OR_NULL(priv->tm_map)) {
> >   		ret = PTR_ERR(priv->tm_map);
> >   		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,
> > @@ -769,6 +815,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;
> > @@ -791,6 +841,20 @@ int __init init_common(struct tsens_priv *priv)
> >   		goto err_put_device;
> >   	}
> > +	priv->rf[TSENS_SW_RST] = devm_regmap_field_alloc(
> > +		dev, priv->srot_map, priv->fields[TSENS_SW_RST]);
> > +	if (IS_ERR(priv->rf[TSENS_SW_RST])) {
> > +		ret = PTR_ERR(priv->rf[TSENS_SW_RST]);
> > +		goto err_put_device;
> > +	}
> > +
> > +	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++) {
> > @@ -806,7 +870,7 @@ int __init init_common(struct tsens_priv *priv)
> >   		}
> >   	}
> > -	if (priv->feat->crit_int) {
> > +	if (priv->feat->crit_int || tsens_version(priv) < VER_0_1) {
> >   		/* Loop might need changes if enum regfield_ids is reordered */
> >   		for (j = CRITICAL_STATUS_0; j <= CRIT_THRESH_15; j += 16) {
> >   			for (i = 0; i < priv->feat->max_sensors; i++) {
> > @@ -844,7 +908,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:
> > @@ -943,10 +1011,14 @@ 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 */
> > +		ret = devm_request_threaded_irq(
> > +			&pdev->dev, irq,
> > +			tsens_version(priv) == VER_0 ? thread_fn : NULL,
> > +			tsens_version(priv) == VER_0 ? NULL : thread_fn,
> > +			tsens_version(priv) == VER_0 ? IRQF_TRIGGER_RISING :
> > +							     IRQF_ONESHOT,
> > +			dev_name(&pdev->dev), priv);
> >   		if (ret)
> >   			dev_err(&pdev->dev, "%s: failed to get irq\n",
> >   				__func__);
> > @@ -975,6 +1047,19 @@ static int tsens_register(struct tsens_priv *priv)
> >   			priv->ops->enable(priv, i);
> >   	}
> > +	/* VER_0 require to set MIN and MAX THRESH
> > +	 * These 2 regs are set using the:
> > +	 * - CRIT_THRESH_0 for MAX THRESH hardcoded to 120°C
> > +	 * - CRIT_THRESH_1 for MIN THRESH hardcoded to   0°C
> > +	 */
> > +	if (tsens_version(priv) < VER_0_1) {
> > +		regmap_field_write(priv->rf[CRIT_THRESH_0],
> > +				   tsens_mC_to_hw(priv->sensor, 120000));
> > +
> > +		regmap_field_write(priv->rf[CRIT_THRESH_1],
> > +				   tsens_mC_to_hw(priv->sensor, 0));
> > +	}
> > +
> >   	ret = tsens_register_irq(priv, "uplow", tsens_irq_thread);
> >   	if (ret < 0)
> >   		return ret;
> > diff --git a/drivers/thermal/qcom/tsens.h b/drivers/thermal/qcom/tsens.h
> > index f40b625f897e..8e6c1fd3ccf5 100644
> > --- a/drivers/thermal/qcom/tsens.h
> > +++ b/drivers/thermal/qcom/tsens.h
> > @@ -13,6 +13,7 @@
> >   #define CAL_DEGC_PT2		120
> >   #define SLOPE_FACTOR		1000
> >   #define SLOPE_DEFAULT		3200
> > +#define TIMEOUT_US		100
> >   #define THRESHOLD_MAX_ADC_CODE	0x3ff
> >   #define THRESHOLD_MIN_ADC_CODE	0x0
> > @@ -25,7 +26,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,
> >   };
> > 

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v11 1/9] drivers: thermal: tsens: Add VER_0 tsens version
  2021-03-19 13:28     ` Ansuel Smith
@ 2021-03-19 14:11       ` Thara Gopinath
  0 siblings, 0 replies; 17+ messages in thread
From: Thara Gopinath @ 2021-03-19 14:11 UTC (permalink / raw)
  To: Ansuel Smith
  Cc: Andy Gross, Bjorn Andersson, Amit Kucheria, Zhang Rui,
	Daniel Lezcano, Rob Herring, linux-arm-msm, linux-pm, devicetree,
	linux-kernel



On 3/19/21 9:28 AM, Ansuel Smith wrote:
> On Fri, Mar 19, 2021 at 09:11:38AM -0400, Thara Gopinath wrote:
>>
>>
>> On 3/18/21 8:52 PM, Ansuel Smith wrote:
>>> 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.
>>
>> Hi Ansuel,
>>
>> There are still checkpatch check warnings in this patch. Please run
>> checkpatch.pl --strict and fix them. Once that is done, you can add
>>
>> Reviewed-by: Thara Gopinath <thara.gopinath@linaro.org>
>>
>> Warm Regards
>> Thara
>>
> 
> Hi,
> thanks a lot for the review. The only warning I have is a line ending
> with ( that i think I can't fix or I will go over the max char for line.
> Do you have something more?

I see two warning for line ending with (. The max char limit is 100.

-- 
Warm Regards
Thara

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH v11 5/9] drivers: thermal: tsens: Fix bug in sensor enable for msm8960
  2021-03-19  0:52 ` [PATCH v11 5/9] drivers: thermal: tsens: Fix bug in sensor enable " Ansuel Smith
  2021-03-19 13:11   ` Thara Gopinath
@ 2021-03-22 10:47   ` Marc Gonzalez
  1 sibling, 0 replies; 17+ messages in thread
From: Marc Gonzalez @ 2021-03-22 10:47 UTC (permalink / raw)
  To: Ansuel Smith, Thara Gopinath
  Cc: Bjorn Andersson, Amit Kucheria, Zhang Rui, Daniel Lezcano,
	linux-arm-msm, linux-kernel

On 19/03/2021 01:52, Ansuel Smith wrote:

> Device based on tsens VER_0 contains a hardware bug that results in some
> problem with sensor enablement. Sensor id 6-11 can't be enabled
> selectively and all of them must be enabled in one step.
> 
> Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
> ---
>  drivers/thermal/qcom/tsens-8960.c | 24 +++++++++++++++++++++---
>  1 file changed, 21 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/thermal/qcom/tsens-8960.c b/drivers/thermal/qcom/tsens-8960.c
> index 86585f439985..bdc64d4188bf 100644
> --- a/drivers/thermal/qcom/tsens-8960.c
> +++ b/drivers/thermal/qcom/tsens-8960.c
> @@ -27,9 +27,9 @@
>  #define EN			BIT(0)
>  #define SW_RST			BIT(1)
>  #define SENSOR0_EN		BIT(3)
> +#define MEASURE_PERIOD		BIT(18)
>  #define SLP_CLK_ENA		BIT(26)
>  #define SLP_CLK_ENA_8660	BIT(24)
> -#define MEASURE_PERIOD		1
>  #define SENSOR0_SHIFT		3
>  
>  /* INT_STATUS_ADDR bitmasks */
> @@ -126,17 +126,35 @@ static int resume_8960(struct tsens_priv *priv)
>  static int enable_8960(struct tsens_priv *priv, int id)
>  {
>  	int ret;
> -	u32 reg, mask;
> +	u32 reg, mask = BIT(id);
>  
>  	ret = regmap_read(priv->tm_map, CNTL_ADDR, &reg);
>  	if (ret)
>  		return ret;
>  
> -	mask = BIT(id + SENSOR0_SHIFT);
> +	/* HARDWARE BUG:
> +	 * On platform with more than 6 sensors, all the remaining
> +	 * sensors needs to be enabled all togheder or underfined

s/On platform/On platforms
s/sensors needs/sensors need
s/togheder/together
s/underfined/undefined

Suggested fix up:

On platforms with more than 6 sensors, all remaining sensors
must be enabled together, otherwise undefined results are expected.

Regards.

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2021-03-22 10:57 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-19  0:52 [PATCH v11 0/9] Add support for ipq8064 tsens Ansuel Smith
2021-03-19  0:52 ` [PATCH v11 1/9] drivers: thermal: tsens: Add VER_0 tsens version Ansuel Smith
2021-03-19 13:11   ` Thara Gopinath
2021-03-19 13:28     ` Ansuel Smith
2021-03-19 14:11       ` Thara Gopinath
2021-03-19  0:52 ` [PATCH v11 2/9] drivers: thermal: tsens: Don't hardcode sensor slope Ansuel Smith
2021-03-19  0:52 ` [PATCH v11 3/9] drivers: thermal: tsens: Convert msm8960 to reg_field Ansuel Smith
2021-03-19  0:52 ` [PATCH v11 4/9] drivers: thermal: tsens: Use init_common for msm8960 Ansuel Smith
2021-03-19  0:52 ` [PATCH v11 5/9] drivers: thermal: tsens: Fix bug in sensor enable " Ansuel Smith
2021-03-19 13:11   ` Thara Gopinath
2021-03-22 10:47   ` Marc Gonzalez
2021-03-19  0:52 ` [PATCH v11 6/9] drivers: thermal: tsens: Replace custom 8960 apis with generic apis Ansuel Smith
2021-03-19 13:12   ` Thara Gopinath
2021-03-19  0:52 ` [PATCH v11 7/9] drivers: thermal: tsens: Drop unused define for msm8960 Ansuel Smith
2021-03-19 13:12   ` Thara Gopinath
2021-03-19  0:52 ` [PATCH v11 8/9] drivers: thermal: tsens: Add support for ipq8064-tsens Ansuel Smith
2021-03-19  0:52 ` [PATCH v11 9/9] dt-bindings: thermal: tsens: Document ipq8064 bindings Ansuel Smith

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.