[PATCH v5 0/5] thermal: renesas: Add support fot RZ/G3E

[PATCH v5 0/5] thermal: renesas: Add support fot 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.

Note to Maintainers: There is a false positive warning reported by
checkpatch.pl on patch 1/5 stating that the regmap_config struct should be
const, despite the fact it's updated in probe().

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

Regards,

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/r9a09g047-sys.c           |   1 +
 drivers/soc/renesas/rz-sysc.c                 |  30 +-
 drivers/soc/renesas/rz-sysc.h                 |   2 +
 drivers/thermal/renesas/Kconfig               |   7 +
 drivers/thermal/renesas/Makefile              |   1 +
 drivers/thermal/renesas/rzg3e_thermal.c       | 443 ++++++++++++++++++
 11 files changed, 621 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 v5 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, let's
register a custom regmap configuration.

Signed-off-by: John Madieu <john.madieu.xa@bp.renesas.com>
---
v1 -> v2: no changes
v2 -> v3: no changes
v3 -> v4: no changes
v4 -> v5: no changes

Note for Maintainers: There is a false positive warning reported by
checkpatch.pl on this patch patch stating that the regmap_config struct
should be const, despite the fact it's updated in probe().

 drivers/soc/renesas/Kconfig         |  1 +
 drivers/soc/renesas/r9a09g047-sys.c |  1 +
 drivers/soc/renesas/rz-sysc.c       | 30 ++++++++++++++++++++++++++++-
 drivers/soc/renesas/rz-sysc.h       |  2 ++
 4 files changed, 33 insertions(+), 1 deletion(-)

diff --git a/drivers/soc/renesas/Kconfig b/drivers/soc/renesas/Kconfig
index 49648cf28bd2..3ffd3a4ca18d 100644
--- a/drivers/soc/renesas/Kconfig
+++ b/drivers/soc/renesas/Kconfig
@@ -388,6 +388,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/r9a09g047-sys.c b/drivers/soc/renesas/r9a09g047-sys.c
index cd2eb7782cfe..5b010a519fab 100644
--- a/drivers/soc/renesas/r9a09g047-sys.c
+++ b/drivers/soc/renesas/r9a09g047-sys.c
@@ -64,4 +64,5 @@ static const struct rz_sysc_soc_id_init_data rzg3e_sys_soc_id_init_data __initco
 
 const struct rz_sysc_init_data rzg3e_sys_init_data = {
 	.soc_id_init_data = &rzg3e_sys_soc_id_init_data,
+	.max_register_offset = 0x170c,
 };
diff --git a/drivers/soc/renesas/rz-sysc.c b/drivers/soc/renesas/rz-sysc.c
index 1c98da37b7d1..bcbc23da954b 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"
@@ -81,6 +83,14 @@ static int rz_sysc_soc_init(struct rz_sysc *sysc, const struct of_device_id *mat
 	return 0;
 }
 
+static struct regmap_config rz_sysc_regmap = {
+	.name = "rz_sysc_regs",
+	.reg_bits = 32,
+	.reg_stride = 4,
+	.val_bits = 32,
+	.fast_io = true,
+};
+
 static const struct of_device_id rz_sysc_match[] = {
 #ifdef CONFIG_SYSC_R9A08G045
 	{ .compatible = "renesas,r9a08g045-sysc", .data = &rzg3s_sysc_init_data },
@@ -97,14 +107,21 @@ 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;
+	if (!data)
+		return -EINVAL;
+
 	sysc = devm_kzalloc(dev, sizeof(*sysc), GFP_KERNEL);
 	if (!sysc)
 		return -ENOMEM;
@@ -114,7 +131,18 @@ 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 (data->max_register_offset) {
+		rz_sysc_regmap.max_register = data->max_register_offset;
+		regmap = devm_regmap_init_mmio(dev, sysc->base, &rz_sysc_regmap);
+		if (IS_ERR(regmap))
+			return PTR_ERR(regmap);
+
+		ret = of_syscon_register_regmap(dev->of_node, regmap);
+	}
+
+	return ret;
 }
 
 static struct platform_driver rz_sysc_driver = {
diff --git a/drivers/soc/renesas/rz-sysc.h b/drivers/soc/renesas/rz-sysc.h
index aa83948c5117..37a3bb2c87f8 100644
--- a/drivers/soc/renesas/rz-sysc.h
+++ b/drivers/soc/renesas/rz-sysc.h
@@ -34,9 +34,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
+ * @max_register_offset: Maximum SYSC register offset to be used by the regmap config
  */
 struct rz_sysc_init_data {
 	const struct rz_sysc_soc_id_init_data *soc_id_init_data;
+	u32 max_register_offset;
 };
 
 extern const struct rz_sysc_init_data rzg3e_sys_init_data;
-- 
2.25.1

[PATCH v5 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>
---
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

 .../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 v5 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>
---
v1 -> v2: fixes IRQ names
v2 -> v3: no changes
v3 -> v4: no changes
v5: removes curly braces arround single-line protected scoped guards

 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 b9f7d2115b57..ba7c95146f01 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -20289,6 +20289,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..fe50df057b74
--- /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 ADC interrupt */
+	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_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_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 v5 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>
---
v1 -> v2: Fix IRQ names
v2 -> v3: remove useless 'renesas,tsu-operating-mode' property'
v3 -> v4: no changes
v5: 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 a6b83e057a40..c49214cb7936 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>;
 		};
 
@@ -302,6 +306,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>;
@@ -502,6 +519,37 @@ gic: interrupt-controller@14900000 {
 		};
 	};
 
