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

[Guenter Roeck <linux@roeck-us.net>]

On Wed, Sep 22, 2021 at 08:32:00PM +0200, Krzysztof Adamski wrote:
> Dnia Wed, Sep 22, 2021 at 05:39:26AM -0700, Guenter Roeck napisał(a):
> > On Wed, Sep 22, 2021 at 09:22:33AM +0200, Krzysztof Adamski wrote:
> > > 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.
> > > 
> > 
> > I never see "someone else does that" as valid argument.
> 
> It is not an argument for "so I should be allowed too" but more like "so
> it is generic enough to make sense for more than a single case" :)
> 
> > Also, DT does support fractional values, via units. It is perfectly
> > valid to describe a voltage as micro-volt, for example.
> 
> True. But doing so for unit-less values isn't as obvious. For real
> fractions we don't even know what the resolution should be and then we
> also may have those rounding errors etc (while with register values we
> know precisely what we get). As usual, we have some pros and
> cons of both approaches. While I agree raw values are not perfect, I
> still think it makes more sense so I vote for them. But my vote,
> obviously, isn't that important here so I'll let you guys decide.
> 

I really have to pass on this one, and leave it up to Rob to decide.
Personally I really really really dislike raw values, but I understand
that this makes me biased. I do realize that converting from a fractional
value to a register value is inherently complex and open to interpretation.
For example. if we define fractional values, what should 1.007000 translate
to ? It would either be 1.008 or 1.004641. Using the register value (0xff,
or -1 for 1.004641) would definitely be simpler and avoid calculations and
rounding.

Guenter

> > If the agreement is to use raw register values, I think the property name
> > should be prefixed with the vendor name, since it won't be a standard
> > property. I'll defer on Rob for that, though.
> 
> Fair enough. If we go that route, we should use "ti,nfactor" (without
> dash) to be consistent with ti513?
> 
> Krzysztof