On Sun, Mar 09, 2025 at 10:39:27AM +0000, John Madieu wrote:

Hi Conor,

quoted

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

quoted

Hi Conor,

quoted

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.

quoted

quoted

quoted

quoted

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 ?

quoted

quoted

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.

quoted

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

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.

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.