[PATCH v2 0/7] thermal: renesas: Add support fot RZ/G3E

[PATCH v2 0/7] thermal: renesas: Add support fot RZ/G3E

[John Madieu]

Hello,

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

The series consists of 7 patches (some of which are not related to the thermal
framework) that progressively add TSU support as follows:
- patch 1/7:    adds syscon/regmap support for accessing system controller
                registers, enabling access to TSU calibration values
- patch 2/7:    adds clock and reset signals to the CPG driver

- patch 3/7:    adds dt-bindings
- patch 4/7:    adds the actual TSU driver for the RZ/G3E
- patch 5/6:    adds safety mechanism to make sure we we protect the chip in
                case of consecutive read failures
- patch 6-7/7:  add DT node and defconfig enablement


Changes:

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

Regards,

John Madieu (7):
  soc: renesas: rz-sysc: add syscon/regmap support
  clk: renesas: r9a09g047: Add clock and reset signals for the TSU IP
  dt-bindings: thermal: r9a09g047-tsu: Document the TSU unit
  thermal: renesas: rzg3e: Add thermal driver for the Renesas RZ/G3E SoC
  thermal: renesas: rzg3e: Add safety check when reading temperature
  arm64: dts: renesas: r9a09g047: Add TSU node
  arm64: defconfig: Enable RZ/G3E thermal

 .../thermal/renesas,r9a09g047-tsu.yaml        | 123 +++++
 MAINTAINERS                                   |   7 +
 arch/arm64/boot/dts/renesas/r9a09g047.dtsi    |  49 ++
 arch/arm64/configs/defconfig                  |   1 +
 drivers/clk/renesas/r9a09g047-cpg.c           |   3 +
 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       | 479 ++++++++++++++++++
 12 files changed, 703 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 v2 1/7] 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

 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 v2 2/7] clk: renesas: r9a09g047: Add clock and reset signals for the TSU IP

[John Madieu]

Add required clocks and resets signals for the TSU IP available on the
Renesas RZ/G3E SoC

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

 drivers/clk/renesas/r9a09g047-cpg.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/clk/renesas/r9a09g047-cpg.c b/drivers/clk/renesas/r9a09g047-cpg.c
index 51fd24c20ed5..ada57964c132 100644
--- a/drivers/clk/renesas/r9a09g047-cpg.c
+++ b/drivers/clk/renesas/r9a09g047-cpg.c
@@ -154,6 +154,8 @@ static const struct rzv2h_mod_clk r9a09g047_mod_clks[] __initconst = {
 						BUS_MSTOP(8, BIT(4))),
 	DEF_MOD("sdhi_2_aclk",			CLK_PLLDTY_ACPU_DIV4, 10, 14, 5, 14,
 						BUS_MSTOP(8, BIT(4))),
+	DEF_MOD("tsu_1_pclk",			CLK_QEXTAL, 16, 10, 8, 10,
+						BUS_MSTOP(2, BIT(15))),
 };
 
 static const struct rzv2h_reset r9a09g047_resets[] __initconst = {
@@ -177,6 +179,7 @@ static const struct rzv2h_reset r9a09g047_resets[] __initconst = {
 	DEF_RST(10, 7, 4, 24),		/* SDHI_0_IXRST */
 	DEF_RST(10, 8, 4, 25),		/* SDHI_1_IXRST */
 	DEF_RST(10, 9, 4, 26),		/* SDHI_2_IXRST */
+	DEF_RST(15, 8, 7, 9),		/* TSU_1_PRESETN */
 };
 
 const struct rzv2h_cpg_info r9a09g047_cpg_info __initconst = {
-- 
2.25.1

[PATCH v2 3/7] 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.

Signed-off-by: John Madieu <john.madieu.xa@bp.renesas.com>
---
v1 -> v2:
 * Fix reg property specifier to get rid of yamlint warnings
 * Fix IRQ name to reflect TSU expectations

 .../thermal/renesas,r9a09g047-tsu.yaml        | 123 ++++++++++++++++++
 1 file changed, 123 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..e786561ddbe3
--- /dev/null
+++ b/Documentation/devicetree/bindings/thermal/renesas,r9a09g047-tsu.yaml
@@ -0,0 +1,123 @@
+# 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:
+    description: |
+      Interrupt specifiers for the TSU:
+      - S12TSUADI1: Conversion complete interrupt signal (pulse)
+      - S12TSUADCMPI1: 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.
+
+  renesas,tsu-operating-mode:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    enum: [0, 1]
+    description: |
+      TSU operating mode:
+      0: Mode 0 - Conversion started by software
+      1: Mode 1 - Conversion started by ELC trigger
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - resets
+  - power-domains
+  - interrupts
+  - interrupt-names
+  - "#thermal-sensor-cells"
+  - renesas,tsu-operating-mode
+  - 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-operating-mode = <0>;
+        renesas,tsu-calibration-sys = <&sys>;
+    };
+
+    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";
+                };
+            };
+        };
+    };
-- 
2.25.1

