[PATCH v4 0/2] thermal: imx91: Add support for i.MX91 thermal monitoring unit

[PATCH v4 0/2] thermal: imx91: Add support for i.MX91 thermal monitoring unit

[Frank Li]

- Add binding doc
- Add imx91 thermal driver
- dts part wait for https://lore.kernel.org/imx/Z04W5chGq5TitB9f@lizhi-Precision-Tower-5810/
merged, so not in this serial.

Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
Changes in v4:
- Add Kryz's review tag for binding
- Add Marco's review tag for driver
- Use devm_add_action()
- Move pm_runtim_put before thermal_of_zone_register()
- Link to v3: https://lore.kernel.org/r/20241212-imx91tmu-v3-0-85e756b29437@nxp.com

Changes in v3:
- add ref thermal-sensor
- restrict #thermal-sensor-cells to 0 only
- Change to unevaluatedProperties

- add IMX91_TMU_ prefix for register define
- remove unused register define
- fix missed pm_runtime_put() at error path in imx91_tmu_get_temp()
- use dev variable in probe function
- use pm_runtime_set_active() in probe
- move START to imx91_tmu_get_temp()
- use DEFINE_RUNTIME_DEV_PM_OPS()
- keep set reset value because there are not sw "reset" bit in controller,
  uboot may change and enable tmu.

- Link to v2: https://lore.kernel.org/r/20241210-imx91tmu-v2-0-5032aad4d88e@nxp.com

Changes in v2:
- use low case for hexvalue
- combine struct imx91_tmu and tmu_sensor
- simplify imx91_tmu_start() and imx91_tmu_enable()
- use s16 for imx91_tmu_get_temp(), which may negative value
- use reverse christmas tree style
- use run time pm
- use oneshot to sample temp
- register thermal zone after hardware init
- Link to v1: https://lore.kernel.org/r/20241209-imx91tmu-v1-0-7859c5387f31@nxp.com

---
Pengfei Li (2):
      dt-bindings: thermal: fsl,imx91-tmu: add bindings for NXP i.MX91 thermal module
      thermal: imx91: Add support for i.MX91 thermal monitoring unit

 .../devicetree/bindings/thermal/fsl,imx91-tmu.yaml |  67 ++++++
 drivers/thermal/Kconfig                            |  10 +
 drivers/thermal/Makefile                           |   1 +
 drivers/thermal/imx91_thermal.c                    | 263 +++++++++++++++++++++
 4 files changed, 341 insertions(+)
---
base-commit: d07c576946b2bc440d6d2073998023e8a0bd7568
change-id: 20241209-imx91tmu-af2a7c042d8d

Best regards,
---
Frank Li <Frank.Li@nxp.com>

[PATCH v4 1/2] dt-bindings: thermal: fsl,imx91-tmu: add bindings for NXP i.MX91 thermal module

[Frank Li]

From: Pengfei Li <pengfei.li_1@nxp.com>

Add bindings documentation for i.MX91 thermal modules.

Signed-off-by: Pengfei Li <pengfei.li_1@nxp.com>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
Change from v2 to v3
- add ref thermal-sensor
- restrict #thermal-sensor-cells to 0 only
- Change to unevaluatedProperties
---
 .../devicetree/bindings/thermal/fsl,imx91-tmu.yaml | 67 ++++++++++++++++++++++
 1 file changed, 67 insertions(+)

diff --git a/Documentation/devicetree/bindings/thermal/fsl,imx91-tmu.yaml b/Documentation/devicetree/bindings/thermal/fsl,imx91-tmu.yaml
new file mode 100644
index 0000000000000..8411f3233bf44
--- /dev/null
+++ b/Documentation/devicetree/bindings/thermal/fsl,imx91-tmu.yaml
@@ -0,0 +1,67 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/thermal/fsl,imx91-tmu.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: NXP i.MX91 Thermal
+
+maintainers:
+  - Pengfei Li <pengfei.li_1@nxp.com>
+
+description:
+  i.MX91 features a new temperature sensor. It includes programmable
+  temperature threshold comparators for both normal and privileged
+  accesses and allows a programmable measurement frequency for the
+  Periodic One-Shot Measurement mode. Additionally, it provides
+  status registers for indicating the end of measurement and threshold
+  violation events.
+
+properties:
+  compatible:
+    items:
+      - const: fsl,imx91-tmu
+
+  reg:
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
+
+  nvmem-cells:
+    items:
+      - description: Phandle to the trim control 1 provided by ocotp
+      - description: Phandle to the trim control 2 provided by ocotp
+
+  nvmem-cell-names:
+    items:
+      - const: trim1
+      - const: trim2
+
+  "#thermal-sensor-cells":
+    const: 0
+
+required:
+  - compatible
+  - reg
+  - clocks
+
+allOf:
+  - $ref: thermal-sensor.yaml
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/imx93-clock.h>
+
+    thermal-sensor@44482000 {
+        compatible = "fsl,imx91-tmu";
+        reg = <0x44482000 0x1000>;
+        #thermal-sensor-cells = <0>;
+        clocks = <&clk IMX93_CLK_TMC_GATE>;
+        nvmem-cells = <&tmu_trim1>, <&tmu_trim2>;
+        nvmem-cell-names = "trim1", "trim2";
+    };
+
+...

-- 
2.34.1

[PATCH v4 2/2] thermal: imx91: Add support for i.MX91 thermal monitoring unit

[Frank Li]

From: Pengfei Li <pengfei.li_1@nxp.com>

Introduce support for the i.MX91 thermal monitoring unit, which features a
single sensor for the CPU. The register layout differs from other chips,
necessitating the creation of a dedicated file for this.

Signed-off-by: Pengfei Li <pengfei.li_1@nxp.com>
Signed-off-by: Peng Fan <peng.fan@nxp.com>
Reviewed-by: Marco Felsch <m.felsch@pengutronix.de>
Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
Change from v3 to v4
- Add Macro's review tag
- Use devm_add_action()
- Move pm_runtim_put before thermal_of_zone_register()

change from v2 to v3
- add IMX91_TMU_ prefix for register define
- remove unused register define
- fix missed pm_runtime_put() at error path in imx91_tmu_get_temp()
- use dev variable in probe function
- use pm_runtime_set_active() in probe
- move START to imx91_tmu_get_temp()
- use DEFINE_RUNTIME_DEV_PM_OPS()
- keep set reset value because there are not sw "reset" bit in controller,
uboot may change and enable tmu.

change from v1 to v2
- use low case for hexvalue
- combine struct imx91_tmu and tmu_sensor
- simplify imx91_tmu_start() and imx91_tmu_enable()
- use s16 for imx91_tmu_get_temp(), which may negative value
- use reverse christmas tree style
- use run time pm
- use oneshot to sample temp
- register thermal zone after hardware init
---
 drivers/thermal/Kconfig         |  10 ++
 drivers/thermal/Makefile        |   1 +
 drivers/thermal/imx91_thermal.c | 263 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 274 insertions(+)

diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
index d3f9686e26e71..da403ed86aeb1 100644
--- a/drivers/thermal/Kconfig
+++ b/drivers/thermal/Kconfig
@@ -296,6 +296,16 @@ config IMX8MM_THERMAL
 	  cpufreq is used as the cooling device to throttle CPUs when the passive
 	  trip is crossed.
 
+config IMX91_THERMAL
+	tristate "Temperature sensor driver for NXP i.MX91 SoC"
+	depends on ARCH_MXC || COMPILE_TEST
+	depends on OF
+	help
+	  Support for Temperature sensor found on NXP i.MX91 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 K3_THERMAL
 	tristate "Texas Instruments K3 thermal support"
 	depends on ARCH_K3 || COMPILE_TEST
diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile
index 9abf43a74f2bb..08da241e6a598 100644
--- a/drivers/thermal/Makefile
+++ b/drivers/thermal/Makefile
@@ -50,6 +50,7 @@ obj-$(CONFIG_ARMADA_THERMAL)	+= armada_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_IMX91_THERMAL)	+= imx91_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/imx91_thermal.c b/drivers/thermal/imx91_thermal.c
new file mode 100644
index 0000000000000..ef5e8e181dd0f
--- /dev/null
+++ b/drivers/thermal/imx91_thermal.c
@@ -0,0 +1,263 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright 2024 NXP.
+ */
+
+#include <linux/bitfield.h>
+#include <linux/clk.h>
+#include <linux/err.h>
+#include <linux/iopoll.h>
+#include <linux/nvmem-consumer.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+#include <linux/thermal.h>
+
+#define IMX91_TMU_STAT0				0x10
+#define IMX91_TMU_STAT0_DRDY0_IF_MASK		BIT(16)
+
+#define IMX91_TMU_DATA0				0x20
+
+#define IMX91_TMU_CTRL1_SET			0x204
+#define IMX91_TMU_CTRL1_CLR			0x208
+#define IMX91_TMU_CTRL1_EN			BIT(31)
+#define IMX91_TMU_CTRL1_START			BIT(30)
+#define IMX91_TMU_CTRL1_STOP			BIT(29)
+#define IMX91_TMU_CTRL1_RES_MASK		GENMASK(19, 18)
+#define IMX91_TMU_CTRL1_MEAS_MODE_MASK		GENMASK(25, 24)
+#define   IMX91_TMU_CTRL1_MEAS_MODE_SINGLE	0
+#define   IMX91_TMU_CTRL1_MEAS_MODE_CONTINUES	1
+#define   IMX91_TMU_CTRL1_MEAS_MODE_PERIODIC	2
+
+#define IMX91_TMU_REF_DIV			0x280
+#define IMX91_TMU_DIV_EN			BIT(31)
+#define IMX91_TMU_DIV_MASK			GENMASK(23, 16)
+#define IMX91_TMU_DIV_MAX			255
+
+#define IMX91_TMU_PUD_ST_CTRL			0x2b0
+#define IMX91_TMU_PUDL_MASK			GENMASK(23, 16)
+
+#define IMX91_TMU_TRIM1				0x2e0
+#define IMX91_TMU_TRIM2				0x2f0
+
+#define IMX91_TMU_TEMP_LOW_LIMIT		-40000
+#define IMX91_TMU_TEMP_HIGH_LIMIT		125000
+
+#define IMX91_TMU_DEFAULT_TRIM1_CONFIG		0xb561bc2d
+#define IMX91_TMU_DEFAULT_TRIM2_CONFIG		0x65d4
+
+struct imx91_tmu {
+	void __iomem *base;
+	struct clk *clk;
+	struct device *dev;
+	struct thermal_zone_device *tzd;
+};
+
+static void imx91_tmu_start(struct imx91_tmu *tmu, bool start)
+{
+	u32 val = start ? IMX91_TMU_CTRL1_START : IMX91_TMU_CTRL1_STOP;
+
+	writel_relaxed(val, tmu->base + IMX91_TMU_CTRL1_SET);
+}
+
+static void imx91_tmu_enable(struct imx91_tmu *tmu, bool enable)
+{
+	u32 reg = enable ? IMX91_TMU_CTRL1_SET : IMX91_TMU_CTRL1_CLR;
+
+	writel_relaxed(IMX91_TMU_CTRL1_EN, tmu->base + reg);
+}
+
+static int imx91_tmu_get_temp(struct thermal_zone_device *tz, int *temp)
+{
+	struct imx91_tmu *tmu = thermal_zone_device_priv(tz);
+	s16 data;
+	int ret;
+	u32 val;
+
+	ret = pm_runtime_resume_and_get(tmu->dev);
+	if (ret < 0)
+		return ret;
+
+	imx91_tmu_start(tmu, true);
+
+	ret = readl_relaxed_poll_timeout(tmu->base + IMX91_TMU_STAT0, val,
+					 val & IMX91_TMU_STAT0_DRDY0_IF_MASK, 1000, 40000);
+	if (ret) {
+		ret = -EAGAIN;
+		goto out;
+	}
+
+	/* DATA0 is 16bit signed number */
+	data = readw_relaxed(tmu->base + IMX91_TMU_DATA0);
+	*temp = data * 1000 / 64;
+	if (*temp < IMX91_TMU_TEMP_LOW_LIMIT || *temp > IMX91_TMU_TEMP_HIGH_LIMIT)
+		ret = -EAGAIN;
+
+out:
+	pm_runtime_put(tmu->dev);
+
+	return ret;
+}
+
+static struct thermal_zone_device_ops tmu_tz_ops = {
+	.get_temp = imx91_tmu_get_temp,
+};
+
+static int imx91_init_from_nvmem_cells(struct imx91_tmu *tmu)
+{
+	struct device *dev = tmu->dev;
+	u32 trim1, trim2;
+	int ret;
+
+	ret = nvmem_cell_read_u32(dev, "trim1", &trim1);
+	if (ret)
+		return ret;
+
+	ret = nvmem_cell_read_u32(dev, "trim2", &trim2);
+	if (ret)
+		return ret;
+
+	if (trim1 == 0 || trim2 == 0)
+		return -EINVAL;
+
+	writel_relaxed(trim1, tmu->base + IMX91_TMU_TRIM1);
+	writel_relaxed(trim2, tmu->base + IMX91_TMU_TRIM2);
+
+	return 0;
+}
+
+static void imx91_tmu_action_remove(void *data)
+{
+	struct imx91_tmu *tmu = data;
+
+	/* disable tmu */
+	imx91_tmu_enable(tmu, false);
+}
+
+static int imx91_tmu_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct imx91_tmu *tmu;
+	unsigned long rate;
+	u32 div;
+	int ret;
+
+	tmu = devm_kzalloc(dev, sizeof(struct imx91_tmu), GFP_KERNEL);
+	if (!tmu)
+		return -ENOMEM;
+
+	tmu->dev = dev;
+
+	tmu->base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(tmu->base))
+		return dev_err_probe(dev, PTR_ERR(tmu->base), "failed to get io resource");
+
+	tmu->clk = devm_clk_get_enabled(dev, NULL);
+	if (IS_ERR(tmu->clk))
+		return dev_err_probe(dev, PTR_ERR(tmu->clk), "failed to get tmu clock\n");
+
+	platform_set_drvdata(pdev, tmu);
+
+	/* disable the monitor during initialization */
+	imx91_tmu_enable(tmu, false);
+	imx91_tmu_start(tmu, false);
+
+	ret = imx91_init_from_nvmem_cells(tmu);
+	if (ret) {
+		writel_relaxed(IMX91_TMU_DEFAULT_TRIM1_CONFIG, tmu->base + IMX91_TMU_TRIM1);
+		writel_relaxed(IMX91_TMU_DEFAULT_TRIM2_CONFIG, tmu->base + IMX91_TMU_TRIM2);
+	}
+
+	/* The typical conv clk is 4MHz, the output freq is 'rate / (div + 1)' */
+	rate = clk_get_rate(tmu->clk);
+	div = (rate / 4000000) - 1;
+	if (div > IMX91_TMU_DIV_MAX)
+		return dev_err_probe(dev, -EINVAL, "clock divider exceed hardware limitation");
+
+	/* Set divider value and enable divider */
+	writel_relaxed(IMX91_TMU_DIV_EN | FIELD_PREP(IMX91_TMU_DIV_MASK, div),
+		       tmu->base + IMX91_TMU_REF_DIV);
+
+	/* Set max power up delay: 'Tpud(ms) = 0xFF * 1000 / 4000000' */
+	writel_relaxed(FIELD_PREP(IMX91_TMU_PUDL_MASK, 100U), tmu->base + IMX91_TMU_PUD_ST_CTRL);
+
+	/*
+	 * Set resolution mode
+	 * 00b - Conversion time = 0.59325 ms
+	 * 01b - Conversion time = 1.10525 ms
+	 * 10b - Conversion time = 2.12925 ms
+	 * 11b - Conversion time = 4.17725 ms
+	 */
+	writel_relaxed(FIELD_PREP(IMX91_TMU_CTRL1_RES_MASK, 0x3), tmu->base + IMX91_TMU_CTRL1_CLR);
+	writel_relaxed(FIELD_PREP(IMX91_TMU_CTRL1_RES_MASK, 0x1), tmu->base + IMX91_TMU_CTRL1_SET);
+
+	writel_relaxed(IMX91_TMU_CTRL1_MEAS_MODE_MASK, tmu->base + IMX91_TMU_CTRL1_CLR);
+	writel_relaxed(FIELD_PREP(IMX91_TMU_CTRL1_MEAS_MODE_MASK, IMX91_TMU_CTRL1_MEAS_MODE_SINGLE),
+		       tmu->base + IMX91_TMU_CTRL1_SET);
+
+	pm_runtime_set_active(dev);
+	devm_pm_runtime_enable(dev);
+	pm_runtime_put(dev);
+
+	tmu->tzd = devm_thermal_of_zone_register(dev, 0, tmu, &tmu_tz_ops);
+	if (IS_ERR(tmu->tzd))
+		return dev_err_probe(dev, PTR_ERR(tmu->tzd),
+				     "failed to register thermal zone sensor\n");
+
+	ret = devm_add_action(dev, imx91_tmu_action_remove, tmu);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failure to add action imx91_tmu_action_remove()\n");
+
+	return 0;
+}
+
+static int imx91_tmu_runtime_suspend(struct device *dev)
+{
+	struct imx91_tmu *tmu = dev_get_drvdata(dev);
+
+	/* disable tmu */
+	imx91_tmu_enable(tmu, false);
+
+	clk_disable_unprepare(tmu->clk);
+
+	return 0;
+}
+
+static int imx91_tmu_runtime_resume(struct device *dev)
+{
+	struct imx91_tmu *tmu = dev_get_drvdata(dev);
+	int ret;
+
+	ret = clk_prepare_enable(tmu->clk);
+	if (ret)
+		return ret;
+
+	imx91_tmu_enable(tmu, true);
+
+	return 0;
+}
+
+static DEFINE_RUNTIME_DEV_PM_OPS(imx91_tmu_pm_ops, imx91_tmu_runtime_suspend,
+				 imx91_tmu_runtime_resume, NULL);
+
+static const struct of_device_id imx91_tmu_table[] = {
+	{ .compatible = "fsl,imx91-tmu", },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, imx91_tmu_table);
+
+static struct platform_driver imx91_tmu = {
+	.driver = {
+		.name	= "imx91_thermal",
+		.pm	= pm_ptr(&imx91_tmu_pm_ops),
+		.of_match_table = imx91_tmu_table,
+	},
+	.probe = imx91_tmu_probe,
+};
+module_platform_driver(imx91_tmu);
+
+MODULE_AUTHOR("Peng Fan <peng.fan@nxp.com>");
+MODULE_DESCRIPTION("i.MX91 Thermal Monitor Unit driver");
+MODULE_LICENSE("GPL");

