[PATCH v6 0/5] thermal: renesas: Add support for RZ/G3E

[PATCH v6 0/5] thermal: renesas: Add support for RZ/G3E

[John Madieu]

This series adds support for the temperature sensor unit (TSU) found on the
Renesas RZ/G3E SoC.

The series consists of 5 patches (one of which is not related to the thermal
framework) that progressively add TSU support as follows:
- patch 1/5:    adds syscon/regmap support for accessing system controller
                registers, enabling access to TSU calibration values

- patch 2-5/5:  adds dt-bindings, actual driver, DT node, and config symbol.

Patch 1/5 has been duplicated at [1] in USB series. Since it was not reviewed
nor merged yet, I use it here to ease the review, so that which ever is
reviewed first get merged.

Changes:

v1 -> v2
 * Fix yaml warnings from dt-binding
 * Update IRQ names to reflect TSU expectations

v2 -> v3
 * Remove useless 'renesas,tsu-operating-mode' property

v3 -> v4
 * Improve commit messages

v4 -> v5
 * Remove useless curly braces on single line-protected scoped guards

v5 -> v6
 * Minor typo fix
 * Constify regmap config in patch 1/5

Regards,

[1] https://patchwork.kernel.org/project/linux-renesas-soc/patch/20250521140943.3830195-2-claudiu.beznea.uj@bp.renesas.com/

John Madieu (5):
  soc: renesas: rz-sysc: Add syscon/regmap support
  dt-bindings: thermal: r9a09g047-tsu: Document the TSU unit
  thermal: renesas: rzg3e: Add thermal driver for the Renesas RZ/G3E SoC
  arm64: dts: renesas: r9a09g047: Add TSU node
  arm64: defconfig: Enable the Renesas RZ/G3E thermal driver

 .../thermal/renesas,r9a09g047-tsu.yaml        |  81 ++++
 MAINTAINERS                                   |   7 +
 arch/arm64/boot/dts/renesas/r9a09g047.dtsi    |  48 ++
 arch/arm64/configs/defconfig                  |   1 +
 drivers/soc/renesas/Kconfig                   |   1 +
 drivers/soc/renesas/r9a08g045-sysc.c          |  10 +
 drivers/soc/renesas/r9a09g047-sys.c           |  10 +
 drivers/soc/renesas/r9a09g057-sys.c           |  10 +
 drivers/soc/renesas/rz-sysc.c                 |  17 +-
 drivers/soc/renesas/rz-sysc.h                 |   3 +
 drivers/thermal/renesas/Kconfig               |   7 +
 drivers/thermal/renesas/Makefile              |   1 +
 drivers/thermal/renesas/rzg3e_thermal.c       | 443 ++++++++++++++++++
 13 files changed, 638 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/thermal/renesas,r9a09g047-tsu.yaml
 create mode 100644 drivers/thermal/renesas/rzg3e_thermal.c

-- 
2.25.1

[PATCH v6 1/5] soc: renesas: rz-sysc: Add syscon/regmap support

[John Madieu]

The RZ/G3E system controller has various registers that control or report
some properties specific to individual IPs. The regmap is registered as a
syscon device to allow these IP drivers to access the registers through the
regmap API.

As other RZ SoCs might have custom read/write callbacks or max-offsets,
add register a custom regmap configuration.

Signed-off-by: John Madieu <john.madieu.xa@bp.renesas.com>
[claudiu.beznea:
 - s/rzg3e_sysc_regmap/rzv2h_sysc_regmap in RZ/V2H sysc
   file
 - do not check the match->data validity in rz_sysc_probe() as it is
   always valid
 - register the regmap if data->regmap_cfg is valid]
Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
---

Changes:

v1 -> v2: no changes
v2 -> v3: no changes
v3 -> v4: no changes
v4 -> v5: no changes
v6: Addressed the review comments received at [1];

[1] https://lore.kernel.org/all/20250330214945.185725-2-john.madieu.xa@bp.renesas.com/

 drivers/soc/renesas/Kconfig          |  1 +
 drivers/soc/renesas/r9a08g045-sysc.c | 10 ++++++++++
 drivers/soc/renesas/r9a09g047-sys.c  | 10 ++++++++++
 drivers/soc/renesas/r9a09g057-sys.c  | 10 ++++++++++
 drivers/soc/renesas/rz-sysc.c        | 17 ++++++++++++++++-
 drivers/soc/renesas/rz-sysc.h        |  3 +++
 6 files changed, 50 insertions(+), 1 deletion(-)

diff --git a/drivers/soc/renesas/Kconfig b/drivers/soc/renesas/Kconfig
index fbc3b69d21a7..f3b7546092d6 100644
--- a/drivers/soc/renesas/Kconfig
+++ b/drivers/soc/renesas/Kconfig
@@ -437,6 +437,7 @@ config RST_RCAR
 
 config SYSC_RZ
 	bool "System controller for RZ SoCs" if COMPILE_TEST
+	select MFD_SYSCON
 
 config SYSC_R9A08G045
 	bool "Renesas RZ/G3S System controller support" if COMPILE_TEST
diff --git a/drivers/soc/renesas/r9a08g045-sysc.c b/drivers/soc/renesas/r9a08g045-sysc.c
index f4db1431e036..0ef6df77e25f 100644
--- a/drivers/soc/renesas/r9a08g045-sysc.c
+++ b/drivers/soc/renesas/r9a08g045-sysc.c
@@ -18,6 +18,16 @@ static const struct rz_sysc_soc_id_init_data rzg3s_sysc_soc_id_init_data __initc
 	.specific_id_mask = GENMASK(27, 0),
 };
 
+static const struct regmap_config rzg3s_sysc_regmap __initconst = {
+	.name = "rzg3s_sysc_regs",
+	.reg_bits = 32,
+	.reg_stride = 4,
+	.val_bits = 32,
+	.fast_io = true,
+	.max_register = 0xe20,
+};
+
 const struct rz_sysc_init_data rzg3s_sysc_init_data __initconst = {
 	.soc_id_init_data = &rzg3s_sysc_soc_id_init_data,
+	.regmap_cfg = &rzg3s_sysc_regmap,
 };
diff --git a/drivers/soc/renesas/r9a09g047-sys.c b/drivers/soc/renesas/r9a09g047-sys.c
index cd2eb7782cfe..a3acf6dd2867 100644
--- a/drivers/soc/renesas/r9a09g047-sys.c
+++ b/drivers/soc/renesas/r9a09g047-sys.c
@@ -62,6 +62,16 @@ static const struct rz_sysc_soc_id_init_data rzg3e_sys_soc_id_init_data __initco
 	.print_id = rzg3e_sys_print_id,
 };
 
+static const struct regmap_config rzg3e_sysc_regmap __initconst = {
+	.name = "rzg3e_sysc_regs",
+	.reg_bits = 32,
+	.reg_stride = 4,
+	.val_bits = 32,
+	.fast_io = true,
+	.max_register = 0x170c,
+};
+
 const struct rz_sysc_init_data rzg3e_sys_init_data = {
 	.soc_id_init_data = &rzg3e_sys_soc_id_init_data,
+	.regmap_cfg = &rzg3e_sysc_regmap,
 };
diff --git a/drivers/soc/renesas/r9a09g057-sys.c b/drivers/soc/renesas/r9a09g057-sys.c
index 4c21cc29edbc..c26821636dce 100644
--- a/drivers/soc/renesas/r9a09g057-sys.c
+++ b/drivers/soc/renesas/r9a09g057-sys.c
@@ -62,6 +62,16 @@ static const struct rz_sysc_soc_id_init_data rzv2h_sys_soc_id_init_data __initco
 	.print_id = rzv2h_sys_print_id,
 };
 
+static const struct regmap_config rzv2h_sysc_regmap __initconst = {
+	.name = "rzv2h_sysc_regs",
+	.reg_bits = 32,
+	.reg_stride = 4,
+	.val_bits = 32,
+	.fast_io = true,
+	.max_register = 0x170c,
+};
+
 const struct rz_sysc_init_data rzv2h_sys_init_data = {
 	.soc_id_init_data = &rzv2h_sys_soc_id_init_data,
+	.regmap_cfg = &rzv2h_sysc_regmap,
 };
diff --git a/drivers/soc/renesas/rz-sysc.c b/drivers/soc/renesas/rz-sysc.c
index ffa65fb4dade..70556a2f55e6 100644
--- a/drivers/soc/renesas/rz-sysc.c
+++ b/drivers/soc/renesas/rz-sysc.c
@@ -6,8 +6,10 @@
  */
 
 #include <linux/io.h>
+#include <linux/mfd/syscon.h>
 #include <linux/of.h>
 #include <linux/platform_device.h>
+#include <linux/regmap.h>
 #include <linux/sys_soc.h>
 
 #include "rz-sysc.h"
