[PATCH RESEND 1/4] dt-bindings: thermal: imx8mm-thermal: Add binding doc for i.MX8MM

[PATCH RESEND 1/4] dt-bindings: thermal: imx8mm-thermal: Add binding doc for i.MX8MM

[Anson Huang]

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

Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
 .../devicetree/bindings/thermal/imx8mm-thermal.txt        | 15 +++++++++++++++
 1 file changed, 15 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/thermal/imx8mm-thermal.txt

diff --git a/Documentation/devicetree/bindings/thermal/imx8mm-thermal.txt b/Documentation/devicetree/bindings/thermal/imx8mm-thermal.txt
new file mode 100644
index 0000000..d09ae82
--- /dev/null
+++ b/Documentation/devicetree/bindings/thermal/imx8mm-thermal.txt
@@ -0,0 +1,15 @@
+* Thermal Monitoring Unit (TMU) on Freescale i.MX8MM SoC
+
+Required properties:
+- compatible : Must be "fsl,imx8mm-tmu".
+- reg : Address range of TMU registers.
+- clocks : TMU's clock source.
+- #thermal-sensor-cells : Should be 0. See ./thermal.txt for a description.
+
+Example:
+tmu: tmu@30260000 {
+	compatible = "fsl,imx8mm-tmu";
+	reg = <0x30260000 0x10000>;
+	clocks = <&clk IMX8MM_CLK_TMU_ROOT>;
+	#thermal-sensor-cells = <0>;
+};
-- 
2.7.4

[PATCH RESEND 2/4] thermal: imx8mm: Add support for i.MX8MM thermal monitoring unit

[Anson Huang]

i.MX8MM has a thermal monitoring unit(TMU) inside, it ONLY has one
sensor for CPU, add support for reading immediate temperature of
this sensor.

Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
This patch is base on patch series: https://lkml.org/lkml/2020/2/19/1258
---
 drivers/thermal/Kconfig          |  10 +++
 drivers/thermal/Makefile         |   1 +
 drivers/thermal/imx8mm_thermal.c | 134 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 145 insertions(+)
 create mode 100644 drivers/thermal/imx8mm_thermal.c

diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
index d1cb8dc..972b169 100644
--- a/drivers/thermal/Kconfig
+++ b/drivers/thermal/Kconfig
@@ -262,6 +262,16 @@ config IMX_SC_THERMAL
 	  sensor. It supports one critical trip point and one
 	  passive trip point for each thermal sensor.
 
+config IMX8MM_THERMAL
+	tristate "Temperature sensor driver for Freescale i.MX8MM SoC"
+	depends on ARCH_MXC
+	depends on OF
+	help
+	  Support for Thermal Monitoring Unit (TMU) found on Freescale i.MX8MM SoC.
+	  It supports one critical trip point and one passive trip point. The
+	  cpufreq is used as the cooling device to throttle CPUs when the passive
+	  trip is crossed.
+
 config MAX77620_THERMAL
 	tristate "Temperature sensor driver for Maxim MAX77620 PMIC"
 	depends on MFD_MAX77620
diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile
index a11a6d8..120a05e 100644
--- a/drivers/thermal/Makefile
+++ b/drivers/thermal/Makefile
@@ -44,6 +44,7 @@ obj-$(CONFIG_ARMADA_THERMAL)	+= armada_thermal.o
 obj-$(CONFIG_TANGO_THERMAL)	+= tango_thermal.o
 obj-$(CONFIG_IMX_THERMAL)	+= imx_thermal.o
 obj-$(CONFIG_IMX_SC_THERMAL)	+= imx_sc_thermal.o
+obj-$(CONFIG_IMX8MM_THERMAL)	+= imx8mm_thermal.o
 obj-$(CONFIG_MAX77620_THERMAL)	+= max77620_thermal.o
 obj-$(CONFIG_QORIQ_THERMAL)	+= qoriq_thermal.o
 obj-$(CONFIG_DA9062_THERMAL)	+= da9062-thermal.o
