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

[Guenter Roeck]

On 10/9/21 8:06 PM, Oskar Senft wrote:
> Hi Guenter
>
> Thanks again for your review!
>
> > > Signed-off-by: Oskar Senft <osk@xxxxxxxxxx>
> > > ---
> >
> > 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?

When you use git send-email, you usually have a patch file to send.
I use git format-patch to create that patch file, add the change
log using an editor, and then send it with git send-email.

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

Ok, make sense.

Thanks,
Guenter

> > > +      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";
> > > +            };
> > > +        };
> > > +    };