@@ -100,14 +102,19 @@ MODULE_DEVICE_TABLE(of, rz_sysc_match);
 
 static int rz_sysc_probe(struct platform_device *pdev)
 {
+	const struct rz_sysc_init_data *data;
 	const struct of_device_id *match;
 	struct device *dev = &pdev->dev;
+	struct regmap *regmap;
 	struct rz_sysc *sysc;
+	int ret;
 
 	match = of_match_node(rz_sysc_match, dev->of_node);
 	if (!match)
 		return -ENODEV;
 
+	data = match->data;
+
 	sysc = devm_kzalloc(dev, sizeof(*sysc), GFP_KERNEL);
 	if (!sysc)
 		return -ENOMEM;
@@ -117,7 +124,15 @@ static int rz_sysc_probe(struct platform_device *pdev)
 		return PTR_ERR(sysc->base);
 
 	sysc->dev = dev;
-	return rz_sysc_soc_init(sysc, match);
+	ret = rz_sysc_soc_init(sysc, match);
+	if (ret || !data->regmap_cfg)
+		return ret;
+
+	regmap = devm_regmap_init_mmio(dev, sysc->base, data->regmap_cfg);
+	if (IS_ERR(regmap))
+		return PTR_ERR(regmap);
+
+	return of_syscon_register_regmap(dev->of_node, regmap);
 }
 
 static struct platform_driver rz_sysc_driver = {
diff --git a/drivers/soc/renesas/rz-sysc.h b/drivers/soc/renesas/rz-sysc.h
index 56bc047a1bff..447008140634 100644
--- a/drivers/soc/renesas/rz-sysc.h
+++ b/drivers/soc/renesas/rz-sysc.h
@@ -9,6 +9,7 @@
 #define __SOC_RENESAS_RZ_SYSC_H__
 
 #include <linux/device.h>
+#include <linux/regmap.h>
 #include <linux/sys_soc.h>
 #include <linux/types.h>
 
@@ -34,9 +35,11 @@ struct rz_sysc_soc_id_init_data {
 /**
  * struct rz_sysc_init_data - RZ SYSC initialization data
  * @soc_id_init_data: RZ SYSC SoC ID initialization data
+ * @regmap_cfg: SoC-specific regmap config
  */
 struct rz_sysc_init_data {
 	const struct rz_sysc_soc_id_init_data *soc_id_init_data;
+	const struct regmap_config *regmap_cfg;
 };
 
 extern const struct rz_sysc_init_data rzg3e_sys_init_data;
-- 
2.25.1

[PATCH v6 2/5] dt-bindings: thermal: r9a09g047-tsu: Document the TSU unit

[John Madieu]

The Renesas RZ/G3E SoC includes a Thermal Sensor Unit (TSU) block designed
to measure the junction temperature. The device provides real-time
temperature measurements for thermal management, utilizing a single
dedicated channel (channel 1) for temperature sensing.

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Signed-off-by: John Madieu <john.madieu.xa@bp.renesas.com>
---

Changes:

v1 -> v2:
 * Fixes reg property specifier to get rid of yamlint warnings
 * Fixes IRQ name to reflect TSU expectations

v2 -> v3:
 * Removees useless 'renesas,tsu-operating-mode' property 

v3 -> v4:
 * Fixes commit message
 * Fixes interrupt description
 * Removes trip point definition

v5: no changes
v6: no changes

 .../thermal/renesas,r9a09g047-tsu.yaml        | 81 +++++++++++++++++++
 1 file changed, 81 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/thermal/renesas,r9a09g047-tsu.yaml

diff --git a/Documentation/devicetree/bindings/thermal/renesas,r9a09g047-tsu.yaml b/Documentation/devicetree/bindings/thermal/renesas,r9a09g047-tsu.yaml
new file mode 100644
index 000000000000..ef9308089bfc
--- /dev/null
+++ b/Documentation/devicetree/bindings/thermal/renesas,r9a09g047-tsu.yaml
@@ -0,0 +1,81 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/thermal/renesas,r9a09g047-tsu.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Renesas RZ/G3E Temperature Sensor Unit (TSU)
+
+maintainers:
+  - John Madieu <john.madieu.xa@bp.renesas.com>
+
+description:
+  The Temperature Sensor Unit (TSU) is an integrated thermal sensor that
+  monitors the chip temperature on the Renesas RZ/G3E SoC. The TSU provides
+  real-time temperature measurements for thermal management.
+
+properties:
+  compatible:
+    const: renesas,r9a09g047-tsu
+
+  reg:
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
+
+  resets:
+    maxItems: 1
+
+  power-domains:
+    maxItems: 1
+
+  interrupts:
+    items:
+      - description: Conversion complete interrupt signal (pulse)
+      - description: Comparison result interrupt signal (level)
+
+  interrupt-names:
+    items:
+      - const: adi
+      - const: adcmpi
+
+  "#thermal-sensor-cells":
+    const: 0
+
+  renesas,tsu-calibration-sys:
+    $ref: /schemas/types.yaml#/definitions/phandle
+    description: |
+      Phandle to the system controller (sys) that contains the TSU
+      calibration values used for temperature calculations.
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - resets
+  - power-domains
+  - interrupts
+  - interrupt-names
+  - "#thermal-sensor-cells"
+  - renesas,tsu-calibration-sys
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/renesas,r9a09g047-cpg.h>
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+
+    tsu: thermal@14002000 {
+        compatible = "renesas,r9a09g047-tsu";
+        reg = <0x14002000 0x1000>;
+        clocks = <&cpg CPG_MOD 0x10a>;
+        resets = <&cpg 0xf8>;
+        power-domains = <&cpg>;
+        interrupts = <GIC_SPI 250 IRQ_TYPE_EDGE_RISING>,
+                     <GIC_SPI 251 IRQ_TYPE_LEVEL_HIGH>;
+        interrupt-names = "adi", "adcmpi";
+        #thermal-sensor-cells = <0>;
+        renesas,tsu-calibration-sys = <&sys>;
+    };
-- 
2.25.1

[PATCH v6 3/5] thermal: renesas: rzg3e: Add thermal driver for the Renesas RZ/G3E SoC

[John Madieu]

The RZ/G3E SoC integrates a Temperature Sensor Unit (TSU) block designed
to monitor the chip's junction temperature. This sensor is connected to
channel 1 of the APB port clock/reset and provides temperature measurements.

It also requires calibration values stored in the system controller registers
for accurate temperature measurement. Add a driver for the Renesas RZ/G3E TSU.

Signed-off-by: John Madieu <john.madieu.xa@bp.renesas.com>
---

Changes:

v1 -> v2: fixes IRQ names
v2 -> v3: no changes
v3 -> v4: no changes
v5: removes curly braces arround single-line protected scoped guards
v6: Clarified comments in driver

 MAINTAINERS                             |   7 +
 drivers/thermal/renesas/Kconfig         |   7 +
 drivers/thermal/renesas/Makefile        |   1 +
 drivers/thermal/renesas/rzg3e_thermal.c | 443 ++++++++++++++++++++++++
 4 files changed, 458 insertions(+)
 create mode 100644 drivers/thermal/renesas/rzg3e_thermal.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 79a8e2c73908..eb11494795e8 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -21161,6 +21161,13 @@ S:	Maintained
 F:	Documentation/devicetree/bindings/iio/potentiometer/renesas,x9250.yaml
 F:	drivers/iio/potentiometer/x9250.c
 
+RENESAS RZ/G3E THERMAL SENSOR UNIT DRIVER
+M:	John Madieu <john.madieu.xa@bp.renesas.com>
+L:	linux-pm@vger.kernel.org
+S:	Maintained
+F:	Documentation/devicetree/bindings/thermal/renesas,r9a09g047-tsu.yaml
+F:	drivers/thermal/renesas/rzg3e_thermal.c
+
 RESET CONTROLLER FRAMEWORK
 M:	Philipp Zabel <p.zabel@pengutronix.de>
 S:	Maintained
diff --git a/drivers/thermal/renesas/Kconfig b/drivers/thermal/renesas/Kconfig
index dcf5fc5ae08e..10cf90fc4bfa 100644
--- a/drivers/thermal/renesas/Kconfig
+++ b/drivers/thermal/renesas/Kconfig
@@ -26,3 +26,10 @@ config RZG2L_THERMAL
 	help
 	  Enable this to plug the RZ/G2L thermal sensor driver into the Linux
 	  thermal framework.
+
+config RZG3E_THERMAL
+	tristate "Renesas RZ/G3E thermal driver"
+	depends on ARCH_RENESAS || COMPILE_TEST
+	help
+	  Enable this to plug the RZ/G3E thermal sensor driver into the Linux
+	  thermal framework.
diff --git a/drivers/thermal/renesas/Makefile b/drivers/thermal/renesas/Makefile
index bf9cb3cb94d6..5a3eba0dedd0 100644
--- a/drivers/thermal/renesas/Makefile
+++ b/drivers/thermal/renesas/Makefile
@@ -3,3 +3,4 @@
 obj-$(CONFIG_RCAR_GEN3_THERMAL)	+= rcar_gen3_thermal.o
 obj-$(CONFIG_RCAR_THERMAL)	+= rcar_thermal.o
 obj-$(CONFIG_RZG2L_THERMAL)	+= rzg2l_thermal.o
+obj-$(CONFIG_RZG3E_THERMAL)	+= rzg3e_thermal.o
diff --git a/drivers/thermal/renesas/rzg3e_thermal.c b/drivers/thermal/renesas/rzg3e_thermal.c
new file mode 100644
index 000000000000..348229da9ef4
--- /dev/null
+++ b/drivers/thermal/renesas/rzg3e_thermal.c
@@ -0,0 +1,443 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Renesas RZ/G3E TSU Temperature Sensor Unit
+ *
+ * Copyright (C) 2025 Renesas Electronics Corporation
+ */
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/iopoll.h>
+#include <linux/kernel.h>
+#include <linux/mfd/syscon.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/reset.h>
+#include <linux/thermal.h>
+#include <linux/units.h>
+
+#include "../thermal_hwmon.h"
+
+/* SYS Trimming register offsets macro */
+#define SYS_TSU_TRMVAL(x) (0x330 + (x) * 4)
+
+/* TSU Register offsets and bits */
+#define TSU_SSUSR		0x00
+#define TSU_SSUSR_EN_TS		BIT(0)
+#define TSU_SSUSR_ADC_PD_TS	BIT(1)
+#define TSU_SSUSR_SOC_TS_EN	BIT(2)
+
+#define TSU_STRGR		0x04
+#define TSU_STRGR_ADST		BIT(0)
+
+#define TSU_SOSR1		0x08
+#define TSU_SOSR1_ADCT_8	0x03
+#define TSU_SOSR1_OUTSEL_AVERAGE	BIT(9)
+
+/* Sensor Code Read Register */
+#define TSU_SCRR		0x10
+#define TSU_SCRR_OUT12BIT_TS	GENMASK(11, 0)
+
+/* Sensor Status Register */
+#define TSU_SSR			0x14
+#define TSU_SSR_CONV_RUNNING	BIT(0)
+
+/* Compare Mode Setting Register */
+#define TSU_CMSR		0x18
+#define TSU_CMSR_CMPEN		BIT(0)
+#define TSU_CMSR_CMPCOND	BIT(1)
+
+/* Lower Limit Setting Register */
+#define TSU_LLSR		0x1C
+#define TSU_LLSR_LIM		GENMASK(11, 0)
+
+/* Upper Limit Setting Register */
+#define TSU_ULSR		0x20
+#define TSU_ULSR_ULIM		GENMASK(11, 0)
+
+/* Interrupt Status Register */
+#define TSU_SISR		0x30
+#define TSU_SISR_ADF		BIT(0)
+#define TSU_SISR_CMPF		BIT(1)
+
+/* Interrupt Enable Register */
+#define TSU_SIER		0x34
+#define TSU_SIER_ADIE		BIT(0)
+#define TSU_SIER_CMPIE		BIT(1)
+
+/* Interrupt Clear Register */
+#define TSU_SICR		0x38
+#define TSU_SICR_ADCLR		BIT(0)
+#define TSU_SICR_CMPCLR		BIT(1)
+
+/* Temperature calculation constants */
+#define TSU_D		41
+#define TSU_E		126
+#define TSU_TRMVAL_MASK	GENMASK(11, 0)
+
+#define TSU_POLL_DELAY_US	50
+#define TSU_TIMEOUT_US		10000
+#define TSU_MIN_CLOCK_RATE	24000000
+
+/**
+ * struct rzg3e_thermal_priv - RZ/G3E thermal private data structure
+ * @base: TSU base address
+ * @dev: device pointer
+ * @syscon: regmap for calibration values
+ * @zone: thermal zone pointer
+ * @mode: current tzd mode
+ * @conv_complete: ADC conversion completion
+ * @reg_lock: protect shared register access
+ * @cached_temp: last computed temperature (milliCelsius)
+ * @trmval: trim (calibration) values
+ */
+struct rzg3e_thermal_priv {
+	void __iomem *base;
+	struct device *dev;
+	struct regmap *syscon;
+	struct thermal_zone_device *zone;
+	enum thermal_device_mode mode;
+	struct completion conv_complete;
+	spinlock_t reg_lock;
+	int cached_temp;
+	u32 trmval[2];
+};
+
+static void rzg3e_thermal_hw_disable(struct rzg3e_thermal_priv *priv)
+{
+	/* Disable all interrupts first */
+	writel(0, priv->base + TSU_SIER);
+	/* Clear any pending interrupts */
+	writel(TSU_SICR_ADCLR | TSU_SICR_CMPCLR, priv->base + TSU_SICR);
+	/* Put device in power down */
+	writel(TSU_SSUSR_ADC_PD_TS, priv->base + TSU_SSUSR);
+}
+
+static void rzg3e_thermal_hw_enable(struct rzg3e_thermal_priv *priv)
+{
+	/* First clear any pending status */
+	writel(TSU_SICR_ADCLR | TSU_SICR_CMPCLR, priv->base + TSU_SICR);
+	/* Disable all interrupts */
+	writel(0, priv->base + TSU_SIER);
+
+	/* Enable thermal sensor */
+	writel(TSU_SSUSR_SOC_TS_EN | TSU_SSUSR_EN_TS, priv->base + TSU_SSUSR);
+	/* Setup for averaging mode with 8 samples */
+	writel(TSU_SOSR1_OUTSEL_AVERAGE | TSU_SOSR1_ADCT_8, priv->base + TSU_SOSR1);
+}
+
+static irqreturn_t rzg3e_thermal_cmp_irq(int irq, void *dev_id)
+{
+	struct rzg3e_thermal_priv *priv = dev_id;
+	u32 status;
+
+	status = readl(priv->base + TSU_SISR);
+	if (!(status & TSU_SISR_CMPF))
+		return IRQ_NONE;
+
+	/* Clear the comparison interrupt flag */
+	writel(TSU_SICR_CMPCLR, priv->base + TSU_SICR);
+
+	return IRQ_WAKE_THREAD;
+}
+
+static irqreturn_t rzg3e_thermal_cmp_threaded_irq(int irq, void *dev_id)
+{
+	struct rzg3e_thermal_priv *priv = dev_id;
+
+	thermal_zone_device_update(priv->zone, THERMAL_EVENT_UNSPECIFIED);
+	return IRQ_HANDLED;
+}
+
+static irqreturn_t rzg3e_thermal_adc_irq(int irq, void *dev_id)
+{
+	struct rzg3e_thermal_priv *priv = dev_id;
+	u32 status;
+	u32 result;
+
+	/* Check if this is our interrupt */
+	status = readl(priv->base + TSU_SISR);
+	if (!(status & TSU_SISR_ADF))
+		return IRQ_NONE;
+
+	/* Disable all interrupts */
+	writel(0, priv->base + TSU_SIER);
+	/* Clear conversion complete interrupt */
+	writel(TSU_SICR_ADCLR, priv->base + TSU_SICR);
+
+	/* Read ADC conversion result */
+	result = readl(priv->base + TSU_SCRR) & TSU_SCRR_OUT12BIT_TS;
+
+	/*
+	 * Calculate temperature using compensation formula
+	 * Section 7.11.7.8 (Temperature Compensation Calculation)
+	 *
+	 * T(°C) = ((e - d) / (c -b)) * (a - b) + d
+	 *
+	 * a = 12 bits temperature code read from the sensor
+	 * b = SYS trmval[0]
+	 * c = SYS trmval[1]
+	 * d = -41
+	 * e = 126
+	 */
+	s64 temp_val = div_s64(((TSU_E + TSU_D) * (s64)(result - priv->trmval[0])),
+				priv->trmval[1] - priv->trmval[0]) - TSU_D;
+	int new_temp = temp_val * MILLIDEGREE_PER_DEGREE;
+
+	scoped_guard(spinlock_irqsave, &priv->reg_lock)
+		priv->cached_temp = new_temp;
+
+	complete(&priv->conv_complete);
+
+	return IRQ_HANDLED;
+}
+
+static int rzg3e_thermal_get_temp(struct thermal_zone_device *zone, int *temp)
+{
+	struct rzg3e_thermal_priv *priv = thermal_zone_device_priv(zone);
+	u32 val;
+	int ret;
+
+	if (priv->mode == THERMAL_DEVICE_DISABLED)
+		return -EBUSY;
+
+	reinit_completion(&priv->conv_complete);
+
+	/* Enable ADC interrupt */
+	writel(TSU_SIER_ADIE, priv->base + TSU_SIER);
+
+	/* Verify no ongoing conversion */
+	ret = readl_poll_timeout_atomic(priv->base + TSU_SSR, val,
+					!(val & TSU_SSR_CONV_RUNNING),
+					TSU_POLL_DELAY_US, TSU_TIMEOUT_US);
+	if (ret) {
+		dev_err(priv->dev, "ADC conversion timed out\n");
+		return ret;
+	}
+
+	/* Start conversion */
+	writel(TSU_STRGR_ADST, priv->base + TSU_STRGR);
+
+	if (!wait_for_completion_timeout(&priv->conv_complete,
+					 msecs_to_jiffies(100))) {
+		dev_err(priv->dev, "ADC conversion completion timeout\n");
+		return -ETIMEDOUT;
+	}
+
+	scoped_guard(spinlock_irqsave, &priv->reg_lock)
+		*temp = priv->cached_temp;
+
+	return 0;
+}
+
+/* Convert temperature in milliCelsius to raw sensor code */
+static int rzg3e_temp_to_raw(struct rzg3e_thermal_priv *priv, int temp_mc)
+{
+	s64 raw = div_s64(((temp_mc / 1000) - TSU_D) *
+			  (priv->trmval[1] - priv->trmval[0]),
+			  (TSU_E - TSU_D));
+	return clamp_val(raw, 0, 0xFFF);
+}
+
+static int rzg3e_thermal_set_trips(struct thermal_zone_device *tz, int low, int high)
+{
+	struct rzg3e_thermal_priv *priv = thermal_zone_device_priv(tz);
+	int ret;
+	int val;
+
+	if (low >= high)
+		return -EINVAL;
+
+	if (priv->mode == THERMAL_DEVICE_DISABLED)
+		return -EBUSY;
+
+	/* Set up comparison interrupt */
+	writel(0, priv->base + TSU_SIER);
+	writel(TSU_SICR_ADCLR | TSU_SICR_CMPCLR, priv->base + TSU_SICR);
+
+	/* Set thresholds */
+	writel(rzg3e_temp_to_raw(priv, low), priv->base + TSU_LLSR);
+	writel(rzg3e_temp_to_raw(priv, high), priv->base + TSU_ULSR);
+
+	/* Configure comparison:
+	 * - Enable comparison function (CMPEN = 1)
+	 * - Set comparison condition (CMPCOND = 0 for out of range)
+	 */
+	writel(TSU_CMSR_CMPEN, priv->base + TSU_CMSR);
+
+	/* Enable comparison irq */
+	writel(TSU_SIER_CMPIE, priv->base + TSU_SIER);
+
+	/* Verify no ongoing conversion */
+	ret = readl_poll_timeout_atomic(priv->base + TSU_SSR, val,
+					!(val & TSU_SSR_CONV_RUNNING),
+					TSU_POLL_DELAY_US, TSU_TIMEOUT_US);
+	if (ret) {
+		dev_err(priv->dev, "ADC conversion timed out\n");
+		return ret;
+	}
+
+	/* Start a conversion to trigger comparison */
+	writel(TSU_STRGR_ADST, priv->base + TSU_STRGR);
+
+	return 0;
+}
+
+static int rzg3e_thermal_get_trimming(struct rzg3e_thermal_priv *priv)
+{
+	int ret;
+
+	ret = regmap_read(priv->syscon, SYS_TSU_TRMVAL(0), &priv->trmval[0]);
+	if (ret)
+		return ret;
+
+	ret = regmap_read(priv->syscon, SYS_TSU_TRMVAL(1), &priv->trmval[1]);
+	if (ret)
+		return ret;
+
+	priv->trmval[0] &= TSU_TRMVAL_MASK;
+	priv->trmval[1] &= TSU_TRMVAL_MASK;
+
+	if (!priv->trmval[0] || !priv->trmval[1])
+		return dev_err_probe(priv->dev, -EINVAL, "invalid trimming values");
+
+	return 0;
+}
+
+static int rzg3e_thermal_change_mode(struct thermal_zone_device *tz,
+				     enum thermal_device_mode mode)
+{
+	struct rzg3e_thermal_priv *priv = thermal_zone_device_priv(tz);
+
+	if (mode == THERMAL_DEVICE_DISABLED)
+		rzg3e_thermal_hw_disable(priv);
+	else
+		rzg3e_thermal_hw_enable(priv);
+
+	priv->mode = mode;
+	return 0;
+}
+
+static const struct thermal_zone_device_ops rzg3e_tz_of_ops = {
+	.get_temp = rzg3e_thermal_get_temp,
+	.set_trips = rzg3e_thermal_set_trips,
+	.change_mode = rzg3e_thermal_change_mode,
+};
+
+static int rzg3e_thermal_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct rzg3e_thermal_priv *priv;
+	struct reset_control *rstc;
+	char *adc_name, *cmp_name;
+	int adc_irq, cmp_irq;
+	struct clk *clk;
+	int ret;
+
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	priv->dev = dev;
+
+	priv->base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(priv->base))
+		return dev_err_probe(dev, PTR_ERR(priv->base),
+				"Failed to map I/O memory");
+
+	priv->syscon = syscon_regmap_lookup_by_phandle(dev->of_node,
+						       "renesas,tsu-calibration-sys");
+	if (IS_ERR(priv->syscon))
+		return dev_err_probe(dev, PTR_ERR(priv->syscon),
+				"Failed to get calibration syscon");
+
+	adc_irq = platform_get_irq_byname(pdev, "adi");
+	if (adc_irq < 0)
+		return adc_irq;
+
+	cmp_irq = platform_get_irq_byname(pdev, "adcmpi");
+	if (cmp_irq < 0)
+		return cmp_irq;
+
+	rstc = devm_reset_control_get_exclusive_deasserted(dev, NULL);
+	if (IS_ERR(rstc))
+		return dev_err_probe(dev, PTR_ERR(rstc),
+				     "Failed to acquire deasserted reset");
+
+	platform_set_drvdata(pdev, priv);
+
+	spin_lock_init(&priv->reg_lock);
+	init_completion(&priv->conv_complete);
+
+	clk = devm_clk_get_enabled(dev, NULL);
+	if (IS_ERR(clk))
+		return dev_err_probe(dev, PTR_ERR(clk),
+				     "Failed to get and enable clock");
+
+	if (clk_get_rate(clk) < TSU_MIN_CLOCK_RATE)
+		return dev_err_probe(dev, -EINVAL,
+				     "Clock rate too low (minimum %d Hz required)",
+				     TSU_MIN_CLOCK_RATE);
+
+	ret = rzg3e_thermal_get_trimming(priv);
+	if (ret)
+		return ret;
+
+	adc_name = devm_kasprintf(dev, GFP_KERNEL, "%s-adc", dev_name(dev));
+	if (!adc_name)
+		return -ENOMEM;
+
+	cmp_name = devm_kasprintf(dev, GFP_KERNEL, "%s-cmp", dev_name(dev));
+	if (!cmp_name)
+		return -ENOMEM;
+
+	/* Unit in a known disabled mode */
+	rzg3e_thermal_hw_disable(priv);
+
+	ret = devm_request_irq(dev, adc_irq, rzg3e_thermal_adc_irq,
+			       IRQF_TRIGGER_RISING, adc_name, priv);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to request ADC IRQ");
+
+	ret = devm_request_threaded_irq(dev, cmp_irq, rzg3e_thermal_cmp_irq,
+					rzg3e_thermal_cmp_threaded_irq,
+					IRQF_TRIGGER_HIGH | IRQF_ONESHOT,
+					cmp_name, priv);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to request comparison IRQ");
+
+	/* Register Thermal Zone */
+	priv->zone = devm_thermal_of_zone_register(dev, 0, priv, &rzg3e_tz_of_ops);
+	if (IS_ERR(priv->zone))
+		return dev_err_probe(dev, PTR_ERR(priv->zone),
+				"Failed to register thermal zone");
+
+	ret = devm_thermal_add_hwmon_sysfs(dev, priv->zone);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to add hwmon sysfs");
+
+	return 0;
+}
+
+static const struct of_device_id rzg3e_thermal_dt_ids[] = {
+	{ .compatible = "renesas,r9a09g047-tsu" },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, rzg3e_thermal_dt_ids);
+
+static struct platform_driver rzg3e_thermal_driver = {
+	.driver = {
+		.name	= "rzg3e_thermal",
+		.of_match_table = rzg3e_thermal_dt_ids,
+	},
+	.probe = rzg3e_thermal_probe,
+};
+module_platform_driver(rzg3e_thermal_driver);
+
+MODULE_DESCRIPTION("Renesas RZ/G3E TSU Thermal Sensor Driver");
+MODULE_AUTHOR("John Madieu <john.madieu.xa@bp.renesas.com>");
+MODULE_LICENSE("GPL");
-- 
2.25.1

[PATCH v6 4/5] arm64: dts: renesas: r9a09g047: Add TSU node

[John Madieu]

Add TSU node along with thermal zones and keep it enabled in the SoC DTSI.

Signed-off-by: John Madieu <john.madieu.xa@bp.renesas.com>
---

Changes:

v1 -> v2: Fix IRQ names
v2 -> v3: remove useless 'renesas,tsu-operating-mode' property'
v3 -> v4: no changes
v5: no changes
v6: no changes

 arch/arm64/boot/dts/renesas/r9a09g047.dtsi | 48 ++++++++++++++++++++++
 1 file changed, 48 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/r9a09g047.dtsi b/arch/arm64/boot/dts/renesas/r9a09g047.dtsi
index 876f70fed433..535da0292d91 100644
--- a/arch/arm64/boot/dts/renesas/r9a09g047.dtsi
+++ b/arch/arm64/boot/dts/renesas/r9a09g047.dtsi
@@ -64,6 +64,7 @@ cpu0: cpu@0 {
 			next-level-cache = <&L3_CA55>;
 			enable-method = "psci";
 			clocks = <&cpg CPG_CORE R9A09G047_CA55_0_CORECLK0>;
+			#cooling-cells = <2>;
 			operating-points-v2 = <&cluster0_opp>;
 		};
 
@@ -74,6 +75,7 @@ cpu1: cpu@100 {
 			next-level-cache = <&L3_CA55>;
 			enable-method = "psci";
 			clocks = <&cpg CPG_CORE R9A09G047_CA55_0_CORECLK1>;
+			#cooling-cells = <2>;
 			operating-points-v2 = <&cluster0_opp>;
 		};
 
@@ -84,6 +86,7 @@ cpu2: cpu@200 {
 			next-level-cache = <&L3_CA55>;
 			enable-method = "psci";
 			clocks = <&cpg CPG_CORE R9A09G047_CA55_0_CORECLK2>;
+			#cooling-cells = <2>;
 			operating-points-v2 = <&cluster0_opp>;
 		};
 
@@ -94,6 +97,7 @@ cpu3: cpu@300 {
 			next-level-cache = <&L3_CA55>;
 			enable-method = "psci";
 			clocks = <&cpg CPG_CORE R9A09G047_CA55_0_CORECLK3>;
+			#cooling-cells = <2>;
 			operating-points-v2 = <&cluster0_opp>;
 		};
 
@@ -391,6 +395,19 @@ wdt3: watchdog@13000400 {
 			status = "disabled";
 		};
 
+		tsu: thermal@14002000 {
+			compatible = "renesas,r9a09g047-tsu";
+			reg = <0 0x14002000 0 0x1000>;
+			interrupts = <GIC_SPI 250 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 251 IRQ_TYPE_LEVEL_HIGH>;
+			interrupt-names = "adi", "adcmpi";
+			clocks = <&cpg CPG_MOD 0x10a>;
+			resets = <&cpg 0xf8>;
+			power-domains = <&cpg>;
+			#thermal-sensor-cells = <0>;
+			renesas,tsu-calibration-sys = <&sys>;
+		};
+
 		i2c0: i2c@14400400 {
 			compatible = "renesas,riic-r9a09g047", "renesas,riic-r9a09g057";
 			reg = <0 0x14400400 0 0x400>;
@@ -671,6 +688,37 @@ sdhi2_vqmmc: vqmmc-regulator {
 		};
 	};
 
