[PATCH V3 1/3] dt-bindings: thermal: imx8mm-thermal: Add support for i.MX8MP

[PATCH V3 1/3] dt-bindings: thermal: imx8mm-thermal: Add support for i.MX8MP

[Anson Huang]

Add thermal binding doc for Freescale's i.MX8MP Thermal Monitoring Unit.

Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
No change.
---
 Documentation/devicetree/bindings/thermal/imx8mm-thermal.txt | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/thermal/imx8mm-thermal.txt b/Documentation/devicetree/bindings/thermal/imx8mm-thermal.txt
index d09ae82..3629d3c 100644
--- a/Documentation/devicetree/bindings/thermal/imx8mm-thermal.txt
+++ b/Documentation/devicetree/bindings/thermal/imx8mm-thermal.txt
@@ -1,10 +1,10 @@
 * Thermal Monitoring Unit (TMU) on Freescale i.MX8MM SoC
 
 Required properties:
-- compatible : Must be "fsl,imx8mm-tmu".
+- compatible : Must be "fsl,imx8mm-tmu" or "fsl,imx8mp-tmu".
 - reg : Address range of TMU registers.
 - clocks : TMU's clock source.
-- #thermal-sensor-cells : Should be 0. See ./thermal.txt for a description.
+- #thermal-sensor-cells : Should be 0 or 1. See ./thermal.txt for a description.
 
 Example:
 tmu: tmu@30260000 {
-- 
2.7.4

[PATCH V3 2/3] thermal: imx8mm: Add i.MX8MP support

[Anson Huang]

i.MX8MP shares same TMU with i.MX8MM, the only difference is i.MX8MP
has two thermal sensors while i.MX8MM ONLY has one, add multiple sensors
support for i.MX8MM TMU driver.

Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
Changes since V2:
	- Fix build warning about test_bit second argument type.
---
 drivers/thermal/imx8mm_thermal.c | 153 +++++++++++++++++++++++++++++++++------
 1 file changed, 129 insertions(+), 24 deletions(-)

diff --git a/drivers/thermal/imx8mm_thermal.c b/drivers/thermal/imx8mm_thermal.c
index d597ceb..0d60f8d 100644
--- a/drivers/thermal/imx8mm_thermal.c
+++ b/drivers/thermal/imx8mm_thermal.c
@@ -5,59 +5,142 @@
  * Author: Anson Huang <Anson.Huang@nxp.com>
  */
 
+#include <linux/bitfield.h>
 #include <linux/clk.h>
 #include <linux/err.h>
 #include <linux/io.h>
 #include <linux/module.h>
 #include <linux/of.h>
-#include <linux/of_address.h>
+#include <linux/of_device.h>
 #include <linux/platform_device.h>
 #include <linux/thermal.h>
 
 #include "thermal_core.h"
 
 #define TER			0x0	/* TMU enable */
+#define TPS			0x4
 #define TRITSR			0x20	/* TMU immediate temp */
 
 #define TER_EN			BIT(31)
-#define TRITSR_VAL_MASK		0xff
+#define TRITSR_TEMP0_VAL_MASK	0xff
+#define TRITSR_TEMP1_VAL_MASK	0xff0000
 
-#define TEMP_LOW_LIMIT		10
+#define PROBE_SEL_ALL		GENMASK(31, 30)
 
-struct imx8mm_tmu {
+#define probe_status_offset(x)	(30 + x)
+#define SIGN_BIT		BIT(7)
+#define TEMP_VAL_MASK		GENMASK(6, 0)
+
+#define VER1_TEMP_LOW_LIMIT	10000
+#define VER2_TEMP_LOW_LIMIT	-40000
+#define VER2_TEMP_HIGH_LIMIT	125000
+
+#define TMU_VER1		0x1
+#define TMU_VER2		0x2
+
+struct thermal_soc_data {
+	u32 num_sensors;
+	u32 version;
+	int (*get_temp)(void *, int *);
+};
+
+struct tmu_sensor {
+	struct imx8mm_tmu *priv;
+	u32 hw_id;
 	struct thermal_zone_device *tzd;
+};
+
+struct imx8mm_tmu {
 	void __iomem *base;
 	struct clk *clk;
+	const struct thermal_soc_data *socdata;
+	struct tmu_sensor sensors[0];
 };
 
-static int tmu_get_temp(void *data, int *temp)
+static int imx8mm_tmu_get_temp(void *data, int *temp)
 {
-	struct imx8mm_tmu *tmu = data;
+	struct tmu_sensor *sensor = data;
+	struct imx8mm_tmu *tmu = sensor->priv;
 	u32 val;
 
-	val = readl_relaxed(tmu->base + TRITSR) & TRITSR_VAL_MASK;
-	if (val < TEMP_LOW_LIMIT)
+	val = readl_relaxed(tmu->base + TRITSR) & TRITSR_TEMP0_VAL_MASK;
+	*temp = val * 1000;
+	if (*temp < VER1_TEMP_LOW_LIMIT)
+		return -EAGAIN;
+
+	return 0;
+}
+
+static int imx8mp_tmu_get_temp(void *data, int *temp)
+{
+	struct tmu_sensor *sensor = data;
+	struct imx8mm_tmu *tmu = sensor->priv;
+	unsigned long val;
+	bool ready;
+
+	val = readl_relaxed(tmu->base + TRITSR);
+	ready = test_bit(probe_status_offset(sensor->hw_id), &val);
+	if (!ready)
 		return -EAGAIN;
 
+	val = sensor->hw_id ? FIELD_GET(TRITSR_TEMP1_VAL_MASK, val) :
+	      FIELD_GET(TRITSR_TEMP0_VAL_MASK, val);
+	if (val & SIGN_BIT) /* negative */
+		val = (~(val & TEMP_VAL_MASK) + 1);
+
 	*temp = val * 1000;
+	if (*temp < VER2_TEMP_LOW_LIMIT || *temp > VER2_TEMP_HIGH_LIMIT)
+		return -EAGAIN;
 
 	return 0;
 }
 
+static int tmu_get_temp(void *data, int *temp)
+{
+	struct tmu_sensor *sensor = data;
+	struct imx8mm_tmu *tmu = sensor->priv;
+
+	return tmu->socdata->get_temp(data, temp);
+}
+
 static struct thermal_zone_of_device_ops tmu_tz_ops = {
 	.get_temp = tmu_get_temp,
 };
 
+static void imx8mm_tmu_enable(struct imx8mm_tmu *tmu, bool enable)
+{
+	u32 val;
+
+	val = readl_relaxed(tmu->base + TER);
+	val = enable ? (val | TER_EN) : (val & ~TER_EN);
+	writel_relaxed(val, tmu->base + TER);
+}
+
+static void imx8mm_tmu_probe_sel_all(struct imx8mm_tmu *tmu)
+{
+	u32 val;
+
+	val = readl_relaxed(tmu->base + TPS);
+	val |= PROBE_SEL_ALL;
+	writel_relaxed(val, tmu->base + TPS);
+}
+
 static int imx8mm_tmu_probe(struct platform_device *pdev)
 {
+	const struct thermal_soc_data *data;
 	struct imx8mm_tmu *tmu;
-	u32 val;
 	int ret;
+	int i;
+
+	data = of_device_get_match_data(&pdev->dev);
 
-	tmu = devm_kzalloc(&pdev->dev, sizeof(struct imx8mm_tmu), GFP_KERNEL);
+	tmu = devm_kzalloc(&pdev->dev, struct_size(tmu, sensors,
+			   data->num_sensors), GFP_KERNEL);
 	if (!tmu)
 		return -ENOMEM;
 
+	tmu->socdata = data;
+
 	tmu->base = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(tmu->base))
 		return PTR_ERR(tmu->base);
@@ -77,20 +160,32 @@ static int imx8mm_tmu_probe(struct platform_device *pdev)
 		return ret;
 	}
 
-	tmu->tzd = devm_thermal_zone_of_sensor_register(&pdev->dev, 0,
-							tmu, &tmu_tz_ops);
-	if (IS_ERR(tmu->tzd)) {
-		dev_err(&pdev->dev,
-			"failed to register thermal zone sensor: %d\n", ret);
-		return PTR_ERR(tmu->tzd);
+	/* disable the monitor during initialization */
+	imx8mm_tmu_enable(tmu, false);
+
+	for (i = 0; i < data->num_sensors; i++) {
+		tmu->sensors[i].priv = tmu;
+		tmu->sensors[i].tzd =
+			devm_thermal_zone_of_sensor_register(&pdev->dev, i,
+							     &tmu->sensors[i],
+							     &tmu_tz_ops);
+		if (IS_ERR(tmu->sensors[i].tzd)) {
+			dev_err(&pdev->dev,
+				"failed to register thermal zone sensor[%d]: %d\n",
+				i, ret);
+			return PTR_ERR(tmu->sensors[i].tzd);
+		}
+		tmu->sensors[i].hw_id = i;
 	}
 
 	platform_set_drvdata(pdev, tmu);
 
+	/* enable all the probes for V2 TMU */
+	if (tmu->socdata->version == TMU_VER2)
+		imx8mm_tmu_probe_sel_all(tmu);
+
 	/* enable the monitor */
-	val = readl_relaxed(tmu->base + TER);
-	val |= TER_EN;
-	writel_relaxed(val, tmu->base + TER);
+	imx8mm_tmu_enable(tmu, true);
 
 	return 0;
 }
@@ -98,12 +193,9 @@ static int imx8mm_tmu_probe(struct platform_device *pdev)
 static int imx8mm_tmu_remove(struct platform_device *pdev)
 {
 	struct imx8mm_tmu *tmu = platform_get_drvdata(pdev);
-	u32 val;
 
 	/* disable TMU */
-	val = readl_relaxed(tmu->base + TER);
-	val &= ~TER_EN;
-	writel_relaxed(val, tmu->base + TER);
+	imx8mm_tmu_enable(tmu, false);
 
 	clk_disable_unprepare(tmu->clk);
 	platform_set_drvdata(pdev, NULL);
@@ -111,8 +203,21 @@ static int imx8mm_tmu_remove(struct platform_device *pdev)
 	return 0;
 }
 
+static struct thermal_soc_data imx8mm_tmu_data = {
+	.num_sensors = 1,
+	.version = TMU_VER1,
+	.get_temp = imx8mm_tmu_get_temp,
+};
+
+static struct thermal_soc_data imx8mp_tmu_data = {
+	.num_sensors = 2,
+	.version = TMU_VER2,
+	.get_temp = imx8mp_tmu_get_temp,
+};
+
 static const struct of_device_id imx8mm_tmu_table[] = {
-	{ .compatible = "fsl,imx8mm-tmu", },
+	{ .compatible = "fsl,imx8mm-tmu", .data = &imx8mm_tmu_data, },
+	{ .compatible = "fsl,imx8mp-tmu", .data = &imx8mp_tmu_data, },
 	{ },
 };
 
-- 
2.7.4

Re: [PATCH V3 2/3] thermal: imx8mm: Add i.MX8MP support

[Daniel Lezcano]

On 23/03/2020 13:28, Anson Huang wrote:
> i.MX8MP shares same TMU with i.MX8MM, the only difference is i.MX8MP
> has two thermal sensors while i.MX8MM ONLY has one, add multiple sensors
> support for i.MX8MM TMU driver.
> 
> Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> ---
> Changes since V2:
> 	- Fix build warning about test_bit second argument type.
> ---

Please, just send a fix on top of this patch because the series is
already merged.


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

RE: [PATCH V3 2/3] thermal: imx8mm: Add i.MX8MP support

[Anson Huang]

Hi, Daniel

> Subject: Re: [PATCH V3 2/3] thermal: imx8mm: Add i.MX8MP support
> 
> On 23/03/2020 13:28, Anson Huang wrote:
> > i.MX8MP shares same TMU with i.MX8MM, the only difference is i.MX8MP
> > has two thermal sensors while i.MX8MM ONLY has one, add multiple
> > sensors support for i.MX8MM TMU driver.
> >
> > Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> > ---
> > Changes since V2:
> > 	- Fix build warning about test_bit second argument type.
> > ---
> 
> Please, just send a fix on top of this patch because the series is already
> merged.

Done. Sorry that I did NOT receive the mail of patch merged, just sent a new patch
on top of it.

Thanks,
Anson

RE: [PATCH V3 2/3] thermal: imx8mm: Add i.MX8MP support

[Anson Huang]

Hi, Shawn

> Subject: Re: [PATCH V3 2/3] thermal: imx8mm: Add i.MX8MP support
> 
> On 23/03/2020 13:28, Anson Huang wrote:
> > i.MX8MP shares same TMU with i.MX8MM, the only difference is i.MX8MP
> > has two thermal sensors while i.MX8MM ONLY has one, add multiple
> > sensors support for i.MX8MM TMU driver.
> >
> > Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> > ---
> > Changes since V2:
> > 	- Fix build warning about test_bit second argument type.
> > ---
> 
> Please, just send a fix on top of this patch because the series is already
> merged.

The patch 1/3, 2/3 are merged, will you pick up the 3/3?

Thanks,
Anson

[PATCH V3 3/3] arm64: dts: imx8mp: Add thermal zones support

[Anson Huang]

i.MX8MP has a TMU inside which supports two thermal zones, add support
for them.

Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
No change.
---
 arch/arm64/boot/dts/freescale/imx8mp.dtsi | 63 +++++++++++++++++++++++++++++++
 1 file changed, 63 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/imx8mp.dtsi b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
index 9b1616e..175165b 100644
--- a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
@@ -7,6 +7,7 @@
 #include <dt-bindings/gpio/gpio.h>
 #include <dt-bindings/input/input.h>
 #include <dt-bindings/interrupt-controller/arm-gic.h>
+#include <dt-bindings/thermal/thermal.h>
 
 #include "imx8mp-pinfunc.h"
 
@@ -43,6 +44,7 @@
 			clocks = <&clk IMX8MP_CLK_ARM>;
 			enable-method = "psci";
 			next-level-cache = <&A53_L2>;
+			#cooling-cells = <2>;
 		};
 
 		A53_1: cpu@1 {
@@ -53,6 +55,7 @@
 			clocks = <&clk IMX8MP_CLK_ARM>;
 			enable-method = "psci";
 			next-level-cache = <&A53_L2>;
+			#cooling-cells = <2>;
 		};
 
 		A53_2: cpu@2 {
@@ -63,6 +66,7 @@
 			clocks = <&clk IMX8MP_CLK_ARM>;
 			enable-method = "psci";
 			next-level-cache = <&A53_L2>;
+			#cooling-cells = <2>;
 		};
 
 		A53_3: cpu@3 {
@@ -73,6 +77,7 @@
 			clocks = <&clk IMX8MP_CLK_ARM>;
 			enable-method = "psci";
 			next-level-cache = <&A53_L2>;
+			#cooling-cells = <2>;
 		};
 
 		A53_L2: l2-cache0 {
@@ -127,6 +132,57 @@
 		method = "smc";
 	};
 