diff --git a/drivers/thermal/imx8mm_thermal.c b/drivers/thermal/imx8mm_thermal.c
new file mode 100644
index 0000000..04f8a8f
--- /dev/null
+++ b/drivers/thermal/imx8mm_thermal.c
@@ -0,0 +1,134 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright 2019 NXP.
+ *
+ */
+
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/platform_device.h>
+#include <linux/thermal.h>
+
+#include "thermal_core.h"
+
+#define TER			0x0	/* TMU enable */
+#define TRITSR			0x20	/* TMU immediate temp */
+
+#define TER_EN			BIT(31)
+#define TRITSR_VAL_MASK		0xff
+
+#define TEMP_LOW_LIMIT		10
+
+struct imx8mm_tmu {
+	struct thermal_zone_device *tzd;
+	void __iomem *base;
+	struct clk *clk;
+};
+
+static int tmu_get_temp(void *data, int *temp)
+{
+	struct imx8mm_tmu *tmu = data;
+	u32 val;
+
+	/* the temp sensor need about 1ms to finish the measurement */
+	usleep_range(1000, 2000);
+
+	val = readl_relaxed(tmu->base + TRITSR) & TRITSR_VAL_MASK;
+	if (val < TEMP_LOW_LIMIT)
+		return -EAGAIN;
+
+	*temp = val * 1000;
+
+	return 0;
+}
+
+static struct thermal_zone_of_device_ops tmu_tz_ops = {
+	.get_temp = tmu_get_temp,
+};
+
+static int imx8mm_tmu_probe(struct platform_device *pdev)
+{
+	struct imx8mm_tmu *tmu;
+	u32 val;
+	int ret;
+
+	tmu = devm_kzalloc(&pdev->dev, sizeof(struct imx8mm_tmu), GFP_KERNEL);
+	if (!tmu)
+		return -ENOMEM;
+
+	tmu->base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(tmu->base))
+		return PTR_ERR(tmu->base);
+
+	tmu->clk = devm_clk_get(&pdev->dev, NULL);
+	if (IS_ERR(tmu->clk)) {
+		ret = PTR_ERR(tmu->clk);
+		if (ret != -EPROBE_DEFER)
+			dev_err(&pdev->dev,
+				"failed to get tmu clock: %d\n", ret);
+		return ret;
+	}
+
+	ret = clk_prepare_enable(tmu->clk);
+	if (ret) {
+		dev_err(&pdev->dev, "failed to enable tmu clock: %d\n", ret);
+		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);
+	}
+
+	platform_set_drvdata(pdev, tmu);
+
+	/* enable the monitor */
+	val = readl_relaxed(tmu->base + TER);
+	val |= TER_EN;
+	writel_relaxed(val, tmu->base + TER);
+
+	return 0;
+}
+
+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);
+
+	clk_disable_unprepare(tmu->clk);
+	platform_set_drvdata(pdev, NULL);
+
+	return 0;
+}
+
+static const struct of_device_id imx8mm_tmu_table[] = {
+	{ .compatible = "fsl,imx8mm-tmu", },
+	{ },
+};
+
+static struct platform_driver imx8mm_tmu = {
+	.driver = {
+		.name	= "i.mx8mm_thermal",
+		.of_match_table = imx8mm_tmu_table,
+	},
+	.probe = imx8mm_tmu_probe,
+	.remove = imx8mm_tmu_remove,
+};
+module_platform_driver(imx8mm_tmu);
+
+MODULE_AUTHOR("Anson Huang <Anson.Huang@nxp.com>");
+MODULE_DESCRIPTION("i.MX8MM Thermal Monitor Unit driver");
+MODULE_LICENSE("GPL v2");
-- 
2.7.4

Re: [PATCH RESEND 2/4] thermal: imx8mm: Add support for i.MX8MM thermal monitoring unit

[Daniel Lezcano]

On 20/02/2020 09:17, Anson Huang wrote:
> i.MX8MM has a thermal monitoring unit(TMU) inside, it ONLY has one
> sensor for CPU, add support for reading immediate temperature of
> this sensor.
> 
> Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> ---
> This patch is base on patch series: https://lkml.org/lkml/2020/2/19/1258
> ---
>  drivers/thermal/Kconfig          |  10 +++
>  drivers/thermal/Makefile         |   1 +
>  drivers/thermal/imx8mm_thermal.c | 134 +++++++++++++++++++++++++++++++++++++++
>  3 files changed, 145 insertions(+)
>  create mode 100644 drivers/thermal/imx8mm_thermal.c
> 
> diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
> index d1cb8dc..972b169 100644
> --- a/drivers/thermal/Kconfig
> +++ b/drivers/thermal/Kconfig
> @@ -262,6 +262,16 @@ config IMX_SC_THERMAL
>  	  sensor. It supports one critical trip point and one
>  	  passive trip point for each thermal sensor.
>  
> +config IMX8MM_THERMAL
> +	tristate "Temperature sensor driver for Freescale i.MX8MM SoC"
> +	depends on ARCH_MXC
> +	depends on OF
> +	help
> +	  Support for Thermal Monitoring Unit (TMU) found on Freescale i.MX8MM SoC.
> +	  It supports one critical trip point and one passive trip point. The
> +	  cpufreq is used as the cooling device to throttle CPUs when the passive
> +	  trip is crossed.
> +
>  config MAX77620_THERMAL
>  	tristate "Temperature sensor driver for Maxim MAX77620 PMIC"
>  	depends on MFD_MAX77620
> diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile
> index a11a6d8..120a05e 100644
> --- a/drivers/thermal/Makefile
> +++ b/drivers/thermal/Makefile
> @@ -44,6 +44,7 @@ obj-$(CONFIG_ARMADA_THERMAL)	+= armada_thermal.o
>  obj-$(CONFIG_TANGO_THERMAL)	+= tango_thermal.o
>  obj-$(CONFIG_IMX_THERMAL)	+= imx_thermal.o
>  obj-$(CONFIG_IMX_SC_THERMAL)	+= imx_sc_thermal.o
> +obj-$(CONFIG_IMX8MM_THERMAL)	+= imx8mm_thermal.o
>  obj-$(CONFIG_MAX77620_THERMAL)	+= max77620_thermal.o
>  obj-$(CONFIG_QORIQ_THERMAL)	+= qoriq_thermal.o
>  obj-$(CONFIG_DA9062_THERMAL)	+= da9062-thermal.o
> diff --git a/drivers/thermal/imx8mm_thermal.c b/drivers/thermal/imx8mm_thermal.c
> new file mode 100644
> index 0000000..04f8a8f
> --- /dev/null
> +++ b/drivers/thermal/imx8mm_thermal.c
> @@ -0,0 +1,134 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright 2019 NXP.

