Re: [PATCH] dt-bindings: hwmon: Add nct7802 bindings

[Guenter Roeck]

On 9/16/21 12:53 PM, Oskar Senft wrote:
> > Ah, using the node name as indication for both sensor type and
> > index ? SGTM, though we'd really need input from Rob.
> > I guess one could also consider something more generic like
> > "temperature-sensor@0", "voltage-sensor@0", and so on (instead
> > of [mis-]using reg and a sensor-type field).
>
> Hmm, in that case, maybe we should just remove the "sensors" section.
>
> i2c {
>      #address-cells = <1>;
>      #size-cells = <0>;
>
>      nct7802@28 {
>          compatible = "nuvoton,nct7802";
>          reg = <0x28>;
>          #address-cells = <1>;
>          #size-cells = <0>;
>
>          temperature-sensor@0 { /* LTD */
>              status = "okay";
>              label = "my local temperature";
>          };
>
>          temperature-sensor@1 { /* RTD1 */
>              status = "okay";
>              mode = <0x2>; /* 3904 transistor */
>              label = "other temperature";
>          };
>
>          temperature-sensor@3 { */ RTD3 */
>              status = "okay";
>              mode = <0x3>; /* thermal diode */
>              label = "3rd temperature";
>         };
>     };
> };

I think there was a reason for "sensors", because there can be other
bindings on the same level. Documentation/devicetree/bindings/hwmon/ltc2978.txt
lists "regulators", for example.

Where did you find the "sensors" example for ltc2978 ? I don't see it
in the upstream kernel. Or was that derived from the official "regulators"
bindings ?

> Now, with "sensors" removed and everything at "top-level", we'll need
> to decide what to do if individual sensors are missing. I guess in
> that case I would just leave the affected sensors alone, i.e. neither
> configure nor disable them and instead read their status from HW. That
> way prior uses of the nct7802 in device trees will continue to behave
> as before as does the EEPROM-style configuration.
>
> I would like to focus on the implementation of the temperature-sensor
> entries for now (i.e. LTD, RTD1, RTD2, RTD3). Support for other sensor
> types could be added in a separate change. Would that be acceptable?

Yes, let's do that. I'd like us to keep the "sensors" subnode to have a clear
association and differentiator to other sub-nodes such as "regulators".
Open is if we can use "temperature-sensor@0" or if it would have to be
a chip specific "ltd", but I think we can sort that out after suggesting
an initial set of bindings to Rob.

Thanks,
Guenter