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

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

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
>

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

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

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

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

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.

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