+	thermal-zones {
+		cpu-thermal {
+			polling-delay = <1000>;
+			polling-delay-passive = <250>;
+			thermal-sensors = <&tsu>;
+
+			cooling-maps {
+				map0 {
+					trip = <&target>;
+					cooling-device = <&cpu0 0 3>, <&cpu1 0 3>,
+							 <&cpu2 0 3>, <&cpu3 0 3>;
+					contribution = <1024>;
+				};
+			};
+
+			trips {
+				target: trip-point {
+					temperature = <95000>;
+					hysteresis = <1000>;
+					type = "passive";
+				};
+
+				sensor_crit: sensor-crit {
+					temperature = <120000>;
+					hysteresis = <1000>;
+					type = "critical";
+				};
+			};
+		};
+	};
+
 	timer {
 		compatible = "arm,armv8-timer";
 		interrupts-extended = <&gic GIC_PPI 13 IRQ_TYPE_LEVEL_LOW>,
-- 
2.25.1

[PATCH v6 5/5] arm64: defconfig: Enable the Renesas RZ/G3E thermal driver

[John Madieu]

Enable the Renesas RZ/G3E thermal driver, as used on the Renesas
RZ/G3E SMARC EVK board.

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Signed-off-by: John Madieu <john.madieu.xa@bp.renesas.com>
---

Changes:

v1 -> v2: no changes
v2 -> v3: no changes
v3 -> v4: update commit message
v5: no changes
v6: no changes

 arch/arm64/configs/defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index 368c242fe945..1b9ceab54408 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -720,6 +720,7 @@ CONFIG_ROCKCHIP_THERMAL=m
 CONFIG_RCAR_THERMAL=y
 CONFIG_RCAR_GEN3_THERMAL=y
 CONFIG_RZG2L_THERMAL=y
+CONFIG_RZG3E_THERMAL=y
 CONFIG_ARMADA_THERMAL=y
 CONFIG_MTK_THERMAL=m
 CONFIG_MTK_LVTS_THERMAL=m
-- 
2.25.1

RE: [PATCH v6 3/5] thermal: renesas: rzg3e: Add thermal driver for the Renesas RZ/G3E SoC

[Biju Das]

Hi John,

> -----Original Message-----
> From: John Madieu <john.madieu.xa@bp.renesas.com>
> Sent: 22 May 2025 19:23
> Subject: [PATCH v6 3/5] thermal: renesas: rzg3e: Add thermal driver for the Renesas RZ/G3E SoC
> 
> The RZ/G3E SoC integrates a Temperature Sensor Unit (TSU) block designed to monitor the chip's
> junction temperature. This sensor is connected to channel 1 of the APB port clock/reset and provides
> temperature measurements.
> 
> It also requires calibration values stored in the system controller registers for accurate temperature
> measurement. Add a driver for the Renesas RZ/G3E TSU.
> 
> Signed-off-by: John Madieu <john.madieu.xa@bp.renesas.com>
> ---
> 
> Changes:
> 
> v1 -> v2: fixes IRQ names
> v2 -> v3: no changes
> v3 -> v4: no changes
> v5: removes curly braces arround single-line protected scoped guards
> v6: Clarified comments in driver
> 
>  MAINTAINERS                             |   7 +
>  drivers/thermal/renesas/Kconfig         |   7 +
>  drivers/thermal/renesas/Makefile        |   1 +
>  drivers/thermal/renesas/rzg3e_thermal.c | 443 ++++++++++++++++++++++++
>  4 files changed, 458 insertions(+)
>  create mode 100644 drivers/thermal/renesas/rzg3e_thermal.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 79a8e2c73908..eb11494795e8 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -21161,6 +21161,13 @@ S:	Maintained
>  F:	Documentation/devicetree/bindings/iio/potentiometer/renesas,x9250.yaml
>  F:	drivers/iio/potentiometer/x9250.c
> 
> +RENESAS RZ/G3E THERMAL SENSOR UNIT DRIVER
> +M:	John Madieu <john.madieu.xa@bp.renesas.com>
> +L:	linux-pm@vger.kernel.org
> +S:	Maintained
> +F:	Documentation/devicetree/bindings/thermal/renesas,r9a09g047-tsu.yaml
> +F:	drivers/thermal/renesas/rzg3e_thermal.c
> +
>  RESET CONTROLLER FRAMEWORK
>  M:	Philipp Zabel <p.zabel@pengutronix.de>
>  S:	Maintained
> diff --git a/drivers/thermal/renesas/Kconfig b/drivers/thermal/renesas/Kconfig index
> dcf5fc5ae08e..10cf90fc4bfa 100644
> --- a/drivers/thermal/renesas/Kconfig
> +++ b/drivers/thermal/renesas/Kconfig
> @@ -26,3 +26,10 @@ config RZG2L_THERMAL
>  	help
>  	  Enable this to plug the RZ/G2L thermal sensor driver into the Linux
>  	  thermal framework.
> +
> +config RZG3E_THERMAL
> +	tristate "Renesas RZ/G3E thermal driver"
> +	depends on ARCH_RENESAS || COMPILE_TEST
> +	help
> +	  Enable this to plug the RZ/G3E thermal sensor driver into the Linux
> +	  thermal framework.
> diff --git a/drivers/thermal/renesas/Makefile b/drivers/thermal/renesas/Makefile
> index bf9cb3cb94d6..5a3eba0dedd0 100644
> --- a/drivers/thermal/renesas/Makefile
> +++ b/drivers/thermal/renesas/Makefile
> @@ -3,3 +3,4 @@
>  obj-$(CONFIG_RCAR_GEN3_THERMAL)	+= rcar_gen3_thermal.o
>  obj-$(CONFIG_RCAR_THERMAL)	+= rcar_thermal.o
>  obj-$(CONFIG_RZG2L_THERMAL)	+= rzg2l_thermal.o
> +obj-$(CONFIG_RZG3E_THERMAL)	+= rzg3e_thermal.o
> diff --git a/drivers/thermal/renesas/rzg3e_thermal.c b/drivers/thermal/renesas/rzg3e_thermal.c
> new file mode 100644
> index 000000000000..348229da9ef4
> --- /dev/null
> +++ b/drivers/thermal/renesas/rzg3e_thermal.c
> @@ -0,0 +1,443 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Renesas RZ/G3E TSU Temperature Sensor Unit
> + *
> + * Copyright (C) 2025 Renesas Electronics Corporation  */ #include
> +<linux/clk.h> #include <linux/delay.h> #include <linux/err.h> #include
> +<linux/interrupt.h> #include <linux/io.h> #include <linux/iopoll.h>
> +#include <linux/kernel.h> #include <linux/mfd/syscon.h> #include
> +<linux/module.h> #include <linux/of_device.h> #include
> +<linux/platform_device.h> #include <linux/regmap.h> #include
> +<linux/reset.h> #include <linux/thermal.h> #include <linux/units.h>
> +
> +#include "../thermal_hwmon.h"
> +
> +/* SYS Trimming register offsets macro */ #define SYS_TSU_TRMVAL(x)
> +(0x330 + (x) * 4)
> +
> +/* TSU Register offsets and bits */
> +#define TSU_SSUSR		0x00
> +#define TSU_SSUSR_EN_TS		BIT(0)
> +#define TSU_SSUSR_ADC_PD_TS	BIT(1)
> +#define TSU_SSUSR_SOC_TS_EN	BIT(2)
> +
> +#define TSU_STRGR		0x04
> +#define TSU_STRGR_ADST		BIT(0)
> +
> +#define TSU_SOSR1		0x08
> +#define TSU_SOSR1_ADCT_8	0x03
> +#define TSU_SOSR1_OUTSEL_AVERAGE	BIT(9)
> +
> +/* Sensor Code Read Register */
> +#define TSU_SCRR		0x10
> +#define TSU_SCRR_OUT12BIT_TS	GENMASK(11, 0)
> +
> +/* Sensor Status Register */
> +#define TSU_SSR			0x14
> +#define TSU_SSR_CONV_RUNNING	BIT(0)
> +
> +/* Compare Mode Setting Register */
> +#define TSU_CMSR		0x18
> +#define TSU_CMSR_CMPEN		BIT(0)
> +#define TSU_CMSR_CMPCOND	BIT(1)
> +
> +/* Lower Limit Setting Register */
> +#define TSU_LLSR		0x1C
> +#define TSU_LLSR_LIM		GENMASK(11, 0)
> +
> +/* Upper Limit Setting Register */
> +#define TSU_ULSR		0x20
> +#define TSU_ULSR_ULIM		GENMASK(11, 0)
> +
> +/* Interrupt Status Register */
> +#define TSU_SISR		0x30
> +#define TSU_SISR_ADF		BIT(0)
> +#define TSU_SISR_CMPF		BIT(1)
> +
> +/* Interrupt Enable Register */
> +#define TSU_SIER		0x34
> +#define TSU_SIER_ADIE		BIT(0)
> +#define TSU_SIER_CMPIE		BIT(1)
> +
> +/* Interrupt Clear Register */
> +#define TSU_SICR		0x38
> +#define TSU_SICR_ADCLR		BIT(0)
> +#define TSU_SICR_CMPCLR		BIT(1)
> +
> +/* Temperature calculation constants */
> +#define TSU_D		41
> +#define TSU_E		126
> +#define TSU_TRMVAL_MASK	GENMASK(11, 0)
> +
> +#define TSU_POLL_DELAY_US	50
> +#define TSU_TIMEOUT_US		10000
> +#define TSU_MIN_CLOCK_RATE	24000000
> +
> +/**
> + * struct rzg3e_thermal_priv - RZ/G3E thermal private data structure
> + * @base: TSU base address
> + * @dev: device pointer
> + * @syscon: regmap for calibration values
> + * @zone: thermal zone pointer
> + * @mode: current tzd mode
> + * @conv_complete: ADC conversion completion
> + * @reg_lock: protect shared register access
> + * @cached_temp: last computed temperature (milliCelsius)
> + * @trmval: trim (calibration) values
> + */
> +struct rzg3e_thermal_priv {
> +	void __iomem *base;
> +	struct device *dev;
> +	struct regmap *syscon;
> +	struct thermal_zone_device *zone;
> +	enum thermal_device_mode mode;
> +	struct completion conv_complete;
> +	spinlock_t reg_lock;
> +	int cached_temp;
> +	u32 trmval[2];
> +};
> +
> +static void rzg3e_thermal_hw_disable(struct rzg3e_thermal_priv *priv) {
> +	/* Disable all interrupts first */
> +	writel(0, priv->base + TSU_SIER);
> +	/* Clear any pending interrupts */
> +	writel(TSU_SICR_ADCLR | TSU_SICR_CMPCLR, priv->base + TSU_SICR);
> +	/* Put device in power down */
> +	writel(TSU_SSUSR_ADC_PD_TS, priv->base + TSU_SSUSR); }
> +
> +static void rzg3e_thermal_hw_enable(struct rzg3e_thermal_priv *priv) {
> +	/* First clear any pending status */
> +	writel(TSU_SICR_ADCLR | TSU_SICR_CMPCLR, priv->base + TSU_SICR);
> +	/* Disable all interrupts */
> +	writel(0, priv->base + TSU_SIER);
> +
> +	/* Enable thermal sensor */
> +	writel(TSU_SSUSR_SOC_TS_EN | TSU_SSUSR_EN_TS, priv->base + TSU_SSUSR);
> +	/* Setup for averaging mode with 8 samples */
> +	writel(TSU_SOSR1_OUTSEL_AVERAGE | TSU_SOSR1_ADCT_8, priv->base +
> +TSU_SOSR1); }
> +
> +static irqreturn_t rzg3e_thermal_cmp_irq(int irq, void *dev_id) {
> +	struct rzg3e_thermal_priv *priv = dev_id;
> +	u32 status;
> +
> +	status = readl(priv->base + TSU_SISR);
> +	if (!(status & TSU_SISR_CMPF))
> +		return IRQ_NONE;
> +
> +	/* Clear the comparison interrupt flag */
> +	writel(TSU_SICR_CMPCLR, priv->base + TSU_SICR);
> +
> +	return IRQ_WAKE_THREAD;
> +}
> +
> +static irqreturn_t rzg3e_thermal_cmp_threaded_irq(int irq, void
> +*dev_id) {
> +	struct rzg3e_thermal_priv *priv = dev_id;
> +
> +	thermal_zone_device_update(priv->zone, THERMAL_EVENT_UNSPECIFIED);
> +	return IRQ_HANDLED;
> +}
> +
> +static irqreturn_t rzg3e_thermal_adc_irq(int irq, void *dev_id) {
> +	struct rzg3e_thermal_priv *priv = dev_id;
> +	u32 status;
> +	u32 result;
> +
> +	/* Check if this is our interrupt */
> +	status = readl(priv->base + TSU_SISR);
> +	if (!(status & TSU_SISR_ADF))
> +		return IRQ_NONE;
> +
> +	/* Disable all interrupts */
> +	writel(0, priv->base + TSU_SIER);
> +	/* Clear conversion complete interrupt */
> +	writel(TSU_SICR_ADCLR, priv->base + TSU_SICR);
> +
> +	/* Read ADC conversion result */
> +	result = readl(priv->base + TSU_SCRR) & TSU_SCRR_OUT12BIT_TS;
> +
> +	/*
> +	 * Calculate temperature using compensation formula
> +	 * Section 7.11.7.8 (Temperature Compensation Calculation)
> +	 *
> +	 * T(°C) = ((e - d) / (c -b)) * (a - b) + d
> +	 *
> +	 * a = 12 bits temperature code read from the sensor
> +	 * b = SYS trmval[0]
> +	 * c = SYS trmval[1]
> +	 * d = -41
> +	 * e = 126
> +	 */
> +	s64 temp_val = div_s64(((TSU_E + TSU_D) * (s64)(result - priv->trmval[0])),
> +				priv->trmval[1] - priv->trmval[0]) - TSU_D;
> +	int new_temp = temp_val * MILLIDEGREE_PER_DEGREE;
> +
> +	scoped_guard(spinlock_irqsave, &priv->reg_lock)
> +		priv->cached_temp = new_temp;
> +
> +	complete(&priv->conv_complete);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static int rzg3e_thermal_get_temp(struct thermal_zone_device *zone, int
> +*temp) {
> +	struct rzg3e_thermal_priv *priv = thermal_zone_device_priv(zone);
> +	u32 val;
> +	int ret;
> +
> +	if (priv->mode == THERMAL_DEVICE_DISABLED)
> +		return -EBUSY;
> +
> +	reinit_completion(&priv->conv_complete);
> +
> +	/* Enable ADC interrupt */
> +	writel(TSU_SIER_ADIE, priv->base + TSU_SIER);
> +
> +	/* Verify no ongoing conversion */
> +	ret = readl_poll_timeout_atomic(priv->base + TSU_SSR, val,
> +					!(val & TSU_SSR_CONV_RUNNING),
> +					TSU_POLL_DELAY_US, TSU_TIMEOUT_US);
> +	if (ret) {
> +		dev_err(priv->dev, "ADC conversion timed out\n");
> +		return ret;
> +	}
> +
> +	/* Start conversion */
> +	writel(TSU_STRGR_ADST, priv->base + TSU_STRGR);
> +
> +	if (!wait_for_completion_timeout(&priv->conv_complete,
> +					 msecs_to_jiffies(100))) {
> +		dev_err(priv->dev, "ADC conversion completion timeout\n");
> +		return -ETIMEDOUT;
> +	}
> +
> +	scoped_guard(spinlock_irqsave, &priv->reg_lock)
> +		*temp = priv->cached_temp;
> +
> +	return 0;
> +}
> +
> +/* Convert temperature in milliCelsius to raw sensor code */ static int
> +rzg3e_temp_to_raw(struct rzg3e_thermal_priv *priv, int temp_mc) {
> +	s64 raw = div_s64(((temp_mc / 1000) - TSU_D) *
> +			  (priv->trmval[1] - priv->trmval[0]),
> +			  (TSU_E - TSU_D));
> +	return clamp_val(raw, 0, 0xFFF);
> +}
> +
> +static int rzg3e_thermal_set_trips(struct thermal_zone_device *tz, int
> +low, int high) {
> +	struct rzg3e_thermal_priv *priv = thermal_zone_device_priv(tz);
> +	int ret;
> +	int val;
> +
> +	if (low >= high)
> +		return -EINVAL;
> +
> +	if (priv->mode == THERMAL_DEVICE_DISABLED)
> +		return -EBUSY;
> +
> +	/* Set up comparison interrupt */
> +	writel(0, priv->base + TSU_SIER);
> +	writel(TSU_SICR_ADCLR | TSU_SICR_CMPCLR, priv->base + TSU_SICR);
> +
> +	/* Set thresholds */
> +	writel(rzg3e_temp_to_raw(priv, low), priv->base + TSU_LLSR);
> +	writel(rzg3e_temp_to_raw(priv, high), priv->base + TSU_ULSR);
> +
> +	/* Configure comparison:
> +	 * - Enable comparison function (CMPEN = 1)
> +	 * - Set comparison condition (CMPCOND = 0 for out of range)
> +	 */
> +	writel(TSU_CMSR_CMPEN, priv->base + TSU_CMSR);
> +
> +	/* Enable comparison irq */
> +	writel(TSU_SIER_CMPIE, priv->base + TSU_SIER);
> +
> +	/* Verify no ongoing conversion */
> +	ret = readl_poll_timeout_atomic(priv->base + TSU_SSR, val,
> +					!(val & TSU_SSR_CONV_RUNNING),
> +					TSU_POLL_DELAY_US, TSU_TIMEOUT_US);
> +	if (ret) {
> +		dev_err(priv->dev, "ADC conversion timed out\n");
> +		return ret;
> +	}
> +
> +	/* Start a conversion to trigger comparison */
> +	writel(TSU_STRGR_ADST, priv->base + TSU_STRGR);
> +
> +	return 0;
> +}
> +
> +static int rzg3e_thermal_get_trimming(struct rzg3e_thermal_priv *priv)
> +{
> +	int ret;
> +
> +	ret = regmap_read(priv->syscon, SYS_TSU_TRMVAL(0), &priv->trmval[0]);
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_read(priv->syscon, SYS_TSU_TRMVAL(1), &priv->trmval[1]);
> +	if (ret)
> +		return ret;

Just checking, which method is better for read/write as system controller registers
needs to be configured  by lot of Driver?

1) Current method using syscon regmap for read/write from client drivers.

2) Using a callback registered with sysc and sysc handles the read/write() during
   callback execution from client drivers.

3) Through exported API and sysc handles the read/write()

Cheers,
Biju

