Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Sender: Guenter Roeck <groeck7@gmail.com>
To:     Oskar Senft <osk@google.com>
Cc:     linux-hwmon@vger.kernel.org, devicetree@vger.kernel.org,
        linux-kernel@vger.kernel.org, Jean Delvare <jdelvare@suse.com>,
        Rob Herring <robh+dt@kernel.org>
References: <20210910130337.2025426-1-osk@google.com>
 <71c17c47-ca9e-e9d2-7b89-cc25b512c06a@roeck-us.net>
 <CABoTLcRZ43EUVzbqWniu64PkB7Yx4RMYKjaBxaSihk+k0Ca-gA@mail.gmail.com>
 <722b27f6-4390-9b5b-f6f2-75ce9e967d12@roeck-us.net>
 <CABoTLcSdkmuBxd5Yh6z2Oqm1-_Vd4J5Ni1i1qq5s07mWu7Ndew@mail.gmail.com>
 <20210914150859.GB3457579@roeck-us.net>
 <CABoTLcQfS5-UL92NR9vbc2YrGJv3oQPYCqAm-diNoq-tkHP_hQ@mail.gmail.com>
 <c410eba9-f6cf-4dbf-797f-48afde9c1898@roeck-us.net>
 <CABoTLcQWXerMWPvWUqjykiNcx9oGoP8aEcuDwcQ36yu-CBc0pA@mail.gmail.com>
 <382858f5-e833-d4b9-f189-449671992ba5@roeck-us.net>
 <CABoTLcST=74wRbtMA2SdmeHd0WmU7id05ouSfw4PFw2nJt_gLw@mail.gmail.com>
From:   Guenter Roeck <linux@roeck-us.net>
Subject: Re: [PATCH] dt-bindings: hwmon: Add nct7802 bindings
Message-ID: <9869ed19-b8ab-d9e5-e791-a02eeb2c5eed@roeck-us.net>
Date:   Thu, 16 Sep 2021 13:07:39 -0700
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101
 Thunderbird/78.13.0
MIME-Version: 1.0
In-Reply-To: <CABoTLcST=74wRbtMA2SdmeHd0WmU7id05ouSfw4PFw2nJt_gLw@mail.gmail.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 8bit
Precedence: bulk

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