[PATCH v2 4/7] 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: fix IRQ names

 MAINTAINERS                             |   7 +
 drivers/thermal/renesas/Kconfig         |   7 +
 drivers/thermal/renesas/Makefile        |   1 +
 drivers/thermal/renesas/rzg3e_thermal.c | 445 ++++++++++++++++++++++++
 4 files changed, 460 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..be9e1d118a67
--- /dev/null
+++ b/drivers/thermal/renesas/rzg3e_thermal.c
@@ -0,0 +1,445 @@
+// 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 v2 5/7] thermal: renesas: rzg3e: Add safety check when reading temperature

[John Madieu]

Becaure reading temperature may fail, add mechanism to panic in case
reading the temperature fails after a given number of trials. This is due
to the thermal core disabling the thermal zone device after a couple of
consecutive attempt failures.

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

 drivers/thermal/renesas/rzg3e_thermal.c | 38 +++++++++++++++++++++++--
 1 file changed, 36 insertions(+), 2 deletions(-)

diff --git a/drivers/thermal/renesas/rzg3e_thermal.c b/drivers/thermal/renesas/rzg3e_thermal.c
index be9e1d118a67..ff80d1b517c8 100644
--- a/drivers/thermal/renesas/rzg3e_thermal.c
+++ b/drivers/thermal/renesas/rzg3e_thermal.c
@@ -83,6 +83,19 @@
 #define TSU_TIMEOUT_US		10000
 #define TSU_MIN_CLOCK_RATE	24000000
 
+/*
+ * Number of consecutive errors before shutdown
+ *
+ * While simulating thermal sensor failure, we have noticed that the thermal
+ * core tries to fetch the temperature a couple times and then disable the
+ * thermal zone device. In case of extreme heat, this might lead to SoC
+ * destruction.
+ *
+ * Let's prevent this by limitating the number of failure and panic in
+ * case it happens.
+ */
+#define MAX_TEMP_READ_ERRORS       10
+
 /**
  * struct rzg3e_thermal_priv - RZ/G3E thermal private data structure
  * @base: TSU base address
@@ -93,6 +106,7 @@
  * @conv_complete: ADC conversion completion
  * @reg_lock: protect shared register access
  * @cached_temp: last computed temperature (milliCelsius)
+ * @error_count: Track consecutive errors
  * @trmval: trim (calibration) values
  */
 struct rzg3e_thermal_priv {
@@ -104,6 +118,7 @@ struct rzg3e_thermal_priv {
 	struct completion conv_complete;
 	spinlock_t reg_lock;
 	int cached_temp;
+	atomic_t error_count;
 	u32 trmval[2];
 };
 
@@ -200,6 +215,7 @@ static irqreturn_t rzg3e_thermal_adc_irq(int irq, void *dev_id)
 static int rzg3e_thermal_get_temp(struct thermal_zone_device *zone, int *temp)
 {
 	struct rzg3e_thermal_priv *priv = thermal_zone_device_priv(zone);
+	int error_count;
 	u32 val;
 	int ret;
 
@@ -217,7 +233,7 @@ static int rzg3e_thermal_get_temp(struct thermal_zone_device *zone, int *temp)
 					TSU_POLL_DELAY_US, TSU_TIMEOUT_US);
 	if (ret) {
 		dev_err(priv->dev, "ADC conversion timed out\n");
-		return ret;
+		goto handle_error;
 	}
 
 	/* Start conversion */
@@ -225,15 +241,33 @@ static int rzg3e_thermal_get_temp(struct thermal_zone_device *zone, int *temp)
 
 	if (!wait_for_completion_timeout(&priv->conv_complete,
 					 msecs_to_jiffies(100))) {
+		ret = -ETIMEDOUT;
 		dev_err(priv->dev, "ADC conversion completion timeout\n");
-		return -ETIMEDOUT;
+		goto handle_error;
 	}
 
 	scoped_guard(spinlock_irqsave, &priv->reg_lock) {
 		*temp = priv->cached_temp;
 	}
 
+	/* Reset error count on successful read */
+	atomic_set(&priv->error_count, 0);
 	return 0;
+
+handle_error:
+	error_count = atomic_inc_return(&priv->error_count);
+	if (error_count >= MAX_TEMP_READ_ERRORS) {
+		dev_emerg(priv->dev,
+			"Failed to read temperature %d times, initiating emergency shutdown\n",
+			error_count);
+		mdelay(100);
+		panic("Temperature sensor failure - emergency shutdown");
+	}
+
+	dev_err(priv->dev, "Failed to read temperature (error %d), attempt %d/%d\n",
+		ret, error_count, MAX_TEMP_READ_ERRORS);
+
+	return ret;
 }
 
 /* Convert temperature in milliCelsius to raw sensor code */