-- 
2.34.1

Re: [PATCH v4 2/2] thermal: imx91: Add support for i.MX91 thermal monitoring unit

[Daniel Lezcano]

On 16/12/2024 20:25, Frank Li wrote:
> From: Pengfei Li <pengfei.li_1@nxp.com>
> 
> Introduce support for the i.MX91 thermal monitoring unit, which features a
> single sensor for the CPU. The register layout differs from other chips,
> necessitating the creation of a dedicated file for this.

Please a bit more information about the sensor (eg. resolution, I guess 1°C)

> Signed-off-by: Pengfei Li <pengfei.li_1@nxp.com>
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> Reviewed-by: Marco Felsch <m.felsch@pengutronix.de>
> Signed-off-by: Frank Li <Frank.Li@nxp.com>
> ---
> Change from v3 to v4
> - Add Macro's review tag
> - Use devm_add_action()
> - Move pm_runtim_put before thermal_of_zone_register()
> 
> change from v2 to v3
> - add IMX91_TMU_ prefix for register define
> - remove unused register define
> - fix missed pm_runtime_put() at error path in imx91_tmu_get_temp()
> - use dev variable in probe function
> - use pm_runtime_set_active() in probe
> - move START to imx91_tmu_get_temp()
> - use DEFINE_RUNTIME_DEV_PM_OPS()
> - keep set reset value because there are not sw "reset" bit in controller,
> uboot may change and enable tmu.
> 
> change from v1 to v2
> - use low case for hexvalue
> - combine struct imx91_tmu and tmu_sensor
> - simplify imx91_tmu_start() and imx91_tmu_enable()
> - use s16 for imx91_tmu_get_temp(), which may negative value
> - use reverse christmas tree style
> - use run time pm
> - use oneshot to sample temp
> - register thermal zone after hardware init
> ---
>   drivers/thermal/Kconfig         |  10 ++
>   drivers/thermal/Makefile        |   1 +
>   drivers/thermal/imx91_thermal.c | 263 ++++++++++++++++++++++++++++++++++++++++
>   3 files changed, 274 insertions(+)
> 
> diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
> index d3f9686e26e71..da403ed86aeb1 100644
> --- a/drivers/thermal/Kconfig
> +++ b/drivers/thermal/Kconfig
> @@ -296,6 +296,16 @@ config IMX8MM_THERMAL
>   	  cpufreq is used as the cooling device to throttle CPUs when the passive
>   	  trip is crossed.
>   
> +config IMX91_THERMAL
> +	tristate "Temperature sensor driver for NXP i.MX91 SoC"
> +	depends on ARCH_MXC || COMPILE_TEST
> +	depends on OF

s/OF/THERMAL_OF/

> +	help
> +	  Support for Temperature sensor found on NXP i.MX91 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.

This help message is inaccurate. It should describe the sensor not the 
thermal configuration which is coming from the device tree for a 
specific platform.

>   config K3_THERMAL
>   	tristate "Texas Instruments K3 thermal support"
>   	depends on ARCH_K3 || COMPILE_TEST
> diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile
> index 9abf43a74f2bb..08da241e6a598 100644
> --- a/drivers/thermal/Makefile
> +++ b/drivers/thermal/Makefile
> @@ -50,6 +50,7 @@ obj-$(CONFIG_ARMADA_THERMAL)	+= armada_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_IMX91_THERMAL)	+= imx91_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/imx91_thermal.c b/drivers/thermal/imx91_thermal.c
> new file mode 100644
> index 0000000000000..ef5e8e181dd0f
> --- /dev/null
> +++ b/drivers/thermal/imx91_thermal.c
> @@ -0,0 +1,263 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright 2024 NXP.
> + */
> +
> +#include <linux/bitfield.h>
> +#include <linux/clk.h>
> +#include <linux/err.h>
> +#include <linux/iopoll.h>
> +#include <linux/nvmem-consumer.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/thermal.h>
> +
> +#define IMX91_TMU_STAT0				0x10
> +#define IMX91_TMU_STAT0_DRDY0_IF_MASK		BIT(16)
> +
> +#define IMX91_TMU_DATA0				0x20
> +
> +#define IMX91_TMU_CTRL1_SET			0x204
> +#define IMX91_TMU_CTRL1_CLR			0x208
> +#define IMX91_TMU_CTRL1_EN			BIT(31)
> +#define IMX91_TMU_CTRL1_START			BIT(30)
> +#define IMX91_TMU_CTRL1_STOP			BIT(29)
> +#define IMX91_TMU_CTRL1_RES_MASK		GENMASK(19, 18)
> +#define IMX91_TMU_CTRL1_MEAS_MODE_MASK		GENMASK(25, 24)
> +#define   IMX91_TMU_CTRL1_MEAS_MODE_SINGLE	0
> +#define   IMX91_TMU_CTRL1_MEAS_MODE_CONTINUES	1
> +#define   IMX91_TMU_CTRL1_MEAS_MODE_PERIODIC	2
> +
> +#define IMX91_TMU_REF_DIV			0x280
> +#define IMX91_TMU_DIV_EN			BIT(31)
> +#define IMX91_TMU_DIV_MASK			GENMASK(23, 16)
> +#define IMX91_TMU_DIV_MAX			255
> +
> +#define IMX91_TMU_PUD_ST_CTRL			0x2b0
> +#define IMX91_TMU_PUDL_MASK			GENMASK(23, 16)
> +
> +#define IMX91_TMU_TRIM1				0x2e0
> +#define IMX91_TMU_TRIM2				0x2f0
> +
> +#define IMX91_TMU_TEMP_LOW_LIMIT		-40000
> +#define IMX91_TMU_TEMP_HIGH_LIMIT		125000
> +
> +#define IMX91_TMU_DEFAULT_TRIM1_CONFIG		0xb561bc2d
> +#define IMX91_TMU_DEFAULT_TRIM2_CONFIG		0x65d4
> +
> +struct imx91_tmu {
> +	void __iomem *base;
> +	struct clk *clk;
> +	struct device *dev;
> +	struct thermal_zone_device *tzd;

This field is pointless because used only in the probe function.

> +};
> +
> +static void imx91_tmu_start(struct imx91_tmu *tmu, bool start)
> +{
> +	u32 val = start ? IMX91_TMU_CTRL1_START : IMX91_TMU_CTRL1_STOP;
> +
> +	writel_relaxed(val, tmu->base + IMX91_TMU_CTRL1_SET);
> +}
> +
> +static void imx91_tmu_enable(struct imx91_tmu *tmu, bool enable)
> +{
> +	u32 reg = enable ? IMX91_TMU_CTRL1_SET : IMX91_TMU_CTRL1_CLR;
> +
> +	writel_relaxed(IMX91_TMU_CTRL1_EN, tmu->base + reg);
> +}
> +
> +static int imx91_tmu_get_temp(struct thermal_zone_device *tz, int *temp)
> +{
> +	struct imx91_tmu *tmu = thermal_zone_device_priv(tz);
> +	s16 data;
> +	int ret;
> +	u32 val;
> +
> +	ret = pm_runtime_resume_and_get(tmu->dev);
> +	if (ret < 0)
> +		return ret;
> +
> +	imx91_tmu_start(tmu, true);

Same question as [1]

Do you really want to start and stop the sensor between two reads ?

> +	ret = readl_relaxed_poll_timeout(tmu->base + IMX91_TMU_STAT0, val,
> +					 val & IMX91_TMU_STAT0_DRDY0_IF_MASK, 1000, 40000);
> +	if (ret) {
> +		ret = -EAGAIN;
> +		goto out;
> +	}
> +
> +	/* DATA0 is 16bit signed number */
> +	data = readw_relaxed(tmu->base + IMX91_TMU_DATA0);
> +	*temp = data * 1000 / 64;

cf units.h

	*temp = (data * MILLIDEGREE_PER_DEGREE) / A_LITERAL;

> +	if (*temp < IMX91_TMU_TEMP_LOW_LIMIT || *temp > IMX91_TMU_TEMP_HIGH_LIMIT)
> +		ret = -EAGAIN;
> +
> +out:
> +	pm_runtime_put(tmu->dev);
> +
> +	return ret;
> +}
> +
> +static struct thermal_zone_device_ops tmu_tz_ops = {
> +	.get_temp = imx91_tmu_get_temp,

Why not add the change_mode ops ?

> +};
> +
> +static int imx91_init_from_nvmem_cells(struct imx91_tmu *tmu)
> +{
> +	struct device *dev = tmu->dev;
> +	u32 trim1, trim2;
> +	int ret;
> +
> +	ret = nvmem_cell_read_u32(dev, "trim1", &trim1);
> +	if (ret)
> +		return ret;
> +
> +	ret = nvmem_cell_read_u32(dev, "trim2", &trim2);
> +	if (ret)
> +		return ret;
> +
> +	if (trim1 == 0 || trim2 == 0)
> +		return -EINVAL;
> +
> +	writel_relaxed(trim1, tmu->base + IMX91_TMU_TRIM1);
> +	writel_relaxed(trim2, tmu->base + IMX91_TMU_TRIM2);
> +
> +	return 0;
> +}
> +
> +static void imx91_tmu_action_remove(void *data)
> +{
> +	struct imx91_tmu *tmu = data;
> +
> +	/* disable tmu */
> +	imx91_tmu_enable(tmu, false);
> +}
> +
> +static int imx91_tmu_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct imx91_tmu *tmu;
> +	unsigned long rate;
> +	u32 div;
> +	int ret;
> +
> +	tmu = devm_kzalloc(dev, sizeof(struct imx91_tmu), GFP_KERNEL);
> +	if (!tmu)
> +		return -ENOMEM;
> +
> +	tmu->dev = dev;
> +
> +	tmu->base = devm_platform_ioremap_resource(pdev, 0);
> +	if (IS_ERR(tmu->base))
> +		return dev_err_probe(dev, PTR_ERR(tmu->base), "failed to get io resource");
> +
> +	tmu->clk = devm_clk_get_enabled(dev, NULL);
> +	if (IS_ERR(tmu->clk))
> +		return dev_err_probe(dev, PTR_ERR(tmu->clk), "failed to get tmu clock\n");
> +
> +	platform_set_drvdata(pdev, tmu);
> +
> +	/* disable the monitor during initialization */
> +	imx91_tmu_enable(tmu, false);
> +	imx91_tmu_start(tmu, false);
> +
> +	ret = imx91_init_from_nvmem_cells(tmu);
> +	if (ret) {
> +		writel_relaxed(IMX91_TMU_DEFAULT_TRIM1_CONFIG, tmu->base + IMX91_TMU_TRIM1);
> +		writel_relaxed(IMX91_TMU_DEFAULT_TRIM2_CONFIG, tmu->base + IMX91_TMU_TRIM2);
> +	}
> +
> +	/* The typical conv clk is 4MHz, the output freq is 'rate / (div + 1)' */
> +	rate = clk_get_rate(tmu->clk);
> +	div = (rate / 4000000) - 1;

