linux-kernel - Re: [PATCH v2 2/2] dt-bindings: hwmon: Add sample averaging property for ADM1275

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]

Date:   Fri, 25 Feb 2022 08:06:11 +0100
From:   Krzysztof Kozlowski <krzk@...nel.org>
To:     Potin Lai <potin.lai@...ntatw.com>,
        Guenter Roeck <linux@...ck-us.net>,
        Jean Delvare <jdelvare@...e.com>,
        Rob Herring <robh+dt@...nel.org>
Cc:     Patrick Williams <patrick@...cx.xyz>, linux-hwmon@...r.kernel.org,
        linux-kernel@...r.kernel.org, devicetree@...r.kernel.org
Subject: Re: [PATCH v2 2/2] dt-bindings: hwmon: Add sample averaging property
 for ADM1275

On 24/02/2022 16:43, Potin Lai wrote:
> Add new properties for binding sample averaging in PMON_CONFIG register
> 
> - adi,volt-curr-sample-average
> - adi,power-sample-average
> 
> Signed-off-by: Potin Lai <potin.lai@...ntatw.com>
> ---
>  .../bindings/hwmon/adi,adm1275.yaml           | 44 +++++++++++++++++++
>  1 file changed, 44 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/hwmon/adi,adm1275.yaml b/Documentation/devicetree/bindings/hwmon/adi,adm1275.yaml
> index 223393d7cafd..325f6827648f 100644
> --- a/Documentation/devicetree/bindings/hwmon/adi,adm1275.yaml
> +++ b/Documentation/devicetree/bindings/hwmon/adi,adm1275.yaml
> @@ -37,6 +37,48 @@ properties:
>      description:
>        Shunt resistor value in micro-Ohm.
>  
> +  adi,volt-curr-sample-average:
> +    description: |
> +      A value to configure VI_AVG in PMON_CONFIG register to indicate a
> +      number of samples to be used to report voltage and currentvalues.

missing space after current.

> +      If set to 7, the 128 samples averaging would be used.
> +
> +    $ref: /schemas/types.yaml#/definitions/uint8

Make it a uint32.

The previous usage of this field was more appropriate. Instead of
keeping register values in DT, it's better to keep logical value. What
if in next cheap the register values have calculation method?

This should be like in v1 - enum for number of samples to take in averaging.

> +    enum:
> +      - 0 # 1 sample averaging
> +      - 1 # 2 sample averaging
> +      - 2 # 4 sample averaging
> +      - 3 # 8 sample averaging
> +      - 4 # 16 sample averaging
> +      - 5 # 32 sample averaging
> +      - 6 # 64 sample averaging
> +      - 7 # 128 sample averaging
> +    default: 0
> +
> +  adi,power-sample-average:
> +    description: |
> +      A value to configure PWR_AVG in PMON_CONFIG register to indicate a
> +      number of samples to be used to report power values.
> +      If set to 7, the 128 samples averaging would be used.

The same.

> +
> +      The chip supports power sample averaging:
> +        "adi,adm1272"
> +        "adi,adm1278"
> +        "adi,adm1293"
> +        "adi,adm1294"

This should be in if-block like here:
https://lore.kernel.org/linux-devicetree/YheqjZQHq0T%2FRSIz@robh.at.kernel.org/T/#m0672807a08c95aba2bccb927d37ff24fde471b8b


Best regards,
Krzysztof