-- 
2.25.1

[PATCH v2 6/7] 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

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

diff --git a/arch/arm64/boot/dts/renesas/r9a09g047.dtsi b/arch/arm64/boot/dts/renesas/r9a09g047.dtsi
index a6b83e057a40..93b57d7ad7b9 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,20 @@ 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-operating-mode = <0>;
+			renesas,tsu-calibration-sys = <&sys>;
+		};
+
 		i2c0: i2c@14400400 {
 			compatible = "renesas,riic-r9a09g047", "renesas,riic-r9a09g057";
 			reg = <0 0x14400400 0 0x400>;
@@ -502,6 +520,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 v2 7/7] arm64: defconfig: Enable RZ/G3E thermal

[John Madieu]

Enable the CONFIG_RZG3E_THERMAL flag for the RZ/G3E SoC.

Signed-off-by: John Madieu <john.madieu.xa@bp.renesas.com>
---
v1 -> v2: 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 v2 3/7] dt-bindings: thermal: r9a09g047-tsu: Document the TSU unit

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 5025 bytes --]

On Thu, Feb 27, 2025 at 01:24:39PM +0100, John Madieu wrote:
> 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.
> 
> Signed-off-by: John Madieu <john.madieu.xa@bp.renesas.com>
> ---
> v1 -> v2:
>  * Fix reg property specifier to get rid of yamlint warnings
>  * Fix IRQ name to reflect TSU expectations
> 
>  .../thermal/renesas,r9a09g047-tsu.yaml        | 123 ++++++++++++++++++
>  1 file changed, 123 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..e786561ddbe3
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/thermal/renesas,r9a09g047-tsu.yaml
> @@ -0,0 +1,123 @@
> +# 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:
> +    description: |
> +      Interrupt specifiers for the TSU:
> +      - S12TSUADI1: Conversion complete interrupt signal (pulse)
> +      - S12TSUADCMPI1: 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.
> +
> +  renesas,tsu-operating-mode:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    enum: [0, 1]
> +    description: |
> +      TSU operating mode:
> +      0: Mode 0 - Conversion started by software
> +      1: Mode 1 - Conversion started by ELC trigger

Can you make this "software" and "elc" or something please, unless
people will genuinely find "0" and 1" to be more informative.
And why doesn't the property have a default?

cheers,
Conor.

> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +  - resets
> +  - power-domains
> +  - interrupts
> +  - interrupt-names
> +  - "#thermal-sensor-cells"
> +  - renesas,tsu-operating-mode
> +  - 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-operating-mode = <0>;
> +        renesas,tsu-calibration-sys = <&sys>;
> +    };
> +
> +    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";
> +                };
> +            };
> +        };
> +    };
> -- 
> 2.25.1
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v2 3/7] dt-bindings: thermal: r9a09g047-tsu: Document the TSU unit

[Rob Herring]

On Fri, Feb 28, 2025 at 07:03:05PM +0000, Conor Dooley wrote:
> On Thu, Feb 27, 2025 at 01:24:39PM +0100, John Madieu wrote:
> > 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.
> > 
> > Signed-off-by: John Madieu <john.madieu.xa@bp.renesas.com>
> > ---
> > v1 -> v2:
> >  * Fix reg property specifier to get rid of yamlint warnings
> >  * Fix IRQ name to reflect TSU expectations
> > 
> >  .../thermal/renesas,r9a09g047-tsu.yaml        | 123 ++++++++++++++++++
> >  1 file changed, 123 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..e786561ddbe3
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/thermal/renesas,r9a09g047-tsu.yaml
> > @@ -0,0 +1,123 @@
> > +# 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:
> > +    description: |
> > +      Interrupt specifiers for the TSU:
> > +      - S12TSUADI1: Conversion complete interrupt signal (pulse)
> > +      - S12TSUADCMPI1: 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.
> > +
> > +  renesas,tsu-operating-mode:
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    enum: [0, 1]
> > +    description: |
> > +      TSU operating mode:
> > +      0: Mode 0 - Conversion started by software
> > +      1: Mode 1 - Conversion started by ELC trigger
> 
> Can you make this "software" and "elc" or something please, unless
> people will genuinely find "0" and 1" to be more informative.
> And why doesn't the property have a default?

