This patch series allow user config PWR_AVG and VI_AVG in PMON_CONF
register by adding properties in device tree.
Example:
adm1278@11 {
compatible = "adi,adm1278";
......
adi,volt-curr-sample-average = <128>;
adi,power-sample-average = <128>;
};
LINK: [v1] https://lore.kernel.org/all/[email protected]/
LINK: [v2] https://lore.kernel.org/all/[email protected]/
Changes v2 --> v3:
- change property type back to u32, use logical value instead of register
value
- fix typo in properties description
- add if-block to descript "adi,power-sample-average" not alloed if
compatible not in the enum list
Changes v1 --> v2:
- use more descriptive property name
- change property type from u32 to u8
- add property value check, valid range between 1 and 7
Potin Lai (2):
hwmon: (adm1275) Allow setting sample averaging
dt-bindings: hwmon: Add sample averaging properties for ADM1275
.../bindings/hwmon/adi,adm1275.yaml | 39 +++++++++++++++++++
drivers/hwmon/pmbus/adm1275.c | 36 +++++++++++++++++
2 files changed, 75 insertions(+)
--
2.17.1
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 <[email protected]>
doc
---
.../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:
+ not:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - adi,adm1272
+ - adi,adm1278
+ - adi,adm1293
+ - adi,adm1294
+then:
+ properties:
+ adi,power-sample-average:
+ description: This property is not allowed.
+
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>;
};
};
--
2.17.1
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 <[email protected]>
>
> doc
You have weirdly formatted commit msg.
> ---
> .../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.
> + not:
Remove negation and list devices where it is not allowed.
> + 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?
> +
> 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
On Mon, 28 Feb 2022 18:37:16 +0800, 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 <[email protected]>
>
> doc
> ---
> .../bindings/hwmon/adi,adm1275.yaml | 39 +++++++++++++++++++
> 1 file changed, 39 insertions(+)
>
My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):
yamllint warnings/errors:
./Documentation/devicetree/bindings/hwmon/adi,adm1275.yaml:68:11: [warning] wrong indentation: expected 12 but found 10 (indentation)
dtschema/dtc warnings/errors:
doc reference errors (make refcheckdocs):
See https://patchwork.ozlabs.org/patch/1598637
This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.
If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:
pip3 install dtschema --upgrade
Please check and re-submit.
On 2/28/22 02: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 <[email protected]>
>
> doc
> ---
> .../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.
> +
Both properties should only accept 1, 2, 4, 8, 16, 32, 64, and 128.
We accept non-exact numbers in sysfs attributes because we can not
expect the end users to know permitted values, but devicetree authors
should know what is acceptable. Valid values can be expressed here
easily with something like
enum: [1, 2, 4, 8, 16, 32, 64, 128]
This can also be easily tested in the code by ensuring that
the devicetree property is in the range of 1..128 and has exactly
one bit set, such as
if (!val || val > 128 || hweight32(val) != 1)
or with something like
if (!val || val > 128 || BIT(__fls(val)) != val)
Guenter
> + $ref: /schemas/types.yaml#/definitions/uint32
> + minimum: 1
> + maximum: 128
> + default: 1
> +
> +if:
> + not:
> + properties:
> + compatible:
> + contains:
> + enum:
> + - adi,adm1272
> + - adi,adm1278
> + - adi,adm1293
> + - adi,adm1294
> +then:
> + properties:
> + adi,power-sample-average:
> + description: This property is not allowed.
> +
> 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>;
> };
> };
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 <[email protected]>
>>
>> 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
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 <[email protected]>
>>>
>>> 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
Guenter Roeck 於 28/02/2022 10:54 pm 寫道:
> On 2/28/22 02: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 <[email protected]>
>>
>> doc
>> ---
>> .../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.
>> +
>
> Both properties should only accept 1, 2, 4, 8, 16, 32, 64, and 128.
>
> We accept non-exact numbers in sysfs attributes because we can not
> expect the end users to know permitted values, but devicetree authors
> should know what is acceptable. Valid values can be expressed here
> easily with something like
> enum: [1, 2, 4, 8, 16, 32, 64, 128]
>
> This can also be easily tested in the code by ensuring that
> the devicetree property is in the range of 1..128 and has exactly
> one bit set, such as
> if (!val || val > 128 || hweight32(val) != 1)
> or with something like
> if (!val || val > 128 || BIT(__fls(val)) != val)
>
> Guenter
>
Got it, will make the property value only allowed with listed values, and add check in drive code.
Thanks,
Potin
>> + $ref: /schemas/types.yaml#/definitions/uint32
>> + minimum: 1
>> + maximum: 128
>> + default: 1
>> +
>> +if:
>> + not:
>> + properties:
>> + compatible:
>> + contains:
>> + enum:
>> + - adi,adm1272
>> + - adi,adm1278
>> + - adi,adm1293
>> + - adi,adm1294
>> +then:
>> + properties:
>> + adi,power-sample-average:
>> + description: This property is not allowed.
>> +
>> 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>;
>> };
>> };
>