Re: [PATCH v3 2/2] dt-bindings: hwmon: Add sample averaging properties for ADM1275

[POTIN LAI]

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@xxxxxxxxxxxx>
>>
>> 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.


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
    else:
      properties:
        adi,enable-power-sample-average: false


Thanks,
Potin
>
>> +
>>  required:
>>    - compatible
>>    - reg
>> @@ -53,5 +90,7 @@ examples:
>>              compatible = "adi,adm1272";
>>              reg = <0x10>;
>>              shunt-resistor-micro-ohms = <500>;
>> +            adi,volt-curr-sample-average = <128>;
>> +            adi,power-sample-average = <128>;
>>          };
>>      };
>
> Best regards,
> Krzysztof