Or just make it boolean.

Who/what decides the mode? If a user is going to want to change this, 
then it should be a runtime control, not a DT property.

Rob

Re: [PATCH v2 2/7] clk: renesas: r9a09g047: Add clock and reset signals for the TSU IP

[Geert Uytterhoeven]

On Thu, 27 Feb 2025 at 13:25, John Madieu <john.madieu.xa@bp.renesas.com> wrote:
> Add required clocks and resets signals for the TSU IP available on the
> Renesas RZ/G3E SoC
>
> Signed-off-by: John Madieu <john.madieu.xa@bp.renesas.com>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
i.e. will queue in renesas-clk for v6.15.

Gr{oetje,eeting}s,

                        Geert

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

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

RE: [PATCH v2 3/7] dt-bindings: thermal: r9a09g047-tsu: Document the TSU unit

[John Madieu]

Hi Conor,

Thanks for your review!

> -----Original Message-----
> From: Conor Dooley <conor@kernel.org>
> Sent: Friday, February 28, 2025 8:03 PM
> To: John Madieu <john.madieu.xa@bp.renesas.com>
> Subject: Re: [PATCH v2 3/7] dt-bindings: thermal: r9a09g047-tsu: Document
> the TSU unit
> 
> On Thu, Feb 27, 2025 at 01:24:39PM +0100, John Madieu wrote:
> > 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.
> >
> > Signed-off-by: John Madieu <john.madieu.xa@bp.renesas.com>
> > ---
> > v1 -> v2:
> >  * Fix reg property specifier to get rid of yamlint warnings
> >  * Fix IRQ name to reflect TSU expectations
> >
> >  .../thermal/renesas,r9a09g047-tsu.yaml        | 123 ++++++++++++++++++
> >  1 file changed, 123 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..e786561ddbe3
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/thermal/renesas,r9a09g047-tsu.
> > +++ yaml
> > @@ -0,0 +1,123 @@
> > +# 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:
> > +    description: |
> > +      Interrupt specifiers for the TSU:
> > +      - S12TSUADI1: Conversion complete interrupt signal (pulse)
> > +      - S12TSUADCMPI1: 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.
> > +
> > +  renesas,tsu-operating-mode:
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    enum: [0, 1]
> > +    description: |
> > +      TSU operating mode:
> > +      0: Mode 0 - Conversion started by software
> > +      1: Mode 1 - Conversion started by ELC trigger
> 
> Can you make this "software" and "elc" or something please, unless people
> will genuinely find "0" and 1" to be more informative.
> And why doesn't the property have a default?

Sorry for miss-specifying.
ELC is an external event trigger. May be should I specify it like that ?

To make sure I got your point, do you mean specifying a default value in
bindings ?

> 
> cheers,
> Conor.

Regards,
John

> 
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - clocks
> > +  - resets
> > +  - power-domains
> > +  - interrupts
> > +  - interrupt-names
> > +  - "#thermal-sensor-cells"
> > +  - renesas,tsu-operating-mode
> > +  - 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-operating-mode = <0>;
> > +        renesas,tsu-calibration-sys = <&sys>;
> > +    };
> > +
> > +    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";
> > +                };
> > +            };
> > +        };
> > +    };
> > --
> > 2.25.1
> >

Re: [PATCH v2 3/7] dt-bindings: thermal: r9a09g047-tsu: Document the TSU unit

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 4575 bytes --]

