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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Sat, 9 Oct 2021 21:10:16 -0700
From:   Guenter Roeck <linux@...ck-us.net>
To:     Oskar Senft <osk@...gle.com>
Cc:     Jean Delvare <jdelvare@...e.com>, Rob Herring <robh+dt@...nel.org>,
        linux-hwmon@...r.kernel.org, linux-kernel@...r.kernel.org,
        devicetree@...r.kernel.org
Subject: Re: [PATCH v5 1/2] dt-bindings: hwmon: Add nct7802 bindings

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