2019-08-27 12:15:28

by Amit Kucheria

[permalink] [raw]
Subject: [PATCH v2 00/15] thermal: qcom: tsens: Add interrupt support

Changes since v1:
- Collected reviews and acks
- Addressed Stephen's review comments (hopefully I got them all).
- Completely removed critical interrupt infrastructure from this series.
Will post that separately.
- Fixed a bug in sign-extension of temperature.
- Fixed DT bindings to use the name of the interrupt e.g. "uplow" and use
platform_get_irq_byname().

Add interrupt support to TSENS. The first 6 patches are general fixes and
cleanups to the driver before interrupt support is introduced.

This series has been developed against qcs404 and sdm845 and then tested on
msm8916 and msm8974 (Thanks Brian). Testing on msm8998 would be appreciated since I don't
have hardware handy.

Amit Kucheria (15):
drivers: thermal: tsens: Get rid of id field in tsens_sensor
drivers: thermal: tsens: Simplify code flow in tsens_probe
drivers: thermal: tsens: Add __func__ identifier to debug statements
drivers: thermal: tsens: Add debugfs support
arm: dts: msm8974: thermal: Add thermal zones for each sensor
arm64: dts: msm8916: thermal: Fixup HW ids for cpu sensors
dt: thermal: tsens: Document interrupt support in tsens driver
arm64: dts: sdm845: thermal: Add interrupt support
arm64: dts: msm8996: thermal: Add interrupt support
arm64: dts: msm8998: thermal: Add interrupt support
arm64: dts: qcs404: thermal: Add interrupt support
arm: dts: msm8974: thermal: Add interrupt support
arm64: dts: msm8916: thermal: Add interrupt support
drivers: thermal: tsens: Create function to return sign-extended
temperature
drivers: thermal: tsens: Add interrupt support

.../bindings/thermal/qcom-tsens.txt | 8 +
arch/arm/boot/dts/qcom-msm8974.dtsi | 108 +++-
arch/arm64/boot/dts/qcom/msm8916.dtsi | 26 +-
arch/arm64/boot/dts/qcom/msm8996.dtsi | 60 +-
arch/arm64/boot/dts/qcom/msm8998.dtsi | 82 +--
arch/arm64/boot/dts/qcom/qcs404.dtsi | 42 +-
arch/arm64/boot/dts/qcom/sdm845.dtsi | 88 +--
drivers/thermal/qcom/tsens-8960.c | 4 +-
drivers/thermal/qcom/tsens-common.c | 528 ++++++++++++++++--
drivers/thermal/qcom/tsens-v0_1.c | 11 +
drivers/thermal/qcom/tsens-v1.c | 29 +
drivers/thermal/qcom/tsens-v2.c | 13 +
drivers/thermal/qcom/tsens.c | 59 +-
drivers/thermal/qcom/tsens.h | 288 ++++++++--
14 files changed, 1089 insertions(+), 257 deletions(-)

--
2.17.1


2019-08-27 12:15:41

by Amit Kucheria

[permalink] [raw]
Subject: [PATCH v2 01/15] drivers: thermal: tsens: Get rid of id field in tsens_sensor

There are two fields - id and hw_id - to track what sensor an action was
to performed on. This was because the sensors connected to a TSENS IP
might not be contiguous i.e. 1, 2, 4, 5 with 3 being skipped.

This causes confusion in the code which uses hw_id sometimes and id
other times (tsens_get_temp, tsens_get_trend).

Switch to only using the hw_id field to track the physical ID of the
sensor. When we iterate through all the sensors connected to an IP
block, we use an index i to loop through the list of sensors, and then
return the actual hw_id that is registered on that index.

Signed-off-by: Amit Kucheria <[email protected]>
Reviewed-by: Stephen Boyd <[email protected]>
Reviewed-by: Daniel Lezcano <[email protected]>
---
drivers/thermal/qcom/tsens-8960.c | 4 ++--
drivers/thermal/qcom/tsens-common.c | 16 +++++++++-------
drivers/thermal/qcom/tsens.c | 11 +++++------
drivers/thermal/qcom/tsens.h | 10 ++++------
4 files changed, 20 insertions(+), 21 deletions(-)

diff --git a/drivers/thermal/qcom/tsens-8960.c b/drivers/thermal/qcom/tsens-8960.c
index 8d9b721dadb65..3e1436fda1ebd 100644
--- a/drivers/thermal/qcom/tsens-8960.c
+++ b/drivers/thermal/qcom/tsens-8960.c
@@ -243,11 +243,11 @@ static inline int code_to_mdegC(u32 adc_code, const struct tsens_sensor *s)
return adc_code * slope + offset;
}

-static int get_temp_8960(struct tsens_priv *priv, int id, int *temp)
+static int get_temp_8960(struct tsens_sensor *s, int *temp)
{
int ret;
u32 code, trdy;
- const struct tsens_sensor *s = &priv->sensor[id];
+ struct tsens_priv *priv = s->priv;
unsigned long timeout;

timeout = jiffies + usecs_to_jiffies(TIMEOUT_US);
diff --git a/drivers/thermal/qcom/tsens-common.c b/drivers/thermal/qcom/tsens-common.c
index 528df88012543..c037bdf92c663 100644
--- a/drivers/thermal/qcom/tsens-common.c
+++ b/drivers/thermal/qcom/tsens-common.c
@@ -83,11 +83,12 @@ static inline int code_to_degc(u32 adc_code, const struct tsens_sensor *s)
return degc;
}

-int get_temp_tsens_valid(struct tsens_priv *priv, int i, int *temp)
+int get_temp_tsens_valid(struct tsens_sensor *s, int *temp)
{
- struct tsens_sensor *s = &priv->sensor[i];
- u32 temp_idx = LAST_TEMP_0 + s->hw_id;
- u32 valid_idx = VALID_0 + s->hw_id;
+ 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 last_temp = 0, valid, mask;
int ret;

@@ -123,12 +124,13 @@ int get_temp_tsens_valid(struct tsens_priv *priv, int i, int *temp)
return 0;
}

-int get_temp_common(struct tsens_priv *priv, int i, int *temp)
+int get_temp_common(struct tsens_sensor *s, int *temp)
{
- struct tsens_sensor *s = &priv->sensor[i];
+ struct tsens_priv *priv = s->priv;
+ int hw_id = s->hw_id;
int last_temp = 0, ret;

- ret = regmap_field_read(priv->rf[LAST_TEMP_0 + s->hw_id], &last_temp);
+ ret = regmap_field_read(priv->rf[LAST_TEMP_0 + hw_id], &last_temp);
if (ret)
return ret;

diff --git a/drivers/thermal/qcom/tsens.c b/drivers/thermal/qcom/tsens.c
index 0627d8615c307..6ed687a6e53cd 100644
--- a/drivers/thermal/qcom/tsens.c
+++ b/drivers/thermal/qcom/tsens.c
@@ -14,19 +14,19 @@

static int tsens_get_temp(void *data, int *temp)
{
- const struct tsens_sensor *s = data;
+ struct tsens_sensor *s = data;
struct tsens_priv *priv = s->priv;

- return priv->ops->get_temp(priv, s->id, temp);
+ return priv->ops->get_temp(s, temp);
}

static int tsens_get_trend(void *data, int trip, enum thermal_trend *trend)
{
- const struct tsens_sensor *s = data;
+ struct tsens_sensor *s = data;
struct tsens_priv *priv = s->priv;

if (priv->ops->get_trend)
- return priv->ops->get_trend(priv, s->id, trend);
+ return priv->ops->get_trend(s, trend);

return -ENOTSUPP;
}
@@ -86,8 +86,7 @@ static int tsens_register(struct tsens_priv *priv)

for (i = 0; i < priv->num_sensors; i++) {
priv->sensor[i].priv = priv;
- priv->sensor[i].id = i;
- tzd = devm_thermal_zone_of_sensor_register(priv->dev, i,
+ tzd = devm_thermal_zone_of_sensor_register(priv->dev, priv->sensor[i].hw_id,
&priv->sensor[i],
&tsens_of_ops);
if (IS_ERR(tzd))
diff --git a/drivers/thermal/qcom/tsens.h b/drivers/thermal/qcom/tsens.h
index 2fd94997245bf..d022e726d0747 100644
--- a/drivers/thermal/qcom/tsens.h
+++ b/drivers/thermal/qcom/tsens.h
@@ -31,7 +31,6 @@ enum tsens_ver {
* @priv: tsens device instance that this sensor is connected to
* @tzd: pointer to the thermal zone that this sensor is in
* @offset: offset of temperature adjustment curve
- * @id: Sensor ID
* @hw_id: HW ID can be used in case of platform-specific IDs
* @slope: slope of temperature adjustment curve
* @status: 8960-specific variable to track 8960 and 8660 status register offset
@@ -40,7 +39,6 @@ struct tsens_sensor {
struct tsens_priv *priv;
struct thermal_zone_device *tzd;
int offset;
- unsigned int id;
unsigned int hw_id;
int slope;
u32 status;
@@ -61,13 +59,13 @@ struct tsens_ops {
/* mandatory callbacks */
int (*init)(struct tsens_priv *priv);
int (*calibrate)(struct tsens_priv *priv);
- int (*get_temp)(struct tsens_priv *priv, int i, int *temp);
+ int (*get_temp)(struct tsens_sensor *s, int *temp);
/* optional callbacks */
int (*enable)(struct tsens_priv *priv, int i);
void (*disable)(struct tsens_priv *priv);
int (*suspend)(struct tsens_priv *priv);
int (*resume)(struct tsens_priv *priv);
- int (*get_trend)(struct tsens_priv *priv, int i, enum thermal_trend *trend);
+ int (*get_trend)(struct tsens_sensor *s, enum thermal_trend *trend);
};

#define REG_FIELD_FOR_EACH_SENSOR11(_name, _offset, _startbit, _stopbit) \
@@ -313,8 +311,8 @@ struct tsens_priv {
char *qfprom_read(struct device *dev, const char *cname);
void compute_intercept_slope(struct tsens_priv *priv, u32 *pt1, u32 *pt2, u32 mode);
int init_common(struct tsens_priv *priv);
-int get_temp_tsens_valid(struct tsens_priv *priv, int i, int *temp);
-int get_temp_common(struct tsens_priv *priv, int i, int *temp);
+int get_temp_tsens_valid(struct tsens_sensor *s, int *temp);
+int get_temp_common(struct tsens_sensor *s, int *temp);

/* TSENS target */
extern const struct tsens_plat_data data_8960;
--
2.17.1

2019-08-27 12:15:52

by Amit Kucheria

[permalink] [raw]
Subject: [PATCH v2 02/15] drivers: thermal: tsens: Simplify code flow in tsens_probe

Move platform_set_drvdata up to avoid an extra 'if (ret)' check after
the call to tsens_register.

Signed-off-by: Amit Kucheria <[email protected]>
Reviewed-by: Stephen Boyd <[email protected]>
Reviewed-by: Daniel Lezcano <[email protected]>
---
drivers/thermal/qcom/tsens.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/thermal/qcom/tsens.c b/drivers/thermal/qcom/tsens.c
index 6ed687a6e53cd..542a7f8c3d962 100644
--- a/drivers/thermal/qcom/tsens.c
+++ b/drivers/thermal/qcom/tsens.c
@@ -149,6 +149,8 @@ static int tsens_probe(struct platform_device *pdev)
priv->feat = data->feat;
priv->fields = data->fields;

+ platform_set_drvdata(pdev, priv);
+
if (!priv->ops || !priv->ops->init || !priv->ops->get_temp)
return -EINVAL;

@@ -167,11 +169,7 @@ static int tsens_probe(struct platform_device *pdev)
}
}

- ret = tsens_register(priv);
-
- platform_set_drvdata(pdev, priv);
-
- return ret;
+ return tsens_register(priv);
}

static int tsens_remove(struct platform_device *pdev)
--
2.17.1

2019-08-27 12:15:54

by Amit Kucheria

[permalink] [raw]
Subject: [PATCH v2 03/15] drivers: thermal: tsens: Add __func__ identifier to debug statements

Printing the function name when enabling debugging makes logs easier to
read.

Signed-off-by: Amit Kucheria <[email protected]>
Reviewed-by: Stephen Boyd <[email protected]>
Reviewed-by: Daniel Lezcano <[email protected]>
---
drivers/thermal/qcom/tsens-common.c | 8 ++++----
drivers/thermal/qcom/tsens.c | 6 +++---
2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/thermal/qcom/tsens-common.c b/drivers/thermal/qcom/tsens-common.c
index c037bdf92c663..7437bfe196e50 100644
--- a/drivers/thermal/qcom/tsens-common.c
+++ b/drivers/thermal/qcom/tsens-common.c
@@ -42,8 +42,8 @@ void compute_intercept_slope(struct tsens_priv *priv, u32 *p1,

for (i = 0; i < priv->num_sensors; i++) {
dev_dbg(priv->dev,
- "sensor%d - data_point1:%#x data_point2:%#x\n",
- i, p1[i], p2[i]);
+ "%s: sensor%d - data_point1:%#x data_point2:%#x\n",
+ __func__, i, p1[i], p2[i]);

priv->sensor[i].slope = SLOPE_DEFAULT;
if (mode == TWO_PT_CALIB) {
@@ -60,7 +60,7 @@ void compute_intercept_slope(struct tsens_priv *priv, u32 *p1,
priv->sensor[i].offset = (p1[i] * SLOPE_FACTOR) -
(CAL_DEGC_PT1 *
priv->sensor[i].slope);
- dev_dbg(priv->dev, "offset:%d\n", priv->sensor[i].offset);
+ dev_dbg(priv->dev, "%s: offset:%d\n", __func__, priv->sensor[i].offset);
}
}

@@ -209,7 +209,7 @@ int __init init_common(struct tsens_priv *priv)
if (ret)
goto err_put_device;
if (!enabled) {
- dev_err(dev, "tsens device is not enabled\n");
+ dev_err(dev, "%s: device not enabled\n", __func__);
ret = -ENODEV;
goto err_put_device;
}
diff --git a/drivers/thermal/qcom/tsens.c b/drivers/thermal/qcom/tsens.c
index 542a7f8c3d962..06c6bbd69a1a7 100644
--- a/drivers/thermal/qcom/tsens.c
+++ b/drivers/thermal/qcom/tsens.c
@@ -127,7 +127,7 @@ static int tsens_probe(struct platform_device *pdev)
of_property_read_u32(np, "#qcom,sensors", &num_sensors);

if (num_sensors <= 0) {
- dev_err(dev, "invalid number of sensors\n");
+ dev_err(dev, "%s: invalid number of sensors\n", __func__);
return -EINVAL;
}

@@ -156,7 +156,7 @@ static int tsens_probe(struct platform_device *pdev)

ret = priv->ops->init(priv);
if (ret < 0) {
- dev_err(dev, "tsens init failed\n");
+ dev_err(dev, "%s: init failed\n", __func__);
return ret;
}

@@ -164,7 +164,7 @@ static int tsens_probe(struct platform_device *pdev)
ret = priv->ops->calibrate(priv);
if (ret < 0) {
if (ret != -EPROBE_DEFER)
- dev_err(dev, "tsens calibration failed\n");
+ dev_err(dev, "%s: calibration failed\n", __func__);
return ret;
}
}
--
2.17.1

2019-08-27 12:16:06

by Amit Kucheria

[permalink] [raw]
Subject: [PATCH v2 04/15] drivers: thermal: tsens: Add debugfs support

Dump some basic version info and sensor details into debugfs. Example
from qcs404 below:

--(/sys/kernel/debug) $ ls tsens/
4a9000.thermal-sensor version
--(/sys/kernel/debug) $ cat tsens/version
1.4.0
--(/sys/kernel/debug) $ cat tsens/4a9000.thermal-sensor/sensors
max: 11
num: 10

id slope offset
------------------------
0 3200 404000
1 3200 404000
2 3200 404000
3 3200 404000
4 3200 404000
5 3200 404000
6 3200 404000
7 3200 404000
8 3200 404000
9 3200 404000

Signed-off-by: Amit Kucheria <[email protected]>
---
drivers/thermal/qcom/tsens-common.c | 83 +++++++++++++++++++++++++++++
drivers/thermal/qcom/tsens.c | 2 +
drivers/thermal/qcom/tsens.h | 6 +++
3 files changed, 91 insertions(+)

diff --git a/drivers/thermal/qcom/tsens-common.c b/drivers/thermal/qcom/tsens-common.c
index 7437bfe196e50..ea2c46cc6a66a 100644
--- a/drivers/thermal/qcom/tsens-common.c
+++ b/drivers/thermal/qcom/tsens-common.c
@@ -3,6 +3,7 @@
* Copyright (c) 2015, The Linux Foundation. All rights reserved.
*/

+#include <linux/debugfs.h>
#include <linux/err.h>
#include <linux/io.h>
#include <linux/nvmem-consumer.h>
@@ -139,6 +140,77 @@ int get_temp_common(struct tsens_sensor *s, int *temp)
return 0;
}

+#ifdef CONFIG_DEBUG_FS
+static int dbg_sensors_show(struct seq_file *s, void *data)
+{
+ struct platform_device *pdev = s->private;
+ struct tsens_priv *priv = platform_get_drvdata(pdev);
+ int i;
+
+ seq_printf(s, "max: %2d\nnum: %2d\n\n",
+ priv->feat->max_sensors, priv->num_sensors);
+
+ seq_puts(s, " id slope offset\n--------------------------\n");
+ for (i = 0; i < priv->num_sensors; i++) {
+ seq_printf(s, "%8d %8d %8d\n", priv->sensor[i].hw_id,
+ priv->sensor[i].slope, priv->sensor[i].offset);
+ }
+
+ return 0;
+}
+
+static int dbg_version_show(struct seq_file *s, void *data)
+{
+ struct platform_device *pdev = s->private;
+ struct tsens_priv *priv = platform_get_drvdata(pdev);
+ u32 maj_ver, min_ver, step_ver;
+ int ret;
+
+ if (tsens_ver(priv) > VER_0_1) {
+ ret = regmap_field_read(priv->rf[VER_MAJOR], &maj_ver);
+ if (ret)
+ return ret;
+ ret = regmap_field_read(priv->rf[VER_MINOR], &min_ver);
+ if (ret)
+ return ret;
+ ret = regmap_field_read(priv->rf[VER_STEP], &step_ver);
+ if (ret)
+ return ret;
+ seq_printf(s, "%d.%d.%d\n", maj_ver, min_ver, step_ver);
+ } else {
+ seq_puts(s, "0.1.0\n");
+ }
+
+ return 0;
+}
+
+DEFINE_SHOW_ATTRIBUTE(dbg_version);
+DEFINE_SHOW_ATTRIBUTE(dbg_sensors);
+
+static void tsens_debug_init(struct platform_device *pdev)
+{
+ struct tsens_priv *priv = platform_get_drvdata(pdev);
+ struct dentry *root, *file;
+
+ root = debugfs_lookup("tsens", NULL);
+ if (!root)
+ priv->debug_root = debugfs_create_dir("tsens", NULL);
+ else
+ priv->debug_root = root;
+
+ file = debugfs_lookup("version", priv->debug_root);
+ if (!file)
+ debugfs_create_file("version", 0444, priv->debug_root,
+ pdev, &dbg_version_fops);
+
+ /* A directory for each instance of the TSENS IP */
+ priv->debug = debugfs_create_dir(dev_name(&pdev->dev), priv->debug_root);
+ debugfs_create_file("sensors", 0444, priv->debug, pdev, &dbg_sensors_fops);
+}
+#else
+static inline void tsens_debug_init(struct platform_device *pdev) {}
+#endif
+
static const struct regmap_config tsens_config = {
.name = "tm",
.reg_bits = 32,
@@ -199,6 +271,15 @@ int __init init_common(struct tsens_priv *priv)
goto err_put_device;
}

+ if (tsens_ver(priv) > VER_0_1) {
+ for (i = VER_MAJOR; i <= VER_STEP; i++) {
+ priv->rf[i] = devm_regmap_field_alloc(dev, priv->srot_map,
+ priv->fields[i]);
+ if (IS_ERR(priv->rf[i]))
+ return PTR_ERR(priv->rf[i]);
+ }
+ }
+
priv->rf[TSENS_EN] = devm_regmap_field_alloc(dev, priv->srot_map,
priv->fields[TSENS_EN]);
if (IS_ERR(priv->rf[TSENS_EN])) {
@@ -238,6 +319,8 @@ int __init init_common(struct tsens_priv *priv)
}
}

+ tsens_debug_init(op);
+
return 0;

err_put_device:
diff --git a/drivers/thermal/qcom/tsens.c b/drivers/thermal/qcom/tsens.c
index 06c6bbd69a1a7..772aa76b50e12 100644
--- a/drivers/thermal/qcom/tsens.c
+++ b/drivers/thermal/qcom/tsens.c
@@ -3,6 +3,7 @@
* Copyright (c) 2015, The Linux Foundation. All rights reserved.
*/

+#include <linux/debugfs.h>
#include <linux/err.h>
#include <linux/module.h>
#include <linux/of.h>
@@ -176,6 +177,7 @@ static int tsens_remove(struct platform_device *pdev)
{
struct tsens_priv *priv = platform_get_drvdata(pdev);

+ debugfs_remove_recursive(priv->debug_root);
if (priv->ops->disable)
priv->ops->disable(priv);

diff --git a/drivers/thermal/qcom/tsens.h b/drivers/thermal/qcom/tsens.h
index d022e726d0747..e1d6af71b2b9a 100644
--- a/drivers/thermal/qcom/tsens.h
+++ b/drivers/thermal/qcom/tsens.h
@@ -292,6 +292,8 @@ struct tsens_context {
* @feat: features of the IP
* @fields: bitfield locations
* @ops: pointer to list of callbacks supported by this device
+ * @debug_root: pointer to debugfs dentry for all tsens
+ * @debug: pointer to debugfs dentry for tsens controller
* @sensor: list of sensors attached to this device
*/
struct tsens_priv {
@@ -305,6 +307,10 @@ struct tsens_priv {
const struct tsens_features *feat;
const struct reg_field *fields;
const struct tsens_ops *ops;
+
+ struct dentry *debug_root;
+ struct dentry *debug;
+
struct tsens_sensor sensor[0];
};

--
2.17.1

2019-08-27 12:16:12

by Amit Kucheria

[permalink] [raw]
Subject: [PATCH v2 05/15] arm: dts: msm8974: thermal: Add thermal zones for each sensor

msm8974 has 11 sensors connected to a single TSENS IP. Define a thermal
zone for each of those sensors to expose the temperature of each zone.

Signed-off-by: Amit Kucheria <[email protected]>
Tested-by: Brian Masney <[email protected]>
---
arch/arm/boot/dts/qcom-msm8974.dtsi | 90 +++++++++++++++++++++++++++++
1 file changed, 90 insertions(+)

diff --git a/arch/arm/boot/dts/qcom-msm8974.dtsi b/arch/arm/boot/dts/qcom-msm8974.dtsi
index 369e58f64145d..d32f639505f1b 100644
--- a/arch/arm/boot/dts/qcom-msm8974.dtsi
+++ b/arch/arm/boot/dts/qcom-msm8974.dtsi
@@ -217,6 +217,96 @@
};
};
};
+
+ q6-dsp-thermal {
+ polling-delay-passive = <250>;
+ polling-delay = <1000>;
+
+ thermal-sensors = <&tsens 1>;
+
+ trips {
+ q6_dsp_alert0: trip-point0 {
+ temperature = <90000>;
+ hysteresis = <2000>;
+ type = "hot";
+ };
+ };
+ };
+
+ modemtx-thermal {
+ polling-delay-passive = <250>;
+ polling-delay = <1000>;
+
+ thermal-sensors = <&tsens 2>;
+
+ trips {
+ modemtx_alert0: trip-point0 {
+ temperature = <90000>;
+ hysteresis = <2000>;
+ type = "hot";
+ };
+ };
+ };
+
+ video-thermal {
+ polling-delay-passive = <0>;
+ polling-delay = <1000>;
+
+ thermal-sensors = <&tsens 3>;
+
+ trips {
+ video_alert0: trip-point0 {
+ temperature = <95000>;
+ hysteresis = <2000>;
+ type = "hot";
+ };
+ };
+ };
+
+ wlan-thermal {
+ polling-delay-passive = <0>;
+ polling-delay = <0>;
+
+ thermal-sensors = <&tsens 4>;
+
+ trips {
+ wlan_alert0: trip-point0 {
+ temperature = <105000>;
+ hysteresis = <2000>;
+ type = "hot";
+ };
+ };
+ };
+
+ gpu-thermal-top {
+ polling-delay-passive = <250>;
+ polling-delay = <1000>;
+
+ thermal-sensors = <&tsens 9>;
+
+ trips {
+ gpu1_alert0: trip-point0 {
+ temperature = <90000>;
+ hysteresis = <2000>;
+ type = "hot";
+ };
+ };
+ };
+
+ gpu-thermal-bottom {
+ polling-delay-passive = <250>;
+ polling-delay = <1000>;
+
+ thermal-sensors = <&tsens 10>;
+
+ trips {
+ gpu2_alert0: trip-point0 {
+ temperature = <90000>;
+ hysteresis = <2000>;
+ type = "hot";
+ };
+ };
+ };
};