On Fri, Mar 07, 2025 at 03:14:05PM +0000, John Madieu wrote:
> Hi Conor,
> 
> Thanks for your review!
> 
> > -----Original Message-----
> > From: Conor Dooley <conor@kernel.org>
> > Sent: Friday, February 28, 2025 8:03 PM
> > To: John Madieu <john.madieu.xa@bp.renesas.com>
> > Subject: Re: [PATCH v2 3/7] dt-bindings: thermal: r9a09g047-tsu: Document
> > the TSU unit
> > 
> > On Thu, Feb 27, 2025 at 01:24:39PM +0100, John Madieu wrote:
> > > 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.
> > >
> > > Signed-off-by: John Madieu <john.madieu.xa@bp.renesas.com>
> > > ---
> > > v1 -> v2:
> > >  * Fix reg property specifier to get rid of yamlint warnings
> > >  * Fix IRQ name to reflect TSU expectations
> > >
> > >  .../thermal/renesas,r9a09g047-tsu.yaml        | 123 ++++++++++++++++++
> > >  1 file changed, 123 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..e786561ddbe3
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/thermal/renesas,r9a09g047-tsu.
> > > +++ yaml
> > > @@ -0,0 +1,123 @@
> > > +# 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:
> > > +    description: |
> > > +      Interrupt specifiers for the TSU:
> > > +      - S12TSUADI1: Conversion complete interrupt signal (pulse)
> > > +      - S12TSUADCMPI1: 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.
> > > +
> > > +  renesas,tsu-operating-mode:
> > > +    $ref: /schemas/types.yaml#/definitions/uint32
> > > +    enum: [0, 1]
> > > +    description: |
> > > +      TSU operating mode:
> > > +      0: Mode 0 - Conversion started by software
> > > +      1: Mode 1 - Conversion started by ELC trigger
> > 
> > Can you make this "software" and "elc" or something please, unless people
> > will genuinely find "0" and 1" to be more informative.
> > And why doesn't the property have a default?
> 
> Sorry for miss-specifying.
> ELC is an external event trigger. May be should I specify it like that ?

If "elc trigger" is meaningful to people using hte device (IOW, it
matches datasheet wording) then that's fine I think.

> To make sure I got your point, do you mean specifying a default value in
> bindings ?

The property doesn't actually need to be required, it could easily have
a default (say software) and only be set in the case of using the elc
trigger - which brings you to Rob's comment that it can just be a
boolean, setting the property if elc and leaving it out of software.

Rob's other comment was

| Who/what decides the mode? If a user is going to want to change this, 
| then it should be a runtime control, not a DT property.

which I think needs an answer ;)

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

RE: [PATCH v2 3/7] dt-bindings: thermal: r9a09g047-tsu: Document the TSU unit

[John Madieu]

Hi Conor,

> -----Original Message-----
> From: Conor Dooley <conor@kernel.org>
> Sent: Friday, March 7, 2025 4:33 PM
> To: John Madieu <john.madieu.xa@bp.renesas.com>
> Subject: Re: [PATCH v2 3/7] dt-bindings: thermal: r9a09g047-tsu: Document
> the TSU unit
> 
> On Fri, Mar 07, 2025 at 03:14:05PM +0000, John Madieu wrote:
> > Hi Conor,
> >
> > Thanks for your review!
> >
> > > -----Original Message-----
> > > From: Conor Dooley <conor@kernel.org>
> > > Sent: Friday, February 28, 2025 8:03 PM
> > > To: John Madieu <john.madieu.xa@bp.renesas.com>
> > > Subject: Re: [PATCH v2 3/7] dt-bindings: thermal: r9a09g047-tsu:
> > > Document the TSU unit
> > >
> > > On Thu, Feb 27, 2025 at 01:24:39PM +0100, John Madieu wrote:
> > > > 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.
> > > >
> > > > Signed-off-by: John Madieu <john.madieu.xa@bp.renesas.com>
> > > > ---
> > > > v1 -> v2:
> > > >  * Fix reg property specifier to get rid of yamlint warnings
> > > >  * Fix IRQ name to reflect TSU expectations
> > > >
> > > > +    enum: [0, 1]
> > > > +    description: |
> > > > +      TSU operating mode:
> > > > +      0: Mode 0 - Conversion started by software
> > > > +      1: Mode 1 - Conversion started by ELC trigger
> > >
> > > Can you make this "software" and "elc" or something please, unless
> > > people will genuinely find "0" and 1" to be more informative.
> > > And why doesn't the property have a default?
> >
> > Sorry for miss-specifying.
> > ELC is an external event trigger. May be should I specify it like that ?
> 
> If "elc trigger" is meaningful to people using hte device (IOW, it matches
> datasheet wording) then that's fine I think.

"elc trigger" matches datasheet wording.

> 
> > To make sure I got your point, do you mean specifying a default value
> > in bindings ?
> 
> The property doesn't actually need to be required, it could easily have a
> default (say software) and only be set in the case of using the elc
> trigger - which brings you to Rob's comment that it can just be a boolean,
> setting the property if elc and leaving it out of software.

Got the point now. I can make it default to software trigger, and add optional
Boolean property to ELC trigger. Let's say "renesas,elc-trigger;"

> 
> Rob's other comment was
> 
> | Who/what decides the mode? If a user is going to want to change this,
> | then it should be a runtime control, not a DT property.

Changes are not possible at runtime. Some customers may want software,
while other may want the external trigger, and this is immutable 
configuration.