Copyright (c) 2020, NXP Semiconductors

Authors: Anson Huang <Anson.Huang@nxp.com>

??

> + *
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/delay.h>
> +#include <linux/err.h>
> +#include <linux/io.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
> +#include <linux/platform_device.h>
> +#include <linux/thermal.h>
> +
> +#include "thermal_core.h"
> +
> +#define TER			0x0	/* TMU enable */
> +#define TRITSR			0x20	/* TMU immediate temp */
> +
> +#define TER_EN			BIT(31)
> +#define TRITSR_VAL_MASK		0xff
> +
> +#define TEMP_LOW_LIMIT		10
> +
> +struct imx8mm_tmu {
> +	struct thermal_zone_device *tzd;
> +	void __iomem *base;
> +	struct clk *clk;
> +};
> +
> +static int tmu_get_temp(void *data, int *temp)
> +{
> +	struct imx8mm_tmu *tmu = data;
> +	u32 val;
> +
> +	/* the temp sensor need about 1ms to finish the measurement */
> +	usleep_range(1000, 2000);

Why do yo need to force a delay here? If the sensor can not be read more
than one time every 1ms, then specify that in the DT switching the
polling to the right value, no?

> +	val = readl_relaxed(tmu->base + TRITSR) & TRITSR_VAL_MASK;
> +	if (val < TEMP_LOW_LIMIT)
> +		return -EAGAIN;>
> +	*temp = val * 1000;
> +	return 0;
> +}
> +
> +static struct thermal_zone_of_device_ops tmu_tz_ops = {
> +	.get_temp = tmu_get_temp,
> +};
> +
> +static int imx8mm_tmu_probe(struct platform_device *pdev)
> +{
> +	struct imx8mm_tmu *tmu;
> +	u32 val;
> +	int ret;
> +
> +	tmu = devm_kzalloc(&pdev->dev, sizeof(struct imx8mm_tmu), GFP_KERNEL);
> +	if (!tmu)
> +		return -ENOMEM;
> +
> +	tmu->base = devm_platform_ioremap_resource(pdev, 0);
> +	if (IS_ERR(tmu->base))
> +		return PTR_ERR(tmu->base);
> +
> +	tmu->clk = devm_clk_get(&pdev->dev, NULL);
> +	if (IS_ERR(tmu->clk)) {
> +		ret = PTR_ERR(tmu->clk);
> +		if (ret != -EPROBE_DEFER)
> +			dev_err(&pdev->dev,
> +				"failed to get tmu clock: %d\n", ret);
> +		return ret;
> +	}
> +
> +	ret = clk_prepare_enable(tmu->clk);
> +	if (ret) {
> +		dev_err(&pdev->dev, "failed to enable tmu clock: %d\n", ret);
> +		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);
> +	}
> +
> +	platform_set_drvdata(pdev, tmu);
> +
> +	/* enable the monitor */
> +	val = readl_relaxed(tmu->base + TER);
> +	val |= TER_EN;
> +	writel_relaxed(val, tmu->base + TER);

A side note perhaps you can wrap:

static int imx8mm_start(struct imx8mm_tmu *tmu)
{
	clk_prepare_enable()
	val = readl_relaxed(tmu->base + TER);
	val |= TER_EN;
	writel_relaxed(val, tmu->base + TER);
}

and

static void imx8mm_stop(struct imx8mm_tmu *tmu)
{
	val = readl_relaxed(tmu->base + TER);
	val &= ~TER_EN;
	writel_relaxed(val, tmu->base + TER);
	clk_disable_unprepare(tmu->clk);
}

So the suspend/resume callbacks can call them directly if you decide to
add them.

> +	return 0;
> +}
> +
> +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);
> +
> +	clk_disable_unprepare(tmu->clk);
> +	platform_set_drvdata(pdev, NULL);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id imx8mm_tmu_table[] = {
> +	{ .compatible = "fsl,imx8mm-tmu", },
> +	{ },
> +};
> +
> +static struct platform_driver imx8mm_tmu = {
> +	.driver = {
> +		.name	= "i.mx8mm_thermal",
> +		.of_match_table = imx8mm_tmu_table,
> +	},
> +	.probe = imx8mm_tmu_probe,
> +	.remove = imx8mm_tmu_remove,
> +};
> +module_platform_driver(imx8mm_tmu);
> +
> +MODULE_AUTHOR("Anson Huang <Anson.Huang@nxp.com>");
> +MODULE_DESCRIPTION("i.MX8MM Thermal Monitor Unit driver");
> +MODULE_LICENSE("GPL v2");
> 


-- 
 <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 RESEND 2/4] thermal: imx8mm: Add support for i.MX8MM thermal monitoring unit

[Anson Huang]

Hi, Daniel

