On Fri, Sep 24, 2021 at 10:18 AM Oskar Senft [off-list ref] wrote:

Hi Rob

quoted

quoted

+maintainers:
+  - Guenter Roeck [off-list ref]

Should be someone that cares about this h/w, not who applies patches.

Hmm, ok. After talking with Guenter, I thought that would be him. But
I can add myself, too, since we're obviously using that HW. Is that
what you mean?

Okay, seems it is Guenter in this case.

quoted

quoted

+    properties:
+      ltd:
+        type: object
+        description: Internal Temperature Sensor ("LTD")

No child properties?

Yes. We really just want the ability to enable / disable that sensor.
What's the correct way in the YAML to describe that? Same for RTD3.

Okay, you need a 'additionalProperties: false' in the schema. (status
will automagically be allowed)

quoted

quoted

+          "type":
+            description: Sensor type (3=thermal diode, 4=thermistor).

2nd time I've seen this property this week[1]. Needs to be more specific
than just 'type'.

Ha yes, the example in [1] came from this patch. I went with this name
to stay in-line with the sysfs name, being "tempX_type". In the
hardware this would be called "mode".

My original proposal [2] was to have this property a string list named
"nuvoton,rtd-modes" with a set of accepted values, i.e. basically an
enum. Splitting this string list into individual sensors makes sense.

The other question that remains open (at least in my view), is whether
naming the sensors "ltd, rtd1, rtd2, rtd3" is the right approach or if
we should really go to naming them "sensor@X" with a reg property set
to X. Note that ltd and rtd3 do not accept any additional
configuration beyond "is enabled" (i.e. "status").

If X is not made up numbering (i.e. corresponds to something in the
datasheet), then using addresses and generic node names are preferred.
Alignment with other similar h/w is also preferred.

quoted

quoted

+            temperature-sensors {
+                ltd {
+                  status = "disabled";

Don't show status in examples.

Hmm, ok. I found it useful to make clear that a sensor can be
disabled, but maybe that's just always the case?

Yeah, this case is a bit special. The node not being present also disables it.

The problem is generally we don't want disabled examples as that turns
off some validation. I have a check for this that I plan to add, but I
don't have a way to have exceptions.

What you could do is just comment out the node. Then you show it, but
don't compile it.

Rob