> 
> which I think needs an answer ;)

Regards,
John

Re: [PATCH v2 3/7] dt-bindings: thermal: r9a09g047-tsu: Document the TSU unit

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 3252 bytes --]

On Fri, Mar 07, 2025 at 03:55:27PM +0000, John Madieu wrote:
> Hi Conor,
> 
> > -----Original Message-----
> > From: Conor Dooley <conor@kernel.org>
> > Sent: Friday, March 7, 2025 4:33 PM
> > To: John Madieu <john.madieu.xa@bp.renesas.com>
> > Subject: Re: [PATCH v2 3/7] dt-bindings: thermal: r9a09g047-tsu: Document
> > the TSU unit
> > 
> > On Fri, Mar 07, 2025 at 03:14:05PM +0000, John Madieu wrote:
> > > Hi Conor,
> > >
> > > Thanks for your review!
> > >
> > > > -----Original Message-----
> > > > From: Conor Dooley <conor@kernel.org>
> > > > Sent: Friday, February 28, 2025 8:03 PM
> > > > To: John Madieu <john.madieu.xa@bp.renesas.com>
> > > > Subject: Re: [PATCH v2 3/7] dt-bindings: thermal: r9a09g047-tsu:
> > > > Document the TSU unit
> > > >
> > > > On Thu, Feb 27, 2025 at 01:24:39PM +0100, John Madieu wrote:
> > > > > 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.
> > > > >
> > > > > Signed-off-by: John Madieu <john.madieu.xa@bp.renesas.com>
> > > > > ---
> > > > > v1 -> v2:
> > > > >  * Fix reg property specifier to get rid of yamlint warnings
> > > > >  * Fix IRQ name to reflect TSU expectations
> > > > >
> > > > > +    enum: [0, 1]
> > > > > +    description: |
> > > > > +      TSU operating mode:
> > > > > +      0: Mode 0 - Conversion started by software
> > > > > +      1: Mode 1 - Conversion started by ELC trigger
> > > >
> > > > Can you make this "software" and "elc" or something please, unless
> > > > people will genuinely find "0" and 1" to be more informative.
> > > > And why doesn't the property have a default?
> > >
> > > Sorry for miss-specifying.
> > > ELC is an external event trigger. May be should I specify it like that ?
> > 
> > If "elc trigger" is meaningful to people using hte device (IOW, it matches
> > datasheet wording) then that's fine I think.
> 
> "elc trigger" matches datasheet wording.
> 
> > 
> > > To make sure I got your point, do you mean specifying a default value
> > > in bindings ?
> > 
> > The property doesn't actually need to be required, it could easily have a
> > default (say software) and only be set in the case of using the elc
> > trigger - which brings you to Rob's comment that it can just be a boolean,
> > setting the property if elc and leaving it out of software.
> 
> Got the point now. I can make it default to software trigger, and add optional
> Boolean property to ELC trigger. Let's say "renesas,elc-trigger;"

Yah, that works.

> 
> > 
> > Rob's other comment was
> > 
> > | Who/what decides the mode? If a user is going to want to change this,
> > | then it should be a runtime control, not a DT property.
> 
> Changes are not possible at runtime. Some customers may want software,
> while other may want the external trigger, and this is immutable 
> configuration.

What makes it immutable? Set by some wiring on the board? I couldn't
find the user in your driver patches to better understand how you were
using it.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

RE: [PATCH v2 3/7] dt-bindings: thermal: r9a09g047-tsu: Document the TSU unit

[John Madieu]

Hi Conor,

