2022-02-23 17:24:02

by Potin Lai

Subject: [PATCH 2/2] dt-bindings: hwmon: Add sample averaging property for ADM1275

Add binding information for "pwr-avg" and "vi-avg" properties

Signed-off-by: Potin Lai <[email protected]>
---
.../devicetree/bindings/hwmon/adi,adm1275.yaml | 10 ++++++++++
1 file changed, 10 insertions(+)

diff --git a/Documentation/devicetree/bindings/hwmon/adi,adm1275.yaml b/Documentation/devicetree/bindings/hwmon/adi,adm1275.yaml
index 223393d7cafd..2525a67a880e 100644
--- a/Documentation/devicetree/bindings/hwmon/adi,adm1275.yaml
+++ b/Documentation/devicetree/bindings/hwmon/adi,adm1275.yaml
@@ -37,6 +37,14 @@ properties:
description:
Shunt resistor value in micro-Ohm.

+ vi-avg:
+ description:
+ Sample averaging for current and voltage.
+
+ pwr-avg:
+ description:
+ Sample averaging for power.
+
required:
- compatible
- reg
@@ -53,5 +61,7 @@ examples:
compatible = "adi,adm1272";
reg = <0x10>;
shunt-resistor-micro-ohms = <500>;
+ vi-avg = <128>;
+ pwr-avg = <128>;
};
};
--
2.17.1

2022-02-24 00:55:51

by Guenter Roeck

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

On 2/23/22 08:38, Potin Lai wrote:
> Add binding information for "pwr-avg" and "vi-avg" properties
>
> Signed-off-by: Potin Lai <[email protected]>
> ---
> .../devicetree/bindings/hwmon/adi,adm1275.yaml | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/hwmon/adi,adm1275.yaml b/Documentation/devicetree/bindings/hwmon/adi,adm1275.yaml
> index 223393d7cafd..2525a67a880e 100644
> --- a/Documentation/devicetree/bindings/hwmon/adi,adm1275.yaml
> +++ b/Documentation/devicetree/bindings/hwmon/adi,adm1275.yaml
> @@ -37,6 +37,14 @@ properties:
> description:
> Shunt resistor value in micro-Ohm.
>
> + vi-avg:
> + description:
> + Sample averaging for current and voltage.
> +
> + pwr-avg:
> + description:
> + Sample averaging for power.
> +

Properties need a better name, prefixed with chip vendor, and the valid range
needs to be provided. Also, the description could be better, eg "Number of samples
to be used to report power values". Also, the chips actually supporting power
sampling need to be listed.

Guenter

> required:
> - compatible
> - reg
> @@ -53,5 +61,7 @@ examples:
> compatible = "adi,adm1272";
> reg = <0x10>;
> shunt-resistor-micro-ohms = <500>;
> + vi-avg = <128>;
> + pwr-avg = <128>;
> };
> };

2022-02-24 01:46:12

by Krzysztof Kozlowski

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

On 23/02/2022 17:38, Potin Lai wrote:
> Add binding information for "pwr-avg" and "vi-avg" properties
>
> Signed-off-by: Potin Lai <[email protected]>
> ---
> .../devicetree/bindings/hwmon/adi,adm1275.yaml | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/hwmon/adi,adm1275.yaml b/Documentation/devicetree/bindings/hwmon/adi,adm1275.yaml
> index 223393d7cafd..2525a67a880e 100644
> --- a/Documentation/devicetree/bindings/hwmon/adi,adm1275.yaml
> +++ b/Documentation/devicetree/bindings/hwmon/adi,adm1275.yaml
> @@ -37,6 +37,14 @@ properties:
> description:
> Shunt resistor value in micro-Ohm.
>
> + vi-avg:
> + description:
> + Sample averaging for current and voltage.

Both do not look like generic properties, so you need:
- type
- vendor prefix ("adi,")
- maximum value?

It would be nice to actually explain what this value means - number of
readings for each averaging (unless it is obvious in the domain...)?

Does the hardware have default value?

> +
> + pwr-avg:
> + description:
> + Sample averaging for power.
> +
> required:
> - compatible
> - reg
> @@ -53,5 +61,7 @@ examples:
> compatible = "adi,adm1272";
> reg = <0x10>;
> shunt-resistor-micro-ohms = <500>;
> + vi-avg = <128>;
> + pwr-avg = <128>;
> };
> };


Best regards,
Krzysztof