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

[Conor Dooley <conor@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 3311 bytes --]

On Sun, Mar 09, 2025 at 10:39:27AM +0000, John Madieu wrote:
> 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.

Ideally a binding is complete, even if the driver isn't. To me
"immutable" would mean something like "the trigger type is determined by
hardware or firmware configuration", but if it is determined by register
writes (e.g. wired up for elc trigger, but you can opt for software
trigger in the driver) then it should be a userspace control.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]