Re: [PATCH v5 1/2] dt-bindings: hwmon: Add nct7802 bindings

Oskar Senft
2021-10-10T03:06:53Z

Hi Guenter

Thanks again for your review!

> > Signed-off-by: Oskar Senft <osk@google.com>
> > ---
>
> change log goes here.
This might be a silly question: I'm using "git send-email", but I
don't think there's a way to edit the e-mail before it goes out. Do I
just add "---\n[Change log]" manually in the commit description?

> > +description: |
> > + The NCT7802Y is a hardware monitor IC which supports one on-die and up to
> > + 5 remote temperature sensors with SMBus interface.
> > +
>
> Just noticed: 5 remote temperature sensors ? Shouldn't that be 3 ?
This includes 2 temperature sensors that are queried via PECI (i.e.
SMBus). I generated the description from the "general description"
section in the datasheet. I think the driver doesn't implement the 2
PECI sensors at this time, but the statement about the HW is still
true.

> > + sensor-type:
> > + items:
> > + - enum:
> > + - temperature
> > + - voltage
> > + temperature-mode:
> > + items:
> > + - enum:
> > + - thermistor
> > + - thermal-diode
> > + required:
> > + - reg
> > + - sensor-type
>
> If I understand correctly, "temperature-mode" is implemented as mandatory
> for channels 1 and 2 if sensor-type is "temperature" (which makes sense).
> No idea though if it is possible to express that in yaml.
> If not, can it be mentioned as comment ?

After doing a bit more searching, I found the amazing "if: then:
else:" construct that allows to express this properly and eliminates
the code duplication. I'll follow up in PATCH v6.

Thanks
Oskar.

>
> > +
> > + channel@2:
> > + type: object
> > + description: Remote Temperature Sensor or Voltage Sensor ("RTD2")
> > + properties:
> > + reg:
> > + const: 2
> > + sensor-type:
> > + items:
> > + - enum:
> > + - temperature
> > + - voltage
> > + temperature-mode:
> > + items:
> > + - enum:
> > + - thermistor
> > + - thermal-diode
> > + required:
> > + - reg
> > + - sensor-type
> > +
> > + channel@3:
> > + type: object
> > + description: Remote Temperature Sensor or Voltage Sensor ("RTD3")
> > + properties:
> > + reg:
> > + const: 3
> > + sensor-type:
> > + items:
> > + - enum:
> > + - temperature
> > + - voltage
> > + required:
> > + - reg
> > + - sensor-type
> > +
> > +required:
> > + - compatible
> > + - reg
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > + - |
> > + i2c {
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > +
> > + nct7802@28 {
> > + compatible = "nuvoton,nct7802";
> > + reg = <0x28>;
> > +
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > +
> > + channel@0 { /* LTD */
> > + reg = <0>;
> > + status = "okay";
> > + };
> > +
> > + channel@1 { /* RTD1 */
> > + reg = <1>;
> > + status = "okay";
> > + sensor-type = "temperature";
> > + temperature-mode = "thermistor";
> > + };
> > +
> > + channel@2 { /* RTD2 */
> > + reg = <2>;
> > + status = "okay";
> > + sensor-type = "temperature";
> > + temperature-mode = "thermal-diode";
> > + };
> > +
> > + channel@3 { /* RTD3 */
> > + reg = <3>;
> > + status = "okay";
> > + sensor-type = "voltage";
> > + };
> > + };
> > + };
> >
>