Re: [PATCH 3/8] dt-bindings: hwmon: Allow specifying channels for lm90

[Slawomir Stepien <sst@poczta.fm>]

On maj 24, 2022 14:17, Slawomir Stepien wrote:
> On maj 24, 2022 13:53, Slawomir Stepien wrote:
> > On maj 20, 2022 07:22, Guenter Roeck wrote:
> > > On 5/20/22 07:09, Krzysztof Kozlowski wrote:
> > > > On 20/05/2022 15:42, Guenter Roeck wrote:
> > > > > > 
> > > > > > > +          A descriptive name for this channel, like "ambient" or "psu".
> > > > > > > +
> > > > > > > +      offset:
> > > > > > > +        description: |
> > > > > > 
> > > > > > This does not look like standard property, so you need vendor and unit
> > > > > > suffix.
> > > > > > 
> > > > > 
> > > > > Temperature offset is a standard property for temperature sensor
> > > > 
> > > > The original description was strictly connected to registers, so that
> > > > one as not a standard. It seems it was just a wording...
> > > > 
> > > > > chips with external channels, implemented by a diode or transistor.
> > > > > Making it non-standard will mean that we'll have lots of
> > > > > "vendor,offset" properties, one each for each vendor selling
> > > > > temperature sensor chips with external channels. This gets
> > > > > more complicated here because the lm90 driver does support chips
> > > > > from several different vendors. Almost all of them support
> > > > > this functionality. Which vendor do you select in this case ?
> > > > > 
> > > > > I would suggest to use temperature-offset-milliseconds, though.
> > > > 
> > > > Yes, this sounds good. Just not seconds but millicelsius, I guess?
> > > > 
> > > 
> > > Uuh, yes. Sorry, must be too early in the morning here.
> > 
> > Hello
> > 
> > I see that: *-millicelsius is defined as uint32-array:
> >   "-millicelsius$":
> >     $ref: "types.yaml#/definitions/uint32-array"
> >     description: Degreee milli-Celsius
> > 
> > But it would be nice to have negative values as the prop value, for example <(-1000)>.
> > 
> > How should I approach that? Is change to this definition possible? If yes, how should it be
> > conducted? On github or via device-tree mailing list?
> > 
> > Or maybe there is a way to overwrite this (using $defs?) for this particular binding? I haven't
> > found any solution that will pass dt_binding_check.
> 
> Well ok, looks like this:
> 
>       temperature-offset-millicelsius:
>         description: Temperature offset to be added to or subtracted from remote temperature measurements.
>         items:
>           items:
>             type: integer
>             minimum: -128000
>             maximum: 127000

This isn't working...from what I've read we cannot just simply overwrite existing schemas.

Krzysztof, Guenter what I should do? Is there a way to match with uint32-array schema and with
schema that allows items in array to be below 0 (seems impossible to me)? I've tried a lot of
combinations today without any luck. Any helpful tips? Thanks!

> Will overwrite the definition...most likely just minimum: -128000 in 2nd items will be enough.
> 
> > > > > > > +          The value (millidegree Celsius) to be programmed in the channel specific offset register
> > > > > > > +          (if supported by device).
> > > > > > 
> > > > > > You described programming model which should not be put in the bindings.
> > > > > > Please describe the hardware.
> > > > > > 
> > > > > 
> > > > > It is a configuration value, which is hardware dependent because
> > > > > it depends on the temperature diode or transistor connected to the chip.
> > > > 
> > > > Sure, so this could be reworded "Offset against some base value for each
> > > > channel temperature", or something similar (you know better than me).
> > > > Referring to registers and where exactly this should be programmed in
> > > > the device is related to device programming model, not to bindings.
> > > > 
> > > 
> > > Maybe something like "Temperature offset to be added to or
> > > subtracted from remote temperature measurements".

-- 
Slawomir Stepien