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

[John Madieu <john.madieu.xa@bp.renesas.com>]

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