linux-kernel - Re: [PATCH 1/2] dt-bindings: iio: adc: adding support for PAC1711

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <77443ac90eb95fbf355509a7f060c05ee1d24d37.camel@microchip.com>
Date: Tue, 25 Nov 2025 17:25:29 +0000
From: <Ariana.Lazar@...rochip.com>
To: <jic23@...nel.org>
CC: <dlechner@...libre.com>, <nuno.sa@...log.com>,
	<linux-iio@...r.kernel.org>, <devicetree@...r.kernel.org>, <robh@...nel.org>,
	<linux-kernel@...r.kernel.org>, <andy@...nel.org>, <krzk+dt@...nel.org>,
	<conor+dt@...nel.org>
Subject: Re: [PATCH 1/2] dt-bindings: iio: adc: adding support for PAC1711

Hi Jonathan,

Thanks for the review.

On Sun, 2025-10-19 at 10:04 +0100, Jonathan Cameron wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you
> know the content is safe
> 
> On Wed, 15 Oct 2025 13:12:15 +0300
> Ariana Lazar <ariana.lazar@...rochip.com> wrote:
> 
> > This is the device tree schema for Microchip PAC1711 single-channel
> > power
> > monitor with accumulator. The device uses 12-bit resolution for
> > voltage and
> > current measurements and 24 bits power calculations. The device
> > supports
> > one 56-bit accumulator register.
> > 
> > PAC1711 measures up to 42V Full-Scale Range.
> > 
> > Signed-off-by: Ariana Lazar <ariana.lazar@...rochip.com>
> > ---
> >  .../bindings/iio/adc/microchip,pac1711.yaml        | 195
> > +++++++++++++++++++++
> >  MAINTAINERS                                        |   6 +
> >  2 files changed, 201 insertions(+)
> > 
> > diff --git
> > a/Documentation/devicetree/bindings/iio/adc/microchip,pac1711.yaml
> > b/Documentation/devicetree/bindings/iio/adc/microchip,pac1711.yaml
> > new file mode 100644
> > index
> > 0000000000000000000000000000000000000000..67edd778981c2f0ed21dda02f
> > 14e383a153169b1
> > --- /dev/null
> > +++
> > b/Documentation/devicetree/bindings/iio/adc/microchip,pac1711.yaml
> 
> > +  microchip,vbus-input-range-microvolt:
> > +    description: |
> > +      Specifies the voltage range in microvolts chosen for the
> > voltage full
> > +      scale range (FSR). The range should be set as <minimum,
> > maximum> by
> > +      hardware design and should not be changed during runtime.
> > +
> > +      The VBUS could be configured into the following full scale
> > range:
> > +        -  VBUS has unipolar 0V to 42V FSR (default)
> > +        -  VBUS has bipolar -42V to 42V FSR
> > +        -  VBUS has bipolar -21V to 21V FSR
> > +    items:
> > +      - enum: [-42000000, -21000000, 0]
> > +      - enum: [21000000, 42000000]
> > +
> > +  microchip,vsense-input-range-microvolt:
> > +    description: |
> > +      Specifies the voltage range in microvolts chosen for the
> > current full
> > +      scale range (FSR). The current is calculated by dividing the
> > vsense
> > +      voltage by the value of the shunt resistor. The range should
> > be set as
> > +      <minimum, maximum> by hardware design and it should not be
> > changed during
> > +      runtime.
> > +
> > +      The VSENSE could be configured into the following full scale
> > range:
> > +        -  VSENSE has unipolar 0 mV to 100V FSR (default)
> > +        -  VSENSE has bipolar -100 mV to 100 mV FSR
> > +        -  VSENSE has bipolar -50 mV to 50 mV FSR
> > +    items:
> > +      - enum: [-100000, -50000, 0]
> > +      - enum: [50000, 100000]
> 
> These range setting things are common enough perhaps it's time to
> standardize
> them as properties of channel sub nodes (the stuff in adc.yaml).

Great idea. I will try to implement this into a separate patch.

> 
> > +
> > +  microchip,accumulation-mode:
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    description: |
> > +      The Hardware Accumulator may be used to accumulate VPOWER,
> > VSENSE or
> > +      VBUS values for any channel. By setting the accumulator for
> > a channel
> > +      to accumulate the VPOWER values gives a measure of
> > accumulated power
> > +      into 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:
> > +       <0>  -  Accumulator accumulates VPOWER (default)
> > +       <1>  -  Accumulator accumulates VSENSE
> > +       <2>  -  Accumulator accumulates VBUS
> 
> This feels like a runtime thing to control. To be in DT it should be
> related
> to the board wiring rather than being a choice between multiple
> things
> being measured.
> > 
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 doesn't 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 doesn't 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.  

Best regards,
Ariana

>