+	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 v5 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>
---
v1 -> v2: no changes
v2 -> v3: no changes
v3 -> v4: update commit message
v5: 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 a1cc3814b09b..91136c0196b5 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -714,6 +714,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 v5 1/5] soc: renesas: rz-sysc: add syscon/regmap support

[Biju Das]

Hi John,

Thanks for the patch.

> -----Original Message-----
> From: John Madieu <john.madieu.xa@bp.renesas.com>
> Sent: 30 March 2025 22:50
> Subject: [PATCH v5 1/5] soc: renesas: rz-sysc: add syscon/regmap support
> 
> 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, let's register a custom regmap
> configuration.
> 
> Signed-off-by: John Madieu <john.madieu.xa@bp.renesas.com>
> ---
> v1 -> v2: no changes
> v2 -> v3: no changes
> v3 -> v4: no changes
> v4 -> v5: no changes
> 
> Note for Maintainers: There is a false positive warning reported by checkpatch.pl on this patch patch
> stating that the regmap_config struct should be const, despite the fact it's updated in probe().
> 
>  drivers/soc/renesas/Kconfig         |  1 +
>  drivers/soc/renesas/r9a09g047-sys.c |  1 +
>  drivers/soc/renesas/rz-sysc.c       | 30 ++++++++++++++++++++++++++++-
>  drivers/soc/renesas/rz-sysc.h       |  2 ++
>  4 files changed, 33 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/soc/renesas/Kconfig b/drivers/soc/renesas/Kconfig index
> 49648cf28bd2..3ffd3a4ca18d 100644
> --- a/drivers/soc/renesas/Kconfig
> +++ b/drivers/soc/renesas/Kconfig
> @@ -388,6 +388,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/r9a09g047-sys.c b/drivers/soc/renesas/r9a09g047-sys.c
> index cd2eb7782cfe..5b010a519fab 100644
> --- a/drivers/soc/renesas/r9a09g047-sys.c
> +++ b/drivers/soc/renesas/r9a09g047-sys.c
> @@ -64,4 +64,5 @@ static const struct rz_sysc_soc_id_init_data rzg3e_sys_soc_id_init_data __initco
> 
>  const struct rz_sysc_init_data rzg3e_sys_init_data = {
>  	.soc_id_init_data = &rzg3e_sys_soc_id_init_data,
> +	.max_register_offset = 0x170c,

 Replace max_register_offset->rz_sysc_regmap
>  };
> diff --git a/drivers/soc/renesas/rz-sysc.c b/drivers/soc/renesas/rz-sysc.c index
> 1c98da37b7d1..bcbc23da954b 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"
> @@ -81,6 +83,14 @@ static int rz_sysc_soc_init(struct rz_sysc *sysc, const struct of_device_id *mat
>  	return 0;
>  }
> 
> +static struct regmap_config rz_sysc_regmap = {
> +	.name = "rz_sysc_regs",
> +	.reg_bits = 32,
> +	.reg_stride = 4,
> +	.val_bits = 32,
> +	.fast_io = true,
> +};
> +

Move this to SoC specific file.

static const struct regmap_config rz_g3e_sysc_regmap = {
	.name = "rz_g3e_sysc_regs",
	.reg_bits = 32,
	.reg_stride = 4,
	.val_bits = 32,
	.fast_io = true,
      .max_register_offset = 0x170c,
};


