On Fri, Mar 07, 2025 at 03:14:05PM +0000, John Madieu wrote:

Hi Conor,

Thanks for your review!

quoted

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

quoted

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