> +
> +	priv->trmval[0] &= TSU_TRMVAL_MASK;
> +	priv->trmval[1] &= TSU_TRMVAL_MASK;
> +
> +	if (!priv->trmval[0] || !priv->trmval[1])
> +		return dev_err_probe(priv->dev, -EINVAL, "invalid trimming values");
> +
> +	return 0;
> +}
> +
> +static int rzg3e_thermal_change_mode(struct thermal_zone_device *tz,
> +				     enum thermal_device_mode mode) {
> +	struct rzg3e_thermal_priv *priv = thermal_zone_device_priv(tz);
> +
> +	if (mode == THERMAL_DEVICE_DISABLED)
> +		rzg3e_thermal_hw_disable(priv);
> +	else
> +		rzg3e_thermal_hw_enable(priv);
> +
> +	priv->mode = mode;
> +	return 0;
> +}
> +
> +static const struct thermal_zone_device_ops rzg3e_tz_of_ops = {
> +	.get_temp = rzg3e_thermal_get_temp,
> +	.set_trips = rzg3e_thermal_set_trips,
> +	.change_mode = rzg3e_thermal_change_mode, };
> +
> +static int rzg3e_thermal_probe(struct platform_device *pdev) {
> +	struct device *dev = &pdev->dev;
> +	struct rzg3e_thermal_priv *priv;
> +	struct reset_control *rstc;
> +	char *adc_name, *cmp_name;
> +	int adc_irq, cmp_irq;
> +	struct clk *clk;
> +	int ret;
> +
> +	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +
> +	priv->dev = dev;
> +
> +	priv->base = devm_platform_ioremap_resource(pdev, 0);
> +	if (IS_ERR(priv->base))
> +		return dev_err_probe(dev, PTR_ERR(priv->base),
> +				"Failed to map I/O memory");
> +
> +	priv->syscon = syscon_regmap_lookup_by_phandle(dev->of_node,
> +						       "renesas,tsu-calibration-sys");
> +	if (IS_ERR(priv->syscon))
> +		return dev_err_probe(dev, PTR_ERR(priv->syscon),
> +				"Failed to get calibration syscon");
> +
> +	adc_irq = platform_get_irq_byname(pdev, "adi");
> +	if (adc_irq < 0)
> +		return adc_irq;
> +
> +	cmp_irq = platform_get_irq_byname(pdev, "adcmpi");
> +	if (cmp_irq < 0)
> +		return cmp_irq;
> +
> +	rstc = devm_reset_control_get_exclusive_deasserted(dev, NULL);
> +	if (IS_ERR(rstc))
> +		return dev_err_probe(dev, PTR_ERR(rstc),
> +				     "Failed to acquire deasserted reset");
> +
> +	platform_set_drvdata(pdev, priv);
> +
> +	spin_lock_init(&priv->reg_lock);
> +	init_completion(&priv->conv_complete);
> +
> +	clk = devm_clk_get_enabled(dev, NULL);
> +	if (IS_ERR(clk))
> +		return dev_err_probe(dev, PTR_ERR(clk),
> +				     "Failed to get and enable clock");
> +
> +	if (clk_get_rate(clk) < TSU_MIN_CLOCK_RATE)
> +		return dev_err_probe(dev, -EINVAL,
> +				     "Clock rate too low (minimum %d Hz required)",
> +				     TSU_MIN_CLOCK_RATE);
> +
> +	ret = rzg3e_thermal_get_trimming(priv);
> +	if (ret)
> +		return ret;
> +
> +	adc_name = devm_kasprintf(dev, GFP_KERNEL, "%s-adc", dev_name(dev));
> +	if (!adc_name)
> +		return -ENOMEM;
> +
> +	cmp_name = devm_kasprintf(dev, GFP_KERNEL, "%s-cmp", dev_name(dev));
> +	if (!cmp_name)
> +		return -ENOMEM;
> +
> +	/* Unit in a known disabled mode */
> +	rzg3e_thermal_hw_disable(priv);
> +
> +	ret = devm_request_irq(dev, adc_irq, rzg3e_thermal_adc_irq,
> +			       IRQF_TRIGGER_RISING, adc_name, priv);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "Failed to request ADC IRQ");
> +
> +	ret = devm_request_threaded_irq(dev, cmp_irq, rzg3e_thermal_cmp_irq,
> +					rzg3e_thermal_cmp_threaded_irq,
> +					IRQF_TRIGGER_HIGH | IRQF_ONESHOT,
> +					cmp_name, priv);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "Failed to request comparison IRQ");
> +
> +	/* Register Thermal Zone */
> +	priv->zone = devm_thermal_of_zone_register(dev, 0, priv, &rzg3e_tz_of_ops);
> +	if (IS_ERR(priv->zone))
> +		return dev_err_probe(dev, PTR_ERR(priv->zone),
> +				"Failed to register thermal zone");
> +
> +	ret = devm_thermal_add_hwmon_sysfs(dev, priv->zone);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "Failed to add hwmon sysfs");
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id rzg3e_thermal_dt_ids[] = {
> +	{ .compatible = "renesas,r9a09g047-tsu" },
> +	{ /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, rzg3e_thermal_dt_ids);
> +
> +static struct platform_driver rzg3e_thermal_driver = {
> +	.driver = {
> +		.name	= "rzg3e_thermal",
> +		.of_match_table = rzg3e_thermal_dt_ids,
> +	},
> +	.probe = rzg3e_thermal_probe,
> +};
> +module_platform_driver(rzg3e_thermal_driver);
> +
> +MODULE_DESCRIPTION("Renesas RZ/G3E TSU Thermal Sensor Driver");
> +MODULE_AUTHOR("John Madieu <john.madieu.xa@bp.renesas.com>");
> +MODULE_LICENSE("GPL");
> --
> 2.25.1

RE: [PATCH v6 3/5] thermal: renesas: rzg3e: Add thermal driver for the Renesas RZ/G3E SoC

[John Madieu]

Hi Biju,

> -----Original Message-----
> From: Biju Das <biju.das.jz@bp.renesas.com>
> Sent: Friday, June 6, 2025 8:17 AM
> To: John Madieu <john.madieu.xa@bp.renesas.com>; John Madieu
> <john.madieu.xa@bp.renesas.com>; conor+dt@kernel.org;
> daniel.lezcano@linaro.org; geert+renesas@glider.be; krzk+dt@kernel.org;
> rafael@kernel.org
> 
> Hi John,
> 
> > -----Original Message-----
> > From: John Madieu <john.madieu.xa@bp.renesas.com>
> > Sent: 22 May 2025 19:23
> > Subject: [PATCH v6 3/5] thermal: renesas: rzg3e: Add thermal driver
> > for the Renesas RZ/G3E SoC
> >
> > The RZ/G3E SoC integrates a Temperature Sensor Unit (TSU) block
> > designed to monitor the chip's junction temperature. This sensor is
> > connected to channel 1 of the APB port clock/reset and provides
> temperature measurements.
> >
> > It also requires calibration values stored in the system controller
> > registers for accurate temperature measurement. Add a driver for the
> Renesas RZ/G3E TSU.
> >
> > Signed-off-by: John Madieu <john.madieu.xa@bp.renesas.com>
> > ---
> >
> > Changes:
> >
> > v1 -> v2: fixes IRQ names
> > v2 -> v3: no changes
> > v3 -> v4: no changes
> > v5: removes curly braces arround single-line protected scoped guards
> > v6: Clarified comments in driver
> >
> >  MAINTAINERS                             |   7 +
> >  drivers/thermal/renesas/Kconfig         |   7 +
> >  drivers/thermal/renesas/Makefile        |   1 +
> >  drivers/thermal/renesas/rzg3e_thermal.c | 443
> > ++++++++++++++++++++++++
> >  4 files changed, 458 insertions(+)
> >  create mode 100644 drivers/thermal/renesas/rzg3e_thermal.c
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS index
> > 79a8e2c73908..eb11494795e8 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -21161,6 +21161,13 @@ S:	Maintained
> >  F:
> 	Documentation/devicetree/bindings/iio/potentiometer/renesas,x9250.ya
> ml
> >  F:	drivers/iio/potentiometer/x9250.c
> >
> > +RENESAS RZ/G3E THERMAL SENSOR UNIT DRIVER
> > +M:	John Madieu <john.madieu.xa@bp.renesas.com>
> > +L:	linux-pm@vger.kernel.org
> > +S:	Maintained
> > +F:	Documentation/devicetree/bindings/thermal/renesas,r9a09g047-tsu.yaml
> > +F:	drivers/thermal/renesas/rzg3e_thermal.c
> > +
> >  RESET CONTROLLER FRAMEWORK
> >  M:	Philipp Zabel <p.zabel@pengutronix.de>
> >  S:	Maintained
> > diff --git a/drivers/thermal/renesas/Kconfig
> > b/drivers/thermal/renesas/Kconfig index dcf5fc5ae08e..10cf90fc4bfa
> > 100644
> > --- a/drivers/thermal/renesas/Kconfig
> > +++ b/drivers/thermal/renesas/Kconfig
> > @@ -26,3 +26,10 @@ config RZG2L_THERMAL
> >  	help
> >  	  Enable this to plug the RZ/G2L thermal sensor driver into the
> Linux
> >  	  thermal framework.
> > +
> > +config RZG3E_THERMAL
> > +	tristate "Renesas RZ/G3E thermal driver"
> > +	depends on ARCH_RENESAS || COMPILE_TEST
> > +	help
> > +	  Enable this to plug the RZ/G3E thermal sensor driver into the
> Linux
> > +	  thermal framework.
> > diff --git a/drivers/thermal/renesas/Makefile
> > b/drivers/thermal/renesas/Makefile
> > index bf9cb3cb94d6..5a3eba0dedd0 100644
> > --- a/drivers/thermal/renesas/Makefile
> > +++ b/drivers/thermal/renesas/Makefile
> > @@ -3,3 +3,4 @@
> >  obj-$(CONFIG_RCAR_GEN3_THERMAL)	+= rcar_gen3_thermal.o
> >  obj-$(CONFIG_RCAR_THERMAL)	+= rcar_thermal.o
> >  obj-$(CONFIG_RZG2L_THERMAL)	+= rzg2l_thermal.o
> > +obj-$(CONFIG_RZG3E_THERMAL)	+= rzg3e_thermal.o
> > diff --git a/drivers/thermal/renesas/rzg3e_thermal.c
> > b/drivers/thermal/renesas/rzg3e_thermal.c
> > new file mode 100644
> > index 000000000000..348229da9ef4
> > --- /dev/null
> > +++ b/drivers/thermal/renesas/rzg3e_thermal.c
> > @@ -0,0 +1,443 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Renesas RZ/G3E TSU Temperature Sensor Unit
> > + *
> > + * Copyright (C) 2025 Renesas Electronics Corporation  */ #include
> > +<linux/clk.h> #include <linux/delay.h> #include <linux/err.h>
> > +#include <linux/interrupt.h> #include <linux/io.h> #include
> > +<linux/iopoll.h> #include <linux/kernel.h> #include
> > +<linux/mfd/syscon.h> #include <linux/module.h> #include
> > +<linux/of_device.h> #include <linux/platform_device.h> #include
> > +<linux/regmap.h> #include <linux/reset.h> #include <linux/thermal.h>
> > +#include <linux/units.h>
> > +
> > +#include "../thermal_hwmon.h"
> > +
> > +/* SYS Trimming register offsets macro */ #define SYS_TSU_TRMVAL(x)
> > +(0x330 + (x) * 4)
> > +
> > +/* TSU Register offsets and bits */
> > +#define TSU_SSUSR		0x00
> > +#define TSU_SSUSR_EN_TS		BIT(0)
> > +#define TSU_SSUSR_ADC_PD_TS	BIT(1)
> > +#define TSU_SSUSR_SOC_TS_EN	BIT(2)
> > +
> > +#define TSU_STRGR		0x04
> > +#define TSU_STRGR_ADST		BIT(0)
> > +
> > +#define TSU_SOSR1		0x08
> > +#define TSU_SOSR1_ADCT_8	0x03
> > +#define TSU_SOSR1_OUTSEL_AVERAGE	BIT(9)
> > +
> > +/* Sensor Code Read Register */
> > +#define TSU_SCRR		0x10
> > +#define TSU_SCRR_OUT12BIT_TS	GENMASK(11, 0)
> > +
> > +/* Sensor Status Register */
> > +#define TSU_SSR			0x14
> > +#define TSU_SSR_CONV_RUNNING	BIT(0)
> > +
> > +/* Compare Mode Setting Register */
> > +#define TSU_CMSR		0x18
> > +#define TSU_CMSR_CMPEN		BIT(0)
> > +#define TSU_CMSR_CMPCOND	BIT(1)
> > +
> > +/* Lower Limit Setting Register */
> > +#define TSU_LLSR		0x1C
> > +#define TSU_LLSR_LIM		GENMASK(11, 0)
> > +
> > +/* Upper Limit Setting Register */
> > +#define TSU_ULSR		0x20
> > +#define TSU_ULSR_ULIM		GENMASK(11, 0)
> > +
> > +/* Interrupt Status Register */
> > +#define TSU_SISR		0x30
> > +#define TSU_SISR_ADF		BIT(0)
> > +#define TSU_SISR_CMPF		BIT(1)
> > +
> > +/* Interrupt Enable Register */
> > +#define TSU_SIER		0x34
> > +#define TSU_SIER_ADIE		BIT(0)
> > +#define TSU_SIER_CMPIE		BIT(1)
> > +
> > +/* Interrupt Clear Register */
> > +#define TSU_SICR		0x38
> > +#define TSU_SICR_ADCLR		BIT(0)
> > +#define TSU_SICR_CMPCLR		BIT(1)
> > +
> > +/* Temperature calculation constants */
> > +#define TSU_D		41
> > +#define TSU_E		126
> > +#define TSU_TRMVAL_MASK	GENMASK(11, 0)
> > +
> > +#define TSU_POLL_DELAY_US	50
> > +#define TSU_TIMEOUT_US		10000
> > +#define TSU_MIN_CLOCK_RATE	24000000
> > +
> > +/**
> > +	/* Start a conversion to trigger comparison */
> > +	writel(TSU_STRGR_ADST, priv->base + TSU_STRGR);
> > +
> > +	return 0;
> > +}
> > +
> > +static int rzg3e_thermal_get_trimming(struct rzg3e_thermal_priv
> > +*priv) {
> > +	int ret;
> > +
> > +	ret = regmap_read(priv->syscon, SYS_TSU_TRMVAL(0), &priv-
> >trmval[0]);
> > +	if (ret)
> > +		return ret;
> > +
> > +	ret = regmap_read(priv->syscon, SYS_TSU_TRMVAL(1), &priv-
> >trmval[1]);
> > +	if (ret)
> > +		return ret;
> 
> Just checking, which method is better for read/write as system controller
> registers needs to be configured  by lot of Driver?
> 
> 1) Current method using syscon regmap for read/write from client drivers.
> 
> 2) Using a callback registered with sysc and sysc handles the read/write()
> during
>    callback execution from client drivers.
> 
> 3) Through exported API and sysc handles the read/write()
> 
> Cheers,
> Biju
> 

I think the current method is the best, as syscon/regmap would
serialize (and cache if need be) accesses. For the second case,
I think we would duplicate what syscon already does, as it provides
generic accesses for register/bit/masking read/write/update. For the
third case, we would need as many APIs in the sysc driver as logical
operations needed by client drivers.

I'm however open to any suggestion that might ease the review of the driver.

Regards,
John

> > +
> > +	priv->trmval[0] &= TSU_TRMVAL_MASK;
> > +	priv->trmval[1] &= TSU_TRMVAL_MASK;
> > +
> > +	if (!priv->trmval[0] || !priv->trmval[1])
> > +		return dev_err_probe(priv->dev, -EINVAL, "invalid trimming
> > +values");
> > +
> > +	return 0;
> > +}
> > +
> > +static int rzg3e_thermal_change_mode(struct thermal_zone_device *tz,
> > +				     enum thermal_device_mode mode) {
> > +	struct rzg3e_thermal_priv *priv = thermal_zone_device_priv(tz);
> > +
> > +	if (mode == THERMAL_DEVICE_DISABLED)
> > +		rzg3e_thermal_hw_disable(priv);
> > +	else
> > +		rzg3e_thermal_hw_enable(priv);
> > +
> > +	priv->mode = mode;
> > +	return 0;
> > +}
> > +
> > +static const struct thermal_zone_device_ops rzg3e_tz_of_ops = {
> > +	.get_temp = rzg3e_thermal_get_temp,
> > +	.set_trips = rzg3e_thermal_set_trips,
> > +	.change_mode = rzg3e_thermal_change_mode, };
> > +
> > +static int rzg3e_thermal_probe(struct platform_device *pdev) {
> > +	struct device *dev = &pdev->dev;
> > +	struct rzg3e_thermal_priv *priv;
> > +	struct reset_control *rstc;
> > +	char *adc_name, *cmp_name;
> > +	int adc_irq, cmp_irq;
> > +	struct clk *clk;
> > +	int ret;
> > +
> > +	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> > +	if (!priv)
> > +		return -ENOMEM;
> > +
> > +	priv->dev = dev;
> > +
> > +	priv->base = devm_platform_ioremap_resource(pdev, 0);
> > +	if (IS_ERR(priv->base))
> > +		return dev_err_probe(dev, PTR_ERR(priv->base),
> > +				"Failed to map I/O memory");
> > +
> > +	priv->syscon = syscon_regmap_lookup_by_phandle(dev->of_node,
> > +						       "renesas,tsu-calibration-sys");
> > +	if (IS_ERR(priv->syscon))
> > +		return dev_err_probe(dev, PTR_ERR(priv->syscon),
> > +				"Failed to get calibration syscon");
> > +
> > +	adc_irq = platform_get_irq_byname(pdev, "adi");
> > +	if (adc_irq < 0)
> > +		return adc_irq;
> > +
> > +	cmp_irq = platform_get_irq_byname(pdev, "adcmpi");
> > +	if (cmp_irq < 0)
> > +		return cmp_irq;
> > +
> > +	rstc = devm_reset_control_get_exclusive_deasserted(dev, NULL);
> > +	if (IS_ERR(rstc))
> > +		return dev_err_probe(dev, PTR_ERR(rstc),
> > +				     "Failed to acquire deasserted reset");
> > +
> > +	platform_set_drvdata(pdev, priv);
> > +
> > +	spin_lock_init(&priv->reg_lock);
> > +	init_completion(&priv->conv_complete);
> > +
> > +	clk = devm_clk_get_enabled(dev, NULL);
> > +	if (IS_ERR(clk))
> > +		return dev_err_probe(dev, PTR_ERR(clk),
> > +				     "Failed to get and enable clock");
> > +
> > +	if (clk_get_rate(clk) < TSU_MIN_CLOCK_RATE)
> > +		return dev_err_probe(dev, -EINVAL,
> > +				     "Clock rate too low (minimum %d Hz
> required)",
> > +				     TSU_MIN_CLOCK_RATE);
> > +
> > +	ret = rzg3e_thermal_get_trimming(priv);
> > +	if (ret)
> > +		return ret;
> > +
> > +	adc_name = devm_kasprintf(dev, GFP_KERNEL, "%s-adc", dev_name(dev));
> > +	if (!adc_name)
> > +		return -ENOMEM;
> > +
> > +	cmp_name = devm_kasprintf(dev, GFP_KERNEL, "%s-cmp", dev_name(dev));
> > +	if (!cmp_name)
> > +		return -ENOMEM;
> > +
> > +	/* Unit in a known disabled mode */
> > +	rzg3e_thermal_hw_disable(priv);
> > +
> > +	ret = devm_request_irq(dev, adc_irq, rzg3e_thermal_adc_irq,
> > +			       IRQF_TRIGGER_RISING, adc_name, priv);
> > +	if (ret)
> > +		return dev_err_probe(dev, ret, "Failed to request ADC IRQ");
> > +
> > +	ret = devm_request_threaded_irq(dev, cmp_irq, rzg3e_thermal_cmp_irq,
> > +					rzg3e_thermal_cmp_threaded_irq,
> > +					IRQF_TRIGGER_HIGH | IRQF_ONESHOT,
> > +					cmp_name, priv);
> > +	if (ret)
> > +		return dev_err_probe(dev, ret, "Failed to request comparison
> IRQ");
> > +
> > +	/* Register Thermal Zone */
> > +	priv->zone = devm_thermal_of_zone_register(dev, 0, priv,
> &rzg3e_tz_of_ops);
> > +	if (IS_ERR(priv->zone))
> > +		return dev_err_probe(dev, PTR_ERR(priv->zone),
> > +				"Failed to register thermal zone");
> > +
> > +	ret = devm_thermal_add_hwmon_sysfs(dev, priv->zone);
> > +	if (ret)
> > +		return dev_err_probe(dev, ret, "Failed to add hwmon sysfs");
> > +
> > +	return 0;
> > +}
> > +
> > +static const struct of_device_id rzg3e_thermal_dt_ids[] = {
> > +	{ .compatible = "renesas,r9a09g047-tsu" },
> > +	{ /* sentinel */ }
> > +};
> > +MODULE_DEVICE_TABLE(of, rzg3e_thermal_dt_ids);
> > +
> > +static struct platform_driver rzg3e_thermal_driver = {
> > +	.driver = {
> > +		.name	= "rzg3e_thermal",
> > +		.of_match_table = rzg3e_thermal_dt_ids,
> > +	},
> > +	.probe = rzg3e_thermal_probe,
> > +};
> > +module_platform_driver(rzg3e_thermal_driver);
> > +
> > +MODULE_DESCRIPTION("Renesas RZ/G3E TSU Thermal Sensor Driver");
> > +MODULE_AUTHOR("John Madieu <john.madieu.xa@bp.renesas.com>");
> > +MODULE_LICENSE("GPL");
> > --
> > 2.25.1

Re: [PATCH v6 3/5] thermal: renesas: rzg3e: Add thermal driver for the Renesas RZ/G3E SoC

[Daniel Lezcano]

