On Fri, Mar 07, 2025 at 03:55:27PM +0000, John Madieu wrote:

Hi Conor,

quoted

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

quoted

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.

quoted

quoted

quoted

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.

quoted

quoted

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.

quoted

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.