linux-kernel - Re: [PATCH] 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 linux-cve-announce PHC
Open Source and information security mailing list archives

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

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

Message-ID: <9869ed19-b8ab-d9e5-e791-a02eeb2c5eed@roeck-us.net>
Date:   Thu, 16 Sep 2021 13:07:39 -0700
From:   Guenter Roeck <linux@...ck-us.net>
To:     Oskar Senft <osk@...gle.com>
Cc:     linux-hwmon@...r.kernel.org, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org, Jean Delvare <jdelvare@...e.com>,
        Rob Herring <robh+dt@...nel.org>
Subject: Re: [PATCH] dt-bindings: hwmon: Add nct7802 bindings

On 9/16/21 12:53 PM, Oskar Senft wrote:
>> Ah, using the node name as indication for both sensor type and
>> index ? SGTM, though we'd really need input from Rob.
>> I guess one could also consider something more generic like
>> "temperature-sensor@0", "voltage-sensor@0", and so on (instead
>> of [mis-]using reg and a sensor-type field).
> 
> Hmm, in that case, maybe we should just remove the "sensors" section.
> 
> i2c {
>      #address-cells = <1>;
>      #size-cells = <0>;
> 
>      nct7802@28 {
>          compatible = "nuvoton,nct7802";
>          reg = <0x28>;
>          #address-cells = <1>;
>          #size-cells = <0>;
> 
>          temperature-sensor@0 { /* LTD */
>              status = "okay";
>              label = "my local temperature";
>          };
> 
>          temperature-sensor@1 { /* RTD1 */
>              status = "okay";
>              mode = <0x2>; /* 3904 transistor */
>              label = "other temperature";
>          };
> 
>          temperature-sensor@3 { */ RTD3 */
>              status = "okay";
>              mode = <0x3>; /* thermal diode */
>              label = "3rd temperature";
>         };
>     };
> };
> 

I think there was a reason for "sensors", because there can be other
bindings on the same level. Documentation/devicetree/bindings/hwmon/ltc2978.txt
lists "regulators", for example.

Where did you find the "sensors" example for ltc2978 ? I don't see it
in the upstream kernel. Or was that derived from the official "regulators"
bindings ?

> Now, with "sensors" removed and everything at "top-level", we'll need
> to decide what to do if individual sensors are missing. I guess in
> that case I would just leave the affected sensors alone, i.e. neither
> configure nor disable them and instead read their status from HW. That
> way prior uses of the nct7802 in device trees will continue to behave
> as before as does the EEPROM-style configuration.
> 
> I would like to focus on the implementation of the temperature-sensor
> entries for now (i.e. LTD, RTD1, RTD2, RTD3). Support for other sensor
> types could be added in a separate change. Would that be acceptable?
> 

Yes, let's do that. I'd like us to keep the "sensors" subnode to have a clear
association and differentiator to other sub-nodes such as "regulators".
Open is if we can use "temperature-sensor@0" or if it would have to be
a chip specific "ltd", but I think we can sort that out after suggesting
an initial set of bindings to Rob.

Thanks,
Guenter