Use literals please (eg. 4 * HZ_PER_MHZ)

> +	if (div > IMX91_TMU_DIV_MAX)
> +		return dev_err_probe(dev, -EINVAL, "clock divider exceed hardware limitation");
> +
> +	/* Set divider value and enable divider */
> +	writel_relaxed(IMX91_TMU_DIV_EN | FIELD_PREP(IMX91_TMU_DIV_MASK, div),
> +		       tmu->base + IMX91_TMU_REF_DIV);
> +
> +	/* Set max power up delay: 'Tpud(ms) = 0xFF * 1000 / 4000000' */
> +	writel_relaxed(FIELD_PREP(IMX91_TMU_PUDL_MASK, 100U), tmu->base + IMX91_TMU_PUD_ST_CTRL);
> +
> +	/*
> +	 * Set resolution mode
> +	 * 00b - Conversion time = 0.59325 ms
> +	 * 01b - Conversion time = 1.10525 ms
> +	 * 10b - Conversion time = 2.12925 ms
> +	 * 11b - Conversion time = 4.17725 ms
> +	 */
> +	writel_relaxed(FIELD_PREP(IMX91_TMU_CTRL1_RES_MASK, 0x3), tmu->base + IMX91_TMU_CTRL1_CLR);
> +	writel_relaxed(FIELD_PREP(IMX91_TMU_CTRL1_RES_MASK, 0x1), tmu->base + IMX91_TMU_CTRL1_SET);
> +
> +	writel_relaxed(IMX91_TMU_CTRL1_MEAS_MODE_MASK, tmu->base + IMX91_TMU_CTRL1_CLR);
> +	writel_relaxed(FIELD_PREP(IMX91_TMU_CTRL1_MEAS_MODE_MASK, IMX91_TMU_CTRL1_MEAS_MODE_SINGLE),
> +		       tmu->base + IMX91_TMU_CTRL1_SET);
> +
> +	pm_runtime_set_active(dev);
> +	devm_pm_runtime_enable(dev);
> +	pm_runtime_put(dev);
> +
> +	tmu->tzd = devm_thermal_of_zone_register(dev, 0, tmu, &tmu_tz_ops);
> +	if (IS_ERR(tmu->tzd))
> +		return dev_err_probe(dev, PTR_ERR(tmu->tzd),
> +				     "failed to register thermal zone sensor\n");
> +
> +	ret = devm_add_action(dev, imx91_tmu_action_remove, tmu);

Should it be moved before devm_thermal_of_zone_register(), so if the 
thermal zone creation fails, it will stop the sensor which was 
previously started ?

> +	if (ret)
> +		return dev_err_probe(dev, ret, "Failure to add action imx91_tmu_action_remove()\n");
> +
> +	return 0;
> +}
> +
> +static int imx91_tmu_runtime_suspend(struct device *dev)
> +{
> +	struct imx91_tmu *tmu = dev_get_drvdata(dev);
> +
> +	/* disable tmu */
> +	imx91_tmu_enable(tmu, false);
> +
> +	clk_disable_unprepare(tmu->clk);
> +
> +	return 0;
> +}
> +
> +static int imx91_tmu_runtime_resume(struct device *dev)
> +{
> +	struct imx91_tmu *tmu = dev_get_drvdata(dev);
> +	int ret;
> +
> +	ret = clk_prepare_enable(tmu->clk);
> +	if (ret)
> +		return ret;
> +
> +	imx91_tmu_enable(tmu, true);
> +
> +	return 0;
> +}
> +
> +static DEFINE_RUNTIME_DEV_PM_OPS(imx91_tmu_pm_ops, imx91_tmu_runtime_suspend,
> +				 imx91_tmu_runtime_resume, NULL);
> +
> +static const struct of_device_id imx91_tmu_table[] = {
> +	{ .compatible = "fsl,imx91-tmu", },
> +	{ },
> +};
> +MODULE_DEVICE_TABLE(of, imx91_tmu_table);
> +
> +static struct platform_driver imx91_tmu = {
> +	.driver = {
> +		.name	= "imx91_thermal",
> +		.pm	= pm_ptr(&imx91_tmu_pm_ops),
> +		.of_match_table = imx91_tmu_table,
> +	},
> +	.probe = imx91_tmu_probe,
> +};
> +module_platform_driver(imx91_tmu);
> +
> +MODULE_AUTHOR("Peng Fan <peng.fan@nxp.com>");
> +MODULE_DESCRIPTION("i.MX91 Thermal Monitor Unit driver");
> +MODULE_LICENSE("GPL");
> 


-- 
<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 v4 2/2] thermal: imx91: Add support for i.MX91 thermal monitoring unit

[Frank Li]

On Fri, Jan 31, 2025 at 11:19:54AM +0100, Daniel Lezcano wrote:
> On 16/12/2024 20:25, Frank Li wrote:
> > From: Pengfei Li <pengfei.li_1@nxp.com>
> >
> > Introduce support for the i.MX91 thermal monitoring unit, which features a
> > single sensor for the CPU. The register layout differs from other chips,
> > necessitating the creation of a dedicated file for this.
>
> Please a bit more information about the sensor (eg. resolution, I guess 1°C)

How about add

"This sensor provides a resolution of 1/64°C (6-bit fraction). For actual
accuracy, refer to the datasheet, as it varies depending on the chip grade."

> > Signed-off-by: Pengfei Li <pengfei.li_1@nxp.com>
> > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > Reviewed-by: Marco Felsch <m.felsch@pengutronix.de>
> > Signed-off-by: Frank Li <Frank.Li@nxp.com>
> > ---
> > Change from v3 to v4
> > - Add Macro's review tag
> > - Use devm_add_action()
> > - Move pm_runtim_put before thermal_of_zone_register()
> >
> > change from v2 to v3
> > - add IMX91_TMU_ prefix for register define
> > - remove unused register define
> > - fix missed pm_runtime_put() at error path in imx91_tmu_get_temp()
> > - use dev variable in probe function
> > - use pm_runtime_set_active() in probe
> > - move START to imx91_tmu_get_temp()
> > - use DEFINE_RUNTIME_DEV_PM_OPS()
> > - keep set reset value because there are not sw "reset" bit in controller,
> > uboot may change and enable tmu.
> >
> > change from v1 to v2
> > - use low case for hexvalue
> > - combine struct imx91_tmu and tmu_sensor
> > - simplify imx91_tmu_start() and imx91_tmu_enable()
> > - use s16 for imx91_tmu_get_temp(), which may negative value
> > - use reverse christmas tree style
> > - use run time pm
> > - use oneshot to sample temp
> > - register thermal zone after hardware init
> > ---
> >   drivers/thermal/Kconfig         |  10 ++
> >   drivers/thermal/Makefile        |   1 +
> >   drivers/thermal/imx91_thermal.c | 263 ++++++++++++++++++++++++++++++++++++++++
> >   3 files changed, 274 insertions(+)
> >
> > diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
> > index d3f9686e26e71..da403ed86aeb1 100644
> > --- a/drivers/thermal/Kconfig
> > +++ b/drivers/thermal/Kconfig
> > @@ -296,6 +296,16 @@ config IMX8MM_THERMAL
> >   	  cpufreq is used as the cooling device to throttle CPUs when the passive
> >   	  trip is crossed.
> > +config IMX91_THERMAL
> > +	tristate "Temperature sensor driver for NXP i.MX91 SoC"
> > +	depends on ARCH_MXC || COMPILE_TEST
> > +	depends on OF
>
> s/OF/THERMAL_OF/
>
> > +	help
> > +	  Support for Temperature sensor found on NXP i.MX91 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.
>
> This help message is inaccurate. It should describe the sensor not the
> thermal configuration which is coming from the device tree for a specific
> platform.

