linux-iio.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: <Ariana.Lazar@microchip.com>
To: <jic23@kernel.org>
Cc: <dlechner@baylibre.com>, <nuno.sa@analog.com>,
	<linux-iio@vger.kernel.org>, <devicetree@vger.kernel.org>,
	<robh@kernel.org>, <linux-kernel@vger.kernel.org>,
	<andy@kernel.org>, <krzk+dt@kernel.org>, <conor+dt@kernel.org>
Subject: Re: [PATCH 1/2] dt-bindings: iio: adc: adding support for PAC1711
Date: Tue, 25 Nov 2025 17:25:29 +0000	[thread overview]
Message-ID: <77443ac90eb95fbf355509a7f060c05ee1d24d37.camel@microchip.com> (raw)
In-Reply-To: <20251019100454.6bfbc3c7@jic23-huawei>

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@microchip.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@microchip.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

> 


  reply	other threads:[~2025-11-25 17:25 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-10-15 10:12 [PATCH 0/2] Adding support for Microchip PAC1711 Ariana Lazar
2025-10-15 10:12 ` [PATCH 1/2] dt-bindings: iio: adc: adding support for PAC1711 Ariana Lazar
2025-10-16 16:19   ` Conor Dooley
2025-10-19  9:04   ` Jonathan Cameron
2025-11-25 17:25     ` Ariana.Lazar [this message]
2025-10-15 10:12 ` [PATCH 2/2] " Ariana Lazar
2025-10-16  5:31   ` kernel test robot
2025-10-19 10:28   ` Jonathan Cameron
2025-11-19 15:31     ` Ariana.Lazar
2025-11-19 15:58       ` Andy Shevchenko
2025-10-23  7:31   ` Dan Carpenter
2025-10-19 10:31 ` [PATCH 0/2] Adding support for Microchip PAC1711 Jonathan Cameron
2025-10-19 15:02   ` Guenter Roeck
2025-10-23 12:36     ` Ariana.Lazar

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=77443ac90eb95fbf355509a7f060c05ee1d24d37.camel@microchip.com \
    --to=ariana.lazar@microchip.com \
    --cc=andy@kernel.org \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=dlechner@baylibre.com \
    --cc=jic23@kernel.org \
    --cc=krzk+dt@kernel.org \
    --cc=linux-iio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=nuno.sa@analog.com \
    --cc=robh@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).