* [PATCH v3 0/4] thermal: tsens: Add support for QCS404 platform
@ 2018-11-27 16:29 Amit Kucheria
2018-11-27 16:29 ` [PATCH v3 1/4] dt: thermal: tsens: Add bindings for qcs404 Amit Kucheria
` (3 more replies)
0 siblings, 4 replies; 11+ messages in thread
From: Amit Kucheria @ 2018-11-27 16:29 UTC (permalink / raw)
To: linux-kernel
Cc: linux-arm-msm, bjorn.andersson, edubezval, andy.gross, vkoul,
khasim.mohammed, Daniel Lezcano, devicetree, linux-pm, linux-soc
Add support for the Qualcomm QCS404 platform that contains v1 of the TSENS
IP. Introduce a fallback binding to handle "v1" functionality.
These patches have been tested on top of 4.20-rc3 + various qcs404-related
DT patches now in Andy's ci-next branch[1].
Changes since v2:
- Fix a conversion error in the get_temp routine the caused negative
temperature readings (Reported by Khasim)
Changes since v1:
- Change p1 and p2 to be fixed-size arrays
- Refactor DT entries to be sorted by address, Thanks Vinod
- Added Acks
Amit Kucheria (4):
dt: thermal: tsens: Add bindings for qcs404
drivers: thermal: tsens: Add generic support for TSENS v1 IP
arm64: dts: qcom: qcs404: Add tsens controller
arm64: dts: qcom: qcs404: Add thermal zones for each sensor
.../bindings/thermal/qcom-tsens.txt | 3 +
arch/arm64/boot/dts/qcom/qcs404.dtsi | 226 ++++++++++++++++++
drivers/thermal/qcom/Makefile | 2 +-
drivers/thermal/qcom/tsens-common.c | 2 +-
drivers/thermal/qcom/tsens-v1.c | 196 +++++++++++++++
drivers/thermal/qcom/tsens.c | 3 +
drivers/thermal/qcom/tsens.h | 3 +-
7 files changed, 432 insertions(+), 3 deletions(-)
create mode 100644 drivers/thermal/qcom/tsens-v1.c
--
2.17.1
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v3 1/4] dt: thermal: tsens: Add bindings for qcs404
2018-11-27 16:29 [PATCH v3 0/4] thermal: tsens: Add support for QCS404 platform Amit Kucheria
@ 2018-11-27 16:29 ` Amit Kucheria
2018-11-27 16:29 ` [PATCH v3 2/4] drivers: thermal: tsens: Add generic support for TSENS v1 IP Amit Kucheria
` (2 subsequent siblings)
3 siblings, 0 replies; 11+ messages in thread
From: Amit Kucheria @ 2018-11-27 16:29 UTC (permalink / raw)
To: linux-kernel
Cc: linux-arm-msm, bjorn.andersson, edubezval, andy.gross, vkoul,
khasim.mohammed, Zhang Rui, Daniel Lezcano, Rob Herring,
Mark Rutland
qcs404 uses v1 of the TSENS IP block. Create a fallback DT property
"qcom,tsens-v1" to gather common code.
Signed-off-by: Amit Kucheria <amit.kucheria@linaro.org>
Reviewed-by: Vinod Koul <vkoul@kernel.org>
Tested-by: Vinod Koul <vkoul@kernel.org>
---
Documentation/devicetree/bindings/thermal/qcom-tsens.txt | 3 +++
1 file changed, 3 insertions(+)
diff --git a/Documentation/devicetree/bindings/thermal/qcom-tsens.txt b/Documentation/devicetree/bindings/thermal/qcom-tsens.txt
index 1d9e8cf61018..799de3062352 100644
--- a/Documentation/devicetree/bindings/thermal/qcom-tsens.txt
+++ b/Documentation/devicetree/bindings/thermal/qcom-tsens.txt
@@ -8,9 +8,12 @@ Required properties:
- "qcom,msm8996-tsens" (MSM8996)
- "qcom,msm8998-tsens", "qcom,tsens-v2" (MSM8998)
- "qcom,sdm845-tsens", "qcom,tsens-v2" (SDM845)
+ - "qcom,qcs404-tsens", "qcom,tsens-v1" (QCS404)
The generic "qcom,tsens-v2" property must be used as a fallback for any SoC
with version 2 of the TSENS IP. MSM8996 is the only exception because the
generic property did not exist when support was added.
+ Similarly, the generic "qcom,tsens-v1" property must be used as a fallback for
+ any SoC with version 1 of the TSENS IP.
- reg: Address range of the thermal registers.
New platforms containing v2.x.y of the TSENS IP must specify the SROT and TM
--
2.17.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v3 2/4] drivers: thermal: tsens: Add generic support for TSENS v1 IP
2018-11-27 16:29 [PATCH v3 0/4] thermal: tsens: Add support for QCS404 platform Amit Kucheria
2018-11-27 16:29 ` [PATCH v3 1/4] dt: thermal: tsens: Add bindings for qcs404 Amit Kucheria
@ 2018-11-27 16:29 ` Amit Kucheria
2018-11-29 16:56 ` Eduardo Valentin
2018-11-27 16:29 ` [PATCH v3 3/4] arm64: dts: qcom: qcs404: Add tsens controller Amit Kucheria
2018-11-27 16:29 ` [PATCH v3 4/4] arm64: dts: qcom: qcs404: Add thermal zones for each sensor Amit Kucheria
3 siblings, 1 reply; 11+ messages in thread
From: Amit Kucheria @ 2018-11-27 16:29 UTC (permalink / raw)
To: linux-kernel
Cc: linux-arm-msm, bjorn.andersson, edubezval, andy.gross, vkoul,
khasim.mohammed, Zhang Rui, Daniel Lezcano
qcs404 has a single TSENS IP block with 10 sensors. It uses version 1.4
of the TSENS IP, functionality for which is encapsulated inside
qcom,tsens-v1 compatible.
Signed-off-by: Amit Kucheria <amit.kucheria@linaro.org>
Reviewed-by: Vinod Koul <vkoul@kernel.org>
Tested-by: Vinod Koul <vkoul@kernel.org>
---
drivers/thermal/qcom/Makefile | 2 +-
drivers/thermal/qcom/tsens-common.c | 2 +-
drivers/thermal/qcom/tsens-v1.c | 196 ++++++++++++++++++++++++++++
drivers/thermal/qcom/tsens.c | 3 +
drivers/thermal/qcom/tsens.h | 3 +-
5 files changed, 203 insertions(+), 3 deletions(-)
create mode 100644 drivers/thermal/qcom/tsens-v1.c
diff --git a/drivers/thermal/qcom/Makefile b/drivers/thermal/qcom/Makefile
index a821929ede0b..60269ee90c43 100644
--- a/drivers/thermal/qcom/Makefile
+++ b/drivers/thermal/qcom/Makefile
@@ -1,2 +1,2 @@
obj-$(CONFIG_QCOM_TSENS) += qcom_tsens.o
-qcom_tsens-y += tsens.o tsens-common.o tsens-8916.o tsens-8974.o tsens-8960.o tsens-v2.o
+qcom_tsens-y += tsens.o tsens-common.o tsens-8916.o tsens-8974.o tsens-8960.o tsens-v2.o tsens-v1.o
diff --git a/drivers/thermal/qcom/tsens-common.c b/drivers/thermal/qcom/tsens-common.c
index 3be4be2e0465..98f77401bc12 100644
--- a/drivers/thermal/qcom/tsens-common.c
+++ b/drivers/thermal/qcom/tsens-common.c
@@ -76,7 +76,7 @@ void compute_intercept_slope(struct tsens_device *tmdev, u32 *p1,
}
}
-static inline int code_to_degc(u32 adc_code, const struct tsens_sensor *s)
+int code_to_degc(u32 adc_code, const struct tsens_sensor *s)
{
int degc, num, den;
diff --git a/drivers/thermal/qcom/tsens-v1.c b/drivers/thermal/qcom/tsens-v1.c
new file mode 100644
index 000000000000..1dbf4fde6da8
--- /dev/null
+++ b/drivers/thermal/qcom/tsens-v1.c
@@ -0,0 +1,196 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2018, Linaro Limited
+ */
+
+#include <linux/regmap.h>
+#include <linux/bitops.h>
+#include "tsens.h"
+
+/* eeprom layout data for qcs404 (v1) */
+#define BASE0_MASK 0x000007f8
+#define BASE1_MASK 0x0007f800
+#define BASE0_SHIFT 3
+#define BASE1_SHIFT 11
+
+#define S0_P1_MASK 0x0000003f
+#define S1_P1_MASK 0x0003f000
+#define S2_P1_MASK 0x3f000000
+#define S3_P1_MASK 0x000003f0
+#define S4_P1_MASK 0x003f0000
+#define S5_P1_MASK 0x0000003f
+#define S6_P1_MASK 0x0003f000
+#define S7_P1_MASK 0x3f000000
+#define S8_P1_MASK 0x000003f0
+#define S9_P1_MASK 0x003f0000
+
+#define S0_P2_MASK 0x00000fc0
+#define S1_P2_MASK 0x00fc0000
+#define S2_P2_MASK_1_0 0xc0000000
+#define S2_P2_MASK_5_2 0x0000000f
+#define S3_P2_MASK 0x0000fc00
+#define S4_P2_MASK 0x0fc00000
+#define S5_P2_MASK 0x00000fc0
+#define S6_P2_MASK 0x00fc0000
+#define S7_P2_MASK_1_0 0xc0000000
+#define S7_P2_MASK_5_2 0x0000000f
+#define S8_P2_MASK 0x0000fc00
+#define S9_P2_MASK 0x0fc00000
+
+#define S0_P1_SHIFT 0
+#define S0_P2_SHIFT 6
+#define S1_P1_SHIFT 12
+#define S1_P2_SHIFT 18
+#define S2_P1_SHIFT 24
+#define S2_P2_SHIFT_1_0 30
+
+#define S2_P2_SHIFT_5_2 0
+#define S3_P1_SHIFT 4
+#define S3_P2_SHIFT 10
+#define S4_P1_SHIFT 16
+#define S4_P2_SHIFT 22
+
+#define S5_P1_SHIFT 0
+#define S5_P2_SHIFT 6
+#define S6_P1_SHIFT 12
+#define S6_P2_SHIFT 18
+#define S7_P1_SHIFT 24
+#define S7_P2_SHIFT_1_0 30
+
+#define S7_P2_SHIFT_5_2 0
+#define S8_P1_SHIFT 4
+#define S8_P2_SHIFT 10
+#define S9_P1_SHIFT 16
+#define S9_P2_SHIFT 22
+
+#define CAL_SEL_MASK 7
+#define CAL_SEL_SHIFT 0
+
+static int calibrate_v1(struct tsens_device *tmdev)
+{
+ u32 base0 = 0, base1 = 0;
+ u32 p1[10], p2[10];
+ u32 mode = 0, lsb = 0, msb = 0;
+ u32 *qfprom_cdata;
+ int i;
+
+ qfprom_cdata = (u32 *)qfprom_read(tmdev->dev, "calib");
+ if (IS_ERR(qfprom_cdata))
+ return PTR_ERR(qfprom_cdata);
+
+ mode = (qfprom_cdata[4] & CAL_SEL_MASK) >> CAL_SEL_SHIFT;
+ dev_dbg(tmdev->dev, "calibration mode is %d\n", mode);
+
+ switch (mode) {
+ case TWO_PT_CALIB:
+ base1 = (qfprom_cdata[4] & BASE1_MASK) >> BASE1_SHIFT;
+ p2[0] = (qfprom_cdata[0] & S0_P2_MASK) >> S0_P2_SHIFT;
+ p2[1] = (qfprom_cdata[0] & S1_P2_MASK) >> S1_P2_SHIFT;
+ /* This value is split over two registers, 2 bits and 4 bits */
+ lsb = (qfprom_cdata[0] & S2_P2_MASK_1_0) >> S2_P2_SHIFT_1_0;
+ msb = (qfprom_cdata[1] & S2_P2_MASK_5_2) >> S2_P2_SHIFT_5_2;
+ p2[2] = msb << 2 | lsb;
+ p2[3] = (qfprom_cdata[1] & S3_P2_MASK) >> S3_P2_SHIFT;
+ p2[4] = (qfprom_cdata[1] & S4_P2_MASK) >> S4_P2_SHIFT;
+ p2[5] = (qfprom_cdata[2] & S5_P2_MASK) >> S5_P2_SHIFT;
+ p2[6] = (qfprom_cdata[2] & S6_P2_MASK) >> S6_P2_SHIFT;
+ /* This value is split over two registers, 2 bits and 4 bits */
+ lsb = (qfprom_cdata[2] & S7_P2_MASK_1_0) >> S7_P2_SHIFT_1_0;
+ msb = (qfprom_cdata[3] & S7_P2_MASK_5_2) >> S7_P2_SHIFT_5_2;
+ p2[7] = msb << 2 | lsb;
+ p2[8] = (qfprom_cdata[3] & S8_P2_MASK) >> S8_P2_SHIFT;
+ p2[9] = (qfprom_cdata[3] & S9_P2_MASK) >> S9_P2_SHIFT;
+ for (i = 0; i < tmdev->num_sensors; i++)
+ p2[i] = ((base1 + p2[i]) << 2);
+ /* Fall through */
+ case ONE_PT_CALIB2:
+ base0 = (qfprom_cdata[4] & BASE0_MASK) >> BASE0_SHIFT;
+ p1[0] = (qfprom_cdata[0] & S0_P1_MASK) >> S0_P1_SHIFT;
+ p1[1] = (qfprom_cdata[0] & S1_P1_MASK) >> S1_P1_SHIFT;
+ p1[2] = (qfprom_cdata[0] & S2_P1_MASK) >> S2_P1_SHIFT;
+ p1[3] = (qfprom_cdata[1] & S3_P1_MASK) >> S3_P1_SHIFT;
+ p1[4] = (qfprom_cdata[1] & S4_P1_MASK) >> S4_P1_SHIFT;
+ p1[5] = (qfprom_cdata[2] & S5_P1_MASK) >> S5_P1_SHIFT;
+ p1[6] = (qfprom_cdata[2] & S6_P1_MASK) >> S6_P1_SHIFT;
+ p1[7] = (qfprom_cdata[2] & S7_P1_MASK) >> S7_P1_SHIFT;
+ p1[8] = (qfprom_cdata[3] & S8_P1_MASK) >> S8_P1_SHIFT;
+ p1[9] = (qfprom_cdata[3] & S9_P1_MASK) >> S9_P1_SHIFT;
+ for (i = 0; i < tmdev->num_sensors; i++)
+ p1[i] = (((base0) + p1[i]) << 2);
+ break;
+ default:
+ for (i = 0; i < tmdev->num_sensors; i++) {
+ p1[i] = 500;
+ p2[i] = 780;
+ }
+ break;
+ }
+
+ compute_intercept_slope(tmdev, p1, p2, mode);
+
+ return 0;
+}
+
+#define STATUS_OFFSET 0x44
+#define LAST_TEMP_MASK 0x3ff
+#define STATUS_VALID_BIT BIT(14)
+
+static int get_temp_tsens_v1(struct tsens_device *tmdev, int id, int *temp)
+{
+ struct tsens_sensor *s = &tmdev->sensor[id];
+ u32 code;
+ unsigned int status_reg;
+ u32 last_temp = 0, last_temp2 = 0, last_temp3 = 0;
+ int ret;
+
+ status_reg = tmdev->tm_offset + STATUS_OFFSET + s->hw_id * 4;
+ ret = regmap_read(tmdev->tm_map, status_reg, &code);
+ if (ret)
+ return ret;
+ last_temp = code & LAST_TEMP_MASK;
+ if (code & STATUS_VALID_BIT)
+ goto done;
+
+ /* Try a second time */
+ ret = regmap_read(tmdev->tm_map, status_reg, &code);
+ if (ret)
+ return ret;
+ if (code & STATUS_VALID_BIT) {
+ last_temp = code & LAST_TEMP_MASK;
+ goto done;
+ } else {
+ last_temp2 = code & LAST_TEMP_MASK;
+ }
+
+ /* Try a third/last time */
+ ret = regmap_read(tmdev->tm_map, status_reg, &code);
+ if (ret)
+ return ret;
+ if (code & STATUS_VALID_BIT) {
+ last_temp = code & LAST_TEMP_MASK;
+ goto done;
+ } else {
+ last_temp3 = code & LAST_TEMP_MASK;
+ }
+
+ if (last_temp == last_temp2)
+ last_temp = last_temp2;
+ else if (last_temp2 == last_temp3)
+ last_temp = last_temp3;
+done:
+ /* Convert temperature from ADC code to milliCelsius */
+ *temp = code_to_degc(last_temp, s) * 1000;
+
+ return 0;
+}
+
+static const struct tsens_ops ops_generic_v1 = {
+ .init = init_common,
+ .calibrate = calibrate_v1,
+ .get_temp = get_temp_tsens_v1,
+};
+
+const struct tsens_data data_tsens_v1 = {
+ .ops = &ops_generic_v1,
+ .reg_offsets = { [SROT_CTRL_OFFSET] = 0x4 },
+};
diff --git a/drivers/thermal/qcom/tsens.c b/drivers/thermal/qcom/tsens.c
index f1ec9bbe4717..d0cc0c09894a 100644
--- a/drivers/thermal/qcom/tsens.c
+++ b/drivers/thermal/qcom/tsens.c
@@ -63,6 +63,9 @@ static const struct of_device_id tsens_table[] = {
}, {
.compatible = "qcom,msm8996-tsens",
.data = &data_8996,
+ }, {
+ .compatible = "qcom,tsens-v1",
+ .data = &data_tsens_v1,
}, {
.compatible = "qcom,tsens-v2",
.data = &data_tsens_v2,
diff --git a/drivers/thermal/qcom/tsens.h b/drivers/thermal/qcom/tsens.h
index 7b7feee5dc46..f8dc96c42b94 100644
--- a/drivers/thermal/qcom/tsens.h
+++ b/drivers/thermal/qcom/tsens.h
@@ -90,9 +90,10 @@ char *qfprom_read(struct device *, const char *);
void compute_intercept_slope(struct tsens_device *, u32 *, u32 *, u32);
int init_common(struct tsens_device *);
int get_temp_common(struct tsens_device *, int, int *);
+int code_to_degc(u32 adc_code, const struct tsens_sensor *s);
/* TSENS v1 targets */
-extern const struct tsens_data data_8916, data_8974, data_8960;
+extern const struct tsens_data data_8916, data_8974, data_8960, data_tsens_v1;
/* TSENS v2 targets */
extern const struct tsens_data data_8996, data_tsens_v2;
--
2.17.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v3 3/4] arm64: dts: qcom: qcs404: Add tsens controller
2018-11-27 16:29 [PATCH v3 0/4] thermal: tsens: Add support for QCS404 platform Amit Kucheria
2018-11-27 16:29 ` [PATCH v3 1/4] dt: thermal: tsens: Add bindings for qcs404 Amit Kucheria
2018-11-27 16:29 ` [PATCH v3 2/4] drivers: thermal: tsens: Add generic support for TSENS v1 IP Amit Kucheria
@ 2018-11-27 16:29 ` Amit Kucheria
2018-11-27 16:29 ` [PATCH v3 4/4] arm64: dts: qcom: qcs404: Add thermal zones for each sensor Amit Kucheria
3 siblings, 0 replies; 11+ messages in thread
From: Amit Kucheria @ 2018-11-27 16:29 UTC (permalink / raw)
To: linux-kernel
Cc: linux-arm-msm, bjorn.andersson, edubezval, andy.gross, vkoul,
khasim.mohammed, David Brown, Rob Herring, Mark Rutland
qcs404 has a single TSENS IP block with 10 sensors. The calibration data
is stored in an eeprom (qfprom) that is accessed through the nvmem
framework. We add the qfprom node to allow the tsens sensors to be
calibrated correctly.
Signed-off-by: Amit Kucheria <amit.kucheria@linaro.org>
Reviewed-by: Vinod Koul <vkoul@kernel.org>
Tested-by: Vinod Koul <vkoul@kernel.org>
---
arch/arm64/boot/dts/qcom/qcs404.dtsi | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/qcs404.dtsi b/arch/arm64/boot/dts/qcom/qcs404.dtsi
index 9b5c16562bbe..57d14d8f0c90 100644
--- a/arch/arm64/boot/dts/qcom/qcs404.dtsi
+++ b/arch/arm64/boot/dts/qcom/qcs404.dtsi
@@ -253,6 +253,16 @@
reg = <0x00060000 0x6000>;
};
+ qfprom: qfprom@a4000 {
+ compatible = "qcom,qfprom";
+ reg = <0x000a4000 0x1000>;
+ #address-cells = <1>;
+ #size-cells = <1>;
+ tsens_caldata: caldata@d0 {
+ reg = <0x1f8 0x14>;
+ };
+ };
+
rng: rng@e3000 {
compatible = "qcom,prng-ee";
reg = <0x000e3000 0x1000>;
@@ -260,6 +270,16 @@
clock-names = "core";
};
+ tsens: thermal-sensor@4a9000 {
+ compatible = "qcom,qcs404-tsens", "qcom,tsens-v1";
+ reg = <0x004a9000 0x1000>, /* TM */
+ <0x004a8000 0x1000>; /* SROT */
+ nvmem-cells = <&tsens_caldata>;
+ nvmem-cell-names = "calib";
+ #qcom,sensors = <10>;
+ #thermal-sensor-cells = <1>;
+ };
+
tlmm: pinctrl@1000000 {
compatible = "qcom,qcs404-pinctrl";
reg = <0x01000000 0x200000>,
--
2.17.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v3 4/4] arm64: dts: qcom: qcs404: Add thermal zones for each sensor
2018-11-27 16:29 [PATCH v3 0/4] thermal: tsens: Add support for QCS404 platform Amit Kucheria
` (2 preceding siblings ...)
2018-11-27 16:29 ` [PATCH v3 3/4] arm64: dts: qcom: qcs404: Add tsens controller Amit Kucheria
@ 2018-11-27 16:29 ` Amit Kucheria
2018-11-29 16:57 ` Eduardo Valentin
3 siblings, 1 reply; 11+ messages in thread
From: Amit Kucheria @ 2018-11-27 16:29 UTC (permalink / raw)
To: linux-kernel
Cc: linux-arm-msm, bjorn.andersson, edubezval, andy.gross, vkoul,
khasim.mohammed, David Brown, Rob Herring, Mark Rutland
qcs404 has 10 sensors connected to the single TSENS IP. Define a thermal
zone for each of those sensors to expose the temperature of each zone.
Signed-off-by: Amit Kucheria <amit.kucheria@linaro.org>
Reviewed-by: Vinod Koul <vkoul@kernel.org>
Tested-by: Vinod Koul <vkoul@kernel.org>
---
arch/arm64/boot/dts/qcom/qcs404.dtsi | 206 +++++++++++++++++++++++++++
1 file changed, 206 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/qcs404.dtsi b/arch/arm64/boot/dts/qcom/qcs404.dtsi
index 57d14d8f0c90..cbc3fd378893 100644
--- a/arch/arm64/boot/dts/qcom/qcs404.dtsi
+++ b/arch/arm64/boot/dts/qcom/qcs404.dtsi
@@ -30,6 +30,7 @@
reg = <0x100>;
enable-method = "psci";
next-level-cache = <&L2_0>;
+ #cooling-cells= <2>;
};
CPU1: cpu@101 {
@@ -38,6 +39,7 @@
reg = <0x101>;
enable-method = "psci";
next-level-cache = <&L2_0>;
+ #cooling-cells= <2>;
};
CPU2: cpu@102 {
@@ -46,6 +48,7 @@
reg = <0x102>;
enable-method = "psci";
next-level-cache = <&L2_0>;
+ #cooling-cells= <2>;
};
CPU3: cpu@103 {
@@ -54,6 +57,7 @@
reg = <0x103>;
enable-method = "psci";
next-level-cache = <&L2_0>;
+ #cooling-cells= <2>;
};
L2_0: l2-cache {
@@ -507,4 +511,206 @@
#interrupt-cells = <2>;
};
};
+
+ thermal-zones {
+ aoss-thermal {
+ polling-delay-passive = <250>;
+ polling-delay = <1000>;
+
+ thermal-sensors = <&tsens 0>;
+
+ trips {
+ aoss_alert: trip0 {
+ temperature = <75000>;
+ hysteresis = <2000>;
+ type = "passive";
+ };
+ aoss_crit: trip1 {
+ temperature = <95000>;
+ hysteresis = <2000>;
+ type = "critical";
+ };
+ };
+ };
+
+ dsp-thermal {
+ polling-delay-passive = <250>;
+ polling-delay = <1000>;
+
+ thermal-sensors = <&tsens 1>;
+
+ trips {
+ dsp_alert: trip0 {
+ temperature = <75000>;
+ hysteresis = <2000>;
+ type = "passive";
+ };
+ dsp_crit: trip1 {
+ temperature = <95000>;
+ hysteresis = <2000>;
+ type = "critical";
+ };
+ };
+ };
+
+ lpass-thermal {
+ polling-delay-passive = <250>;
+ polling-delay = <1000>;
+
+ thermal-sensors = <&tsens 2>;
+
+ trips {
+ lpass_alert: trip0 {
+ temperature = <75000>;
+ hysteresis = <2000>;
+ type = "passive";
+ };
+ lpass_crit: trip1 {
+ temperature = <95000>;
+ hysteresis = <2000>;
+ type = "critical";
+ };
+ };
+ };
+
+ wlan-thermal {
+ polling-delay-passive = <250>;
+ polling-delay = <1000>;
+
+ thermal-sensors = <&tsens 3>;
+
+ trips {
+ wlan_alert: trip0 {
+ temperature = <75000>;
+ hysteresis = <2000>;
+ type = "passive";
+ };
+ wlan_crit: trip1 {
+ temperature = <95000>;
+ hysteresis = <2000>;
+ type = "critical";
+ };
+ };
+ };
+
+ cluster-thermal {
+ polling-delay-passive = <250>;
+ polling-delay = <1000>;
+
+ thermal-sensors = <&tsens 4>;
+
+ trips {
+ cluster_alert: cluster_alert {
+ temperature = <75000>;
+ hysteresis = <2000>;
+ type = "passive";
+ };
+ cluster_crit: cluster_crit {
+ temperature = <110000>;
+ hysteresis = <2000>;
+ type = "critical";
+ };
+ };
+ };
+
+ cpu-thermal0 {
+ polling-delay-passive = <250>;
+ polling-delay = <1000>;
+
+ thermal-sensors = <&tsens 5>;
+
+ trips {
+ cpu_alert0: cpu_alert0 {
+ temperature = <75000>;
+ hysteresis = <2000>;
+ type = "passive";
+ };
+ cpu_crit0: cpu_crit0 {
+ temperature = <110000>;
+ hysteresis = <2000>;
+ type = "critical";
+ };
+ };
+ };
+
+ cpu-thermal1 {
+ polling-delay-passive = <250>;
+ polling-delay = <1000>;
+
+ thermal-sensors = <&tsens 6>;
+
+ trips {
+ cpu_alert1: trip0 {
+ temperature = <75000>;
+ hysteresis = <2000>;
+ type = "passive";
+ };
+ cpu_crit1: trip1 {
+ temperature = <110000>;
+ hysteresis = <2000>;
+ type = "critical";
+ };
+ };
+ };
+
+ cpu-thermal2 {
+ polling-delay-passive = <250>;
+ polling-delay = <1000>;
+
+ thermal-sensors = <&tsens 7>;
+
+ trips {
+ cpu_alert2: trip0 {
+ temperature = <75000>;
+ hysteresis = <2000>;
+ type = "passive";
+ };
+ cpu_crit2: trip1 {
+ temperature = <110000>;
+ hysteresis = <2000>;
+ type = "critical";
+ };
+ };
+ };
+
+ cpu-thermal3 {
+ polling-delay-passive = <250>;
+ polling-delay = <1000>;
+
+ thermal-sensors = <&tsens 8>;
+
+ trips {
+ cpu_alert3: trip0 {
+ temperature = <75000>;
+ hysteresis = <2000>;
+ type = "passive";
+ };
+ cpu_crit3: trip1 {
+ temperature = <110000>;
+ hysteresis = <2000>;
+ type = "critical";
+ };
+ };
+ };
+
+ gpu-thermal {
+ polling-delay-passive = <250>;
+ polling-delay = <1000>;
+
+ thermal-sensors = <&tsens 9>;
+
+ trips {
+ gpu_alert: trip0 {
+ temperature = <75000>;
+ hysteresis = <2000>;
+ type = "passive";
+ };
+ gpu_crit: trip1 {
+ temperature = <95000>;
+ hysteresis = <2000>;
+ type = "critical";
+ };
+ };
+ };
+ };
};
--
2.17.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v3 2/4] drivers: thermal: tsens: Add generic support for TSENS v1 IP
2018-11-27 16:29 ` [PATCH v3 2/4] drivers: thermal: tsens: Add generic support for TSENS v1 IP Amit Kucheria
@ 2018-11-29 16:56 ` Eduardo Valentin
2018-12-04 11:24 ` Amit Kucheria
0 siblings, 1 reply; 11+ messages in thread
From: Eduardo Valentin @ 2018-11-29 16:56 UTC (permalink / raw)
To: Amit Kucheria
Cc: linux-kernel, linux-arm-msm, bjorn.andersson, andy.gross, vkoul,
khasim.mohammed, Zhang Rui, Daniel Lezcano
On Tue, Nov 27, 2018 at 09:59:05PM +0530, Amit Kucheria wrote:
> qcs404 has a single TSENS IP block with 10 sensors. It uses version 1.4
> of the TSENS IP, functionality for which is encapsulated inside
> qcom,tsens-v1 compatible.
>
> Signed-off-by: Amit Kucheria <amit.kucheria@linaro.org>
> Reviewed-by: Vinod Koul <vkoul@kernel.org>
> Tested-by: Vinod Koul <vkoul@kernel.org>
> ---
> drivers/thermal/qcom/Makefile | 2 +-
> drivers/thermal/qcom/tsens-common.c | 2 +-
> drivers/thermal/qcom/tsens-v1.c | 196 ++++++++++++++++++++++++++++
> drivers/thermal/qcom/tsens.c | 3 +
> drivers/thermal/qcom/tsens.h | 3 +-
> 5 files changed, 203 insertions(+), 3 deletions(-)
> create mode 100644 drivers/thermal/qcom/tsens-v1.c
>
> diff --git a/drivers/thermal/qcom/Makefile b/drivers/thermal/qcom/Makefile
> index a821929ede0b..60269ee90c43 100644
> --- a/drivers/thermal/qcom/Makefile
> +++ b/drivers/thermal/qcom/Makefile
> @@ -1,2 +1,2 @@
> obj-$(CONFIG_QCOM_TSENS) += qcom_tsens.o
> -qcom_tsens-y += tsens.o tsens-common.o tsens-8916.o tsens-8974.o tsens-8960.o tsens-v2.o
> +qcom_tsens-y += tsens.o tsens-common.o tsens-8916.o tsens-8974.o tsens-8960.o tsens-v2.o tsens-v1.o
> diff --git a/drivers/thermal/qcom/tsens-common.c b/drivers/thermal/qcom/tsens-common.c
> index 3be4be2e0465..98f77401bc12 100644
> --- a/drivers/thermal/qcom/tsens-common.c
> +++ b/drivers/thermal/qcom/tsens-common.c
> @@ -76,7 +76,7 @@ void compute_intercept_slope(struct tsens_device *tmdev, u32 *p1,
> }
> }
>
> -static inline int code_to_degc(u32 adc_code, const struct tsens_sensor *s)
> +int code_to_degc(u32 adc_code, const struct tsens_sensor *s)
> {
> int degc, num, den;
>
> diff --git a/drivers/thermal/qcom/tsens-v1.c b/drivers/thermal/qcom/tsens-v1.c
> new file mode 100644
> index 000000000000..1dbf4fde6da8
> --- /dev/null
> +++ b/drivers/thermal/qcom/tsens-v1.c
> @@ -0,0 +1,196 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2018, Linaro Limited
> + */
> +
> +#include <linux/regmap.h>
> +#include <linux/bitops.h>
> +#include "tsens.h"
> +
> +/* eeprom layout data for qcs404 (v1) */
> +#define BASE0_MASK 0x000007f8
> +#define BASE1_MASK 0x0007f800
> +#define BASE0_SHIFT 3
> +#define BASE1_SHIFT 11
> +
> +#define S0_P1_MASK 0x0000003f
> +#define S1_P1_MASK 0x0003f000
> +#define S2_P1_MASK 0x3f000000
> +#define S3_P1_MASK 0x000003f0
> +#define S4_P1_MASK 0x003f0000
> +#define S5_P1_MASK 0x0000003f
> +#define S6_P1_MASK 0x0003f000
> +#define S7_P1_MASK 0x3f000000
> +#define S8_P1_MASK 0x000003f0
> +#define S9_P1_MASK 0x003f0000
> +
> +#define S0_P2_MASK 0x00000fc0
> +#define S1_P2_MASK 0x00fc0000
> +#define S2_P2_MASK_1_0 0xc0000000
> +#define S2_P2_MASK_5_2 0x0000000f
> +#define S3_P2_MASK 0x0000fc00
> +#define S4_P2_MASK 0x0fc00000
> +#define S5_P2_MASK 0x00000fc0
> +#define S6_P2_MASK 0x00fc0000
> +#define S7_P2_MASK_1_0 0xc0000000
> +#define S7_P2_MASK_5_2 0x0000000f
> +#define S8_P2_MASK 0x0000fc00
> +#define S9_P2_MASK 0x0fc00000
> +
> +#define S0_P1_SHIFT 0
> +#define S0_P2_SHIFT 6
> +#define S1_P1_SHIFT 12
> +#define S1_P2_SHIFT 18
> +#define S2_P1_SHIFT 24
> +#define S2_P2_SHIFT_1_0 30
> +
> +#define S2_P2_SHIFT_5_2 0
> +#define S3_P1_SHIFT 4
> +#define S3_P2_SHIFT 10
> +#define S4_P1_SHIFT 16
> +#define S4_P2_SHIFT 22
> +
> +#define S5_P1_SHIFT 0
> +#define S5_P2_SHIFT 6
> +#define S6_P1_SHIFT 12
> +#define S6_P2_SHIFT 18
> +#define S7_P1_SHIFT 24
> +#define S7_P2_SHIFT_1_0 30
> +
> +#define S7_P2_SHIFT_5_2 0
> +#define S8_P1_SHIFT 4
> +#define S8_P2_SHIFT 10
> +#define S9_P1_SHIFT 16
> +#define S9_P2_SHIFT 22
> +
> +#define CAL_SEL_MASK 7
> +#define CAL_SEL_SHIFT 0
> +
> +static int calibrate_v1(struct tsens_device *tmdev)
> +{
> + u32 base0 = 0, base1 = 0;
> + u32 p1[10], p2[10];
> + u32 mode = 0, lsb = 0, msb = 0;
> + u32 *qfprom_cdata;
> + int i;
> +
> + qfprom_cdata = (u32 *)qfprom_read(tmdev->dev, "calib");
> + if (IS_ERR(qfprom_cdata))
> + return PTR_ERR(qfprom_cdata);
> +
> + mode = (qfprom_cdata[4] & CAL_SEL_MASK) >> CAL_SEL_SHIFT;
> + dev_dbg(tmdev->dev, "calibration mode is %d\n", mode);
> +
> + switch (mode) {
> + case TWO_PT_CALIB:
> + base1 = (qfprom_cdata[4] & BASE1_MASK) >> BASE1_SHIFT;
> + p2[0] = (qfprom_cdata[0] & S0_P2_MASK) >> S0_P2_SHIFT;
> + p2[1] = (qfprom_cdata[0] & S1_P2_MASK) >> S1_P2_SHIFT;
> + /* This value is split over two registers, 2 bits and 4 bits */
> + lsb = (qfprom_cdata[0] & S2_P2_MASK_1_0) >> S2_P2_SHIFT_1_0;
> + msb = (qfprom_cdata[1] & S2_P2_MASK_5_2) >> S2_P2_SHIFT_5_2;
> + p2[2] = msb << 2 | lsb;
> + p2[3] = (qfprom_cdata[1] & S3_P2_MASK) >> S3_P2_SHIFT;
> + p2[4] = (qfprom_cdata[1] & S4_P2_MASK) >> S4_P2_SHIFT;
> + p2[5] = (qfprom_cdata[2] & S5_P2_MASK) >> S5_P2_SHIFT;
> + p2[6] = (qfprom_cdata[2] & S6_P2_MASK) >> S6_P2_SHIFT;
> + /* This value is split over two registers, 2 bits and 4 bits */
> + lsb = (qfprom_cdata[2] & S7_P2_MASK_1_0) >> S7_P2_SHIFT_1_0;
> + msb = (qfprom_cdata[3] & S7_P2_MASK_5_2) >> S7_P2_SHIFT_5_2;
> + p2[7] = msb << 2 | lsb;
> + p2[8] = (qfprom_cdata[3] & S8_P2_MASK) >> S8_P2_SHIFT;
> + p2[9] = (qfprom_cdata[3] & S9_P2_MASK) >> S9_P2_SHIFT;
> + for (i = 0; i < tmdev->num_sensors; i++)
> + p2[i] = ((base1 + p2[i]) << 2);
> + /* Fall through */
> + case ONE_PT_CALIB2:
> + base0 = (qfprom_cdata[4] & BASE0_MASK) >> BASE0_SHIFT;
> + p1[0] = (qfprom_cdata[0] & S0_P1_MASK) >> S0_P1_SHIFT;
> + p1[1] = (qfprom_cdata[0] & S1_P1_MASK) >> S1_P1_SHIFT;
> + p1[2] = (qfprom_cdata[0] & S2_P1_MASK) >> S2_P1_SHIFT;
> + p1[3] = (qfprom_cdata[1] & S3_P1_MASK) >> S3_P1_SHIFT;
> + p1[4] = (qfprom_cdata[1] & S4_P1_MASK) >> S4_P1_SHIFT;
> + p1[5] = (qfprom_cdata[2] & S5_P1_MASK) >> S5_P1_SHIFT;
> + p1[6] = (qfprom_cdata[2] & S6_P1_MASK) >> S6_P1_SHIFT;
> + p1[7] = (qfprom_cdata[2] & S7_P1_MASK) >> S7_P1_SHIFT;
> + p1[8] = (qfprom_cdata[3] & S8_P1_MASK) >> S8_P1_SHIFT;
> + p1[9] = (qfprom_cdata[3] & S9_P1_MASK) >> S9_P1_SHIFT;
> + for (i = 0; i < tmdev->num_sensors; i++)
> + p1[i] = (((base0) + p1[i]) << 2);
> + break;
> + default:
> + for (i = 0; i < tmdev->num_sensors; i++) {
> + p1[i] = 500;
> + p2[i] = 780;
... Wouldn't be nice to know what 500 or 780 stands for here?
Why these defaults? Do we expect to have further patches to keep
updating these?
> + }
> + break;
> + }
> +
> + compute_intercept_slope(tmdev, p1, p2, mode);
> +
> + return 0;
> +}
> +
> +#define STATUS_OFFSET 0x44
> +#define LAST_TEMP_MASK 0x3ff
> +#define STATUS_VALID_BIT BIT(14)
> +
> +static int get_temp_tsens_v1(struct tsens_device *tmdev, int id, int *temp)
> +{
> + struct tsens_sensor *s = &tmdev->sensor[id];
> + u32 code;
> + unsigned int status_reg;
> + u32 last_temp = 0, last_temp2 = 0, last_temp3 = 0;
> + int ret;
> +
> + status_reg = tmdev->tm_offset + STATUS_OFFSET + s->hw_id * 4;
> + ret = regmap_read(tmdev->tm_map, status_reg, &code);
> + if (ret)
> + return ret;
> + last_temp = code & LAST_TEMP_MASK;
> + if (code & STATUS_VALID_BIT)
> + goto done;
> +
> + /* Try a second time */
> + ret = regmap_read(tmdev->tm_map, status_reg, &code);
> + if (ret)
> + return ret;
> + if (code & STATUS_VALID_BIT) {
> + last_temp = code & LAST_TEMP_MASK;
> + goto done;
> + } else {
> + last_temp2 = code & LAST_TEMP_MASK;
> + }
> +
> + /* Try a third/last time */
> + ret = regmap_read(tmdev->tm_map, status_reg, &code);
> + if (ret)
> + return ret;
> + if (code & STATUS_VALID_BIT) {
> + last_temp = code & LAST_TEMP_MASK;
> + goto done;
> + } else {
> + last_temp3 = code & LAST_TEMP_MASK;
> + }
> +
> + if (last_temp == last_temp2)
> + last_temp = last_temp2;
> + else if (last_temp2 == last_temp3)
> + last_temp = last_temp3;
> +done:
> + /* Convert temperature from ADC code to milliCelsius */
> + *temp = code_to_degc(last_temp, s) * 1000;
This three strikes/read approach seams awkward. Is this a broken
sensor or are you missing the programming steps? Maybe you need to
either wait for a IRQ on temp ready, or maybe you need simply wait
some amount of cycles before the sensor is ready with new temp/ADC
conversion, no?
Also, the goto's dont really help, as we dont really have any
resource to rollback here..
> +
> + return 0;
> +}
> +
> +static const struct tsens_ops ops_generic_v1 = {
> + .init = init_common,
> + .calibrate = calibrate_v1,
> + .get_temp = get_temp_tsens_v1,
> +};
> +
> +const struct tsens_data data_tsens_v1 = {
> + .ops = &ops_generic_v1,
> + .reg_offsets = { [SROT_CTRL_OFFSET] = 0x4 },
> +};
> diff --git a/drivers/thermal/qcom/tsens.c b/drivers/thermal/qcom/tsens.c
> index f1ec9bbe4717..d0cc0c09894a 100644
> --- a/drivers/thermal/qcom/tsens.c
> +++ b/drivers/thermal/qcom/tsens.c
> @@ -63,6 +63,9 @@ static const struct of_device_id tsens_table[] = {
> }, {
> .compatible = "qcom,msm8996-tsens",
> .data = &data_8996,
> + }, {
> + .compatible = "qcom,tsens-v1",
> + .data = &data_tsens_v1,
> }, {
> .compatible = "qcom,tsens-v2",
> .data = &data_tsens_v2,
> diff --git a/drivers/thermal/qcom/tsens.h b/drivers/thermal/qcom/tsens.h
> index 7b7feee5dc46..f8dc96c42b94 100644
> --- a/drivers/thermal/qcom/tsens.h
> +++ b/drivers/thermal/qcom/tsens.h
> @@ -90,9 +90,10 @@ char *qfprom_read(struct device *, const char *);
> void compute_intercept_slope(struct tsens_device *, u32 *, u32 *, u32);
> int init_common(struct tsens_device *);
> int get_temp_common(struct tsens_device *, int, int *);
> +int code_to_degc(u32 adc_code, const struct tsens_sensor *s);
>
> /* TSENS v1 targets */
> -extern const struct tsens_data data_8916, data_8974, data_8960;
> +extern const struct tsens_data data_8916, data_8974, data_8960, data_tsens_v1;
> /* TSENS v2 targets */
> extern const struct tsens_data data_8996, data_tsens_v2;
>
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3 4/4] arm64: dts: qcom: qcs404: Add thermal zones for each sensor
2018-11-27 16:29 ` [PATCH v3 4/4] arm64: dts: qcom: qcs404: Add thermal zones for each sensor Amit Kucheria
@ 2018-11-29 16:57 ` Eduardo Valentin
2018-12-04 11:26 ` Amit Kucheria
0 siblings, 1 reply; 11+ messages in thread
From: Eduardo Valentin @ 2018-11-29 16:57 UTC (permalink / raw)
To: Amit Kucheria
Cc: linux-kernel, linux-arm-msm, bjorn.andersson, andy.gross, vkoul,
khasim.mohammed, David Brown, Rob Herring, Mark Rutland
On Tue, Nov 27, 2018 at 09:59:07PM +0530, Amit Kucheria wrote:
> qcs404 has 10 sensors connected to the single TSENS IP. Define a thermal
> zone for each of those sensors to expose the temperature of each zone.
>
> Signed-off-by: Amit Kucheria <amit.kucheria@linaro.org>
> Reviewed-by: Vinod Koul <vkoul@kernel.org>
> Tested-by: Vinod Koul <vkoul@kernel.org>
> ---
> arch/arm64/boot/dts/qcom/qcs404.dtsi | 206 +++++++++++++++++++++++++++
> 1 file changed, 206 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/qcs404.dtsi b/arch/arm64/boot/dts/qcom/qcs404.dtsi
> index 57d14d8f0c90..cbc3fd378893 100644
> --- a/arch/arm64/boot/dts/qcom/qcs404.dtsi
> +++ b/arch/arm64/boot/dts/qcom/qcs404.dtsi
> @@ -30,6 +30,7 @@
> reg = <0x100>;
> enable-method = "psci";
> next-level-cache = <&L2_0>;
> + #cooling-cells= <2>;
> };
>
> CPU1: cpu@101 {
> @@ -38,6 +39,7 @@
> reg = <0x101>;
> enable-method = "psci";
> next-level-cache = <&L2_0>;
> + #cooling-cells= <2>;
> };
>
> CPU2: cpu@102 {
> @@ -46,6 +48,7 @@
> reg = <0x102>;
> enable-method = "psci";
> next-level-cache = <&L2_0>;
> + #cooling-cells= <2>;
> };
>
> CPU3: cpu@103 {
> @@ -54,6 +57,7 @@
> reg = <0x103>;
> enable-method = "psci";
> next-level-cache = <&L2_0>;
> + #cooling-cells= <2>;
> };
>
> L2_0: l2-cache {
> @@ -507,4 +511,206 @@
> #interrupt-cells = <2>;
> };
> };
> +
> + thermal-zones {
> + aoss-thermal {
> + polling-delay-passive = <250>;
> + polling-delay = <1000>;
> +
> + thermal-sensors = <&tsens 0>;
> +
> + trips {
> + aoss_alert: trip0 {
> + temperature = <75000>;
> + hysteresis = <2000>;
> + type = "passive";
A passive trip and yet no cooling map associate to it..
> + };
> + aoss_crit: trip1 {
> + temperature = <95000>;
> + hysteresis = <2000>;
> + type = "critical";
> + };
> + };
> + };
> +
> + dsp-thermal {
> + polling-delay-passive = <250>;
> + polling-delay = <1000>;
> +
> + thermal-sensors = <&tsens 1>;
> +
> + trips {
> + dsp_alert: trip0 {
> + temperature = <75000>;
> + hysteresis = <2000>;
> + type = "passive";
> + };
> + dsp_crit: trip1 {
> + temperature = <95000>;
> + hysteresis = <2000>;
> + type = "critical";
> + };
> + };
> + };
> +
> + lpass-thermal {
> + polling-delay-passive = <250>;
> + polling-delay = <1000>;
> +
> + thermal-sensors = <&tsens 2>;
> +
> + trips {
> + lpass_alert: trip0 {
> + temperature = <75000>;
> + hysteresis = <2000>;
> + type = "passive";
> + };
> + lpass_crit: trip1 {
> + temperature = <95000>;
> + hysteresis = <2000>;
> + type = "critical";
> + };
> + };
> + };
> +
> + wlan-thermal {
> + polling-delay-passive = <250>;
> + polling-delay = <1000>;
> +
> + thermal-sensors = <&tsens 3>;
> +
> + trips {
> + wlan_alert: trip0 {
> + temperature = <75000>;
> + hysteresis = <2000>;
> + type = "passive";
> + };
> + wlan_crit: trip1 {
> + temperature = <95000>;
> + hysteresis = <2000>;
> + type = "critical";
> + };
> + };
> + };
> +
> + cluster-thermal {
> + polling-delay-passive = <250>;
> + polling-delay = <1000>;
> +
> + thermal-sensors = <&tsens 4>;
> +
> + trips {
> + cluster_alert: cluster_alert {
> + temperature = <75000>;
> + hysteresis = <2000>;
> + type = "passive";
> + };
> + cluster_crit: cluster_crit {
> + temperature = <110000>;
> + hysteresis = <2000>;
> + type = "critical";
> + };
> + };
> + };
> +
> + cpu-thermal0 {
> + polling-delay-passive = <250>;
> + polling-delay = <1000>;
> +
> + thermal-sensors = <&tsens 5>;
> +
> + trips {
> + cpu_alert0: cpu_alert0 {
> + temperature = <75000>;
> + hysteresis = <2000>;
> + type = "passive";
> + };
> + cpu_crit0: cpu_crit0 {
> + temperature = <110000>;
> + hysteresis = <2000>;
> + type = "critical";
> + };
> + };
> + };
> +
> + cpu-thermal1 {
> + polling-delay-passive = <250>;
> + polling-delay = <1000>;
> +
> + thermal-sensors = <&tsens 6>;
> +
> + trips {
> + cpu_alert1: trip0 {
> + temperature = <75000>;
> + hysteresis = <2000>;
> + type = "passive";
> + };
> + cpu_crit1: trip1 {
> + temperature = <110000>;
> + hysteresis = <2000>;
> + type = "critical";
> + };
> + };
> + };
> +
> + cpu-thermal2 {
> + polling-delay-passive = <250>;
> + polling-delay = <1000>;
> +
> + thermal-sensors = <&tsens 7>;
> +
> + trips {
> + cpu_alert2: trip0 {
> + temperature = <75000>;
> + hysteresis = <2000>;
> + type = "passive";
> + };
> + cpu_crit2: trip1 {
> + temperature = <110000>;
> + hysteresis = <2000>;
> + type = "critical";
> + };
> + };
> + };
> +
> + cpu-thermal3 {
> + polling-delay-passive = <250>;
> + polling-delay = <1000>;
> +
> + thermal-sensors = <&tsens 8>;
> +
> + trips {
> + cpu_alert3: trip0 {
> + temperature = <75000>;
> + hysteresis = <2000>;
> + type = "passive";
> + };
> + cpu_crit3: trip1 {
> + temperature = <110000>;
> + hysteresis = <2000>;
> + type = "critical";
> + };
> + };
> + };
> +
> + gpu-thermal {
> + polling-delay-passive = <250>;
> + polling-delay = <1000>;
> +
> + thermal-sensors = <&tsens 9>;
> +
> + trips {
> + gpu_alert: trip0 {
> + temperature = <75000>;
> + hysteresis = <2000>;
> + type = "passive";
> + };
> + gpu_crit: trip1 {
> + temperature = <95000>;
> + hysteresis = <2000>;
> + type = "critical";
> + };
> + };
> + };
> + };
> };
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3 2/4] drivers: thermal: tsens: Add generic support for TSENS v1 IP
2018-11-29 16:56 ` Eduardo Valentin
@ 2018-12-04 11:24 ` Amit Kucheria
2018-12-04 16:43 ` Eduardo Valentin
0 siblings, 1 reply; 11+ messages in thread
From: Amit Kucheria @ 2018-12-04 11:24 UTC (permalink / raw)
To: Eduardo Valentin
Cc: Linux Kernel Mailing List, linux-arm-msm, Bjorn Andersson,
Andy Gross, vkoul, Khasim Syed Mohammed, Zhang Rui,
Daniel Lezcano
On Thu, Nov 29, 2018 at 10:26 PM Eduardo Valentin <edubezval@gmail.com> wrote:
>
> On Tue, Nov 27, 2018 at 09:59:05PM +0530, Amit Kucheria wrote:
<snip>
> > + qfprom_cdata = (u32 *)qfprom_read(tmdev->dev, "calib");
> > + if (IS_ERR(qfprom_cdata))
> > + return PTR_ERR(qfprom_cdata);
> > +
> > + mode = (qfprom_cdata[4] & CAL_SEL_MASK) >> CAL_SEL_SHIFT;
> > + dev_dbg(tmdev->dev, "calibration mode is %d\n", mode);
> > +
> > + switch (mode) {
> > + case TWO_PT_CALIB:
> > + base1 = (qfprom_cdata[4] & BASE1_MASK) >> BASE1_SHIFT;
> > + p2[0] = (qfprom_cdata[0] & S0_P2_MASK) >> S0_P2_SHIFT;
> > + p2[1] = (qfprom_cdata[0] & S1_P2_MASK) >> S1_P2_SHIFT;
> > + /* This value is split over two registers, 2 bits and 4 bits */
> > + lsb = (qfprom_cdata[0] & S2_P2_MASK_1_0) >> S2_P2_SHIFT_1_0;
> > + msb = (qfprom_cdata[1] & S2_P2_MASK_5_2) >> S2_P2_SHIFT_5_2;
> > + p2[2] = msb << 2 | lsb;
> > + p2[3] = (qfprom_cdata[1] & S3_P2_MASK) >> S3_P2_SHIFT;
> > + p2[4] = (qfprom_cdata[1] & S4_P2_MASK) >> S4_P2_SHIFT;
> > + p2[5] = (qfprom_cdata[2] & S5_P2_MASK) >> S5_P2_SHIFT;
> > + p2[6] = (qfprom_cdata[2] & S6_P2_MASK) >> S6_P2_SHIFT;
> > + /* This value is split over two registers, 2 bits and 4 bits */
> > + lsb = (qfprom_cdata[2] & S7_P2_MASK_1_0) >> S7_P2_SHIFT_1_0;
> > + msb = (qfprom_cdata[3] & S7_P2_MASK_5_2) >> S7_P2_SHIFT_5_2;
> > + p2[7] = msb << 2 | lsb;
> > + p2[8] = (qfprom_cdata[3] & S8_P2_MASK) >> S8_P2_SHIFT;
> > + p2[9] = (qfprom_cdata[3] & S9_P2_MASK) >> S9_P2_SHIFT;
> > + for (i = 0; i < tmdev->num_sensors; i++)
> > + p2[i] = ((base1 + p2[i]) << 2);
> > + /* Fall through */
> > + case ONE_PT_CALIB2:
> > + base0 = (qfprom_cdata[4] & BASE0_MASK) >> BASE0_SHIFT;
> > + p1[0] = (qfprom_cdata[0] & S0_P1_MASK) >> S0_P1_SHIFT;
> > + p1[1] = (qfprom_cdata[0] & S1_P1_MASK) >> S1_P1_SHIFT;
> > + p1[2] = (qfprom_cdata[0] & S2_P1_MASK) >> S2_P1_SHIFT;
> > + p1[3] = (qfprom_cdata[1] & S3_P1_MASK) >> S3_P1_SHIFT;
> > + p1[4] = (qfprom_cdata[1] & S4_P1_MASK) >> S4_P1_SHIFT;
> > + p1[5] = (qfprom_cdata[2] & S5_P1_MASK) >> S5_P1_SHIFT;
> > + p1[6] = (qfprom_cdata[2] & S6_P1_MASK) >> S6_P1_SHIFT;
> > + p1[7] = (qfprom_cdata[2] & S7_P1_MASK) >> S7_P1_SHIFT;
> > + p1[8] = (qfprom_cdata[3] & S8_P1_MASK) >> S8_P1_SHIFT;
> > + p1[9] = (qfprom_cdata[3] & S9_P1_MASK) >> S9_P1_SHIFT;
> > + for (i = 0; i < tmdev->num_sensors; i++)
> > + p1[i] = (((base0) + p1[i]) << 2);
> > + break;
> > + default:
> > + for (i = 0; i < tmdev->num_sensors; i++) {
> > + p1[i] = 500;
> > + p2[i] = 780;
> ... Wouldn't be nice to know what 500 or 780 stands for here?
Indeed, however I haven't found a suitable explanations in the
downstream trees or documentation yet. tsens-8974.c and tsens-8916.c
seem to have similar magic numbers.
> Why these defaults? Do we expect to have further patches to keep
> updating these?
No we don't expect them to change. I wish I had a better answer, but
honestly I don't. I'll keep looking for a explanation to replace these
magic numbers.
> > + }
> > + break;
> > + }
> > +
> > + compute_intercept_slope(tmdev, p1, p2, mode);
> > +
> > + return 0;
> > +}
> > +
> > +#define STATUS_OFFSET 0x44
> > +#define LAST_TEMP_MASK 0x3ff
> > +#define STATUS_VALID_BIT BIT(14)
> > +
> > +static int get_temp_tsens_v1(struct tsens_device *tmdev, int id, int *temp)
> > +{
> > + struct tsens_sensor *s = &tmdev->sensor[id];
> > + u32 code;
> > + unsigned int status_reg;
> > + u32 last_temp = 0, last_temp2 = 0, last_temp3 = 0;
> > + int ret;
> > +
> > + status_reg = tmdev->tm_offset + STATUS_OFFSET + s->hw_id * 4;
> > + ret = regmap_read(tmdev->tm_map, status_reg, &code);
> > + if (ret)
> > + return ret;
> > + last_temp = code & LAST_TEMP_MASK;
> > + if (code & STATUS_VALID_BIT)
> > + goto done;
> > +
> > + /* Try a second time */
> > + ret = regmap_read(tmdev->tm_map, status_reg, &code);
> > + if (ret)
> > + return ret;
> > + if (code & STATUS_VALID_BIT) {
> > + last_temp = code & LAST_TEMP_MASK;
> > + goto done;
> > + } else {
> > + last_temp2 = code & LAST_TEMP_MASK;
> > + }
> > +
> > + /* Try a third/last time */
> > + ret = regmap_read(tmdev->tm_map, status_reg, &code);
> > + if (ret)
> > + return ret;
> > + if (code & STATUS_VALID_BIT) {
> > + last_temp = code & LAST_TEMP_MASK;
> > + goto done;
> > + } else {
> > + last_temp3 = code & LAST_TEMP_MASK;
> > + }
> > +
> > + if (last_temp == last_temp2)
> > + last_temp = last_temp2;
> > + else if (last_temp2 == last_temp3)
> > + last_temp = last_temp3;
> > +done:
> > + /* Convert temperature from ADC code to milliCelsius */
> > + *temp = code_to_degc(last_temp, s) * 1000;
>
> This three strikes/read approach seams awkward. Is this a broken
> sensor or are you missing the programming steps? Maybe you need to
> either wait for a IRQ on temp ready, or maybe you need simply wait
> some amount of cycles before the sensor is ready with new temp/ADC
> conversion, no?
This "algorithm" for reading data is specified in the Tsens IP HW
documentation almost word for word. The status_reg contains both the
VALID_BIT and the temperature, so we don't know if this is valid data
until we read the register. I'll be posting IRQ support soon.
Again, this algorithm is used for tsens-v2.c and tsens-8916.c so I
haven't tried to clean this up, yet. I have an experimental branch to
hide IP-specific register changes behind the regmap interface. This
should get rid of copies of these functions across the various files
but that isn't yet ready to post yet.
> Also, the goto's dont really help, as we dont really have any
> resource to rollback here..
Agreed. :-) This function could be made much clearer.
If I fix this, should I fix it in the other files right away, or can
we wait a bit more to clean this up as part of the move to regmap? I
think it'd be logically simpier to clean this up in a separate series
than introducing changes to the known working function as part of new
machine enablement.
Thanks for the review.
Regards,
Amit
> > +
> > + return 0;
> > +}
> > +
> > +static const struct tsens_ops ops_generic_v1 = {
> > + .init = init_common,
> > + .calibrate = calibrate_v1,
> > + .get_temp = get_temp_tsens_v1,
> > +};
> > +
> > +const struct tsens_data data_tsens_v1 = {
> > + .ops = &ops_generic_v1,
> > + .reg_offsets = { [SROT_CTRL_OFFSET] = 0x4 },
> > +};
> > diff --git a/drivers/thermal/qcom/tsens.c b/drivers/thermal/qcom/tsens.c
> > index f1ec9bbe4717..d0cc0c09894a 100644
> > --- a/drivers/thermal/qcom/tsens.c
> > +++ b/drivers/thermal/qcom/tsens.c
> > @@ -63,6 +63,9 @@ static const struct of_device_id tsens_table[] = {
> > }, {
> > .compatible = "qcom,msm8996-tsens",
> > .data = &data_8996,
> > + }, {
> > + .compatible = "qcom,tsens-v1",
> > + .data = &data_tsens_v1,
> > }, {
> > .compatible = "qcom,tsens-v2",
> > .data = &data_tsens_v2,
> > diff --git a/drivers/thermal/qcom/tsens.h b/drivers/thermal/qcom/tsens.h
> > index 7b7feee5dc46..f8dc96c42b94 100644
> > --- a/drivers/thermal/qcom/tsens.h
> > +++ b/drivers/thermal/qcom/tsens.h
> > @@ -90,9 +90,10 @@ char *qfprom_read(struct device *, const char *);
> > void compute_intercept_slope(struct tsens_device *, u32 *, u32 *, u32);
> > int init_common(struct tsens_device *);
> > int get_temp_common(struct tsens_device *, int, int *);
> > +int code_to_degc(u32 adc_code, const struct tsens_sensor *s);
> >
> > /* TSENS v1 targets */
> > -extern const struct tsens_data data_8916, data_8974, data_8960;
> > +extern const struct tsens_data data_8916, data_8974, data_8960, data_tsens_v1;
> > /* TSENS v2 targets */
> > extern const struct tsens_data data_8996, data_tsens_v2;
> >
> > --
> > 2.17.1
> >
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3 4/4] arm64: dts: qcom: qcs404: Add thermal zones for each sensor
2018-11-29 16:57 ` Eduardo Valentin
@ 2018-12-04 11:26 ` Amit Kucheria
2018-12-04 16:36 ` Eduardo Valentin
0 siblings, 1 reply; 11+ messages in thread
From: Amit Kucheria @ 2018-12-04 11:26 UTC (permalink / raw)
To: Eduardo Valentin
Cc: Linux Kernel Mailing List, linux-arm-msm, Bjorn Andersson,
Andy Gross, vkoul, Khasim Syed Mohammed, David Brown,
Rob Herring, Mark Rutland
On Thu, Nov 29, 2018 at 10:27 PM Eduardo Valentin <edubezval@gmail.com> wrote:
>
> On Tue, Nov 27, 2018 at 09:59:07PM +0530, Amit Kucheria wrote:
> > qcs404 has 10 sensors connected to the single TSENS IP. Define a thermal
> > zone for each of those sensors to expose the temperature of each zone.
> >
> > Signed-off-by: Amit Kucheria <amit.kucheria@linaro.org>
> > Reviewed-by: Vinod Koul <vkoul@kernel.org>
> > Tested-by: Vinod Koul <vkoul@kernel.org>
> > ---
> > arch/arm64/boot/dts/qcom/qcs404.dtsi | 206 +++++++++++++++++++++++++++
> > 1 file changed, 206 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/qcom/qcs404.dtsi b/arch/arm64/boot/dts/qcom/qcs404.dtsi
> > index 57d14d8f0c90..cbc3fd378893 100644
> > --- a/arch/arm64/boot/dts/qcom/qcs404.dtsi
> > +++ b/arch/arm64/boot/dts/qcom/qcs404.dtsi
> > @@ -30,6 +30,7 @@
> > reg = <0x100>;
> > enable-method = "psci";
> > next-level-cache = <&L2_0>;
> > + #cooling-cells= <2>;
> > };
> >
> > CPU1: cpu@101 {
> > @@ -38,6 +39,7 @@
> > reg = <0x101>;
> > enable-method = "psci";
> > next-level-cache = <&L2_0>;
> > + #cooling-cells= <2>;
> > };
> >
> > CPU2: cpu@102 {
> > @@ -46,6 +48,7 @@
> > reg = <0x102>;
> > enable-method = "psci";
> > next-level-cache = <&L2_0>;
> > + #cooling-cells= <2>;
> > };
> >
> > CPU3: cpu@103 {
> > @@ -54,6 +57,7 @@
> > reg = <0x103>;
> > enable-method = "psci";
> > next-level-cache = <&L2_0>;
> > + #cooling-cells= <2>;
> > };
> >
> > L2_0: l2-cache {
> > @@ -507,4 +511,206 @@
> > #interrupt-cells = <2>;
> > };
> > };
> > +
> > + thermal-zones {
> > + aoss-thermal {
> > + polling-delay-passive = <250>;
> > + polling-delay = <1000>;
> > +
> > + thermal-sensors = <&tsens 0>;
> > +
> > + trips {
> > + aoss_alert: trip0 {
> > + temperature = <75000>;
> > + hysteresis = <2000>;
> > + type = "passive";
>
>
> A passive trip and yet no cooling map associate to it..
Oops, was waiting for cpufreq to be enabled on the platform. I'll get
rid of the trips for now.
>
> > + };
> > + aoss_crit: trip1 {
> > + temperature = <95000>;
> > + hysteresis = <2000>;
> > + type = "critical";
> > + };
> > + };
> > + };
> > +
> > + dsp-thermal {
> > + polling-delay-passive = <250>;
> > + polling-delay = <1000>;
> > +
> > + thermal-sensors = <&tsens 1>;
> > +
> > + trips {
> > + dsp_alert: trip0 {
> > + temperature = <75000>;
> > + hysteresis = <2000>;
> > + type = "passive";
> > + };
> > + dsp_crit: trip1 {
> > + temperature = <95000>;
> > + hysteresis = <2000>;
> > + type = "critical";
> > + };
> > + };
> > + };
> > +
> > + lpass-thermal {
> > + polling-delay-passive = <250>;
> > + polling-delay = <1000>;
> > +
> > + thermal-sensors = <&tsens 2>;
> > +
> > + trips {
> > + lpass_alert: trip0 {
> > + temperature = <75000>;
> > + hysteresis = <2000>;
> > + type = "passive";
> > + };
> > + lpass_crit: trip1 {
> > + temperature = <95000>;
> > + hysteresis = <2000>;
> > + type = "critical";
> > + };
> > + };
> > + };
> > +
> > + wlan-thermal {
> > + polling-delay-passive = <250>;
> > + polling-delay = <1000>;
> > +
> > + thermal-sensors = <&tsens 3>;
> > +
> > + trips {
> > + wlan_alert: trip0 {
> > + temperature = <75000>;
> > + hysteresis = <2000>;
> > + type = "passive";
> > + };
> > + wlan_crit: trip1 {
> > + temperature = <95000>;
> > + hysteresis = <2000>;
> > + type = "critical";
> > + };
> > + };
> > + };
> > +
> > + cluster-thermal {
> > + polling-delay-passive = <250>;
> > + polling-delay = <1000>;
> > +
> > + thermal-sensors = <&tsens 4>;
> > +
> > + trips {
> > + cluster_alert: cluster_alert {
> > + temperature = <75000>;
> > + hysteresis = <2000>;
> > + type = "passive";
> > + };
> > + cluster_crit: cluster_crit {
> > + temperature = <110000>;
> > + hysteresis = <2000>;
> > + type = "critical";
> > + };
> > + };
> > + };
> > +
> > + cpu-thermal0 {
> > + polling-delay-passive = <250>;
> > + polling-delay = <1000>;
> > +
> > + thermal-sensors = <&tsens 5>;
> > +
> > + trips {
> > + cpu_alert0: cpu_alert0 {
> > + temperature = <75000>;
> > + hysteresis = <2000>;
> > + type = "passive";
> > + };
> > + cpu_crit0: cpu_crit0 {
> > + temperature = <110000>;
> > + hysteresis = <2000>;
> > + type = "critical";
> > + };
> > + };
> > + };
> > +
> > + cpu-thermal1 {
> > + polling-delay-passive = <250>;
> > + polling-delay = <1000>;
> > +
> > + thermal-sensors = <&tsens 6>;
> > +
> > + trips {
> > + cpu_alert1: trip0 {
> > + temperature = <75000>;
> > + hysteresis = <2000>;
> > + type = "passive";
> > + };
> > + cpu_crit1: trip1 {
> > + temperature = <110000>;
> > + hysteresis = <2000>;
> > + type = "critical";
> > + };
> > + };
> > + };
> > +
> > + cpu-thermal2 {
> > + polling-delay-passive = <250>;
> > + polling-delay = <1000>;
> > +
> > + thermal-sensors = <&tsens 7>;
> > +
> > + trips {
> > + cpu_alert2: trip0 {
> > + temperature = <75000>;
> > + hysteresis = <2000>;
> > + type = "passive";
> > + };
> > + cpu_crit2: trip1 {
> > + temperature = <110000>;
> > + hysteresis = <2000>;
> > + type = "critical";
> > + };
> > + };
> > + };
> > +
> > + cpu-thermal3 {
> > + polling-delay-passive = <250>;
> > + polling-delay = <1000>;
> > +
> > + thermal-sensors = <&tsens 8>;
> > +
> > + trips {
> > + cpu_alert3: trip0 {
> > + temperature = <75000>;
> > + hysteresis = <2000>;
> > + type = "passive";
> > + };
> > + cpu_crit3: trip1 {
> > + temperature = <110000>;
> > + hysteresis = <2000>;
> > + type = "critical";
> > + };
> > + };
> > + };
> > +
> > + gpu-thermal {
> > + polling-delay-passive = <250>;
> > + polling-delay = <1000>;
> > +
> > + thermal-sensors = <&tsens 9>;
> > +
> > + trips {
> > + gpu_alert: trip0 {
> > + temperature = <75000>;
> > + hysteresis = <2000>;
> > + type = "passive";
> > + };
> > + gpu_crit: trip1 {
> > + temperature = <95000>;
> > + hysteresis = <2000>;
> > + type = "critical";
> > + };
> > + };
> > + };
> > + };
> > };
> > --
> > 2.17.1
> >
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3 4/4] arm64: dts: qcom: qcs404: Add thermal zones for each sensor
2018-12-04 11:26 ` Amit Kucheria
@ 2018-12-04 16:36 ` Eduardo Valentin
0 siblings, 0 replies; 11+ messages in thread
From: Eduardo Valentin @ 2018-12-04 16:36 UTC (permalink / raw)
To: Amit Kucheria
Cc: Linux Kernel Mailing List, linux-arm-msm, Bjorn Andersson,
Andy Gross, vkoul, Khasim Syed Mohammed, David Brown,
Rob Herring, Mark Rutland
On Tue, Dec 04, 2018 at 04:56:15PM +0530, Amit Kucheria wrote:
> On Thu, Nov 29, 2018 at 10:27 PM Eduardo Valentin <edubezval@gmail.com> wrote:
> >
> > On Tue, Nov 27, 2018 at 09:59:07PM +0530, Amit Kucheria wrote:
> > > qcs404 has 10 sensors connected to the single TSENS IP. Define a thermal
> > > zone for each of those sensors to expose the temperature of each zone.
> > >
> > > Signed-off-by: Amit Kucheria <amit.kucheria@linaro.org>
> > > Reviewed-by: Vinod Koul <vkoul@kernel.org>
> > > Tested-by: Vinod Koul <vkoul@kernel.org>
> > > ---
> > > arch/arm64/boot/dts/qcom/qcs404.dtsi | 206 +++++++++++++++++++++++++++
> > > 1 file changed, 206 insertions(+)
> > >
> > > diff --git a/arch/arm64/boot/dts/qcom/qcs404.dtsi b/arch/arm64/boot/dts/qcom/qcs404.dtsi
> > > index 57d14d8f0c90..cbc3fd378893 100644
> > > --- a/arch/arm64/boot/dts/qcom/qcs404.dtsi
> > > +++ b/arch/arm64/boot/dts/qcom/qcs404.dtsi
> > > @@ -30,6 +30,7 @@
> > > reg = <0x100>;
> > > enable-method = "psci";
> > > next-level-cache = <&L2_0>;
> > > + #cooling-cells= <2>;
> > > };
> > >
> > > CPU1: cpu@101 {
> > > @@ -38,6 +39,7 @@
> > > reg = <0x101>;
> > > enable-method = "psci";
> > > next-level-cache = <&L2_0>;
> > > + #cooling-cells= <2>;
> > > };
> > >
> > > CPU2: cpu@102 {
> > > @@ -46,6 +48,7 @@
> > > reg = <0x102>;
> > > enable-method = "psci";
> > > next-level-cache = <&L2_0>;
> > > + #cooling-cells= <2>;
> > > };
> > >
> > > CPU3: cpu@103 {
> > > @@ -54,6 +57,7 @@
> > > reg = <0x103>;
> > > enable-method = "psci";
> > > next-level-cache = <&L2_0>;
> > > + #cooling-cells= <2>;
> > > };
> > >
> > > L2_0: l2-cache {
> > > @@ -507,4 +511,206 @@
> > > #interrupt-cells = <2>;
> > > };
> > > };
> > > +
> > > + thermal-zones {
> > > + aoss-thermal {
> > > + polling-delay-passive = <250>;
> > > + polling-delay = <1000>;
> > > +
> > > + thermal-sensors = <&tsens 0>;
> > > +
> > > + trips {
> > > + aoss_alert: trip0 {
> > > + temperature = <75000>;
> > > + hysteresis = <2000>;
> > > + type = "passive";
> >
> >
> > A passive trip and yet no cooling map associate to it..
>
> Oops, was waiting for cpufreq to be enabled on the platform. I'll get
> rid of the trips for now.
Well, the expectation is to have a fully defined thermal zone. Without
trips or cooling maps they are not complete.
>
> >
> > > + };
> > > + aoss_crit: trip1 {
> > > + temperature = <95000>;
> > > + hysteresis = <2000>;
> > > + type = "critical";
> > > + };
> > > + };
> > > + };
> > > +
> > > + dsp-thermal {
> > > + polling-delay-passive = <250>;
> > > + polling-delay = <1000>;
> > > +
> > > + thermal-sensors = <&tsens 1>;
> > > +
> > > + trips {
> > > + dsp_alert: trip0 {
> > > + temperature = <75000>;
> > > + hysteresis = <2000>;
> > > + type = "passive";
> > > + };
> > > + dsp_crit: trip1 {
> > > + temperature = <95000>;
> > > + hysteresis = <2000>;
> > > + type = "critical";
> > > + };
> > > + };
> > > + };
> > > +
> > > + lpass-thermal {
> > > + polling-delay-passive = <250>;
> > > + polling-delay = <1000>;
> > > +
> > > + thermal-sensors = <&tsens 2>;
> > > +
> > > + trips {
> > > + lpass_alert: trip0 {
> > > + temperature = <75000>;
> > > + hysteresis = <2000>;
> > > + type = "passive";
> > > + };
> > > + lpass_crit: trip1 {
> > > + temperature = <95000>;
> > > + hysteresis = <2000>;
> > > + type = "critical";
> > > + };
> > > + };
> > > + };
> > > +
> > > + wlan-thermal {
> > > + polling-delay-passive = <250>;
> > > + polling-delay = <1000>;
> > > +
> > > + thermal-sensors = <&tsens 3>;
> > > +
> > > + trips {
> > > + wlan_alert: trip0 {
> > > + temperature = <75000>;
> > > + hysteresis = <2000>;
> > > + type = "passive";
> > > + };
> > > + wlan_crit: trip1 {
> > > + temperature = <95000>;
> > > + hysteresis = <2000>;
> > > + type = "critical";
> > > + };
> > > + };
> > > + };
> > > +
> > > + cluster-thermal {
> > > + polling-delay-passive = <250>;
> > > + polling-delay = <1000>;
> > > +
> > > + thermal-sensors = <&tsens 4>;
> > > +
> > > + trips {
> > > + cluster_alert: cluster_alert {
> > > + temperature = <75000>;
> > > + hysteresis = <2000>;
> > > + type = "passive";
> > > + };
> > > + cluster_crit: cluster_crit {
> > > + temperature = <110000>;
> > > + hysteresis = <2000>;
> > > + type = "critical";
> > > + };
> > > + };
> > > + };
> > > +
> > > + cpu-thermal0 {
> > > + polling-delay-passive = <250>;
> > > + polling-delay = <1000>;
> > > +
> > > + thermal-sensors = <&tsens 5>;
> > > +
> > > + trips {
> > > + cpu_alert0: cpu_alert0 {
> > > + temperature = <75000>;
> > > + hysteresis = <2000>;
> > > + type = "passive";
> > > + };
> > > + cpu_crit0: cpu_crit0 {
> > > + temperature = <110000>;
> > > + hysteresis = <2000>;
> > > + type = "critical";
> > > + };
> > > + };
> > > + };
> > > +
> > > + cpu-thermal1 {
> > > + polling-delay-passive = <250>;
> > > + polling-delay = <1000>;
> > > +
> > > + thermal-sensors = <&tsens 6>;
> > > +
> > > + trips {
> > > + cpu_alert1: trip0 {
> > > + temperature = <75000>;
> > > + hysteresis = <2000>;
> > > + type = "passive";
> > > + };
> > > + cpu_crit1: trip1 {
> > > + temperature = <110000>;
> > > + hysteresis = <2000>;
> > > + type = "critical";
> > > + };
> > > + };
> > > + };
> > > +
> > > + cpu-thermal2 {
> > > + polling-delay-passive = <250>;
> > > + polling-delay = <1000>;
> > > +
> > > + thermal-sensors = <&tsens 7>;
> > > +
> > > + trips {
> > > + cpu_alert2: trip0 {
> > > + temperature = <75000>;
> > > + hysteresis = <2000>;
> > > + type = "passive";
> > > + };
> > > + cpu_crit2: trip1 {
> > > + temperature = <110000>;
> > > + hysteresis = <2000>;
> > > + type = "critical";
> > > + };
> > > + };
> > > + };
> > > +
> > > + cpu-thermal3 {
> > > + polling-delay-passive = <250>;
> > > + polling-delay = <1000>;
> > > +
> > > + thermal-sensors = <&tsens 8>;
> > > +
> > > + trips {
> > > + cpu_alert3: trip0 {
> > > + temperature = <75000>;
> > > + hysteresis = <2000>;
> > > + type = "passive";
> > > + };
> > > + cpu_crit3: trip1 {
> > > + temperature = <110000>;
> > > + hysteresis = <2000>;
> > > + type = "critical";
> > > + };
> > > + };
> > > + };
> > > +
> > > + gpu-thermal {
> > > + polling-delay-passive = <250>;
> > > + polling-delay = <1000>;
> > > +
> > > + thermal-sensors = <&tsens 9>;
> > > +
> > > + trips {
> > > + gpu_alert: trip0 {
> > > + temperature = <75000>;
> > > + hysteresis = <2000>;
> > > + type = "passive";
> > > + };
> > > + gpu_crit: trip1 {
> > > + temperature = <95000>;
> > > + hysteresis = <2000>;
> > > + type = "critical";
> > > + };
> > > + };
> > > + };
> > > + };
> > > };
> > > --
> > > 2.17.1
> > >
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3 2/4] drivers: thermal: tsens: Add generic support for TSENS v1 IP
2018-12-04 11:24 ` Amit Kucheria
@ 2018-12-04 16:43 ` Eduardo Valentin
0 siblings, 0 replies; 11+ messages in thread
From: Eduardo Valentin @ 2018-12-04 16:43 UTC (permalink / raw)
To: Amit Kucheria
Cc: Linux Kernel Mailing List, linux-arm-msm, Bjorn Andersson,
Andy Gross, vkoul, Khasim Syed Mohammed, Zhang Rui,
Daniel Lezcano
On Tue, Dec 04, 2018 at 04:54:21PM +0530, Amit Kucheria wrote:
> On Thu, Nov 29, 2018 at 10:26 PM Eduardo Valentin <edubezval@gmail.com> wrote:
> >
> > On Tue, Nov 27, 2018 at 09:59:05PM +0530, Amit Kucheria wrote:
>
> <snip>
>
> > > + qfprom_cdata = (u32 *)qfprom_read(tmdev->dev, "calib");
> > > + if (IS_ERR(qfprom_cdata))
> > > + return PTR_ERR(qfprom_cdata);
> > > +
> > > + mode = (qfprom_cdata[4] & CAL_SEL_MASK) >> CAL_SEL_SHIFT;
> > > + dev_dbg(tmdev->dev, "calibration mode is %d\n", mode);
> > > +
> > > + switch (mode) {
> > > + case TWO_PT_CALIB:
> > > + base1 = (qfprom_cdata[4] & BASE1_MASK) >> BASE1_SHIFT;
> > > + p2[0] = (qfprom_cdata[0] & S0_P2_MASK) >> S0_P2_SHIFT;
> > > + p2[1] = (qfprom_cdata[0] & S1_P2_MASK) >> S1_P2_SHIFT;
> > > + /* This value is split over two registers, 2 bits and 4 bits */
> > > + lsb = (qfprom_cdata[0] & S2_P2_MASK_1_0) >> S2_P2_SHIFT_1_0;
> > > + msb = (qfprom_cdata[1] & S2_P2_MASK_5_2) >> S2_P2_SHIFT_5_2;
> > > + p2[2] = msb << 2 | lsb;
> > > + p2[3] = (qfprom_cdata[1] & S3_P2_MASK) >> S3_P2_SHIFT;
> > > + p2[4] = (qfprom_cdata[1] & S4_P2_MASK) >> S4_P2_SHIFT;
> > > + p2[5] = (qfprom_cdata[2] & S5_P2_MASK) >> S5_P2_SHIFT;
> > > + p2[6] = (qfprom_cdata[2] & S6_P2_MASK) >> S6_P2_SHIFT;
> > > + /* This value is split over two registers, 2 bits and 4 bits */
> > > + lsb = (qfprom_cdata[2] & S7_P2_MASK_1_0) >> S7_P2_SHIFT_1_0;
> > > + msb = (qfprom_cdata[3] & S7_P2_MASK_5_2) >> S7_P2_SHIFT_5_2;
> > > + p2[7] = msb << 2 | lsb;
> > > + p2[8] = (qfprom_cdata[3] & S8_P2_MASK) >> S8_P2_SHIFT;
> > > + p2[9] = (qfprom_cdata[3] & S9_P2_MASK) >> S9_P2_SHIFT;
> > > + for (i = 0; i < tmdev->num_sensors; i++)
> > > + p2[i] = ((base1 + p2[i]) << 2);
> > > + /* Fall through */
> > > + case ONE_PT_CALIB2:
> > > + base0 = (qfprom_cdata[4] & BASE0_MASK) >> BASE0_SHIFT;
> > > + p1[0] = (qfprom_cdata[0] & S0_P1_MASK) >> S0_P1_SHIFT;
> > > + p1[1] = (qfprom_cdata[0] & S1_P1_MASK) >> S1_P1_SHIFT;
> > > + p1[2] = (qfprom_cdata[0] & S2_P1_MASK) >> S2_P1_SHIFT;
> > > + p1[3] = (qfprom_cdata[1] & S3_P1_MASK) >> S3_P1_SHIFT;
> > > + p1[4] = (qfprom_cdata[1] & S4_P1_MASK) >> S4_P1_SHIFT;
> > > + p1[5] = (qfprom_cdata[2] & S5_P1_MASK) >> S5_P1_SHIFT;
> > > + p1[6] = (qfprom_cdata[2] & S6_P1_MASK) >> S6_P1_SHIFT;
> > > + p1[7] = (qfprom_cdata[2] & S7_P1_MASK) >> S7_P1_SHIFT;
> > > + p1[8] = (qfprom_cdata[3] & S8_P1_MASK) >> S8_P1_SHIFT;
> > > + p1[9] = (qfprom_cdata[3] & S9_P1_MASK) >> S9_P1_SHIFT;
> > > + for (i = 0; i < tmdev->num_sensors; i++)
> > > + p1[i] = (((base0) + p1[i]) << 2);
> > > + break;
> > > + default:
> > > + for (i = 0; i < tmdev->num_sensors; i++) {
> > > + p1[i] = 500;
> > > + p2[i] = 780;
> > ... Wouldn't be nice to know what 500 or 780 stands for here?
>
> Indeed, however I haven't found a suitable explanations in the
> downstream trees or documentation yet. tsens-8974.c and tsens-8916.c
> seem to have similar magic numbers.
>
Well, it does not need to be exactly like it is in downstream, does it?
:-)
> > Why these defaults? Do we expect to have further patches to keep
> > updating these?
>
> No we don't expect them to change. I wish I had a better answer, but
> honestly I don't. I'll keep looking for a explanation to replace these
> magic numbers.
>
Yeah, I see, the problem that I have is that these are slope and
offsets, and these constants change from chip version/rev, essentially
due to sensor location, process, etc, so hardcoding in the probe seams
to be asking for future patching.
> > > + }
> > > + break;
> > > + }
> > > +
> > > + compute_intercept_slope(tmdev, p1, p2, mode);
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +#define STATUS_OFFSET 0x44
> > > +#define LAST_TEMP_MASK 0x3ff
> > > +#define STATUS_VALID_BIT BIT(14)
> > > +
> > > +static int get_temp_tsens_v1(struct tsens_device *tmdev, int id, int *temp)
> > > +{
> > > + struct tsens_sensor *s = &tmdev->sensor[id];
> > > + u32 code;
> > > + unsigned int status_reg;
> > > + u32 last_temp = 0, last_temp2 = 0, last_temp3 = 0;
> > > + int ret;
> > > +
> > > + status_reg = tmdev->tm_offset + STATUS_OFFSET + s->hw_id * 4;
> > > + ret = regmap_read(tmdev->tm_map, status_reg, &code);
> > > + if (ret)
> > > + return ret;
> > > + last_temp = code & LAST_TEMP_MASK;
> > > + if (code & STATUS_VALID_BIT)
> > > + goto done;
> > > +
> > > + /* Try a second time */
> > > + ret = regmap_read(tmdev->tm_map, status_reg, &code);
> > > + if (ret)
> > > + return ret;
> > > + if (code & STATUS_VALID_BIT) {
> > > + last_temp = code & LAST_TEMP_MASK;
> > > + goto done;
> > > + } else {
> > > + last_temp2 = code & LAST_TEMP_MASK;
> > > + }
> > > +
> > > + /* Try a third/last time */
> > > + ret = regmap_read(tmdev->tm_map, status_reg, &code);
> > > + if (ret)
> > > + return ret;
> > > + if (code & STATUS_VALID_BIT) {
> > > + last_temp = code & LAST_TEMP_MASK;
> > > + goto done;
> > > + } else {
> > > + last_temp3 = code & LAST_TEMP_MASK;
> > > + }
> > > +
> > > + if (last_temp == last_temp2)
> > > + last_temp = last_temp2;
> > > + else if (last_temp2 == last_temp3)
> > > + last_temp = last_temp3;
> > > +done:
> > > + /* Convert temperature from ADC code to milliCelsius */
> > > + *temp = code_to_degc(last_temp, s) * 1000;
> >
> > This three strikes/read approach seams awkward. Is this a broken
> > sensor or are you missing the programming steps? Maybe you need to
> > either wait for a IRQ on temp ready, or maybe you need simply wait
> > some amount of cycles before the sensor is ready with new temp/ADC
> > conversion, no?
>
> This "algorithm" for reading data is specified in the Tsens IP HW
> documentation almost word for word. The status_reg contains both the
> VALID_BIT and the temperature, so we don't know if this is valid data
> until we read the register. I'll be posting IRQ support soon.
>
> Again, this algorithm is used for tsens-v2.c and tsens-8916.c so I
> haven't tried to clean this up, yet. I have an experimental branch to
> hide IP-specific register changes behind the regmap interface. This
> should get rid of copies of these functions across the various files
> but that isn't yet ready to post yet.
>
> > Also, the goto's dont really help, as we dont really have any
> > resource to rollback here..
>
> Agreed. :-) This function could be made much clearer.
>
> If I fix this, should I fix it in the other files right away, or can
> we wait a bit more to clean this up as part of the move to regmap? I
> think it'd be logically simpier to clean this up in a separate series
> than introducing changes to the known working function as part of new
> machine enablement.
>
> Thanks for the review.
>
> Regards,
> Amit
>
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static const struct tsens_ops ops_generic_v1 = {
> > > + .init = init_common,
> > > + .calibrate = calibrate_v1,
> > > + .get_temp = get_temp_tsens_v1,
> > > +};
> > > +
> > > +const struct tsens_data data_tsens_v1 = {
> > > + .ops = &ops_generic_v1,
> > > + .reg_offsets = { [SROT_CTRL_OFFSET] = 0x4 },
> > > +};
> > > diff --git a/drivers/thermal/qcom/tsens.c b/drivers/thermal/qcom/tsens.c
> > > index f1ec9bbe4717..d0cc0c09894a 100644
> > > --- a/drivers/thermal/qcom/tsens.c
> > > +++ b/drivers/thermal/qcom/tsens.c
> > > @@ -63,6 +63,9 @@ static const struct of_device_id tsens_table[] = {
> > > }, {
> > > .compatible = "qcom,msm8996-tsens",
> > > .data = &data_8996,
> > > + }, {
> > > + .compatible = "qcom,tsens-v1",
> > > + .data = &data_tsens_v1,
> > > }, {
> > > .compatible = "qcom,tsens-v2",
> > > .data = &data_tsens_v2,
> > > diff --git a/drivers/thermal/qcom/tsens.h b/drivers/thermal/qcom/tsens.h
> > > index 7b7feee5dc46..f8dc96c42b94 100644
> > > --- a/drivers/thermal/qcom/tsens.h
> > > +++ b/drivers/thermal/qcom/tsens.h
> > > @@ -90,9 +90,10 @@ char *qfprom_read(struct device *, const char *);
> > > void compute_intercept_slope(struct tsens_device *, u32 *, u32 *, u32);
> > > int init_common(struct tsens_device *);
> > > int get_temp_common(struct tsens_device *, int, int *);
> > > +int code_to_degc(u32 adc_code, const struct tsens_sensor *s);
> > >
> > > /* TSENS v1 targets */
> > > -extern const struct tsens_data data_8916, data_8974, data_8960;
> > > +extern const struct tsens_data data_8916, data_8974, data_8960, data_tsens_v1;
> > > /* TSENS v2 targets */
> > > extern const struct tsens_data data_8996, data_tsens_v2;
> > >
> > > --
> > > 2.17.1
> > >
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2018-12-04 16:43 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-27 16:29 [PATCH v3 0/4] thermal: tsens: Add support for QCS404 platform Amit Kucheria
2018-11-27 16:29 ` [PATCH v3 1/4] dt: thermal: tsens: Add bindings for qcs404 Amit Kucheria
2018-11-27 16:29 ` [PATCH v3 2/4] drivers: thermal: tsens: Add generic support for TSENS v1 IP Amit Kucheria
2018-11-29 16:56 ` Eduardo Valentin
2018-12-04 11:24 ` Amit Kucheria
2018-12-04 16:43 ` Eduardo Valentin
2018-11-27 16:29 ` [PATCH v3 3/4] arm64: dts: qcom: qcs404: Add tsens controller Amit Kucheria
2018-11-27 16:29 ` [PATCH v3 4/4] arm64: dts: qcom: qcs404: Add thermal zones for each sensor Amit Kucheria
2018-11-29 16:57 ` Eduardo Valentin
2018-12-04 11:26 ` Amit Kucheria
2018-12-04 16:36 ` Eduardo Valentin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).