On maj 20, 2022 12:13, Krzysztof Kozlowski wrote: > On 20/05/2022 11:32, Slawomir Stepien wrote: > > From: Slawomir Stepien <slawomir.stepien@xxxxxxxxx> > > > > Add binding description for temperature channels. Currently, support for > > label and offset is implemented. > > > > Signed-off-by: Slawomir Stepien <slawomir.stepien@xxxxxxxxx> > > --- > > .../bindings/hwmon/national,lm90.yaml | 39 +++++++++++++++++++ > > 1 file changed, 39 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/hwmon/national,lm90.yaml b/Documentation/devicetree/bindings/hwmon/national,lm90.yaml > > index 066c02541fcf..9a5aa78d4db1 100644 > > --- a/Documentation/devicetree/bindings/hwmon/national,lm90.yaml > > +++ b/Documentation/devicetree/bindings/hwmon/national,lm90.yaml > > @@ -62,6 +62,37 @@ required: > > > > additionalProperties: false > > > > +patternProperties: > > Which models use this? This is used in tmp421 model. > > + "^channel@([0-2])$": > > + type: object > > + description: | > > No need for | Will fix in v2. > > + Represents channels of the device and their specific configuration. > > + > > + properties: > > + reg: > > + description: | > > The same. Will fix in v2. > > + The channel number. 0 is local channel, 1-2 are remote channels. > > + items: > > + minimum: 0 > > + maximum: 2 > > + > > + label: > > + description: | > > The same. Will fix in v2. > > + 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. Currently in lm90 we have support for devices that have different width (including sign) for offset register. We have 10 bits, 11 bits and 12 bits. Do I understand correctly that I can use the same vendor prefix if the width is the same? Just like "ti" was used for adi and ti in "ti,extended-range-enable"? For example: adi,10-bit-offset-millicelsius # (only for adt7481) adi,11-bit-offset-millicelsius # (for adt7461 but also for lm90 and others) ti,12-bit-offset-millicelsius # (ti - since only tmp451 and tmp461 supports 12 bit) > > + 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. I am also not sure about the "-millicelsius" in example above. From device point-of-view, this offset is just two's complement, so is it more desirable to have the values here as just bytes rather than millicelsius? > > + For remote channels only. > > + $ref: /schemas/types.yaml#/definitions/int32 > > + default: 0 > > + > > + required: > > + - reg > > + > > + additionalProperties: false > > + > > examples: > > - | > > #include <dt-bindings/interrupt-controller/irq.h> > > @@ -76,5 +107,13 @@ examples: > > vcc-supply = <&palmas_ldo6_reg>; > > interrupts = <4 IRQ_TYPE_LEVEL_LOW>; > > #thermal-sensor-cells = <1>; > > + #address-cells = <1>; > > + #size-cells = <0>; > I assume you tested the bindings with dt_bindings_check? > > I have some doubts, as this should fail. I did. All was fine. What should fail here? > > + > > + channel@0 { > > + reg = <0x0>; > > + label = "internal"; > > + offset = <1000>; > > + }; > > }; > > }; -- Slawomir Stepien