> -----Original Message-----
> From: Conor Dooley <conor@kernel.org>
> Sent: Friday, March 7, 2025 5:04 PM
> To: John Madieu <john.madieu.xa@bp.renesas.com>
> Subject: Re: [PATCH v2 3/7] dt-bindings: thermal: r9a09g047-tsu: Document
> the TSU unit
> 
> On Fri, Mar 07, 2025 at 03:55:27PM +0000, John Madieu wrote:
> > Hi Conor,
> >
> > > > > > Signed-off-by: John Madieu <john.madieu.xa@bp.renesas.com>
> > > > > > ---
> > > > > > v1 -> v2:
> > > > > >  * Fix reg property specifier to get rid of yamlint warnings
> > > > > >  * Fix IRQ name to reflect TSU expectations
> > > > > >
> > > > > > +    enum: [0, 1]
> > > > > > +    description: |
> > > > > > +      TSU operating mode:
> > > > > > +      0: Mode 0 - Conversion started by software
> > > > > > +      1: Mode 1 - Conversion started by ELC trigger
> > > > >
> > > > > Can you make this "software" and "elc" or something please,
> > > > > unless people will genuinely find "0" and 1" to be more
> informative.
> > > > > And why doesn't the property have a default?
> > > >
> > > > Sorry for miss-specifying.
> > > > ELC is an external event trigger. May be should I specify it like
> that ?
> > >
> > > If "elc trigger" is meaningful to people using hte device (IOW, it
> > > matches datasheet wording) then that's fine I think.
> >
> > "elc trigger" matches datasheet wording.
> >
> > >
> > > > To make sure I got your point, do you mean specifying a default
> > > > value in bindings ?
> > >
> > > The property doesn't actually need to be required, it could easily
> > > have a default (say software) and only be set in the case of using
> > > the elc trigger - which brings you to Rob's comment that it can just
> > > be a boolean, setting the property if elc and leaving it out of
> software.
> >
> > Got the point now. I can make it default to software trigger, and add
> > optional Boolean property to ELC trigger. Let's say "renesas,elc-
> trigger;"
> 
> Yah, that works.
> 
> >
> > >
> > > Rob's other comment was
> > >
> > > | Who/what decides the mode? If a user is going to want to change
> > > | this, then it should be a runtime control, not a DT property.
> >
> > Changes are not possible at runtime. Some customers may want software,
> > while other may want the external trigger, and this is immutable
> > configuration.
> 
> What makes it immutable? Set by some wiring on the board? I couldn't find
> the user in your driver patches to better understand how you were using
> it.

I haven't prototyped ELC trigger yet. Since the hardware manual
describes about ELC trigger, I have documented it in bindings. If you
think, it is not needed at this stage, then I can drop it now and
revisit later.

Regards,
John

Re: [PATCH v2 3/7] dt-bindings: thermal: r9a09g047-tsu: Document the TSU unit

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 3311 bytes --]

On Sun, Mar 09, 2025 at 10:39:27AM +0000, John Madieu wrote:
> Hi Conor,
> 
> > -----Original Message-----
> > From: Conor Dooley <conor@kernel.org>
> > Sent: Friday, March 7, 2025 5:04 PM
> > To: John Madieu <john.madieu.xa@bp.renesas.com>
> > Subject: Re: [PATCH v2 3/7] dt-bindings: thermal: r9a09g047-tsu: Document
> > the TSU unit
> > 
> > On Fri, Mar 07, 2025 at 03:55:27PM +0000, John Madieu wrote:
> > > Hi Conor,
> > >
> > > > > > > Signed-off-by: John Madieu <john.madieu.xa@bp.renesas.com>
> > > > > > > ---
> > > > > > > v1 -> v2:
> > > > > > >  * Fix reg property specifier to get rid of yamlint warnings
> > > > > > >  * Fix IRQ name to reflect TSU expectations
> > > > > > >
> > > > > > > +    enum: [0, 1]
> > > > > > > +    description: |
> > > > > > > +      TSU operating mode:
> > > > > > > +      0: Mode 0 - Conversion started by software
> > > > > > > +      1: Mode 1 - Conversion started by ELC trigger
> > > > > >
> > > > > > Can you make this "software" and "elc" or something please,
> > > > > > unless people will genuinely find "0" and 1" to be more
> > informative.
> > > > > > And why doesn't the property have a default?
> > > > >
> > > > > Sorry for miss-specifying.
> > > > > ELC is an external event trigger. May be should I specify it like
> > that ?
> > > >
> > > > If "elc trigger" is meaningful to people using hte device (IOW, it
> > > > matches datasheet wording) then that's fine I think.
> > >
> > > "elc trigger" matches datasheet wording.
> > >
> > > >
> > > > > To make sure I got your point, do you mean specifying a default
> > > > > value in bindings ?
> > > >
> > > > The property doesn't actually need to be required, it could easily
> > > > have a default (say software) and only be set in the case of using
> > > > the elc trigger - which brings you to Rob's comment that it can just
> > > > be a boolean, setting the property if elc and leaving it out of
> > software.
> > >
> > > Got the point now. I can make it default to software trigger, and add
> > > optional Boolean property to ELC trigger. Let's say "renesas,elc-
> > trigger;"
> > 
> > Yah, that works.
> > 
> > >
> > > >
> > > > Rob's other comment was
> > > >
> > > > | Who/what decides the mode? If a user is going to want to change
> > > > | this, then it should be a runtime control, not a DT property.
> > >
> > > Changes are not possible at runtime. Some customers may want software,
> > > while other may want the external trigger, and this is immutable
> > > configuration.
> > 
> > What makes it immutable? Set by some wiring on the board? I couldn't find
> > the user in your driver patches to better understand how you were using
> > it.
> 
> I haven't prototyped ELC trigger yet. Since the hardware manual
> describes about ELC trigger, I have documented it in bindings. If you
> think, it is not needed at this stage, then I can drop it now and
> revisit later.