How about

It includes one sensor and six comparators. Each of them compares the
temperature value (from the sensor) against the programmable threshold
values. Support program the direction of comparison (greater than or
less than).
>
> >   config K3_THERMAL
> >   	tristate "Texas Instruments K3 thermal support"
> >   	depends on ARCH_K3 || COMPILE_TEST
> > diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile
> > index 9abf43a74f2bb..08da241e6a598 100644
> > --- a/drivers/thermal/Makefile
> > +++ b/drivers/thermal/Makefile
> > @@ -50,6 +50,7 @@ obj-$(CONFIG_ARMADA_THERMAL)	+= armada_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_IMX91_THERMAL)	+= imx91_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/imx91_thermal.c b/drivers/thermal/imx91_thermal.c
> > new file mode 100644
> > index 0000000000000..ef5e8e181dd0f
> > --- /dev/null
> > +++ b/drivers/thermal/imx91_thermal.c
> > @@ -0,0 +1,263 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright 2024 NXP.
> > + */
> > +
> > +#include <linux/bitfield.h>
> > +#include <linux/clk.h>
> > +#include <linux/err.h>
> > +#include <linux/iopoll.h>
> > +#include <linux/nvmem-consumer.h>
> > +#include <linux/module.h>
> > +#include <linux/of.h>
> > +#include <linux/of_device.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/pm_runtime.h>
> > +#include <linux/thermal.h>
> > +
> > +#define IMX91_TMU_STAT0				0x10
> > +#define IMX91_TMU_STAT0_DRDY0_IF_MASK		BIT(16)
> > +
> > +#define IMX91_TMU_DATA0				0x20
> > +
> > +#define IMX91_TMU_CTRL1_SET			0x204
> > +#define IMX91_TMU_CTRL1_CLR			0x208
> > +#define IMX91_TMU_CTRL1_EN			BIT(31)
> > +#define IMX91_TMU_CTRL1_START			BIT(30)
> > +#define IMX91_TMU_CTRL1_STOP			BIT(29)
> > +#define IMX91_TMU_CTRL1_RES_MASK		GENMASK(19, 18)
> > +#define IMX91_TMU_CTRL1_MEAS_MODE_MASK		GENMASK(25, 24)
> > +#define   IMX91_TMU_CTRL1_MEAS_MODE_SINGLE	0
> > +#define   IMX91_TMU_CTRL1_MEAS_MODE_CONTINUES	1
> > +#define   IMX91_TMU_CTRL1_MEAS_MODE_PERIODIC	2
> > +
> > +#define IMX91_TMU_REF_DIV			0x280
> > +#define IMX91_TMU_DIV_EN			BIT(31)
> > +#define IMX91_TMU_DIV_MASK			GENMASK(23, 16)
> > +#define IMX91_TMU_DIV_MAX			255
> > +
> > +#define IMX91_TMU_PUD_ST_CTRL			0x2b0
> > +#define IMX91_TMU_PUDL_MASK			GENMASK(23, 16)
> > +
> > +#define IMX91_TMU_TRIM1				0x2e0
> > +#define IMX91_TMU_TRIM2				0x2f0
> > +
> > +#define IMX91_TMU_TEMP_LOW_LIMIT		-40000
> > +#define IMX91_TMU_TEMP_HIGH_LIMIT		125000
> > +
> > +#define IMX91_TMU_DEFAULT_TRIM1_CONFIG		0xb561bc2d
> > +#define IMX91_TMU_DEFAULT_TRIM2_CONFIG		0x65d4
> > +
> > +struct imx91_tmu {
> > +	void __iomem *base;
> > +	struct clk *clk;
> > +	struct device *dev;
> > +	struct thermal_zone_device *tzd;
>
> This field is pointless because used only in the probe function.
>
> > +};
> > +
> > +static void imx91_tmu_start(struct imx91_tmu *tmu, bool start)
> > +{
> > +	u32 val = start ? IMX91_TMU_CTRL1_START : IMX91_TMU_CTRL1_STOP;
> > +
> > +	writel_relaxed(val, tmu->base + IMX91_TMU_CTRL1_SET);
> > +}
> > +
> > +static void imx91_tmu_enable(struct imx91_tmu *tmu, bool enable)
> > +{
> > +	u32 reg = enable ? IMX91_TMU_CTRL1_SET : IMX91_TMU_CTRL1_CLR;
> > +
> > +	writel_relaxed(IMX91_TMU_CTRL1_EN, tmu->base + reg);
> > +}
> > +
> > +static int imx91_tmu_get_temp(struct thermal_zone_device *tz, int *temp)
> > +{
> > +	struct imx91_tmu *tmu = thermal_zone_device_priv(tz);
> > +	s16 data;
> > +	int ret;
> > +	u32 val;
> > +
> > +	ret = pm_runtime_resume_and_get(tmu->dev);
> > +	if (ret < 0)
> > +		return ret;
> > +
> > +	imx91_tmu_start(tmu, true);
>
> Same question as [1]
>
> Do you really want to start and stop the sensor between two reads ?

This version use simple oneshot mode. So need call start every time.

>
> > +	ret = readl_relaxed_poll_timeout(tmu->base + IMX91_TMU_STAT0, val,
> > +					 val & IMX91_TMU_STAT0_DRDY0_IF_MASK, 1000, 40000);
> > +	if (ret) {
> > +		ret = -EAGAIN;
> > +		goto out;
> > +	}
> > +
> > +	/* DATA0 is 16bit signed number */
> > +	data = readw_relaxed(tmu->base + IMX91_TMU_DATA0);
> > +	*temp = data * 1000 / 64;
>
> cf units.h
>
> 	*temp = (data * MILLIDEGREE_PER_DEGREE) / A_LITERAL;
>
> > +	if (*temp < IMX91_TMU_TEMP_LOW_LIMIT || *temp > IMX91_TMU_TEMP_HIGH_LIMIT)
> > +		ret = -EAGAIN;
> > +
> > +out:
> > +	pm_runtime_put(tmu->dev);
> > +
> > +	return ret;
> > +}
> > +
> > +static struct thermal_zone_device_ops tmu_tz_ops = {
> > +	.get_temp = imx91_tmu_get_temp,
>
> Why not add the change_mode ops ?

I plan add later when add .set_trip_temp and irq support.

Frank
>
> > +};
> > +
> > +static int imx91_init_from_nvmem_cells(struct imx91_tmu *tmu)
> > +{
> > +	struct device *dev = tmu->dev;
> > +	u32 trim1, trim2;
> > +	int ret;
> > +
> > +	ret = nvmem_cell_read_u32(dev, "trim1", &trim1);
> > +	if (ret)
> > +		return ret;
> > +
> > +	ret = nvmem_cell_read_u32(dev, "trim2", &trim2);
> > +	if (ret)
> > +		return ret;
> > +
> > +	if (trim1 == 0 || trim2 == 0)
> > +		return -EINVAL;
> > +
> > +	writel_relaxed(trim1, tmu->base + IMX91_TMU_TRIM1);
> > +	writel_relaxed(trim2, tmu->base + IMX91_TMU_TRIM2);
> > +
> > +	return 0;
> > +}
> > +
> > +static void imx91_tmu_action_remove(void *data)
> > +{
> > +	struct imx91_tmu *tmu = data;
> > +
> > +	/* disable tmu */
> > +	imx91_tmu_enable(tmu, false);
> > +}
> > +
> > +static int imx91_tmu_probe(struct platform_device *pdev)
> > +{
> > +	struct device *dev = &pdev->dev;
> > +	struct imx91_tmu *tmu;
> > +	unsigned long rate;
> > +	u32 div;
> > +	int ret;
> > +
> > +	tmu = devm_kzalloc(dev, sizeof(struct imx91_tmu), GFP_KERNEL);
> > +	if (!tmu)
> > +		return -ENOMEM;
> > +
> > +	tmu->dev = dev;
> > +
> > +	tmu->base = devm_platform_ioremap_resource(pdev, 0);
> > +	if (IS_ERR(tmu->base))
> > +		return dev_err_probe(dev, PTR_ERR(tmu->base), "failed to get io resource");
> > +
> > +	tmu->clk = devm_clk_get_enabled(dev, NULL);
> > +	if (IS_ERR(tmu->clk))
> > +		return dev_err_probe(dev, PTR_ERR(tmu->clk), "failed to get tmu clock\n");
> > +
> > +	platform_set_drvdata(pdev, tmu);
> > +
> > +	/* disable the monitor during initialization */
> > +	imx91_tmu_enable(tmu, false);
> > +	imx91_tmu_start(tmu, false);
> > +
> > +	ret = imx91_init_from_nvmem_cells(tmu);
> > +	if (ret) {
> > +		writel_relaxed(IMX91_TMU_DEFAULT_TRIM1_CONFIG, tmu->base + IMX91_TMU_TRIM1);
> > +		writel_relaxed(IMX91_TMU_DEFAULT_TRIM2_CONFIG, tmu->base + IMX91_TMU_TRIM2);
> > +	}
> > +
> > +	/* The typical conv clk is 4MHz, the output freq is 'rate / (div + 1)' */
> > +	rate = clk_get_rate(tmu->clk);
> > +	div = (rate / 4000000) - 1;
>
> Use literals please (eg. 4 * HZ_PER_MHZ)
>
> > +	if (div > IMX91_TMU_DIV_MAX)
> > +		return dev_err_probe(dev, -EINVAL, "clock divider exceed hardware limitation");
> > +
> > +	/* Set divider value and enable divider */
> > +	writel_relaxed(IMX91_TMU_DIV_EN | FIELD_PREP(IMX91_TMU_DIV_MASK, div),
> > +		       tmu->base + IMX91_TMU_REF_DIV);
> > +
> > +	/* Set max power up delay: 'Tpud(ms) = 0xFF * 1000 / 4000000' */
> > +	writel_relaxed(FIELD_PREP(IMX91_TMU_PUDL_MASK, 100U), tmu->base + IMX91_TMU_PUD_ST_CTRL);
> > +
> > +	/*
> > +	 * Set resolution mode
> > +	 * 00b - Conversion time = 0.59325 ms
> > +	 * 01b - Conversion time = 1.10525 ms
> > +	 * 10b - Conversion time = 2.12925 ms
> > +	 * 11b - Conversion time = 4.17725 ms
> > +	 */
> > +	writel_relaxed(FIELD_PREP(IMX91_TMU_CTRL1_RES_MASK, 0x3), tmu->base + IMX91_TMU_CTRL1_CLR);
> > +	writel_relaxed(FIELD_PREP(IMX91_TMU_CTRL1_RES_MASK, 0x1), tmu->base + IMX91_TMU_CTRL1_SET);
> > +
> > +	writel_relaxed(IMX91_TMU_CTRL1_MEAS_MODE_MASK, tmu->base + IMX91_TMU_CTRL1_CLR);
> > +	writel_relaxed(FIELD_PREP(IMX91_TMU_CTRL1_MEAS_MODE_MASK, IMX91_TMU_CTRL1_MEAS_MODE_SINGLE),
> > +		       tmu->base + IMX91_TMU_CTRL1_SET);
> > +
> > +	pm_runtime_set_active(dev);
> > +	devm_pm_runtime_enable(dev);
> > +	pm_runtime_put(dev);
> > +
> > +	tmu->tzd = devm_thermal_of_zone_register(dev, 0, tmu, &tmu_tz_ops);
> > +	if (IS_ERR(tmu->tzd))
> > +		return dev_err_probe(dev, PTR_ERR(tmu->tzd),
> > +				     "failed to register thermal zone sensor\n");
> > +
> > +	ret = devm_add_action(dev, imx91_tmu_action_remove, tmu);
>
> Should it be moved before devm_thermal_of_zone_register(), so if the thermal
> zone creation fails, it will stop the sensor which was previously started ?

pm_runtime_put already disable sensor before call
devm_thermal_of_zone_register().

imx91_tmu_get_temp() will call runtime_get when devm_thermal_of_zone_register()
call to it.

Frank

>
> > +	if (ret)
> > +		return dev_err_probe(dev, ret, "Failure to add action imx91_tmu_action_remove()\n");
> > +
> > +	return 0;
> > +}
> > +
> > +static int imx91_tmu_runtime_suspend(struct device *dev)
> > +{
> > +	struct imx91_tmu *tmu = dev_get_drvdata(dev);
> > +
> > +	/* disable tmu */
> > +	imx91_tmu_enable(tmu, false);
> > +
> > +	clk_disable_unprepare(tmu->clk);
> > +
> > +	return 0;
> > +}
> > +
> > +static int imx91_tmu_runtime_resume(struct device *dev)
> > +{
> > +	struct imx91_tmu *tmu = dev_get_drvdata(dev);
> > +	int ret;
> > +
> > +	ret = clk_prepare_enable(tmu->clk);
> > +	if (ret)
> > +		return ret;
> > +
> > +	imx91_tmu_enable(tmu, true);
> > +
> > +	return 0;
> > +}
> > +
> > +static DEFINE_RUNTIME_DEV_PM_OPS(imx91_tmu_pm_ops, imx91_tmu_runtime_suspend,
> > +				 imx91_tmu_runtime_resume, NULL);
> > +
> > +static const struct of_device_id imx91_tmu_table[] = {
> > +	{ .compatible = "fsl,imx91-tmu", },
> > +	{ },
> > +};
> > +MODULE_DEVICE_TABLE(of, imx91_tmu_table);
> > +
> > +static struct platform_driver imx91_tmu = {
> > +	.driver = {
> > +		.name	= "imx91_thermal",
> > +		.pm	= pm_ptr(&imx91_tmu_pm_ops),
> > +		.of_match_table = imx91_tmu_table,
> > +	},
> > +	.probe = imx91_tmu_probe,
> > +};
> > +module_platform_driver(imx91_tmu);
> > +
> > +MODULE_AUTHOR("Peng Fan <peng.fan@nxp.com>");
> > +MODULE_DESCRIPTION("i.MX91 Thermal Monitor Unit driver");
> > +MODULE_LICENSE("GPL");
> >
>
>
> --
> <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 v4 2/2] thermal: imx91: Add support for i.MX91 thermal monitoring unit

