linux-pm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [v4,2/7] arm64: dts: mt8183: add dynamic power coefficients
       [not found] <20200323121537.22697-1-michael.kao@mediatek.com>
@ 2020-03-23 12:15 ` Michael Kao
  2020-03-23 12:15 ` [v4,3/7] arm64: dts: mt8183: Add #cooling-cells to CPU nodes Michael Kao
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 16+ messages in thread
From: Michael Kao @ 2020-03-23 12:15 UTC (permalink / raw)
  To: Matthias Brugger, Zhang Rui, Eduardo Valentin, Daniel Lezcano,
	Rob Herring, Mark Rutland, hsinyi, linux-pm, srv_heupstream,
	michael.kao
  Cc: devicetree, linux-kernel, linux-arm-kernel, linux-mediatek

From: "michael.kao" <michael.kao@mediatek.com>

Add dynamic power coefficients for all cores and update those of
CPU0 and CPU4.

Signed-off-by: Michael Kao <michael.kao@mediatek.com>
---
 arch/arm64/boot/dts/mediatek/mt8183.dtsi | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arch/arm64/boot/dts/mediatek/mt8183.dtsi b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
index 59f97217aaa8..2e2527c3369a 100644
--- a/arch/arm64/boot/dts/mediatek/mt8183.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
@@ -75,6 +75,7 @@
 			reg = <0x000>;
 			enable-method = "psci";
 			capacity-dmips-mhz = <741>;
+			dynamic-power-coefficient = <84>;
 		};
 
 		cpu1: cpu@1 {
@@ -83,6 +84,7 @@
 			reg = <0x001>;
 			enable-method = "psci";
 			capacity-dmips-mhz = <741>;
+			dynamic-power-coefficient = <84>;
 		};
 
 		cpu2: cpu@2 {
@@ -91,6 +93,7 @@
 			reg = <0x002>;
 			enable-method = "psci";
 			capacity-dmips-mhz = <741>;
+			dynamic-power-coefficient = <84>;
 		};
 
 		cpu3: cpu@3 {
@@ -99,6 +102,7 @@
 			reg = <0x003>;
 			enable-method = "psci";
 			capacity-dmips-mhz = <741>;
+			dynamic-power-coefficient = <84>;
 		};
 
 		cpu4: cpu@100 {
@@ -107,6 +111,7 @@
 			reg = <0x100>;
 			enable-method = "psci";
 			capacity-dmips-mhz = <1024>;
+			dynamic-power-coefficient = <211>;
 		};
 
 		cpu5: cpu@101 {
@@ -115,6 +120,7 @@
 			reg = <0x101>;
 			enable-method = "psci";
 			capacity-dmips-mhz = <1024>;
+			dynamic-power-coefficient = <211>;
 		};
 
 		cpu6: cpu@102 {
@@ -123,6 +129,7 @@
 			reg = <0x102>;
 			enable-method = "psci";
 			capacity-dmips-mhz = <1024>;
+			dynamic-power-coefficient = <211>;
 		};
 
 		cpu7: cpu@103 {
@@ -131,6 +138,7 @@
 			reg = <0x103>;
 			enable-method = "psci";
 			capacity-dmips-mhz = <1024>;
+			dynamic-power-coefficient = <211>;
 		};
 	};
 
-- 
2.18.0

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

* [v4,3/7] arm64: dts: mt8183: Add #cooling-cells to CPU nodes
       [not found] <20200323121537.22697-1-michael.kao@mediatek.com>
  2020-03-23 12:15 ` [v4,2/7] arm64: dts: mt8183: add dynamic power coefficients Michael Kao
@ 2020-03-23 12:15 ` Michael Kao
  2020-03-23 12:15 ` [v4,4/7] arm64: dts: mt8183: Configure CPU cooling Michael Kao
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 16+ messages in thread
From: Michael Kao @ 2020-03-23 12:15 UTC (permalink / raw)
  To: Matthias Brugger, Zhang Rui, Eduardo Valentin, Daniel Lezcano,
	Rob Herring, Mark Rutland, hsinyi, linux-pm, srv_heupstream,
	michael.kao
  Cc: devicetree, linux-kernel, linux-arm-kernel, linux-mediatek

From: "michael.kao" <michael.kao@mediatek.com>

The #cooling-cells property needs to be specified to allow a CPU
to be used as cooling device.

Signed-off-by: Michael Kao <michael.kao@mediatek.com>
---
 arch/arm64/boot/dts/mediatek/mt8183.dtsi | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/arch/arm64/boot/dts/mediatek/mt8183.dtsi b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
index 2e2527c3369a..182fa6264e0d 100644
--- a/arch/arm64/boot/dts/mediatek/mt8183.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
@@ -11,6 +11,7 @@
 #include <dt-bindings/power/mt8183-power.h>
 #include <dt-bindings/reset-controller/mt8183-resets.h>
 #include "mt8183-pinfunc.h"
+#include <dt-bindings/thermal/thermal.h>
 
 / {
 	compatible = "mediatek,mt8183";
@@ -76,6 +77,7 @@
 			enable-method = "psci";
 			capacity-dmips-mhz = <741>;
 			dynamic-power-coefficient = <84>;
+			#cooling-cells = <2>;
 		};
 
 		cpu1: cpu@1 {
@@ -85,6 +87,7 @@
 			enable-method = "psci";
 			capacity-dmips-mhz = <741>;
 			dynamic-power-coefficient = <84>;
+			#cooling-cells = <2>;
 		};
 
 		cpu2: cpu@2 {
@@ -94,6 +97,7 @@
 			enable-method = "psci";
 			capacity-dmips-mhz = <741>;
 			dynamic-power-coefficient = <84>;
+			#cooling-cells = <2>;
 		};
 
 		cpu3: cpu@3 {
@@ -103,6 +107,7 @@
 			enable-method = "psci";
 			capacity-dmips-mhz = <741>;
 			dynamic-power-coefficient = <84>;
+			#cooling-cells = <2>;
 		};
 
 		cpu4: cpu@100 {
@@ -112,6 +117,7 @@
 			enable-method = "psci";
 			capacity-dmips-mhz = <1024>;
 			dynamic-power-coefficient = <211>;
+			#cooling-cells = <2>;
 		};
 
 		cpu5: cpu@101 {
@@ -121,6 +127,7 @@
 			enable-method = "psci";
 			capacity-dmips-mhz = <1024>;
 			dynamic-power-coefficient = <211>;
+			#cooling-cells = <2>;
 		};
 
 		cpu6: cpu@102 {
@@ -130,6 +137,7 @@
 			enable-method = "psci";
 			capacity-dmips-mhz = <1024>;
 			dynamic-power-coefficient = <211>;
+			#cooling-cells = <2>;
 		};
 
 		cpu7: cpu@103 {
@@ -139,6 +147,7 @@
 			enable-method = "psci";
 			capacity-dmips-mhz = <1024>;
 			dynamic-power-coefficient = <211>;
+			#cooling-cells = <2>;
 		};
 	};
 
-- 
2.18.0

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

* [v4,4/7] arm64: dts: mt8183: Configure CPU cooling
       [not found] <20200323121537.22697-1-michael.kao@mediatek.com>
  2020-03-23 12:15 ` [v4,2/7] arm64: dts: mt8183: add dynamic power coefficients Michael Kao
  2020-03-23 12:15 ` [v4,3/7] arm64: dts: mt8183: Add #cooling-cells to CPU nodes Michael Kao
@ 2020-03-23 12:15 ` Michael Kao
  2020-03-23 12:15 ` [v4,5/7] thermal: mediatek: mt8183: fix bank number settings Michael Kao
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 16+ messages in thread
From: Michael Kao @ 2020-03-23 12:15 UTC (permalink / raw)
  To: Matthias Brugger, Zhang Rui, Eduardo Valentin, Daniel Lezcano,
	Rob Herring, Mark Rutland, hsinyi, linux-pm, srv_heupstream,
	michael.kao
  Cc: devicetree, linux-kernel, linux-arm-kernel, linux-mediatek,
	Matthias Kaehlcke

From: Matthias Kaehlcke <mka@chromium.org>

Add two passive trip points at 68°C and 80°C for the CPU temperature.

Signed-off-by: Matthias Kaehlcke <mka@chromium.org>
Signed-off-by: Michael Kao <michael.kao@mediatek.com>
---
 arch/arm64/boot/dts/mediatek/mt8183.dtsi | 55 ++++++++++++++++++++++++
 1 file changed, 55 insertions(+)

diff --git a/arch/arm64/boot/dts/mediatek/mt8183.dtsi b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
index 182fa6264e0d..59ab2957d85d 100644
--- a/arch/arm64/boot/dts/mediatek/mt8183.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
@@ -675,6 +675,61 @@
 				polling-delay = <500>;
 				thermal-sensors = <&thermal 0>;
 				sustainable-power = <5000>;
+
+				trips {
+					threshold: trip-point@0 {
+						temperature = <68000>;
+						hysteresis = <2000>;
+						type = "passive";
+					};
+
+					target: trip-point@1 {
+						temperature = <80000>;
+						hysteresis = <2000>;
+						type = "passive";
+					};
+
+					cpu_crit: cpu-crit {
+						temperature = <115000>;
+						hysteresis = <2000>;
+						type = "critical";
+					};
+				};
+
+				cooling-maps {
+					map0 {
+						trip = <&target>;
+						cooling-device = <&cpu0
+							THERMAL_NO_LIMIT
+							THERMAL_NO_LIMIT>,
+								 <&cpu1
+							THERMAL_NO_LIMIT
+							THERMAL_NO_LIMIT>,
+								 <&cpu2
+							THERMAL_NO_LIMIT
+							THERMAL_NO_LIMIT>,
+								 <&cpu3
+							THERMAL_NO_LIMIT
+							THERMAL_NO_LIMIT>;
+						contribution = <3072>;
+					};
+					map1 {
+						trip = <&target>;
+						cooling-device = <&cpu4
+							THERMAL_NO_LIMIT
+							THERMAL_NO_LIMIT>,
+								 <&cpu5
+							THERMAL_NO_LIMIT
+							THERMAL_NO_LIMIT>,
+								 <&cpu6
+							THERMAL_NO_LIMIT
+							THERMAL_NO_LIMIT>,
+								 <&cpu7
+							THERMAL_NO_LIMIT
+							THERMAL_NO_LIMIT>;
+						contribution = <1024>;
+					};
+				};
 			};
 
 			/* The tzts1 ~ tzts6 don't need to polling */
-- 
2.18.0

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

* [v4,5/7] thermal: mediatek: mt8183: fix bank number settings
       [not found] <20200323121537.22697-1-michael.kao@mediatek.com>
                   ` (2 preceding siblings ...)
  2020-03-23 12:15 ` [v4,4/7] arm64: dts: mt8183: Configure CPU cooling Michael Kao
@ 2020-03-23 12:15 ` Michael Kao
  2020-05-22 15:33   ` Daniel Lezcano
  2020-03-23 12:15 ` [v4,6/7] thermal: mediatek: add another get_temp ops for thermal sensors Michael Kao
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 16+ messages in thread
From: Michael Kao @ 2020-03-23 12:15 UTC (permalink / raw)
  To: Matthias Brugger, Zhang Rui, Eduardo Valentin, Daniel Lezcano,
	Rob Herring, Mark Rutland, hsinyi, linux-pm, srv_heupstream,
	michael.kao
  Cc: devicetree, linux-kernel, linux-arm-kernel, linux-mediatek

