linux-kernel - Re: [PATCH v3 2/2] dt-bindings: hwmon: Add sample averaging properties 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:   Mon, 28 Feb 2022 17:20:24 +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 v3 2/2] dt-bindings: hwmon: Add sample averaging
 properties for ADM1275

On 28/02/2022 17:13, POTIN LAI wrote:
> 
> Krzysztof Kozlowski 於 28/02/2022 10:25 pm 寫道:
>> On 28/02/2022 11:37, Potin Lai wrote:
>>> Add documentation of new properties for sample averaging in PMON_CONFIG
>>> register.
>>>
>>> New properties:
>>> - adi,volt-curr-sample-average
>>> - adi,power-sample-average
>>>
>>> Signed-off-by: Potin Lai <potin.lai@...ntatw.com>
>>>
>>> doc
>> You have weirdly formatted commit msg.
> It must be pasted from somewhere accidentally, sorry.
>>> ---
>>>  .../bindings/hwmon/adi,adm1275.yaml           | 39 +++++++++++++++++++
>>>  1 file changed, 39 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/hwmon/adi,adm1275.yaml b/Documentation/devicetree/bindings/hwmon/adi,adm1275.yaml
>>> index 223393d7cafd..bc4206b257a8 100644
>>> --- a/Documentation/devicetree/bindings/hwmon/adi,adm1275.yaml
>>> +++ b/Documentation/devicetree/bindings/hwmon/adi,adm1275.yaml
>>> @@ -37,6 +37,43 @@ properties:
>>>      description:
>>>        Shunt resistor value in micro-Ohm.
>>>  
>>> +  adi,volt-curr-sample-average:
>>> +    description: |
>>> +      Number of samples to be used to report voltage and current values.
>>> +      If the configured value is not a power of 2, sample averaging number
>>> +      will be configured with smaller and closest power of 2.
>>> +
>>> +    $ref: /schemas/types.yaml#/definitions/uint32
>>> +    minimum: 1
>>> +    maximum: 128
>>> +    default: 1
>>> +
>>> +  adi,power-sample-average:
>>> +    description: |
>>> +      Number of samples to be used to report power values.
>>> +      If the configured value is not a power of 2, sample averaging number
>>> +      will be configured with smaller and closest power of 2.
>>> +
>>> +    $ref: /schemas/types.yaml#/definitions/uint32
>>> +    minimum: 1
>>> +    maximum: 128
>>> +    default: 1
>>> +
>>> +if:
>> This should be in allOf.
> will add it.
>>
>>> +  not:
>> Remove negation and list devices where it is not allowed.
> will remove it.
>>
>>> +    properties:
>>> +      compatible:
>>> +        contains:
>>> +          enum:
>>> +          - adi,adm1272
>>> +          - adi,adm1278
>>> +          - adi,adm1293
>>> +          - adi,adm1294
>>> +then:
>>> +  properties:
>>> +    adi,power-sample-average:
>>> +      description: This property is not allowed.
>> This does not work. Please test it - add not allowed property to such
>> devices and look for error. I gave you the example how it should be
>> done. Why doing it in a different way which does not work?
>>
> Sorry for misunderstanding from original example. I rechecked the example and made a modification as below, before sending out new patch, would you mind help me review it and let me know if anything improper? Thank you.

Don't take the example by copying and pasting. It has to be adjusted, I
just explained there in example how to disallow a property.

> 
> 
> dependencies:
>   adi,enable-power-sample-average: [ 'adi,power-sample-average' ]
>   adi,power-sample-average: [ 'adi,enable-power-sample-average' ]
> 
> allOf:
>   - if:
>       properties:
>         compatible:
>           contains:
>             enum:
>               - adi,adm1272
>               - adi,adm1278
>               - adi,adm1293
>               - adi,adm1294
>     then:
>       required:
>         - adi,enable-power-sample-average

This does not look correct, because it is not a required property.

You should have "if:then: adi,enable-power-sample-average: false" etc"

>     else:
>       properties:
>         adi,enable-power-sample-average: false
> 
> 


Best regards,
Krzysztof