[Daniel Lezcano]

On 31/01/2025 23:22, Frank Li wrote:
> On Fri, Jan 31, 2025 at 11:19:54AM +0100, Daniel Lezcano wrote:
>> On 16/12/2024 20:25, Frank Li wrote:
>>> From: Pengfei Li <pengfei.li_1@nxp.com>
>>>
>>> Introduce support for the i.MX91 thermal monitoring unit, which features a
>>> single sensor for the CPU. The register layout differs from other chips,
>>> necessitating the creation of a dedicated file for this.
>>
>> Please a bit more information about the sensor (eg. resolution, I guess 1°C)
> 
> How about add
> 
> "This sensor provides a resolution of 1/64°C (6-bit fraction). For actual
> accuracy, refer to the datasheet, as it varies depending on the chip grade."

A bit of more information is welcome :) eg is it interrupt driven ?

Also Link: <link to the datasheet>

>>> Signed-off-by: Pengfei Li <pengfei.li_1@nxp.com>
>>> Signed-off-by: Peng Fan <peng.fan@nxp.com>
>>> Reviewed-by: Marco Felsch <m.felsch@pengutronix.de>
>>> Signed-off-by: Frank Li <Frank.Li@nxp.com>
>>> ---
>>> Change from v3 to v4
>>> - Add Macro's review tag
>>> - Use devm_add_action()
>>> - Move pm_runtim_put before thermal_of_zone_register()
>>>
>>> change from v2 to v3
>>> - add IMX91_TMU_ prefix for register define
>>> - remove unused register define
>>> - fix missed pm_runtime_put() at error path in imx91_tmu_get_temp()
>>> - use dev variable in probe function
>>> - use pm_runtime_set_active() in probe
>>> - move START to imx91_tmu_get_temp()
>>> - use DEFINE_RUNTIME_DEV_PM_OPS()
>>> - keep set reset value because there are not sw "reset" bit in controller,
>>> uboot may change and enable tmu.
>>>
>>> change from v1 to v2
>>> - use low case for hexvalue
>>> - combine struct imx91_tmu and tmu_sensor
>>> - simplify imx91_tmu_start() and imx91_tmu_enable()
>>> - use s16 for imx91_tmu_get_temp(), which may negative value
>>> - use reverse christmas tree style
>>> - use run time pm
>>> - use oneshot to sample temp
>>> - register thermal zone after hardware init
>>> ---
>>>    drivers/thermal/Kconfig         |  10 ++
>>>    drivers/thermal/Makefile        |   1 +
>>>    drivers/thermal/imx91_thermal.c | 263 ++++++++++++++++++++++++++++++++++++++++
>>>    3 files changed, 274 insertions(+)
>>>
>>> diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
>>> index d3f9686e26e71..da403ed86aeb1 100644
>>> --- a/drivers/thermal/Kconfig
>>> +++ b/drivers/thermal/Kconfig
>>> @@ -296,6 +296,16 @@ config IMX8MM_THERMAL
>>>    	  cpufreq is used as the cooling device to throttle CPUs when the passive
>>>    	  trip is crossed.
>>> +config IMX91_THERMAL
>>> +	tristate "Temperature sensor driver for NXP i.MX91 SoC"
>>> +	depends on ARCH_MXC || COMPILE_TEST
>>> +	depends on OF
>>
>> s/OF/THERMAL_OF/
>>
>>> +	help
>>> +	  Support for Temperature sensor found on NXP i.MX91 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.
>>
>> This help message is inaccurate. It should describe the sensor not the
>> thermal configuration which is coming from the device tree for a specific
>> platform.
> 
> How about
> 
> It includes one sensor and six comparators. Each of them compares the
> temperature value (from the sensor) against the programmable threshold
> values. Support program the direction of comparison (greater than or
> less than).

