linux-kernel - Re: [PATCH 2/4] dt-bindings: hwmon: Added new properties to the devicetree

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <177ef05b-0cca-be25-afad-ac518d9f6473@linaro.org>
Date:   Wed, 13 Sep 2023 17:40:35 +0200
From:   Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To:     Daniel Matyas <daniel.matyas@...log.com>
Cc:     Jean Delvare <jdelvare@...e.com>,
        Guenter Roeck <linux@...ck-us.net>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Conor Dooley <conor+dt@...nel.org>,
        Jonathan Corbet <corbet@....net>, linux-hwmon@...r.kernel.org,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-doc@...r.kernel.org
Subject: Re: [PATCH 2/4] dt-bindings: hwmon: Added new properties to the
 devicetree

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@...log.com>
> ---
>  .../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