On Thu, May 22, 2025 at 08:22:46PM +0200, John Madieu wrote:
> The RZ/G3E SoC integrates a Temperature Sensor Unit (TSU) block designed
> to monitor the chip's junction temperature. This sensor is connected to
> channel 1 of the APB port clock/reset and provides temperature measurements.
> 
> It also requires calibration values stored in the system controller registers
> for accurate temperature measurement. Add a driver for the Renesas RZ/G3E TSU.
> 
> Signed-off-by: John Madieu <john.madieu.xa@bp.renesas.com>
> ---
> 
> Changes:
> 
> v1 -> v2: fixes IRQ names
> v2 -> v3: no changes
> v3 -> v4: no changes
> v5: removes curly braces arround single-line protected scoped guards
> v6: Clarified comments in driver
> 
>  MAINTAINERS                             |   7 +
>  drivers/thermal/renesas/Kconfig         |   7 +
>  drivers/thermal/renesas/Makefile        |   1 +
>  drivers/thermal/renesas/rzg3e_thermal.c | 443 ++++++++++++++++++++++++
>  4 files changed, 458 insertions(+)
>  create mode 100644 drivers/thermal/renesas/rzg3e_thermal.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 79a8e2c73908..eb11494795e8 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -21161,6 +21161,13 @@ S:	Maintained
>  F:	Documentation/devicetree/bindings/iio/potentiometer/renesas,x9250.yaml
>  F:	drivers/iio/potentiometer/x9250.c
>  
> +RENESAS RZ/G3E THERMAL SENSOR UNIT DRIVER
> +M:	John Madieu <john.madieu.xa@bp.renesas.com>
> +L:	linux-pm@vger.kernel.org
> +S:	Maintained
> +F:	Documentation/devicetree/bindings/thermal/renesas,r9a09g047-tsu.yaml
> +F:	drivers/thermal/renesas/rzg3e_thermal.c
> +
>  RESET CONTROLLER FRAMEWORK
>  M:	Philipp Zabel <p.zabel@pengutronix.de>
>  S:	Maintained
> diff --git a/drivers/thermal/renesas/Kconfig b/drivers/thermal/renesas/Kconfig
> index dcf5fc5ae08e..10cf90fc4bfa 100644
> --- a/drivers/thermal/renesas/Kconfig
> +++ b/drivers/thermal/renesas/Kconfig
> @@ -26,3 +26,10 @@ config RZG2L_THERMAL
>  	help
>  	  Enable this to plug the RZ/G2L thermal sensor driver into the Linux
>  	  thermal framework.
> +
> +config RZG3E_THERMAL
> +	tristate "Renesas RZ/G3E thermal driver"
> +	depends on ARCH_RENESAS || COMPILE_TEST
> +	help
> +	  Enable this to plug the RZ/G3E thermal sensor driver into the Linux
> +	  thermal framework.
> diff --git a/drivers/thermal/renesas/Makefile b/drivers/thermal/renesas/Makefile
> index bf9cb3cb94d6..5a3eba0dedd0 100644
> --- a/drivers/thermal/renesas/Makefile
> +++ b/drivers/thermal/renesas/Makefile
> @@ -3,3 +3,4 @@
>  obj-$(CONFIG_RCAR_GEN3_THERMAL)	+= rcar_gen3_thermal.o
>  obj-$(CONFIG_RCAR_THERMAL)	+= rcar_thermal.o
>  obj-$(CONFIG_RZG2L_THERMAL)	+= rzg2l_thermal.o
> +obj-$(CONFIG_RZG3E_THERMAL)	+= rzg3e_thermal.o
> diff --git a/drivers/thermal/renesas/rzg3e_thermal.c b/drivers/thermal/renesas/rzg3e_thermal.c
> new file mode 100644
> index 000000000000..348229da9ef4
> --- /dev/null
> +++ b/drivers/thermal/renesas/rzg3e_thermal.c
> @@ -0,0 +1,443 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Renesas RZ/G3E TSU Temperature Sensor Unit
> + *
> + * Copyright (C) 2025 Renesas Electronics Corporation
> + */
> +#include <linux/clk.h>
> +#include <linux/delay.h>
> +#include <linux/err.h>
> +#include <linux/interrupt.h>
> +#include <linux/io.h>
> +#include <linux/iopoll.h>
> +#include <linux/kernel.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/module.h>
> +#include <linux/of_device.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +#include <linux/reset.h>
> +#include <linux/thermal.h>
> +#include <linux/units.h>
> +
> +#include "../thermal_hwmon.h"
> +
> +/* SYS Trimming register offsets macro */
> +#define SYS_TSU_TRMVAL(x) (0x330 + (x) * 4)
> +
> +/* TSU Register offsets and bits */
> +#define TSU_SSUSR		0x00
> +#define TSU_SSUSR_EN_TS		BIT(0)
> +#define TSU_SSUSR_ADC_PD_TS	BIT(1)
> +#define TSU_SSUSR_SOC_TS_EN	BIT(2)
> +
> +#define TSU_STRGR		0x04
> +#define TSU_STRGR_ADST		BIT(0)
> +
> +#define TSU_SOSR1		0x08
> +#define TSU_SOSR1_ADCT_8	0x03
> +#define TSU_SOSR1_OUTSEL_AVERAGE	BIT(9)
> +
> +/* Sensor Code Read Register */
> +#define TSU_SCRR		0x10
> +#define TSU_SCRR_OUT12BIT_TS	GENMASK(11, 0)
> +
> +/* Sensor Status Register */
> +#define TSU_SSR			0x14
> +#define TSU_SSR_CONV_RUNNING	BIT(0)
> +
> +/* Compare Mode Setting Register */
> +#define TSU_CMSR		0x18
> +#define TSU_CMSR_CMPEN		BIT(0)
> +#define TSU_CMSR_CMPCOND	BIT(1)
> +
> +/* Lower Limit Setting Register */
> +#define TSU_LLSR		0x1C
> +#define TSU_LLSR_LIM		GENMASK(11, 0)
> +
> +/* Upper Limit Setting Register */
> +#define TSU_ULSR		0x20
> +#define TSU_ULSR_ULIM		GENMASK(11, 0)
> +
> +/* Interrupt Status Register */
> +#define TSU_SISR		0x30
> +#define TSU_SISR_ADF		BIT(0)
> +#define TSU_SISR_CMPF		BIT(1)
> +
> +/* Interrupt Enable Register */
> +#define TSU_SIER		0x34
> +#define TSU_SIER_ADIE		BIT(0)
> +#define TSU_SIER_CMPIE		BIT(1)
> +
> +/* Interrupt Clear Register */
> +#define TSU_SICR		0x38
> +#define TSU_SICR_ADCLR		BIT(0)
> +#define TSU_SICR_CMPCLR		BIT(1)
> +
> +/* Temperature calculation constants */
> +#define TSU_D		41
> +#define TSU_E		126
> +#define TSU_TRMVAL_MASK	GENMASK(11, 0)
> +
> +#define TSU_POLL_DELAY_US	50
> +#define TSU_TIMEOUT_US		10000
> +#define TSU_MIN_CLOCK_RATE	24000000
> +
> +/**
> + * struct rzg3e_thermal_priv - RZ/G3E thermal private data structure
> + * @base: TSU base address
> + * @dev: device pointer
> + * @syscon: regmap for calibration values
> + * @zone: thermal zone pointer
> + * @mode: current tzd mode
> + * @conv_complete: ADC conversion completion
> + * @reg_lock: protect shared register access
> + * @cached_temp: last computed temperature (milliCelsius)
> + * @trmval: trim (calibration) values
> + */
> +struct rzg3e_thermal_priv {
> +	void __iomem *base;
> +	struct device *dev;
> +	struct regmap *syscon;
> +	struct thermal_zone_device *zone;
> +	enum thermal_device_mode mode;
> +	struct completion conv_complete;
> +	spinlock_t reg_lock;
> +	int cached_temp;
> +	u32 trmval[2];
> +};
> +
> +static void rzg3e_thermal_hw_disable(struct rzg3e_thermal_priv *priv)
> +{
> +	/* Disable all interrupts first */
> +	writel(0, priv->base + TSU_SIER);
> +	/* Clear any pending interrupts */
> +	writel(TSU_SICR_ADCLR | TSU_SICR_CMPCLR, priv->base + TSU_SICR);
> +	/* Put device in power down */
> +	writel(TSU_SSUSR_ADC_PD_TS, priv->base + TSU_SSUSR);
> +}
> +
> +static void rzg3e_thermal_hw_enable(struct rzg3e_thermal_priv *priv)
> +{
> +	/* First clear any pending status */
> +	writel(TSU_SICR_ADCLR | TSU_SICR_CMPCLR, priv->base + TSU_SICR);
> +	/* Disable all interrupts */
> +	writel(0, priv->base + TSU_SIER);
> +
> +	/* Enable thermal sensor */
> +	writel(TSU_SSUSR_SOC_TS_EN | TSU_SSUSR_EN_TS, priv->base + TSU_SSUSR);
> +	/* Setup for averaging mode with 8 samples */
> +	writel(TSU_SOSR1_OUTSEL_AVERAGE | TSU_SOSR1_ADCT_8, priv->base + TSU_SOSR1);
> +}
> +
> +static irqreturn_t rzg3e_thermal_cmp_irq(int irq, void *dev_id)
> +{
> +	struct rzg3e_thermal_priv *priv = dev_id;
> +	u32 status;
> +
> +	status = readl(priv->base + TSU_SISR);
> +	if (!(status & TSU_SISR_CMPF))
> +		return IRQ_NONE;
> +
> +	/* Clear the comparison interrupt flag */
> +	writel(TSU_SICR_CMPCLR, priv->base + TSU_SICR);
> +
> +	return IRQ_WAKE_THREAD;
> +}
> +
> +static irqreturn_t rzg3e_thermal_cmp_threaded_irq(int irq, void *dev_id)
> +{
> +	struct rzg3e_thermal_priv *priv = dev_id;
> +
> +	thermal_zone_device_update(priv->zone, THERMAL_EVENT_UNSPECIFIED);
> +	return IRQ_HANDLED;
> +}
> +
> +static irqreturn_t rzg3e_thermal_adc_irq(int irq, void *dev_id)
> +{
> +	struct rzg3e_thermal_priv *priv = dev_id;
> +	u32 status;
> +	u32 result;
> +
> +	/* Check if this is our interrupt */
> +	status = readl(priv->base + TSU_SISR);
> +	if (!(status & TSU_SISR_ADF))
> +		return IRQ_NONE;
> +
> +	/* Disable all interrupts */
> +	writel(0, priv->base + TSU_SIER);
> +	/* Clear conversion complete interrupt */
> +	writel(TSU_SICR_ADCLR, priv->base + TSU_SICR);
> +
> +	/* Read ADC conversion result */
> +	result = readl(priv->base + TSU_SCRR) & TSU_SCRR_OUT12BIT_TS;
> +
> +	/*
> +	 * Calculate temperature using compensation formula
> +	 * Section 7.11.7.8 (Temperature Compensation Calculation)
> +	 *
> +	 * T(°C) = ((e - d) / (c -b)) * (a - b) + d
> +	 *
> +	 * a = 12 bits temperature code read from the sensor
> +	 * b = SYS trmval[0]
> +	 * c = SYS trmval[1]
> +	 * d = -41
> +	 * e = 126
> +	 */
> +	s64 temp_val = div_s64(((TSU_E + TSU_D) * (s64)(result - priv->trmval[0])),
> +				priv->trmval[1] - priv->trmval[0]) - TSU_D;
> +	int new_temp = temp_val * MILLIDEGREE_PER_DEGREE;
> +
> +	scoped_guard(spinlock_irqsave, &priv->reg_lock)
> +		priv->cached_temp = new_temp;
> +
> +	complete(&priv->conv_complete);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static int rzg3e_thermal_get_temp(struct thermal_zone_device *zone, int *temp)
> +{
> +	struct rzg3e_thermal_priv *priv = thermal_zone_device_priv(zone);
> +	u32 val;
> +	int ret;
> +
> +	if (priv->mode == THERMAL_DEVICE_DISABLED)
> +		return -EBUSY;

Why ?

> +	reinit_completion(&priv->conv_complete);
> +
> +	/* Enable ADC interrupt */
> +	writel(TSU_SIER_ADIE, priv->base + TSU_SIER);

Why enable irq here ?

> +	/* Verify no ongoing conversion */
> +	ret = readl_poll_timeout_atomic(priv->base + TSU_SSR, val,
> +					!(val & TSU_SSR_CONV_RUNNING),
> +					TSU_POLL_DELAY_US, TSU_TIMEOUT_US);
> +	if (ret) {
> +		dev_err(priv->dev, "ADC conversion timed out\n");
> +		return ret;
> +	}
> +
> +	/* Start conversion */
> +	writel(TSU_STRGR_ADST, priv->base + TSU_STRGR);
> +
> +	if (!wait_for_completion_timeout(&priv->conv_complete,
> +					 msecs_to_jiffies(100))) {
> +		dev_err(priv->dev, "ADC conversion completion timeout\n");
> +		return -ETIMEDOUT;
> +	}

Can you explain what is happening here ?

> +	scoped_guard(spinlock_irqsave, &priv->reg_lock)
> +		*temp = priv->cached_temp;
> +
> +	return 0;
> +}
> +
> +/* Convert temperature in milliCelsius to raw sensor code */
> +static int rzg3e_temp_to_raw(struct rzg3e_thermal_priv *priv, int temp_mc)
> +{
> +	s64 raw = div_s64(((temp_mc / 1000) - TSU_D) *
> +			  (priv->trmval[1] - priv->trmval[0]),
> +			  (TSU_E - TSU_D));
> +	return clamp_val(raw, 0, 0xFFF);
> +}
> +
> +static int rzg3e_thermal_set_trips(struct thermal_zone_device *tz, int low, int high)
> +{
> +	struct rzg3e_thermal_priv *priv = thermal_zone_device_priv(tz);
> +	int ret;
> +	int val;
> +
> +	if (low >= high)
> +		return -EINVAL;
> +
> +	if (priv->mode == THERMAL_DEVICE_DISABLED)
> +		return -EBUSY;

That is not supposed to happen. set_trips is called from
thermal_zone_device_update but the thermal zone is disabled, the
function bails out, thus it should not call this callback.

> +	/* Set up comparison interrupt */
> +	writel(0, priv->base + TSU_SIER);
> +	writel(TSU_SICR_ADCLR | TSU_SICR_CMPCLR, priv->base + TSU_SICR);
> +
> +	/* Set thresholds */
> +	writel(rzg3e_temp_to_raw(priv, low), priv->base + TSU_LLSR);
> +	writel(rzg3e_temp_to_raw(priv, high), priv->base + TSU_ULSR);
> +
> +	/* Configure comparison:
> +	 * - Enable comparison function (CMPEN = 1)
> +	 * - Set comparison condition (CMPCOND = 0 for out of range)
> +	 */
> +	writel(TSU_CMSR_CMPEN, priv->base + TSU_CMSR);
> +
> +	/* Enable comparison irq */
> +	writel(TSU_SIER_CMPIE, priv->base + TSU_SIER);
> +
> +	/* Verify no ongoing conversion */
> +	ret = readl_poll_timeout_atomic(priv->base + TSU_SSR, val,
> +					!(val & TSU_SSR_CONV_RUNNING),
> +					TSU_POLL_DELAY_US, TSU_TIMEOUT_US);
> +	if (ret) {
> +		dev_err(priv->dev, "ADC conversion timed out\n");
> +		return ret;
> +	}
> +
> +	/* Start a conversion to trigger comparison */
> +	writel(TSU_STRGR_ADST, priv->base + TSU_STRGR);
> +
> +	return 0;
> +}
> +
> +static int rzg3e_thermal_get_trimming(struct rzg3e_thermal_priv *priv)
> +{
> +	int ret;
> +
> +	ret = regmap_read(priv->syscon, SYS_TSU_TRMVAL(0), &priv->trmval[0]);
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_read(priv->syscon, SYS_TSU_TRMVAL(1), &priv->trmval[1]);
> +	if (ret)
> +		return ret;
> +
> +	priv->trmval[0] &= TSU_TRMVAL_MASK;
> +	priv->trmval[1] &= TSU_TRMVAL_MASK;
> +
> +	if (!priv->trmval[0] || !priv->trmval[1])
> +		return dev_err_probe(priv->dev, -EINVAL, "invalid trimming values");
> +
> +	return 0;
> +}
> +
> +static int rzg3e_thermal_change_mode(struct thermal_zone_device *tz,
> +				     enum thermal_device_mode mode)
> +{
> +	struct rzg3e_thermal_priv *priv = thermal_zone_device_priv(tz);
> +
> +	if (mode == THERMAL_DEVICE_DISABLED)
> +		rzg3e_thermal_hw_disable(priv);
> +	else
> +		rzg3e_thermal_hw_enable(priv);
> +
> +	priv->mode = mode;
> +	return 0;
> +}
> +
> +static const struct thermal_zone_device_ops rzg3e_tz_of_ops = {
> +	.get_temp = rzg3e_thermal_get_temp,
> +	.set_trips = rzg3e_thermal_set_trips,
> +	.change_mode = rzg3e_thermal_change_mode,
> +};
> +
> +static int rzg3e_thermal_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct rzg3e_thermal_priv *priv;
> +	struct reset_control *rstc;
> +	char *adc_name, *cmp_name;
> +	int adc_irq, cmp_irq;
> +	struct clk *clk;
> +	int ret;
> +
> +	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +
> +	priv->dev = dev;
> +
> +	priv->base = devm_platform_ioremap_resource(pdev, 0);
> +	if (IS_ERR(priv->base))
> +		return dev_err_probe(dev, PTR_ERR(priv->base),
> +				"Failed to map I/O memory");
> +
> +	priv->syscon = syscon_regmap_lookup_by_phandle(dev->of_node,
> +						       "renesas,tsu-calibration-sys");
> +	if (IS_ERR(priv->syscon))
> +		return dev_err_probe(dev, PTR_ERR(priv->syscon),
> +				"Failed to get calibration syscon");
> +
> +	adc_irq = platform_get_irq_byname(pdev, "adi");
> +	if (adc_irq < 0)
> +		return adc_irq;
> +
> +	cmp_irq = platform_get_irq_byname(pdev, "adcmpi");
> +	if (cmp_irq < 0)
> +		return cmp_irq;
> +
> +	rstc = devm_reset_control_get_exclusive_deasserted(dev, NULL);
> +	if (IS_ERR(rstc))
> +		return dev_err_probe(dev, PTR_ERR(rstc),
> +				     "Failed to acquire deasserted reset");
> +
> +	platform_set_drvdata(pdev, priv);
> +
> +	spin_lock_init(&priv->reg_lock);
> +	init_completion(&priv->conv_complete);
> +
> +	clk = devm_clk_get_enabled(dev, NULL);
> +	if (IS_ERR(clk))
> +		return dev_err_probe(dev, PTR_ERR(clk),
> +				     "Failed to get and enable clock");
> +
> +	if (clk_get_rate(clk) < TSU_MIN_CLOCK_RATE)
> +		return dev_err_probe(dev, -EINVAL,
> +				     "Clock rate too low (minimum %d Hz required)",
> +				     TSU_MIN_CLOCK_RATE);
> +
> +	ret = rzg3e_thermal_get_trimming(priv);
> +	if (ret)
> +		return ret;
> +
> +	adc_name = devm_kasprintf(dev, GFP_KERNEL, "%s-adc", dev_name(dev));
> +	if (!adc_name)
> +		return -ENOMEM;
> +
> +	cmp_name = devm_kasprintf(dev, GFP_KERNEL, "%s-cmp", dev_name(dev));
> +	if (!cmp_name)
> +		return -ENOMEM;
> +
> +	/* Unit in a known disabled mode */
> +	rzg3e_thermal_hw_disable(priv);
> +
> +	ret = devm_request_irq(dev, adc_irq, rzg3e_thermal_adc_irq,
> +			       IRQF_TRIGGER_RISING, adc_name, priv);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "Failed to request ADC IRQ");
> +
> +	ret = devm_request_threaded_irq(dev, cmp_irq, rzg3e_thermal_cmp_irq,
> +					rzg3e_thermal_cmp_threaded_irq,
> +					IRQF_TRIGGER_HIGH | IRQF_ONESHOT,
> +					cmp_name, priv);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "Failed to request comparison IRQ");
> +
> +	/* Register Thermal Zone */
> +	priv->zone = devm_thermal_of_zone_register(dev, 0, priv, &rzg3e_tz_of_ops);
> +	if (IS_ERR(priv->zone))
> +		return dev_err_probe(dev, PTR_ERR(priv->zone),
> +				"Failed to register thermal zone");
> +
> +	ret = devm_thermal_add_hwmon_sysfs(dev, priv->zone);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "Failed to add hwmon sysfs");
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id rzg3e_thermal_dt_ids[] = {
> +	{ .compatible = "renesas,r9a09g047-tsu" },
> +	{ /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, rzg3e_thermal_dt_ids);
> +
> +static struct platform_driver rzg3e_thermal_driver = {
> +	.driver = {
> +		.name	= "rzg3e_thermal",
> +		.of_match_table = rzg3e_thermal_dt_ids,
> +	},
> +	.probe = rzg3e_thermal_probe,
> +};
> +module_platform_driver(rzg3e_thermal_driver);
> +
> +MODULE_DESCRIPTION("Renesas RZ/G3E TSU Thermal Sensor Driver");
> +MODULE_AUTHOR("John Madieu <john.madieu.xa@bp.renesas.com>");
> +MODULE_LICENSE("GPL");
> -- 
> 2.25.1
> 

-- 

 <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 v6 3/5] thermal: renesas: rzg3e: Add thermal driver for the Renesas RZ/G3E SoC