> Subject: Re: [PATCH RESEND 2/4] thermal: imx8mm: Add support for
> i.MX8MM thermal monitoring unit
> 
> On 20/02/2020 09:17, Anson Huang wrote:
> > i.MX8MM has a thermal monitoring unit(TMU) inside, it ONLY has one
> > sensor for CPU, add support for reading immediate temperature of this
> > sensor.
> >
> > Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> > ---
> > This patch is base on patch series:
> > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flkml
> > .org%2Flkml%2F2020%2F2%2F19%2F1258&amp;data=02%7C01%7CAnson.
> Huang%40nx
> >
> p.com%7Ca98c5f5da20240aaf3a708d7bb7542b8%7C686ea1d3bc2b4c6fa92c
> d99c5c3
> >
> 01635%7C0%7C0%7C637183984613271294&amp;sdata=lJ4Yzp57kJg2fOIusb2
> 9e%2FB
> > 2P6Cpq6iSVQqS571IDOk%3D&amp;reserved=0
> > ---
> >  drivers/thermal/Kconfig          |  10 +++
> >  drivers/thermal/Makefile         |   1 +
> >  drivers/thermal/imx8mm_thermal.c | 134
> > +++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 145 insertions(+)
> >  create mode 100644 drivers/thermal/imx8mm_thermal.c
> >
> > diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig index
> > d1cb8dc..972b169 100644
> > --- a/drivers/thermal/Kconfig
> > +++ b/drivers/thermal/Kconfig
> > @@ -262,6 +262,16 @@ config IMX_SC_THERMAL
> >  	  sensor. It supports one critical trip point and one
> >  	  passive trip point for each thermal sensor.
> >
> > +config IMX8MM_THERMAL
> > +	tristate "Temperature sensor driver for Freescale i.MX8MM SoC"
> > +	depends on ARCH_MXC
> > +	depends on OF
> > +	help
> > +	  Support for Thermal Monitoring Unit (TMU) found on Freescale
> i.MX8MM SoC.
> > +	  It supports one critical trip point and one passive trip point. The
> > +	  cpufreq is used as the cooling device to throttle CPUs when the
> passive
> > +	  trip is crossed.
> > +
> >  config MAX77620_THERMAL
> >  	tristate "Temperature sensor driver for Maxim MAX77620 PMIC"
> >  	depends on MFD_MAX77620
> > diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile index
> > a11a6d8..120a05e 100644
> > --- a/drivers/thermal/Makefile
> > +++ b/drivers/thermal/Makefile
> > @@ -44,6 +44,7 @@ obj-$(CONFIG_ARMADA_THERMAL)	+=
> armada_thermal.o
> >  obj-$(CONFIG_TANGO_THERMAL)	+= tango_thermal.o
> >  obj-$(CONFIG_IMX_THERMAL)	+= imx_thermal.o
> >  obj-$(CONFIG_IMX_SC_THERMAL)	+= imx_sc_thermal.o
> > +obj-$(CONFIG_IMX8MM_THERMAL)	+= imx8mm_thermal.o
> >  obj-$(CONFIG_MAX77620_THERMAL)	+= max77620_thermal.o
> >  obj-$(CONFIG_QORIQ_THERMAL)	+= qoriq_thermal.o
> >  obj-$(CONFIG_DA9062_THERMAL)	+= da9062-thermal.o
> > diff --git a/drivers/thermal/imx8mm_thermal.c
> > b/drivers/thermal/imx8mm_thermal.c
> > new file mode 100644
> > index 0000000..04f8a8f
> > --- /dev/null
> > +++ b/drivers/thermal/imx8mm_thermal.c
> > @@ -0,0 +1,134 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright 2019 NXP.
> 
> Copyright (c) 2020, NXP Semiconductors
> 
> Authors: Anson Huang <Anson.Huang@nxp.com>

OK, I will update the copyright year and add the author.