Ideally a binding is complete, even if the driver isn't. To me
"immutable" would mean something like "the trigger type is determined by
hardware or firmware configuration", but if it is determined by register
writes (e.g. wired up for elc trigger, but you can opt for software
trigger in the driver) then it should be a userspace control.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

RE: [PATCH v2 3/7] dt-bindings: thermal: r9a09g047-tsu: Document the TSU unit

[John Madieu]

Hi Conor,

> -----Original Message-----
> From: Conor Dooley <conor@kernel.org>
> Sent: Monday, March 10, 2025 5:15 PM
> To: John Madieu <john.madieu.xa@bp.renesas.com>
> Subject: Re: [PATCH v2 3/7] dt-bindings: thermal: r9a09g047-tsu: Document
> the TSU unit
> 
> On Sun, Mar 09, 2025 at 10:39:27AM +0000, John Madieu wrote:
> > Hi Conor,
> > > > Changes are not possible at runtime. Some customers may want
> > > > software, while other may want the external trigger, and this is
> > > > immutable configuration.
> > >
> > > What makes it immutable? Set by some wiring on the board? I couldn't
> > > find the user in your driver patches to better understand how you
> > > were using it.
> >
> > I haven't prototyped ELC trigger yet. Since the hardware manual
> > describes about ELC trigger, I have documented it in bindings. If you
> > think, it is not needed at this stage, then I can drop it now and
> > revisit later.
> 
> Ideally a binding is complete, even if the driver isn't. To me "immutable"
> would mean something like "the trigger type is determined by hardware or
> firmware configuration", but if it is determined by register writes (e.g.
> wired up for elc trigger, but you can opt for software trigger in the
> driver) then it should be a userspace control.

It is complete, and I confirm, this can be changed by register writes.
Apart from defining default to 0, should I implement userspace change
support now ?

Or should I keep it as it is, just setting default to 0 (thus making
the property optional), and add support for userspace change when I add
ELC support.

My other question is, in case I must add userspace change support now, would
sysfs be Ok ? If yes, is there any path recommendations ?


Regards,
John

Re: [PATCH v2 3/7] dt-bindings: thermal: r9a09g047-tsu: Document the TSU unit

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 2086 bytes --]

On Tue, Mar 11, 2025 at 11:24:59AM +0000, John Madieu wrote:
> Hi Conor,
> 
> > -----Original Message-----
> > From: Conor Dooley <conor@kernel.org>
> > Sent: Monday, March 10, 2025 5:15 PM
> > To: John Madieu <john.madieu.xa@bp.renesas.com>
> > Subject: Re: [PATCH v2 3/7] dt-bindings: thermal: r9a09g047-tsu: Document
> > the TSU unit
> > 
> > On Sun, Mar 09, 2025 at 10:39:27AM +0000, John Madieu wrote:
> > > Hi Conor,
> > > > > Changes are not possible at runtime. Some customers may want
> > > > > software, while other may want the external trigger, and this is
> > > > > immutable configuration.
> > > >
> > > > What makes it immutable? Set by some wiring on the board? I couldn't
> > > > find the user in your driver patches to better understand how you
> > > > were using it.
> > >
> > > I haven't prototyped ELC trigger yet. Since the hardware manual
> > > describes about ELC trigger, I have documented it in bindings. If you
> > > think, it is not needed at this stage, then I can drop it now and
> > > revisit later.
> > 
> > Ideally a binding is complete, even if the driver isn't. To me "immutable"
> > would mean something like "the trigger type is determined by hardware or
> > firmware configuration", but if it is determined by register writes (e.g.
> > wired up for elc trigger, but you can opt for software trigger in the
> > driver) then it should be a userspace control.
> 
> It is complete, and I confirm, this can be changed by register writes.
> Apart from defining default to 0, should I implement userspace change
> support now ?
> 
> Or should I keep it as it is, just setting default to 0 (thus making
> the property optional), and add support for userspace change when I add
> ELC support.
> 
> My other question is, in case I must add userspace change support now, would
> sysfs be Ok ? If yes, is there any path recommendations ?

How to change it from userspace ain't my domain, sorry. Just drop the
property since isn't something determined by the hardware, but rather by
what you put into the registers.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]