[John Madieu]

Hi Daniel,

Thanks for your review.

> -----Original Message-----
> From: Daniel Lezcano <daniel.lezcano@linaro.org>
> Sent: Wednesday, July 16, 2025 11:11 PM
> To: John Madieu <john.madieu.xa@bp.renesas.com>
> Subject: Re: [PATCH v6 3/5] thermal: renesas: rzg3e: Add thermal driver
> for the Renesas RZ/G3E SoC
> 
> On Thu, May 22, 2025 at 08:22:46PM +0200, John Madieu wrote:
> > The RZ/G3E SoC integrates a Temperature Sensor Unit (TSU) block
> > designed to monitor the chip's junction temperature. This sensor is
> > connected to channel 1 of the APB port clock/reset and provides
> temperature measurements.
> >
> > It also requires calibration values stored in the system controller
> > registers for accurate temperature measurement. Add a driver for the
> Renesas RZ/G3E TSU.
> >
> > Signed-off-by: John Madieu <john.madieu.xa@bp.renesas.com>
> > ---
> >
> > Changes:
> >
> > v1 -> v2: fixes IRQ names
> > v2 -> v3: no changes
> > v3 -> v4: no changes
> > v5: removes curly braces arround single-line protected scoped guards
> > v6: Clarified comments in driver
> >
> >  MAINTAINERS                             |   7 +
> >  drivers/thermal/renesas/Kconfig         |   7 +
> >  drivers/thermal/renesas/Makefile        |   1 +
> >  drivers/thermal/renesas/rzg3e_thermal.c | 443
> > ++++++++++++++++++++++++
> >  4 files changed, 458 insertions(+)
> >  create mode 100644 drivers/thermal/renesas/rzg3e_thermal.c
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS index
> > 79a8e2c73908..eb11494795e8 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -21161,6 +21161,13 @@ S:	Maintained
> >  F:
> 	Documentation/devicetree/bindings/iio/potentiometer/renesas,x9250.ya
> ml
> >  F:	drivers/iio/potentiometer/x9250.c
> >
> > +RENESAS RZ/G3E THERMAL SENSOR UNIT DRIVER
> > +M:	John Madieu <john.madieu.xa@bp.renesas.com>
> > +L:	linux-pm@vger.kernel.org
> > +S:	Maintained
> > +F:	Documentation/devicetree/bindings/thermal/renesas,r9a09g047-tsu.yaml
> > +F:	drivers/thermal/renesas/rzg3e_thermal.c
> > +
> >  RESET CONTROLLER FRAMEWORK
> >  M:	Philipp Zabel <p.zabel@pengutronix.de>
> >  S:	Maintained
> > diff --git a/drivers/thermal/renesas/Kconfig
> > b/drivers/thermal/renesas/Kconfig index dcf5fc5ae08e..10cf90fc4bfa
> > 100644
> > --- a/drivers/thermal/renesas/Kconfig
> > +++ b/drivers/thermal/renesas/Kconfig
> > @@ -26,3 +26,10 @@ config RZG2L_THERMAL
> >  	help
> >  	  Enable this to plug the RZ/G2L thermal sensor driver into the
> Linux
> >  	  thermal framework.
> > +
> > +config RZG3E_THERMAL
> > +	tristate "Renesas RZ/G3E thermal driver"
> > +	depends on ARCH_RENESAS || COMPILE_TEST
> > +	help
> > +	  Enable this to plug the RZ/G3E thermal sensor driver into the
> Linux
> > +	  thermal framework.
> > diff --git a/drivers/thermal/renesas/Makefile
> > b/drivers/thermal/renesas/Makefile
> > index bf9cb3cb94d6..5a3eba0dedd0 100644
> > --- a/drivers/thermal/renesas/Makefile
> > +++ b/drivers/thermal/renesas/Makefile
> > @@ -3,3 +3,4 @@
> >  obj-$(CONFIG_RCAR_GEN3_THERMAL)	+= rcar_gen3_thermal.o
> >  obj-$(CONFIG_RCAR_THERMAL)	+= rcar_thermal.o
> >  obj-$(CONFIG_RZG2L_THERMAL)	+= rzg2l_thermal.o
> > +obj-$(CONFIG_RZG3E_THERMAL)	+= rzg3e_thermal.o
> > diff --git a/drivers/thermal/renesas/rzg3e_thermal.c
> > b/drivers/thermal/renesas/rzg3e_thermal.c
> > new file mode 100644
> > index 000000000000..348229da9ef4
> > --- /dev/null
> > +++ b/drivers/thermal/renesas/rzg3e_thermal.c
> > @@ -0,0 +1,443 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Renesas RZ/G3E TSU Temperature Sensor Unit
> > + *
> > + * Copyright (C) 2025 Renesas Electronics Corporation  */ #include
> > +<linux/clk.h> #include <linux/delay.h> #include <linux/err.h>
> > +#include <linux/interrupt.h> #include <linux/io.h> #include
> > +<linux/iopoll.h> #include <linux/kernel.h> #include
> > +<linux/mfd/syscon.h> #include <linux/module.h> #include
> > +<linux/of_device.h> #include <linux/platform_device.h> #include
> > +<linux/regmap.h> #include <linux/reset.h> #include <linux/thermal.h>
> > +#include <linux/units.h>
> > +
> > +#include "../thermal_hwmon.h"
> > +
> > +/* SYS Trimming register offsets macro */ #define SYS_TSU_TRMVAL(x)
> > +(0x330 + (x) * 4)
> > +
> > +/* TSU Register offsets and bits */
> > +#define TSU_SSUSR		0x00
> > +#define TSU_SSUSR_EN_TS		BIT(0)
> > +#define TSU_SSUSR_ADC_PD_TS	BIT(1)
> > +#define TSU_SSUSR_SOC_TS_EN	BIT(2)
> > +
> > +#define TSU_STRGR		0x04
> > +#define TSU_STRGR_ADST		BIT(0)
> > +
> > +#define TSU_SOSR1		0x08
> > +#define TSU_SOSR1_ADCT_8	0x03
> > +#define TSU_SOSR1_OUTSEL_AVERAGE	BIT(9)
> > +
> > +/* Sensor Code Read Register */
> > +#define TSU_SCRR		0x10
> > +#define TSU_SCRR_OUT12BIT_TS	GENMASK(11, 0)
> > +
> > +/* Sensor Status Register */
> > +#define TSU_SSR			0x14
> > +#define TSU_SSR_CONV_RUNNING	BIT(0)
> > +
> > +/* Compare Mode Setting Register */
> > +#define TSU_CMSR		0x18
> > +#define TSU_CMSR_CMPEN		BIT(0)
> > +#define TSU_CMSR_CMPCOND	BIT(1)
> > +
> > +/* Lower Limit Setting Register */
> > +#define TSU_LLSR		0x1C
> > +#define TSU_LLSR_LIM		GENMASK(11, 0)
> > +
> > +/* Upper Limit Setting Register */
> > +#define TSU_ULSR		0x20
> > +#define TSU_ULSR_ULIM		GENMASK(11, 0)
> > +
> > +/* Interrupt Status Register */
> > +#define TSU_SISR		0x30
> > +#define TSU_SISR_ADF		BIT(0)
> > +#define TSU_SISR_CMPF		BIT(1)
> > +
> > +/* Interrupt Enable Register */
> > +#define TSU_SIER		0x34
> > +#define TSU_SIER_ADIE		BIT(0)
> > +#define TSU_SIER_CMPIE		BIT(1)
> > +
> > +/* Interrupt Clear Register */
> > +#define TSU_SICR		0x38
> > +#define TSU_SICR_ADCLR		BIT(0)
> > +#define TSU_SICR_CMPCLR		BIT(1)
> > +
> > +/* Temperature calculation constants */
> > +#define TSU_D		41
> > +#define TSU_E		126
> > +#define TSU_TRMVAL_MASK	GENMASK(11, 0)
> > +
> > +#define TSU_POLL_DELAY_US	50
> > +#define TSU_TIMEOUT_US		10000
> > +#define TSU_MIN_CLOCK_RATE	24000000
> > +
> > +/**
> > + * struct rzg3e_thermal_priv - RZ/G3E thermal private data structure
> > + * @base: TSU base address
> > + * @dev: device pointer
> > + * @syscon: regmap for calibration values
> > + * @zone: thermal zone pointer
> > + * @mode: current tzd mode
> > + * @conv_complete: ADC conversion completion
> > + * @reg_lock: protect shared register access
> > + * @cached_temp: last computed temperature (milliCelsius)
> > + * @trmval: trim (calibration) values  */ struct rzg3e_thermal_priv {
> > +	void __iomem *base;
> > +	struct device *dev;
> > +	struct regmap *syscon;
> > +	struct thermal_zone_device *zone;
> > +	enum thermal_device_mode mode;
> > +	struct completion conv_complete;
> > +	spinlock_t reg_lock;
> > +	int cached_temp;
> > +	u32 trmval[2];
> > +};
> > +
> > +static void rzg3e_thermal_hw_disable(struct rzg3e_thermal_priv *priv)
> > +{
> > +	/* Disable all interrupts first */
> > +	writel(0, priv->base + TSU_SIER);
> > +	/* Clear any pending interrupts */
> > +	writel(TSU_SICR_ADCLR | TSU_SICR_CMPCLR, priv->base + TSU_SICR);
> > +	/* Put device in power down */
> > +	writel(TSU_SSUSR_ADC_PD_TS, priv->base + TSU_SSUSR); }
> > +
> > +static void rzg3e_thermal_hw_enable(struct rzg3e_thermal_priv *priv)
> > +{
> > +	/* First clear any pending status */
> > +	writel(TSU_SICR_ADCLR | TSU_SICR_CMPCLR, priv->base + TSU_SICR);
> > +	/* Disable all interrupts */
> > +	writel(0, priv->base + TSU_SIER);
> > +
> > +	/* Enable thermal sensor */
> > +	writel(TSU_SSUSR_SOC_TS_EN | TSU_SSUSR_EN_TS, priv->base +
> TSU_SSUSR);
> > +	/* Setup for averaging mode with 8 samples */
> > +	writel(TSU_SOSR1_OUTSEL_AVERAGE | TSU_SOSR1_ADCT_8, priv->base +
> > +TSU_SOSR1); }
> > +
> > +static irqreturn_t rzg3e_thermal_cmp_irq(int irq, void *dev_id) {
> > +	struct rzg3e_thermal_priv *priv = dev_id;
> > +	u32 status;
> > +
> > +	status = readl(priv->base + TSU_SISR);
> > +	if (!(status & TSU_SISR_CMPF))
> > +		return IRQ_NONE;
> > +
> > +	/* Clear the comparison interrupt flag */
> > +	writel(TSU_SICR_CMPCLR, priv->base + TSU_SICR);
> > +
> > +	return IRQ_WAKE_THREAD;
> > +}
> > +
> > +static irqreturn_t rzg3e_thermal_cmp_threaded_irq(int irq, void
> > +*dev_id) {
> > +	struct rzg3e_thermal_priv *priv = dev_id;
> > +
> > +	thermal_zone_device_update(priv->zone, THERMAL_EVENT_UNSPECIFIED);
> > +	return IRQ_HANDLED;
> > +}
> > +
> > +static irqreturn_t rzg3e_thermal_adc_irq(int irq, void *dev_id) {
> > +	struct rzg3e_thermal_priv *priv = dev_id;
> > +	u32 status;
> > +	u32 result;
> > +
> > +	/* Check if this is our interrupt */
> > +	status = readl(priv->base + TSU_SISR);
> > +	if (!(status & TSU_SISR_ADF))
> > +		return IRQ_NONE;
> > +
> > +	/* Disable all interrupts */
> > +	writel(0, priv->base + TSU_SIER);
> > +	/* Clear conversion complete interrupt */
> > +	writel(TSU_SICR_ADCLR, priv->base + TSU_SICR);
> > +
> > +	/* Read ADC conversion result */
> > +	result = readl(priv->base + TSU_SCRR) & TSU_SCRR_OUT12BIT_TS;
> > +
> > +	/*
> > +	 * Calculate temperature using compensation formula
> > +	 * Section 7.11.7.8 (Temperature Compensation Calculation)
> > +	 *
> > +	 * T(°C) = ((e - d) / (c -b)) * (a - b) + d
> > +	 *
> > +	 * a = 12 bits temperature code read from the sensor
> > +	 * b = SYS trmval[0]
> > +	 * c = SYS trmval[1]
> > +	 * d = -41
> > +	 * e = 126
> > +	 */
> > +	s64 temp_val = div_s64(((TSU_E + TSU_D) * (s64)(result - priv-
> >trmval[0])),
> > +				priv->trmval[1] - priv->trmval[0]) - TSU_D;
> > +	int new_temp = temp_val * MILLIDEGREE_PER_DEGREE;
> > +
> > +	scoped_guard(spinlock_irqsave, &priv->reg_lock)
> > +		priv->cached_temp = new_temp;
> > +
> > +	complete(&priv->conv_complete);
> > +
> > +	return IRQ_HANDLED;
> > +}
> > +
> > +static int rzg3e_thermal_get_temp(struct thermal_zone_device *zone,
> > +int *temp) {
> > +	struct rzg3e_thermal_priv *priv = thermal_zone_device_priv(zone);
> > +	u32 val;
> > +	int ret;
> > +
> > +	if (priv->mode == THERMAL_DEVICE_DISABLED)
> > +		return -EBUSY;
> 
> Why ?
> 

As I kept internal reference to the device enablement state, I
thought it was necessary to check it here to ensure hardware
is not accessed inappropriately. After double checking I could see
that the thermal core already handles it.

Will remove in next version.

> > +	reinit_completion(&priv->conv_complete);
> > +
> > +	/* Enable ADC interrupt */
> > +	writel(TSU_SIER_ADIE, priv->base + TSU_SIER);
> 
> Why enable irq here ?
> 

I did it this way because, in 'set_trips' callback, the
driver does trigger conversion to check whether the current
temperature is part of the window or not, and triggers the
comparison interrupt accordingly. Because of that, I did not
want the conversion-complete interrupt to also be triggered.

That's the reason why I enable conversion-complete interrupt
in 'get_temp', to make sure its interrupt is being triggered
only when the thermal core calls it.

Should I do it another way ?


> > +	/* Verify no ongoing conversion */
> > +	ret = readl_poll_timeout_atomic(priv->base + TSU_SSR, val,
> > +					!(val & TSU_SSR_CONV_RUNNING),
> > +					TSU_POLL_DELAY_US, TSU_TIMEOUT_US);
> > +	if (ret) {
> > +		dev_err(priv->dev, "ADC conversion timed out\n");
> > +		return ret;
> > +	}
> > +
> > +	/* Start conversion */
> > +	writel(TSU_STRGR_ADST, priv->base + TSU_STRGR);
> > +
> > +	if (!wait_for_completion_timeout(&priv->conv_complete,
> > +					 msecs_to_jiffies(100))) {
> > +		dev_err(priv->dev, "ADC conversion completion timeout\n");
> > +		return -ETIMEDOUT;
> > +	}
> 
> Can you explain what is happening here ?
> 

I might not get what you are asking, but since I compute the
temperature in the hard IRQ handler, I just wait for it to complete
and notify the completion so I can grab the processed value to notify
the thermal core.

Please let me know if this does not answer your question.

> > +	scoped_guard(spinlock_irqsave, &priv->reg_lock)
> > +		*temp = priv->cached_temp;
> > +
> > +	return 0;
> > +}
> > +
> > +/* Convert temperature in milliCelsius to raw sensor code */ static
> > +int rzg3e_temp_to_raw(struct rzg3e_thermal_priv *priv, int temp_mc) {
> > +	s64 raw = div_s64(((temp_mc / 1000) - TSU_D) *
> > +			  (priv->trmval[1] - priv->trmval[0]),
> > +			  (TSU_E - TSU_D));
> > +	return clamp_val(raw, 0, 0xFFF);
> > +}
> > +
> > +static int rzg3e_thermal_set_trips(struct thermal_zone_device *tz,
> > +int low, int high) {
> > +	struct rzg3e_thermal_priv *priv = thermal_zone_device_priv(tz);
> > +	int ret;
> > +	int val;
> > +
> > +	if (low >= high)
> > +		return -EINVAL;
> > +
> > +	if (priv->mode == THERMAL_DEVICE_DISABLED)
> > +		return -EBUSY;
> 
> That is not supposed to happen. set_trips is called from
> thermal_zone_device_update but the thermal zone is disabled, the function
> bails out, thus it should not call this callback.
> 

Got it. I'll remove it in the next series.

> > +	/* Set up comparison interrupt */
> > +	writel(0, priv->base + TSU_SIER);
> > +	writel(TSU_SICR_ADCLR | TSU_SICR_CMPCLR, priv->base + TSU_SICR);
> > +
> > +	/* Set thresholds */
> > +	writel(rzg3e_temp_to_raw(priv, low), priv->base + TSU_LLSR);
> > +	writel(rzg3e_temp_to_raw(priv, high), priv->base + TSU_ULSR);
> > +
> > +	/* Configure comparison:
> > +	 * - Enable comparison function (CMPEN = 1)
> > +	 * - Set comparison condition (CMPCOND = 0 for out of range)
> > +	 */
> > +	writel(TSU_CMSR_CMPEN, priv->base + TSU_CMSR);
> > +
> > +	/* Enable comparison irq */
> > +	writel(TSU_SIER_CMPIE, priv->base + TSU_SIER);
> > +
> > +	/* Verify no ongoing conversion */
> > +	ret = readl_poll_timeout_atomic(priv->base + TSU_SSR, val,
> > +					!(val & TSU_SSR_CONV_RUNNING),
> > +					TSU_POLL_DELAY_US, TSU_TIMEOUT_US);
> > +	if (ret) {
> > +		dev_err(priv->dev, "ADC conversion timed out\n");
> > +		return ret;
> > +	}
> > +
> > +	/* Start a conversion to trigger comparison */
> > +	writel(TSU_STRGR_ADST, priv->base + TSU_STRGR);
> > +
> > +	return 0;
> > +}
> > +
> > +static int rzg3e_thermal_get_trimming(struct rzg3e_thermal_priv
> > +*priv) {
> > +	int ret;
> > +
> > +	ret = regmap_read(priv->syscon, SYS_TSU_TRMVAL(0), &priv-
> >trmval[0]);
> > +	if (ret)
> > +		return ret;
> > +
> > +	ret = regmap_read(priv->syscon, SYS_TSU_TRMVAL(1), &priv-
> >trmval[1]);
> > +	if (ret)
> > +		return ret;
> > +
> > +	priv->trmval[0] &= TSU_TRMVAL_MASK;
> > +	priv->trmval[1] &= TSU_TRMVAL_MASK;
> > +
> > +	if (!priv->trmval[0] || !priv->trmval[1])
> > +		return dev_err_probe(priv->dev, -EINVAL, "invalid trimming
> > +values");
> > +
> > +	return 0;
> > +}
> > +
> > +static int rzg3e_thermal_change_mode(struct thermal_zone_device *tz,
> > +				     enum thermal_device_mode mode) {
> > +	struct rzg3e_thermal_priv *priv = thermal_zone_device_priv(tz);
> > +
> > +	if (mode == THERMAL_DEVICE_DISABLED)
> > +		rzg3e_thermal_hw_disable(priv);
> > +	else
> > +		rzg3e_thermal_hw_enable(priv);
> > +
> > +	priv->mode = mode;
> > +	return 0;
> > +}
> > +
> > +static const struct thermal_zone_device_ops rzg3e_tz_of_ops = {
> > +	.get_temp = rzg3e_thermal_get_temp,
> > +	.set_trips = rzg3e_thermal_set_trips,
> > +	.change_mode = rzg3e_thermal_change_mode, };
> > +
> > +static int rzg3e_thermal_probe(struct platform_device *pdev) {
> > +	struct device *dev = &pdev->dev;
> > +	struct rzg3e_thermal_priv *priv;
> > +	struct reset_control *rstc;
> > +	char *adc_name, *cmp_name;
> > +	int adc_irq, cmp_irq;
> > +	struct clk *clk;
> > +	int ret;
> > +
> > +	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> > +	if (!priv)
> > +		return -ENOMEM;
> > +
> > +	priv->dev = dev;
> > +
> > +	priv->base = devm_platform_ioremap_resource(pdev, 0);
> > +	if (IS_ERR(priv->base))
> > +		return dev_err_probe(dev, PTR_ERR(priv->base),
> > +				"Failed to map I/O memory");
> > +
> > +	priv->syscon = syscon_regmap_lookup_by_phandle(dev->of_node,
> > +						       "renesas,tsu-calibration-sys");
> > +	if (IS_ERR(priv->syscon))
> > +		return dev_err_probe(dev, PTR_ERR(priv->syscon),
> > +				"Failed to get calibration syscon");
> > +
> > +	adc_irq = platform_get_irq_byname(pdev, "adi");
> > +	if (adc_irq < 0)
> > +		return adc_irq;
> > +
> > +	cmp_irq = platform_get_irq_byname(pdev, "adcmpi");
> > +	if (cmp_irq < 0)
> > +		return cmp_irq;
> > +
> > +	rstc = devm_reset_control_get_exclusive_deasserted(dev, NULL);
> > +	if (IS_ERR(rstc))
> > +		return dev_err_probe(dev, PTR_ERR(rstc),
> > +				     "Failed to acquire deasserted reset");
> > +
> > +	platform_set_drvdata(pdev, priv);
> > +
> > +	spin_lock_init(&priv->reg_lock);
> > +	init_completion(&priv->conv_complete);
> > +
> > +	clk = devm_clk_get_enabled(dev, NULL);
> > +	if (IS_ERR(clk))
> > +		return dev_err_probe(dev, PTR_ERR(clk),
> > +				     "Failed to get and enable clock");
> > +
> > +	if (clk_get_rate(clk) < TSU_MIN_CLOCK_RATE)
> > +		return dev_err_probe(dev, -EINVAL,
> > +				     "Clock rate too low (minimum %d Hz
> required)",
> > +				     TSU_MIN_CLOCK_RATE);
> > +
> > +	ret = rzg3e_thermal_get_trimming(priv);
> > +	if (ret)
> > +		return ret;
> > +
> > +	adc_name = devm_kasprintf(dev, GFP_KERNEL, "%s-adc", dev_name(dev));
> > +	if (!adc_name)
> > +		return -ENOMEM;
> > +
> > +	cmp_name = devm_kasprintf(dev, GFP_KERNEL, "%s-cmp", dev_name(dev));
> > +	if (!cmp_name)
> > +		return -ENOMEM;
> > +
> > +	/* Unit in a known disabled mode */
> > +	rzg3e_thermal_hw_disable(priv);
> > +
> > +	ret = devm_request_irq(dev, adc_irq, rzg3e_thermal_adc_irq,
> > +			       IRQF_TRIGGER_RISING, adc_name, priv);
> > +	if (ret)
> > +		return dev_err_probe(dev, ret, "Failed to request ADC IRQ");
> > +
> > +	ret = devm_request_threaded_irq(dev, cmp_irq, rzg3e_thermal_cmp_irq,
> > +					rzg3e_thermal_cmp_threaded_irq,
> > +					IRQF_TRIGGER_HIGH | IRQF_ONESHOT,
> > +					cmp_name, priv);
> > +	if (ret)
> > +		return dev_err_probe(dev, ret, "Failed to request comparison
> IRQ");
> > +
> > +	/* Register Thermal Zone */
> > +	priv->zone = devm_thermal_of_zone_register(dev, 0, priv,
> &rzg3e_tz_of_ops);
> > +	if (IS_ERR(priv->zone))
> > +		return dev_err_probe(dev, PTR_ERR(priv->zone),
> > +				"Failed to register thermal zone");
> > +
> > +	ret = devm_thermal_add_hwmon_sysfs(dev, priv->zone);
> > +	if (ret)
> > +		return dev_err_probe(dev, ret, "Failed to add hwmon sysfs");
> > +
> > +	return 0;
> > +}
> > +
> > +static const struct of_device_id rzg3e_thermal_dt_ids[] = {
> > +	{ .compatible = "renesas,r9a09g047-tsu" },
> > +	{ /* sentinel */ }
> > +};
> > +MODULE_DEVICE_TABLE(of, rzg3e_thermal_dt_ids);
> > +
> > +static struct platform_driver rzg3e_thermal_driver = {
> > +	.driver = {
> > +		.name	= "rzg3e_thermal",
> > +		.of_match_table = rzg3e_thermal_dt_ids,
> > +	},
> > +	.probe = rzg3e_thermal_probe,
> > +};
> > +module_platform_driver(rzg3e_thermal_driver);
> > +
> > +MODULE_DESCRIPTION("Renesas RZ/G3E TSU Thermal Sensor Driver");
> > +MODULE_AUTHOR("John Madieu <john.madieu.xa@bp.renesas.com>");
> > +MODULE_LICENSE("GPL");
> > --
> > 2.25.1
> >
> 
> --
> 
>  <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

Regards,
John

Re: [PATCH v6 1/5] soc: renesas: rz-sysc: Add syscon/regmap support

[Geert Uytterhoeven]

Hi John,

On Thu, 22 May 2025 at 20:23, John Madieu <john.madieu.xa@bp.renesas.com> wrote:
> The RZ/G3E system controller has various registers that control or report
> some properties specific to individual IPs. The regmap is registered as a
> syscon device to allow these IP drivers to access the registers through the
> regmap API.
>
> As other RZ SoCs might have custom read/write callbacks or max-offsets,
> add register a custom regmap configuration.
>
> Signed-off-by: John Madieu <john.madieu.xa@bp.renesas.com>
> [claudiu.beznea:
>  - s/rzg3e_sysc_regmap/rzv2h_sysc_regmap in RZ/V2H sysc
>    file
>  - do not check the match->data validity in rz_sysc_probe() as it is
>    always valid
>  - register the regmap if data->regmap_cfg is valid]
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

Thanks for your patch!

As you probably noticed, I am not a big fan of syscon, so I try
to avoid getting involved with syscon patches as much as possible.
But this patch has appeared in multiple series, so I am afraid I cannot
avoid it anymore ;-)