> > + *
> > + */
> > +
> > +#include <linux/clk.h>
> > +#include <linux/delay.h>
> > +#include <linux/err.h>
> > +#include <linux/io.h>
> > +#include <linux/module.h>
> > +#include <linux/of.h>
> > +#include <linux/of_address.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/thermal.h>
> > +
> > +#include "thermal_core.h"
> > +
> > +#define TER			0x0	/* TMU enable */
> > +#define TRITSR			0x20	/* TMU immediate temp */
> > +
> > +#define TER_EN			BIT(31)
> > +#define TRITSR_VAL_MASK		0xff
> > +
> > +#define TEMP_LOW_LIMIT		10
> > +
> > +struct imx8mm_tmu {
> > +	struct thermal_zone_device *tzd;
> > +	void __iomem *base;
> > +	struct clk *clk;
> > +};
> > +
> > +static int tmu_get_temp(void *data, int *temp) {
> > +	struct imx8mm_tmu *tmu = data;
> > +	u32 val;
> > +
> > +	/* the temp sensor need about 1ms to finish the measurement */
> > +	usleep_range(1000, 2000);
> 
> Why do yo need to force a delay here? If the sensor can not be read more
> than one time every 1ms, then specify that in the DT switching the polling to
> the right value, no?

The polling time(2 seconds) is OK for this case, adding this sleep is to prevent user from reading
temperature from sysfs interface very frequently like less than 1ms, does it make sense? 


> 
> > +	val = readl_relaxed(tmu->base + TRITSR) & TRITSR_VAL_MASK;
> > +	if (val < TEMP_LOW_LIMIT)
> > +		return -EAGAIN;>
> > +	*temp = val * 1000;
> > +	return 0;
> > +}
> > +
> > +static struct thermal_zone_of_device_ops tmu_tz_ops = {
> > +	.get_temp = tmu_get_temp,
> > +};
> > +
> > +static int imx8mm_tmu_probe(struct platform_device *pdev) {
> > +	struct imx8mm_tmu *tmu;
> > +	u32 val;
> > +	int ret;
> > +
> > +	tmu = devm_kzalloc(&pdev->dev, sizeof(struct imx8mm_tmu),
> GFP_KERNEL);
> > +	if (!tmu)
> > +		return -ENOMEM;
> > +
> > +	tmu->base = devm_platform_ioremap_resource(pdev, 0);
> > +	if (IS_ERR(tmu->base))
> > +		return PTR_ERR(tmu->base);
> > +
> > +	tmu->clk = devm_clk_get(&pdev->dev, NULL);
> > +	if (IS_ERR(tmu->clk)) {
> > +		ret = PTR_ERR(tmu->clk);
> > +		if (ret != -EPROBE_DEFER)
> > +			dev_err(&pdev->dev,
> > +				"failed to get tmu clock: %d\n", ret);
> > +		return ret;
> > +	}
> > +
> > +	ret = clk_prepare_enable(tmu->clk);
> > +	if (ret) {
> > +		dev_err(&pdev->dev, "failed to enable tmu clock: %d\n", ret);
> > +		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);
> > +	}
> > +
> > +	platform_set_drvdata(pdev, tmu);
> > +
> > +	/* enable the monitor */
> > +	val = readl_relaxed(tmu->base + TER);
> > +	val |= TER_EN;
> > +	writel_relaxed(val, tmu->base + TER);
> 
> A side note perhaps you can wrap:
> 
> static int imx8mm_start(struct imx8mm_tmu *tmu) {
> 	clk_prepare_enable()
> 	val = readl_relaxed(tmu->base + TER);
> 	val |= TER_EN;
> 	writel_relaxed(val, tmu->base + TER);
> }
> 
> and
> 
> static void imx8mm_stop(struct imx8mm_tmu *tmu) {
> 	val = readl_relaxed(tmu->base + TER);
> 	val &= ~TER_EN;
> 	writel_relaxed(val, tmu->base + TER);
> 	clk_disable_unprepare(tmu->clk);
> }
> 
> So the suspend/resume callbacks can call them directly if you decide to add
> them.
> 

Thanks for the suggestion, so far, the i.MX8MM thermal driver no need to have suspend/resume
callback, its clock/function will be automatically disabled after system enter suspend, if suspend/resume
callback is necessary later, I will wrap it.

Thanks,
Anson


> > +	return 0;
> > +}
> > +
> > +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);
> > +
> > +	clk_disable_unprepare(tmu->clk);
> > +	platform_set_drvdata(pdev, NULL);
> > +
> > +	return 0;
> > +}
> > +
> > +static const struct of_device_id imx8mm_tmu_table[] = {
> > +	{ .compatible = "fsl,imx8mm-tmu", },
> > +	{ },
> > +};
> > +
> > +static struct platform_driver imx8mm_tmu = {
> > +	.driver = {
> > +		.name	= "i.mx8mm_thermal",
> > +		.of_match_table = imx8mm_tmu_table,
> > +	},
> > +	.probe = imx8mm_tmu_probe,
> > +	.remove = imx8mm_tmu_remove,
> > +};
> > +module_platform_driver(imx8mm_tmu);
> > +
> > +MODULE_AUTHOR("Anson Huang <Anson.Huang@nxp.com>");
> > +MODULE_DESCRIPTION("i.MX8MM Thermal Monitor Unit driver");
> > +MODULE_LICENSE("GPL v2");
> >
> 
> 
> --
> 
> <https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.
> linaro.org%2F&amp;data=02%7C01%7CAnson.Huang%40nxp.com%7Ca98c5f
> 5da20240aaf3a708d7bb7542b8%7C686ea1d3bc2b4c6fa92cd99c5c301635%7
> C0%7C0%7C637183984613271294&amp;sdata=k6RmLpJLujsgfNDDSaBCA%2F
> gqUg7V%2Bex%2FvggZZeDTgOs%3D&amp;reserved=0> Linaro.org │ Open
> source software for ARM SoCs
> 
> Follow Linaro:
> <https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.
> facebook.com%2Fpages%2FLinaro&amp;data=02%7C01%7CAnson.Huang%4
> 0nxp.com%7Ca98c5f5da20240aaf3a708d7bb7542b8%7C686ea1d3bc2b4c6fa
> 92cd99c5c301635%7C0%7C0%7C637183984613271294&amp;sdata=p1DDLK
> OLCE6wfVkVAk6F6dWoi9ectXvP0AfxWq1ixHw%3D&amp;reserved=0>
> Facebook |
> <https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Ftwitte
> r.com%2F%23!%2Flinaroorg&amp;data=02%7C01%7CAnson.Huang%40nxp.c
> om%7Ca98c5f5da20240aaf3a708d7bb7542b8%7C686ea1d3bc2b4c6fa92cd99
> c5c301635%7C0%7C0%7C637183984613271294&amp;sdata=kdTPCBFzhu5J9
> R2ccrfEvgHNhMoiYyqhShXJZcpha2U%3D&amp;reserved=0> Twitter |
> <https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.
> linaro.org%2Flinaro-
> blog%2F&amp;data=02%7C01%7CAnson.Huang%40nxp.com%7Ca98c5f5da2
> 0240aaf3a708d7bb7542b8%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7
> C0%7C637183984613271294&amp;sdata=vRqSYzQYTl1Wq8NZftukynnylaBazF
> LjijKlu%2B8CWiI%3D&amp;reserved=0> Blog

