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

From: Guenter Roeck
Date: Thu Sep 16 2021 - 15:35:03 EST


On 9/16/21 12:19 PM, Oskar Senft wrote:
Hi Guenter

Would it be acceptable to simply number the sensors and document which
sensor has which number?

Something like this:
0 = LTD
1 = RTD1
...

That might be a possibility, though it would have to be well defined
for each chip (nct7802 also has voltage sensors). We'll have to discuss
this with Rob.

Personally I think I would prefer using a type qualifier - that seems
cleaner. But that is really a matter of opinion.

Another existing way I found is in ltc2978. Following that, we could
do it as follows:

i2c {
#address-cells = <1>;
#size-cells = <0>;

nct7802@28 {
compatible = "nuvoton,nct7802";
reg = <0x28>;
#address-cells = <1>;
#size-cells = <0>;

sensors {
ltd {
status = "okay";
label = "my local temperature";
};

rtd1 {
status = "okay";
mode = <0x2>; /* 3904 transistor */
label = "other temperature";
};

rtd3 {
status = "okay";
mode = <0x3>; /* thermal diode */
label = "3rd temperature";
};
};
};
};


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).

Thanks,
Guenter


The NCT7802Y can self-program from an EEPROM, so I assume we should
honor the "power-up configuration" obtained from there? I.e. if no
configuration is provided in the device tree, the driver should use
whatever configuration the chip has when the driver is loaded.

Definitely yes. My question was more what to do if the information
in devicetree nodes is incomplete.
I think there are two cases:
1) If the new "sensor" tree is missing, the driver should behave as it
does today to not break existing users.
2) If the new "sensor" tree is present, then each of the sensors that
should be disabled needs to have "status = 'okay'" and have the mode
set (unless it's LTD). In the above example, rtd2 is missing and would
therefore be considered disabled.

Does that make sense? I still need to find out whether this is
actually valid DT and how to express that in the YAML, though ...

Thanks
Oskar.