>  static const struct of_device_id rz_sysc_match[] = {  #ifdef CONFIG_SYSC_R9A08G045
>  	{ .compatible = "renesas,r9a08g045-sysc", .data = &rzg3s_sysc_init_data }, @@ -97,14 +107,21 @@
> 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;
> +	if (!data)
> +		return -EINVAL;
> +
>  	sysc = devm_kzalloc(dev, sizeof(*sysc), GFP_KERNEL);
>  	if (!sysc)
>  		return -ENOMEM;
> @@ -114,7 +131,18 @@ 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 (data->max_register_offset) {
	If(data->rz_sysc_regmap)

> +		rz_sysc_regmap.max_register = data->max_register_offset;
Drop it.

> +		regmap = devm_regmap_init_mmio(dev, sysc->base, &rz_sysc_regmap);

regmap = devm_regmap_init_mmio(dev, sysc->base, data->rz_sysc_regmap);

> +		if (IS_ERR(regmap))
> +			return PTR_ERR(regmap);
> +
> +		ret = of_syscon_register_regmap(dev->of_node, regmap);
> +	}
> +
> +	return ret;
>  }
> 
>  static struct platform_driver rz_sysc_driver = { diff --git a/drivers/soc/renesas/rz-sysc.h
> b/drivers/soc/renesas/rz-sysc.h index aa83948c5117..37a3bb2c87f8 100644
> --- a/drivers/soc/renesas/rz-sysc.h
> +++ b/drivers/soc/renesas/rz-sysc.h
> @@ -34,9 +34,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
> + * @max_register_offset: Maximum SYSC register offset to be used by the
> + regmap config
>   */
>  struct rz_sysc_init_data {
>  	const struct rz_sysc_soc_id_init_data *soc_id_init_data;
> +	u32 max_register_offset;

      const struct regmap_config *rz_sysc_regmap;

Cheers,
Biju

>  };
> 
>  extern const struct rz_sysc_init_data rzg3e_sys_init_data;
> --
> 2.25.1

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

[ALOK TIWARI]

On 31-03-2025 03:19, 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>
> ---
> v1 -> v2: fixes IRQ names
> v2 -> v3: no changes
> v3 -> v4: no changes
> v5: removes curly braces arround single-line protected scoped guards
> 
>   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 b9f7d2115b57..ba7c95146f01 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -20289,6 +20289,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..fe50df057b74
> --- /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 ADC interrupt */
> +	writel(0, priv->base + TSU_SIER);
what is difference b/w /* Disable all interrupts * / used in 
thermal_hw_enabl and Disable ADC interrupt ? The same comment can be 
used for better readability.
> +	/* 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;
> +}
> +
always return 0 here ? what, if (!priv) return -EINVAL; ?

> +static const struct thermal_zone_device_ops rzg3e_tz_ops = {
> +	.get_temp = rzg3e_thermal_get_temp,
> +	.set_trips = rzg3e_thermal_set_trips,
> +	.change_mode = rzg3e_thermal_change_mode,
> +};
other renesas driver defined as rzg2l_tz_of_ops, can be used similar one 
rzg3e_tz_of_ops for consistency!
> +
> +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_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");


Thanks,
Alok

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

[Biju Das]

Hi Alok,

> -----Original Message-----
> From: ALOK TIWARI <alok.a.tiwari@oracle.com>
> Sent: 31 March 2025 19:11
> Subject: Re : [PATCH v5 3/5] thermal: renesas: rzg3e: Add thermal driver for the Renesas RZ/G3E SoC
> 
> 
> 
> On 31-03-2025 03:19, 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>
> > ---
> > v1 -> v2: fixes IRQ names
> > v2 -> v3: no changes
> > v3 -> v4: no changes
> > v5: removes curly braces arround single-line protected scoped guards
> >
> >   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
> > b9f7d2115b57..ba7c95146f01 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -20289,6 +20289,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..fe50df057b74
> > --- /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"

<snip>
> > +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;
> > +}
> > +
> always return 0 here ? what, if (!priv) return -EINVAL; ?

priv cannot be NULL as it is checked in probe().
Maybe replace static int rzg3e_thermal_change_mode->
static void rzg3e_thermal_change_mode().

Cheers,
Biju


Cheers,
Biju

> 

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

[John Madieu]

Hi Alok,

Thanks for your feedback.

> -----Original Message-----
> From: ALOK TIWARI <alok.a.tiwari@oracle.com>
> Sent: Monday, March 31, 2025 8:11 PM
> To: John Madieu <john.madieu.xa@bp.renesas.com>; geert+renesas@glider.be;
> conor+dt@kernel.org; krzk+dt@kernel.org; robh@kernel.org;
> rafael@kernel.org; daniel.lezcano@linaro.org
> Subject: Re : [PATCH v5 3/5] thermal: renesas: rzg3e: Add thermal driver
> for the Renesas RZ/G3E SoC
> 
> 
> 
> On 31-03-2025 03:19, 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>
> > ---
> > v1 -> v2: fixes IRQ names
> > v2 -> v3: no changes
> > v3 -> v4: no changes
> > v5: removes curly braces arround single-line protected scoped guards
> >
> >   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
> > b9f7d2115b57..ba7c95146f01 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -20289,6 +20289,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..fe50df057b74
> > --- /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 ADC interrupt */
> > +	writel(0, priv->base + TSU_SIER);
> what is difference b/w /* Disable all interrupts * / used in
> thermal_hw_enabl and Disable ADC interrupt ? The same comment can be used
> for better readability.

Noted. I got your point. Thanks.

> > +	/* 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;
> > +}
> > +
> always return 0 here ? what, if (!priv) return -EINVAL; ?

priv cannot be NULL here, guaranteed from probe().
Returning 0 here is expected by the thermal framework to notify
ops success.

> 
> > +static const struct thermal_zone_device_ops rzg3e_tz_ops = {
> > +	.get_temp = rzg3e_thermal_get_temp,
> > +	.set_trips = rzg3e_thermal_set_trips,
> > +	.change_mode = rzg3e_thermal_change_mode, };
> other renesas driver defined as rzg2l_tz_of_ops, can be used similar one
> rzg3e_tz_of_ops for consistency!

Thanks for pointing it out. Makes sense. Will double check and
update accordingly.

> > +
> > +static int rzg3e_thermal_probe(struct platform_device *pdev) {
> > +	struct device *dev = &pdev->dev;
> > +	struct rzg3e_thermal_priv *priv;
> > +	struct reset_control *rstc;
> 
> Thanks,
> Alok

Regards,
John

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

[ALOK TIWARI]

Hi John,

Thanks for your reply.

On 01-04-2025 17:35, John Madieu wrote:
> Hi Alok,
> 
> Thanks for your feedback.
> 
>> -----Original Message-----
>> From: ALOK TIWARI <alok.a.tiwari@oracle.com>
>> Sent: Monday, March 31, 2025 8:11 PM
>> To: John Madieu <john.madieu.xa@bp.renesas.com>; geert+renesas@glider.be;
>> conor+dt@kernel.org; krzk+dt@kernel.org; robh@kernel.org;
>> rafael@kernel.org; daniel.lezcano@linaro.org
>> Subject: Re : [PATCH v5 3/5] thermal: renesas: rzg3e: Add thermal driver
>> for the Renesas RZ/G3E SoC
>>
>>
>>
>> On 31-03-2025 03:19, John Madieu wrote:
>>> The RZ/G3E SoC integrates a Temperature Sensor Unit (TSU) block

>>> +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;
>>> +}
>>> +
>> always return 0 here ? what, if (!priv) return -EINVAL; ?
> 
> priv cannot be NULL here, guaranteed from probe().
> Returning 0 here is expected by the thermal framework to notify
> ops success.
> 

I agreed. priv cannot be NULL.
It appears that return 0 is deliberate in this case. can we add 
Meaningful comment which help to code readability.

not sure if user call ioctl(fd, THERMAL_IOC_SET_MODE, 2) and it returns 
0 with thermal enable.
that create possibility to thermal_core call thermal_notify_tz_disable 
if any case

>>> +static const struct thermal_zone_device_ops rzg3e_tz_ops = {
>>> +	.get_temp = rzg3e_thermal_get_temp,
>>> +	.set_trips = rzg3e_thermal_set_trips,
>>> +	.change_mode = rzg3e_thermal_change_mode, };
>> other renesas driver defined as rzg2l_tz_of_ops, can be used similar one
>> rzg3e_tz_of_ops for consistency!
> 
> Thanks for pointing it out. Makes sense. Will double check and
> update accordingly.
> 
>>> +
>>> +static int rzg3e_thermal_probe(struct platform_device *pdev) {
>>> +	struct device *dev = &pdev->dev;
>>> +	struct rzg3e_thermal_priv *priv;
>>> +	struct reset_control *rstc;
>>
>> Thanks,
>> Alok
> 
> Regards,
> John

Thanks,
Alok

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

[John Madieu]

Hi Alok,

> -----Original Message-----
> From: ALOK TIWARI <alok.a.tiwari@oracle.com>
> Sent: Tuesday, April 1, 2025 8:24 PM
> To: John Madieu <john.madieu.xa@bp.renesas.com>; geert+renesas@glider.be;
> conor+dt@kernel.org; krzk+dt@kernel.org; robh@kernel.org;
> rafael@kernel.org; daniel.lezcano@linaro.org
> Subject: Re : [PATCH v5 3/5] thermal: renesas: rzg3e: Add thermal driver
> for the Renesas RZ/G3E SoC
> 
> Hi John,
> 
> Thanks for your reply.
> 
> On 01-04-2025 17:35, John Madieu wrote:
> > Hi Alok,
> >
> > Thanks for your feedback.
> >
> >> -----Original Message-----
> >> From: ALOK TIWARI <alok.a.tiwari@oracle.com>
> >> Sent: Monday, March 31, 2025 8:11 PM
> >> To: John Madieu <john.madieu.xa@bp.renesas.com>;
> >> geert+renesas@glider.be;
> >> conor+dt@kernel.org; krzk+dt@kernel.org; robh@kernel.org;
> >> rafael@kernel.org; daniel.lezcano@linaro.org
> >> Subject: Re : [PATCH v5 3/5] thermal: renesas: rzg3e: Add thermal
> >> driver for the Renesas RZ/G3E SoC
> >>
> >>
> >>
> >> On 31-03-2025 03:19, John Madieu wrote:
> >>> The RZ/G3E SoC integrates a Temperature Sensor Unit (TSU) block
> 
> >>> +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;
> >>> +}
> >>> +
> >> always return 0 here ? what, if (!priv) return -EINVAL; ?
> >
> > priv cannot be NULL here, guaranteed from probe().
> > Returning 0 here is expected by the thermal framework to notify ops
> > success.
> >
> 
> I agreed. priv cannot be NULL.
> It appears that return 0 is deliberate in this case. can we add Meaningful
> comment which help to code readability.
> 

This is a standard tzd ops, which expects 0 to be returned
for disabling/enabling success. This is how it's implemented
in every Thermal driver I've gone through in the -next tree.

> not sure if user call ioctl(fd, THERMAL_IOC_SET_MODE, 2) and it returns
> 0 with thermal enable.
> that create possibility to thermal_core call thermal_notify_tz_disable if
> any case
> 
Am I missing something ?

> >>> +static const struct thermal_zone_device_ops rzg3e_tz_ops = {
> >>> +	.get_temp = rzg3e_thermal_get_temp,
> >>> +	.set_trips = rzg3e_thermal_set_trips,
> >>> +	.change_mode = rzg3e_thermal_change_mode, };
> >> other renesas driver defined as rzg2l_tz_of_ops, can be used similar
> >> one rzg3e_tz_of_ops for consistency!
> >
> > Thanks for pointing it out. Makes sense. Will double check and update
> > accordingly.
> >
> >>> +
> >>> +static int rzg3e_thermal_probe(struct platform_device *pdev) {
> >>> +	struct device *dev = &pdev->dev;
> >>> +	struct rzg3e_thermal_priv *priv;
> >>> +	struct reset_control *rstc;
> >>
Regards,
John

RE: [PATCH v5 1/5] soc: renesas: rz-sysc: add syscon/regmap support

[John Madieu]

Hi Biju,

Thanks for your feedback.

> -----Original Message-----
> From: Biju Das <biju.das.jz@bp.renesas.com>
> Sent: Monday, March 31, 2025 8:08 AM
> To: John Madieu <john.madieu.xa@bp.renesas.com>; geert+renesas@glider.be;
> conor+dt@kernel.org; krzk+dt@kernel.org; robh@kernel.org;
> rafael@kernel.org; daniel.lezcano@linaro.org
> Subject: RE: [PATCH v5 1/5] soc: renesas: rz-sysc: add syscon/regmap
> support
> 
> Hi John,
> 
> Thanks for the patch.
> 
> > -----Original Message-----
> > From: John Madieu <john.madieu.xa@bp.renesas.com>
> > Sent: 30 March 2025 22:50
> > Subject: [PATCH v5 1/5] soc: renesas: rz-sysc: add syscon/regmap
> > support
> >
> > 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, let's register a custom regmap configuration.
> >
> > Signed-off-by: John Madieu <john.madieu.xa@bp.renesas.com>
> > ---
> > v1 -> v2: no changes
> > v2 -> v3: no changes
> > v3 -> v4: no changes
> > v4 -> v5: no changes
> >
> > Note for Maintainers: There is a false positive warning reported by
> > checkpatch.pl on this patch patch stating that the regmap_config struct
> should be const, despite the fact it's updated in probe().
> >
> >  drivers/soc/renesas/Kconfig         |  1 +
> >  drivers/soc/renesas/r9a09g047-sys.c |  1 +
> >  drivers/soc/renesas/rz-sysc.c       | 30 ++++++++++++++++++++++++++++-
> >  drivers/soc/renesas/rz-sysc.h       |  2 ++
> >  4 files changed, 33 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/soc/renesas/Kconfig b/drivers/soc/renesas/Kconfig
> > index 49648cf28bd2..3ffd3a4ca18d 100644
> > --- a/drivers/soc/renesas/Kconfig
> > +++ b/drivers/soc/renesas/Kconfig
> > @@ -388,6 +388,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/r9a09g047-sys.c
> > b/drivers/soc/renesas/r9a09g047-sys.c
> > index cd2eb7782cfe..5b010a519fab 100644
> > --- a/drivers/soc/renesas/r9a09g047-sys.c
> > +++ b/drivers/soc/renesas/r9a09g047-sys.c
> > @@ -64,4 +64,5 @@ static const struct rz_sysc_soc_id_init_data
> > rzg3e_sys_soc_id_init_data __initco
> >
> >  const struct rz_sysc_init_data rzg3e_sys_init_data = {
> >  	.soc_id_init_data = &rzg3e_sys_soc_id_init_data,
> > +	.max_register_offset = 0x170c,
> 
>  Replace max_register_offset->rz_sysc_regmap
> >  };
> > diff --git a/drivers/soc/renesas/rz-sysc.c
> > b/drivers/soc/renesas/rz-sysc.c index 1c98da37b7d1..bcbc23da954b
> > 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"
> > @@ -81,6 +83,14 @@ static int rz_sysc_soc_init(struct rz_sysc *sysc,
> const struct of_device_id *mat
> >  	return 0;
> >  }
> >
> > +static struct regmap_config rz_sysc_regmap = {
> > +	.name = "rz_sysc_regs",
> > +	.reg_bits = 32,
> > +	.reg_stride = 4,
> > +	.val_bits = 32,
> > +	.fast_io = true,
> > +};
> > +
> 
> Move this to SoC specific file.
> 
> static const struct regmap_config rz_g3e_sysc_regmap = {
> 	.name = "rz_g3e_sysc_regs",
> 	.reg_bits = 32,
> 	.reg_stride = 4,
> 	.val_bits = 32,
> 	.fast_io = true,
>       .max_register_offset = 0x170c,
> };
> 

I've implemented this SoC-specific const regmap_config* approach,
and checkpatch does not warn anymore.

Geert, if you are Ok with this approach, I can send the v6
along with some other comment fixes that I've received.

> 
> >  static const struct of_device_id rz_sysc_match[] = {  #ifdef
> CONFIG_SYSC_R9A08G045
> >  	{ .compatible = "renesas,r9a08g045-sysc", .data =
> > &rzg3s_sysc_init_data }, @@ -97,14 +107,21 @@ 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;
> > +	if (!data)
> > +		return -EINVAL;
> > +
> >  	sysc = devm_kzalloc(dev, sizeof(*sysc), GFP_KERNEL);
> >  	if (!sysc)
> >  		return -ENOMEM;
> > @@ -114,7 +131,18 @@ 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 (data->max_register_offset) {
> 	If(data->rz_sysc_regmap)
> 
> > +		rz_sysc_regmap.max_register = data->max_register_offset;
> Drop it.
> 
> > +		regmap = devm_regmap_init_mmio(dev, sysc->base,
> &rz_sysc_regmap);
> 
> regmap = devm_regmap_init_mmio(dev, sysc->base, data->rz_sysc_regmap);
> 
> > +		if (IS_ERR(regmap))
> > +			return PTR_ERR(regmap);
> > +
> > +		ret = of_syscon_register_regmap(dev->of_node, regmap);
> > +	}
> > +
> > +	return ret;
> >  }
> >
> >  static struct platform_driver rz_sysc_driver = { diff --git
> > a/drivers/soc/renesas/rz-sysc.h b/drivers/soc/renesas/rz-sysc.h index
> > aa83948c5117..37a3bb2c87f8 100644
> > --- a/drivers/soc/renesas/rz-sysc.h
> > +++ b/drivers/soc/renesas/rz-sysc.h
> > @@ -34,9 +34,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
> > + * @max_register_offset: Maximum SYSC register offset to be used by
> > + the regmap config
> >   */
> >  struct rz_sysc_init_data {
> >  	const struct rz_sysc_soc_id_init_data *soc_id_init_data;
> > +	u32 max_register_offset;
> 
>       const struct regmap_config *rz_sysc_regmap;
> 
> Cheers,
> Biju
> 
> >  };
> >
> >  extern const struct rz_sysc_init_data rzg3e_sys_init_data;
> > --
> > 2.25.1

Regards,
John

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

[Claudiu Beznea]

Hi, John,

On 31.03.2025 00:49, John Madieu 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, let's
> register a custom regmap configuration.
> 
> Signed-off-by: John Madieu <john.madieu.xa@bp.renesas.com>
> ---
> v1 -> v2: no changes
> v2 -> v3: no changes
> v3 -> v4: no changes
> v4 -> v5: no changes
> 
> Note for Maintainers: There is a false positive warning reported by
> checkpatch.pl on this patch patch stating that the regmap_config struct
> should be const, despite the fact it's updated in probe().
> 
>  drivers/soc/renesas/Kconfig         |  1 +
>  drivers/soc/renesas/r9a09g047-sys.c |  1 +
>  drivers/soc/renesas/rz-sysc.c       | 30 ++++++++++++++++++++++++++++-
>  drivers/soc/renesas/rz-sysc.h       |  2 ++
>  4 files changed, 33 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/soc/renesas/Kconfig b/drivers/soc/renesas/Kconfig
> index 49648cf28bd2..3ffd3a4ca18d 100644
> --- a/drivers/soc/renesas/Kconfig
> +++ b/drivers/soc/renesas/Kconfig
> @@ -388,6 +388,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/r9a09g047-sys.c b/drivers/soc/renesas/r9a09g047-sys.c
> index cd2eb7782cfe..5b010a519fab 100644
> --- a/drivers/soc/renesas/r9a09g047-sys.c
> +++ b/drivers/soc/renesas/r9a09g047-sys.c
> @@ -64,4 +64,5 @@ static const struct rz_sysc_soc_id_init_data rzg3e_sys_soc_id_init_data __initco
>  
>  const struct rz_sysc_init_data rzg3e_sys_init_data = {
>  	.soc_id_init_data = &rzg3e_sys_soc_id_init_data,
> +	.max_register_offset = 0x170c,
>  };
> diff --git a/drivers/soc/renesas/rz-sysc.c b/drivers/soc/renesas/rz-sysc.c
> index 1c98da37b7d1..bcbc23da954b 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"
> @@ -81,6 +83,14 @@ static int rz_sysc_soc_init(struct rz_sysc *sysc, const struct of_device_id *mat
>  	return 0;
>  }
>  
> +static struct regmap_config rz_sysc_regmap = {
> +	.name = "rz_sysc_regs",
> +	.reg_bits = 32,
> +	.reg_stride = 4,
> +	.val_bits = 32,
> +	.fast_io = true,
> +};
> +
>  static const struct of_device_id rz_sysc_match[] = {
>  #ifdef CONFIG_SYSC_R9A08G045
>  	{ .compatible = "renesas,r9a08g045-sysc", .data = &rzg3s_sysc_init_data },
> @@ -97,14 +107,21 @@ 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;
> +	if (!data)
> +		return -EINVAL;
> +
>  	sysc = devm_kzalloc(dev, sizeof(*sysc), GFP_KERNEL);
>  	if (!sysc)
>  		return -ENOMEM;
> @@ -114,7 +131,18 @@ 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);

The return value of rz_sysc_soc_init() is lost in case
data->max_register_offset != 0. Is there a reason for this?

> +
> +	if (data->max_register_offset) {
> +		rz_sysc_regmap.max_register = data->max_register_offset;
> +		regmap = devm_regmap_init_mmio(dev, sysc->base, &rz_sysc_regmap);
> +		if (IS_ERR(regmap))
> +			return PTR_ERR(regmap);
> +
> +		ret = of_syscon_register_regmap(dev->of_node, regmap);
> +	}
> +
> +	return ret;
>  }
>  
>  static struct platform_driver rz_sysc_driver = {
> diff --git a/drivers/soc/renesas/rz-sysc.h b/drivers/soc/renesas/rz-sysc.h
> index aa83948c5117..37a3bb2c87f8 100644
> --- a/drivers/soc/renesas/rz-sysc.h
> +++ b/drivers/soc/renesas/rz-sysc.h
> @@ -34,9 +34,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
> + * @max_register_offset: Maximum SYSC register offset to be used by the regmap config
>   */
>  struct rz_sysc_init_data {
>  	const struct rz_sysc_soc_id_init_data *soc_id_init_data;
> +	u32 max_register_offset;
>  };
>  
>  extern const struct rz_sysc_init_data rzg3e_sys_init_data;

RE: [PATCH v5 1/5] soc: renesas: rz-sysc: add syscon/regmap support

[John Madieu]

Hi Claudiu,

Thanks for the feedback.

> -----Original Message-----
> From: Claudiu Beznea <claudiu.beznea@tuxon.dev>
> Sent: Thursday, April 3, 2025 10:04 AM
> To: John Madieu <john.madieu.xa@bp.renesas.com>; geert+renesas@glider.be;
> conor+dt@kernel.org; krzk+dt@kernel.org; robh@kernel.org;
> rafael@kernel.org; daniel.lezcano@linaro.org
> Cc: magnus.damm@gmail.com; devicetree@vger.kernel.org;
> john.madieu@gmail.com; rui.zhang@intel.com; linux-kernel@vger.kernel.org;
> linux-renesas-soc@vger.kernel.org; sboyd@kernel.org; Biju Das
> <biju.das.jz@bp.renesas.com>; linux-pm@vger.kernel.org;
> lukasz.luba@arm.com
> Subject: Re: [PATCH v5 1/5] soc: renesas: rz-sysc: add syscon/regmap
> support
> 
> Hi, John,
> 
> On 31.03.2025 00:49, John Madieu 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, let's register a custom regmap configuration.
> >
> > Signed-off-by: John Madieu <john.madieu.xa@bp.renesas.com>
> > ---
> > v1 -> v2: no changes
> > v2 -> v3: no changes
> > v3 -> v4: no changes
> > v4 -> v5: no changes
> >
> > Note for Maintainers: There is a false positive warning reported by
> > checkpatch.pl on this patch patch stating that the regmap_config
> > struct should be const, despite the fact it's updated in probe().
> >
> >  drivers/soc/renesas/Kconfig         |  1 +
> >  drivers/soc/renesas/r9a09g047-sys.c |  1 +
> >  drivers/soc/renesas/rz-sysc.c       | 30 ++++++++++++++++++++++++++++-
> >  drivers/soc/renesas/rz-sysc.h       |  2 ++
> >  4 files changed, 33 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/soc/renesas/Kconfig b/drivers/soc/renesas/Kconfig
> > index 49648cf28bd2..3ffd3a4ca18d 100644
> > --- a/drivers/soc/renesas/Kconfig
> > +++ b/drivers/soc/renesas/Kconfig
> > @@ -388,6 +388,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/r9a09g047-sys.c
> > b/drivers/soc/renesas/r9a09g047-sys.c
> > index cd2eb7782cfe..5b010a519fab 100644
> > --- a/drivers/soc/renesas/r9a09g047-sys.c
> > +++ b/drivers/soc/renesas/r9a09g047-sys.c
> > @@ -64,4 +64,5 @@ static const struct rz_sysc_soc_id_init_data
> > rzg3e_sys_soc_id_init_data __initco
> >
> >  const struct rz_sysc_init_data rzg3e_sys_init_data = {
> >  	.soc_id_init_data = &rzg3e_sys_soc_id_init_data,
> > +	.max_register_offset = 0x170c,
> >  };
> > diff --git a/drivers/soc/renesas/rz-sysc.c
> > b/drivers/soc/renesas/rz-sysc.c index 1c98da37b7d1..bcbc23da954b
> > 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"
> > @@ -81,6 +83,14 @@ static int rz_sysc_soc_init(struct rz_sysc *sysc,
> const struct of_device_id *mat
> >  	return 0;
> >  }
> >
> > +static struct regmap_config rz_sysc_regmap = {
> > +	.name = "rz_sysc_regs",
> > +	.reg_bits = 32,
> > +	.reg_stride = 4,
> > +	.val_bits = 32,
> > +	.fast_io = true,
> > +};
> > +
> >  static const struct of_device_id rz_sysc_match[] = {  #ifdef
> > CONFIG_SYSC_R9A08G045
> >  	{ .compatible = "renesas,r9a08g045-sysc", .data =
> > &rzg3s_sysc_init_data }, @@ -97,14 +107,21 @@ 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;
> > +	if (!data)
> > +		return -EINVAL;
> > +
> >  	sysc = devm_kzalloc(dev, sizeof(*sysc), GFP_KERNEL);
> >  	if (!sysc)
> >  		return -ENOMEM;
> > @@ -114,7 +131,18 @@ 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);
> 
> The return value of rz_sysc_soc_init() is lost in case
> data->max_register_offset != 0. Is there a reason for this?

There is no reason for this. Will fix it for v6.

> 
> > +
> > +	if (data->max_register_offset) {
> > +		rz_sysc_regmap.max_register = data->max_register_offset;
> > +		regmap = devm_regmap_init_mmio(dev, sysc->base,
> &rz_sysc_regmap);
> > +		if (IS_ERR(regmap))
> > +			return PTR_ERR(regmap);
> > +
> > +		ret = of_syscon_register_regmap(dev->of_node, regmap);
> > +	}
> > +
> > +	return ret;
> >  }
> >
> >  static struct platform_driver rz_sysc_driver = { diff --git
> > a/drivers/soc/renesas/rz-sysc.h b/drivers/soc/renesas/rz-sysc.h index
> > aa83948c5117..37a3bb2c87f8 100644
> > --- a/drivers/soc/renesas/rz-sysc.h
> > +++ b/drivers/soc/renesas/rz-sysc.h
> > @@ -34,9 +34,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
> > + * @max_register_offset: Maximum SYSC register offset to be used by
> > + the regmap config
> >   */
> >  struct rz_sysc_init_data {
> >  	const struct rz_sysc_soc_id_init_data *soc_id_init_data;
> > +	u32 max_register_offset;
> >  };
> >
> >  extern const struct rz_sysc_init_data rzg3e_sys_init_data;

Regards,
John

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

[ALOK TIWARI]

Hi John,

On 03-04-2025 00:52, John Madieu wrote:
> Hi Alok,
> 
>> -----Original Message-----
>> From: ALOK TIWARI <alok.a.tiwari@oracle.com>
>> Sent: Tuesday, April 1, 2025 8:24 PM
>> To: John Madieu <john.madieu.xa@bp.renesas.com>; geert+renesas@glider.be;
>> conor+dt@kernel.org; krzk+dt@kernel.org; robh@kernel.org;
>> rafael@kernel.org; daniel.lezcano@linaro.org
>> Subject: Re : [PATCH v5 3/5] thermal: renesas: rzg3e: Add thermal driver
>> for the Renesas RZ/G3E SoC
>>
>> Hi John,
>>
>> Thanks for your reply.
>>
>> On 01-04-2025 17:35, John Madieu wrote:
>>> Hi Alok,
>>>
>>> Thanks for your feedback.
>>>
>>>> -----Original Message-----
>>>> From: ALOK TIWARI <alok.a.tiwari@oracle.com>
>>>> Sent: Monday, March 31, 2025 8:11 PM
>>>> To: John Madieu <john.madieu.xa@bp.renesas.com>;
>>>> geert+renesas@glider.be;
>>>> conor+dt@kernel.org; krzk+dt@kernel.org; robh@kernel.org;
>>>> rafael@kernel.org; daniel.lezcano@linaro.org
>>>> Subject: Re : [PATCH v5 3/5] thermal: renesas: rzg3e: Add thermal
>>>> driver for the Renesas RZ/G3E SoC
>>>>
>>>>
>>>>
>>>> On 31-03-2025 03:19, John Madieu wrote:
>>>>> The RZ/G3E SoC integrates a Temperature Sensor Unit (TSU) block
>>
>>>>> +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;
>>>>> +}
>>>>> +
>>>> always return 0 here ? what, if (!priv) return -EINVAL; ?
>>>
>>> priv cannot be NULL here, guaranteed from probe().
>>> Returning 0 here is expected by the thermal framework to notify ops
>>> success.
>>>
>>
>> I agreed. priv cannot be NULL.
>> It appears that return 0 is deliberate in this case. can we add Meaningful
>> comment which help to code readability.
>>
> 
> This is a standard tzd ops, which expects 0 to be returned
> for disabling/enabling success. This is how it's implemented
> in every Thermal driver I've gone through in the -next tree.
> 
>> not sure if user call ioctl(fd, THERMAL_IOC_SET_MODE, 2) and it returns
>> 0 with thermal enable.
>> that create possibility to thermal_core call thermal_notify_tz_disable if
>> any case
>>
> Am I missing something ?
> 