MT8183_NUM_ZONES should be set to 1
because MT8183 doesn't have multiple banks.

Fixes: a4ffe6b52d27 ("thermal: mediatek: add support for MT8183")
Signed-off-by: Michael Kao <michael.kao@mediatek.com>
Signed-off-by: Hsin-Yi Wang <hsinyi@chromium.org>
---
 drivers/thermal/mtk_thermal.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/thermal/mtk_thermal.c b/drivers/thermal/mtk_thermal.c
index 76e30603d4d5..6b7ef1993d7e 100644
--- a/drivers/thermal/mtk_thermal.c
+++ b/drivers/thermal/mtk_thermal.c
@@ -211,6 +211,9 @@ enum {
 /* The total number of temperature sensors in the MT8183 */
 #define MT8183_NUM_SENSORS	6
 
+/* The number of banks in the MT8183 */
+#define MT8183_NUM_ZONES               1
+
 /* The number of sensing points per bank */
 #define MT8183_NUM_SENSORS_PER_ZONE	 6
 
@@ -497,7 +500,7 @@ static const struct mtk_thermal_data mt7622_thermal_data = {
  */
 static const struct mtk_thermal_data mt8183_thermal_data = {
 	.auxadc_channel = MT8183_TEMP_AUXADC_CHANNEL,
-	.num_banks = MT8183_NUM_SENSORS_PER_ZONE,
+	.num_banks = MT8183_NUM_ZONES,
 	.num_sensors = MT8183_NUM_SENSORS,
 	.vts_index = mt8183_vts_index,
 	.cali_val = MT8183_CALIBRATION,
-- 
2.18.0

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

* [v4,6/7] thermal: mediatek: add another get_temp ops for thermal sensors
       [not found] <20200323121537.22697-1-michael.kao@mediatek.com>
                   ` (3 preceding siblings ...)
  2020-03-23 12:15 ` [v4,5/7] thermal: mediatek: mt8183: fix bank number settings Michael Kao
@ 2020-03-23 12:15 ` Michael Kao
  2020-05-22 15:32   ` Daniel Lezcano
  2020-05-21 12:51 ` [v4,0/7] Add Mediatek thermal dirver and dtsi Matthias Brugger
       [not found] ` <20200323121537.22697-8-michael.kao@mediatek.com>
  6 siblings, 1 reply; 16+ messages in thread
From: Michael Kao @ 2020-03-23 12:15 UTC (permalink / raw)
  To: Matthias Brugger, Zhang Rui, Eduardo Valentin, Daniel Lezcano,
	Rob Herring, Mark Rutland, hsinyi, linux-pm, srv_heupstream,
	michael.kao
  Cc: devicetree, linux-kernel, linux-arm-kernel, linux-mediatek

Provide thermal zone to read thermal sensor
in the SoC. We can read all the thermal sensors
value in the SoC by the node /sys/class/thermal/

In mtk_thermal_bank_temperature, return -EAGAIN instead of -EACCESS
on the first read of sensor that often are bogus values.
This can avoid following warning on boot:

  thermal thermal_zone6: failed to read out thermal zone (-13)

Signed-off-by: Michael Kao <michael.kao@mediatek.com>
Signed-off-by: Hsin-Yi Wang <hsinyi@chromium.org>
---
 drivers/thermal/mtk_thermal.c | 74 +++++++++++++++++++++++++++++++----
 1 file changed, 67 insertions(+), 7 deletions(-)

diff --git a/drivers/thermal/mtk_thermal.c b/drivers/thermal/mtk_thermal.c
index 6b7ef1993d7e..9eaca432920e 100644
--- a/drivers/thermal/mtk_thermal.c
+++ b/drivers/thermal/mtk_thermal.c
@@ -225,6 +225,11 @@ enum {
 
 struct mtk_thermal;
 
+struct mtk_thermal_zone {
+	struct mtk_thermal *mt;
+	int id;
+};
+
 struct thermal_bank_cfg {
 	unsigned int num_sensors;
 	const int *sensors;
@@ -607,7 +612,7 @@ static int mtk_thermal_bank_temperature(struct mtk_thermal_bank *bank)
 		 * not immediately shut down.
 		 */
 		if (temp > 200000)
-			temp = 0;
+			temp = -EAGAIN;
 
 		if (temp > max)
 			max = temp;
@@ -618,7 +623,8 @@ static int mtk_thermal_bank_temperature(struct mtk_thermal_bank *bank)
 
 static int mtk_read_temp(void *data, int *temperature)
 {
-	struct mtk_thermal *mt = data;
+	struct mtk_thermal_zone *tz = data;
+	struct mtk_thermal *mt = tz->mt;
 	int i;
 	int tempmax = INT_MIN;
 
@@ -637,10 +643,44 @@ static int mtk_read_temp(void *data, int *temperature)
 	return 0;
 }
 
+static int mtk_read_sensor_temp(void *data, int *temperature)
+{
+	struct mtk_thermal_zone *tz = data;
+	struct mtk_thermal *mt = tz->mt;
+	const struct mtk_thermal_data *conf = mt->conf;
+	int id = tz->id - 1;
+	int temp = INT_MIN;
+	u32 raw;
+
+	if (id < 0)
+		return  -EACCES;
+
+	raw = readl(mt->thermal_base + conf->msr[id]);
+
+	temp = raw_to_mcelsius(mt, id, raw);
+
+	/*
+	 * The first read of a sensor often contains very high bogus
+	 * temperature value. Filter these out so that the system does
+	 * not immediately shut down.
+	 */
+
+	if (temp > 200000)
+		return  -EAGAIN;
+
+	*temperature = temp;
+
+	return 0;
+}
+
 static const struct thermal_zone_of_device_ops mtk_thermal_ops = {
 	.get_temp = mtk_read_temp,
 };
 
+static const struct thermal_zone_of_device_ops mtk_thermal_sensor_ops = {
+	.get_temp = mtk_read_sensor_temp,
+};
+
 static void mtk_thermal_init_bank(struct mtk_thermal *mt, int num,
 				  u32 apmixed_phys_base, u32 auxadc_phys_base,
 				  int ctrl_id)
@@ -873,6 +913,7 @@ static int mtk_thermal_probe(struct platform_device *pdev)
 	struct resource *res;
 	u64 auxadc_phys_base, apmixed_phys_base;
 	struct thermal_zone_device *tzdev;
+	struct mtk_thermal_zone *tz;
 
 	mt = devm_kzalloc(&pdev->dev, sizeof(*mt), GFP_KERNEL);
 	if (!mt)
@@ -954,11 +995,30 @@ static int mtk_thermal_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, mt);
 
-	tzdev = devm_thermal_zone_of_sensor_register(&pdev->dev, 0, mt,
-						     &mtk_thermal_ops);
-	if (IS_ERR(tzdev)) {
-		ret = PTR_ERR(tzdev);
-		goto err_disable_clk_peri_therm;
+	for (i = 0; i < mt->conf->num_sensors + 1; i++) {
+		tz = kmalloc(sizeof(*tz), GFP_KERNEL);
+		if (!tz)
+			return -ENOMEM;
+
+		tz->mt = mt;
+		tz->id = i;
+
+		tzdev = devm_thermal_zone_of_sensor_register(&pdev->dev, i,
+							     tz, (i == 0) ?
+				&mtk_thermal_ops : &mtk_thermal_sensor_ops);
+
+		if (IS_ERR(tzdev)) {
+			if (PTR_ERR(tzdev) == -ENODEV) {
+				dev_warn(&pdev->dev,
+					 "sensor %d not registered in thermal zone in dt\n",
+					 i);
+				continue;
+			}
+			if (PTR_ERR(tzdev) == -EACCES) {
+				ret = PTR_ERR(tzdev);
+				goto err_disable_clk_peri_therm;
+			}
+		}
 	}
 
 	return 0;
-- 
2.18.0

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

* Re: [v4,7/7] thermal: mediatek: use spinlock to protect PTPCORESEL
       [not found]   ` <1589439322.11120.2.camel@mtksdccf07>
@ 2020-05-14  8:08     ` Daniel Lezcano
  0 siblings, 0 replies; 16+ messages in thread
From: Daniel Lezcano @ 2020-05-14  8:08 UTC (permalink / raw)
  To: Michael Kao, Matthias Brugger
  Cc: Zhang Rui, Eduardo Valentin, Rob Herring, Mark Rutland, hsinyi,
	linux-pm, srv_heupstream, devicetree, linux-mediatek,
	linux-kernel, linux-arm-kernel

On 14/05/2020 08:55, Michael Kao wrote:
> On Mon, 2020-03-23 at 20:15 +0800, Michael Kao wrote:
>> From: "michael.kao" <michael.kao@mediatek.com>
>>
>> The driver of thermal and svs will use the
>> same register for the project which should select
>> bank before reading sensor value.
>>
>> Signed-off-by: Michael Kao <michael.kao@mediatek.com>
>> ---
>>  drivers/thermal/mtk_thermal.c | 9 ++++-----
>>  1 file changed, 4 insertions(+), 5 deletions(-)
>>

[ ... ]

> Hi Matthias,

Those patches fall under the thermal framework umbrella.

Thanks
  -- Daniel


-- 
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

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

* Re: [v4,0/7] Add Mediatek thermal dirver and dtsi
       [not found] <20200323121537.22697-1-michael.kao@mediatek.com>
                   ` (4 preceding siblings ...)
  2020-03-23 12:15 ` [v4,6/7] thermal: mediatek: add another get_temp ops for thermal sensors Michael Kao
@ 2020-05-21 12:51 ` Matthias Brugger
  2020-05-22  4:07   ` Michael Kao
  2020-05-22  4:09   ` Michael Kao
       [not found] ` <20200323121537.22697-8-michael.kao@mediatek.com>
  6 siblings, 2 replies; 16+ messages in thread
From: Matthias Brugger @ 2020-05-21 12:51 UTC (permalink / raw)
  To: Michael Kao, Zhang Rui, Eduardo Valentin, Daniel Lezcano,
	Rob Herring, Mark Rutland, hsinyi, linux-pm, srv_heupstream
  Cc: devicetree, linux-kernel, linux-arm-kernel, linux-mediatek

Hi Michael,

On 23/03/2020 13:15, Michael Kao wrote:
> This patchset supports for MT8183 chip to mtk_thermal.c.
> Add thermal zone of all the thermal sensor in SoC for
> another get temperatrue. They don't need to thermal throttle.
> And we bind coolers for thermal zone nodes of cpu_thermal.
> 
> Rebase to kernel-5.6-rc1.
> 
> Update content:
> 
> [1/7]
>     - Squash thermal zone settings in the dtsi from [v3,5/8]
>       arm64: dts: mt8183: Increase polling frequency for CPU thermal zone
> 
>     - Remove the property of interrupts and mediatek,hw-reset-temp
> 
> [2/7]
>     - Correct commit message
> 
> [4/7]
>     - Change the target temperature to the 80C and change the commit message
> 
> [6/7]
>     - Adjust newline alignment
> 
>     - Fix the judgement on the return value of registering thermal zone
> 
> This patch series base on these patches [1].
> 
> [v7,3/3] PM / AVS: SVS: Introduce SVS engine (https://patchwork.kernel.org/patch/11439829/)
> 
> Matthias Kaehlcke (1):
>   arm64: dts: mt8183: Configure CPU cooling
> 
> Michael Kao (6):
>   arm64: dts: mt8183: add thermal zone node
>   arm64: dts: mt8183: add dynamic power coefficients
>   arm64: dts: mt8183: Add #cooling-cells to CPU nodes
>   thermal: mediatek: mt8183: fix bank number settings

Do I understand correctly that we need to fix the bank number before we can add
the device tree changes. And that the last two patches are enhancements for the
driver but needed to get a working version?

Regards,
Matthias

>   thermal: mediatek: add another get_temp ops for thermal sensors
>   thermal: mediatek: use spinlock to protect PTPCORESEL
> 
>  arch/arm64/boot/dts/mediatek/mt8183.dtsi | 156 +++++++++++++++++++++++
>  drivers/thermal/mtk_thermal.c            |  88 +++++++++++--
>  2 files changed, 231 insertions(+), 13 deletions(-)
> 

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

* Re: [v4,0/7] Add Mediatek thermal dirver and dtsi
  2020-05-21 12:51 ` [v4,0/7] Add Mediatek thermal dirver and dtsi Matthias Brugger
@ 2020-05-22  4:07   ` Michael Kao
  2020-05-22  4:09   ` Michael Kao
  1 sibling, 0 replies; 16+ messages in thread
From: Michael Kao @ 2020-05-22  4:07 UTC (permalink / raw)
  To: Matthias Brugger
  Cc: Zhang Rui, Eduardo Valentin, Daniel Lezcano, Rob Herring,
	Mark Rutland, hsinyi, linux-pm, srv_heupstream, devicetree,
	linux-kernel, linux-arm-kernel, linux-mediatek

On Thu, 2020-05-21 at 14:51 +0200, Matthias Brugger wrote:
> Hi Michael,
> 
> On 23/03/2020 13:15, Michael Kao wrote:
> > This patchset supports for MT8183 chip to mtk_thermal.c.
> > Add thermal zone of all the thermal sensor in SoC for
> > another get temperatrue. They don't need to thermal throttle.
> > And we bind coolers for thermal zone nodes of cpu_thermal.
> > 
> > Rebase to kernel-5.6-rc1.
> > 
> > Update content:
> > 
> > [1/7]
> >     - Squash thermal zone settings in the dtsi from [v3,5/8]
> >       arm64: dts: mt8183: Increase polling frequency for CPU thermal zone
> > 
> >     - Remove the property of interrupts and mediatek,hw-reset-temp
> > 
> > [2/7]
> >     - Correct commit message
> > 
> > [4/7]
> >     - Change the target temperature to the 80C and change the commit message
> > 
> > [6/7]
> >     - Adjust newline alignment
> > 
> >     - Fix the judgement on the return value of registering thermal zone
> > 
> > This patch series base on these patches [1].
> > 
> > [v7,3/3] PM / AVS: SVS: Introduce SVS engine (https://patchwork.kernel.org/patch/11439829/)
> > 
> > Matthias Kaehlcke (1):
> >   arm64: dts: mt8183: Configure CPU cooling
> > 
> > Michael Kao (6):
> >   arm64: dts: mt8183: add thermal zone node
> >   arm64: dts: mt8183: add dynamic power coefficients
> >   arm64: dts: mt8183: Add #cooling-cells to CPU nodes
> >   thermal: mediatek: mt8183: fix bank number settings
> 
> Do I understand correctly that we need to fix the bank number before we can add
> the device tree changes. And that the last two patches are enhancements for the
> driver but needed to get a working version?
> 
> Regards,
> Matthias

Hi Matthias,

There is one bank setting of mt8183 config.
If the device tree merged first. I worry that it will crash when the
thermal zone read temperature.
It will access the invalid index of bank.
So please add the patch "fix bank number settings "
first.222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222

> 
> >   thermal: mediatek: add another get_temp ops for thermal sensors
> >   thermal: mediatek: use spinlock to protect PTPCORESEL
> > 
> >  arch/arm64/boot/dts/mediatek/mt8183.dtsi | 156 +++++++++++++++++++++++
> >  drivers/thermal/mtk_thermal.c            |  88 +++++++++++--
> >  2 files changed, 231 insertions(+), 13 deletions(-)
> > 


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

* Re: [v4,0/7] Add Mediatek thermal dirver and dtsi
  2020-05-21 12:51 ` [v4,0/7] Add Mediatek thermal dirver and dtsi Matthias Brugger
  2020-05-22  4:07   ` Michael Kao
@ 2020-05-22  4:09   ` Michael Kao
  1 sibling, 0 replies; 16+ messages in thread
From: Michael Kao @ 2020-05-22  4:09 UTC (permalink / raw)
  To: Matthias Brugger
  Cc: Zhang Rui, Eduardo Valentin, Daniel Lezcano, Rob Herring,
	Mark Rutland, hsinyi, linux-pm, srv_heupstream, devicetree,
	linux-kernel, linux-arm-kernel, linux-mediatek

On Thu, 2020-05-21 at 14:51 +0200, Matthias Brugger wrote:
> Hi Michael,
> 
> On 23/03/2020 13:15, Michael Kao wrote:
> > This patchset supports for MT8183 chip to mtk_thermal.c.
> > Add thermal zone of all the thermal sensor in SoC for
> > another get temperatrue. They don't need to thermal throttle.
> > And we bind coolers for thermal zone nodes of cpu_thermal.
> > 
> > Rebase to kernel-5.6-rc1.
> > 
> > Update content:
> > 
> > [1/7]
> >     - Squash thermal zone settings in the dtsi from [v3,5/8]
> >       arm64: dts: mt8183: Increase polling frequency for CPU thermal zone
> > 
> >     - Remove the property of interrupts and mediatek,hw-reset-temp
> > 
> > [2/7]
> >     - Correct commit message
> > 
> > [4/7]
> >     - Change the target temperature to the 80C and change the commit message
> > 
> > [6/7]
> >     - Adjust newline alignment
> > 
> >     - Fix the judgement on the return value of registering thermal zone
> > 
> > This patch series base on these patches [1].
> > 
> > [v7,3/3] PM / AVS: SVS: Introduce SVS engine (https://patchwork.kernel.org/patch/11439829/)
> > 
> > Matthias Kaehlcke (1):
> >   arm64: dts: mt8183: Configure CPU cooling
> > 
> > Michael Kao (6):
> >   arm64: dts: mt8183: add thermal zone node
> >   arm64: dts: mt8183: add dynamic power coefficients
> >   arm64: dts: mt8183: Add #cooling-cells to CPU nodes
> >   thermal: mediatek: mt8183: fix bank number settings
> 
> Do I understand correctly that we need to fix the bank number before we can add
> the device tree changes. And that the last two patches are enhancements for the
> driver but needed to get a working version?
> 
> Regards,
> Matthias
> 
Hi Matthias,

There is one bank setting of mt8183 config.
If the device tree merged first. I worry that it will crash when the
thermal zone read temperature.
It will access the invalid index of bank.
So please wait the patch "fix bank number settings " merged first.
Thanks!

/* MT8183 thermal sensor data */
static const int mt8183_bank_data[MT8183_NUM_SENSORS] = {
	MT8183_TS1, MT8183_TS2, MT8183_TS3, MT8183_TS4, MT8183_TS5,
MT8183_TSABB
}; 

Best Regards,
Michael


> >   thermal: mediatek: add another get_temp ops for thermal sensors
> >   thermal: mediatek: use spinlock to protect PTPCORESEL
> > 
> >  arch/arm64/boot/dts/mediatek/mt8183.dtsi | 156 +++++++++++++++++++++++
> >  drivers/thermal/mtk_thermal.c            |  88 +++++++++++--
> >  2 files changed, 231 insertions(+), 13 deletions(-)
> > 


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

* Re: [v4,6/7] thermal: mediatek: add another get_temp ops for thermal sensors
  2020-03-23 12:15 ` [v4,6/7] thermal: mediatek: add another get_temp ops for thermal sensors Michael Kao
@ 2020-05-22 15:32   ` Daniel Lezcano
  2020-07-07  3:07     ` [v4, 6/7] " Michael Kao
  0 siblings, 1 reply; 16+ messages in thread
From: Daniel Lezcano @ 2020-05-22 15:32 UTC (permalink / raw)
  To: Michael Kao, Matthias Brugger, Zhang Rui, Eduardo Valentin,
	Rob Herring, Mark Rutland, hsinyi, linux-pm, srv_heupstream
  Cc: devicetree, linux-kernel, linux-arm-kernel, linux-mediatek

On 23/03/2020 13:15, Michael Kao wrote:
> Provide thermal zone to read thermal sensor
> in the SoC. We can read all the thermal sensors
> value in the SoC by the node /sys/class/thermal/
> 
> In mtk_thermal_bank_temperature, return -EAGAIN instead of -EACCESS
> on the first read of sensor that often are bogus values.
> This can avoid following warning on boot:
> 
>   thermal thermal_zone6: failed to read out thermal zone (-13)

What the source problem of the sensor returning bogus values?



> Signed-off-by: Michael Kao <michael.kao@mediatek.com>
> Signed-off-by: Hsin-Yi Wang <hsinyi@chromium.org>
> ---
>  drivers/thermal/mtk_thermal.c | 74 +++++++++++++++++++++++++++++++----
>  1 file changed, 67 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/thermal/mtk_thermal.c b/drivers/thermal/mtk_thermal.c
> index 6b7ef1993d7e..9eaca432920e 100644
> --- a/drivers/thermal/mtk_thermal.c
> +++ b/drivers/thermal/mtk_thermal.c
> @@ -225,6 +225,11 @@ enum {
>  
>  struct mtk_thermal;
>  
> +struct mtk_thermal_zone {
> +	struct mtk_thermal *mt;
> +	int id;
> +};
> +
>  struct thermal_bank_cfg {
>  	unsigned int num_sensors;
>  	const int *sensors;
> @@ -607,7 +612,7 @@ static int mtk_thermal_bank_temperature(struct mtk_thermal_bank *bank)
>  		 * not immediately shut down.
>  		 */
>  		if (temp > 200000)
> -			temp = 0;
> +			temp = -EAGAIN;
>  
>  		if (temp > max)
>  			max = temp;
> @@ -618,7 +623,8 @@ static int mtk_thermal_bank_temperature(struct mtk_thermal_bank *bank)
>  
>  static int mtk_read_temp(void *data, int *temperature)
>  {
> -	struct mtk_thermal *mt = data;
> +	struct mtk_thermal_zone *tz = data;
> +	struct mtk_thermal *mt = tz->mt;
>  	int i;
>  	int tempmax = INT_MIN;
>  
> @@ -637,10 +643,44 @@ static int mtk_read_temp(void *data, int *temperature)
>  	return 0;
>  }
>  
> +static int mtk_read_sensor_temp(void *data, int *temperature)
> +{
> +	struct mtk_thermal_zone *tz = data;
> +	struct mtk_thermal *mt = tz->mt;
> +	const struct mtk_thermal_data *conf = mt->conf;
> +	int id = tz->id - 1;
> +	int temp = INT_MIN;
> +	u32 raw;
> +
> +	if (id < 0)
> +		return  -EACCES;
> +
> +	raw = readl(mt->thermal_base + conf->msr[id]);
> +
> +	temp = raw_to_mcelsius(mt, id, raw);
> +
> +	/*
> +	 * The first read of a sensor often contains very high bogus
> +	 * temperature value. Filter these out so that the system does
> +	 * not immediately shut down.
> +	 */
> +
> +	if (temp > 200000)
> +		return  -EAGAIN;
> +
> +	*temperature = temp;
> +
> +	return 0;
> +}
> +
>  static const struct thermal_zone_of_device_ops mtk_thermal_ops = {
>  	.get_temp = mtk_read_temp,
>  };
>  
> +static const struct thermal_zone_of_device_ops mtk_thermal_sensor_ops = {
> +	.get_temp = mtk_read_sensor_temp,
> +};
> +
>  static void mtk_thermal_init_bank(struct mtk_thermal *mt, int num,
>  				  u32 apmixed_phys_base, u32 auxadc_phys_base,
>  				  int ctrl_id)
> @@ -873,6 +913,7 @@ static int mtk_thermal_probe(struct platform_device *pdev)
>  	struct resource *res;
>  	u64 auxadc_phys_base, apmixed_phys_base;
>  	struct thermal_zone_device *tzdev;
> +	struct mtk_thermal_zone *tz;
>  
>  	mt = devm_kzalloc(&pdev->dev, sizeof(*mt), GFP_KERNEL);
>  	if (!mt)
> @@ -954,11 +995,30 @@ static int mtk_thermal_probe(struct platform_device *pdev)
>  
>  	platform_set_drvdata(pdev, mt);
>  
> -	tzdev = devm_thermal_zone_of_sensor_register(&pdev->dev, 0, mt,
> -						     &mtk_thermal_ops);
> -	if (IS_ERR(tzdev)) {
> -		ret = PTR_ERR(tzdev);
> -		goto err_disable_clk_peri_therm;
> +	for (i = 0; i < mt->conf->num_sensors + 1; i++) {
> +		tz = kmalloc(sizeof(*tz), GFP_KERNEL);
> +		if (!tz)
> +			return -ENOMEM;
> +
> +		tz->mt = mt;
> +		tz->id = i;
> +
> +		tzdev = devm_thermal_zone_of_sensor_register(&pdev->dev, i,
> +							     tz, (i == 0) ?
> +				&mtk_thermal_ops : &mtk_thermal_sensor_ops);
> +
> +		if (IS_ERR(tzdev)) {
> +			if (PTR_ERR(tzdev) == -ENODEV) {
> +				dev_warn(&pdev->dev,
> +					 "sensor %d not registered in thermal zone in dt\n",
> +					 i);
> +				continue;
> +			}
> +			if (PTR_ERR(tzdev) == -EACCES) {
> +				ret = PTR_ERR(tzdev);
> +				goto err_disable_clk_peri_therm;
> +			}
> +		}
>  	}
>  
>  	return 0;
> 


-- 
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

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

* Re: [v4,5/7] thermal: mediatek: mt8183: fix bank number settings
  2020-03-23 12:15 ` [v4,5/7] thermal: mediatek: mt8183: fix bank number settings Michael Kao
@ 2020-05-22 15:33   ` Daniel Lezcano
  0 siblings, 0 replies; 16+ messages in thread
From: Daniel Lezcano @ 2020-05-22 15:33 UTC (permalink / raw)
  To: Michael Kao, Matthias Brugger, Zhang Rui, Eduardo Valentin,
	Rob Herring, Mark Rutland, hsinyi, linux-pm, srv_heupstream
  Cc: devicetree, linux-kernel, linux-arm-kernel, linux-mediatek

On 23/03/2020 13:15, Michael Kao wrote:
> MT8183_NUM_ZONES should be set to 1
> because MT8183 doesn't have multiple banks.

Apparently this is a hot fix. Can you confirm and explain what is the
procedure to hit the bug?

I'll pick it for the next -rc if possible

> Fixes: a4ffe6b52d27 ("thermal: mediatek: add support for MT8183")
> Signed-off-by: Michael Kao <michael.kao@mediatek.com>
> Signed-off-by: Hsin-Yi Wang <hsinyi@chromium.org>
> ---
>  drivers/thermal/mtk_thermal.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/thermal/mtk_thermal.c b/drivers/thermal/mtk_thermal.c
> index 76e30603d4d5..6b7ef1993d7e 100644
> --- a/drivers/thermal/mtk_thermal.c
> +++ b/drivers/thermal/mtk_thermal.c
> @@ -211,6 +211,9 @@ enum {
>  /* The total number of temperature sensors in the MT8183 */
>  #define MT8183_NUM_SENSORS	6
>  
> +/* The number of banks in the MT8183 */
> +#define MT8183_NUM_ZONES               1
> +
>  /* The number of sensing points per bank */
>  #define MT8183_NUM_SENSORS_PER_ZONE	 6
>  
> @@ -497,7 +500,7 @@ static const struct mtk_thermal_data mt7622_thermal_data = {
>   */
>  static const struct mtk_thermal_data mt8183_thermal_data = {
>  	.auxadc_channel = MT8183_TEMP_AUXADC_CHANNEL,
> -	.num_banks = MT8183_NUM_SENSORS_PER_ZONE,
> +	.num_banks = MT8183_NUM_ZONES,
>  	.num_sensors = MT8183_NUM_SENSORS,
>  	.vts_index = mt8183_vts_index,
>  	.cali_val = MT8183_CALIBRATION,
> 


-- 
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

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

* Re: [v4,7/7] thermal: mediatek: use spinlock to protect PTPCORESEL
       [not found] ` <20200323121537.22697-8-michael.kao@mediatek.com>
       [not found]   ` <1589439322.11120.2.camel@mtksdccf07>
@ 2020-05-22 15:36   ` Daniel Lezcano
       [not found]     ` <1591329023.12739.0.camel@mtksdccf07>
  1 sibling, 1 reply; 16+ messages in thread
From: Daniel Lezcano @ 2020-05-22 15:36 UTC (permalink / raw)
  To: Michael Kao, Matthias Brugger, Zhang Rui, Eduardo Valentin,
	Rob Herring, Mark Rutland, hsinyi, linux-pm, srv_heupstream
  Cc: devicetree, linux-kernel, linux-arm-kernel, linux-mediatek

On 23/03/2020 13:15, Michael Kao wrote:
> From: "michael.kao" <michael.kao@mediatek.com>
> 
> The driver of thermal and svs will use the
> same register for the project which should select
> bank before reading sensor value.

Here there is a design problem AFAICT. The sensor should not be using
external locks.



> Signed-off-by: Michael Kao <michael.kao@mediatek.com>
> ---
>  drivers/thermal/mtk_thermal.c | 9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/thermal/mtk_thermal.c b/drivers/thermal/mtk_thermal.c
> index 9eaca432920e..594ad4f0f8cd 100644
> --- a/drivers/thermal/mtk_thermal.c
> +++ b/drivers/thermal/mtk_thermal.c
> @@ -22,6 +22,7 @@
>  #include <linux/thermal.h>
>  #include <linux/reset.h>
>  #include <linux/types.h>
> +#include <linux/power/mtk_svs.h>
>  
>  /* AUXADC Registers */
>  #define AUXADC_CON1_SET_V	0x008
> @@ -262,7 +263,7 @@ struct mtk_thermal {
>  	struct clk *clk_peri_therm;
>  	struct clk *clk_auxadc;
>  	/* lock: for getting and putting banks */
> -	struct mutex lock;
> +	unsigned long flags;
>  
>  	/* Calibration values */
>  	s32 adc_ge;
> @@ -561,7 +562,7 @@ static void mtk_thermal_get_bank(struct mtk_thermal_bank *bank)
>  	u32 val;
>  
>  	if (mt->conf->need_switch_bank) {
> -		mutex_lock(&mt->lock);
> +		mt->flags = claim_mtk_svs_lock();
>  
>  		val = readl(mt->thermal_base + PTPCORESEL);
>  		val &= ~0xf;
> @@ -581,7 +582,7 @@ static void mtk_thermal_put_bank(struct mtk_thermal_bank *bank)
>  	struct mtk_thermal *mt = bank->mt;
>  
>  	if (mt->conf->need_switch_bank)
> -		mutex_unlock(&mt->lock);
> +		release_mtk_svs_lock(mt->flags);
>  }
>  
>  /**
> @@ -938,8 +939,6 @@ static int mtk_thermal_probe(struct platform_device *pdev)
>  	if (ret)
>  		return ret;
>  
> -	mutex_init(&mt->lock);
> -
>  	mt->dev = &pdev->dev;
>  
>  	auxadc = of_parse_phandle(np, "mediatek,auxadc", 0);
> 


-- 
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

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

* Re: [v4, 6/7] thermal: mediatek: add another get_temp ops for thermal sensors
  2020-05-22 15:32   ` Daniel Lezcano
@ 2020-07-07  3:07     ` Michael Kao
  0 siblings, 0 replies; 16+ messages in thread
From: Michael Kao @ 2020-07-07  3:07 UTC (permalink / raw)
  To: Daniel Lezcano
  Cc: Matthias Brugger, Zhang Rui, Eduardo Valentin, Rob Herring,
	Mark Rutland, hsinyi, linux-pm, srv_heupstream, devicetree,
	linux-mediatek, linux-kernel, linux-arm-kernel

On Fri, 2020-05-22 at 17:32 +0200, Daniel Lezcano wrote:
> On 23/03/2020 13:15, Michael Kao wrote:
> > Provide thermal zone to read thermal sensor
> > in the SoC. We can read all the thermal sensors
> > value in the SoC by the node /sys/class/thermal/
> > 
> > In mtk_thermal_bank_temperature, return -EAGAIN instead of -EACCESS
> > on the first read of sensor that often are bogus values.
> > This can avoid following warning on boot:
> > 
> >   thermal thermal_zone6: failed to read out thermal zone (-13)
> 
> What the source problem of the sensor returning bogus values?

> The thermal controller will depend on the AUXADC module to read raw data from sensor.
Sometimes it will report wrong value because the AUXADC not ready or
other issue.
The thermal driver should add the judgement to prevent wrong temperature
value to cause
system immediately shut down.
> 
> > Signed-off-by: Michael Kao <michael.kao@mediatek.com>
> > Signed-off-by: Hsin-Yi Wang <hsinyi@chromium.org>
> > ---
> >  drivers/thermal/mtk_thermal.c | 74 +++++++++++++++++++++++++++++++----
> >  1 file changed, 67 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/thermal/mtk_thermal.c b/drivers/thermal/mtk_thermal.c
> > index 6b7ef1993d7e..9eaca432920e 100644
> > --- a/drivers/thermal/mtk_thermal.c
> > +++ b/drivers/thermal/mtk_thermal.c
> > @@ -225,6 +225,11 @@ enum {
> >  
> >  struct mtk_thermal;
> >  
> > +struct mtk_thermal_zone {
> > +	struct mtk_thermal *mt;
> > +	int id;
> > +};
> > +
> >  struct thermal_bank_cfg {
> >  	unsigned int num_sensors;
> >  	const int *sensors;
> > @@ -607,7 +612,7 @@ static int mtk_thermal_bank_temperature(struct mtk_thermal_bank *bank)
> >  		 * not immediately shut down.
> >  		 */
> >  		if (temp > 200000)
> > -			temp = 0;
> > +			temp = -EAGAIN;
> >  
> >  		if (temp > max)
> >  			max = temp;
> > @@ -618,7 +623,8 @@ static int mtk_thermal_bank_temperature(struct mtk_thermal_bank *bank)
> >  
> >  static int mtk_read_temp(void *data, int *temperature)
> >  {
> > -	struct mtk_thermal *mt = data;
> > +	struct mtk_thermal_zone *tz = data;
> > +	struct mtk_thermal *mt = tz->mt;
> >  	int i;
> >  	int tempmax = INT_MIN;
> >  
> > @@ -637,10 +643,44 @@ static int mtk_read_temp(void *data, int *temperature)
> >  	return 0;
> >  }
> >  
> > +static int mtk_read_sensor_temp(void *data, int *temperature)
> > +{
> > +	struct mtk_thermal_zone *tz = data;
> > +	struct mtk_thermal *mt = tz->mt;
> > +	const struct mtk_thermal_data *conf = mt->conf;
> > +	int id = tz->id - 1;
> > +	int temp = INT_MIN;
> > +	u32 raw;
> > +
> > +	if (id < 0)
> > +		return  -EACCES;
> > +
> > +	raw = readl(mt->thermal_base + conf->msr[id]);
> > +
> > +	temp = raw_to_mcelsius(mt, id, raw);
> > +
> > +	/*
> > +	 * The first read of a sensor often contains very high bogus
> > +	 * temperature value. Filter these out so that the system does
> > +	 * not immediately shut down.
> > +	 */
> > +
> > +	if (temp > 200000)
> > +		return  -EAGAIN;
> > +
> > +	*temperature = temp;
> > +
> > +	return 0;
> > +}
> > +
> >  static const struct thermal_zone_of_device_ops mtk_thermal_ops = {
> >  	.get_temp = mtk_read_temp,
> >  };
> >  
> > +static const struct thermal_zone_of_device_ops mtk_thermal_sensor_ops = {
> > +	.get_temp = mtk_read_sensor_temp,
> > +};
> > +
> >  static void mtk_thermal_init_bank(struct mtk_thermal *mt, int num,
> >  				  u32 apmixed_phys_base, u32 auxadc_phys_base,
> >  				  int ctrl_id)
> > @@ -873,6 +913,7 @@ static int mtk_thermal_probe(struct platform_device *pdev)
> >  	struct resource *res;
> >  	u64 auxadc_phys_base, apmixed_phys_base;
> >  	struct thermal_zone_device *tzdev;
> > +	struct mtk_thermal_zone *tz;
> >  
> >  	mt = devm_kzalloc(&pdev->dev, sizeof(*mt), GFP_KERNEL);
> >  	if (!mt)
> > @@ -954,11 +995,30 @@ static int mtk_thermal_probe(struct platform_device *pdev)
> >  
> >  	platform_set_drvdata(pdev, mt);
> >  
> > -	tzdev = devm_thermal_zone_of_sensor_register(&pdev->dev, 0, mt,
> > -						     &mtk_thermal_ops);
> > -	if (IS_ERR(tzdev)) {
> > -		ret = PTR_ERR(tzdev);
> > -		goto err_disable_clk_peri_therm;
> > +	for (i = 0; i < mt->conf->num_sensors + 1; i++) {
> > +		tz = kmalloc(sizeof(*tz), GFP_KERNEL);
> > +		if (!tz)
> > +			return -ENOMEM;
> > +
> > +		tz->mt = mt;
> > +		tz->id = i;
> > +
> > +		tzdev = devm_thermal_zone_of_sensor_register(&pdev->dev, i,
> > +							     tz, (i == 0) ?
> > +				&mtk_thermal_ops : &mtk_thermal_sensor_ops);
> > +
> > +		if (IS_ERR(tzdev)) {
> > +			if (PTR_ERR(tzdev) == -ENODEV) {
> > +				dev_warn(&pdev->dev,
> > +					 "sensor %d not registered in thermal zone in dt\n",
> > +					 i);
> > +				continue;
> > +			}
> > +			if (PTR_ERR(tzdev) == -EACCES) {
> > +				ret = PTR_ERR(tzdev);
> > +				goto err_disable_clk_peri_therm;
> > +			}
> > +		}
> >  	}
> >  
> >  	return 0;
> > 
> 
> 


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

* Re: [v4,7/7] thermal: mediatek: use spinlock to protect PTPCORESEL
       [not found]       ` <1594090621.20216.0.camel@mtksdccf07>
@ 2020-07-07  8:11         ` Daniel Lezcano
  0 siblings, 0 replies; 16+ messages in thread
From: Daniel Lezcano @ 2020-07-07  8:11 UTC (permalink / raw)
  To: Michael Kao
  Cc: Mark Rutland, devicetree, srv_heupstream, linux-pm, linux-kernel,
	Eduardo Valentin, Rob Herring, linux-mediatek, hsinyi,
	Matthias Brugger, Zhang Rui, linux-arm-kernel

On 07/07/2020 04:57, Michael Kao wrote:

[ ... ]

> Gently ping.

Michael,

it is impossible to have this series merged if there is a so big delay
between the comments / questions and the answers (or no answers at all).



-- 
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

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

* Re: [v4,7/7] thermal: mediatek: use spinlock to protect PTPCORESEL
       [not found]     ` <1591329023.12739.0.camel@mtksdccf07>
       [not found]       ` <1594090621.20216.0.camel@mtksdccf07>
@ 2020-07-07  8:13       ` Daniel Lezcano
  2020-07-08 10:00         ` Michael Kao
  1 sibling, 1 reply; 16+ messages in thread
From: Daniel Lezcano @ 2020-07-07  8:13 UTC (permalink / raw)
  To: Michael Kao
  Cc: Matthias Brugger, Zhang Rui, Eduardo Valentin, Rob Herring,
	Mark Rutland, hsinyi, linux-pm, srv_heupstream, devicetree,
	linux-kernel, linux-arm-kernel, linux-mediatek

On 05/06/2020 05:50, Michael Kao wrote:
> On Fri, 2020-05-22 at 17:36 +0200, Daniel Lezcano wrote:
>> On 23/03/2020 13:15, Michael Kao wrote:
>>> From: "michael.kao" <michael.kao@mediatek.com>
>>>
>>> The driver of thermal and svs will use the
>>> same register for the project which should select
>>> bank before reading sensor value.
>>
>> Here there is a design problem AFAICT. The sensor should not be using
>> external locks.
>>
> The PTPCORESEL is a common register used by svs and thermal.
> The thermal need to ensure PTPCORESEL register will not be changed by
> svs when thermal switch bank to read raw data of temperature.
> So we use svs_lock to make sure there is no conflict between the two
> drivers.

Why not use regmap ?

>>> Signed-off-by: Michael Kao <michael.kao@mediatek.com>
>>> ---
>>>  drivers/thermal/mtk_thermal.c | 9 ++++-----
>>>  1 file changed, 4 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/drivers/thermal/mtk_thermal.c b/drivers/thermal/mtk_thermal.c
>>> index 9eaca432920e..594ad4f0f8cd 100644
>>> --- a/drivers/thermal/mtk_thermal.c
>>> +++ b/drivers/thermal/mtk_thermal.c
>>> @@ -22,6 +22,7 @@
>>>  #include <linux/thermal.h>
>>>  #include <linux/reset.h>
>>>  #include <linux/types.h>
>>> +#include <linux/power/mtk_svs.h>
>>>  
>>>  /* AUXADC Registers */
>>>  #define AUXADC_CON1_SET_V	0x008
>>> @@ -262,7 +263,7 @@ struct mtk_thermal {
>>>  	struct clk *clk_peri_therm;
>>>  	struct clk *clk_auxadc;
>>>  	/* lock: for getting and putting banks */
>>> -	struct mutex lock;
>>> +	unsigned long flags;
>>>  
>>>  	/* Calibration values */
>>>  	s32 adc_ge;
>>> @@ -561,7 +562,7 @@ static void mtk_thermal_get_bank(struct mtk_thermal_bank *bank)
>>>  	u32 val;
>>>  
>>>  	if (mt->conf->need_switch_bank) {
>>> -		mutex_lock(&mt->lock);
>>> +		mt->flags = claim_mtk_svs_lock();
>>>  
>>>  		val = readl(mt->thermal_base + PTPCORESEL);
>>>  		val &= ~0xf;
>>> @@ -581,7 +582,7 @@ static void mtk_thermal_put_bank(struct mtk_thermal_bank *bank)
>>>  	struct mtk_thermal *mt = bank->mt;
>>>  
>>>  	if (mt->conf->need_switch_bank)
>>> -		mutex_unlock(&mt->lock);
>>> +		release_mtk_svs_lock(mt->flags);
>>>  }
>>>  
>>>  /**
>>> @@ -938,8 +939,6 @@ static int mtk_thermal_probe(struct platform_device *pdev)
>>>  	if (ret)
>>>  		return ret;
>>>  
>>> -	mutex_init(&mt->lock);
>>> -
>>>  	mt->dev = &pdev->dev;
>>>  
>>>  	auxadc = of_parse_phandle(np, "mediatek,auxadc", 0);
>>>
>>
>>
> 


-- 
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

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

* Re: [v4,7/7] thermal: mediatek: use spinlock to protect PTPCORESEL
  2020-07-07  8:13       ` Daniel Lezcano
@ 2020-07-08 10:00         ` Michael Kao
  0 siblings, 0 replies; 16+ messages in thread
From: Michael Kao @ 2020-07-08 10:00 UTC (permalink / raw)
  To: Daniel Lezcano
  Cc: Matthias Brugger, Zhang Rui, Eduardo Valentin, Rob Herring,
	Mark Rutland, hsinyi, linux-pm, srv_heupstream, devicetree,
	linux-kernel, linux-arm-kernel, linux-mediatek

On Tue, 2020-07-07 at 10:13 +0200, Daniel Lezcano wrote:
> On 05/06/2020 05:50, Michael Kao wrote:
> > On Fri, 2020-05-22 at 17:36 +0200, Daniel Lezcano wrote:
> >> On 23/03/2020 13:15, Michael Kao wrote:
> >>> From: "michael.kao" <michael.kao@mediatek.com>
> >>>
> >>> The driver of thermal and svs will use the
> >>> same register for the project which should select
> >>> bank before reading sensor value.
> >>
> >> Here there is a design problem AFAICT. The sensor should not be using
> >> external locks.
> >>
> > The PTPCORESEL is a common register used by svs and thermal.
> > The thermal need to ensure PTPCORESEL register will not be changed by
> > svs when thermal switch bank to read raw data of temperature.
> > So we use svs_lock to make sure there is no conflict between the two
> > drivers.
> 
> Why not use regmap ?
> 
Hi Daniel,

We are not sure the regmap can solve the problem.
The regmap can lock CORESELECT. But it can not protect 
the other sensors registers which thermal controller start to
read temperature need to access.

After internal disscussion, we don't have this kind of experience to
use regmap to protect some peice of code.
We will do some research of this kind of usage.
Thanks for your suggestion.

> >>> Signed-off-by: Michael Kao <michael.kao@mediatek.com>
> >>> ---
> >>>  drivers/thermal/mtk_thermal.c | 9 ++++-----
> >>>  1 file changed, 4 insertions(+), 5 deletions(-)
> >>>
> >>> diff --git a/drivers/thermal/mtk_thermal.c b/drivers/thermal/mtk_thermal.c
> >>> index 9eaca432920e..594ad4f0f8cd 100644
> >>> --- a/drivers/thermal/mtk_thermal.c
> >>> +++ b/drivers/thermal/mtk_thermal.c
> >>> @@ -22,6 +22,7 @@
> >>>  #include <linux/thermal.h>
> >>>  #include <linux/reset.h>
> >>>  #include <linux/types.h>
> >>> +#include <linux/power/mtk_svs.h>
> >>>  
> >>>  /* AUXADC Registers */
> >>>  #define AUXADC_CON1_SET_V	0x008
> >>> @@ -262,7 +263,7 @@ struct mtk_thermal {
> >>>  	struct clk *clk_peri_therm;
> >>>  	struct clk *clk_auxadc;
> >>>  	/* lock: for getting and putting banks */
> >>> -	struct mutex lock;
> >>> +	unsigned long flags;
> >>>  
> >>>  	/* Calibration values */
> >>>  	s32 adc_ge;
> >>> @@ -561,7 +562,7 @@ static void mtk_thermal_get_bank(struct mtk_thermal_bank *bank)
> >>>  	u32 val;
> >>>  
> >>>  	if (mt->conf->need_switch_bank) {
> >>> -		mutex_lock(&mt->lock);
> >>> +		mt->flags = claim_mtk_svs_lock();
> >>>  
> >>>  		val = readl(mt->thermal_base + PTPCORESEL);
> >>>  		val &= ~0xf;
> >>> @@ -581,7 +582,7 @@ static void mtk_thermal_put_bank(struct mtk_thermal_bank *bank)
> >>>  	struct mtk_thermal *mt = bank->mt;
> >>>  
> >>>  	if (mt->conf->need_switch_bank)
> >>> -		mutex_unlock(&mt->lock);
> >>> +		release_mtk_svs_lock(mt->flags);
> >>>  }
> >>>  
> >>>  /**
> >>> @@ -938,8 +939,6 @@ static int mtk_thermal_probe(struct platform_device *pdev)
> >>>  	if (ret)
> >>>  		return ret;
> >>>  
> >>> -	mutex_init(&mt->lock);
> >>> -
> >>>  	mt->dev = &pdev->dev;
> >>>  
> >>>  	auxadc = of_parse_phandle(np, "mediatek,auxadc", 0);
> >>>
> >>
> >>
> > 
> 
> 


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

end of thread, other threads:[~2020-07-08 10:00 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <20200323121537.22697-1-michael.kao@mediatek.com>
2020-03-23 12:15 ` [v4,2/7] arm64: dts: mt8183: add dynamic power coefficients Michael Kao
2020-03-23 12:15 ` [v4,3/7] arm64: dts: mt8183: Add #cooling-cells to CPU nodes Michael Kao
2020-03-23 12:15 ` [v4,4/7] arm64: dts: mt8183: Configure CPU cooling Michael Kao
2020-03-23 12:15 ` [v4,5/7] thermal: mediatek: mt8183: fix bank number settings Michael Kao
2020-05-22 15:33   ` Daniel Lezcano
2020-03-23 12:15 ` [v4,6/7] thermal: mediatek: add another get_temp ops for thermal sensors Michael Kao
2020-05-22 15:32   ` Daniel Lezcano
2020-07-07  3:07     ` [v4, 6/7] " Michael Kao
2020-05-21 12:51 ` [v4,0/7] Add Mediatek thermal dirver and dtsi Matthias Brugger
2020-05-22  4:07   ` Michael Kao
2020-05-22  4:09   ` Michael Kao
     [not found] ` <20200323121537.22697-8-michael.kao@mediatek.com>
     [not found]   ` <1589439322.11120.2.camel@mtksdccf07>
2020-05-14  8:08     ` [v4,7/7] thermal: mediatek: use spinlock to protect PTPCORESEL Daniel Lezcano
2020-05-22 15:36   ` Daniel Lezcano
     [not found]     ` <1591329023.12739.0.camel@mtksdccf07>
     [not found]       ` <1594090621.20216.0.camel@mtksdccf07>
2020-07-07  8:11         ` Daniel Lezcano
2020-07-07  8:13       ` Daniel Lezcano
2020-07-08 10:00         ` Michael Kao

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).