Sounds good except the last sentence, I suggest " ... values. The 
direction of the comparison is configurable (greater / lesser than).

>>>    config K3_THERMAL
>>>    	tristate "Texas Instruments K3 thermal support"
>>>    	depends on ARCH_K3 || COMPILE_TEST
>>> diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile
>>> index 9abf43a74f2bb..08da241e6a598 100644
>>> --- a/drivers/thermal/Makefile
>>> +++ b/drivers/thermal/Makefile
>>> @@ -50,6 +50,7 @@ obj-$(CONFIG_ARMADA_THERMAL)	+= armada_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_IMX91_THERMAL)	+= imx91_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/imx91_thermal.c b/drivers/thermal/imx91_thermal.c
>>> new file mode 100644
>>> index 0000000000000..ef5e8e181dd0f
>>> --- /dev/null
>>> +++ b/drivers/thermal/imx91_thermal.c
>>> @@ -0,0 +1,263 @@
>>> +// SPDX-License-Identifier: GPL-2.0
>>> +/*
>>> + * Copyright 2024 NXP.
>>> + */
>>> +
>>> +#include <linux/bitfield.h>
>>> +#include <linux/clk.h>
>>> +#include <linux/err.h>
>>> +#include <linux/iopoll.h>
>>> +#include <linux/nvmem-consumer.h>
>>> +#include <linux/module.h>
>>> +#include <linux/of.h>
>>> +#include <linux/of_device.h>
>>> +#include <linux/platform_device.h>
>>> +#include <linux/pm_runtime.h>
>>> +#include <linux/thermal.h>
>>> +
>>> +#define IMX91_TMU_STAT0				0x10
>>> +#define IMX91_TMU_STAT0_DRDY0_IF_MASK		BIT(16)
>>> +
>>> +#define IMX91_TMU_DATA0				0x20
>>> +
>>> +#define IMX91_TMU_CTRL1_SET			0x204
>>> +#define IMX91_TMU_CTRL1_CLR			0x208
>>> +#define IMX91_TMU_CTRL1_EN			BIT(31)
>>> +#define IMX91_TMU_CTRL1_START			BIT(30)
>>> +#define IMX91_TMU_CTRL1_STOP			BIT(29)
>>> +#define IMX91_TMU_CTRL1_RES_MASK		GENMASK(19, 18)
>>> +#define IMX91_TMU_CTRL1_MEAS_MODE_MASK		GENMASK(25, 24)
>>> +#define   IMX91_TMU_CTRL1_MEAS_MODE_SINGLE	0
>>> +#define   IMX91_TMU_CTRL1_MEAS_MODE_CONTINUES	1
>>> +#define   IMX91_TMU_CTRL1_MEAS_MODE_PERIODIC	2
>>> +
>>> +#define IMX91_TMU_REF_DIV			0x280
>>> +#define IMX91_TMU_DIV_EN			BIT(31)
>>> +#define IMX91_TMU_DIV_MASK			GENMASK(23, 16)
>>> +#define IMX91_TMU_DIV_MAX			255
>>> +
>>> +#define IMX91_TMU_PUD_ST_CTRL			0x2b0
>>> +#define IMX91_TMU_PUDL_MASK			GENMASK(23, 16)
>>> +
>>> +#define IMX91_TMU_TRIM1				0x2e0
>>> +#define IMX91_TMU_TRIM2				0x2f0
>>> +
>>> +#define IMX91_TMU_TEMP_LOW_LIMIT		-40000
>>> +#define IMX91_TMU_TEMP_HIGH_LIMIT		125000
>>> +
>>> +#define IMX91_TMU_DEFAULT_TRIM1_CONFIG		0xb561bc2d
>>> +#define IMX91_TMU_DEFAULT_TRIM2_CONFIG		0x65d4
>>> +
>>> +struct imx91_tmu {
>>> +	void __iomem *base;
>>> +	struct clk *clk;
>>> +	struct device *dev;
>>> +	struct thermal_zone_device *tzd;
>>
>> This field is pointless because used only in the probe function.
>>
>>> +};
>>> +
>>> +static void imx91_tmu_start(struct imx91_tmu *tmu, bool start)
>>> +{
>>> +	u32 val = start ? IMX91_TMU_CTRL1_START : IMX91_TMU_CTRL1_STOP;
>>> +
>>> +	writel_relaxed(val, tmu->base + IMX91_TMU_CTRL1_SET);
>>> +}
>>> +
>>> +static void imx91_tmu_enable(struct imx91_tmu *tmu, bool enable)
>>> +{
>>> +	u32 reg = enable ? IMX91_TMU_CTRL1_SET : IMX91_TMU_CTRL1_CLR;
>>> +
>>> +	writel_relaxed(IMX91_TMU_CTRL1_EN, tmu->base + reg);
>>> +}
>>> +
>>> +static int imx91_tmu_get_temp(struct thermal_zone_device *tz, int *temp)
>>> +{
>>> +	struct imx91_tmu *tmu = thermal_zone_device_priv(tz);
>>> +	s16 data;
>>> +	int ret;
>>> +	u32 val;
>>> +
>>> +	ret = pm_runtime_resume_and_get(tmu->dev);
>>> +	if (ret < 0)
>>> +		return ret;
>>> +
>>> +	imx91_tmu_start(tmu, true);
>>
>> Same question as [1]
>>
>> Do you really want to start and stop the sensor between two reads ?
> 
> This version use simple oneshot mode. So need call start every time.

The question remains the same. Why is one shot / single preferred than 
'continuous' ?

Stopping the measurement between two reads can have an impact on the 
accuracy and it adds an overhead when doing a poll read which is not 
desirable when reaching the thermal pressure where the mitigation kicks in.


>>> +	ret = readl_relaxed_poll_timeout(tmu->base + IMX91_TMU_STAT0, val,
>>> +					 val & IMX91_TMU_STAT0_DRDY0_IF_MASK, 1000, 40000);
>>> +	if (ret) {
>>> +		ret = -EAGAIN;
>>> +		goto out;
>>> +	}
>>> +
>>> +	/* DATA0 is 16bit signed number */
>>> +	data = readw_relaxed(tmu->base + IMX91_TMU_DATA0);
>>> +	*temp = data * 1000 / 64;
>>
>> cf units.h
>>
>> 	*temp = (data * MILLIDEGREE_PER_DEGREE) / A_LITERAL;
>>
>>> +	if (*temp < IMX91_TMU_TEMP_LOW_LIMIT || *temp > IMX91_TMU_TEMP_HIGH_LIMIT)
>>> +		ret = -EAGAIN;
>>> +
>>> +out:
>>> +	pm_runtime_put(tmu->dev);
>>> +
>>> +	return ret;
>>> +}
>>> +
>>> +static struct thermal_zone_device_ops tmu_tz_ops = {
>>> +	.get_temp = imx91_tmu_get_temp,
>>
>> Why not add the change_mode ops ?
> 
> I plan add later when add .set_trip_temp and irq support.
> 
> Frank
>>
>>> +};
>>> +
>>> +static int imx91_init_from_nvmem_cells(struct imx91_tmu *tmu)
>>> +{
>>> +	struct device *dev = tmu->dev;
>>> +	u32 trim1, trim2;
>>> +	int ret;
>>> +
>>> +	ret = nvmem_cell_read_u32(dev, "trim1", &trim1);
>>> +	if (ret)
>>> +		return ret;
>>> +
>>> +	ret = nvmem_cell_read_u32(dev, "trim2", &trim2);
>>> +	if (ret)
>>> +		return ret;
>>> +
>>> +	if (trim1 == 0 || trim2 == 0)
>>> +		return -EINVAL;
>>> +
>>> +	writel_relaxed(trim1, tmu->base + IMX91_TMU_TRIM1);
>>> +	writel_relaxed(trim2, tmu->base + IMX91_TMU_TRIM2);
>>> +
>>> +	return 0;
>>> +}
>>> +
>>> +static void imx91_tmu_action_remove(void *data)
>>> +{
>>> +	struct imx91_tmu *tmu = data;
>>> +
>>> +	/* disable tmu */
>>> +	imx91_tmu_enable(tmu, false);
>>> +}
>>> +
>>> +static int imx91_tmu_probe(struct platform_device *pdev)
>>> +{
>>> +	struct device *dev = &pdev->dev;
>>> +	struct imx91_tmu *tmu;
>>> +	unsigned long rate;
>>> +	u32 div;
>>> +	int ret;
>>> +
>>> +	tmu = devm_kzalloc(dev, sizeof(struct imx91_tmu), GFP_KERNEL);
>>> +	if (!tmu)
>>> +		return -ENOMEM;
>>> +
>>> +	tmu->dev = dev;
>>> +
>>> +	tmu->base = devm_platform_ioremap_resource(pdev, 0);
>>> +	if (IS_ERR(tmu->base))
>>> +		return dev_err_probe(dev, PTR_ERR(tmu->base), "failed to get io resource");
>>> +
>>> +	tmu->clk = devm_clk_get_enabled(dev, NULL);
>>> +	if (IS_ERR(tmu->clk))
>>> +		return dev_err_probe(dev, PTR_ERR(tmu->clk), "failed to get tmu clock\n");
>>> +
>>> +	platform_set_drvdata(pdev, tmu);
>>> +
>>> +	/* disable the monitor during initialization */
>>> +	imx91_tmu_enable(tmu, false);
>>> +	imx91_tmu_start(tmu, false);
>>> +
>>> +	ret = imx91_init_from_nvmem_cells(tmu);
>>> +	if (ret) {
>>> +		writel_relaxed(IMX91_TMU_DEFAULT_TRIM1_CONFIG, tmu->base + IMX91_TMU_TRIM1);
>>> +		writel_relaxed(IMX91_TMU_DEFAULT_TRIM2_CONFIG, tmu->base + IMX91_TMU_TRIM2);
>>> +	}
>>> +
>>> +	/* The typical conv clk is 4MHz, the output freq is 'rate / (div + 1)' */
>>> +	rate = clk_get_rate(tmu->clk);
>>> +	div = (rate / 4000000) - 1;
>>
>> Use literals please (eg. 4 * HZ_PER_MHZ)
>>
>>> +	if (div > IMX91_TMU_DIV_MAX)
>>> +		return dev_err_probe(dev, -EINVAL, "clock divider exceed hardware limitation");
>>> +
>>> +	/* Set divider value and enable divider */
>>> +	writel_relaxed(IMX91_TMU_DIV_EN | FIELD_PREP(IMX91_TMU_DIV_MASK, div),
>>> +		       tmu->base + IMX91_TMU_REF_DIV);
>>> +
>>> +	/* Set max power up delay: 'Tpud(ms) = 0xFF * 1000 / 4000000' */
>>> +	writel_relaxed(FIELD_PREP(IMX91_TMU_PUDL_MASK, 100U), tmu->base + IMX91_TMU_PUD_ST_CTRL);
>>> +
>>> +	/*
>>> +	 * Set resolution mode
>>> +	 * 00b - Conversion time = 0.59325 ms
>>> +	 * 01b - Conversion time = 1.10525 ms
>>> +	 * 10b - Conversion time = 2.12925 ms
>>> +	 * 11b - Conversion time = 4.17725 ms
>>> +	 */
>>> +	writel_relaxed(FIELD_PREP(IMX91_TMU_CTRL1_RES_MASK, 0x3), tmu->base + IMX91_TMU_CTRL1_CLR);
>>> +	writel_relaxed(FIELD_PREP(IMX91_TMU_CTRL1_RES_MASK, 0x1), tmu->base + IMX91_TMU_CTRL1_SET);
>>> +
>>> +	writel_relaxed(IMX91_TMU_CTRL1_MEAS_MODE_MASK, tmu->base + IMX91_TMU_CTRL1_CLR);
>>> +	writel_relaxed(FIELD_PREP(IMX91_TMU_CTRL1_MEAS_MODE_MASK, IMX91_TMU_CTRL1_MEAS_MODE_SINGLE),
>>> +		       tmu->base + IMX91_TMU_CTRL1_SET);
>>> +
>>> +	pm_runtime_set_active(dev);
>>> +	devm_pm_runtime_enable(dev);
>>> +	pm_runtime_put(dev);
>>> +
>>> +	tmu->tzd = devm_thermal_of_zone_register(dev, 0, tmu, &tmu_tz_ops);
>>> +	if (IS_ERR(tmu->tzd))
>>> +		return dev_err_probe(dev, PTR_ERR(tmu->tzd),
>>> +				     "failed to register thermal zone sensor\n");
>>> +
>>> +	ret = devm_add_action(dev, imx91_tmu_action_remove, tmu);
>>
>> Should it be moved before devm_thermal_of_zone_register(), so if the thermal
>> zone creation fails, it will stop the sensor which was previously started ?
> 
> pm_runtime_put already disable sensor before call
> devm_thermal_of_zone_register().
> 
> imx91_tmu_get_temp() will call runtime_get when devm_thermal_of_zone_register()
> call to it.
> 
> Frank
> 
>>
>>> +	if (ret)
>>> +		return dev_err_probe(dev, ret, "Failure to add action imx91_tmu_action_remove()\n");
>>> +
>>> +	return 0;
>>> +}
>>> +
>>> +static int imx91_tmu_runtime_suspend(struct device *dev)
>>> +{
>>> +	struct imx91_tmu *tmu = dev_get_drvdata(dev);
>>> +
>>> +	/* disable tmu */
>>> +	imx91_tmu_enable(tmu, false);
>>> +
>>> +	clk_disable_unprepare(tmu->clk);
>>> +
>>> +	return 0;
>>> +}
>>> +
>>> +static int imx91_tmu_runtime_resume(struct device *dev)
>>> +{
>>> +	struct imx91_tmu *tmu = dev_get_drvdata(dev);
>>> +	int ret;
>>> +
>>> +	ret = clk_prepare_enable(tmu->clk);
>>> +	if (ret)
>>> +		return ret;
>>> +
>>> +	imx91_tmu_enable(tmu, true);
>>> +
>>> +	return 0;
>>> +}
>>> +
>>> +static DEFINE_RUNTIME_DEV_PM_OPS(imx91_tmu_pm_ops, imx91_tmu_runtime_suspend,
>>> +				 imx91_tmu_runtime_resume, NULL);
>>> +
>>> +static const struct of_device_id imx91_tmu_table[] = {
>>> +	{ .compatible = "fsl,imx91-tmu", },
>>> +	{ },
>>> +};
>>> +MODULE_DEVICE_TABLE(of, imx91_tmu_table);
>>> +
>>> +static struct platform_driver imx91_tmu = {
>>> +	.driver = {
>>> +		.name	= "imx91_thermal",
>>> +		.pm	= pm_ptr(&imx91_tmu_pm_ops),
>>> +		.of_match_table = imx91_tmu_table,
>>> +	},
>>> +	.probe = imx91_tmu_probe,
>>> +};
>>> +module_platform_driver(imx91_tmu);
>>> +
>>> +MODULE_AUTHOR("Peng Fan <peng.fan@nxp.com>");
>>> +MODULE_DESCRIPTION("i.MX91 Thermal Monitor Unit driver");
>>> +MODULE_LICENSE("GPL");
>>>
>>
>>
>> --
>> <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


