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

[John Madieu]

Hi Conor,

> -----Original Message-----
> From: Conor Dooley <conor@xxxxxxxxxx>
> Sent: Friday, March 7, 2025 4:33 PM
> To: John Madieu <john.madieu.xa@xxxxxxxxxxxxxx>
> 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@xxxxxxxxxx>
> > > Sent: Friday, February 28, 2025 8:03 PM
> > > To: John Madieu <john.madieu.xa@xxxxxxxxxxxxxx>
> > > 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@xxxxxxxxxxxxxx>
> > > > ---
> > > > 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