On 25/02/2022 09:41, Guenter Roeck wrote: > On 2/24/22 23:31, Krzysztof Kozlowski wrote: >> On 25/02/2022 08:06, Krzysztof Kozlowski wrote: >>> On 24/02/2022 16:43, Potin Lai wrote: >>>> Add new properties for binding sample averaging in PMON_CONFIG register >>>> >>>> - adi,volt-curr-sample-average >>>> - adi,power-sample-average >>>> >>>> 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..325f6827648f 100644 >>>> --- a/Documentation/devicetree/bindings/hwmon/adi,adm1275.yaml >>>> +++ b/Documentation/devicetree/bindings/hwmon/adi,adm1275.yaml >>>> @@ -37,6 +37,48 @@ properties: >>>> description: >>>> Shunt resistor value in micro-Ohm. >>>> >>>> + adi,volt-curr-sample-average: >>>> + description: | >>>> + A value to configure VI_AVG in PMON_CONFIG register to indicate a >>>> + number of samples to be used to report voltage and currentvalues. >>> >>> missing space after current. >>> >>>> + If set to 7, the 128 samples averaging would be used. >>>> + >>>> + $ref: /schemas/types.yaml#/definitions/uint8 >>> >>> Make it a uint32. >>> >>> The previous usage of this field was more appropriate. Instead of >>> keeping register values in DT, it's better to keep logical value. What >>> if in next cheap the register values have calculation method? >>> >>> This should be like in v1 - enum for number of samples to take in averaging. >>> >> >> One more thought: this field could also stay in current approach if you >> change the meaning from "value to configure VI_AVG" to something like >> "the exponent used to determine the number of samples, where the base is 2". >> >> This approach would allow you to skip the "ilog" in the code. It sill >> won't be that easily scalable if another chip comes with different >> formula, but I think that's unlikely. >> > > The standard hwmon ABI expects the number of samples, it isn't always a > power of 2, and the driver already implements it (with ilog2) as sysfs > attribute. I don't really see the point of "optimizing" something > like this to be chip specific just to avoid some error checking. Thanks for confirming. +1 Best regards, Krzysztof