Re: [PATCH RESEND 2/4] thermal: imx8mm: Add support for i.MX8MM thermal monitoring unit

[Daniel Lezcano]

On 28/02/2020 02:12, Anson Huang wrote:
> Hi, Daniel

[ ... ]

>>> +static int tmu_get_temp(void *data, int *temp) {
>>> +	struct imx8mm_tmu *tmu = data;
>>> +	u32 val;
>>> +
>>> +	/* the temp sensor need about 1ms to finish the measurement */
>>> +	usleep_range(1000, 2000);
>>
>> Why do yo need to force a delay here? If the sensor can not be read more
>> than one time every 1ms, then specify that in the DT switching the polling to
>> the right value, no?
> 
> The polling time(2 seconds) is OK for this case, adding this sleep is to prevent user from reading
> temperature from sysfs interface very frequently like less than 1ms, does it make sense? 

Not really, well except if the user is able to press the keys in less
than 1ms :)

If the userspace writes a polling script reading the temperature in a
busy loop, there is nothing we can do against silly programming :/

However, it could interesting to add a <min polling interval> in the
thermal structure and handle that from the core framework by caching the
last value and return it in case get_temp is called too fast.

-- 
 <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 RESEND 2/4] thermal: imx8mm: Add support for i.MX8MM thermal monitoring unit

[Anson Huang]

Hi, Daniel

> Subject: Re: [PATCH RESEND 2/4] thermal: imx8mm: Add support for
> i.MX8MM thermal monitoring unit
> 
> On 28/02/2020 02:12, Anson Huang wrote:
> > Hi, Daniel
> 
> [ ... ]
> 
> >>> +static int tmu_get_temp(void *data, int *temp) {
> >>> +	struct imx8mm_tmu *tmu = data;
> >>> +	u32 val;
> >>> +
> >>> +	/* the temp sensor need about 1ms to finish the measurement */
> >>> +	usleep_range(1000, 2000);
> >>
> >> Why do yo need to force a delay here? If the sensor can not be read
> >> more than one time every 1ms, then specify that in the DT switching
> >> the polling to the right value, no?
> >
> > The polling time(2 seconds) is OK for this case, adding this sleep is
> > to prevent user from reading temperature from sysfs interface very
> frequently like less than 1ms, does it make sense?
> 
> Not really, well except if the user is able to press the keys in less than 1ms :)
> 
> If the userspace writes a polling script reading the temperature in a busy loop,
> there is nothing we can do against silly programming :/

Make sense😊

> 
> However, it could interesting to add a <min polling interval> in the thermal
> structure and handle that from the core framework by caching the last value
> and return it in case get_temp is called too fast.

OK, I will resend V2 patch set to remove the sleep in this driver and see if there is
real issue of such corner case, if any issue observed, I will try summit a patch to core
framework later.

Thanks,
Anson

[PATCH RESEND 3/4] arm64: defconfig: Enable CONFIG_IMX8MM_THERMAL as module

[Anson Huang]

Enable CONFIG_IMX8MM_THERMAL as module to support i.MX8MM
thermal driver.

Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
This patch is base on patch series: https://lkml.org/lkml/2020/2/19/1258
---
 arch/arm64/configs/defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index 1ff479c..2382e60 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -454,6 +454,7 @@ CONFIG_CPU_THERMAL=y
 CONFIG_THERMAL_EMULATION=y
 CONFIG_QORIQ_THERMAL=m
 CONFIG_IMX_SC_THERMAL=m
+CONFIG_IMX8MM_THERMAL=m
 CONFIG_ROCKCHIP_THERMAL=m
 CONFIG_RCAR_THERMAL=y
 CONFIG_RCAR_GEN3_THERMAL=y
-- 
2.7.4

[PATCH RESEND 4/4] arm64: dts: imx8mm: Add thermal zone support

[Anson Huang]

Add thermal zone and tmu node to support i.MX8MM thermal
driver, ONLY cpu thermal zone is supported, and cpu cooling
is also added.

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

diff --git a/arch/arm64/boot/dts/freescale/imx8mm.dtsi b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
index b3d0b29..e438095 100644
--- a/arch/arm64/boot/dts/freescale/imx8mm.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
@@ -68,6 +68,7 @@
 			nvmem-cells = <&cpu_speed_grade>;
 			nvmem-cell-names = "speed_grade";
 			cpu-idle-states = <&cpu_pd_wait>;