> --- a/drivers/soc/renesas/r9a08g045-sysc.c
> +++ b/drivers/soc/renesas/r9a08g045-sysc.c
> @@ -18,6 +18,16 @@ static const struct rz_sysc_soc_id_init_data rzg3s_sysc_soc_id_init_data __initc
>         .specific_id_mask = GENMASK(27, 0),
>  };
>
> +static const struct regmap_config rzg3s_sysc_regmap __initconst = {
> +       .name = "rzg3s_sysc_regs",
> +       .reg_bits = 32,
> +       .reg_stride = 4,
> +       .val_bits = 32,
> +       .fast_io = true,
> +       .max_register = 0xe20,
> +};

Struct regmap_config is a rather large structure, and the only
SoC-specific members are the .name (which doesn't really matter) and
.max_register members.  Hence please move .max_register back to struct
rz_sysc_init_data (like in v5), and allocate struct regmap_config
dynamically (by embedding it into struct rz_sysc).

> +
>  const struct rz_sysc_init_data rzg3s_sysc_init_data __initconst = {
>         .soc_id_init_data = &rzg3s_sysc_soc_id_init_data,
> +       .regmap_cfg = &rzg3s_sysc_regmap,
>  };

> --- a/drivers/soc/renesas/rz-sysc.c
> +++ b/drivers/soc/renesas/rz-sysc.c
> @@ -117,7 +124,15 @@ static int rz_sysc_probe(struct platform_device *pdev)
>                 return PTR_ERR(sysc->base);
>
>         sysc->dev = dev;
> -       return rz_sysc_soc_init(sysc, match);
> +       ret = rz_sysc_soc_init(sysc, match);
> +       if (ret || !data->regmap_cfg)

data->regmap_cfg should never be NULL (but this doesn't matter anymore,
in the context of my request above).

> +               return ret;
> +
> +       regmap = devm_regmap_init_mmio(dev, sysc->base, data->regmap_cfg);
> +       if (IS_ERR(regmap))
> +               return PTR_ERR(regmap);
> +
> +       return of_syscon_register_regmap(dev->of_node, regmap);
>  }

The rest LGTM.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH v6 3/5] thermal: renesas: rzg3e: Add thermal driver for the Renesas RZ/G3E SoC

[Geert Uytterhoeven]

Hi John,

On Thu, 22 May 2025 at 20:23, John Madieu <john.madieu.xa@bp.renesas.com> wrote:
> The RZ/G3E SoC integrates a Temperature Sensor Unit (TSU) block designed
> to monitor the chip's junction temperature. This sensor is connected to
> channel 1 of the APB port clock/reset and provides temperature measurements.
>
> It also requires calibration values stored in the system controller registers
> for accurate temperature measurement. Add a driver for the Renesas RZ/G3E TSU.
>
> Signed-off-by: John Madieu <john.madieu.xa@bp.renesas.com>

Thanks for your patch!

The TSUs in RZ/V2H and RZ/V2N seem to be identical to the one in RZ/G3E.
However, RZ/V2H and RZ/V2N have two instances, while RZ/G3 has only one.

> --- /dev/null
> +++ b/drivers/thermal/renesas/rzg3e_thermal.c
> @@ -0,0 +1,443 @@

> +/* SYS Trimming register offsets macro */
> +#define SYS_TSU_TRMVAL(x) (0x330 + (x) * 4)

RZ/V2H and RZ/V2N have a second set of trim values for the second
TSU instance.  So I guess you want to specify the offset in DT instead.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH v6 3/5] thermal: renesas: rzg3e: Add thermal driver for the Renesas RZ/G3E SoC

[Daniel Lezcano]

On 31/07/2025 19:19, John Madieu wrote:
> Hi Daniel,
> 
> Thanks for your review.
> 
>> -----Original Message-----
>> From: Daniel Lezcano <daniel.lezcano@linaro.org>
>> Sent: Wednesday, July 16, 2025 11:11 PM
>> To: John Madieu <john.madieu.xa@bp.renesas.com>
>> Subject: Re: [PATCH v6 3/5] thermal: renesas: rzg3e: Add thermal driver
>> for the Renesas RZ/G3E SoC
>>
>> On Thu, May 22, 2025 at 08:22:46PM +0200, John Madieu wrote:
>>> The RZ/G3E SoC integrates a Temperature Sensor Unit (TSU) block
>>> designed to monitor the chip's junction temperature. This sensor is
>>> connected to channel 1 of the APB port clock/reset and provides
>> temperature measurements.
>>>
>>> It also requires calibration values stored in the system controller
>>> registers for accurate temperature measurement. Add a driver for the
>> Renesas RZ/G3E TSU.
>>>
>>> Signed-off-by: John Madieu <john.madieu.xa@bp.renesas.com>
>>> ---

[ ... ]

>>> +static int rzg3e_thermal_get_temp(struct thermal_zone_device *zone,
>>> +int *temp) {
>>> +	struct rzg3e_thermal_priv *priv = thermal_zone_device_priv(zone);
>>> +	u32 val;
>>> +	int ret;
>>> +
>>> +	if (priv->mode == THERMAL_DEVICE_DISABLED)
>>> +		return -EBUSY;

[ ... ]

>>> +	reinit_completion(&priv->conv_complete);
>>> +
>>> +	/* Enable ADC interrupt */
>>> +	writel(TSU_SIER_ADIE, priv->base + TSU_SIER);
>>
>> Why enable irq here ?
>>
> 
> I did it this way because, in 'set_trips' callback, the
> driver does trigger conversion to check whether the current
> temperature is part of the window or not, and triggers the
> comparison interrupt accordingly. Because of that, I did not
> want the conversion-complete interrupt to also be triggered.
> 
> That's the reason why I enable conversion-complete interrupt
> in 'get_temp', to make sure its interrupt is being triggered
> only when the thermal core calls it.
> 
> Should I do it another way ?

I don't ATM, the approach is very unusual so I'm still trying to figure 
out what is this completion approach and readl_poll_timeout_atomic. At 
the first glance I would say it is wrong.


>>> +	/* Verify no ongoing conversion */
>>> +	ret = readl_poll_timeout_atomic(priv->base + TSU_SSR, val,
>>> +					!(val & TSU_SSR_CONV_RUNNING),
>>> +					TSU_POLL_DELAY_US, TSU_TIMEOUT_US);
>>> +	if (ret) {
>>> +		dev_err(priv->dev, "ADC conversion timed out\n");
>>> +		return ret;
>>> +	}
>>> +
>>> +	/* Start conversion */
>>> +	writel(TSU_STRGR_ADST, priv->base + TSU_STRGR);
>>> +
>>> +	if (!wait_for_completion_timeout(&priv->conv_complete,
>>> +					 msecs_to_jiffies(100))) {
>>> +		dev_err(priv->dev, "ADC conversion completion timeout\n");
>>> +		return -ETIMEDOUT;
>>> +	}
>>
>> Can you explain what is happening here ?
>>
> 
> I might not get what you are asking, but since I compute the
> temperature in the hard IRQ handler, I just wait for it to complete
> and notify the completion so I can grab the processed value to notify
> the thermal core.
> 
> Please let me know if this does not answer your question.

Can you describe how the sensor works ? And perhaps if you have a 
pointer to some documentation ?
  [ ... ]

-- 
<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 v6 3/5] thermal: renesas: rzg3e: Add thermal driver for the Renesas RZ/G3E SoC

[John Madieu]

Hi Danoel,

