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

Guenter Roeck
2021-10-10T04:10:22Z

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