cpu-pmu {
--
2.17.1

2019-08-27 12:16:22

by Amit Kucheria

[permalink] [raw]
Subject: [PATCH v2 06/15] arm64: dts: msm8916: thermal: Fixup HW ids for cpu sensors

msm8916 uses sensors 0, 1, 2, 4 and 5. Sensor 3 is NOT used. Fixup the
device tree so that the correct sensor ID is used and as a result we can
actually check the temperature for the cpu2_3 sensor.

Signed-off-by: Amit Kucheria <[email protected]>
Reviewed-by: Daniel Lezcano <[email protected]>
---
arch/arm64/boot/dts/qcom/msm8916.dtsi | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/msm8916.dtsi b/arch/arm64/boot/dts/qcom/msm8916.dtsi
index 5ea9fb8f2f87d..8686e101905cc 100644
--- a/arch/arm64/boot/dts/qcom/msm8916.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8916.dtsi
@@ -179,7 +179,7 @@
polling-delay-passive = <250>;
polling-delay = <1000>;

- thermal-sensors = <&tsens 4>;
+ thermal-sensors = <&tsens 5>;

trips {
cpu0_1_alert0: trip-point@0 {
@@ -209,7 +209,7 @@
polling-delay-passive = <250>;
polling-delay = <1000>;

- thermal-sensors = <&tsens 3>;
+ thermal-sensors = <&tsens 4>;

trips {
cpu2_3_alert0: trip-point@0 {
--
2.17.1

2019-08-27 12:16:39

by Amit Kucheria

[permalink] [raw]
Subject: [PATCH v2 09/15] arm64: dts: msm8996: thermal: Add interrupt support

Register upper-lower interrupts for each of the two tsens controllers.

Signed-off-by: Amit Kucheria <[email protected]>
---
arch/arm64/boot/dts/qcom/msm8996.dtsi | 60 ++++++++++++++-------------
1 file changed, 32 insertions(+), 28 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/msm8996.dtsi b/arch/arm64/boot/dts/qcom/msm8996.dtsi
index 96c0a481f454e..bb763b362c162 100644
--- a/arch/arm64/boot/dts/qcom/msm8996.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8996.dtsi
@@ -175,8 +175,8 @@

thermal-zones {
cpu0-thermal {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;

thermal-sensors = <&tsens0 3>;

@@ -196,8 +196,8 @@
};

cpu1-thermal {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;

thermal-sensors = <&tsens0 5>;

@@ -217,8 +217,8 @@
};

cpu2-thermal {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;

thermal-sensors = <&tsens0 8>;

@@ -238,8 +238,8 @@
};

cpu3-thermal {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;

thermal-sensors = <&tsens0 10>;

@@ -259,8 +259,8 @@
};

gpu-thermal-top {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;

thermal-sensors = <&tsens1 6>;

@@ -274,8 +274,8 @@
};

gpu-thermal-bottom {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;

thermal-sensors = <&tsens1 7>;

@@ -289,8 +289,8 @@
};

m4m-thermal {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;

thermal-sensors = <&tsens0 1>;

@@ -304,8 +304,8 @@
};

l3-or-venus-thermal {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;

thermal-sensors = <&tsens0 2>;

@@ -319,8 +319,8 @@
};

cluster0-l2-thermal {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;

thermal-sensors = <&tsens0 7>;

@@ -334,8 +334,8 @@
};

cluster1-l2-thermal {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;

thermal-sensors = <&tsens0 12>;

@@ -349,8 +349,8 @@
};

camera-thermal {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;

thermal-sensors = <&tsens1 1>;

@@ -364,8 +364,8 @@
};

q6-dsp-thermal {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;

thermal-sensors = <&tsens1 2>;

@@ -379,8 +379,8 @@
};

mem-thermal {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;

thermal-sensors = <&tsens1 3>;

@@ -394,8 +394,8 @@
};

modemtx-thermal {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;

thermal-sensors = <&tsens1 4>;

@@ -591,6 +591,8 @@
reg = <0x4a9000 0x1000>, /* TM */
<0x4a8000 0x1000>; /* SROT */
#qcom,sensors = <13>;
+ interrupts = <GIC_SPI 458 IRQ_TYPE_LEVEL_HIGH>;
+ interrupt-names = "uplow";
#thermal-sensor-cells = <1>;
};

@@ -599,6 +601,8 @@
reg = <0x4ad000 0x1000>, /* TM */
<0x4ac000 0x1000>; /* SROT */
#qcom,sensors = <8>;
+ interrupts = <GIC_SPI 184 IRQ_TYPE_LEVEL_HIGH>;
+ interrupt-names = "uplow";
#thermal-sensor-cells = <1>;
};

--
2.17.1

2019-08-27 12:16:41

by Amit Kucheria

[permalink] [raw]
Subject: [PATCH v2 07/15] dt: thermal: tsens: Document interrupt support in tsens driver

Define two new required properties to define interrupts and
interrupt-names for tsens.

Signed-off-by: Amit Kucheria <[email protected]>
---
Documentation/devicetree/bindings/thermal/qcom-tsens.txt | 8 ++++++++
1 file changed, 8 insertions(+)

diff --git a/Documentation/devicetree/bindings/thermal/qcom-tsens.txt b/Documentation/devicetree/bindings/thermal/qcom-tsens.txt
index 673cc1831ee9d..686bede72f846 100644
--- a/Documentation/devicetree/bindings/thermal/qcom-tsens.txt
+++ b/Documentation/devicetree/bindings/thermal/qcom-tsens.txt
@@ -22,6 +22,8 @@ Required properties:

- #thermal-sensor-cells : Should be 1. See ./thermal.txt for a description.
- #qcom,sensors: Number of sensors in tsens block
+- interrupts: Interrupts generated from Always-On subsystem (AOSS)
+- interrupt-names: Must be one of the following: "uplow", "critical"
- Refer to Documentation/devicetree/bindings/nvmem/nvmem.txt to know how to specify
nvmem cells

@@ -40,6 +42,9 @@ tsens0: thermal-sensor@c263000 {
reg = <0xc263000 0x1ff>, /* TM */
<0xc222000 0x1ff>; /* SROT */
#qcom,sensors = <13>;
+ interrupts = <GIC_SPI 506 IRQ_TYPE_LEVEL_HIGH>;
+ interrupt-names = "uplow";
+
#thermal-sensor-cells = <1>;
};

@@ -51,5 +56,8 @@ tsens: thermal-sensor@4a9000 {
nvmem-cells = <&tsens_caldata>;
nvmem-cell-names = "calib";
#qcom,sensors = <10>;
+ interrupts = <GIC_SPI 506 IRQ_TYPE_LEVEL_HIGH>;
+ interrupt-names = "uplow";
+
#thermal-sensor-cells = <1>;
};
--
2.17.1

2019-08-27 12:16:42

by Amit Kucheria

[permalink] [raw]
Subject: [PATCH v2 08/15] arm64: dts: sdm845: thermal: Add interrupt support

Register upper-lower interrupts for each of the two tsens controllers.

Signed-off-by: Amit Kucheria <[email protected]>
---
arch/arm64/boot/dts/qcom/sdm845.dtsi | 88 +++++++++++++++-------------
1 file changed, 46 insertions(+), 42 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi
index 4babff5f19b5c..fdd74c39b744e 100644
--- a/arch/arm64/boot/dts/qcom/sdm845.dtsi
+++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi
@@ -2386,6 +2386,8 @@
reg = <0 0x0c263000 0 0x1ff>, /* TM */
<0 0x0c222000 0 0x1ff>; /* SROT */
#qcom,sensors = <13>;
+ interrupts = <GIC_SPI 506 IRQ_TYPE_LEVEL_HIGH>;
+ interrupt-names = "uplow";
#thermal-sensor-cells = <1>;
};

@@ -2394,6 +2396,8 @@
reg = <0 0x0c265000 0 0x1ff>, /* TM */
<0 0x0c223000 0 0x1ff>; /* SROT */
#qcom,sensors = <8>;
+ interrupts = <GIC_SPI 507 IRQ_TYPE_LEVEL_HIGH>;
+ interrupt-names = "uplow";
#thermal-sensor-cells = <1>;
};

@@ -2712,8 +2716,8 @@

thermal-zones {
cpu0-thermal {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;

thermal-sensors = <&tsens0 1>;

@@ -2756,8 +2760,8 @@
};

cpu1-thermal {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;

thermal-sensors = <&tsens0 2>;

@@ -2800,8 +2804,8 @@
};

cpu2-thermal {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;

thermal-sensors = <&tsens0 3>;

@@ -2844,8 +2848,8 @@
};

cpu3-thermal {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;

thermal-sensors = <&tsens0 4>;

@@ -2888,8 +2892,8 @@
};

cpu4-thermal {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;

thermal-sensors = <&tsens0 7>;

@@ -2932,8 +2936,8 @@
};

cpu5-thermal {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;

thermal-sensors = <&tsens0 8>;

@@ -2976,8 +2980,8 @@
};

cpu6-thermal {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;

thermal-sensors = <&tsens0 9>;

@@ -3020,8 +3024,8 @@
};

cpu7-thermal {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;

thermal-sensors = <&tsens0 10>;

@@ -3064,8 +3068,8 @@
};

aoss0-thermal {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;

thermal-sensors = <&tsens0 0>;

@@ -3079,8 +3083,8 @@
};

cluster0-thermal {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;

thermal-sensors = <&tsens0 5>;

@@ -3099,8 +3103,8 @@
};

cluster1-thermal {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;

thermal-sensors = <&tsens0 6>;

@@ -3119,8 +3123,8 @@
};

gpu-thermal-top {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;

thermal-sensors = <&tsens0 11>;

@@ -3134,8 +3138,8 @@
};

gpu-thermal-bottom {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;

thermal-sensors = <&tsens0 12>;

@@ -3149,8 +3153,8 @@
};

aoss1-thermal {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;

thermal-sensors = <&tsens1 0>;

@@ -3164,8 +3168,8 @@
};

q6-modem-thermal {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;

thermal-sensors = <&tsens1 1>;

@@ -3179,8 +3183,8 @@
};

mem-thermal {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;

thermal-sensors = <&tsens1 2>;

@@ -3194,8 +3198,8 @@
};

wlan-thermal {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;

thermal-sensors = <&tsens1 3>;

@@ -3209,8 +3213,8 @@
};

q6-hvx-thermal {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;

thermal-sensors = <&tsens1 4>;

@@ -3224,8 +3228,8 @@
};

camera-thermal {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;

thermal-sensors = <&tsens1 5>;

@@ -3239,8 +3243,8 @@
};

video-thermal {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;

thermal-sensors = <&tsens1 6>;

@@ -3254,8 +3258,8 @@
};

modem-thermal {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;

thermal-sensors = <&tsens1 7>;

--
2.17.1

2019-08-27 12:17:01

by Amit Kucheria

[permalink] [raw]
Subject: [PATCH v2 11/15] arm64: dts: qcs404: thermal: Add interrupt support

Register upper-lower interrupt for the tsens controller.

Signed-off-by: Amit Kucheria <[email protected]>
---
arch/arm64/boot/dts/qcom/qcs404.dtsi | 42 +++++++++++++++-------------
1 file changed, 22 insertions(+), 20 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/qcs404.dtsi b/arch/arm64/boot/dts/qcom/qcs404.dtsi
index 3d0789775009c..065a60d50a07f 100644
--- a/arch/arm64/boot/dts/qcom/qcs404.dtsi
+++ b/arch/arm64/boot/dts/qcom/qcs404.dtsi
@@ -280,6 +280,8 @@
nvmem-cells = <&tsens_caldata>;
nvmem-cell-names = "calib";
#qcom,sensors = <10>;
+ interrupts = <GIC_SPI 184 IRQ_TYPE_LEVEL_HIGH>;
+ interrupt-names = "uplow";
#thermal-sensor-cells = <1>;
};

@@ -1071,8 +1073,8 @@

thermal-zones {
aoss-thermal {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;

thermal-sensors = <&tsens 0>;

@@ -1086,8 +1088,8 @@
};

q6-hvx-thermal {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;

thermal-sensors = <&tsens 1>;

@@ -1101,8 +1103,8 @@
};

lpass-thermal {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;

thermal-sensors = <&tsens 2>;

@@ -1116,8 +1118,8 @@
};

wlan-thermal {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;

thermal-sensors = <&tsens 3>;

@@ -1131,8 +1133,8 @@
};

cluster-thermal {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;

thermal-sensors = <&tsens 4>;

@@ -1165,8 +1167,8 @@
};

cpu0-thermal {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;

thermal-sensors = <&tsens 5>;

@@ -1199,8 +1201,8 @@
};

cpu1-thermal {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;

thermal-sensors = <&tsens 6>;

@@ -1233,8 +1235,8 @@
};

cpu2-thermal {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;

thermal-sensors = <&tsens 7>;

@@ -1267,8 +1269,8 @@
};

cpu3-thermal {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;

thermal-sensors = <&tsens 8>;

@@ -1301,8 +1303,8 @@
};

gpu-thermal {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;

thermal-sensors = <&tsens 9>;

--
2.17.1

2019-08-27 12:17:01

by Amit Kucheria

[permalink] [raw]
Subject: [PATCH v2 12/15] arm: dts: msm8974: thermal: Add interrupt support

Register upper-lower interrupt for the tsens controller.

Signed-off-by: Amit Kucheria <[email protected]>
Tested-by: Brian Masney <[email protected]>
---
arch/arm/boot/dts/qcom-msm8974.dtsi | 36 +++++++++++++++--------------
1 file changed, 19 insertions(+), 17 deletions(-)

diff --git a/arch/arm/boot/dts/qcom-msm8974.dtsi b/arch/arm/boot/dts/qcom-msm8974.dtsi
index d32f639505f1b..290f7c3827d45 100644
--- a/arch/arm/boot/dts/qcom-msm8974.dtsi
+++ b/arch/arm/boot/dts/qcom-msm8974.dtsi
@@ -139,8 +139,8 @@

thermal-zones {
cpu-thermal0 {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;

thermal-sensors = <&tsens 5>;

@@ -159,8 +159,8 @@
};

cpu-thermal1 {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;

thermal-sensors = <&tsens 6>;

@@ -179,8 +179,8 @@
};

cpu-thermal2 {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;

thermal-sensors = <&tsens 7>;

@@ -199,8 +199,8 @@
};

cpu-thermal3 {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;

thermal-sensors = <&tsens 8>;

@@ -219,8 +219,8 @@
};

q6-dsp-thermal {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;

thermal-sensors = <&tsens 1>;

@@ -234,8 +234,8 @@
};

modemtx-thermal {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;

thermal-sensors = <&tsens 2>;

@@ -250,7 +250,7 @@

video-thermal {
polling-delay-passive = <0>;
- polling-delay = <1000>;
+ polling-delay = <0>;

thermal-sensors = <&tsens 3>;

@@ -279,8 +279,8 @@
};

gpu-thermal-top {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;

thermal-sensors = <&tsens 9>;

@@ -294,8 +294,8 @@
};

gpu-thermal-bottom {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;

thermal-sensors = <&tsens 10>;

@@ -531,6 +531,8 @@
nvmem-cells = <&tsens_calib>, <&tsens_backup>;
nvmem-cell-names = "calib", "calib_backup";
#qcom,sensors = <11>;
+ interrupts = <GIC_SPI 184 IRQ_TYPE_LEVEL_HIGH>;
+ interrupt-names = "uplow";
#thermal-sensor-cells = <1>;
};

--
2.17.1

2019-08-27 12:17:18

by Amit Kucheria

[permalink] [raw]
Subject: [PATCH v2 15/15] drivers: thermal: tsens: Add interrupt support

Depending on the IP version, TSENS supports upper, lower, max, min and
critical threshold interrupts. We only add support for upper and lower
threshold interrupts for now.

TSENSv2 has an irq [status|clear|mask] bit tuple for each sensor while
earlier versions only have a single bit per sensor to denote status and
clear. At each interrupt, we reprogram the new upper and lower threshold
in the .set_trip callback.

Signed-off-by: Amit Kucheria <[email protected]>
---
drivers/thermal/qcom/tsens-common.c | 377 ++++++++++++++++++++++++++--
drivers/thermal/qcom/tsens-v0_1.c | 11 +
drivers/thermal/qcom/tsens-v1.c | 29 +++
drivers/thermal/qcom/tsens-v2.c | 13 +
drivers/thermal/qcom/tsens.c | 32 ++-
drivers/thermal/qcom/tsens.h | 270 ++++++++++++++++----
6 files changed, 669 insertions(+), 63 deletions(-)

diff --git a/drivers/thermal/qcom/tsens-common.c b/drivers/thermal/qcom/tsens-common.c
index 06b44cfd5eab9..c549f8e1488ba 100644
--- a/drivers/thermal/qcom/tsens-common.c
+++ b/drivers/thermal/qcom/tsens-common.c
@@ -13,6 +13,30 @@
#include <linux/regmap.h>
#include "tsens.h"

+/** struct tsens_irq_data - IRQ status and temperature violations
+ * @up_viol: upper threshold violated
+ * @up_thresh: upper threshold temperature value
+ * @up_irq_mask: mask register for upper threshold irqs
+ * @up_irq_clear: clear register for uppper threshold irqs
+ * @low_viol: lower threshold violated
+ * @low_thresh: lower threshold temperature value
+ * @low_irq_mask: mask register for lower threshold irqs
+ * @low_irq_clear: clear register for lower threshold irqs
+ *
+ * Structure containing data about temperature threshold settings and
+ * irq status if they were violated.
+ */
+struct tsens_irq_data {
+ u32 up_viol;
+ int up_thresh;
+ u32 up_irq_mask;
+ u32 up_irq_clear;
+ u32 low_viol;
+ int low_thresh;
+ u32 low_irq_mask;
+ u32 low_irq_clear;
+};
+
char *qfprom_read(struct device *dev, const char *cname)
{
struct nvmem_cell *cell;
@@ -65,6 +89,14 @@ void compute_intercept_slope(struct tsens_priv *priv, u32 *p1,
}
}

+static inline u32 degc_to_code(int degc, const struct tsens_sensor *sensor)
+{
+ u64 code = (degc * sensor->slope + sensor->offset) / SLOPE_FACTOR;
+
+ pr_debug("%s: raw_code: 0x%llx, degc:%d\n", __func__, code, degc);
+ return clamp_val(code, THRESHOLD_MIN_ADC_CODE, THRESHOLD_MAX_ADC_CODE);
+}
+
static inline int code_to_degc(u32 adc_code, const struct tsens_sensor *s)
{
int degc, num, den;
@@ -114,6 +146,314 @@ static int tsens_hw_to_mC(struct tsens_sensor *s, int field)
return sign_extend32(temp, priv->tempres) * 100;
}

+/**
+ * tsens_mC_to_hw - Return correct value to be written to threshold
+ * registers, whether in ADC code or deciCelsius depending on IP version
+ */
+static int tsens_mC_to_hw(struct tsens_sensor *s, int temp)
+{
+ struct tsens_priv *priv = s->priv;
+
+ if (priv->feat->adc) {
+ /* milliC to C to adc code */
+ return degc_to_code(temp / 1000, s);
+ }
+
+ /* milliC to deciC */
+ return temp / 100;
+}
+
+static inline unsigned int tsens_ver(struct tsens_priv *priv)
+{
+ return priv->feat->ver_major;
+}
+
+/**
+ * tsens_set_interrupt_v1 - Disable an interrupt (enable = false)
+ * Re-enable an interrupt (enable = true)
+ */
+static void tsens_set_interrupt_v1(struct tsens_priv *priv, u32 hw_id,
+ enum tsens_irq_type irq_type, bool enable)
+{
+ u32 index;
+
+ if (enable) {
+ switch (irq_type) {
+ case UPPER:
+ index = UP_INT_CLEAR_0 + hw_id;
+ break;
+ case LOWER:
+ index = LOW_INT_CLEAR_0 + hw_id;
+ break;
+ }
+ regmap_field_write(priv->rf[index], 0);
+ } else {
+ switch (irq_type) {
+ case UPPER:
+ index = UP_INT_CLEAR_0 + hw_id;
+ break;
+ case LOWER:
+ index = LOW_INT_CLEAR_0 + hw_id;
+ break;
+ }
+ regmap_field_write(priv->rf[index], 1);
+ }
+}
+
+/**
+ * tsens_set_interrupt_v2 - Disable an interrupt (enable = false)
+ * Re-enable an interrupt (enable = true)
+ */
+static void tsens_set_interrupt_v2(struct tsens_priv *priv, u32 hw_id,
+ enum tsens_irq_type irq_type, bool enable)
+{
+ u32 index_mask, index_clear;
+
+ if (enable) {
+ switch (irq_type) {
+ case UPPER:
+ index_mask = UP_INT_MASK_0 + hw_id;
+ break;
+ case LOWER:
+ index_mask = LOW_INT_MASK_0 + hw_id;
+ break;
+ }
+ regmap_field_write(priv->rf[index_mask], 0);
+ } else {
+ /* To disable the interrupt flag for a sensor:
+ * 1. Mask further interrupts for this sensor
+ * 2. Write 1 followed by 0 to clear the interrupt
+ */
+ switch (irq_type) {
+ case UPPER:
+ index_mask = UP_INT_MASK_0 + hw_id;
+ index_clear = UP_INT_CLEAR_0 + hw_id;
+ break;
+ case LOWER:
+ index_mask = LOW_INT_MASK_0 + hw_id;
+ index_clear = LOW_INT_CLEAR_0 + hw_id;
+ break;
+ }
+ regmap_field_write(priv->rf[index_mask], 1);
+ regmap_field_write(priv->rf[index_clear], 1);
+ regmap_field_write(priv->rf[index_clear], 0);
+ }
+}
+
+/**
+ * tsens_set_interrupt - Disable an interrupt (enable = false)
+ * Re-enable an interrupt (enable = true)
+ */
+static void tsens_set_interrupt(struct tsens_priv *priv, u32 hw_id,
+ enum tsens_irq_type irq_type, bool enable)
+{
+ dev_dbg(priv->dev, "[%u] %s: %s -> %s\n", hw_id, __func__,
+ irq_type ? ((irq_type == 1) ? "UP" : "CRITICAL") : "LOW",
+ enable ? "en" : "dis");
+ if (tsens_ver(priv) > VER_1_X)
+ tsens_set_interrupt_v2(priv, hw_id, irq_type, enable);
+ else
+ tsens_set_interrupt_v1(priv, hw_id, irq_type, enable);
+}
+
+/**
+ * tsens_threshold_violated - Check if a sensor temperature violated a preset threshold
+ *
+ * Return: 0 if threshold was not violated, 1 if it was violated and negative
+ * errno in case of errors
+ */
+static int tsens_threshold_violated(struct tsens_priv *priv, u32 hw_id,
+ struct tsens_irq_data *d)
+{
+ int ret;
+
+ ret = regmap_field_read(priv->rf[UPPER_STATUS_0 + hw_id], &d->up_viol);
+ if (ret)
+ return ret;
+ ret = regmap_field_read(priv->rf[LOWER_STATUS_0 + hw_id], &d->low_viol);
+ if (ret)
+ return ret;
+ if (d->up_viol || d->low_viol)
+ return 1;
+
+ return 0;
+}
+
+static int tsens_read_irq_state(struct tsens_priv *priv, u32 hw_id,
+ struct tsens_sensor *s, struct tsens_irq_data *d)
+{
+ int ret;
+
+ ret = regmap_field_read(priv->rf[UP_INT_CLEAR_0 + hw_id], &d->up_irq_clear);
+ if (ret)
+ return ret;
+ ret = regmap_field_read(priv->rf[LOW_INT_CLEAR_0 + hw_id], &d->low_irq_clear);
+ if (ret)
+ return ret;
+ if (tsens_ver(priv) > VER_1_X) {
+ ret = regmap_field_read(priv->rf[UP_INT_MASK_0 + hw_id], &d->up_irq_mask);
+ if (ret)
+ return ret;
+ ret = regmap_field_read(priv->rf[LOW_INT_MASK_0 + hw_id], &d->low_irq_mask);
+ if (ret)
+ return ret;
+ } else {
+ /* No mask register on older TSENS */
+ d->up_irq_mask = 0;
+ d->low_irq_mask = 0;
+ }
+
+ d->up_thresh = tsens_hw_to_mC(s, UP_THRESH_0 + hw_id);
+ d->low_thresh = tsens_hw_to_mC(s, LOW_THRESH_0 + hw_id);
+
+ dev_dbg(priv->dev, "[%u] %s%s: status(%u|%u) | clr(%u|%u) | mask(%u|%u)\n",
+ hw_id, __func__, (d->up_viol || d->low_viol) ? "(V)" : "",
+ d->low_viol, d->up_viol, d->low_irq_clear, d->up_irq_clear,
+ d->low_irq_mask, d->up_irq_mask);
+ dev_dbg(priv->dev, "[%u] %s%s: thresh: (%d:%d)\n", hw_id, __func__,
+ (d->up_viol || d->low_viol) ? "(violation)" : "",
+ d->low_thresh, d->up_thresh);
+
+ return 0;
+}
+
+static inline u32 masked_irq(u32 hw_id, u32 mask, enum tsens_ver ver)
+{
+ if (ver > VER_1_X)
+ return mask & (1 << hw_id);
+
+ /* v1, v0.1 don't have a irq mask register */
+ return 0;
+}
+
+irqreturn_t tsens_irq_thread(int irq, void *data)
+{
+ struct tsens_priv *priv = data;
+ struct tsens_irq_data d;
+ bool enable = true, disable = false;
+ unsigned long flags;
+ int temp, ret, i;
+
+ /*
+ * Check if any sensor raised an IRQ - for each sensor connected to the
+ * TSENS block if it set the threshold violation bit.
+ */
+ for (i = 0; i < priv->num_sensors; i++) {
+ bool trigger = 0;
+ struct tsens_sensor *s = &priv->sensor[i];
+ u32 hw_id = s->hw_id;
+
+ if (IS_ERR(priv->sensor[i].tzd))
+ continue;
+ if (!tsens_threshold_violated(priv, hw_id, &d))
+ continue;
+ ret = get_temp_tsens_valid(s, &temp);
+ if (ret) {
+ dev_err(priv->dev, "[%u] %s: error reading sensor\n", hw_id, __func__);
+ continue;
+ }
+
+ spin_lock_irqsave(&priv->ul_lock, flags);
+
+ tsens_read_irq_state(priv, hw_id, s, &d);
+
+ if (d.up_viol &&
+ !masked_irq(hw_id, d.up_irq_mask, tsens_ver(priv))) {
+ tsens_set_interrupt(priv, hw_id, UPPER, disable);
+ if (d.up_thresh > temp) {
+ dev_dbg(priv->dev, "[%u] %s: re-arm upper\n",
+ priv->sensor[i].hw_id, __func__);
+ /* unmask the interrupt for this sensor */
+ tsens_set_interrupt(priv, hw_id, UPPER, enable);
+ } else {
+ trigger = 1;
+ /* Keep irq masked */
+ }
+ } else if (d.low_viol &&
+ !masked_irq(hw_id, d.low_irq_mask, tsens_ver(priv))) {
+ tsens_set_interrupt(priv, hw_id, LOWER, disable);
+ if (d.low_thresh < temp) {
+ dev_dbg(priv->dev, "[%u] %s: re-arm low\n",
+ priv->sensor[i].hw_id, __func__);
+ /* unmask the interrupt for this sensor */
+ tsens_set_interrupt(priv, hw_id, LOWER, enable);
+ } else {
+ trigger = 1;
+ /* Keep irq masked */
+ }
+ }
+
+ spin_unlock_irqrestore(&priv->ul_lock, flags);
+
+ if (trigger) {
+ dev_dbg(priv->dev, "[%u] %s: TZ update trigger (%d mC)\n",
+ hw_id, __func__, temp);
+ thermal_zone_device_update(priv->sensor[i].tzd,
+ THERMAL_EVENT_UNSPECIFIED);
+ } else {
+ dev_dbg(priv->dev, "[%u] %s: no violation: %d\n",
+ hw_id, __func__, temp);
+ }
+ }
+
+ return IRQ_HANDLED;
+}
+
+int tsens_set_trips(void *_sensor, int low, int high)
+{
+ struct tsens_sensor *s = _sensor;
+ struct tsens_priv *priv = s->priv;
+ struct device *dev = priv->dev;
+ struct tsens_irq_data d;
+ unsigned long flags;
+ int high_val, low_val, cl_high, cl_low;
+ bool enable = true;
+ u32 hw_id = s->hw_id;
+
+ dev_dbg(dev, "[%u] %s: proposed thresholds: (%d:%d)\n",
+ hw_id, __func__, low, high);
+
+ cl_high = clamp_val(high, -40000, 120000);
+ cl_low = clamp_val(low, -40000, 120000);
+
+ high_val = tsens_mC_to_hw(s, cl_high);
+ low_val = tsens_mC_to_hw(s, cl_low);
+
+ spin_lock_irqsave(&priv->ul_lock, flags);
+
+ tsens_read_irq_state(priv, hw_id, s, &d);
+
+ /* Write the new thresholds and clear the status */
+ regmap_field_write(priv->rf[LOW_THRESH_0 + hw_id], low_val);
+ regmap_field_write(priv->rf[UP_THRESH_0 + hw_id], high_val);
+ tsens_set_interrupt(priv, hw_id, LOWER, enable);
+ tsens_set_interrupt(priv, hw_id, UPPER, enable);
+
+ spin_unlock_irqrestore(&priv->ul_lock, flags);
+
+ dev_dbg(dev, "[%u] %s: (%d:%d)->(%d:%d)\n",
+ s->hw_id, __func__, d.low_thresh, d.up_thresh, cl_low, cl_high);
+
+ return 0;
+}
+
+int tsens_enable_irq(struct tsens_priv *priv)
+{
+ int ret;
+ int val = tsens_ver(priv) > VER_1_X ? 7 : 1;
+
+ ret = regmap_field_write(priv->rf[INT_EN], val);
+ if (ret < 0)
+ dev_err(priv->dev, "%s: failed to enable interrupts\n", __func__);
+
+ return ret;
+}
+
+void tsens_disable_irq(struct tsens_priv *priv)
+{
+ regmap_field_write(priv->rf[INT_EN], 0);
+}
+
int get_temp_tsens_valid(struct tsens_sensor *s, int *temp)
{
struct tsens_priv *priv = s->priv;
@@ -319,28 +659,31 @@ int __init init_common(struct tsens_priv *priv)
ret = PTR_ERR(priv->rf[SENSOR_EN]);
goto err_put_device;
}
- /* now alloc regmap_fields in tm_map */
- for (i = 0, j = LAST_TEMP_0; i < priv->feat->max_sensors; i++, j++) {
- priv->rf[j] = devm_regmap_field_alloc(dev, priv->tm_map,
- priv->fields[j]);
- if (IS_ERR(priv->rf[j])) {
- ret = PTR_ERR(priv->rf[j]);
- goto err_put_device;
- }
+ priv->rf[INT_EN] = devm_regmap_field_alloc(dev, priv->tm_map,
+ priv->fields[INT_EN]);
+ if (IS_ERR(priv->rf[INT_EN])) {
+ ret = PTR_ERR(priv->rf[INT_EN]);
+ goto err_put_device;
}

- /* Save away resolution of signed temperature value for this IP */
- priv->tempres = priv->fields[LAST_TEMP_0].msb - priv->fields[LAST_TEMP_0].lsb;
-
- for (i = 0, j = VALID_0; i < priv->feat->max_sensors; i++, j++) {
- priv->rf[j] = devm_regmap_field_alloc(dev, priv->tm_map,
- priv->fields[j]);
- if (IS_ERR(priv->rf[j])) {
- ret = PTR_ERR(priv->rf[j]);
- 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++) {
+ int idx = j + i;
+
+ priv->rf[idx] = devm_regmap_field_alloc(dev, priv->tm_map,
+ priv->fields[idx]);
+ if (IS_ERR(priv->rf[idx])) {
+ ret = PTR_ERR(priv->rf[idx]);
+ goto err_put_device;
+ }
}
}
+ /* Save away resolution of signed temperature value for this IP */
+ priv->tempres = priv->fields[LAST_TEMP_0].msb - priv->fields[LAST_TEMP_0].lsb;

+ spin_lock_init(&priv->ul_lock);
+ tsens_enable_irq(priv);
tsens_debug_init(op);

return 0;
diff --git a/drivers/thermal/qcom/tsens-v0_1.c b/drivers/thermal/qcom/tsens-v0_1.c
index 6f26fadf4c279..a267b66e61d6b 100644
--- a/drivers/thermal/qcom/tsens-v0_1.c
+++ b/drivers/thermal/qcom/tsens-v0_1.c
@@ -339,9 +339,20 @@ static const struct reg_field tsens_v0_1_regfields[MAX_REGFIELDS] = {
/* INTERRUPT ENABLE */
[INT_EN] = REG_FIELD(TM_INT_EN_OFF, 0, 0),

+ /* UPPER/LOWER TEMPERATURE THRESHOLDS */
+ REG_FIELD_FOR_EACH_SENSOR11(LOW_THRESH, TM_Sn_UPPER_LOWER_STATUS_CTRL_OFF, 0, 9),
+ REG_FIELD_FOR_EACH_SENSOR11(UP_THRESH, TM_Sn_UPPER_LOWER_STATUS_CTRL_OFF, 10, 19),
+
+ /* UPPER/LOWER INTERRUPTS [CLEAR/STATUS] */
+ REG_FIELD_FOR_EACH_SENSOR11(LOW_INT_CLEAR, TM_Sn_UPPER_LOWER_STATUS_CTRL_OFF, 20, 20),
+ REG_FIELD_FOR_EACH_SENSOR11(UP_INT_CLEAR, TM_Sn_UPPER_LOWER_STATUS_CTRL_OFF, 21, 21),
+
+ /* NO CRITICAL INTERRUPT SUPPORT on v0.1 */
+
/* Sn_STATUS */
REG_FIELD_FOR_EACH_SENSOR11(LAST_TEMP, TM_Sn_STATUS_OFF, 0, 9),
/* No VALID field on v0.1 */
+ /* xxx_STATUS bits: 1 == threshold violated */
REG_FIELD_FOR_EACH_SENSOR11(MIN_STATUS, TM_Sn_STATUS_OFF, 10, 10),
REG_FIELD_FOR_EACH_SENSOR11(LOWER_STATUS, TM_Sn_STATUS_OFF, 11, 11),
REG_FIELD_FOR_EACH_SENSOR11(UPPER_STATUS, TM_Sn_STATUS_OFF, 12, 12),
diff --git a/drivers/thermal/qcom/tsens-v1.c b/drivers/thermal/qcom/tsens-v1.c
index 10b595d4f6199..86259c9821be8 100644
--- a/drivers/thermal/qcom/tsens-v1.c
+++ b/drivers/thermal/qcom/tsens-v1.c
@@ -17,6 +17,8 @@
#define TM_Sn_UPPER_LOWER_STATUS_CTRL_OFF 0x0004
#define TM_Sn_STATUS_OFF 0x0044
#define TM_TRDY_OFF 0x0084
+#define TM_HIGH_LOW_INT_STATUS_OFF 0x0088
+#define TM_HIGH_LOW_Sn_INT_THRESHOLD_OFF 0x0090

/* eeprom layout data for qcs404/405 (v1) */
#define BASE0_MASK 0x000007f8
@@ -167,9 +169,36 @@ static const struct reg_field tsens_v1_regfields[MAX_REGFIELDS] = {
/* INTERRUPT ENABLE */
[INT_EN] = REG_FIELD(TM_INT_EN_OFF, 0, 0),

+ /* UPPER/LOWER TEMPERATURE THRESHOLDS */
+ REG_FIELD_FOR_EACH_SENSOR11(LOW_THRESH, TM_Sn_UPPER_LOWER_STATUS_CTRL_OFF, 0, 9),
+ REG_FIELD_FOR_EACH_SENSOR11(UP_THRESH, TM_Sn_UPPER_LOWER_STATUS_CTRL_OFF, 10, 19),
+
+ /* UPPER/LOWER INTERRUPTS [CLEAR/STATUS] */
+ REG_FIELD_FOR_EACH_SENSOR11(LOW_INT_CLEAR, TM_Sn_UPPER_LOWER_STATUS_CTRL_OFF, 20, 20),
+ REG_FIELD_FOR_EACH_SENSOR11(UP_INT_CLEAR, TM_Sn_UPPER_LOWER_STATUS_CTRL_OFF, 21, 21),
+ [LOW_INT_STATUS_0] = REG_FIELD(TM_HIGH_LOW_INT_STATUS_OFF, 0, 0),
+ [LOW_INT_STATUS_1] = REG_FIELD(TM_HIGH_LOW_INT_STATUS_OFF, 1, 1),
+ [LOW_INT_STATUS_2] = REG_FIELD(TM_HIGH_LOW_INT_STATUS_OFF, 2, 2),
+ [LOW_INT_STATUS_3] = REG_FIELD(TM_HIGH_LOW_INT_STATUS_OFF, 3, 3),
+ [LOW_INT_STATUS_4] = REG_FIELD(TM_HIGH_LOW_INT_STATUS_OFF, 4, 4),
+ [LOW_INT_STATUS_5] = REG_FIELD(TM_HIGH_LOW_INT_STATUS_OFF, 5, 5),
+ [LOW_INT_STATUS_6] = REG_FIELD(TM_HIGH_LOW_INT_STATUS_OFF, 6, 6),
+ [LOW_INT_STATUS_7] = REG_FIELD(TM_HIGH_LOW_INT_STATUS_OFF, 7, 7),
+ [UP_INT_STATUS_0] = REG_FIELD(TM_HIGH_LOW_INT_STATUS_OFF, 8, 8),
+ [UP_INT_STATUS_1] = REG_FIELD(TM_HIGH_LOW_INT_STATUS_OFF, 9, 9),
+ [UP_INT_STATUS_2] = REG_FIELD(TM_HIGH_LOW_INT_STATUS_OFF, 10, 10),
+ [UP_INT_STATUS_3] = REG_FIELD(TM_HIGH_LOW_INT_STATUS_OFF, 11, 11),
+ [UP_INT_STATUS_4] = REG_FIELD(TM_HIGH_LOW_INT_STATUS_OFF, 12, 12),
+ [UP_INT_STATUS_5] = REG_FIELD(TM_HIGH_LOW_INT_STATUS_OFF, 13, 13),
+ [UP_INT_STATUS_6] = REG_FIELD(TM_HIGH_LOW_INT_STATUS_OFF, 14, 14),
+ [UP_INT_STATUS_7] = REG_FIELD(TM_HIGH_LOW_INT_STATUS_OFF, 15, 15),
+
+ /* NO CRITICAL INTERRUPT SUPPORT on v1 */
+
/* Sn_STATUS */
REG_FIELD_FOR_EACH_SENSOR11(LAST_TEMP, TM_Sn_STATUS_OFF, 0, 9),
REG_FIELD_FOR_EACH_SENSOR11(VALID, TM_Sn_STATUS_OFF, 14, 14),
+ /* xxx_STATUS bits: 1 == threshold violated */
REG_FIELD_FOR_EACH_SENSOR11(MIN_STATUS, TM_Sn_STATUS_OFF, 10, 10),
REG_FIELD_FOR_EACH_SENSOR11(LOWER_STATUS, TM_Sn_STATUS_OFF, 11, 11),
REG_FIELD_FOR_EACH_SENSOR11(UPPER_STATUS, TM_Sn_STATUS_OFF, 12, 12),
diff --git a/drivers/thermal/qcom/tsens-v2.c b/drivers/thermal/qcom/tsens-v2.c
index 0a4f2b8fcab6c..a4d15e1abfddd 100644
--- a/drivers/thermal/qcom/tsens-v2.c
+++ b/drivers/thermal/qcom/tsens-v2.c
@@ -50,9 +50,22 @@ static const struct reg_field tsens_v2_regfields[MAX_REGFIELDS] = {
/* v2 has separate enables for UPPER/LOWER/CRITICAL interrupts */
[INT_EN] = REG_FIELD(TM_INT_EN_OFF, 0, 2),

+ /* TEMPERATURE THRESHOLDS */
+ REG_FIELD_FOR_EACH_SENSOR16(LOW_THRESH, TM_Sn_UPPER_LOWER_THRESHOLD_OFF, 0, 11),
+ REG_FIELD_FOR_EACH_SENSOR16(UP_THRESH, TM_Sn_UPPER_LOWER_THRESHOLD_OFF, 12, 23),
+
+ /* INTERRUPTS [CLEAR/STATUS/MASK] */
+ REG_FIELD_SPLIT_BITS_0_15(LOW_INT_STATUS, TM_UPPER_LOWER_INT_STATUS_OFF),
+ REG_FIELD_SPLIT_BITS_0_15(LOW_INT_CLEAR, TM_UPPER_LOWER_INT_CLEAR_OFF),
+ REG_FIELD_SPLIT_BITS_0_15(LOW_INT_MASK, TM_UPPER_LOWER_INT_MASK_OFF),
+ REG_FIELD_SPLIT_BITS_16_31(UP_INT_STATUS, TM_UPPER_LOWER_INT_STATUS_OFF),
+ REG_FIELD_SPLIT_BITS_16_31(UP_INT_CLEAR, TM_UPPER_LOWER_INT_CLEAR_OFF),
+ REG_FIELD_SPLIT_BITS_16_31(UP_INT_MASK, TM_UPPER_LOWER_INT_MASK_OFF),
+
/* Sn_STATUS */
REG_FIELD_FOR_EACH_SENSOR16(LAST_TEMP, TM_Sn_STATUS_OFF, 0, 11),
REG_FIELD_FOR_EACH_SENSOR16(VALID, TM_Sn_STATUS_OFF, 21, 21),
+ /* xxx_STATUS bits: 1 == threshold violated */
REG_FIELD_FOR_EACH_SENSOR16(MIN_STATUS, TM_Sn_STATUS_OFF, 16, 16),
REG_FIELD_FOR_EACH_SENSOR16(LOWER_STATUS, TM_Sn_STATUS_OFF, 17, 17),
REG_FIELD_FOR_EACH_SENSOR16(UPPER_STATUS, TM_Sn_STATUS_OFF, 18, 18),
diff --git a/drivers/thermal/qcom/tsens.c b/drivers/thermal/qcom/tsens.c
index 772aa76b50e12..a4335717aeede 100644
--- a/drivers/thermal/qcom/tsens.c
+++ b/drivers/thermal/qcom/tsens.c
@@ -7,6 +7,7 @@
#include <linux/err.h>
#include <linux/module.h>
#include <linux/of.h>
+#include <linux/of_platform.h>
#include <linux/platform_device.h>
#include <linux/pm.h>
#include <linux/slab.h>
@@ -78,12 +79,14 @@ MODULE_DEVICE_TABLE(of, tsens_table);
static const struct thermal_zone_of_device_ops tsens_of_ops = {
.get_temp = tsens_get_temp,
.get_trend = tsens_get_trend,
+ .set_trips = tsens_set_trips,
};

static int tsens_register(struct tsens_priv *priv)
{
- int i;
+ int i, ret, irq;
struct thermal_zone_device *tzd;
+ struct platform_device *pdev;

for (i = 0; i < priv->num_sensors; i++) {
priv->sensor[i].priv = priv;
@@ -96,7 +99,33 @@ static int tsens_register(struct tsens_priv *priv)
if (priv->ops->enable)
priv->ops->enable(priv, i);
}
+
+ pdev = of_find_device_by_node(priv->dev->of_node);
+ if (!pdev) {
+ dev_err(&pdev->dev, "%s: device node not found in DT\n", __func__);
+ return -ENODEV;
+ }
+
+ irq = platform_get_irq_byname(pdev, "uplow");
+ if (irq < 0) {
+ dev_err(&pdev->dev, "%s: missing irq in dt: uplow\n", __func__);
+ return irq;
+ }
+
+ ret = devm_request_threaded_irq(&pdev->dev, irq,
+ NULL, tsens_irq_thread,
+ IRQF_TRIGGER_HIGH | IRQF_ONESHOT,
+ dev_name(&pdev->dev), priv);
+ if (ret) {
+ dev_err(&pdev->dev, "%s: failed to get irq\n", __func__);
+ goto err_put_device;
+ }
+ enable_irq_wake(irq);
return 0;
+
+err_put_device:
+ put_device(&pdev->dev);
+ return ret;
}

static int tsens_probe(struct platform_device *pdev)
@@ -178,6 +207,7 @@ static int tsens_remove(struct platform_device *pdev)
struct tsens_priv *priv = platform_get_drvdata(pdev);

debugfs_remove_recursive(priv->debug_root);
+ tsens_disable_irq(priv);
if (priv->ops->disable)
priv->ops->disable(priv);

diff --git a/drivers/thermal/qcom/tsens.h b/drivers/thermal/qcom/tsens.h
index 409395e436f32..b960c77168347 100644
--- a/drivers/thermal/qcom/tsens.h
+++ b/drivers/thermal/qcom/tsens.h
@@ -13,8 +13,10 @@
#define CAL_DEGC_PT2 120
#define SLOPE_FACTOR 1000
#define SLOPE_DEFAULT 3200
+#define THRESHOLD_MAX_ADC_CODE 0x3ff
+#define THRESHOLD_MIN_ADC_CODE 0x0

-
+#include <linux/interrupt.h>
#include <linux/thermal.h>
#include <linux/regmap.h>

@@ -26,6 +28,11 @@ enum tsens_ver {
VER_2_X,
};

+enum tsens_irq_type {
+ LOWER,
+ UPPER,
+};
+
/**
* struct tsens_sensor - data for each sensor connected to the tsens device
* @priv: tsens device instance that this sensor is connected to
@@ -99,22 +106,66 @@ struct tsens_ops {
[_name##_##14] = REG_FIELD(_offset + 56, _startbit, _stopbit), \
[_name##_##15] = REG_FIELD(_offset + 60, _startbit, _stopbit)

-/* reg_field IDs to use as an index into an array */
+#define REG_FIELD_SPLIT_BITS_0_15(_name, _offset) \
+ [_name##_##0] = REG_FIELD(_offset, 0, 0), \
+ [_name##_##1] = REG_FIELD(_offset, 1, 1), \
+ [_name##_##2] = REG_FIELD(_offset, 2, 2), \
+ [_name##_##3] = REG_FIELD(_offset, 3, 3), \
+ [_name##_##4] = REG_FIELD(_offset, 4, 4), \
+ [_name##_##5] = REG_FIELD(_offset, 5, 5), \
+ [_name##_##6] = REG_FIELD(_offset, 6, 6), \
+ [_name##_##7] = REG_FIELD(_offset, 7, 7), \
+ [_name##_##8] = REG_FIELD(_offset, 8, 8), \
+ [_name##_##9] = REG_FIELD(_offset, 9, 9), \
+ [_name##_##10] = REG_FIELD(_offset, 10, 10), \
+ [_name##_##11] = REG_FIELD(_offset, 11, 11), \
+ [_name##_##12] = REG_FIELD(_offset, 12, 12), \
+ [_name##_##13] = REG_FIELD(_offset, 13, 13), \
+ [_name##_##14] = REG_FIELD(_offset, 14, 14), \
+ [_name##_##15] = REG_FIELD(_offset, 15, 15)
+
+#define REG_FIELD_SPLIT_BITS_16_31(_name, _offset) \
+ [_name##_##0] = REG_FIELD(_offset, 16, 16), \
+ [_name##_##1] = REG_FIELD(_offset, 17, 17), \
+ [_name##_##2] = REG_FIELD(_offset, 18, 18), \
+ [_name##_##3] = REG_FIELD(_offset, 19, 19), \
+ [_name##_##4] = REG_FIELD(_offset, 20, 20), \
+ [_name##_##5] = REG_FIELD(_offset, 21, 21), \
+ [_name##_##6] = REG_FIELD(_offset, 22, 22), \
+ [_name##_##7] = REG_FIELD(_offset, 23, 23), \
+ [_name##_##8] = REG_FIELD(_offset, 24, 24), \
+ [_name##_##9] = REG_FIELD(_offset, 25, 25), \
+ [_name##_##10] = REG_FIELD(_offset, 26, 26), \
+ [_name##_##11] = REG_FIELD(_offset, 27, 27), \
+ [_name##_##12] = REG_FIELD(_offset, 28, 28), \
+ [_name##_##13] = REG_FIELD(_offset, 29, 29), \
+ [_name##_##14] = REG_FIELD(_offset, 30, 30), \
+ [_name##_##15] = REG_FIELD(_offset, 31, 31)
+
+/*
+ * reg_field IDs to use as an index into an array
+ * If you change the order of the entries, check the devm_regmap_field_alloc()
+ * calls in init_common()
+ */
enum regfield_ids {
/* ----- SROT ------ */
/* HW_VER */
- VER_MAJOR = 0,
+ VER_MAJOR,
VER_MINOR,
VER_STEP,
/* CTRL_OFFSET */
- TSENS_EN = 3,
+ TSENS_EN,
TSENS_SW_RST,
SENSOR_EN,
CODE_OR_TEMP,

/* ----- TM ------ */
+ /* TRDY */
+ TRDY,
+ /* INTERRUPT ENABLE */
+ INT_EN, /* v2+ has separate enables for crit, upper and lower irq */
/* STATUS */
- LAST_TEMP_0 = 7, /* Last temperature reading */
+ LAST_TEMP_0, /* Last temperature reading */
LAST_TEMP_1,
LAST_TEMP_2,
LAST_TEMP_3,
@@ -130,7 +181,7 @@ enum regfield_ids {
LAST_TEMP_13,
LAST_TEMP_14,
LAST_TEMP_15,
- VALID_0 = 23, /* VALID reading or not */
+ VALID_0, /* VALID reading or not */
VALID_1,
VALID_2,
VALID_3,
@@ -146,38 +197,6 @@ enum regfield_ids {
VALID_13,
VALID_14,
VALID_15,
- MIN_STATUS_0, /* MIN threshold violated */
- MIN_STATUS_1,
- MIN_STATUS_2,
- MIN_STATUS_3,
- MIN_STATUS_4,
- MIN_STATUS_5,
- MIN_STATUS_6,
- MIN_STATUS_7,
- MIN_STATUS_8,
- MIN_STATUS_9,
- MIN_STATUS_10,
- MIN_STATUS_11,
- MIN_STATUS_12,
- MIN_STATUS_13,
- MIN_STATUS_14,
- MIN_STATUS_15,
- MAX_STATUS_0, /* MAX threshold violated */
- MAX_STATUS_1,
- MAX_STATUS_2,
- MAX_STATUS_3,
- MAX_STATUS_4,
- MAX_STATUS_5,
- MAX_STATUS_6,
- MAX_STATUS_7,
- MAX_STATUS_8,
- MAX_STATUS_9,
- MAX_STATUS_10,
- MAX_STATUS_11,
- MAX_STATUS_12,
- MAX_STATUS_13,
- MAX_STATUS_14,
- MAX_STATUS_15,
LOWER_STATUS_0, /* LOWER threshold violated */
LOWER_STATUS_1,
LOWER_STATUS_2,
@@ -194,6 +213,70 @@ enum regfield_ids {
LOWER_STATUS_13,
LOWER_STATUS_14,
LOWER_STATUS_15,
+ LOW_INT_STATUS_0, /* LOWER interrupt status */
+ LOW_INT_STATUS_1,
+ LOW_INT_STATUS_2,
+ LOW_INT_STATUS_3,
+ LOW_INT_STATUS_4,
+ LOW_INT_STATUS_5,
+ LOW_INT_STATUS_6,
+ LOW_INT_STATUS_7,
+ LOW_INT_STATUS_8,
+ LOW_INT_STATUS_9,
+ LOW_INT_STATUS_10,
+ LOW_INT_STATUS_11,
+ LOW_INT_STATUS_12,
+ LOW_INT_STATUS_13,
+ LOW_INT_STATUS_14,
+ LOW_INT_STATUS_15,
+ LOW_INT_CLEAR_0, /* LOWER interrupt clear */
+ LOW_INT_CLEAR_1,
+ LOW_INT_CLEAR_2,
+ LOW_INT_CLEAR_3,
+ LOW_INT_CLEAR_4,
+ LOW_INT_CLEAR_5,
+ LOW_INT_CLEAR_6,
+ LOW_INT_CLEAR_7,
+ LOW_INT_CLEAR_8,
+ LOW_INT_CLEAR_9,
+ LOW_INT_CLEAR_10,
+ LOW_INT_CLEAR_11,
+ LOW_INT_CLEAR_12,
+ LOW_INT_CLEAR_13,
+ LOW_INT_CLEAR_14,
+ LOW_INT_CLEAR_15,
+ LOW_INT_MASK_0, /* LOWER interrupt mask */
+ LOW_INT_MASK_1,
+ LOW_INT_MASK_2,
+ LOW_INT_MASK_3,
+ LOW_INT_MASK_4,
+ LOW_INT_MASK_5,
+ LOW_INT_MASK_6,
+ LOW_INT_MASK_7,
+ LOW_INT_MASK_8,
+ LOW_INT_MASK_9,
+ LOW_INT_MASK_10,
+ LOW_INT_MASK_11,
+ LOW_INT_MASK_12,
+ LOW_INT_MASK_13,
+ LOW_INT_MASK_14,
+ LOW_INT_MASK_15,
+ LOW_THRESH_0, /* LOWER threshold values */
+ LOW_THRESH_1,
+ LOW_THRESH_2,
+ LOW_THRESH_3,
+ LOW_THRESH_4,
+ LOW_THRESH_5,
+ LOW_THRESH_6,
+ LOW_THRESH_7,
+ LOW_THRESH_8,
+ LOW_THRESH_9,
+ LOW_THRESH_10,
+ LOW_THRESH_11,
+ LOW_THRESH_12,
+ LOW_THRESH_13,
+ LOW_THRESH_14,
+ LOW_THRESH_15,
UPPER_STATUS_0, /* UPPER threshold violated */
UPPER_STATUS_1,
UPPER_STATUS_2,
@@ -210,6 +293,70 @@ enum regfield_ids {
UPPER_STATUS_13,
UPPER_STATUS_14,
UPPER_STATUS_15,
+ UP_INT_STATUS_0, /* UPPER interrupt status */
+ UP_INT_STATUS_1,
+ UP_INT_STATUS_2,
+ UP_INT_STATUS_3,
+ UP_INT_STATUS_4,
+ UP_INT_STATUS_5,
+ UP_INT_STATUS_6,
+ UP_INT_STATUS_7,
+ UP_INT_STATUS_8,
+ UP_INT_STATUS_9,
+ UP_INT_STATUS_10,
+ UP_INT_STATUS_11,
+ UP_INT_STATUS_12,
+ UP_INT_STATUS_13,
+ UP_INT_STATUS_14,
+ UP_INT_STATUS_15,
+ UP_INT_CLEAR_0, /* UPPER interrupt clear */
+ UP_INT_CLEAR_1,
+ UP_INT_CLEAR_2,
+ UP_INT_CLEAR_3,
+ UP_INT_CLEAR_4,
+ UP_INT_CLEAR_5,
+ UP_INT_CLEAR_6,
+ UP_INT_CLEAR_7,
+ UP_INT_CLEAR_8,
+ UP_INT_CLEAR_9,
+ UP_INT_CLEAR_10,
+ UP_INT_CLEAR_11,
+ UP_INT_CLEAR_12,
+ UP_INT_CLEAR_13,
+ UP_INT_CLEAR_14,
+ UP_INT_CLEAR_15,
+ UP_INT_MASK_0, /* UPPER interrupt mask */
+ UP_INT_MASK_1,
+ UP_INT_MASK_2,
+ UP_INT_MASK_3,
+ UP_INT_MASK_4,
+ UP_INT_MASK_5,
+ UP_INT_MASK_6,
+ UP_INT_MASK_7,
+ UP_INT_MASK_8,
+ UP_INT_MASK_9,
+ UP_INT_MASK_10,
+ UP_INT_MASK_11,
+ UP_INT_MASK_12,
+ UP_INT_MASK_13,
+ UP_INT_MASK_14,
+ UP_INT_MASK_15,
+ UP_THRESH_0, /* UPPER threshold values */
+ UP_THRESH_1,
+ UP_THRESH_2,
+ UP_THRESH_3,
+ UP_THRESH_4,
+ UP_THRESH_5,
+ UP_THRESH_6,
+ UP_THRESH_7,
+ UP_THRESH_8,
+ UP_THRESH_9,
+ UP_THRESH_10,
+ UP_THRESH_11,
+ UP_THRESH_12,
+ UP_THRESH_13,
+ UP_THRESH_14,
+ UP_THRESH_15,
CRITICAL_STATUS_0, /* CRITICAL threshold violated */
CRITICAL_STATUS_1,
CRITICAL_STATUS_2,
@@ -226,13 +373,38 @@ enum regfield_ids {
CRITICAL_STATUS_13,
CRITICAL_STATUS_14,
CRITICAL_STATUS_15,
- /* TRDY */
- TRDY,
- /* INTERRUPT ENABLE */
- INT_EN, /* Pre-V1, V1.x */
- LOW_INT_EN, /* V2.x */
- UP_INT_EN, /* V2.x */
- CRIT_INT_EN, /* V2.x */
+ MIN_STATUS_0, /* MIN threshold violated */
+ MIN_STATUS_1,
+ MIN_STATUS_2,
+ MIN_STATUS_3,
+ MIN_STATUS_4,
+ MIN_STATUS_5,
+ MIN_STATUS_6,
+ MIN_STATUS_7,
+ MIN_STATUS_8,
+ MIN_STATUS_9,
+ MIN_STATUS_10,
+ MIN_STATUS_11,
+ MIN_STATUS_12,
+ MIN_STATUS_13,
+ MIN_STATUS_14,
+ MIN_STATUS_15,
+ MAX_STATUS_0, /* MAX threshold violated */
+ MAX_STATUS_1,
+ MAX_STATUS_2,
+ MAX_STATUS_3,
+ MAX_STATUS_4,
+ MAX_STATUS_5,
+ MAX_STATUS_6,
+ MAX_STATUS_7,
+ MAX_STATUS_8,
+ MAX_STATUS_9,
+ MAX_STATUS_10,
+ MAX_STATUS_11,
+ MAX_STATUS_12,
+ MAX_STATUS_13,
+ MAX_STATUS_14,
+ MAX_STATUS_15,

/* Keep last */
MAX_REGFIELDS
@@ -304,6 +476,10 @@ struct tsens_priv {
struct regmap *tm_map;
struct regmap *srot_map;
u32 tm_offset;
+
+ /* lock for upper/lower threshold interrupts */
+ spinlock_t ul_lock;
+
struct regmap_field *rf[MAX_REGFIELDS];
struct tsens_context ctx;
const struct tsens_features *feat;
@@ -321,6 +497,10 @@ void compute_intercept_slope(struct tsens_priv *priv, u32 *pt1, u32 *pt2, u32 mo
int init_common(struct tsens_priv *priv);
int get_temp_tsens_valid(struct tsens_sensor *s, int *temp);
int get_temp_common(struct tsens_sensor *s, int *temp);
+int tsens_enable_irq(struct tsens_priv *priv);
+void tsens_disable_irq(struct tsens_priv *priv);
+int tsens_set_trips(void *_sensor, int low, int high);
+irqreturn_t tsens_irq_thread(int irq, void *data);

/* TSENS target */
extern const struct tsens_plat_data data_8960;
--
2.17.1

2019-08-27 12:17:59

by Amit Kucheria

[permalink] [raw]
Subject: [PATCH v2 10/15] arm64: dts: msm8998: thermal: Add interrupt support

Register upper-lower interrupts for each of the two tsens controllers.

Signed-off-by: Amit Kucheria <[email protected]>
---
arch/arm64/boot/dts/qcom/msm8998.dtsi | 82 ++++++++++++++-------------
1 file changed, 42 insertions(+), 40 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/msm8998.dtsi b/arch/arm64/boot/dts/qcom/msm8998.dtsi
index c13ed7aeb1e0c..1e2f77b38f2c1 100644
--- a/arch/arm64/boot/dts/qcom/msm8998.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8998.dtsi
@@ -440,8 +440,8 @@

thermal-zones {
cpu0-thermal {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;

thermal-sensors = <&tsens0 1>;

@@ -461,8 +461,8 @@
};

cpu1-thermal {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;

thermal-sensors = <&tsens0 2>;

@@ -482,8 +482,8 @@
};

cpu2-thermal {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;

thermal-sensors = <&tsens0 3>;

@@ -503,8 +503,8 @@
};

cpu3-thermal {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;

thermal-sensors = <&tsens0 4>;

@@ -524,8 +524,8 @@
};

cpu4-thermal {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;

thermal-sensors = <&tsens0 7>;

@@ -545,8 +545,8 @@
};

cpu5-thermal {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;

thermal-sensors = <&tsens0 8>;

@@ -566,8 +566,8 @@
};

cpu6-thermal {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;

thermal-sensors = <&tsens0 9>;

@@ -587,8 +587,8 @@
};

cpu7-thermal {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;

thermal-sensors = <&tsens0 10>;

@@ -608,8 +608,8 @@
};

gpu-thermal-bottom {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;

thermal-sensors = <&tsens0 12>;

@@ -623,8 +623,8 @@
};

gpu-thermal-top {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;

thermal-sensors = <&tsens0 13>;

@@ -638,8 +638,8 @@
};

clust0-mhm-thermal {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;

thermal-sensors = <&tsens0 5>;

@@ -653,8 +653,8 @@
};

clust1-mhm-thermal {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;

thermal-sensors = <&tsens0 6>;

@@ -668,8 +668,8 @@
};

cluster1-l2-thermal {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;

thermal-sensors = <&tsens0 11>;

@@ -683,8 +683,8 @@
};

modem-thermal {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;

thermal-sensors = <&tsens1 1>;

@@ -698,8 +698,8 @@
};

mem-thermal {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;

thermal-sensors = <&tsens1 2>;

@@ -713,8 +713,8 @@
};

wlan-thermal {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;

thermal-sensors = <&tsens1 3>;

@@ -728,8 +728,8 @@
};

q6-dsp-thermal {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;

thermal-sensors = <&tsens1 4>;

@@ -743,8 +743,8 @@
};

camera-thermal {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;

thermal-sensors = <&tsens1 5>;

@@ -758,8 +758,8 @@
};

multimedia-thermal {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;

thermal-sensors = <&tsens1 6>;

@@ -845,8 +845,9 @@
compatible = "qcom,msm8998-tsens", "qcom,tsens-v2";
reg = <0x10ab000 0x1000>, /* TM */
<0x10aa000 0x1000>; /* SROT */
-
#qcom,sensors = <14>;
+ interrupts = <GIC_SPI 458 IRQ_TYPE_LEVEL_HIGH>;
+ interrupt-names = "uplow";
#thermal-sensor-cells = <1>;
};

@@ -854,8 +855,9 @@
compatible = "qcom,msm8998-tsens", "qcom,tsens-v2";
reg = <0x10ae000 0x1000>, /* TM */
<0x10ad000 0x1000>; /* SROT */
-
#qcom,sensors = <8>;
+ interrupts = <GIC_SPI 184 IRQ_TYPE_LEVEL_HIGH>;
+ interrupt-names = "uplow";
#thermal-sensor-cells = <1>;
};

--
2.17.1

2019-08-27 12:18:21

by Amit Kucheria

[permalink] [raw]
Subject: [PATCH v2 14/15] drivers: thermal: tsens: Create function to return sign-extended temperature

Hide the details of how to convert values read from TSENS HW to mCelsius
behind a function. All versions of the IP can be supported as a result.

Signed-off-by: Amit Kucheria <[email protected]>
---
drivers/thermal/qcom/tsens-common.c | 50 +++++++++++++++++++++--------
drivers/thermal/qcom/tsens.h | 2 ++
2 files changed, 38 insertions(+), 14 deletions(-)

diff --git a/drivers/thermal/qcom/tsens-common.c b/drivers/thermal/qcom/tsens-common.c
index ea2c46cc6a66a..06b44cfd5eab9 100644
--- a/drivers/thermal/qcom/tsens-common.c
+++ b/drivers/thermal/qcom/tsens-common.c
@@ -84,13 +84,43 @@ static inline int code_to_degc(u32 adc_code, const struct tsens_sensor *s)
return degc;
}

+/**
+ * tsens_hw_to_mC - Return sign-extended temperature in mCelsius.
+ * @s: Pointer to sensor struct
+ * @field: Index into regmap_field array pointing to temperature data
+ *
+ * This function handles temperature returned in ADC code or deciCelsius
+ * depending on IP version.
+ *
+ * Return: Temperature in milliCelsius on success, a negative errno will
+ * be returned in error cases
+ */
+static int tsens_hw_to_mC(struct tsens_sensor *s, int field)
+{
+ struct tsens_priv *priv = s->priv;
+ u32 temp = 0;
+ int ret;
+
+ ret = regmap_field_read(priv->rf[field], &temp);
+ if (ret)
+ return ret;
+
+ if (priv->feat->adc) {
+ /* Convert temperature from ADC code to milliCelsius */
+ return code_to_degc(temp, s) * 1000;
+ }
+
+ /* deciCelsius -> milliCelsius along with sign extension */
+ return sign_extend32(temp, priv->tempres) * 100;
+}
+
int get_temp_tsens_valid(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 last_temp = 0, valid, mask;
+ u32 valid;
int ret;

ret = regmap_field_read(priv->rf[valid_idx], &valid);
@@ -108,19 +138,7 @@ int get_temp_tsens_valid(struct tsens_sensor *s, int *temp)
}

/* Valid bit is set, OK to read the temperature */
- ret = regmap_field_read(priv->rf[temp_idx], &last_temp);
- if (ret)
- return ret;
-
- if (priv->feat->adc) {
- /* Convert temperature from ADC code to milliCelsius */
- *temp = code_to_degc(last_temp, s) * 1000;
- } else {
- mask = GENMASK(priv->fields[LAST_TEMP_0].msb,
- priv->fields[LAST_TEMP_0].lsb);
- /* Convert temperature from deciCelsius to milliCelsius */
- *temp = sign_extend32(last_temp, fls(mask) - 1) * 100;
- }
+ *temp = tsens_hw_to_mC(s, temp_idx);

return 0;
}
@@ -310,6 +328,10 @@ int __init init_common(struct tsens_priv *priv)
goto err_put_device;
}
}
+
+ /* Save away resolution of signed temperature value for this IP */
+ priv->tempres = priv->fields[LAST_TEMP_0].msb - priv->fields[LAST_TEMP_0].lsb;
+
for (i = 0, j = VALID_0; i < priv->feat->max_sensors; i++, j++) {
priv->rf[j] = devm_regmap_field_alloc(dev, priv->tm_map,
priv->fields[j]);
diff --git a/drivers/thermal/qcom/tsens.h b/drivers/thermal/qcom/tsens.h
index e1d6af71b2b9a..409395e436f32 100644
--- a/drivers/thermal/qcom/tsens.h
+++ b/drivers/thermal/qcom/tsens.h
@@ -283,6 +283,7 @@ struct tsens_context {
* struct tsens_priv - private data for each instance of the tsens IP
* @dev: pointer to struct device
* @num_sensors: number of sensors enabled on this device
+ * @tempres: number of bits used to represent signed temperature (resolution)
* @tm_map: pointer to TM register address space
* @srot_map: pointer to SROT register address space
* @tm_offset: deal with old device trees that don't address TM and SROT
@@ -299,6 +300,7 @@ struct tsens_context {
struct tsens_priv {
struct device *dev;
u32 num_sensors;
+ u32 tempres;
struct regmap *tm_map;
struct regmap *srot_map;
u32 tm_offset;
--
2.17.1

2019-08-27 12:18:26

by Amit Kucheria

[permalink] [raw]
Subject: [PATCH v2 13/15] arm64: dts: msm8916: thermal: Add interrupt support

Register upper-lower interrupt for the tsens controller.

Signed-off-by: Amit Kucheria <[email protected]>
---
arch/arm64/boot/dts/qcom/msm8916.dtsi | 22 ++++++++++++----------
1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/msm8916.dtsi b/arch/arm64/boot/dts/qcom/msm8916.dtsi
index 8686e101905cc..c0d0492d90ec0 100644
--- a/arch/arm64/boot/dts/qcom/msm8916.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8916.dtsi
@@ -176,8 +176,8 @@

thermal-zones {
cpu0_1-thermal {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;

thermal-sensors = <&tsens 5>;

@@ -206,8 +206,8 @@
};

cpu2_3-thermal {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;

thermal-sensors = <&tsens 4>;

@@ -236,8 +236,8 @@
};

gpu-thermal {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;

thermal-sensors = <&tsens 2>;

@@ -256,8 +256,8 @@
};

camera-thermal {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;

thermal-sensors = <&tsens 1>;

@@ -271,8 +271,8 @@
};

modem-thermal {
- polling-delay-passive = <250>;
- polling-delay = <1000>;
+ polling-delay-passive = <0>;
+ polling-delay = <0>;

thermal-sensors = <&tsens 0>;

@@ -816,6 +816,8 @@
nvmem-cells = <&tsens_caldata>, <&tsens_calsel>;
nvmem-cell-names = "calib", "calib_sel";
#qcom,sensors = <5>;
+ interrupts = <GIC_SPI 184 IRQ_TYPE_LEVEL_HIGH>;
+ interrupt-names = "uplow";
#thermal-sensor-cells = <1>;
};

--
2.17.1

2019-08-28 00:33:39

by Stephen Boyd

[permalink] [raw]
Subject: Re: [PATCH v2 04/15] drivers: thermal: tsens: Add debugfs support

Quoting Amit Kucheria (2019-08-27 05:14:00)
> Dump some basic version info and sensor details into debugfs. Example
> from qcs404 below:
>
> --(/sys/kernel/debug) $ ls tsens/
> 4a9000.thermal-sensor version
> --(/sys/kernel/debug) $ cat tsens/version
> 1.4.0
> --(/sys/kernel/debug) $ cat tsens/4a9000.thermal-sensor/sensors
> max: 11
> num: 10
>
> id slope offset
> ------------------------

Reviewed-by: Stephen Boyd <[email protected]>

2019-08-28 00:34:00

by Stephen Boyd

[permalink] [raw]
Subject: Re: [PATCH v2 05/15] arm: dts: msm8974: thermal: Add thermal zones for each sensor

Quoting Amit Kucheria (2019-08-27 05:14:01)
> msm8974 has 11 sensors connected to a single TSENS IP. Define a thermal
> zone for each of those sensors to expose the temperature of each zone.
>
> Signed-off-by: Amit Kucheria <[email protected]>
> Tested-by: Brian Masney <[email protected]>
> ---

Reviewed-by: Stephen Boyd <[email protected]>

2019-08-28 00:34:09

by Stephen Boyd

[permalink] [raw]
Subject: Re: [PATCH v2 06/15] arm64: dts: msm8916: thermal: Fixup HW ids for cpu sensors

Quoting Amit Kucheria (2019-08-27 05:14:02)
> msm8916 uses sensors 0, 1, 2, 4 and 5. Sensor 3 is NOT used. Fixup the
> device tree so that the correct sensor ID is used and as a result we can
> actually check the temperature for the cpu2_3 sensor.
>
> Signed-off-by: Amit Kucheria <[email protected]>
> Reviewed-by: Daniel Lezcano <[email protected]>
> ---

Reviewed-by: Stephen Boyd <[email protected]>

2019-08-28 00:35:02

by Stephen Boyd

[permalink] [raw]
Subject: Re: [PATCH v2 07/15] dt: thermal: tsens: Document interrupt support in tsens driver

Quoting Amit Kucheria (2019-08-27 05:14:03)
> Define two new required properties to define interrupts and
> interrupt-names for tsens.
>
> Signed-off-by: Amit Kucheria <[email protected]>
> ---
> Documentation/devicetree/bindings/thermal/qcom-tsens.txt | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/thermal/qcom-tsens.txt b/Documentation/devicetree/bindings/thermal/qcom-tsens.txt
> index 673cc1831ee9d..686bede72f846 100644
> --- a/Documentation/devicetree/bindings/thermal/qcom-tsens.txt
> +++ b/Documentation/devicetree/bindings/thermal/qcom-tsens.txt
> @@ -22,6 +22,8 @@ Required properties:
>
> - #thermal-sensor-cells : Should be 1. See ./thermal.txt for a description.
> - #qcom,sensors: Number of sensors in tsens block
> +- interrupts: Interrupts generated from Always-On subsystem (AOSS)

Is it always one? interrupt-names makes it sound like it.

> +- interrupt-names: Must be one of the following: "uplow", "critical"
> - Refer to Documentation/devicetree/bindings/nvmem/nvmem.txt to know how to specify
> nvmem cells
>

2019-08-28 00:36:53

by Stephen Boyd

[permalink] [raw]
Subject: Re: [PATCH v2 08/15] arm64: dts: sdm845: thermal: Add interrupt support

Quoting Amit Kucheria (2019-08-27 05:14:04)
> Register upper-lower interrupts for each of the two tsens controllers.
>
> Signed-off-by: Amit Kucheria <[email protected]>
> ---

Reviewed-by: Stephen Boyd <[email protected]>

2019-08-28 00:37:49

by Stephen Boyd

[permalink] [raw]
Subject: Re: [PATCH v2 09/15] arm64: dts: msm8996: thermal: Add interrupt support

Quoting Amit Kucheria (2019-08-27 05:14:05)
> Register upper-lower interrupts for each of the two tsens controllers.
>
> Signed-off-by: Amit Kucheria <[email protected]>
> ---
> arch/arm64/boot/dts/qcom/msm8996.dtsi | 60 ++++++++++++++-------------
> 1 file changed, 32 insertions(+), 28 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/qcom/msm8996.dtsi b/arch/arm64/boot/dts/qcom/msm8996.dtsi
> index 96c0a481f454e..bb763b362c162 100644
> --- a/arch/arm64/boot/dts/qcom/msm8996.dtsi
> +++ b/arch/arm64/boot/dts/qcom/msm8996.dtsi
> @@ -175,8 +175,8 @@
>
> thermal-zones {
> cpu0-thermal {
> - polling-delay-passive = <250>;
> - polling-delay = <1000>;
> + polling-delay-passive = <0>;
> + polling-delay = <0>;

Is it really necessary to change the configuration here to be 0 instead
of some number? Why can't we detect that there's an interrupt and then
ignore these properties?

2019-08-28 00:41:08

by Stephen Boyd

[permalink] [raw]
Subject: Re: [PATCH v2 14/15] drivers: thermal: tsens: Create function to return sign-extended temperature

Quoting Amit Kucheria (2019-08-27 05:14:10)
> diff --git a/drivers/thermal/qcom/tsens-common.c b/drivers/thermal/qcom/tsens-common.c
> index ea2c46cc6a66a..06b44cfd5eab9 100644
> --- a/drivers/thermal/qcom/tsens-common.c
> +++ b/drivers/thermal/qcom/tsens-common.c
> @@ -84,13 +84,43 @@ static inline int code_to_degc(u32 adc_code, const struct tsens_sensor *s)
> return degc;
> }
>
> +/**
> + * tsens_hw_to_mC - Return sign-extended temperature in mCelsius.
> + * @s: Pointer to sensor struct

sensor? This isn't golang!

> + * @field: Index into regmap_field array pointing to temperature data
> + *
> + * This function handles temperature returned in ADC code or deciCelsius
> + * depending on IP version.
> + *
> + * Return: Temperature in milliCelsius on success, a negative errno will
> + * be returned in error cases
> + */
> +static int tsens_hw_to_mC(struct tsens_sensor *s, int field)
> +{
> + struct tsens_priv *priv = s->priv;
> + u32 temp = 0;
> + int ret;
> +
> + ret = regmap_field_read(priv->rf[field], &temp);
> + if (ret)
> + return ret;
> +
> + if (priv->feat->adc) {
> + /* Convert temperature from ADC code to milliCelsius */

Nitpick: Move this comment above the if and drop the braces.

> + return code_to_degc(temp, s) * 1000;
> + }
> +
> + /* deciCelsius -> milliCelsius along with sign extension */
> + return sign_extend32(temp, priv->tempres) * 100;
> +}
> +
> int get_temp_tsens_valid(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 last_temp = 0, valid, mask;
> + u32 valid;
> int ret;
>
> ret = regmap_field_read(priv->rf[valid_idx], &valid);
> @@ -310,6 +328,10 @@ int __init init_common(struct tsens_priv *priv)
> goto err_put_device;
> }
> }
> +
> + /* Save away resolution of signed temperature value for this IP */
> + priv->tempres = priv->fields[LAST_TEMP_0].msb - priv->fields[LAST_TEMP_0].lsb;
> +

Why not just calculate this in the function that uses it? Is there a
reason to stash it away in the struct?

> for (i = 0, j = VALID_0; i < priv->feat->max_sensors; i++, j++) {
> priv->rf[j] = devm_regmap_field_alloc(dev, priv->tm_map,
> priv->fields[j]);

2019-08-28 15:46:04

by Stephen Boyd

[permalink] [raw]
Subject: Re: [PATCH v2 14/15] drivers: thermal: tsens: Create function to return sign-extended temperature

Quoting Amit Kucheria (2019-08-28 03:35:28)
> (Resending, replied only to Stephen by mistake)
>
> On Wed, Aug 28, 2019 at 6:08 AM Stephen Boyd <[email protected]> wrote:
> >
> > Quoting Amit Kucheria (2019-08-27 05:14:10)
> > > @@ -310,6 +328,10 @@ int __init init_common(struct tsens_priv *priv)
> > >                         goto err_put_device;
> > >                 }
> > >         }
> > > +
> > > +       /* Save away resolution of signed temperature value for this IP */
> > > +       priv->tempres = priv->fields[LAST_TEMP_0].msb - priv->fields
> [LAST_TEMP_0].lsb;
> > > +
> >
> > Why not just calculate this in the function that uses it? Is there a
> > reason to stash it away in the struct?
>
> To avoid recalculating in an often-called function. It doesn't change for an IP
> version.
>
> We can't make it static either inside that function since the initializer isn't
> constant.
>

This sounds like a super micro optimization. It's a couple derefs and a
subtraction. If it isn't used anywhere else please just move it into the
function where it's used.

2019-08-28 21:43:49

by Stephen Boyd

[permalink] [raw]
Subject: Re: [PATCH v2 15/15] drivers: thermal: tsens: Add interrupt support

Quoting Amit Kucheria (2019-08-27 05:14:11)
> diff --git a/drivers/thermal/qcom/tsens-common.c b/drivers/thermal/qcom/tsens-common.c
> index 06b44cfd5eab9..c549f8e1488ba 100644
> --- a/drivers/thermal/qcom/tsens-common.c
> +++ b/drivers/thermal/qcom/tsens-common.c
> @@ -114,6 +146,314 @@ static int tsens_hw_to_mC(struct tsens_sensor *s, int field)
> return sign_extend32(temp, priv->tempres) * 100;
> }
>
> +/**
> + * tsens_mC_to_hw - Return correct value to be written to threshold
> + * registers, whether in ADC code or deciCelsius depending on IP version

Document arguments and return value? Maybe summary can be 'convert
tsens temperature to hardware register value'?

> + */
> +static int tsens_mC_to_hw(struct tsens_sensor *s, int temp)
> +{
> + struct tsens_priv *priv = s->priv;
> +
> + if (priv->feat->adc) {
> + /* milliC to C to adc code */
> + return degc_to_code(temp / 1000, s);
> + }
> +
> + /* milliC to deciC */
> + return temp / 100;
> +}
> +
> +static inline unsigned int tsens_ver(struct tsens_priv *priv)

Can this return the enum instead of unsigned int?

> +{
> + return priv->feat->ver_major;
> +}
> +
> +/**
> + * tsens_set_interrupt_v1 - Disable an interrupt (enable = false)
> + * Re-enable an interrupt (enable = true)
> + */
> +static void tsens_set_interrupt_v1(struct tsens_priv *priv, u32 hw_id,
> + enum tsens_irq_type irq_type, bool enable)
> +{
> + u32 index;
> +
> + if (enable) {
> + switch (irq_type) {
> + case UPPER:
> + index = UP_INT_CLEAR_0 + hw_id;
> + break;
> + case LOWER:
> + index = LOW_INT_CLEAR_0 + hw_id;
> + break;
> + }
> + regmap_field_write(priv->rf[index], 0);
> + } else {
> + switch (irq_type) {
> + case UPPER:
> + index = UP_INT_CLEAR_0 + hw_id;
> + break;
> + case LOWER:
> + index = LOW_INT_CLEAR_0 + hw_id;
> + break;
> + }
> + regmap_field_write(priv->rf[index], 1);
> + }

De-dup the switch statement and have

regmap_field_write(priv->rf[index], enable ? 1 : 0);

> +}
> +
> +/**
> + * tsens_set_interrupt_v2 - Disable an interrupt (enable = false)
> + * Re-enable an interrupt (enable = true)
> + */
> +static void tsens_set_interrupt_v2(struct tsens_priv *priv, u32 hw_id,
> + enum tsens_irq_type irq_type, bool enable)
> +{
> + u32 index_mask, index_clear;
> +
> + if (enable) {
> + switch (irq_type) {
> + case UPPER:
> + index_mask = UP_INT_MASK_0 + hw_id;
> + break;
> + case LOWER:
> + index_mask = LOW_INT_MASK_0 + hw_id;
> + break;
> + }
> + regmap_field_write(priv->rf[index_mask], 0);
> + } else {
> + /* To disable the interrupt flag for a sensor:

Nitpick: Wrong comment style.

> + * 1. Mask further interrupts for this sensor
> + * 2. Write 1 followed by 0 to clear the interrupt
> + */
> + switch (irq_type) {
> + case UPPER:
> + index_mask = UP_INT_MASK_0 + hw_id;
> + index_clear = UP_INT_CLEAR_0 + hw_id;
> + break;
> + case LOWER:
> + index_mask = LOW_INT_MASK_0 + hw_id;
> + index_clear = LOW_INT_CLEAR_0 + hw_id;
> + break;
> + }

Please extract index_mask and index_clear assignments to one switch
statement and then change the sequence to an if/else

if (enable) {
regmap_field_write(priv->rf[index_mask], 1);
regmap_field_write(priv->rf[index_clear], 1);
regmap_field_write(priv->rf[index_clear], 0);
} else {
regmap_field_write(priv->rf[index_mask], 0);
}

> +}
> +
> +/**
> + * tsens_set_interrupt - Disable an interrupt (enable = false)
> + * Re-enable an interrupt (enable = true)
> + */
> +static void tsens_set_interrupt(struct tsens_priv *priv, u32 hw_id,
> + enum tsens_irq_type irq_type, bool enable)
> +{
> + dev_dbg(priv->dev, "[%u] %s: %s -> %s\n", hw_id, __func__,
> + irq_type ? ((irq_type == 1) ? "UP" : "CRITICAL") : "LOW",
> + enable ? "en" : "dis");
> + if (tsens_ver(priv) > VER_1_X)
> + tsens_set_interrupt_v2(priv, hw_id, irq_type, enable);
> + else
> + tsens_set_interrupt_v1(priv, hw_id, irq_type, enable);
> +}
> +
> +/**
> + * tsens_threshold_violated - Check if a sensor temperature violated a preset threshold
> + *

Document arguments?

> + * Return: 0 if threshold was not violated, 1 if it was violated and negative
> + * errno in case of errors
> + */
> +static int tsens_threshold_violated(struct tsens_priv *priv, u32 hw_id,
> + struct tsens_irq_data *d)
> +{
> + int ret;
> +
> + ret = regmap_field_read(priv->rf[UPPER_STATUS_0 + hw_id], &d->up_viol);
> + if (ret)
> + return ret;
> + ret = regmap_field_read(priv->rf[LOWER_STATUS_0 + hw_id], &d->low_viol);
> + if (ret)
> + return ret;
> + if (d->up_viol || d->low_viol)
> + return 1;
> +
> + return 0;
> +}
> +
> +static int tsens_read_irq_state(struct tsens_priv *priv, u32 hw_id,
> + struct tsens_sensor *s, struct tsens_irq_data *d)
> +{
> + int ret;
> +
> + ret = regmap_field_read(priv->rf[UP_INT_CLEAR_0 + hw_id], &d->up_irq_clear);
> + if (ret)
> + return ret;
> + ret = regmap_field_read(priv->rf[LOW_INT_CLEAR_0 + hw_id], &d->low_irq_clear);
> + if (ret)
> + return ret;
> + if (tsens_ver(priv) > VER_1_X) {
> + ret = regmap_field_read(priv->rf[UP_INT_MASK_0 + hw_id], &d->up_irq_mask);
> + if (ret)
> + return ret;
> + ret = regmap_field_read(priv->rf[LOW_INT_MASK_0 + hw_id], &d->low_irq_mask);
> + if (ret)
> + return ret;
> + } else {
> + /* No mask register on older TSENS */
> + d->up_irq_mask = 0;
> + d->low_irq_mask = 0;
> + }
> +
> + d->up_thresh = tsens_hw_to_mC(s, UP_THRESH_0 + hw_id);
> + d->low_thresh = tsens_hw_to_mC(s, LOW_THRESH_0 + hw_id);
> +
> + dev_dbg(priv->dev, "[%u] %s%s: status(%u|%u) | clr(%u|%u) | mask(%u|%u)\n",
> + hw_id, __func__, (d->up_viol || d->low_viol) ? "(V)" : "",
> + d->low_viol, d->up_viol, d->low_irq_clear, d->up_irq_clear,
> + d->low_irq_mask, d->up_irq_mask);
> + dev_dbg(priv->dev, "[%u] %s%s: thresh: (%d:%d)\n", hw_id, __func__,
> + (d->up_viol || d->low_viol) ? "(violation)" : "",
> + d->low_thresh, d->up_thresh);
> +
> + return 0;
> +}
> +
> +static inline u32 masked_irq(u32 hw_id, u32 mask, enum tsens_ver ver)
> +{
> + if (ver > VER_1_X)
> + return mask & (1 << hw_id);
> +
> + /* v1, v0.1 don't have a irq mask register */
> + return 0;
> +}
> +
> +irqreturn_t tsens_irq_thread(int irq, void *data)
> +{
> + struct tsens_priv *priv = data;
> + struct tsens_irq_data d;
> + bool enable = true, disable = false;
> + unsigned long flags;
> + int temp, ret, i;
> +
> + /*
> + * Check if any sensor raised an IRQ - for each sensor connected to the
> + * TSENS block if it set the threshold violation bit.
> + */
> + for (i = 0; i < priv->num_sensors; i++) {
> + bool trigger = 0;

How about trigger = false? It's a bool.

> + struct tsens_sensor *s = &priv->sensor[i];
> + u32 hw_id = s->hw_id;
> +
> + if (IS_ERR(priv->sensor[i].tzd))
> + continue;
> + if (!tsens_threshold_violated(priv, hw_id, &d))
> + continue;
> + ret = get_temp_tsens_valid(s, &temp);
> + if (ret) {
> + dev_err(priv->dev, "[%u] %s: error reading sensor\n", hw_id, __func__);

I hope there isn't an interrupt storm where we're trying to print out
messages from the irq handler.

> + continue;
> + }
> +
> + spin_lock_irqsave(&priv->ul_lock, flags);
> +
> + tsens_read_irq_state(priv, hw_id, s, &d);
> +
> + if (d.up_viol &&
> + !masked_irq(hw_id, d.up_irq_mask, tsens_ver(priv))) {
> + tsens_set_interrupt(priv, hw_id, UPPER, disable);
> + if (d.up_thresh > temp) {
> + dev_dbg(priv->dev, "[%u] %s: re-arm upper\n",
> + priv->sensor[i].hw_id, __func__);
> + /* unmask the interrupt for this sensor */
> + tsens_set_interrupt(priv, hw_id, UPPER, enable);
> + } else {
> + trigger = 1;
> + /* Keep irq masked */
> + }
> + } else if (d.low_viol &&
> + !masked_irq(hw_id, d.low_irq_mask, tsens_ver(priv))) {
> + tsens_set_interrupt(priv, hw_id, LOWER, disable);
> + if (d.low_thresh < temp) {
> + dev_dbg(priv->dev, "[%u] %s: re-arm low\n",
> + priv->sensor[i].hw_id, __func__);
> + /* unmask the interrupt for this sensor */
> + tsens_set_interrupt(priv, hw_id, LOWER, enable);
> + } else {
> + trigger = 1;
> + /* Keep irq masked */
> + }
> + }
> +
> + spin_unlock_irqrestore(&priv->ul_lock, flags);
> +
> + if (trigger) {
> + dev_dbg(priv->dev, "[%u] %s: TZ update trigger (%d mC)\n",
> + hw_id, __func__, temp);
> + thermal_zone_device_update(priv->sensor[i].tzd,
> + THERMAL_EVENT_UNSPECIFIED);
> + } else {
> + dev_dbg(priv->dev, "[%u] %s: no violation: %d\n",
> + hw_id, __func__, temp);
> + }
> + }
> +
> + return IRQ_HANDLED;

Should we return IRQ_NONE in the case that the above for loop didn't
find anything in those if/else-ifs?

> +}
> +
> +int tsens_set_trips(void *_sensor, int low, int high)
> +{
> + struct tsens_sensor *s = _sensor;
> + struct tsens_priv *priv = s->priv;
> + struct device *dev = priv->dev;
> + struct tsens_irq_data d;
> + unsigned long flags;
> + int high_val, low_val, cl_high, cl_low;
> + bool enable = true;
> + u32 hw_id = s->hw_id;
> +
> + dev_dbg(dev, "[%u] %s: proposed thresholds: (%d:%d)\n",
> + hw_id, __func__, low, high);
> +
> + cl_high = clamp_val(high, -40000, 120000);
> + cl_low = clamp_val(low, -40000, 120000);
> +
> + high_val = tsens_mC_to_hw(s, cl_high);
> + low_val = tsens_mC_to_hw(s, cl_low);
> +
> + spin_lock_irqsave(&priv->ul_lock, flags);
> +
> + tsens_read_irq_state(priv, hw_id, s, &d);
> +
> + /* Write the new thresholds and clear the status */
> + regmap_field_write(priv->rf[LOW_THRESH_0 + hw_id], low_val);
> + regmap_field_write(priv->rf[UP_THRESH_0 + hw_id], high_val);
> + tsens_set_interrupt(priv, hw_id, LOWER, enable);
> + tsens_set_interrupt(priv, hw_id, UPPER, enable);

Just pass true? Why is there an enable local variable?

> +
> + spin_unlock_irqrestore(&priv->ul_lock, flags);
> +
> + dev_dbg(dev, "[%u] %s: (%d:%d)->(%d:%d)\n",
> + s->hw_id, __func__, d.low_thresh, d.up_thresh, cl_low, cl_high);
> +
> + return 0;
> +}
> +
[...]
> @@ -319,28 +659,31 @@ int __init init_common(struct tsens_priv *priv)
> ret = PTR_ERR(priv->rf[SENSOR_EN]);
> goto err_put_device;
> }
> - /* now alloc regmap_fields in tm_map */
> - for (i = 0, j = LAST_TEMP_0; i < priv->feat->max_sensors; i++, j++) {
> - priv->rf[j] = devm_regmap_field_alloc(dev, priv->tm_map,
> - priv->fields[j]);
> - if (IS_ERR(priv->rf[j])) {
> - ret = PTR_ERR(priv->rf[j]);
> - goto err_put_device;
> - }
> + priv->rf[INT_EN] = devm_regmap_field_alloc(dev, priv->tm_map,
> + priv->fields[INT_EN]);
> + if (IS_ERR(priv->rf[INT_EN])) {
> + ret = PTR_ERR(priv->rf[INT_EN]);
> + goto err_put_device;
> }
>
> - /* Save away resolution of signed temperature value for this IP */
> - priv->tempres = priv->fields[LAST_TEMP_0].msb - priv->fields[LAST_TEMP_0].lsb;
> -
> - for (i = 0, j = VALID_0; i < priv->feat->max_sensors; i++, j++) {
> - priv->rf[j] = devm_regmap_field_alloc(dev, priv->tm_map,
> - priv->fields[j]);
> - if (IS_ERR(priv->rf[j])) {
> - ret = PTR_ERR(priv->rf[j]);
> - 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++) {
> + int idx = j + i;
> +
> + priv->rf[idx] = devm_regmap_field_alloc(dev, priv->tm_map,
> + priv->fields[idx]);
> + if (IS_ERR(priv->rf[idx])) {
> + ret = PTR_ERR(priv->rf[idx]);
> + goto err_put_device;
> + }
> }
> }
> + /* Save away resolution of signed temperature value for this IP */
> + priv->tempres = priv->fields[LAST_TEMP_0].msb - priv->fields[LAST_TEMP_0].lsb;

Leave this where it was, i.e. before the for loop? Or is that a bug and
it doesn't actually work unless it's after the for loop? In which case,
this should go to the previous patch.

>
> + spin_lock_init(&priv->ul_lock);
> + tsens_enable_irq(priv);
> tsens_debug_init(op);
>
> return 0;
> diff --git a/drivers/thermal/qcom/tsens.c b/drivers/thermal/qcom/tsens.c
> index 772aa76b50e12..a4335717aeede 100644
> --- a/drivers/thermal/qcom/tsens.c
> +++ b/drivers/thermal/qcom/tsens.c
> @@ -96,7 +99,33 @@ static int tsens_register(struct tsens_priv *priv)
> if (priv->ops->enable)
> priv->ops->enable(priv, i);
> }
> +
> + pdev = of_find_device_by_node(priv->dev->of_node);
> + if (!pdev) {
> + dev_err(&pdev->dev, "%s: device node not found in DT\n", __func__);

Do we really need this? Maybe just bail out in silence because this
should never happen?

> + return -ENODEV;
> + }
> +
> + irq = platform_get_irq_byname(pdev, "uplow");
> + if (irq < 0) {
> + dev_err(&pdev->dev, "%s: missing irq in dt: uplow\n", __func__);

You can drop the error print. I upstreamed a change to print the error
generically in the core.

> + return irq;

Did we need to put_device() here?

> + }
> +
> + ret = devm_request_threaded_irq(&pdev->dev, irq,
> + NULL, tsens_irq_thread,
> + IRQF_TRIGGER_HIGH | IRQF_ONESHOT,
> + dev_name(&pdev->dev), priv);
> + if (ret) {
> + dev_err(&pdev->dev, "%s: failed to get irq\n", __func__);
> + goto err_put_device;
> + }
> + enable_irq_wake(irq);
> return 0;
> +
> +err_put_device:
> + put_device(&pdev->dev);
> + return ret;

2019-08-29 03:54:40

by Thara Gopinath

[permalink] [raw]
Subject: Re: [PATCH v2 15/15] drivers: thermal: tsens: Add interrupt support

Hi Amit,

On 08/27/2019 08:14 AM, Amit Kucheria wrote:
> Depending on the IP version, TSENS supports upper, lower, max, min and
> critical threshold interrupts. We only add support for upper and lower
> threshold interrupts for now.
>
> TSENSv2 has an irq [status|clear|mask] bit tuple for each sensor while
> earlier versions only have a single bit per sensor to denote status and
> clear. At each interrupt, we reprogram the new upper and lower threshold
> in the .set_trip callback.
>
> Signed-off-by: Amit Kucheria <[email protected]>
> ---
> drivers/thermal/qcom/tsens-common.c | 377 ++++++++++++++++++++++++++--
> drivers/thermal/qcom/tsens-v0_1.c | 11 +
> drivers/thermal/qcom/tsens-v1.c | 29 +++
> drivers/thermal/qcom/tsens-v2.c | 13 +
> drivers/thermal/qcom/tsens.c | 32 ++-
> drivers/thermal/qcom/tsens.h | 270 ++++++++++++++++----
> 6 files changed, 669 insertions(+), 63 deletions(-)
>
> diff --git a/drivers/thermal/qcom/tsens-common.c b/drivers/thermal/qcom/tsens-common.c
> index 06b44cfd5eab9..c549f8e1488ba 100644
> --- a/drivers/thermal/qcom/tsens-common.c
> +++ b/drivers/thermal/qcom/tsens-common.c
> @@ -13,6 +13,30 @@
> #include <linux/regmap.h>
> #include "tsens.h"
>
> +/** struct tsens_irq_data - IRQ status and temperature violations
> + * @up_viol: upper threshold violated
> + * @up_thresh: upper threshold temperature value
> + * @up_irq_mask: mask register for upper threshold irqs
> + * @up_irq_clear: clear register for uppper threshold irqs
> + * @low_viol: lower threshold violated
> + * @low_thresh: lower threshold temperature value
> + * @low_irq_mask: mask register for lower threshold irqs
> + * @low_irq_clear: clear register for lower threshold irqs
> + *
> + * Structure containing data about temperature threshold settings and
> + * irq status if they were violated.
> + */
> +struct tsens_irq_data {
> + u32 up_viol;
> + int up_thresh;
> + u32 up_irq_mask;
> + u32 up_irq_clear;
> + u32 low_viol;
> + int low_thresh;
> + u32 low_irq_mask;
> + u32 low_irq_clear;
> +};
> +
> char *qfprom_read(struct device *dev, const char *cname)
> {
> struct nvmem_cell *cell;
> @@ -65,6 +89,14 @@ void compute_intercept_slope(struct tsens_priv *priv, u32 *p1,
> }
> }
>
> +static inline u32 degc_to_code(int degc, const struct tsens_sensor *sensor)
> +{
> + u64 code = (degc * sensor->slope + sensor->offset) / SLOPE_FACTOR;
> +
> + pr_debug("%s: raw_code: 0x%llx, degc:%d\n", __func__, code, degc);
> + return clamp_val(code, THRESHOLD_MIN_ADC_CODE, THRESHOLD_MAX_ADC_CODE);
> +}
> +
> static inline int code_to_degc(u32 adc_code, const struct tsens_sensor *s)
> {
> int degc, num, den;
> @@ -114,6 +146,314 @@ static int tsens_hw_to_mC(struct tsens_sensor *s, int field)
> return sign_extend32(temp, priv->tempres) * 100;
> }
>
> +/**
> + * tsens_mC_to_hw - Return correct value to be written to threshold
> + * registers, whether in ADC code or deciCelsius depending on IP version
> + */
> +static int tsens_mC_to_hw(struct tsens_sensor *s, int temp)
> +{
> + struct tsens_priv *priv = s->priv;
> +
> + if (priv->feat->adc) {
> + /* milliC to C to adc code */
> + return degc_to_code(temp / 1000, s);
> + }
> +
> + /* milliC to deciC */
> + return temp / 100;
> +}
> +
> +static inline unsigned int tsens_ver(struct tsens_priv *priv)
> +{
> + return priv->feat->ver_major;
> +}
> +
> +/**
> + * tsens_set_interrupt_v1 - Disable an interrupt (enable = false)
> + * Re-enable an interrupt (enable = true)
> + */
> +static void tsens_set_interrupt_v1(struct tsens_priv *priv, u32 hw_id,
> + enum tsens_irq_type irq_type, bool enable)
> +{
> + u32 index;
> +
> + if (enable) {
> + switch (irq_type) {
> + case UPPER:
> + index = UP_INT_CLEAR_0 + hw_id;
> + break;
> + case LOWER:
> + index = LOW_INT_CLEAR_0 + hw_id;
> + break;
> + }
> + regmap_field_write(priv->rf[index], 0);
> + } else {
> + switch (irq_type) {
> + case UPPER:
> + index = UP_INT_CLEAR_0 + hw_id;
> + break;
> + case LOWER:
> + index = LOW_INT_CLEAR_0 + hw_id;
> + break;
> + }
> + regmap_field_write(priv->rf[index], 1);
> + }
> +}
> +
> +/**
> + * tsens_set_interrupt_v2 - Disable an interrupt (enable = false)
> + * Re-enable an interrupt (enable = true)
> + */
> +static void tsens_set_interrupt_v2(struct tsens_priv *priv, u32 hw_id,
> + enum tsens_irq_type irq_type, bool enable)
> +{
> + u32 index_mask, index_clear;
> +
> + if (enable) {
> + switch (irq_type) {
> + case UPPER:
> + index_mask = UP_INT_MASK_0 + hw_id;
> + break;
> + case LOWER:
> + index_mask = LOW_INT_MASK_0 + hw_id;
> + break;
> + }
> + regmap_field_write(priv->rf[index_mask], 0);
> + } else {
> + /* To disable the interrupt flag for a sensor:
> + * 1. Mask further interrupts for this sensor
> + * 2. Write 1 followed by 0 to clear the interrupt
> + */
> + switch (irq_type) {
> + case UPPER:
> + index_mask = UP_INT_MASK_0 + hw_id;
> + index_clear = UP_INT_CLEAR_0 + hw_id;
> + break;
> + case LOWER:
> + index_mask = LOW_INT_MASK_0 + hw_id;
> + index_clear = LOW_INT_CLEAR_0 + hw_id;
> + break;
> + }
> + regmap_field_write(priv->rf[index_mask], 1);
> + regmap_field_write(priv->rf[index_clear], 1);
> + regmap_field_write(priv->rf[index_clear], 0);
> + }
> +}
> +
> +/**
> + * tsens_set_interrupt - Disable an interrupt (enable = false)
> + * Re-enable an interrupt (enable = true)
> + */
> +static void tsens_set_interrupt(struct tsens_priv *priv, u32 hw_id,
> + enum tsens_irq_type irq_type, bool enable)
> +{
> + dev_dbg(priv->dev, "[%u] %s: %s -> %s\n", hw_id, __func__,
> + irq_type ? ((irq_type == 1) ? "UP" : "CRITICAL") : "LOW",
> + enable ? "en" : "dis");
> + if (tsens_ver(priv) > VER_1_X)
> + tsens_set_interrupt_v2(priv, hw_id, irq_type, enable);
> + else
> + tsens_set_interrupt_v1(priv, hw_id, irq_type, enable);
> +}
> +
> +/**
> + * tsens_threshold_violated - Check if a sensor temperature violated a preset threshold
> + *
> + * Return: 0 if threshold was not violated, 1 if it was violated and negative
> + * errno in case of errors
> + */
> +static int tsens_threshold_violated(struct tsens_priv *priv, u32 hw_id,
> + struct tsens_irq_data *d)
> +{
> + int ret;
> +
> + ret = regmap_field_read(priv->rf[UPPER_STATUS_0 + hw_id], &d->up_viol);
> + if (ret)
> + return ret;
> + ret = regmap_field_read(priv->rf[LOWER_STATUS_0 + hw_id], &d->low_viol);
> + if (ret)
> + return ret;
> + if (d->up_viol || d->low_viol)
> + return 1;
> +
> + return 0;
> +}
> +
> +static int tsens_read_irq_state(struct tsens_priv *priv, u32 hw_id,
> + struct tsens_sensor *s, struct tsens_irq_data *d)
> +{
> + int ret;
> +
> + ret = regmap_field_read(priv->rf[UP_INT_CLEAR_0 + hw_id], &d->up_irq_clear);
> + if (ret)
> + return ret;
> + ret = regmap_field_read(priv->rf[LOW_INT_CLEAR_0 + hw_id], &d->low_irq_clear);
> + if (ret)
> + return ret;
> + if (tsens_ver(priv) > VER_1_X) {
> + ret = regmap_field_read(priv->rf[UP_INT_MASK_0 + hw_id], &d->up_irq_mask);
> + if (ret)
> + return ret;
> + ret = regmap_field_read(priv->rf[LOW_INT_MASK_0 + hw_id], &d->low_irq_mask);
> + if (ret)
> + return ret;
> + } else {
> + /* No mask register on older TSENS */
> + d->up_irq_mask = 0;
> + d->low_irq_mask = 0;
> + }
> +
> + d->up_thresh = tsens_hw_to_mC(s, UP_THRESH_0 + hw_id);
> + d->low_thresh = tsens_hw_to_mC(s, LOW_THRESH_0 + hw_id);
> +
> + dev_dbg(priv->dev, "[%u] %s%s: status(%u|%u) | clr(%u|%u) | mask(%u|%u)\n",
> + hw_id, __func__, (d->up_viol || d->low_viol) ? "(V)" : "",
> + d->low_viol, d->up_viol, d->low_irq_clear, d->up_irq_clear,
> + d->low_irq_mask, d->up_irq_mask);
> + dev_dbg(priv->dev, "[%u] %s%s: thresh: (%d:%d)\n", hw_id, __func__,
> + (d->up_viol || d->low_viol) ? "(violation)" : "",
> + d->low_thresh, d->up_thresh);
> +
> + return 0;
> +}
> +
> +static inline u32 masked_irq(u32 hw_id, u32 mask, enum tsens_ver ver)
> +{
> + if (ver > VER_1_X)
> + return mask & (1 << hw_id);
> +
> + /* v1, v0.1 don't have a irq mask register */
> + return 0;
> +}
> +
> +irqreturn_t tsens_irq_thread(int irq, void *data)
> +{
> + struct tsens_priv *priv = data;
> + struct tsens_irq_data d;
> + bool enable = true, disable = false;
> + unsigned long flags;
> + int temp, ret, i;
> +
> + /*
> + * Check if any sensor raised an IRQ - for each sensor connected to the
> + * TSENS block if it set the threshold violation bit.
> + */
> + for (i = 0; i < priv->num_sensors; i++) {
> + bool trigger = 0;
> + struct tsens_sensor *s = &priv->sensor[i];
> + u32 hw_id = s->hw_id;
> +
> + if (IS_ERR(priv->sensor[i].tzd))
> + continue;
> + if (!tsens_threshold_violated(priv, hw_id, &d))
> + continue;
> + ret = get_temp_tsens_valid(s, &temp);
> + if (ret) {
> + dev_err(priv->dev, "[%u] %s: error reading sensor\n", hw_id, __func__);
> + continue;
> + }
> +
> + spin_lock_irqsave(&priv->ul_lock, flags);
> +
> + tsens_read_irq_state(priv, hw_id, s, &d);
> +
> + if (d.up_viol &&
> + !masked_irq(hw_id, d.up_irq_mask, tsens_ver(priv))) {
> + tsens_set_interrupt(priv, hw_id, UPPER, disable);
> + if (d.up_thresh > temp) {
> + dev_dbg(priv->dev, "[%u] %s: re-arm upper\n",
> + priv->sensor[i].hw_id, __func__);
> + /* unmask the interrupt for this sensor */
> + tsens_set_interrupt(priv, hw_id, UPPER, enable);
> + } else {
> + trigger = 1;
> + /* Keep irq masked */
> + }
> + } else if (d.low_viol &&
> + !masked_irq(hw_id, d.low_irq_mask, tsens_ver(priv))) {
> + tsens_set_interrupt(priv, hw_id, LOWER, disable);
> + if (d.low_thresh < temp) {
> + dev_dbg(priv->dev, "[%u] %s: re-arm low\n",
> + priv->sensor[i].hw_id, __func__);
> + /* unmask the interrupt for this sensor */
> + tsens_set_interrupt(priv, hw_id, LOWER, enable);
> + } else {
> + trigger = 1;
> + /* Keep irq masked */
> + }
> + }
> +
> + spin_unlock_irqrestore(&priv->ul_lock, flags);
> +
> + if (trigger) {
> + dev_dbg(priv->dev, "[%u] %s: TZ update trigger (%d mC)\n",
> + hw_id, __func__, temp);
> + thermal_zone_device_update(priv->sensor[i].tzd,
> + THERMAL_EVENT_UNSPECIFIED);
Why not use thermal_notify_framework? Or do you want to loop over all
registered trips ?

Another comment I have is regarding support for multiple thermal zone.
It might not be needed now, but presence of the same sensor can serve
multiple zones (one for cooling and one for warming) . I am not sure if
you want to put some infrastructure in place for that as well for
interrupt handling.


Regards
Thara
> + } else {
> + dev_dbg(priv->dev, "[%u] %s: no violation: %d\n",
> + hw_id, __func__, temp);
> + }
> + }
> +
> + return IRQ_HANDLED;
> +}
> +
> +int tsens_set_trips(void *_sensor, int low, int high)
> +{
> + struct tsens_sensor *s = _sensor;
> + struct tsens_priv *priv = s->priv;
> + struct device *dev = priv->dev;
> + struct tsens_irq_data d;
> + unsigned long flags;
> + int high_val, low_val, cl_high, cl_low;
> + bool enable = true;
> + u32 hw_id = s->hw_id;
> +
> + dev_dbg(dev, "[%u] %s: proposed thresholds: (%d:%d)\n",
> + hw_id, __func__, low, high);
> +
> + cl_high = clamp_val(high, -40000, 120000);
> + cl_low = clamp_val(low, -40000, 120000);
> +
> + high_val = tsens_mC_to_hw(s, cl_high);
> + low_val = tsens_mC_to_hw(s, cl_low);
> +
> + spin_lock_irqsave(&priv->ul_lock, flags);
> +
> + tsens_read_irq_state(priv, hw_id, s, &d);
> +
> + /* Write the new thresholds and clear the status */
> + regmap_field_write(priv->rf[LOW_THRESH_0 + hw_id], low_val);
> + regmap_field_write(priv->rf[UP_THRESH_0 + hw_id], high_val);
> + tsens_set_interrupt(priv, hw_id, LOWER, enable);
> + tsens_set_interrupt(priv, hw_id, UPPER, enable);
> +
> + spin_unlock_irqrestore(&priv->ul_lock, flags);
> +
> + dev_dbg(dev, "[%u] %s: (%d:%d)->(%d:%d)\n",
> + s->hw_id, __func__, d.low_thresh, d.up_thresh, cl_low, cl_high);
> +
> + return 0;
> +}
> +
> +int tsens_enable_irq(struct tsens_priv *priv)
> +{
> + int ret;
> + int val = tsens_ver(priv) > VER_1_X ? 7 : 1;
> +
> + ret = regmap_field_write(priv->rf[INT_EN], val);
> + if (ret < 0)
> + dev_err(priv->dev, "%s: failed to enable interrupts\n", __func__);
> +
> + return ret;
> +}
> +
> +void tsens_disable_irq(struct tsens_priv *priv)
> +{
> + regmap_field_write(priv->rf[INT_EN], 0);
> +}
> +
> int get_temp_tsens_valid(struct tsens_sensor *s, int *temp)
> {
> struct tsens_priv *priv = s->priv;
> @@ -319,28 +659,31 @@ int __init init_common(struct tsens_priv *priv)
> ret = PTR_ERR(priv->rf[SENSOR_EN]);
> goto err_put_device;
> }
> - /* now alloc regmap_fields in tm_map */
> - for (i = 0, j = LAST_TEMP_0; i < priv->feat->max_sensors; i++, j++) {
> - priv->rf[j] = devm_regmap_field_alloc(dev, priv->tm_map,
> - priv->fields[j]);
> - if (IS_ERR(priv->rf[j])) {
> - ret = PTR_ERR(priv->rf[j]);
> - goto err_put_device;
> - }
> + priv->rf[INT_EN] = devm_regmap_field_alloc(dev, priv->tm_map,
> + priv->fields[INT_EN]);
> + if (IS_ERR(priv->rf[INT_EN])) {
> + ret = PTR_ERR(priv->rf[INT_EN]);
> + goto err_put_device;
> }
>
> - /* Save away resolution of signed temperature value for this IP */
> - priv->tempres = priv->fields[LAST_TEMP_0].msb - priv->fields[LAST_TEMP_0].lsb;
> -
> - for (i = 0, j = VALID_0; i < priv->feat->max_sensors; i++, j++) {
> - priv->rf[j] = devm_regmap_field_alloc(dev, priv->tm_map,
> - priv->fields[j]);
> - if (IS_ERR(priv->rf[j])) {
> - ret = PTR_ERR(priv->rf[j]);
> - 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++) {
> + int idx = j + i;
> +
> + priv->rf[idx] = devm_regmap_field_alloc(dev, priv->tm_map,
> + priv->fields[idx]);
> + if (IS_ERR(priv->rf[idx])) {
> + ret = PTR_ERR(priv->rf[idx]);
> + goto err_put_device;
> + }
> }
> }
> + /* Save away resolution of signed temperature value for this IP */
> + priv->tempres = priv->fields[LAST_TEMP_0].msb - priv->fields[LAST_TEMP_0].lsb;
>
> + spin_lock_init(&priv->ul_lock);
> + tsens_enable_irq(priv);
> tsens_debug_init(op);
>
> return 0;
> diff --git a/drivers/thermal/qcom/tsens-v0_1.c b/drivers/thermal/qcom/tsens-v0_1.c
> index 6f26fadf4c279..a267b66e61d6b 100644
> --- a/drivers/thermal/qcom/tsens-v0_1.c
> +++ b/drivers/thermal/qcom/tsens-v0_1.c
> @@ -339,9 +339,20 @@ static const struct reg_field tsens_v0_1_regfields[MAX_REGFIELDS] = {
> /* INTERRUPT ENABLE */
> [INT_EN] = REG_FIELD(TM_INT_EN_OFF, 0, 0),
>
> + /* UPPER/LOWER TEMPERATURE THRESHOLDS */
> + REG_FIELD_FOR_EACH_SENSOR11(LOW_THRESH, TM_Sn_UPPER_LOWER_STATUS_CTRL_OFF, 0, 9),
> + REG_FIELD_FOR_EACH_SENSOR11(UP_THRESH, TM_Sn_UPPER_LOWER_STATUS_CTRL_OFF, 10, 19),
> +
> + /* UPPER/LOWER INTERRUPTS [CLEAR/STATUS] */
> + REG_FIELD_FOR_EACH_SENSOR11(LOW_INT_CLEAR, TM_Sn_UPPER_LOWER_STATUS_CTRL_OFF, 20, 20),
> + REG_FIELD_FOR_EACH_SENSOR11(UP_INT_CLEAR, TM_Sn_UPPER_LOWER_STATUS_CTRL_OFF, 21, 21),
> +
> + /* NO CRITICAL INTERRUPT SUPPORT on v0.1 */
> +
> /* Sn_STATUS */
> REG_FIELD_FOR_EACH_SENSOR11(LAST_TEMP, TM_Sn_STATUS_OFF, 0, 9),
> /* No VALID field on v0.1 */
> + /* xxx_STATUS bits: 1 == threshold violated */
> REG_FIELD_FOR_EACH_SENSOR11(MIN_STATUS, TM_Sn_STATUS_OFF, 10, 10),
> REG_FIELD_FOR_EACH_SENSOR11(LOWER_STATUS, TM_Sn_STATUS_OFF, 11, 11),
> REG_FIELD_FOR_EACH_SENSOR11(UPPER_STATUS, TM_Sn_STATUS_OFF, 12, 12),
> diff --git a/drivers/thermal/qcom/tsens-v1.c b/drivers/thermal/qcom/tsens-v1.c
> index 10b595d4f6199..86259c9821be8 100644
> --- a/drivers/thermal/qcom/tsens-v1.c
> +++ b/drivers/thermal/qcom/tsens-v1.c
> @@ -17,6 +17,8 @@
> #define TM_Sn_UPPER_LOWER_STATUS_CTRL_OFF 0x0004
> #define TM_Sn_STATUS_OFF 0x0044
> #define TM_TRDY_OFF 0x0084
> +#define TM_HIGH_LOW_INT_STATUS_OFF 0x0088
> +#define TM_HIGH_LOW_Sn_INT_THRESHOLD_OFF 0x0090
>
> /* eeprom layout data for qcs404/405 (v1) */
> #define BASE0_MASK 0x000007f8
> @@ -167,9 +169,36 @@ static const struct reg_field tsens_v1_regfields[MAX_REGFIELDS] = {
> /* INTERRUPT ENABLE */
> [INT_EN] = REG_FIELD(TM_INT_EN_OFF, 0, 0),
>
> + /* UPPER/LOWER TEMPERATURE THRESHOLDS */
> + REG_FIELD_FOR_EACH_SENSOR11(LOW_THRESH, TM_Sn_UPPER_LOWER_STATUS_CTRL_OFF, 0, 9),
> + REG_FIELD_FOR_EACH_SENSOR11(UP_THRESH, TM_Sn_UPPER_LOWER_STATUS_CTRL_OFF, 10, 19),
> +
> + /* UPPER/LOWER INTERRUPTS [CLEAR/STATUS] */
> + REG_FIELD_FOR_EACH_SENSOR11(LOW_INT_CLEAR, TM_Sn_UPPER_LOWER_STATUS_CTRL_OFF, 20, 20),
> + REG_FIELD_FOR_EACH_SENSOR11(UP_INT_CLEAR, TM_Sn_UPPER_LOWER_STATUS_CTRL_OFF, 21, 21),
> + [LOW_INT_STATUS_0] = REG_FIELD(TM_HIGH_LOW_INT_STATUS_OFF, 0, 0),
> + [LOW_INT_STATUS_1] = REG_FIELD(TM_HIGH_LOW_INT_STATUS_OFF, 1, 1),
> + [LOW_INT_STATUS_2] = REG_FIELD(TM_HIGH_LOW_INT_STATUS_OFF, 2, 2),
> + [LOW_INT_STATUS_3] = REG_FIELD(TM_HIGH_LOW_INT_STATUS_OFF, 3, 3),
> + [LOW_INT_STATUS_4] = REG_FIELD(TM_HIGH_LOW_INT_STATUS_OFF, 4, 4),
> + [LOW_INT_STATUS_5] = REG_FIELD(TM_HIGH_LOW_INT_STATUS_OFF, 5, 5),
> + [LOW_INT_STATUS_6] = REG_FIELD(TM_HIGH_LOW_INT_STATUS_OFF, 6, 6),
> + [LOW_INT_STATUS_7] = REG_FIELD(TM_HIGH_LOW_INT_STATUS_OFF, 7, 7),
> + [UP_INT_STATUS_0] = REG_FIELD(TM_HIGH_LOW_INT_STATUS_OFF, 8, 8),
> + [UP_INT_STATUS_1] = REG_FIELD(TM_HIGH_LOW_INT_STATUS_OFF, 9, 9),
> + [UP_INT_STATUS_2] = REG_FIELD(TM_HIGH_LOW_INT_STATUS_OFF, 10, 10),
> + [UP_INT_STATUS_3] = REG_FIELD(TM_HIGH_LOW_INT_STATUS_OFF, 11, 11),
> + [UP_INT_STATUS_4] = REG_FIELD(TM_HIGH_LOW_INT_STATUS_OFF, 12, 12),
> + [UP_INT_STATUS_5] = REG_FIELD(TM_HIGH_LOW_INT_STATUS_OFF, 13, 13),
> + [UP_INT_STATUS_6] = REG_FIELD(TM_HIGH_LOW_INT_STATUS_OFF, 14, 14),
> + [UP_INT_STATUS_7] = REG_FIELD(TM_HIGH_LOW_INT_STATUS_OFF, 15, 15),
> +
> + /* NO CRITICAL INTERRUPT SUPPORT on v1 */
> +
> /* Sn_STATUS */
> REG_FIELD_FOR_EACH_SENSOR11(LAST_TEMP, TM_Sn_STATUS_OFF, 0, 9),
> REG_FIELD_FOR_EACH_SENSOR11(VALID, TM_Sn_STATUS_OFF, 14, 14),
> + /* xxx_STATUS bits: 1 == threshold violated */
> REG_FIELD_FOR_EACH_SENSOR11(MIN_STATUS, TM_Sn_STATUS_OFF, 10, 10),
> REG_FIELD_FOR_EACH_SENSOR11(LOWER_STATUS, TM_Sn_STATUS_OFF, 11, 11),
> REG_FIELD_FOR_EACH_SENSOR11(UPPER_STATUS, TM_Sn_STATUS_OFF, 12, 12),
> diff --git a/drivers/thermal/qcom/tsens-v2.c b/drivers/thermal/qcom/tsens-v2.c
> index 0a4f2b8fcab6c..a4d15e1abfddd 100644
> --- a/drivers/thermal/qcom/tsens-v2.c
> +++ b/drivers/thermal/qcom/tsens-v2.c
> @@ -50,9 +50,22 @@ static const struct reg_field tsens_v2_regfields[MAX_REGFIELDS] = {
> /* v2 has separate enables for UPPER/LOWER/CRITICAL interrupts */
> [INT_EN] = REG_FIELD(TM_INT_EN_OFF, 0, 2),
>
> + /* TEMPERATURE THRESHOLDS */
> + REG_FIELD_FOR_EACH_SENSOR16(LOW_THRESH, TM_Sn_UPPER_LOWER_THRESHOLD_OFF, 0, 11),
> + REG_FIELD_FOR_EACH_SENSOR16(UP_THRESH, TM_Sn_UPPER_LOWER_THRESHOLD_OFF, 12, 23),
> +
> + /* INTERRUPTS [CLEAR/STATUS/MASK] */
> + REG_FIELD_SPLIT_BITS_0_15(LOW_INT_STATUS, TM_UPPER_LOWER_INT_STATUS_OFF),
> + REG_FIELD_SPLIT_BITS_0_15(LOW_INT_CLEAR, TM_UPPER_LOWER_INT_CLEAR_OFF),
> + REG_FIELD_SPLIT_BITS_0_15(LOW_INT_MASK, TM_UPPER_LOWER_INT_MASK_OFF),
> + REG_FIELD_SPLIT_BITS_16_31(UP_INT_STATUS, TM_UPPER_LOWER_INT_STATUS_OFF),
> + REG_FIELD_SPLIT_BITS_16_31(UP_INT_CLEAR, TM_UPPER_LOWER_INT_CLEAR_OFF),
> + REG_FIELD_SPLIT_BITS_16_31(UP_INT_MASK, TM_UPPER_LOWER_INT_MASK_OFF),
> +
> /* Sn_STATUS */
> REG_FIELD_FOR_EACH_SENSOR16(LAST_TEMP, TM_Sn_STATUS_OFF, 0, 11),
> REG_FIELD_FOR_EACH_SENSOR16(VALID, TM_Sn_STATUS_OFF, 21, 21),
> + /* xxx_STATUS bits: 1 == threshold violated */
> REG_FIELD_FOR_EACH_SENSOR16(MIN_STATUS, TM_Sn_STATUS_OFF, 16, 16),
> REG_FIELD_FOR_EACH_SENSOR16(LOWER_STATUS, TM_Sn_STATUS_OFF, 17, 17),
> REG_FIELD_FOR_EACH_SENSOR16(UPPER_STATUS, TM_Sn_STATUS_OFF, 18, 18),
> diff --git a/drivers/thermal/qcom/tsens.c b/drivers/thermal/qcom/tsens.c
> index 772aa76b50e12..a4335717aeede 100644
> --- a/drivers/thermal/qcom/tsens.c
> +++ b/drivers/thermal/qcom/tsens.c
> @@ -7,6 +7,7 @@
> #include <linux/err.h>
> #include <linux/module.h>
> #include <linux/of.h>
> +#include <linux/of_platform.h>
> #include <linux/platform_device.h>
> #include <linux/pm.h>
> #include <linux/slab.h>
> @@ -78,12 +79,14 @@ MODULE_DEVICE_TABLE(of, tsens_table);
> static const struct thermal_zone_of_device_ops tsens_of_ops = {
> .get_temp = tsens_get_temp,
> .get_trend = tsens_get_trend,
> + .set_trips = tsens_set_trips,
> };
>
> static int tsens_register(struct tsens_priv *priv)
> {
> - int i;
> + int i, ret, irq;
> struct thermal_zone_device *tzd;
> + struct platform_device *pdev;
>
> for (i = 0; i < priv->num_sensors; i++) {
> priv->sensor[i].priv = priv;
> @@ -96,7 +99,33 @@ static int tsens_register(struct tsens_priv *priv)
> if (priv->ops->enable)
> priv->ops->enable(priv, i);
> }
> +
> + pdev = of_find_device_by_node(priv->dev->of_node);
> + if (!pdev) {
> + dev_err(&pdev->dev, "%s: device node not found in DT\n", __func__);
> + return -ENODEV;
> + }
> +
> + irq = platform_get_irq_byname(pdev, "uplow");
> + if (irq < 0) {
> + dev_err(&pdev->dev, "%s: missing irq in dt: uplow\n", __func__);
> + return irq;
> + }
> +
> + ret = devm_request_threaded_irq(&pdev->dev, irq,
> + NULL, tsens_irq_thread,
> + IRQF_TRIGGER_HIGH | IRQF_ONESHOT,
> + dev_name(&pdev->dev), priv);
> + if (ret) {
> + dev_err(&pdev->dev, "%s: failed to get irq\n", __func__);
> + goto err_put_device;
> + }
> + enable_irq_wake(irq);
> return 0;
> +
> +err_put_device:
> + put_device(&pdev->dev);
> + return ret;
> }
>
> static int tsens_probe(struct platform_device *pdev)
> @@ -178,6 +207,7 @@ static int tsens_remove(struct platform_device *pdev)
> struct tsens_priv *priv = platform_get_drvdata(pdev);
>
> debugfs_remove_recursive(priv->debug_root);
> + tsens_disable_irq(priv);
> if (priv->ops->disable)
> priv->ops->disable(priv);
>
> diff --git a/drivers/thermal/qcom/tsens.h b/drivers/thermal/qcom/tsens.h
> index 409395e436f32..b960c77168347 100644
> --- a/drivers/thermal/qcom/tsens.h
> +++ b/drivers/thermal/qcom/tsens.h
> @@ -13,8 +13,10 @@
> #define CAL_DEGC_PT2 120
> #define SLOPE_FACTOR 1000
> #define SLOPE_DEFAULT 3200
> +#define THRESHOLD_MAX_ADC_CODE 0x3ff
> +#define THRESHOLD_MIN_ADC_CODE 0x0
>
> -
> +#include <linux/interrupt.h>
> #include <linux/thermal.h>
> #include <linux/regmap.h>
>
> @@ -26,6 +28,11 @@ enum tsens_ver {
> VER_2_X,
> };
>
> +enum tsens_irq_type {
> + LOWER,
> + UPPER,
> +};
> +
> /**
> * struct tsens_sensor - data for each sensor connected to the tsens device
> * @priv: tsens device instance that this sensor is connected to
> @@ -99,22 +106,66 @@ struct tsens_ops {
> [_name##_##14] = REG_FIELD(_offset + 56, _startbit, _stopbit), \
> [_name##_##15] = REG_FIELD(_offset + 60, _startbit, _stopbit)
>
> -/* reg_field IDs to use as an index into an array */
> +#define REG_FIELD_SPLIT_BITS_0_15(_name, _offset) \
> + [_name##_##0] = REG_FIELD(_offset, 0, 0), \
> + [_name##_##1] = REG_FIELD(_offset, 1, 1), \
> + [_name##_##2] = REG_FIELD(_offset, 2, 2), \
> + [_name##_##3] = REG_FIELD(_offset, 3, 3), \
> + [_name##_##4] = REG_FIELD(_offset, 4, 4), \
> + [_name##_##5] = REG_FIELD(_offset, 5, 5), \
> + [_name##_##6] = REG_FIELD(_offset, 6, 6), \
> + [_name##_##7] = REG_FIELD(_offset, 7, 7), \
> + [_name##_##8] = REG_FIELD(_offset, 8, 8), \
> + [_name##_##9] = REG_FIELD(_offset, 9, 9), \
> + [_name##_##10] = REG_FIELD(_offset, 10, 10), \
> + [_name##_##11] = REG_FIELD(_offset, 11, 11), \
> + [_name##_##12] = REG_FIELD(_offset, 12, 12), \
> + [_name##_##13] = REG_FIELD(_offset, 13, 13), \
> + [_name##_##14] = REG_FIELD(_offset, 14, 14), \
> + [_name##_##15] = REG_FIELD(_offset, 15, 15)
> +
> +#define REG_FIELD_SPLIT_BITS_16_31(_name, _offset) \
> + [_name##_##0] = REG_FIELD(_offset, 16, 16), \
> + [_name##_##1] = REG_FIELD(_offset, 17, 17), \
> + [_name##_##2] = REG_FIELD(_offset, 18, 18), \
> + [_name##_##3] = REG_FIELD(_offset, 19, 19), \
> + [_name##_##4] = REG_FIELD(_offset, 20, 20), \
> + [_name##_##5] = REG_FIELD(_offset, 21, 21), \
> + [_name##_##6] = REG_FIELD(_offset, 22, 22), \
> + [_name##_##7] = REG_FIELD(_offset, 23, 23), \
> + [_name##_##8] = REG_FIELD(_offset, 24, 24), \
> + [_name##_##9] = REG_FIELD(_offset, 25, 25), \
> + [_name##_##10] = REG_FIELD(_offset, 26, 26), \
> + [_name##_##11] = REG_FIELD(_offset, 27, 27), \
> + [_name##_##12] = REG_FIELD(_offset, 28, 28), \
> + [_name##_##13] = REG_FIELD(_offset, 29, 29), \
> + [_name##_##14] = REG_FIELD(_offset, 30, 30), \
> + [_name##_##15] = REG_FIELD(_offset, 31, 31)
> +
> +/*
> + * reg_field IDs to use as an index into an array
> + * If you change the order of the entries, check the devm_regmap_field_alloc()
> + * calls in init_common()
> + */
> enum regfield_ids {
> /* ----- SROT ------ */
> /* HW_VER */
> - VER_MAJOR = 0,
> + VER_MAJOR,
> VER_MINOR,
> VER_STEP,
> /* CTRL_OFFSET */
> - TSENS_EN = 3,
> + TSENS_EN,
> TSENS_SW_RST,
> SENSOR_EN,
> CODE_OR_TEMP,
>
> /* ----- TM ------ */
> + /* TRDY */
> + TRDY,
> + /* INTERRUPT ENABLE */
> + INT_EN, /* v2+ has separate enables for crit, upper and lower irq */
> /* STATUS */
> - LAST_TEMP_0 = 7, /* Last temperature reading */
> + LAST_TEMP_0, /* Last temperature reading */
> LAST_TEMP_1,
> LAST_TEMP_2,
> LAST_TEMP_3,
> @@ -130,7 +181,7 @@ enum regfield_ids {
> LAST_TEMP_13,
> LAST_TEMP_14,
> LAST_TEMP_15,
> - VALID_0 = 23, /* VALID reading or not */
> + VALID_0, /* VALID reading or not */
> VALID_1,
> VALID_2,
> VALID_3,
> @@ -146,38 +197,6 @@ enum regfield_ids {
> VALID_13,
> VALID_14,
> VALID_15,
> - MIN_STATUS_0, /* MIN threshold violated */
> - MIN_STATUS_1,
> - MIN_STATUS_2,
> - MIN_STATUS_3,
> - MIN_STATUS_4,
> - MIN_STATUS_5,
> - MIN_STATUS_6,
> - MIN_STATUS_7,
> - MIN_STATUS_8,
> - MIN_STATUS_9,
> - MIN_STATUS_10,
> - MIN_STATUS_11,
> - MIN_STATUS_12,
> - MIN_STATUS_13,
> - MIN_STATUS_14,
> - MIN_STATUS_15,
> - MAX_STATUS_0, /* MAX threshold violated */
> - MAX_STATUS_1,
> - MAX_STATUS_2,
> - MAX_STATUS_3,
> - MAX_STATUS_4,
> - MAX_STATUS_5,
> - MAX_STATUS_6,
> - MAX_STATUS_7,
> - MAX_STATUS_8,
> - MAX_STATUS_9,
> - MAX_STATUS_10,
> - MAX_STATUS_11,
> - MAX_STATUS_12,
> - MAX_STATUS_13,
> - MAX_STATUS_14,
> - MAX_STATUS_15,
> LOWER_STATUS_0, /* LOWER threshold violated */
> LOWER_STATUS_1,
> LOWER_STATUS_2,
> @@ -194,6 +213,70 @@ enum regfield_ids {
> LOWER_STATUS_13,
> LOWER_STATUS_14,
> LOWER_STATUS_15,
> + LOW_INT_STATUS_0, /* LOWER interrupt status */
> + LOW_INT_STATUS_1,
> + LOW_INT_STATUS_2,
> + LOW_INT_STATUS_3,
> + LOW_INT_STATUS_4,
> + LOW_INT_STATUS_5,
> + LOW_INT_STATUS_6,
> + LOW_INT_STATUS_7,
> + LOW_INT_STATUS_8,
> + LOW_INT_STATUS_9,
> + LOW_INT_STATUS_10,
> + LOW_INT_STATUS_11,
> + LOW_INT_STATUS_12,
> + LOW_INT_STATUS_13,
> + LOW_INT_STATUS_14,
> + LOW_INT_STATUS_15,
> + LOW_INT_CLEAR_0, /* LOWER interrupt clear */
> + LOW_INT_CLEAR_1,
> + LOW_INT_CLEAR_2,
> + LOW_INT_CLEAR_3,
> + LOW_INT_CLEAR_4,
> + LOW_INT_CLEAR_5,
> + LOW_INT_CLEAR_6,
> + LOW_INT_CLEAR_7,
> + LOW_INT_CLEAR_8,
> + LOW_INT_CLEAR_9,
> + LOW_INT_CLEAR_10,
> + LOW_INT_CLEAR_11,
> + LOW_INT_CLEAR_12,
> + LOW_INT_CLEAR_13,
> + LOW_INT_CLEAR_14,
> + LOW_INT_CLEAR_15,
> + LOW_INT_MASK_0, /* LOWER interrupt mask */
> + LOW_INT_MASK_1,
> + LOW_INT_MASK_2,
> + LOW_INT_MASK_3,
> + LOW_INT_MASK_4,
> + LOW_INT_MASK_5,
> + LOW_INT_MASK_6,
> + LOW_INT_MASK_7,
> + LOW_INT_MASK_8,
> + LOW_INT_MASK_9,
> + LOW_INT_MASK_10,
> + LOW_INT_MASK_11,
> + LOW_INT_MASK_12,
> + LOW_INT_MASK_13,
> + LOW_INT_MASK_14,
> + LOW_INT_MASK_15,
> + LOW_THRESH_0, /* LOWER threshold values */
> + LOW_THRESH_1,
> + LOW_THRESH_2,
> + LOW_THRESH_3,
> + LOW_THRESH_4,
> + LOW_THRESH_5,
> + LOW_THRESH_6,
> + LOW_THRESH_7,
> + LOW_THRESH_8,
> + LOW_THRESH_9,
> + LOW_THRESH_10,
> + LOW_THRESH_11,
> + LOW_THRESH_12,
> + LOW_THRESH_13,
> + LOW_THRESH_14,
> + LOW_THRESH_15,
> UPPER_STATUS_0, /* UPPER threshold violated */
> UPPER_STATUS_1,
> UPPER_STATUS_2,
> @@ -210,6 +293,70 @@ enum regfield_ids {
> UPPER_STATUS_13,
> UPPER_STATUS_14,
> UPPER_STATUS_15,
> + UP_INT_STATUS_0, /* UPPER interrupt status */
> + UP_INT_STATUS_1,
> + UP_INT_STATUS_2,
> + UP_INT_STATUS_3,
> + UP_INT_STATUS_4,
> + UP_INT_STATUS_5,
> + UP_INT_STATUS_6,
> + UP_INT_STATUS_7,
> + UP_INT_STATUS_8,
> + UP_INT_STATUS_9,
> + UP_INT_STATUS_10,
> + UP_INT_STATUS_11,
> + UP_INT_STATUS_12,
> + UP_INT_STATUS_13,
> + UP_INT_STATUS_14,
> + UP_INT_STATUS_15,
> + UP_INT_CLEAR_0, /* UPPER interrupt clear */
> + UP_INT_CLEAR_1,
> + UP_INT_CLEAR_2,
> + UP_INT_CLEAR_3,
> + UP_INT_CLEAR_4,
> + UP_INT_CLEAR_5,
> + UP_INT_CLEAR_6,
> + UP_INT_CLEAR_7,
> + UP_INT_CLEAR_8,
> + UP_INT_CLEAR_9,
> + UP_INT_CLEAR_10,
> + UP_INT_CLEAR_11,
> + UP_INT_CLEAR_12,
> + UP_INT_CLEAR_13,
> + UP_INT_CLEAR_14,
> + UP_INT_CLEAR_15,
> + UP_INT_MASK_0, /* UPPER interrupt mask */
> + UP_INT_MASK_1,
> + UP_INT_MASK_2,
> + UP_INT_MASK_3,
> + UP_INT_MASK_4,
> + UP_INT_MASK_5,
> + UP_INT_MASK_6,
> + UP_INT_MASK_7,
> + UP_INT_MASK_8,
> + UP_INT_MASK_9,
> + UP_INT_MASK_10,
> + UP_INT_MASK_11,
> + UP_INT_MASK_12,
> + UP_INT_MASK_13,
> + UP_INT_MASK_14,
> + UP_INT_MASK_15,
> + UP_THRESH_0, /* UPPER threshold values */
> + UP_THRESH_1,
> + UP_THRESH_2,
> + UP_THRESH_3,
> + UP_THRESH_4,
> + UP_THRESH_5,
> + UP_THRESH_6,
> + UP_THRESH_7,
> + UP_THRESH_8,
> + UP_THRESH_9,
> + UP_THRESH_10,
> + UP_THRESH_11,
> + UP_THRESH_12,
> + UP_THRESH_13,
> + UP_THRESH_14,
> + UP_THRESH_15,
> CRITICAL_STATUS_0, /* CRITICAL threshold violated */
> CRITICAL_STATUS_1,
> CRITICAL_STATUS_2,
> @@ -226,13 +373,38 @@ enum regfield_ids {
> CRITICAL_STATUS_13,
> CRITICAL_STATUS_14,
> CRITICAL_STATUS_15,
> - /* TRDY */
> - TRDY,
> - /* INTERRUPT ENABLE */
> - INT_EN, /* Pre-V1, V1.x */
> - LOW_INT_EN, /* V2.x */
> - UP_INT_EN, /* V2.x */
> - CRIT_INT_EN, /* V2.x */
> + MIN_STATUS_0, /* MIN threshold violated */
> + MIN_STATUS_1,
> + MIN_STATUS_2,
> + MIN_STATUS_3,
> + MIN_STATUS_4,
> + MIN_STATUS_5,
> + MIN_STATUS_6,
> + MIN_STATUS_7,
> + MIN_STATUS_8,
> + MIN_STATUS_9,
> + MIN_STATUS_10,
> + MIN_STATUS_11,
> + MIN_STATUS_12,
> + MIN_STATUS_13,
> + MIN_STATUS_14,
> + MIN_STATUS_15,
> + MAX_STATUS_0, /* MAX threshold violated */
> + MAX_STATUS_1,
> + MAX_STATUS_2,
> + MAX_STATUS_3,
> + MAX_STATUS_4,
> + MAX_STATUS_5,
> + MAX_STATUS_6,
> + MAX_STATUS_7,
> + MAX_STATUS_8,
> + MAX_STATUS_9,
> + MAX_STATUS_10,
> + MAX_STATUS_11,
> + MAX_STATUS_12,
> + MAX_STATUS_13,
> + MAX_STATUS_14,
> + MAX_STATUS_15,
>
> /* Keep last */
> MAX_REGFIELDS
> @@ -304,6 +476,10 @@ struct tsens_priv {
> struct regmap *tm_map;
> struct regmap *srot_map;
> u32 tm_offset;
> +
> + /* lock for upper/lower threshold interrupts */
> + spinlock_t ul_lock;
> +
> struct regmap_field *rf[MAX_REGFIELDS];
> struct tsens_context ctx;
> const struct tsens_features *feat;
> @@ -321,6 +497,10 @@ void compute_intercept_slope(struct tsens_priv *priv, u32 *pt1, u32 *pt2, u32 mo
> int init_common(struct tsens_priv *priv);
> int get_temp_tsens_valid(struct tsens_sensor *s, int *temp);
> int get_temp_common(struct tsens_sensor *s, int *temp);
> +int tsens_enable_irq(struct tsens_priv *priv);
> +void tsens_disable_irq(struct tsens_priv *priv);
> +int tsens_set_trips(void *_sensor, int low, int high);
> +irqreturn_t tsens_irq_thread(int irq, void *data);
>
> /* TSENS target */
> extern const struct tsens_plat_data data_8960;
>


--
Regards
Thara

2019-08-29 08:49:44

by Amit Kucheria

[permalink] [raw]
Subject: Re: [PATCH v2 07/15] dt: thermal: tsens: Document interrupt support in tsens driver

On Wed, Aug 28, 2019 at 6:03 AM Stephen Boyd <[email protected]> wrote:
>
> Quoting Amit Kucheria (2019-08-27 05:14:03)
> > Define two new required properties to define interrupts and
> > interrupt-names for tsens.
> >
> > Signed-off-by: Amit Kucheria <[email protected]>
> > ---
> > Documentation/devicetree/bindings/thermal/qcom-tsens.txt | 8 ++++++++
> > 1 file changed, 8 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/thermal/qcom-tsens.txt b/Documentation/devicetree/bindings/thermal/qcom-tsens.txt
> > index 673cc1831ee9d..686bede72f846 100644
> > --- a/Documentation/devicetree/bindings/thermal/qcom-tsens.txt
> > +++ b/Documentation/devicetree/bindings/thermal/qcom-tsens.txt
> > @@ -22,6 +22,8 @@ Required properties:
> >
> > - #thermal-sensor-cells : Should be 1. See ./thermal.txt for a description.
> > - #qcom,sensors: Number of sensors in tsens block
> > +- interrupts: Interrupts generated from Always-On subsystem (AOSS)
>
> Is it always one? interrupt-names makes it sound like it.
>
> > +- interrupt-names: Must be one of the following: "uplow", "critical"

Will fix to "one or more of the following"

> > - Refer to Documentation/devicetree/bindings/nvmem/nvmem.txt to know how to specify
> > nvmem cells
> >

2019-08-29 12:33:58

by Amit Kucheria

[permalink] [raw]
Subject: Re: [PATCH v2 15/15] drivers: thermal: tsens: Add interrupt support

On Thu, Aug 29, 2019 at 3:12 AM Stephen Boyd <[email protected]> wrote:
>
> Quoting Amit Kucheria (2019-08-27 05:14:11)
> > diff --git a/drivers/thermal/qcom/tsens-common.c b/drivers/thermal/qcom/tsens-common.c
> > index 06b44cfd5eab9..c549f8e1488ba 100644
> > --- a/drivers/thermal/qcom/tsens-common.c
> > +++ b/drivers/thermal/qcom/tsens-common.c
> > @@ -114,6 +146,314 @@ static int tsens_hw_to_mC(struct tsens_sensor *s, int field)
> > return sign_extend32(temp, priv->tempres) * 100;
> > }
> >
> > +/**
> > + * tsens_mC_to_hw - Return correct value to be written to threshold
> > + * registers, whether in ADC code or deciCelsius depending on IP version
>
> Document arguments and return value? Maybe summary can be 'convert
> tsens temperature to hardware register value'?

Fixed.

> > + */
> > +static int tsens_mC_to_hw(struct tsens_sensor *s, int temp)
> > +{
> > + struct tsens_priv *priv = s->priv;
> > +
> > + if (priv->feat->adc) {
> > + /* milliC to C to adc code */
> > + return degc_to_code(temp / 1000, s);
> > + }
> > +
> > + /* milliC to deciC */
> > + return temp / 100;
> > +}
> > +
> > +static inline unsigned int tsens_ver(struct tsens_priv *priv)
>
> Can this return the enum instead of unsigned int?

Fixed.

> > +{
> > + return priv->feat->ver_major;
> > +}
> > +
> > +/**
> > + * tsens_set_interrupt_v1 - Disable an interrupt (enable = false)
> > + * Re-enable an interrupt (enable = true)
> > + */
> > +static void tsens_set_interrupt_v1(struct tsens_priv *priv, u32 hw_id,
> > + enum tsens_irq_type irq_type, bool enable)
> > +{
> > + u32 index;
> > +
> > + if (enable) {
> > + switch (irq_type) {
> > + case UPPER:
> > + index = UP_INT_CLEAR_0 + hw_id;
> > + break;
> > + case LOWER:
> > + index = LOW_INT_CLEAR_0 + hw_id;
> > + break;
> > + }
> > + regmap_field_write(priv->rf[index], 0);
> > + } else {
> > + switch (irq_type) {
> > + case UPPER:
> > + index = UP_INT_CLEAR_0 + hw_id;
> > + break;
> > + case LOWER:
> > + index = LOW_INT_CLEAR_0 + hw_id;
> > + break;
> > + }
> > + regmap_field_write(priv->rf[index], 1);
> > + }
>
> De-dup the switch statement and have
>
> regmap_field_write(priv->rf[index], enable ? 1 : 0);

Fixed.

> > +}
> > +
> > +/**
> > + * tsens_set_interrupt_v2 - Disable an interrupt (enable = false)
> > + * Re-enable an interrupt (enable = true)
> > + */
> > +static void tsens_set_interrupt_v2(struct tsens_priv *priv, u32 hw_id,
> > + enum tsens_irq_type irq_type, bool enable)
> > +{
> > + u32 index_mask, index_clear;
> > +
> > + if (enable) {
> > + switch (irq_type) {
> > + case UPPER:
> > + index_mask = UP_INT_MASK_0 + hw_id;
> > + break;
> > + case LOWER:
> > + index_mask = LOW_INT_MASK_0 + hw_id;
> > + break;
> > + }
> > + regmap_field_write(priv->rf[index_mask], 0);
> > + } else {
> > + /* To disable the interrupt flag for a sensor:
>
> Nitpick: Wrong comment style.
>

Fixed.

> > + * 1. Mask further interrupts for this sensor
> > + * 2. Write 1 followed by 0 to clear the interrupt
> > + */
> > + switch (irq_type) {
> > + case UPPER:
> > + index_mask = UP_INT_MASK_0 + hw_id;
> > + index_clear = UP_INT_CLEAR_0 + hw_id;
> > + break;
> > + case LOWER:
> > + index_mask = LOW_INT_MASK_0 + hw_id;
> > + index_clear = LOW_INT_CLEAR_0 + hw_id;
> > + break;
> > + }
>
> Please extract index_mask and index_clear assignments to one switch
> statement and then change the sequence to an if/else
>
> if (enable) {
> regmap_field_write(priv->rf[index_mask], 1);
> regmap_field_write(priv->rf[index_clear], 1);
> regmap_field_write(priv->rf[index_clear], 0);
> } else {
> regmap_field_write(priv->rf[index_mask], 0);
> }


Fixed.

> > +}
> > +
> > +/**
> > + * tsens_set_interrupt - Disable an interrupt (enable = false)
> > + * Re-enable an interrupt (enable = true)
> > + */
> > +static void tsens_set_interrupt(struct tsens_priv *priv, u32 hw_id,
> > + enum tsens_irq_type irq_type, bool enable)
> > +{
> > + dev_dbg(priv->dev, "[%u] %s: %s -> %s\n", hw_id, __func__,
> > + irq_type ? ((irq_type == 1) ? "UP" : "CRITICAL") : "LOW",
> > + enable ? "en" : "dis");
> > + if (tsens_ver(priv) > VER_1_X)
> > + tsens_set_interrupt_v2(priv, hw_id, irq_type, enable);
> > + else
> > + tsens_set_interrupt_v1(priv, hw_id, irq_type, enable);
> > +}
> > +
> > +/**
> > + * tsens_threshold_violated - Check if a sensor temperature violated a preset threshold
> > + *
>
> Document arguments?

Fixed.

> > + * Return: 0 if threshold was not violated, 1 if it was violated and negative
> > + * errno in case of errors
> > + */
> > +static int tsens_threshold_violated(struct tsens_priv *priv, u32 hw_id,
> > + struct tsens_irq_data *d)
> > +{
> > + int ret;
> > +
> > + ret = regmap_field_read(priv->rf[UPPER_STATUS_0 + hw_id], &d->up_viol);
> > + if (ret)
> > + return ret;
> > + ret = regmap_field_read(priv->rf[LOWER_STATUS_0 + hw_id], &d->low_viol);
> > + if (ret)
> > + return ret;
> > + if (d->up_viol || d->low_viol)
> > + return 1;
> > +
> > + return 0;
> > +}
> > +
> > +static int tsens_read_irq_state(struct tsens_priv *priv, u32 hw_id,
> > + struct tsens_sensor *s, struct tsens_irq_data *d)
> > +{
> > + int ret;
> > +
> > + ret = regmap_field_read(priv->rf[UP_INT_CLEAR_0 + hw_id], &d->up_irq_clear);
> > + if (ret)
> > + return ret;
> > + ret = regmap_field_read(priv->rf[LOW_INT_CLEAR_0 + hw_id], &d->low_irq_clear);
> > + if (ret)
> > + return ret;
> > + if (tsens_ver(priv) > VER_1_X) {
> > + ret = regmap_field_read(priv->rf[UP_INT_MASK_0 + hw_id], &d->up_irq_mask);
> > + if (ret)
> > + return ret;
> > + ret = regmap_field_read(priv->rf[LOW_INT_MASK_0 + hw_id], &d->low_irq_mask);
> > + if (ret)
> > + return ret;
> > + } else {
> > + /* No mask register on older TSENS */
> > + d->up_irq_mask = 0;
> > + d->low_irq_mask = 0;
> > + }
> > +
> > + d->up_thresh = tsens_hw_to_mC(s, UP_THRESH_0 + hw_id);
> > + d->low_thresh = tsens_hw_to_mC(s, LOW_THRESH_0 + hw_id);
> > +
> > + dev_dbg(priv->dev, "[%u] %s%s: status(%u|%u) | clr(%u|%u) | mask(%u|%u)\n",
> > + hw_id, __func__, (d->up_viol || d->low_viol) ? "(V)" : "",
> > + d->low_viol, d->up_viol, d->low_irq_clear, d->up_irq_clear,
> > + d->low_irq_mask, d->up_irq_mask);
> > + dev_dbg(priv->dev, "[%u] %s%s: thresh: (%d:%d)\n", hw_id, __func__,
> > + (d->up_viol || d->low_viol) ? "(violation)" : "",
> > + d->low_thresh, d->up_thresh);
> > +
> > + return 0;
> > +}
> > +
> > +static inline u32 masked_irq(u32 hw_id, u32 mask, enum tsens_ver ver)
> > +{
> > + if (ver > VER_1_X)
> > + return mask & (1 << hw_id);
> > +
> > + /* v1, v0.1 don't have a irq mask register */
> > + return 0;
> > +}
> > +
> > +irqreturn_t tsens_irq_thread(int irq, void *data)
> > +{
> > + struct tsens_priv *priv = data;
> > + struct tsens_irq_data d;
> > + bool enable = true, disable = false;
> > + unsigned long flags;
> > + int temp, ret, i;
> > +
> > + /*
> > + * Check if any sensor raised an IRQ - for each sensor connected to the
> > + * TSENS block if it set the threshold violation bit.
> > + */
> > + for (i = 0; i < priv->num_sensors; i++) {
> > + bool trigger = 0;
>
> How about trigger = false? It's a bool.

Fixed.

> > + struct tsens_sensor *s = &priv->sensor[i];
> > + u32 hw_id = s->hw_id;
> > +
> > + if (IS_ERR(priv->sensor[i].tzd))
> > + continue;
> > + if (!tsens_threshold_violated(priv, hw_id, &d))
> > + continue;
> > + ret = get_temp_tsens_valid(s, &temp);
> > + if (ret) {
> > + dev_err(priv->dev, "[%u] %s: error reading sensor\n", hw_id, __func__);
>
> I hope there isn't an interrupt storm where we're trying to print out
> messages from the irq handler.
>
> > + continue;
> > + }
> > +
> > + spin_lock_irqsave(&priv->ul_lock, flags);
> > +
> > + tsens_read_irq_state(priv, hw_id, s, &d);
> > +
> > + if (d.up_viol &&
> > + !masked_irq(hw_id, d.up_irq_mask, tsens_ver(priv))) {
> > + tsens_set_interrupt(priv, hw_id, UPPER, disable);
> > + if (d.up_thresh > temp) {
> > + dev_dbg(priv->dev, "[%u] %s: re-arm upper\n",
> > + priv->sensor[i].hw_id, __func__);
> > + /* unmask the interrupt for this sensor */
> > + tsens_set_interrupt(priv, hw_id, UPPER, enable);
> > + } else {
> > + trigger = 1;
> > + /* Keep irq masked */
> > + }
> > + } else if (d.low_viol &&
> > + !masked_irq(hw_id, d.low_irq_mask, tsens_ver(priv))) {
> > + tsens_set_interrupt(priv, hw_id, LOWER, disable);
> > + if (d.low_thresh < temp) {
> > + dev_dbg(priv->dev, "[%u] %s: re-arm low\n",
> > + priv->sensor[i].hw_id, __func__);
> > + /* unmask the interrupt for this sensor */
> > + tsens_set_interrupt(priv, hw_id, LOWER, enable);
> > + } else {
> > + trigger = 1;
> > + /* Keep irq masked */
> > + }
> > + }
> > +
> > + spin_unlock_irqrestore(&priv->ul_lock, flags);
> > +
> > + if (trigger) {
> > + dev_dbg(priv->dev, "[%u] %s: TZ update trigger (%d mC)\n",
> > + hw_id, __func__, temp);
> > + thermal_zone_device_update(priv->sensor[i].tzd,
> > + THERMAL_EVENT_UNSPECIFIED);
> > + } else {
> > + dev_dbg(priv->dev, "[%u] %s: no violation: %d\n",
> > + hw_id, __func__, temp);
> > + }
> > + }
> > +
> > + return IRQ_HANDLED;
>
> Should we return IRQ_NONE in the case that the above for loop didn't
> find anything in those if/else-ifs?

The upper/lower interrupts are non-sticky, level-triggered. So if the
temp returns to within the thresholds in the time that a IRQ was
triggered and the handler scheduled, we might not see any threshold
violations/interrupt bits set.

It feels to me that this is a case of the IRQ being handled
(automagically) instead of IRQ_NONE. The definition of IRQ_NONE[1]
also seems to suggest that it should be used when the IRQ wasn't
handled. But it was handled in this case (although, automatically),
wasn't it?

[1] https://elixir.bootlin.com/linux/latest/source/include/linux/irqreturn.h#L7

> > +}
> > +
> > +int tsens_set_trips(void *_sensor, int low, int high)
> > +{
> > + struct tsens_sensor *s = _sensor;
> > + struct tsens_priv *priv = s->priv;
> > + struct device *dev = priv->dev;
> > + struct tsens_irq_data d;
> > + unsigned long flags;
> > + int high_val, low_val, cl_high, cl_low;
> > + bool enable = true;
> > + u32 hw_id = s->hw_id;
> > +
> > + dev_dbg(dev, "[%u] %s: proposed thresholds: (%d:%d)\n",
> > + hw_id, __func__, low, high);
> > +
> > + cl_high = clamp_val(high, -40000, 120000);
> > + cl_low = clamp_val(low, -40000, 120000);
> > +
> > + high_val = tsens_mC_to_hw(s, cl_high);
> > + low_val = tsens_mC_to_hw(s, cl_low);
> > +
> > + spin_lock_irqsave(&priv->ul_lock, flags);
> > +
> > + tsens_read_irq_state(priv, hw_id, s, &d);
> > +
> > + /* Write the new thresholds and clear the status */
> > + regmap_field_write(priv->rf[LOW_THRESH_0 + hw_id], low_val);
> > + regmap_field_write(priv->rf[UP_THRESH_0 + hw_id], high_val);
> > + tsens_set_interrupt(priv, hw_id, LOWER, enable);
> > + tsens_set_interrupt(priv, hw_id, UPPER, enable);
>
> Just pass true? Why is there an enable local variable?

Good catch. Left over from a refactor with more convoluted logic.

> > +
> > + spin_unlock_irqrestore(&priv->ul_lock, flags);
> > +
> > + dev_dbg(dev, "[%u] %s: (%d:%d)->(%d:%d)\n",
> > + s->hw_id, __func__, d.low_thresh, d.up_thresh, cl_low, cl_high);
> > +
> > + return 0;
> > +}
> > +
> [...]
> > @@ -319,28 +659,31 @@ int __init init_common(struct tsens_priv *priv)
> > ret = PTR_ERR(priv->rf[SENSOR_EN]);
> > goto err_put_device;
> > }
> > - /* now alloc regmap_fields in tm_map */
> > - for (i = 0, j = LAST_TEMP_0; i < priv->feat->max_sensors; i++, j++) {
> > - priv->rf[j] = devm_regmap_field_alloc(dev, priv->tm_map,
> > - priv->fields[j]);
> > - if (IS_ERR(priv->rf[j])) {
> > - ret = PTR_ERR(priv->rf[j]);
> > - goto err_put_device;
> > - }
> > + priv->rf[INT_EN] = devm_regmap_field_alloc(dev, priv->tm_map,
> > + priv->fields[INT_EN]);
> > + if (IS_ERR(priv->rf[INT_EN])) {
> > + ret = PTR_ERR(priv->rf[INT_EN]);
> > + goto err_put_device;
> > }
> >
> > - /* Save away resolution of signed temperature value for this IP */
> > - priv->tempres = priv->fields[LAST_TEMP_0].msb - priv->fields[LAST_TEMP_0].lsb;
> > -
> > - for (i = 0, j = VALID_0; i < priv->feat->max_sensors; i++, j++) {
> > - priv->rf[j] = devm_regmap_field_alloc(dev, priv->tm_map,
> > - priv->fields[j]);
> > - if (IS_ERR(priv->rf[j])) {
> > - ret = PTR_ERR(priv->rf[j]);
> > - 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++) {
> > + int idx = j + i;
> > +
> > + priv->rf[idx] = devm_regmap_field_alloc(dev, priv->tm_map,
> > + priv->fields[idx]);
> > + if (IS_ERR(priv->rf[idx])) {
> > + ret = PTR_ERR(priv->rf[idx]);
> > + goto err_put_device;
> > + }
> > }
> > }
> > + /* Save away resolution of signed temperature value for this IP */
> > + priv->tempres = priv->fields[LAST_TEMP_0].msb - priv->fields[LAST_TEMP_0].lsb;
>
> Leave this where it was, i.e. before the for loop? Or is that a bug and
> it doesn't actually work unless it's after the for loop? In which case,
> this should go to the previous patch.

I've gotten rid of this completely now.

> >
> > + spin_lock_init(&priv->ul_lock);
> > + tsens_enable_irq(priv);
> > tsens_debug_init(op);
> >
> > return 0;
> > diff --git a/drivers/thermal/qcom/tsens.c b/drivers/thermal/qcom/tsens.c
> > index 772aa76b50e12..a4335717aeede 100644
> > --- a/drivers/thermal/qcom/tsens.c
> > +++ b/drivers/thermal/qcom/tsens.c
> > @@ -96,7 +99,33 @@ static int tsens_register(struct tsens_priv *priv)
> > if (priv->ops->enable)
> > priv->ops->enable(priv, i);
> > }
> > +
> > + pdev = of_find_device_by_node(priv->dev->of_node);
> > + if (!pdev) {
> > + dev_err(&pdev->dev, "%s: device node not found in DT\n", __func__);
>
> Do we really need this? Maybe just bail out in silence because this
> should never happen?

Fixed.

> > + return -ENODEV;
> > + }
> > +
> > + irq = platform_get_irq_byname(pdev, "uplow");
> > + if (irq < 0) {
> > + dev_err(&pdev->dev, "%s: missing irq in dt: uplow\n", __func__);
>
> You can drop the error print. I upstreamed a change to print the error
> generically in the core.

Fixed.

> > + return irq;
>
> Did we need to put_device() here?

Refactored to goto err_put_device


> > + }
> > +
> > + ret = devm_request_threaded_irq(&pdev->dev, irq,
> > + NULL, tsens_irq_thread,
> > + IRQF_TRIGGER_HIGH | IRQF_ONESHOT,
> > + dev_name(&pdev->dev), priv);
> > + if (ret) {
> > + dev_err(&pdev->dev, "%s: failed to get irq\n", __func__);
> > + goto err_put_device;
> > + }
> > + enable_irq_wake(irq);
> > return 0;
> > +
> > +err_put_device:
> > + put_device(&pdev->dev);
> > + return ret;

2019-08-29 14:09:16

by Daniel Thompson

[permalink] [raw]
Subject: Re: [PATCH v2 03/15] drivers: thermal: tsens: Add __func__ identifier to debug statements

On Tue, Aug 27, 2019 at 05:43:59PM +0530, Amit Kucheria wrote:
> Printing the function name when enabling debugging makes logs easier to
> read.
>
> Signed-off-by: Amit Kucheria <[email protected]>
> Reviewed-by: Stephen Boyd <[email protected]>
> Reviewed-by: Daniel Lezcano <[email protected]>

This should need to be manually added at each call site; it is already
built into the logging system (the f flag for dynamic debug)?


Daniel.

> ---
> drivers/thermal/qcom/tsens-common.c | 8 ++++----
> drivers/thermal/qcom/tsens.c | 6 +++---
> 2 files changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/thermal/qcom/tsens-common.c b/drivers/thermal/qcom/tsens-common.c
> index c037bdf92c663..7437bfe196e50 100644
> --- a/drivers/thermal/qcom/tsens-common.c
> +++ b/drivers/thermal/qcom/tsens-common.c
> @@ -42,8 +42,8 @@ void compute_intercept_slope(struct tsens_priv *priv, u32 *p1,
>
> for (i = 0; i < priv->num_sensors; i++) {
> dev_dbg(priv->dev,
> - "sensor%d - data_point1:%#x data_point2:%#x\n",
> - i, p1[i], p2[i]);
> + "%s: sensor%d - data_point1:%#x data_point2:%#x\n",
> + __func__, i, p1[i], p2[i]);
>
> priv->sensor[i].slope = SLOPE_DEFAULT;
> if (mode == TWO_PT_CALIB) {
> @@ -60,7 +60,7 @@ void compute_intercept_slope(struct tsens_priv *priv, u32 *p1,
> priv->sensor[i].offset = (p1[i] * SLOPE_FACTOR) -
> (CAL_DEGC_PT1 *
> priv->sensor[i].slope);
> - dev_dbg(priv->dev, "offset:%d\n", priv->sensor[i].offset);
> + dev_dbg(priv->dev, "%s: offset:%d\n", __func__, priv->sensor[i].offset);
> }
> }
>
> @@ -209,7 +209,7 @@ int __init init_common(struct tsens_priv *priv)
> if (ret)
> goto err_put_device;
> if (!enabled) {
> - dev_err(dev, "tsens device is not enabled\n");
> + dev_err(dev, "%s: device not enabled\n", __func__);
> ret = -ENODEV;
> goto err_put_device;
> }
> diff --git a/drivers/thermal/qcom/tsens.c b/drivers/thermal/qcom/tsens.c
> index 542a7f8c3d962..06c6bbd69a1a7 100644
> --- a/drivers/thermal/qcom/tsens.c
> +++ b/drivers/thermal/qcom/tsens.c
> @@ -127,7 +127,7 @@ static int tsens_probe(struct platform_device *pdev)
> of_property_read_u32(np, "#qcom,sensors", &num_sensors);
>
> if (num_sensors <= 0) {
> - dev_err(dev, "invalid number of sensors\n");
> + dev_err(dev, "%s: invalid number of sensors\n", __func__);
> return -EINVAL;
> }
>
> @@ -156,7 +156,7 @@ static int tsens_probe(struct platform_device *pdev)
>
> ret = priv->ops->init(priv);
> if (ret < 0) {
> - dev_err(dev, "tsens init failed\n");
> + dev_err(dev, "%s: init failed\n", __func__);
> return ret;
> }
>
> @@ -164,7 +164,7 @@ static int tsens_probe(struct platform_device *pdev)
> ret = priv->ops->calibrate(priv);
> if (ret < 0) {
> if (ret != -EPROBE_DEFER)
> - dev_err(dev, "tsens calibration failed\n");
> + dev_err(dev, "%s: calibration failed\n", __func__);
> return ret;
> }
> }
> --
> 2.17.1
>

2019-08-29 14:30:29

by Amit Kucheria

[permalink] [raw]
Subject: Re: [PATCH v2 03/15] drivers: thermal: tsens: Add __func__ identifier to debug statements

On Thu, Aug 29, 2019 at 7:35 PM Daniel Thompson
<[email protected]> wrote:
>
> On Tue, Aug 27, 2019 at 05:43:59PM +0530, Amit Kucheria wrote:
> > Printing the function name when enabling debugging makes logs easier to
> > read.
> >
> > Signed-off-by: Amit Kucheria <[email protected]>
> > Reviewed-by: Stephen Boyd <[email protected]>
> > Reviewed-by: Daniel Lezcano <[email protected]>
>
> This should need to be manually added at each call site; it is already
> built into the logging system (the f flag for dynamic debug)?

I assume you meant "shouldn't".

I haven't yet integrated dynamic debug into my daily workflow.

Last time I looked at it, it was a bit bothersome to use because I
needed to lookup exact line numbers to trigger useful information. And
those line numbers constantly keep changing as I work on the driver,
so it was a bit painful to script. Not to mention the syntax to frob
the correct files in debugfs to enable this functionality.

As opposed to this, adding the following to the makefile is so easy. :-)

CFLAGS_tsens-common.o := -DDEBUG

Perhaps I am using it all wrong? How would I go about using dynamic
debug instead of this patch?

Regards,
Amit

2019-08-29 14:54:46

by Stephen Boyd

[permalink] [raw]
Subject: Re: [PATCH v2 07/15] dt: thermal: tsens: Document interrupt support in tsens driver

Quoting Amit Kucheria (2019-08-29 01:48:27)
> On Wed, Aug 28, 2019 at 6:03 AM Stephen Boyd <[email protected]> wrote:
> >
> > Quoting Amit Kucheria (2019-08-27 05:14:03)
> > > Define two new required properties to define interrupts and
> > > interrupt-names for tsens.
> > >
> > > Signed-off-by: Amit Kucheria <[email protected]>
> > > ---
> > > Documentation/devicetree/bindings/thermal/qcom-tsens.txt | 8 ++++++++
> > > 1 file changed, 8 insertions(+)
> > >
> > > diff --git a/Documentation/devicetree/bindings/thermal/qcom-tsens.txt b/Documentation/devicetree/bindings/thermal/qcom-tsens.txt
> > > index 673cc1831ee9d..686bede72f846 100644
> > > --- a/Documentation/devicetree/bindings/thermal/qcom-tsens.txt
> > > +++ b/Documentation/devicetree/bindings/thermal/qcom-tsens.txt
> > > @@ -22,6 +22,8 @@ Required properties:
> > >
> > > - #thermal-sensor-cells : Should be 1. See ./thermal.txt for a description.
> > > - #qcom,sensors: Number of sensors in tsens block
> > > +- interrupts: Interrupts generated from Always-On subsystem (AOSS)
> >
> > Is it always one? interrupt-names makes it sound like it.
> >
> > > +- interrupt-names: Must be one of the following: "uplow", "critical"
>
> Will fix to "one or more of the following"
>

Can we get a known quantity of interrupts for a particular compatible
string instead? Let's be as specific as possible. The index matters too,
so please list them in the order that is desired.

2019-08-29 14:55:39

by Stephen Boyd

[permalink] [raw]
Subject: Re: [PATCH v2 15/15] drivers: thermal: tsens: Add interrupt support

Quoting Amit Kucheria (2019-08-29 05:30:59)
> On Thu, Aug 29, 2019 at 3:12 AM Stephen Boyd <[email protected]> wrote:
> >
> > Quoting Amit Kucheria (2019-08-27 05:14:11)
> > > + thermal_zone_device_update(priv->sensor[i].tzd,
> > > + THERMAL_EVENT_UNSPECIFIED);
> > > + } else {
> > > + dev_dbg(priv->dev, "[%u] %s: no violation: %d\n",
> > > + hw_id, __func__, temp);
> > > + }
> > > + }
> > > +
> > > + return IRQ_HANDLED;
> >
> > Should we return IRQ_NONE in the case that the above for loop didn't
> > find anything in those if/else-ifs?
>
> The upper/lower interrupts are non-sticky, level-triggered. So if the
> temp returns to within the thresholds in the time that a IRQ was
> triggered and the handler scheduled, we might not see any threshold
> violations/interrupt bits set.
>
> It feels to me that this is a case of the IRQ being handled
> (automagically) instead of IRQ_NONE. The definition of IRQ_NONE[1]
> also seems to suggest that it should be used when the IRQ wasn't
> handled. But it was handled in this case (although, automatically),
> wasn't it?

Ok I see. Sounds fine then to always return IRQ_HANDLED. Maybe you can
add a comment to this effect right above the return statement.

2019-08-29 15:20:29

by Daniel Thompson

[permalink] [raw]
Subject: Re: [PATCH v2 03/15] drivers: thermal: tsens: Add __func__ identifier to debug statements

On Thu, Aug 29, 2019 at 07:58:45PM +0530, Amit Kucheria wrote:
> On Thu, Aug 29, 2019 at 7:35 PM Daniel Thompson
> <[email protected]> wrote:
> >
> > On Tue, Aug 27, 2019 at 05:43:59PM +0530, Amit Kucheria wrote:
> > > Printing the function name when enabling debugging makes logs easier to
> > > read.
> > >
> > > Signed-off-by: Amit Kucheria <[email protected]>
> > > Reviewed-by: Stephen Boyd <[email protected]>
> > > Reviewed-by: Daniel Lezcano <[email protected]>
> >
> > This should need to be manually added at each call site; it is already
> > built into the logging system (the f flag for dynamic debug)?
>
> I assume you meant "shouldn't".

Quite so. Sorry about that.


> I haven't yet integrated dynamic debug into my daily workflow.
>
> Last time I looked at it, it was a bit bothersome to use because I
> needed to lookup exact line numbers to trigger useful information. And
> those line numbers constantly keep changing as I work on the driver,
> so it was a bit painful to script. Not to mention the syntax to frob
> the correct files in debugfs to enable this functionality.
>
> As opposed to this, adding the following to the makefile is so easy. :-)
>
> CFLAGS_tsens-common.o := -DDEBUG
>
> Perhaps I am using it all wrong? How would I go about using dynamic
> debug instead of this patch?

Throwing dyndbg="file <fname>.c +pf" onto the kernel command line is a
good start (+p enables debug level prints, +f causes messages to include
the function name).

When the C files map to module names (whether the modules are actually
built-in or not) then <module>.dyndbg=+pf is a bit cleaner and allows
you to debug the whole of a driver without how it is decomposed into
files.

There are (many) other controls to play with[1] but the above should be
sufficient to simulate -DDEBUG .


Daniel.

[1]
https://www.kernel.org/doc/html/latest/admin-guide/dynamic-debug-howto.html

2019-08-29 16:16:03

by Amit Kucheria

[permalink] [raw]
Subject: Re: [PATCH v2 03/15] drivers: thermal: tsens: Add __func__ identifier to debug statements

On Thu, Aug 29, 2019 at 8:49 PM Daniel Thompson
<[email protected]> wrote:
>
> On Thu, Aug 29, 2019 at 07:58:45PM +0530, Amit Kucheria wrote:
> > On Thu, Aug 29, 2019 at 7:35 PM Daniel Thompson
> > <[email protected]> wrote:
> > >
> > > On Tue, Aug 27, 2019 at 05:43:59PM +0530, Amit Kucheria wrote:
> > > > Printing the function name when enabling debugging makes logs easier to
> > > > read.
> > > >
> > > > Signed-off-by: Amit Kucheria <[email protected]>
> > > > Reviewed-by: Stephen Boyd <[email protected]>
> > > > Reviewed-by: Daniel Lezcano <[email protected]>
> > >
> > > This should need to be manually added at each call site; it is already
> > > built into the logging system (the f flag for dynamic debug)?
> >
> > I assume you meant "shouldn't".
>
> Quite so. Sorry about that.
>
> > I haven't yet integrated dynamic debug into my daily workflow.
> >
> > Last time I looked at it, it was a bit bothersome to use because I
> > needed to lookup exact line numbers to trigger useful information. And
> > those line numbers constantly keep changing as I work on the driver,
> > so it was a bit painful to script. Not to mention the syntax to frob
> > the correct files in debugfs to enable this functionality.
> >
> > As opposed to this, adding the following to the makefile is so easy. :-)
> >
> > CFLAGS_tsens-common.o := -DDEBUG
> >
> > Perhaps I am using it all wrong? How would I go about using dynamic
> > debug instead of this patch?
>
> Throwing dyndbg="file <fname>.c +pf" onto the kernel command line is a
> good start (+p enables debug level prints, +f causes messages to include
> the function name).

That's useful to know.

$ git grep __func__ | wc -l
30914

Want to send some patches? :-)

> When the C files map to module names (whether the modules are actually
> built-in or not) then <module>.dyndbg=+pf is a bit cleaner and allows
> you to debug the whole of a driver without how it is decomposed into
> files.

And if changing the kernel cmdline options isn't possible or is inconvenient?

> There are (many) other controls to play with[1] but the above should be
> sufficient to simulate -DDEBUG .

The "hard" bit is explicitly poking the line number in a file to
activate a paricular pr_dbg statement. Even if I scripted it, those
lines numbers keep changing in an actively developed driver.

Somehow, I've always felt dyndbg was more useful to debug a production
system where recompiling the kernel wasn't an option e.g. reporting an
issue back to a distro-kernel vendor.

> Daniel.
>
> [1]
> https://www.kernel.org/doc/html/latest/admin-guide/dynamic-debug-howto.html

2019-08-29 16:36:26

by Daniel Thompson

[permalink] [raw]
Subject: Re: [PATCH v2 03/15] drivers: thermal: tsens: Add __func__ identifier to debug statements

On Thu, Aug 29, 2019 at 09:44:04PM +0530, Amit Kucheria wrote:
> On Thu, Aug 29, 2019 at 8:49 PM Daniel Thompson
> <[email protected]> wrote:
> >
> > On Thu, Aug 29, 2019 at 07:58:45PM +0530, Amit Kucheria wrote:
> > > On Thu, Aug 29, 2019 at 7:35 PM Daniel Thompson
> > > <[email protected]> wrote:
> > > >
> > > > On Tue, Aug 27, 2019 at 05:43:59PM +0530, Amit Kucheria wrote:
> > > > > Printing the function name when enabling debugging makes logs easier to
> > > > > read.
> > > > >
> > > > > Signed-off-by: Amit Kucheria <[email protected]>
> > > > > Reviewed-by: Stephen Boyd <[email protected]>
> > > > > Reviewed-by: Daniel Lezcano <[email protected]>
> > > >
> > > > This should need to be manually added at each call site; it is already
> > > > built into the logging system (the f flag for dynamic debug)?
> > >
> > > I assume you meant "shouldn't".
> >
> > Quite so. Sorry about that.
> >
> > > I haven't yet integrated dynamic debug into my daily workflow.
> > >
> > > Last time I looked at it, it was a bit bothersome to use because I
> > > needed to lookup exact line numbers to trigger useful information. And
> > > those line numbers constantly keep changing as I work on the driver,
> > > so it was a bit painful to script. Not to mention the syntax to frob
> > > the correct files in debugfs to enable this functionality.
> > >
> > > As opposed to this, adding the following to the makefile is so easy. :-)
> > >
> > > CFLAGS_tsens-common.o := -DDEBUG
> > >
> > > Perhaps I am using it all wrong? How would I go about using dynamic
> > > debug instead of this patch?
> >
> > Throwing dyndbg="file <fname>.c +pf" onto the kernel command line is a
> > good start (+p enables debug level prints, +f causes messages to include
> > the function name).
>
> That's useful to know.
>
> $ git grep __func__ | wc -l
> 30914
>
> Want to send some patches? :-)

I know. Sad isn't it?

To be fair plenty of patches already circulate tidying up this sort of
thing (along with the removal of inane messages such as informing us
that a function run).


> > When the C files map to module names (whether the modules are actually
> > built-in or not) then <module>.dyndbg=+pf is a bit cleaner and allows
> > you to debug the whole of a driver without how it is decomposed into
> > files.
>
> And if changing the kernel cmdline options isn't possible or is inconvenient?

Architectures where this problem offer CONFIG_CMDLINE_FORCE meaning if
you are already building a custom kernel you can override whatever
cmdline the bootloader gives you.


> > There are (many) other controls to play with[1] but the above should be
> > sufficient to simulate -DDEBUG .
>
> The "hard" bit is explicitly poking the line number in a file to
> activate a paricular pr_dbg statement. Even if I scripted it, those
> lines numbers keep changing in an actively developed driver.

Line numbers? Nothing I suggested contained a line number.


Daniel.

>
> Somehow, I've always felt dyndbg was more useful to debug a production
> system where recompiling the kernel wasn't an option e.g. reporting an
> issue back to a distro-kernel vendor.
>
> > Daniel.
> >
> > [1]
> > https://www.kernel.org/doc/html/latest/admin-guide/dynamic-debug-howto.html

2019-08-29 16:37:02

by Amit Kucheria

[permalink] [raw]
Subject: Re: [PATCH v2 07/15] dt: thermal: tsens: Document interrupt support in tsens driver

On Thu, Aug 29, 2019 at 8:23 PM Stephen Boyd <[email protected]> wrote:
>
> Quoting Amit Kucheria (2019-08-29 01:48:27)
> > On Wed, Aug 28, 2019 at 6:03 AM Stephen Boyd <[email protected]> wrote:
> > >
> > > Quoting Amit Kucheria (2019-08-27 05:14:03)
> > > > Define two new required properties to define interrupts and
> > > > interrupt-names for tsens.
> > > >
> > > > Signed-off-by: Amit Kucheria <[email protected]>
> > > > ---
> > > > Documentation/devicetree/bindings/thermal/qcom-tsens.txt | 8 ++++++++
> > > > 1 file changed, 8 insertions(+)
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/thermal/qcom-tsens.txt b/Documentation/devicetree/bindings/thermal/qcom-tsens.txt
> > > > index 673cc1831ee9d..686bede72f846 100644
> > > > --- a/Documentation/devicetree/bindings/thermal/qcom-tsens.txt
> > > > +++ b/Documentation/devicetree/bindings/thermal/qcom-tsens.txt
> > > > @@ -22,6 +22,8 @@ Required properties:
> > > >
> > > > - #thermal-sensor-cells : Should be 1. See ./thermal.txt for a description.
> > > > - #qcom,sensors: Number of sensors in tsens block
> > > > +- interrupts: Interrupts generated from Always-On subsystem (AOSS)
> > >
> > > Is it always one? interrupt-names makes it sound like it.
> > >
> > > > +- interrupt-names: Must be one of the following: "uplow", "critical"
> >
> > Will fix to "one or more of the following"
> >
>
> Can we get a known quantity of interrupts for a particular compatible
> string instead? Let's be as specific as possible. The index matters too,
> so please list them in the order that is desired.

I *think* we can predict what platforms have uplow and critical
interrupts based on IP version currently[1]. For newer interrupt
types, we might need more fine-grained platform compatibles.

[1] Caveat: this is based only on the list of platforms I've currently
looked at, there might be something internally that breaks these
rules.

2019-08-30 11:35:01

by Amit Kucheria

[permalink] [raw]
Subject: Re: [PATCH v2 07/15] dt: thermal: tsens: Document interrupt support in tsens driver

On Thu, Aug 29, 2019 at 10:04 PM Amit Kucheria <[email protected]> wrote:
>
> On Thu, Aug 29, 2019 at 8:23 PM Stephen Boyd <[email protected]> wrote:
> >
> > Quoting Amit Kucheria (2019-08-29 01:48:27)
> > > On Wed, Aug 28, 2019 at 6:03 AM Stephen Boyd <[email protected]> wrote:
> > > >
> > > > Quoting Amit Kucheria (2019-08-27 05:14:03)
> > > > > Define two new required properties to define interrupts and
> > > > > interrupt-names for tsens.
> > > > >
> > > > > Signed-off-by: Amit Kucheria <[email protected]>
> > > > > ---
> > > > > Documentation/devicetree/bindings/thermal/qcom-tsens.txt | 8 ++++++++
> > > > > 1 file changed, 8 insertions(+)
> > > > >
> > > > > diff --git a/Documentation/devicetree/bindings/thermal/qcom-tsens.txt b/Documentation/devicetree/bindings/thermal/qcom-tsens.txt
> > > > > index 673cc1831ee9d..686bede72f846 100644
> > > > > --- a/Documentation/devicetree/bindings/thermal/qcom-tsens.txt
> > > > > +++ b/Documentation/devicetree/bindings/thermal/qcom-tsens.txt
> > > > > @@ -22,6 +22,8 @@ Required properties:
> > > > >
> > > > > - #thermal-sensor-cells : Should be 1. See ./thermal.txt for a description.
> > > > > - #qcom,sensors: Number of sensors in tsens block
> > > > > +- interrupts: Interrupts generated from Always-On subsystem (AOSS)
> > > >
> > > > Is it always one? interrupt-names makes it sound like it.
> > > >
> > > > > +- interrupt-names: Must be one of the following: "uplow", "critical"
> > >
> > > Will fix to "one or more of the following"
> > >
> >
> > Can we get a known quantity of interrupts for a particular compatible
> > string instead? Let's be as specific as possible. The index matters too,
> > so please list them in the order that is desired.
>
> I *think* we can predict what platforms have uplow and critical
> interrupts based on IP version currently[1]. For newer interrupt
> types, we might need more fine-grained platform compatibles.
>
> [1] Caveat: this is based only on the list of platforms I've currently
> looked at, there might be something internally that breaks these
> rules.

What do you think if we changed the wording to something like the following,

- interrupt-names: Must be one of the following depending on IP version:
For compatibles qcom,msm8916-tsens, qcom,msm8974-tsens,
qcom,qcs404-tsens, qcom,tsens-v1, use
interrupt-names = "uplow";
For compatibles qcom,msm8996-tsens, qcom,msm8998-tsens,
qcom,sdm845-tsens, qcom,tsens-v2, use
interrupt-names = "uplow", "critical";

2019-08-30 15:58:04

by Stephen Boyd

[permalink] [raw]
Subject: Re: [PATCH v2 07/15] dt: thermal: tsens: Document interrupt support in tsens driver

Quoting Amit Kucheria (2019-08-30 04:32:54)
> On Thu, Aug 29, 2019 at 10:04 PM Amit Kucheria <[email protected]> wrote:
> >
> > On Thu, Aug 29, 2019 at 8:23 PM Stephen Boyd <[email protected]> wrote:
> > >
> > > Can we get a known quantity of interrupts for a particular compatible
> > > string instead? Let's be as specific as possible. The index matters too,
> > > so please list them in the order that is desired.
> >
> > I *think* we can predict what platforms have uplow and critical
> > interrupts based on IP version currently[1]. For newer interrupt
> > types, we might need more fine-grained platform compatibles.
> >
> > [1] Caveat: this is based only on the list of platforms I've currently
> > looked at, there might be something internally that breaks these
> > rules.
>
> What do you think if we changed the wording to something like the following,
>
> - interrupt-names: Must be one of the following depending on IP version:
> For compatibles qcom,msm8916-tsens, qcom,msm8974-tsens,
> qcom,qcs404-tsens, qcom,tsens-v1, use
> interrupt-names = "uplow";
> For compatibles qcom,msm8996-tsens, qcom,msm8998-tsens,
> qcom,sdm845-tsens, qcom,tsens-v2, use
> interrupt-names = "uplow", "critical";

Ok. I would still prefer YAML/JSON schema for this binding so that it's
much more explicit about numbers and the order of interrupts, etc.

2019-08-30 16:42:36

by Amit Kucheria

[permalink] [raw]
Subject: Re: [PATCH v2 07/15] dt: thermal: tsens: Document interrupt support in tsens driver

On Fri, Aug 30, 2019 at 9:25 PM Stephen Boyd <[email protected]> wrote:
>
> Quoting Amit Kucheria (2019-08-30 04:32:54)
> > On Thu, Aug 29, 2019 at 10:04 PM Amit Kucheria <[email protected]> wrote:
> > >
> > > On Thu, Aug 29, 2019 at 8:23 PM Stephen Boyd <[email protected]> wrote:
> > > >
> > > > Can we get a known quantity of interrupts for a particular compatible
> > > > string instead? Let's be as specific as possible. The index matters too,
> > > > so please list them in the order that is desired.
> > >
> > > I *think* we can predict what platforms have uplow and critical
> > > interrupts based on IP version currently[1]. For newer interrupt
> > > types, we might need more fine-grained platform compatibles.
> > >
> > > [1] Caveat: this is based only on the list of platforms I've currently
> > > looked at, there might be something internally that breaks these
> > > rules.
> >
> > What do you think if we changed the wording to something like the following,
> >
> > - interrupt-names: Must be one of the following depending on IP version:
> > For compatibles qcom,msm8916-tsens, qcom,msm8974-tsens,
> > qcom,qcs404-tsens, qcom,tsens-v1, use
> > interrupt-names = "uplow";
> > For compatibles qcom,msm8996-tsens, qcom,msm8998-tsens,
> > qcom,sdm845-tsens, qcom,tsens-v2, use
> > interrupt-names = "uplow", "critical";
>
> Ok. I would still prefer YAML/JSON schema for this binding so that it's
> much more explicit about numbers and the order of interrupts, etc.

OK, I'll look around for some examples.

2019-08-30 20:11:28

by Amit Kucheria

[permalink] [raw]
Subject: Re: [PATCH v2 09/15] arm64: dts: msm8996: thermal: Add interrupt support

On Wed, Aug 28, 2019 at 6:05 AM Stephen Boyd <[email protected]> wrote:
>
> Quoting Amit Kucheria (2019-08-27 05:14:05)
> > Register upper-lower interrupts for each of the two tsens controllers.
> >
> > Signed-off-by: Amit Kucheria <[email protected]>
> > ---
> > arch/arm64/boot/dts/qcom/msm8996.dtsi | 60 ++++++++++++++-------------
> > 1 file changed, 32 insertions(+), 28 deletions(-)
> >
> > diff --git a/arch/arm64/boot/dts/qcom/msm8996.dtsi b/arch/arm64/boot/dts/qcom/msm8996.dtsi
> > index 96c0a481f454e..bb763b362c162 100644
> > --- a/arch/arm64/boot/dts/qcom/msm8996.dtsi
> > +++ b/arch/arm64/boot/dts/qcom/msm8996.dtsi
> > @@ -175,8 +175,8 @@
> >
> > thermal-zones {
> > cpu0-thermal {
> > - polling-delay-passive = <250>;
> > - polling-delay = <1000>;
> > + polling-delay-passive = <0>;
> > + polling-delay = <0>;
>
> Is it really necessary to change the configuration here to be 0 instead
> of some number? Why can't we detect that there's an interrupt and then
> ignore these properties?

AFAICT, the thermal core currently depends on the passive and
polling_delay being set to 0 to avoid setting dispatching polling work
to a workqueue. If we leave the values to set, we'll continue to poll
inspite of an interrupt. See
thermal_core.c:thermal_zone_device_set_polling()

But I agree, the core should detect the presence of an interrupt
property and ignore the polling intervals. I'll see if I can fix this
up later.

Regards,
Amit

2019-09-10 19:12:09

by Amit Kucheria

[permalink] [raw]
Subject: Re: [PATCH v2 15/15] drivers: thermal: tsens: Add interrupt support

Hi Thara,

Thanks for the review. Please find replies below.

On Thu, Aug 29, 2019 at 9:23 AM Thara Gopinath
<[email protected]> wrote:
>
> Hi Amit,
>
> On 08/27/2019 08:14 AM, Amit Kucheria wrote:
> > Depending on the IP version, TSENS supports upper, lower, max, min and
> > critical threshold interrupts. We only add support for upper and lower
> > threshold interrupts for now.
> >
> > TSENSv2 has an irq [status|clear|mask] bit tuple for each sensor while
> > earlier versions only have a single bit per sensor to denote status and
> > clear. At each interrupt, we reprogram the new upper and lower threshold
> > in the .set_trip callback.
> >
> > Signed-off-by: Amit Kucheria <[email protected]>
> > ---
> > drivers/thermal/qcom/tsens-common.c | 377 ++++++++++++++++++++++++++--
> > drivers/thermal/qcom/tsens-v0_1.c | 11 +
> > drivers/thermal/qcom/tsens-v1.c | 29 +++
> > drivers/thermal/qcom/tsens-v2.c | 13 +
> > drivers/thermal/qcom/tsens.c | 32 ++-
> > drivers/thermal/qcom/tsens.h | 270 ++++++++++++++++----
> > 6 files changed, 669 insertions(+), 63 deletions(-)
> >
> > diff --git a/drivers/thermal/qcom/tsens-common.c b/drivers/thermal/qcom/tsens-common.c
> > index 06b44cfd5eab9..c549f8e1488ba 100644
> > --- a/drivers/thermal/qcom/tsens-common.c
> > +++ b/drivers/thermal/qcom/tsens-common.c

<snip>

Please snip liberally when only replying to a small part of the patch. :-)

> > +
> > + spin_unlock_irqrestore(&priv->ul_lock, flags);
> > +
> > + if (trigger) {
> > + dev_dbg(priv->dev, "[%u] %s: TZ update trigger (%d mC)\n",
> > + hw_id, __func__, temp);
> > + thermal_zone_device_update(priv->sensor[i].tzd,
> > + THERMAL_EVENT_UNSPECIFIED);
> Why not use thermal_notify_framework? Or do you want to loop over all
> registered trips ?

Mainly so we get a driver call-back to set_trips where we might want
to change behaviour once we cross certain thresholds.

> Another comment I have is regarding support for multiple thermal zone.
> It might not be needed now, but presence of the same sensor can serve
> multiple zones (one for cooling and one for warming) . I am not sure if
> you want to put some infrastructure in place for that as well for
> interrupt handling.

It is premature to add such infrastructure to the driver until we
decide how we're going to implement support for this, IMO.

<snip>