+	thermal-zones {
+		cpu-thermal {
+			polling-delay-passive = <250>;
+			polling-delay = <2000>;
+			thermal-sensors = <&tmu 0x0>;
+			trips {
+				cpu_alert0: trip0 {
+					temperature = <85000>;
+					hysteresis = <2000>;
+					type = "passive";
+				};
+
+				cpu_crit0: trip1 {
+					temperature = <95000>;
+					hysteresis = <2000>;
+					type = "critical";
+				};
+			};
+
+			cooling-maps {
+				map0 {
+					trip = <&cpu_alert0>;
+					cooling-device =
+						<&A53_0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
+						<&A53_1 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
+						<&A53_2 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
+						<&A53_3 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
+				};
+			};
+		};
+
+		soc-thermal {
+			polling-delay-passive = <250>;
+			polling-delay = <2000>;
+			thermal-sensors = <&tmu 0x1>;
+			trips {
+				soc_alert0: trip0 {
+					temperature = <85000>;
+					hysteresis = <2000>;
+					type = "passive";
+				};
+
+				soc_crit0: trip1 {
+					temperature = <95000>;
+					hysteresis = <2000>;
+					type = "critical";
+				};
+			};
+		};
+	};
+
 	timer {
 		compatible = "arm,armv8-timer";
 		interrupts = <GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(6) | IRQ_TYPE_LEVEL_LOW)>,
