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

[Guenter Roeck <linux@roeck-us.net>]

On 10/9/21 8:06 PM, Oskar Senft wrote:
> 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?
> 

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