+			#cooling-cells = <2>;
 		};
 
 		A53_1: cpu@1 {
@@ -80,6 +81,7 @@
 			next-level-cache = <&A53_L2>;
 			operating-points-v2 = <&a53_opp_table>;
 			cpu-idle-states = <&cpu_pd_wait>;
+			#cooling-cells = <2>;
 		};
 
 		A53_2: cpu@2 {
@@ -92,6 +94,7 @@
 			next-level-cache = <&A53_L2>;
 			operating-points-v2 = <&a53_opp_table>;
 			cpu-idle-states = <&cpu_pd_wait>;
+			#cooling-cells = <2>;
 		};
 
 		A53_3: cpu@3 {
@@ -104,6 +107,7 @@
 			next-level-cache = <&A53_L2>;
 			operating-points-v2 = <&a53_opp_table>;
 			cpu-idle-states = <&cpu_pd_wait>;
+			#cooling-cells = <2>;
 		};
 
 		A53_L2: l2-cache0 {
@@ -204,6 +208,38 @@
 		arm,no-tick-in-suspend;
 	};
 
+	thermal-zones {
+		cpu-thermal {
+			polling-delay-passive = <250>;
+			polling-delay = <2000>;
+			thermal-sensors = <&tmu>;
+			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>;
+				};
+			};
+		};
+	};
+
 	usbphynop1: usbphynop1 {
 		compatible = "usb-nop-xceiv";
 		clocks = <&clk IMX8MM_CLK_USB_PHY_REF>;
@@ -363,6 +399,13 @@
 				gpio-ranges = <&iomuxc 0 119 30>;
 			};
 