@@ -215,6 +271,13 @@
 				gpio-ranges = <&iomuxc 0 114 30>;
 			};
 
+			tmu: tmu@30260000 {
+				compatible = "fsl,imx8mp-tmu";
+				reg = <0x30260000 0x10000>;
+				clocks = <&clk IMX8MP_CLK_TSENSOR_ROOT>;
+				#thermal-sensor-cells = <1>;
+			};
+
 			wdog1: watchdog@30280000 {
 				compatible = "fsl,imx8mp-wdt", "fsl,imx21-wdt";
 				reg = <0x30280000 0x10000>;
-- 
2.7.4

Re: [PATCH V3 3/3] arm64: dts: imx8mp: Add thermal zones support

[Amit Kucheria]

On Mon, Mar 23, 2020 at 6:05 PM Anson Huang <Anson.Huang@nxp.com> wrote:
>
> i.MX8MP has a TMU inside which supports two thermal zones, add support
> for them.
>
> Signed-off-by: Anson Huang <Anson.Huang@nxp.com>


[snip]

>
> +       thermal-zones {
> +               cpu-thermal {
> +                       polling-delay-passive = <250>;
> +                       polling-delay = <2000>;
> +                       thermal-sensors = <&tmu 0x0>;

No need for 0x0, just use 0

> +                       trips {
> +                               cpu_alert0: trip0 {
> +                                       temperature = <85000>;
> +                                       hysteresis = <2000>;
> +                                       type = "passive";
> +                               };
> +
> +                               cpu_crit0: trip1 {
> +                                       temperature = <95000>;
> +                                       hysteresis = <2000>;
> +                                       type = "critical";
> +                               };
> +                       };
> +
> +                       cooling-maps {
> +                               map0 {
> +                                       trip = <&cpu_alert0>;
> +                                       cooling-device =
> +                                               <&A53_0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> +                                               <&A53_1 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> +                                               <&A53_2 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> +                                               <&A53_3 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
> +                               };
> +                       };
> +               };
> +
> +               soc-thermal {
> +                       polling-delay-passive = <250>;
> +                       polling-delay = <2000>;
> +                       thermal-sensors = <&tmu 0x1>;

No need for 0x1, just use 1

> +                       trips {
> +                               soc_alert0: trip0 {
> +                                       temperature = <85000>;
> +                                       hysteresis = <2000>;
> +                                       type = "passive";
> +                               };
> +
> +                               soc_crit0: trip1 {
> +                                       temperature = <95000>;
> +                                       hysteresis = <2000>;
> +                                       type = "critical";
> +                               };
> +                       };

You need a cooling-map here since you have a passive trip point.


> +               };
> +       };
> +
>         timer {
>                 compatible = "arm,armv8-timer";
>                 interrupts = <GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(6) | IRQ_TYPE_LEVEL_LOW)>,
> @@ -215,6 +271,13 @@
>                                 gpio-ranges = <&iomuxc 0 114 30>;
>                         };
>
> +                       tmu: tmu@30260000 {
> +                               compatible = "fsl,imx8mp-tmu";
> +                               reg = <0x30260000 0x10000>;
> +                               clocks = <&clk IMX8MP_CLK_TSENSOR_ROOT>;
> +                               #thermal-sensor-cells = <1>;
> +                       };
> +
>                         wdog1: watchdog@30280000 {
>                                 compatible = "fsl,imx8mp-wdt", "fsl,imx21-wdt";
>                                 reg = <0x30280000 0x10000>;
> --
> 2.7.4
>

RE: [PATCH V3 3/3] arm64: dts: imx8mp: Add thermal zones support

[Anson Huang]

Hi, Amit

> Subject: Re: [PATCH V3 3/3] arm64: dts: imx8mp: Add thermal zones support
> 
> On Mon, Mar 23, 2020 at 6:05 PM Anson Huang <Anson.Huang@nxp.com>
> wrote:
> >
> > i.MX8MP has a TMU inside which supports two thermal zones, add support
> > for them.
> >
> > Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> 
> 
> [snip]
> 
> >
> > +       thermal-zones {
> > +               cpu-thermal {
> > +                       polling-delay-passive = <250>;
> > +                       polling-delay = <2000>;
> > +                       thermal-sensors = <&tmu 0x0>;
> 
> No need for 0x0, just use 0

OK.

> 
> > +                       trips {
> > +                               cpu_alert0: trip0 {
> > +                                       temperature = <85000>;
> > +                                       hysteresis = <2000>;
> > +                                       type = "passive";
> > +                               };
> > +
> > +                               cpu_crit0: trip1 {
> > +                                       temperature = <95000>;
> > +                                       hysteresis = <2000>;
> > +                                       type = "critical";
> > +                               };
> > +                       };
> > +
> > +                       cooling-maps {
> > +                               map0 {
> > +                                       trip = <&cpu_alert0>;
> > +                                       cooling-device =
> > +                                               <&A53_0
> THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> > +                                               <&A53_1
> THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> > +                                               <&A53_2
> THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> > +                                               <&A53_3
> THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
> > +                               };
> > +                       };
> > +               };
> > +
> > +               soc-thermal {
> > +                       polling-delay-passive = <250>;
> > +                       polling-delay = <2000>;
> > +                       thermal-sensors = <&tmu 0x1>;
> 
> No need for 0x1, just use 1

OK.

> 
> > +                       trips {
> > +                               soc_alert0: trip0 {
> > +                                       temperature = <85000>;
> > +                                       hysteresis = <2000>;
> > +                                       type = "passive";
> > +                               };
> > +
> > +                               soc_crit0: trip1 {
> > +                                       temperature = <95000>;
> > +                                       hysteresis = <2000>;
> > +                                       type = "critical";
> > +                               };
> > +                       };
> 
> You need a cooling-map here since you have a passive trip point.

Currently, there is no cooling map defined for soc thermal zone, the cpufreq cooling
is mapped to cpu thermal zone already, so do you think it is OK to leave it as no cooling
map, or it is better to put cpufreq cooling for soc thermal zone as well?


Thanks,
Anson

Re: [PATCH V3 3/3] arm64: dts: imx8mp: Add thermal zones support

[Amit Kucheria]

On Fri, Apr 3, 2020 at 9:04 AM Anson Huang <anson.huang@nxp.com> wrote:
>
> Hi, Amit
>
> > Subject: Re: [PATCH V3 3/3] arm64: dts: imx8mp: Add thermal zones support
> >
> > On Mon, Mar 23, 2020 at 6:05 PM Anson Huang <Anson.Huang@nxp.com>
> > wrote:
> > >
> > > i.MX8MP has a TMU inside which supports two thermal zones, add support
> > > for them.
> > >
> > > Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> >
> >
> > [snip]
> >
> > >
> > > +       thermal-zones {
> > > +               cpu-thermal {
> > > +                       polling-delay-passive = <250>;
> > > +                       polling-delay = <2000>;
> > > +                       thermal-sensors = <&tmu 0x0>;
> >
> > No need for 0x0, just use 0
>
> OK.
>
> >
> > > +                       trips {
> > > +                               cpu_alert0: trip0 {
> > > +                                       temperature = <85000>;
> > > +                                       hysteresis = <2000>;
> > > +                                       type = "passive";
> > > +                               };
> > > +
> > > +                               cpu_crit0: trip1 {
> > > +                                       temperature = <95000>;
> > > +                                       hysteresis = <2000>;
> > > +                                       type = "critical";
> > > +                               };
> > > +                       };
> > > +
> > > +                       cooling-maps {
> > > +                               map0 {
> > > +                                       trip = <&cpu_alert0>;
> > > +                                       cooling-device =
> > > +                                               <&A53_0
> > THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> > > +                                               <&A53_1
> > THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> > > +                                               <&A53_2
> > THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> > > +                                               <&A53_3
> > THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
> > > +                               };
> > > +                       };
> > > +               };
> > > +
> > > +               soc-thermal {
> > > +                       polling-delay-passive = <250>;
> > > +                       polling-delay = <2000>;
> > > +                       thermal-sensors = <&tmu 0x1>;
> >
> > No need for 0x1, just use 1
>
> OK.
>
> >
> > > +                       trips {
> > > +                               soc_alert0: trip0 {
> > > +                                       temperature = <85000>;
> > > +                                       hysteresis = <2000>;
> > > +                                       type = "passive";
> > > +                               };
> > > +
> > > +                               soc_crit0: trip1 {
> > > +                                       temperature = <95000>;
> > > +                                       hysteresis = <2000>;
> > > +                                       type = "critical";
> > > +                               };
> > > +                       };
> >
> > You need a cooling-map here since you have a passive trip point.
>
> Currently, there is no cooling map defined for soc thermal zone, the cpufreq cooling
> is mapped to cpu thermal zone already, so do you think it is OK to leave it as no cooling
> map, or it is better to put cpufreq cooling for soc thermal zone as well?
>

If there is no cooling, why do you need a passive trip point? Just
make it a hot trip that will send you a nofication (if .notify
callback registered).

Regards,
Amit

RE: [PATCH V3 3/3] arm64: dts: imx8mp: Add thermal zones support

[Anson Huang]

Hi, Amit

> Subject: Re: [PATCH V3 3/3] arm64: dts: imx8mp: Add thermal zones support
> 
> On Fri, Apr 3, 2020 at 9:04 AM Anson Huang <anson.huang@nxp.com> wrote:
> >
> > Hi, Amit
> >
> > > Subject: Re: [PATCH V3 3/3] arm64: dts: imx8mp: Add thermal zones
> > > support
> > >
> > > On Mon, Mar 23, 2020 at 6:05 PM Anson Huang
> <Anson.Huang@nxp.com>
> > > wrote:
> > > >
> > > > i.MX8MP has a TMU inside which supports two thermal zones, add
> > > > support for them.
> > > >
> > > > Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> > >
> > >
> > > [snip]
> > >
> > > >
> > > > +       thermal-zones {
> > > > +               cpu-thermal {
> > > > +                       polling-delay-passive = <250>;
> > > > +                       polling-delay = <2000>;
> > > > +                       thermal-sensors = <&tmu 0x0>;
> > >
> > > No need for 0x0, just use 0
> >
> > OK.
> >
> > >
> > > > +                       trips {
> > > > +                               cpu_alert0: trip0 {
> > > > +                                       temperature = <85000>;
> > > > +                                       hysteresis = <2000>;
> > > > +                                       type = "passive";
> > > > +                               };
> > > > +
> > > > +                               cpu_crit0: trip1 {
> > > > +                                       temperature = <95000>;
> > > > +                                       hysteresis = <2000>;
> > > > +                                       type = "critical";
> > > > +                               };
> > > > +                       };
> > > > +
> > > > +                       cooling-maps {
> > > > +                               map0 {
> > > > +                                       trip = <&cpu_alert0>;
> > > > +                                       cooling-device =
> > > > +                                               <&A53_0
> > > THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> > > > +                                               <&A53_1
> > > THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> > > > +                                               <&A53_2
> > > THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> > > > +                                               <&A53_3
> > > THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
> > > > +                               };
> > > > +                       };
> > > > +               };
> > > > +
> > > > +               soc-thermal {
> > > > +                       polling-delay-passive = <250>;
> > > > +                       polling-delay = <2000>;
> > > > +                       thermal-sensors = <&tmu 0x1>;
> > >
> > > No need for 0x1, just use 1
> >
> > OK.
> >
> > >
> > > > +                       trips {
> > > > +                               soc_alert0: trip0 {
> > > > +                                       temperature = <85000>;
> > > > +                                       hysteresis = <2000>;
> > > > +                                       type = "passive";
> > > > +                               };
> > > > +
> > > > +                               soc_crit0: trip1 {
> > > > +                                       temperature = <95000>;
> > > > +                                       hysteresis = <2000>;
> > > > +                                       type = "critical";
> > > > +                               };
> > > > +                       };
> > >
> > > You need a cooling-map here since you have a passive trip point.
> >
> > Currently, there is no cooling map defined for soc thermal zone, the
> > cpufreq cooling is mapped to cpu thermal zone already, so do you think
> > it is OK to leave it as no cooling map, or it is better to put cpufreq cooling for
> soc thermal zone as well?
> >
> 
> If there is no cooling, why do you need a passive trip point? Just make it a hot
> trip that will send you a nofication (if .notify callback registered).

OK, understood, I prefer to keep the passive trip point same as cpu thermal zone,
so I will also put the cpufreq cooling map for soc thermal zone.

Thanks,
Anson

Re: [PATCH V3 1/3] dt-bindings: thermal: imx8mm-thermal: Add support for i.MX8MP

[Amit Kucheria]

Hi Anson,

On Mon, Mar 23, 2020 at 6:05 PM Anson Huang <Anson.Huang@nxp.com> wrote:
>
> Add thermal binding doc for Freescale's i.MX8MP Thermal Monitoring Unit.
>

I realise this got merged already, but please send a patch converting
this binding to yaml at some point.

Regards,
Amit

> Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> ---
> No change.
> ---
>  Documentation/devicetree/bindings/thermal/imx8mm-thermal.txt | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/thermal/imx8mm-thermal.txt b/Documentation/devicetree/bindings/thermal/imx8mm-thermal.txt
> index d09ae82..3629d3c 100644
> --- a/Documentation/devicetree/bindings/thermal/imx8mm-thermal.txt
> +++ b/Documentation/devicetree/bindings/thermal/imx8mm-thermal.txt
> @@ -1,10 +1,10 @@
>  * Thermal Monitoring Unit (TMU) on Freescale i.MX8MM SoC
>
>  Required properties:
> -- compatible : Must be "fsl,imx8mm-tmu".
> +- compatible : Must be "fsl,imx8mm-tmu" or "fsl,imx8mp-tmu".
>  - reg : Address range of TMU registers.
>  - clocks : TMU's clock source.
> -- #thermal-sensor-cells : Should be 0. See ./thermal.txt for a description.
> +- #thermal-sensor-cells : Should be 0 or 1. See ./thermal.txt for a description.
>
>  Example:
>  tmu: tmu@30260000 {
> --
> 2.7.4
>

RE: [PATCH V3 1/3] dt-bindings: thermal: imx8mm-thermal: Add support for i.MX8MP

[Anson Huang]

Hi, Amit

> Subject: Re: [PATCH V3 1/3] dt-bindings: thermal: imx8mm-thermal: Add
> support for i.MX8MP
> 
> Hi Anson,
> 
> On Mon, Mar 23, 2020 at 6:05 PM Anson Huang <Anson.Huang@nxp.com>
> wrote:
> >
> > Add thermal binding doc for Freescale's i.MX8MP Thermal Monitoring Unit.
> >
> 
> I realise this got merged already, but please send a patch converting this
> binding to yaml at some point.

Got it, I will find some time to convert i.MX thermal binding doc to yaml format.

Thanks,
Anson