That's fine. it is not supporting ioctl.
usually thermal core driver do check if(mode == THERMAL_DEVICE_ENABLED) 
condition, else part is by-default.
as it is supporting only enable/disable case from 
/sys/class/thermal/thermal_zone0/mode. it does not create any issue.
LGTM.

>>>>> +static const struct thermal_zone_device_ops rzg3e_tz_ops = {
>>>>> +	.get_temp = rzg3e_thermal_get_temp,
>>>>> +	.set_trips = rzg3e_thermal_set_trips,
>>>>> +	.change_mode = rzg3e_thermal_change_mode, };
>>>> other renesas driver defined as rzg2l_tz_of_ops, can be used similar
>>>> one rzg3e_tz_of_ops for consistency!
>>>
>>> Thanks for pointing it out. Makes sense. Will double check and update
>>> accordingly.
>>>
>>>>> +
>>>>> +static int rzg3e_thermal_probe(struct platform_device *pdev) {
>>>>> +	struct device *dev = &pdev->dev;
>>>>> +	struct rzg3e_thermal_priv *priv;
>>>>> +	struct reset_control *rstc;
>>>>
> Regards,
> John

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

[ALOK TIWARI]

Hi John,

On 03-04-2025 00:52, John Madieu wrote:
> Hi Alok,
> 
>> -----Original Message-----
>> From: ALOK TIWARI <alok.a.tiwari@oracle.com>
>> Sent: Tuesday, April 1, 2025 8:24 PM
>> To: John Madieu <john.madieu.xa@bp.renesas.com>; geert+renesas@glider.be;
>> conor+dt@kernel.org; krzk+dt@kernel.org; robh@kernel.org;
>> rafael@kernel.org; daniel.lezcano@linaro.org
>> Subject: Re : [PATCH v5 3/5] thermal: renesas: rzg3e: Add thermal driver
>> for the Renesas RZ/G3E SoC
>>
>> Hi John,
>>
>> Thanks for your reply.
>>
>> On 01-04-2025 17:35, John Madieu wrote:
>>> Hi Alok,
>>>
>>> Thanks for your feedback.
>>>
>>>> -----Original Message-----
>>>> From: ALOK TIWARI <alok.a.tiwari@oracle.com>
>>>> Sent: Monday, March 31, 2025 8:11 PM
>>>> To: John Madieu <john.madieu.xa@bp.renesas.com>;
>>>> geert+renesas@glider.be;
>>>> conor+dt@kernel.org; krzk+dt@kernel.org; robh@kernel.org;
>>>> rafael@kernel.org; daniel.lezcano@linaro.org
>>>> Subject: Re : [PATCH v5 3/5] thermal: renesas: rzg3e: Add thermal
>>>> driver for the Renesas RZ/G3E SoC
>>>>
>>>>
>>>>
>>>> On 31-03-2025 03:19, John Madieu wrote:
>>>>> The RZ/G3E SoC integrates a Temperature Sensor Unit (TSU) block
>>
>>>>> +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;
>>>>> +}
>>>>> +
>>>> always return 0 here ? what, if (!priv) return -EINVAL; ?
>>>
>>> priv cannot be NULL here, guaranteed from probe().
>>> Returning 0 here is expected by the thermal framework to notify ops
>>> success.
>>>
>>
>> I agreed. priv cannot be NULL.
>> It appears that return 0 is deliberate in this case. can we add Meaningful
>> comment which help to code readability.
>>
> 
> This is a standard tzd ops, which expects 0 to be returned
> for disabling/enabling success. This is how it's implemented
> in every Thermal driver I've gone through in the -next tree.
> 
>> not sure if user call ioctl(fd, THERMAL_IOC_SET_MODE, 2) and it returns
>> 0 with thermal enable.
>> that create possibility to thermal_core call thermal_notify_tz_disable if
>> any case
>>
> Am I missing something ?
> 

That's fine. it is not supporting ioctl.
usually thermal core driver do check if(mode == THERMAL_DEVICE_ENABLED) 
condition, else part is by-default.
as it is supporting only enable/disable case from 
/sys/class/thermal/thermal_zone0/mode. it does not create any issue.
LGTM.

Thanks,
Alok
>>>>> +static const struct thermal_zone_device_ops rzg3e_tz_ops = {
>>>>> +	.get_temp = rzg3e_thermal_get_temp,
>>>>> +	.set_trips = rzg3e_thermal_set_trips,
>>>>> +	.change_mode = rzg3e_thermal_change_mode, };
>>>> other renesas driver defined as rzg2l_tz_of_ops, can be used similar
>>>> one rzg3e_tz_of_ops for consistency!
>>>
>>> Thanks for pointing it out. Makes sense. Will double check and update
>>> accordingly.
>>>
>>>>> +
>>>>> +static int rzg3e_thermal_probe(struct platform_device *pdev) {
>>>>> +	struct device *dev = &pdev->dev;
>>>>> +	struct rzg3e_thermal_priv *priv;
>>>>> +	struct reset_control *rstc;
>>>>
> Regards,
> John