> -----Original Message-----
> From: Daniel Lezcano <daniel.lezcano@linaro.org>
> Sent: Monday, August 4, 2025 6:08 PM
> To: John Madieu <john.madieu.xa@bp.renesas.com>
> Subject: Re: [PATCH v6 3/5] thermal: renesas: rzg3e: Add thermal driver for
> the Renesas RZ/G3E SoC
> 
> On 31/07/2025 19:19, John Madieu wrote:
> > Hi Daniel,
> >
> > Thanks for your review.
> >
> >> -----Original Message-----
> >> From: Daniel Lezcano <daniel.lezcano@linaro.org>
> >> Sent: Wednesday, July 16, 2025 11:11 PM
> >> To: John Madieu <john.madieu.xa@bp.renesas.com>
> >> Subject: Re: [PATCH v6 3/5] thermal: renesas: rzg3e: Add thermal
> >> driver for the Renesas RZ/G3E SoC
> >>
> >> On Thu, May 22, 2025 at 08:22:46PM +0200, John Madieu wrote:
> >>> The RZ/G3E SoC integrates a Temperature Sensor Unit (TSU) block
> >>> designed to monitor the chip's junction temperature. This sensor is
> >>> connected to channel 1 of the APB port clock/reset and provides
> >> temperature measurements.
> >>>
> >>> It also requires calibration values stored in the system controller
> >>> registers for accurate temperature measurement. Add a driver for the
> >> Renesas RZ/G3E TSU.
> >>>
> >>> Signed-off-by: John Madieu <john.madieu.xa@bp.renesas.com>
> >>> ---
> 
> [ ... ]
> 
> >>> +static int rzg3e_thermal_get_temp(struct thermal_zone_device *zone,
> >>> +int *temp) {
> >>> +	struct rzg3e_thermal_priv *priv = thermal_zone_device_priv(zone);
> >>> +	u32 val;
> >>> +	int ret;
> >>> +
> >>> +	if (priv->mode == THERMAL_DEVICE_DISABLED)
> >>> +		return -EBUSY;
> 
> [ ... ]
> 
> >>> +	reinit_completion(&priv->conv_complete);
> >>> +
> >>> +	/* Enable ADC interrupt */
> >>> +	writel(TSU_SIER_ADIE, priv->base + TSU_SIER);
> >>
> >> Why enable irq here ?
> >>
> >
> > I did it this way because, in 'set_trips' callback, the driver does
> > trigger conversion to check whether the current temperature is part of
> > the window or not, and triggers the comparison interrupt accordingly.
> > Because of that, I did not want the conversion-complete interrupt to
> > also be triggered.
> >
> > That's the reason why I enable conversion-complete interrupt in
> > 'get_temp', to make sure its interrupt is being triggered only when
> > the thermal core calls it.
> >
> > Should I do it another way ?
> 
> I don't ATM, the approach is very unusual so I'm still trying to figure out
> what is this completion approach and readl_poll_timeout_atomic. At the
> first glance I would say it is wrong.
> 
> 
> >>> +	/* Verify no ongoing conversion */
> >>> +	ret = readl_poll_timeout_atomic(priv->base + TSU_SSR, val,
> >>> +					!(val & TSU_SSR_CONV_RUNNING),
> >>> +					TSU_POLL_DELAY_US, TSU_TIMEOUT_US);
> >>> +	if (ret) {
> >>> +		dev_err(priv->dev, "ADC conversion timed out\n");
> >>> +		return ret;
> >>> +	}
> >>> +
> >>> +	/* Start conversion */
> >>> +	writel(TSU_STRGR_ADST, priv->base + TSU_STRGR);
> >>> +
> >>> +	if (!wait_for_completion_timeout(&priv->conv_complete,
> >>> +					 msecs_to_jiffies(100))) {
> >>> +		dev_err(priv->dev, "ADC conversion completion timeout\n");
> >>> +		return -ETIMEDOUT;
> >>> +	}
> >>
> >> Can you explain what is happening here ?
> >>
> >
> > I might not get what you are asking, but since I compute the
> > temperature in the hard IRQ handler, I just wait for it to complete
> > and notify the completion so I can grab the processed value to notify
> > the thermal core.
> >
> > Please let me know if this does not answer your question.
> 
> Can you describe how the sensor works ? And perhaps if you have a pointer
> to some documentation ?

Here is the documentation [1]. The thermal device is referred to as TSU.

[1] https://www.renesas.com/en/document/mah/rzg3e-group-users-manual-hardware?r=25574493

>   [ ... ]
Regards,
John

RE: [PATCH v6 3/5] thermal: renesas: rzg3e: Add thermal driver for the Renesas RZ/G3E SoC

[John Madieu]

Hi Geert,

Thanks for your review!

> -----Original Message-----
> From: Geert Uytterhoeven <geert@linux-m68k.org>
> Sent: Monday, August 4, 2025 11:28 AM
> To: John Madieu <john.madieu.xa@bp.renesas.com>
> Subject: Re: [PATCH v6 3/5] thermal: renesas: rzg3e: Add thermal driver for
> the Renesas RZ/G3E SoC
> 
> Hi John,
> 
> On Thu, 22 May 2025 at 20:23, John Madieu <john.madieu.xa@bp.renesas.com>
> wrote:
> > The RZ/G3E SoC integrates a Temperature Sensor Unit (TSU) block
> > designed to monitor the chip's junction temperature. This sensor is
> > connected to channel 1 of the APB port clock/reset and provides
> temperature measurements.
> >
> > It also requires calibration values stored in the system controller
> > registers for accurate temperature measurement. Add a driver for the
> Renesas RZ/G3E TSU.
> >
> > Signed-off-by: John Madieu <john.madieu.xa@bp.renesas.com>
> 
> Thanks for your patch!
> 
> The TSUs in RZ/V2H and RZ/V2N seem to be identical to the one in RZ/G3E.
> However, RZ/V2H and RZ/V2N have two instances, while RZ/G3 has only one.
> 

This is true.

> > --- /dev/null
> > +++ b/drivers/thermal/renesas/rzg3e_thermal.c
> > @@ -0,0 +1,443 @@
> 
> > +/* SYS Trimming register offsets macro */ #define SYS_TSU_TRMVAL(x)
> > +(0x330 + (x) * 4)
> 
> RZ/V2H and RZ/V2N have a second set of trim values for the second TSU
> instance.  So I guess you want to specify the offset in DT instead.
> 

What do you think of 'renesas,tsu-channel' property or alike
Property to specify the channel being used ?

> Gr{oetje,eeting}s,
> 
>                         Geert
> 

Regards,
John

> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-
> m68k.org
> 
> In personal conversations with technical people, I call myself a hacker.
> But when I'm talking to journalists I just say "programmer" or something
> like that.
>                                 -- Linus Torvalds

Re: [PATCH v6 3/5] thermal: renesas: rzg3e: Add thermal driver for the Renesas RZ/G3E SoC

[Geert Uytterhoeven]

Hi John,

On Tue, 5 Aug 2025 at 10:27, John Madieu <john.madieu.xa@bp.renesas.com> wrote:
> > From: Geert Uytterhoeven <geert@linux-m68k.org>
> > On Thu, 22 May 2025 at 20:23, John Madieu <john.madieu.xa@bp.renesas.com>
> > wrote:
> > > The RZ/G3E SoC integrates a Temperature Sensor Unit (TSU) block
> > > designed to monitor the chip's junction temperature. This sensor is
> > > connected to channel 1 of the APB port clock/reset and provides
> > temperature measurements.
> > >
> > > It also requires calibration values stored in the system controller
> > > registers for accurate temperature measurement. Add a driver for the
> > Renesas RZ/G3E TSU.
> > >
> > > Signed-off-by: John Madieu <john.madieu.xa@bp.renesas.com>
> >
> > Thanks for your patch!
> >
> > The TSUs in RZ/V2H and RZ/V2N seem to be identical to the one in RZ/G3E.
> > However, RZ/V2H and RZ/V2N have two instances, while RZ/G3 has only one.
>
> This is true.
>
> > > --- /dev/null
> > > +++ b/drivers/thermal/renesas/rzg3e_thermal.c
> > > @@ -0,0 +1,443 @@
> >
> > > +/* SYS Trimming register offsets macro */ #define SYS_TSU_TRMVAL(x)
> > > +(0x330 + (x) * 4)
> >
> > RZ/V2H and RZ/V2N have a second set of trim values for the second TSU
> > instance.  So I guess you want to specify the offset in DT instead.
>
> What do you think of 'renesas,tsu-channel' property or alike
> Property to specify the channel being used ?

While I agree instance IDs canbe useful (sometimes), the DT maintainers
do not like them very much, cfr. commit 6a57cf210711c068 ("docs: dt:
writing-bindings: Document discouraged instance IDs"), which prefers
cell/phandle arguments.

For this particular case:
  1. The instance ID for the single TSU on RZ/G3E would be one, not zero
     (oh, the SYS_LSI_OTPTSU1TRMVAL[01] register names do contain "TSU1"),
  2. It will break the moment a new SoC is released that stores trim
     values at different offsets in the SYSC block.

Hence a property containing a SYSC phandle and register offset sounds
better to me.
Thoughts?

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

RE: [PATCH v6 3/5] thermal: renesas: rzg3e: Add thermal driver for the Renesas RZ/G3E SoC

[John Madieu]

Hi Geert,

Thanks for the feedback!

> -----Original Message-----
> From: Geert Uytterhoeven <geert@linux-m68k.org>
> Sent: Tuesday, August 5, 2025 10:47 AM
> To: John Madieu <john.madieu.xa@bp.renesas.com>
> Subject: Re: [PATCH v6 3/5] thermal: renesas: rzg3e: Add thermal driver for
> the Renesas RZ/G3E SoC
> 
> Hi John,
> 
> On Tue, 5 Aug 2025 at 10:27, John Madieu <john.madieu.xa@bp.renesas.com>
> wrote:
> > > From: Geert Uytterhoeven <geert@linux-m68k.org> On Thu, 22 May 2025
> > > at 20:23, John Madieu <john.madieu.xa@bp.renesas.com>
> > > wrote:
> > > > The RZ/G3E SoC integrates a Temperature Sensor Unit (TSU) block
> > > > designed to monitor the chip's junction temperature. This sensor
> > > > is connected to channel 1 of the APB port clock/reset and provides
> > > temperature measurements.
> > >
> > > RZ/V2H and RZ/V2N have a second set of trim values for the second
> > > TSU instance.  So I guess you want to specify the offset in DT instead.
> >
> > What do you think of 'renesas,tsu-channel' property or alike Property
> > to specify the channel being used ?
> 
> While I agree instance IDs canbe useful (sometimes), the DT maintainers do
> not like them very much, cfr. commit 6a57cf210711c068 ("docs: dt:
> writing-bindings: Document discouraged instance IDs"), which prefers
> cell/phandle arguments.
> 
> For this particular case:
>   1. The instance ID for the single TSU on RZ/G3E would be one, not zero
>      (oh, the SYS_LSI_OTPTSU1TRMVAL[01] register names do contain "TSU1"),
>   2. It will break the moment a new SoC is released that stores trim
>      values at different offsets in the SYSC block.
> 
> Hence a property containing a SYSC phandle and register offset sounds
> better to me.

This sounds good to me. I see something like:

renesas,tsu-channel1 = <&sysc off1>;
renesas,tsu-channel2 = <&sysc off2>; /* Optional, for V2H */

/* or */

renesas,tsu-channel-map = <&sysc off1 off2>;

I would go for the first option to make it easier for V2H
(while adding support for it later) so it can choose using
either, or both, regardless of the index.

What do you think ?

> Thoughts?
> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 

Regards,
John

> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-
> m68k.org
> 
> In personal conversations with technical people, I call myself a hacker.
> But when I'm talking to journalists I just say "programmer" or something
> like that.
>                                 -- Linus Torvalds

Re: [PATCH v6 3/5] thermal: renesas: rzg3e: Add thermal driver for the Renesas RZ/G3E SoC

[Geert Uytterhoeven]

Hi John,

On Tue, 5 Aug 2025 at 11:22, John Madieu <john.madieu.xa@bp.renesas.com> wrote:
> > From: Geert Uytterhoeven <geert@linux-m68k.org>
> > On Tue, 5 Aug 2025 at 10:27, John Madieu <john.madieu.xa@bp.renesas.com>
> > wrote:
> > > > From: Geert Uytterhoeven <geert@linux-m68k.org> On Thu, 22 May 2025
> > > > at 20:23, John Madieu <john.madieu.xa@bp.renesas.com>
> > > > wrote:
> > > > > The RZ/G3E SoC integrates a Temperature Sensor Unit (TSU) block
> > > > > designed to monitor the chip's junction temperature. This sensor
> > > > > is connected to channel 1 of the APB port clock/reset and provides
> > > > temperature measurements.
> > > >
> > > > RZ/V2H and RZ/V2N have a second set of trim values for the second
> > > > TSU instance.  So I guess you want to specify the offset in DT instead.
> > >
> > > What do you think of 'renesas,tsu-channel' property or alike Property
> > > to specify the channel being used ?
> >
> > While I agree instance IDs canbe useful (sometimes), the DT maintainers do
> > not like them very much, cfr. commit 6a57cf210711c068 ("docs: dt:
> > writing-bindings: Document discouraged instance IDs"), which prefers
> > cell/phandle arguments.
> >
> > For this particular case:
> >   1. The instance ID for the single TSU on RZ/G3E would be one, not zero
> >      (oh, the SYS_LSI_OTPTSU1TRMVAL[01] register names do contain "TSU1"),
> >   2. It will break the moment a new SoC is released that stores trim
> >      values at different offsets in the SYSC block.
> >
> > Hence a property containing a SYSC phandle and register offset sounds
> > better to me.
>
> This sounds good to me. I see something like:
>
> renesas,tsu-channel1 = <&sysc off1>;
> renesas,tsu-channel2 = <&sysc off2>; /* Optional, for V2H */
>
> /* or */
>
> renesas,tsu-channel-map = <&sysc off1 off2>;
>
> I would go for the first option to make it easier for V2H
> (while adding support for it later) so it can choose using
> either, or both, regardless of the index.
>
> What do you think ?

As the property would be part of the TSU node, it would always
refer to that specific channel/instance, so e.g.

    renesas,tsu-trim = <&sysc 0x320>;

for the first TSU instance, and

    renesas,tsu-trim = <&sysc 0x330>;

for the second instance.

P.S. Please don't write "V2H" on its own, as both R-Car V2H and RZ/V2H
     exist in the Renesas SoC portfolio ;-)

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

RE: [PATCH v6 3/5] thermal: renesas: rzg3e: Add thermal driver for the Renesas RZ/G3E SoC

[John Madieu]

Hi Geert,

> -----Original Message-----
> From: Geert Uytterhoeven <geert@linux-m68k.org>
> Sent: Tuesday, August 5, 2025 11:35 AM
> To: John Madieu <john.madieu.xa@bp.renesas.com>
> Subject: Re: [PATCH v6 3/5] thermal: renesas: rzg3e: Add thermal driver for
> the Renesas RZ/G3E SoC
> 
> Hi John,
> 
> On Tue, 5 Aug 2025 at 11:22, John Madieu <john.madieu.xa@bp.renesas.com>
> wrote:
> > > From: Geert Uytterhoeven <geert@linux-m68k.org> On Tue, 5 Aug 2025
> > > at 10:27, John Madieu <john.madieu.xa@bp.renesas.com>
> > > wrote:
> > > > > From: Geert Uytterhoeven <geert@linux-m68k.org> On Thu, 22 May
> > > > > 2025 at 20:23, John Madieu <john.madieu.xa@bp.renesas.com>
> > > > > wrote:
> > > > > > The RZ/G3E SoC integrates a Temperature Sensor Unit (TSU)
> > > > > > block designed to monitor the chip's junction temperature.
> > > > > > This sensor is connected to channel 1 of the APB port
> > > > > > clock/reset and provides
> > > > > temperature measurements.
> >
> > /* or */
> >
> > renesas,tsu-channel-map = <&sysc off1 off2>;
> >
> > I would go for the first option to make it easier for V2H (while
> > adding support for it later) so it can choose using either, or both,
> > regardless of the index.
> >
> > What do you think ?
> 
> As the property would be part of the TSU node, it would always refer to
> that specific channel/instance, so e.g.
> 
>     renesas,tsu-trim = <&sysc 0x320>;
> 
> for the first TSU instance, and
> 
>     renesas,tsu-trim = <&sysc 0x330>;
> 
> for the second instance.

Will go for this, with both naming and phandle/offset.

> 
> P.S. Please don't write "V2H" on its own, as both R-Car V2H and RZ/V2H
>      exist in the Renesas SoC portfolio ;-)

My bad. Will take care of this next times.

> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-
> m68k.org
> 
> In personal conversations with technical people, I call myself a hacker.
> But when I'm talking to journalists I just say "programmer" or something
> like that.
>                                 -- Linus Torvalds

Regards,
John

Re: [PATCH v6 3/5] thermal: renesas: rzg3e: Add thermal driver for the Renesas RZ/G3E SoC

[Daniel Lezcano]

Hi John,


On 05/08/2025 09:49, John Madieu wrote:

[ ... ]

>>> I might not get what you are asking, but since I compute the
>>> temperature in the hard IRQ handler, I just wait for it to complete
>>> and notify the completion so I can grab the processed value to notify
>>> the thermal core.
>>>
>>> Please let me know if this does not answer your question.
>>
>> Can you describe how the sensor works ? And perhaps if you have a pointer
>> to some documentation ?
> 
> Here is the documentation [1]. The thermal device is referred to as TSU.
> 
> [1] https://www.renesas.com/en/document/mah/rzg3e-group-users-manual-hardware?r=25574493
> 
>>    [ ... ]

Thanks for the pointer. I got it now ;)

I'm a bit worried about the latency introduced by this mechanism when 
the system is entering in a thermal pressure episode.

The get_temp function wait for a completion up to 100ms, it is a lot. 
Especially if the userspace can be reading the temperature and blocking 
the read.

There is also the spin_lock taken introducing another lock while the 
get_temp function is holding a mutex on the thermal zone.

Did you it that under stress ?

-- 
<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 v6 1/5] soc: renesas: rz-sysc: Add syscon/regmap support

[John Madieu]

Hi Geert,

Thanks for your review!

> -----Original Message-----
> From: Geert Uytterhoeven <geert@linux-m68k.org>
> Sent: Monday, August 4, 2025 11:19 AM
> To: John Madieu <john.madieu.xa@bp.renesas.com>
> Subject: Re: [PATCH v6 1/5] soc: renesas: rz-sysc: Add syscon/regmap
> support
> 
> Hi John,
> 
> On Thu, 22 May 2025 at 20:23, John Madieu <john.madieu.xa@bp.renesas.com>
> wrote:
> > The RZ/G3E system controller has various registers that control or
> > report some properties specific to individual IPs. The regmap is
> > registered as a syscon device to allow these IP drivers to access the
> > registers through the regmap API.
> >
> > As other RZ SoCs might have custom read/write callbacks or
> > max-offsets, add register a custom regmap configuration.
> >
> > Signed-off-by: John Madieu <john.madieu.xa@bp.renesas.com>
> > [claudiu.beznea:
> >  - s/rzg3e_sysc_regmap/rzv2h_sysc_regmap in RZ/V2H sysc
> >    file
> >  - do not check the match->data validity in rz_sysc_probe() as it is
> >    always valid
> >  - register the regmap if data->regmap_cfg is valid]
> > Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> 
> Thanks for your patch!
> 
> As you probably noticed, I am not a big fan of syscon, so I try to avoid
> getting involved with syscon patches as much as possible.
> But this patch has appeared in multiple series, so I am afraid I cannot
> avoid it anymore ;-)
> 
> > --- a/drivers/soc/renesas/r9a08g045-sysc.c
> > +++ b/drivers/soc/renesas/r9a08g045-sysc.c
> > @@ -18,6 +18,16 @@ static const struct rz_sysc_soc_id_init_data
> rzg3s_sysc_soc_id_init_data __initc
> >         .specific_id_mask = GENMASK(27, 0),  };
> >
> > +static const struct regmap_config rzg3s_sysc_regmap __initconst = {
> > +       .name = "rzg3s_sysc_regs",
> > +       .reg_bits = 32,
> > +       .reg_stride = 4,
> > +       .val_bits = 32,
> > +       .fast_io = true,
> > +       .max_register = 0xe20,
> > +};
> 
> Struct regmap_config is a rather large structure, and the only SoC-specific
> members are the .name (which doesn't really matter) and .max_register
> members.  Hence please move .max_register back to struct rz_sysc_init_data
> (like in v5), and allocate struct regmap_config dynamically (by embedding
> it into struct rz_sysc).
> 
> > +
> >  const struct rz_sysc_init_data rzg3s_sysc_init_data __initconst = {
> >         .soc_id_init_data = &rzg3s_sysc_soc_id_init_data,
> > +       .regmap_cfg = &rzg3s_sysc_regmap,
> >  };
> 
> > --- a/drivers/soc/renesas/rz-sysc.c
> > +++ b/drivers/soc/renesas/rz-sysc.c
> > @@ -117,7 +124,15 @@ static int rz_sysc_probe(struct platform_device
> *pdev)
> >                 return PTR_ERR(sysc->base);
> >
> >         sysc->dev = dev;
> > -       return rz_sysc_soc_init(sysc, match);
> > +       ret = rz_sysc_soc_init(sysc, match);
> > +       if (ret || !data->regmap_cfg)
> 
> data->regmap_cfg should never be NULL (but this doesn't matter anymore,
> in the context of my request above).
> 
> > +               return ret;
> > +
> > +       regmap = devm_regmap_init_mmio(dev, sysc->base, data-
> >regmap_cfg);
> > +       if (IS_ERR(regmap))
> > +               return PTR_ERR(regmap);
> > +
> > +       return of_syscon_register_regmap(dev->of_node, regmap);
> >  }
> 
> The rest LGTM.

Will make the changes as per the v5. Thanks!

> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-
> m68k.org
> 
> In personal conversations with technical people, I call myself a hacker.
> But when I'm talking to journalists I just say "programmer" or something
> like that.
>                                 -- Linus Torvalds

Regards,
John

RE: [PATCH v6 3/5] thermal: renesas: rzg3e: Add thermal driver for the Renesas RZ/G3E SoC

[John Madieu]

Hi Daniel,

Thanks again for the feedback.

> -----Original Message-----
> From: Daniel Lezcano <daniel.lezcano@linaro.org>
> Sent: Tuesday, August 5, 2025 12:06 PM
> To: John Madieu <john.madieu.xa@bp.renesas.com>
> Subject: Re: [PATCH v6 3/5] thermal: renesas: rzg3e: Add thermal driver for
> the Renesas RZ/G3E SoC
> 
> 
> Hi John,
> 
> 
> On 05/08/2025 09:49, John Madieu wrote:
> 
> [ ... ]
> 
> >>> I might not get what you are asking, but since I compute the
> >>> temperature in the hard IRQ handler, I just wait for it to complete
> >>> and notify the completion so I can grab the processed value to
> >>> notify the thermal core.
> >>>
> >>> Please let me know if this does not answer your question.
> >>
> >> Can you describe how the sensor works ? And perhaps if you have a
> >> pointer to some documentation ?
> >
> > Here is the documentation [1]. The thermal device is referred to as TSU.
> >
> > [1]
> > https://www.renesas.com/en/document/mah/rzg3e-group-users-manual-hardw
> > are?r=25574493
> >
> >>    [ ... ]
> 
> Thanks for the pointer. I got it now ;)
> 
> I'm a bit worried about the latency introduced by this mechanism when the
> system is entering in a thermal pressure episode.
> 
> The get_temp function wait for a completion up to 100ms, it is a lot.
> Especially if the userspace can be reading the temperature and blocking the
> read.
> 
> There is also the spin_lock taken introducing another lock while the
> get_temp function is holding a mutex on the thermal zone.
> 
> Did you it that under stress ?
> 

After spending some time stressing the driver, I've noticed a
couple of issues:

 * Spinlock/mutex mux caused some issues, and I had cases where
   it timed-out while not even under stress.
 
 * Mixing both compare (cmp) and conversion complete (conv) IRQs
 introduced some latencies and inconsistencies.

After spending some time in the datasheet, I could notice that one
conversion takes around 50µs. In average mode (8 samples per conversion),
it would take 400µs, which I doubled (for margin) and used as timeout in
v7 series that is already ready. 

Moreover, as per datasheet recommendations, I kept comparison IRQ 
(for trip point violation) only, while using polling for get_temp()
(with the 800µs timeout), which gives better results.


If you don't mind, I'll send a v7 which addresses all the
requests from Geert as well as the above-mentioned changes.

It includes:

 * 800µs timeout for get_temp() in polling read
 * No spinlock/mutex mix and no completion anymore
 * Comparison-only IRQ for trip point violations
 * + Geert's change requests

What do you think ?

Regards,
John.

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