+			tmu: tmu@30260000 {
+				compatible = "fsl,imx8mm-tmu";
+				reg = <0x30260000 0x10000>;
+				clocks = <&clk IMX8MM_CLK_TMU_ROOT>;
+				#thermal-sensor-cells = <0>;
+			};
+
 			wdog1: watchdog@30280000 {
 				compatible = "fsl,imx8mm-wdt", "fsl,imx21-wdt";
 				reg = <0x30280000 0x10000>;
-- 
2.7.4

[PATCH RESEND 1/4] dt-bindings: thermal: imx8mm-thermal: Add binding doc for i.MX8MM

[Anson Huang]

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

Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
 .../devicetree/bindings/thermal/imx8mm-thermal.txt        | 15 +++++++++++++++
 1 file changed, 15 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/thermal/imx8mm-thermal.txt

diff --git a/Documentation/devicetree/bindings/thermal/imx8mm-thermal.txt b/Documentation/devicetree/bindings/thermal/imx8mm-thermal.txt
new file mode 100644
index 0000000..d09ae82
--- /dev/null
+++ b/Documentation/devicetree/bindings/thermal/imx8mm-thermal.txt
@@ -0,0 +1,15 @@
+* Thermal Monitoring Unit (TMU) on Freescale i.MX8MM SoC
+
+Required properties:
+- compatible : Must be "fsl,imx8mm-tmu".
+- reg : Address range of TMU registers.
+- clocks : TMU's clock source.
+- #thermal-sensor-cells : Should be 0. See ./thermal.txt for a description.
+
+Example:
+tmu: tmu@30260000 {
+	compatible = "fsl,imx8mm-tmu";
+	reg = <0x30260000 0x10000>;
+	clocks = <&clk IMX8MM_CLK_TMU_ROOT>;
+	#thermal-sensor-cells = <0>;
+};
-- 
2.7.4

[PATCH RESEND 2/4] thermal: imx8mm: Add support for i.MX8MM thermal monitoring unit

[Anson Huang]

i.MX8MM has a thermal monitoring unit(TMU) inside, it ONLY has one
sensor for CPU, add support for reading immediate temperature of
this sensor.

Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
 drivers/thermal/Kconfig          |  10 +++
 drivers/thermal/Makefile         |   1 +
 drivers/thermal/imx8mm_thermal.c | 134 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 145 insertions(+)
 create mode 100644 drivers/thermal/imx8mm_thermal.c

diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
index 129b9ed..e09ff16 100644
--- a/drivers/thermal/Kconfig
+++ b/drivers/thermal/Kconfig
@@ -233,6 +233,16 @@ config IMX_THERMAL
 	  cpufreq is used as the cooling device to throttle CPUs when the
 	  passive trip is crossed.
 
+config IMX8MM_THERMAL
+	tristate "Temperature sensor driver for Freescale i.MX8MM SoC"
+	depends on ARCH_MXC
+	depends on OF
+	help
+	  Support for Thermal Monitoring Unit (TMU) found on Freescale i.MX8MM SoC.
+	  It supports one critical trip point and one passive trip point. The
+	  cpufreq is used as the cooling device to throttle CPUs when the passive
+	  trip is crossed.
+
 config MAX77620_THERMAL
 	tristate "Temperature sensor driver for Maxim MAX77620 PMIC"
 	depends on MFD_MAX77620
diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile
index baeb70b..f2608f0 100644
--- a/drivers/thermal/Makefile
+++ b/drivers/thermal/Makefile
@@ -41,6 +41,7 @@ obj-$(CONFIG_DB8500_THERMAL)	+= db8500_thermal.o
 obj-$(CONFIG_ARMADA_THERMAL)	+= armada_thermal.o
 obj-$(CONFIG_TANGO_THERMAL)	+= tango_thermal.o
 obj-$(CONFIG_IMX_THERMAL)	+= imx_thermal.o
+obj-$(CONFIG_IMX8MM_THERMAL)	+= imx8mm_thermal.o
 obj-$(CONFIG_MAX77620_THERMAL)	+= max77620_thermal.o
 obj-$(CONFIG_QORIQ_THERMAL)	+= qoriq_thermal.o
 obj-$(CONFIG_DA9062_THERMAL)	+= da9062-thermal.o
diff --git a/drivers/thermal/imx8mm_thermal.c b/drivers/thermal/imx8mm_thermal.c
new file mode 100644
index 0000000..04f8a8f
--- /dev/null
+++ b/drivers/thermal/imx8mm_thermal.c
@@ -0,0 +1,134 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright 2019 NXP.
+ *
+ */
+
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/platform_device.h>
+#include <linux/thermal.h>
+
+#include "thermal_core.h"
+
+#define TER			0x0	/* TMU enable */
+#define TRITSR			0x20	/* TMU immediate temp */
+
+#define TER_EN			BIT(31)
+#define TRITSR_VAL_MASK		0xff
+
+#define TEMP_LOW_LIMIT		10
+
+struct imx8mm_tmu {
+	struct thermal_zone_device *tzd;
+	void __iomem *base;
+	struct clk *clk;
+};
+
+static int tmu_get_temp(void *data, int *temp)
+{
+	struct imx8mm_tmu *tmu = data;
+	u32 val;
+
+	/* the temp sensor need about 1ms to finish the measurement */
+	usleep_range(1000, 2000);
+
+	val = readl_relaxed(tmu->base + TRITSR) & TRITSR_VAL_MASK;
+	if (val < TEMP_LOW_LIMIT)
+		return -EAGAIN;
+
+	*temp = val * 1000;
+
+	return 0;
+}
+
+static struct thermal_zone_of_device_ops tmu_tz_ops = {
+	.get_temp = tmu_get_temp,
+};
+
+static int imx8mm_tmu_probe(struct platform_device *pdev)
+{
+	struct imx8mm_tmu *tmu;
+	u32 val;
+	int ret;
+
+	tmu = devm_kzalloc(&pdev->dev, sizeof(struct imx8mm_tmu), GFP_KERNEL);
+	if (!tmu)
+		return -ENOMEM;
+
+	tmu->base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(tmu->base))
+		return PTR_ERR(tmu->base);
+
+	tmu->clk = devm_clk_get(&pdev->dev, NULL);
+	if (IS_ERR(tmu->clk)) {
+		ret = PTR_ERR(tmu->clk);
+		if (ret != -EPROBE_DEFER)
+			dev_err(&pdev->dev,
+				"failed to get tmu clock: %d\n", ret);
+		return ret;
+	}
+
+	ret = clk_prepare_enable(tmu->clk);
+	if (ret) {
+		dev_err(&pdev->dev, "failed to enable tmu clock: %d\n", ret);
+		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);
+	}
+
+	platform_set_drvdata(pdev, tmu);
+
+	/* enable the monitor */
+	val = readl_relaxed(tmu->base + TER);
+	val |= TER_EN;
+	writel_relaxed(val, tmu->base + TER);
+
+	return 0;
+}
+
+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);
+
+	clk_disable_unprepare(tmu->clk);
+	platform_set_drvdata(pdev, NULL);
+
+	return 0;
+}
+
+static const struct of_device_id imx8mm_tmu_table[] = {
+	{ .compatible = "fsl,imx8mm-tmu", },
+	{ },
+};
+
+static struct platform_driver imx8mm_tmu = {
+	.driver = {
+		.name	= "i.mx8mm_thermal",
+		.of_match_table = imx8mm_tmu_table,
+	},
+	.probe = imx8mm_tmu_probe,
+	.remove = imx8mm_tmu_remove,
+};
+module_platform_driver(imx8mm_tmu);
+
+MODULE_AUTHOR("Anson Huang <Anson.Huang@nxp.com>");
+MODULE_DESCRIPTION("i.MX8MM Thermal Monitor Unit driver");
+MODULE_LICENSE("GPL v2");
-- 
2.7.4

Re: [PATCH RESEND 2/4] thermal: imx8mm: Add support for i.MX8MM thermal monitoring unit

[Fabio Estevam]

Hi  Anson,

On Mon, Oct 28, 2019 at 3:07 AM Anson Huang <Anson.Huang@nxp.com> wrote:

> +       ret = clk_prepare_enable(tmu->clk);
> +       if (ret) {
> +               dev_err(&pdev->dev, "failed to enable tmu clock: %d\n", ret);
> +               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);

You should disable the tmu clock in the error path.

> +               return PTR_ERR(tmu->tzd);