On 3/1/22 05:21, Krzysztof Kozlowski wrote:
On 01/03/2022 13:42, POTIN LAI wrote:
Krzysztof Kozlowski 於 1/03/2022 7:16 pm 寫道:
On 01/03/2022 11:39, 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
- adi,power-sample-average-enable
Signed-off-by: Potin Lai <potin.lai@xxxxxxxxxxxx>
---
.../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..1b612dc06992 100644
--- a/Documentation/devicetree/bindings/hwmon/adi,adm1275.yaml
+++ b/Documentation/devicetree/bindings/hwmon/adi,adm1275.yaml
@@ -37,6 +37,47 @@ 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
+ enum: [1, 2, 4, 8, 16, 32, 64, 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
+ enum: [1, 2, 4, 8, 16, 32, 64, 128]
+ default: 1
+
+ adi,power-sample-average-enable:
+ description: Enable sample averaging for power reading.
+ type: boolean
Why do you need this property? Voltage/current sampling is enabled in
your driver with presence of adi,volt-curr-sample-average. Why power
sampling is different?
For "adi,power-sample-average", adm1075, adm1275 & adm127 don't have config reg for power sample average, so I add boolean type property to enable it
But for "adi,power-sample-average-enable", all chips have ability of configuring, so it doesn't need a property to enable or disable.
So the reason to add separate property is that this feature can be
disabled. Since your driver does not disable it, it seems it is a
default state to have it disabled and you have to enable it, right?
Where is the enable code? I see you only write the sample averaging
value with adm1275_write_pmon_config(). There is no enable...
But wait, the power averaging is being disabled by writing 0 to
register, which is not allowed by bindings. How one can disable it?
Valid register values are 0..7, matching dt property values of
BIT(x) or 1 .. 128. Setting the property value to 1 (= 1 sample)
is equivalent to disabling sampling.
I don't see any usage of "adi,power-sample-average-enable", neither in
driver nor in hardware. I also do not see the need for it, the purpose.
Then second part, you added default value of 1 to
adi,volt-curr-sample-average and adi,power-sample-average. If the
property is missing, then the default of 1 is applied, right? But
datasheet says that default is 128!
The bindings neither match hardware nor driver. They look entirely
independent. This is wrong. They should instead be strongly related to
the hardware, describe the hardware. Then the driver should implement
proper logic for it.
Agreed.
Guenter
