Re: [PATCH v2 1/2] dt-bindings: iio: adc: add support for PAC1711

[Ariana.Lazar]

Hi David,

Thank you for the review. Please see my comments below.

> > +
> > +  microchip,accumulation-mode:
> > +    $ref: /schemas/types.yaml#/definitions/string
> > +    description: |
> > +      The Hardware Accumulator may be used to accumulate VPOWER or
> > VSENSE values
> > +      for any channel. By setting the accumulator for a channel to
> > accumulate
> > +      the VPOWER values gives a measure of accumulated power over
> > a time period,
> > +      which is equivalent to energy. Setting the accumulator for a
> > channel to
> > +      accumulate VSENSE values gives a measure of accumulated
> > current, which is
> > +      equivalent to charge.
> > +
> > +      The Hardware Accumulator could be configured as:
> > +       "vpower" - Accumulator accumulates VPOWER (energy)
> > +       "vsense" - Accumulator accumulates VSENSE (Coulomb Counter)
> > +    enum: [vpower, vsense]
> > +    default: vpower
> 
> Why does this one have to be a DT property? Can it not be switched
> at runtime to accumulate one or the other at different times?

This property aims to specify what kind of hardware is intended to be
used/available for the user.

There are two main cases here:
- the user wants to measure also the current/power consumed before the
driver insertion (e.g. from the boot to user control) and if this is a
runtime setting, the hardware accumulator will be reset by the default
configuration the driver starts with.
- the driver does not know what type of hardware it's dealing with. In
case the part is monitoring the charge/discharge current it does not
make sense in user-space to change the accumulator to calculate energy.
Same if the hardware is intended to calculate energy it does not make
sense in user-space to change to Coulomb counter. Changing the setting
from one mode to another will reset the hardware accumulator inside the
chip. 

> 
> Also datahseet says it can accumulate vbus measurements.
> 
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - vdd-supply
> > +  - shunt-resistor-micro-ohms
> > +

In the previous version of this patch series it was recommended to drop
VBUS accumulation option from the supported functionalities because it
has no practical usecase (other then maybe long term average) as
Jonathan suggested in the review for version 1:
https://lore.kernel.org/all/20251015-pac1711-v1-2-976949e36367@xxxxxxxxxxxxx/

I will readd it if it is needed.

Best regards,
Ariana