On 13/09/2023 17:48, Matyas, Daniel wrote: > > >> -----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? Code is okay, after Guenter's explanation. However please fix the subject and improve the commit msg. Best regards, Krzysztof
