Re: [PATCH 8/8] dt-bindings: hwmon: allow specifying channels for tmp421

[Krzysztof Adamski <krzysztof.adamski@nokia.com>]

Dnia Tue, Sep 21, 2021 at 05:58:31AM -0700, Guenter Roeck napisał(a):
>>
>> ti,n-factor
>
>n-factor isn't just supported by TI sensors, though it isn't always called
>n-factor. Maxim (eg MAX6581) uses the term "ideality factor", though they
>also refer to the factor as "N" in the datasheet.
>
>So question is if we make this ti,n-factor and maxim,n-factor, or if we make
>it generic and define some kind of generic units. Thoughts ? My personal
>preference would be a generic definition, but is not a strong preference.

That was exactly my way of thinking here - many sensors have n-factor
parameter and this is the name I see most often.

That being said, maybe it should be "nfactor" instead of "n-factor", as
this is what tmp513 is using?

>In regard to units, the n-factor is, as the name says, a factor. Default
>value is 1.008. The value range for MAX6581 is 0.999 to 1.030. For TMP421
>it is 0.706542 to 1.747977. So the scondary question is if the value
>written should be the register value (as proposed here) or the absolute
>factor (eg in micro-units).

Since expressing the fractional values in DT isn't well supported and
(at least here) hardware guys like to think in terms of register values
so this is what I proposed. Also, I just noticed that, for example,
TMP531 is using register values as well.

>> > +    i2c {
>> > +      #address-cells = <1>;
>> > +      #size-cells = <0>;
>> > +
>> > +      sensor@4c {
>> > +        compatible = "ti,tmp422";
>> > +        reg = <0x4c>;
>> > +        #address-cells = <1>;
>> > +        #size-cells = <0>;
>> > +
>> > +        input@0 {
>> > +          reg = <0x0>;
>> > +          n-factor = <0x1>;
>> > +          label = "local";
>> > +        };
>
>In the context or other sensors, question here is if we can make the
>bindings generic. We have been discussing this for NCT7802Y. The main
>question for me is how to handle different sensor types. TMP421 is
>easy because it only has one type of sensors, but there are other
>devices which also have, for example, voltage and/or current sensors.
>NCT7802 is an example for that. We just had a set of bindings for that
>chip proposed at
>https://patchwork.kernel.org/project/linux-hwmon/patch/20210921004627.2786132-1-osk@google.com/
>
>Would it be possible to determine a generic scheme that works for all
>chips ? 

Just wanted to note that the bindings I propse were not completely made
up by me. I based them on existing ina3221 bindings so I feel like my
proposition is at least in line with that one.

> I can see two problems:
>- How to express sensor types. The NCT7802 submission proposes another level
>  of indirection, ie
>
>  temperature-sensors {
>> > +
>> > +        input@1 {
>> > +          reg = <0x1>;
>> > +          n-factor = <0x0>;
>> > +          label = "somelabel";
>> > +        };
>> > +
>> > +        input@2 {
>> > +          reg = <0x2>;
>> > +          status = "disabled";
>> > +        };
>> > +      };
>> > +    };
>    };
>
>The second question is how to express sensor index. One option is the solution
>suggested here, ie to use reg=<> as sensor index. The second is the solution
>suggested in the 7802 bindings, where the (chip specific) name is used as
>sensor index.
>
>+            temperature-sensors {
>+                ltd {
>+                  status = "disabled";
>+                };
>+
>+                rtd1 {
>+                  status = "okay";
>+                  type = <4> /* thermistor */;
>+                };
>+            };
>
>I personally don't have a strong opinion either way, but I would like to see
>a single solution for all sensor chips.

For me, the problem with using this style is that it is sometimes hard
to come up with good names, especially in simple devices where all you
have are channels.. which BTW may be called differently as well. So we
would end up having some device with channel0, channel1, etc, and others
might have input0, input1, etc.
This argument goes the other way around as well - some devies have no
way to easily enumerate their "subdevices" it would be hard to assing
proper numbers to them.

For this reason I think it would make sense to actually use both schemes
- reg for cases where the enumeration makes sesne (when we have
   channels, inputs, etc)
- names otherwise, when there is no natual way to enumerate the devices
   by an index.