> -----Original Message----- > From: Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx> > Sent: Wednesday, September 13, 2023 6:41 PM > To: Matyas, Daniel <Daniel.Matyas@xxxxxxxxxx> > Cc: Jean Delvare <jdelvare@xxxxxxxx>; Guenter Roeck <linux@roeck- > us.net>; Rob Herring <robh+dt@xxxxxxxxxx>; Krzysztof Kozlowski > <krzysztof.kozlowski+dt@xxxxxxxxxx>; Conor Dooley > <conor+dt@xxxxxxxxxx>; Jonathan Corbet <corbet@xxxxxxx>; linux- > hwmon@xxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx; linux- > kernel@xxxxxxxxxxxxxxx; linux-doc@xxxxxxxxxxxxxxx > Subject: Re: [PATCH 2/4] dt-bindings: hwmon: Added new properties to > the devicetree > > [External] > > On 13/09/2023 17:21, Daniel Matyas wrote: > > Subject: not much improved. I am sorry, but you are not adding new > properties to entire devicetree of entire world. You are actually not > adding anything to any devicetree, because these are bindings (which is > obvious, as said by prefix). > > You got comments on this. > > > These attributes are: > > - adi,comp-int - boolean property > > - adi,alrm-pol - can be 0, 1 (if not present, default value) > > - adi,flt-q - can be 1, 2, 4, 8 (if not present, default value) > > - adi,timeout-enable - boolean property > > Don't repeat what the code does. Explain why you are adding it, what is > the purpose. > > > > > These modify the corresponding bits in the configuration register. > > > > Signed-off-by: Daniel Matyas <daniel.matyas@xxxxxxxxxx> > > --- > > .../bindings/hwmon/adi,max31827.yaml | 35 > +++++++++++++++++++ > > 1 file changed, 35 insertions(+) > > > > diff --git > a/Documentation/devicetree/bindings/hwmon/adi,max31827.yaml > > b/Documentation/devicetree/bindings/hwmon/adi,max31827.yaml > > index 2dc8b07b4d3b..6bde71bdb8dd 100644 > > --- a/Documentation/devicetree/bindings/hwmon/adi,max31827.yaml > > +++ > b/Documentation/devicetree/bindings/hwmon/adi,max31827.yaml > > @@ -32,6 +32,37 @@ properties: > > Must have values in the interval (1.6V; 3.6V) in order for the device > to > > function correctly. > > > > + adi,comp-int: > > + description: > > + If present interrupt mode is used. If not present comparator mode > is used > > + (default). > > Why this is a property of hardware? > > > + type: boolean > > + > > + adi,alrm-pol: > > + description: > > + Sets the alarms active state. > > + - 0 = active low > > + - 1 = active high > > + For max31827 and max31828 the default alarm polarity is low. For > max31829 > > + it is high. > > + $ref: /schemas/types.yaml#/definitions/uint32 > > + enum: [0, 1] > > + > > + adi,flt-q: > > + description: > > + Select how many consecutive temperature faults must occur > before > > + overtemperature or undertemperature faults are indicated in the > > + corresponding status bits. > > + For max31827 default fault queue is 1. For max31828 and max31829 > it is 4. > > + $ref: /schemas/types.yaml#/definitions/uint32 > > + enum: [1, 2, 4, 8] > > + > > + adi,timeout-enable: > > + description: > > + Enables timeout. Bus timeout resets the I2C-compatible interface > when SCL > > + is low for more than 30ms (nominal). > > Why this is a property of hardware? > > > Best regards, > Krzysztof Thank you for your quick reply, Krzysztof! I do not know exactly why does are a property of hardware. I just did what Guenter suggested here: https://marc.info/?l=linux-i2c&m=169201795715687&w=2 and here: https://patchwork.kernel.org/project/linux-hwmon/patch/20230911083735.11795-2-daniel.matyas@xxxxxxxxxx/ Daniel