-- 
<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 v4 0/2] thermal: imx91: Add support for i.MX91 thermal monitoring unit

[Frank Li]

On Mon, Dec 16, 2024 at 02:25:13PM -0500, Frank Li wrote:
> - Add binding doc
> - Add imx91 thermal driver
> - dts part wait for https://lore.kernel.org/imx/Z04W5chGq5TitB9f@lizhi-Precision-Tower-5810/
> merged, so not in this serial.
>
> Signed-off-by: Frank Li <Frank.Li@nxp.com>

Rafael J. Wysocki and Daniel Lezcano

	Do you have chance to check this patches?

Frank

> ---
> Changes in v4:
> - Add Kryz's review tag for binding
> - Add Marco's review tag for driver
> - Use devm_add_action()
> - Move pm_runtim_put before thermal_of_zone_register()
> - Link to v3: https://lore.kernel.org/r/20241212-imx91tmu-v3-0-85e756b29437@nxp.com
>
> Changes in v3:
> - add ref thermal-sensor
> - restrict #thermal-sensor-cells to 0 only
> - Change to unevaluatedProperties
>
> - add IMX91_TMU_ prefix for register define
> - remove unused register define
> - fix missed pm_runtime_put() at error path in imx91_tmu_get_temp()
> - use dev variable in probe function
> - use pm_runtime_set_active() in probe
> - move START to imx91_tmu_get_temp()
> - use DEFINE_RUNTIME_DEV_PM_OPS()
> - keep set reset value because there are not sw "reset" bit in controller,
>   uboot may change and enable tmu.
>
> - Link to v2: https://lore.kernel.org/r/20241210-imx91tmu-v2-0-5032aad4d88e@nxp.com
>
> Changes in v2:
> - use low case for hexvalue
> - combine struct imx91_tmu and tmu_sensor
> - simplify imx91_tmu_start() and imx91_tmu_enable()
> - use s16 for imx91_tmu_get_temp(), which may negative value
> - use reverse christmas tree style
> - use run time pm
> - use oneshot to sample temp
> - register thermal zone after hardware init
> - Link to v1: https://lore.kernel.org/r/20241209-imx91tmu-v1-0-7859c5387f31@nxp.com
>
> ---
> Pengfei Li (2):
>       dt-bindings: thermal: fsl,imx91-tmu: add bindings for NXP i.MX91 thermal module
>       thermal: imx91: Add support for i.MX91 thermal monitoring unit
>
>  .../devicetree/bindings/thermal/fsl,imx91-tmu.yaml |  67 ++++++
>  drivers/thermal/Kconfig                            |  10 +
>  drivers/thermal/Makefile                           |   1 +
>  drivers/thermal/imx91_thermal.c                    | 263 +++++++++++++++++++++
>  4 files changed, 341 insertions(+)
> ---
> base-commit: d07c576946b2bc440d6d2073998023e8a0bd7568
> change-id: 20241209-imx91tmu-af2a7c042d8d
>
> Best regards,
> ---
> Frank Li <Frank.Li@nxp.com>
>