mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: David Lechner <dlechner@baylibre.com>
To: "Ariana Lazar" <ariana.lazar@microchip.com>,
	"Jonathan Cameron" <jic23@kernel.org>,
	"Nuno Sá" <nuno.sa@analog.com>,
	"Andy Shevchenko" <andy@kernel.org>,
	"Rob Herring" <robh@kernel.org>,
	"Krzysztof Kozlowski" <krzk+dt@kernel.org>,
	"Conor Dooley" <conor+dt@kernel.org>
Cc: linux-iio@vger.kernel.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 1/2] dt-bindings: iio: adc: add support for PAC1711
Date: Sat, 1 Aug 2026 12:30:43 -0500	[thread overview]
Message-ID: <a869acd6-9c77-4495-a581-232ca88ac9e2@baylibre.com> (raw)
In-Reply-To: <20260728-pac1711-v2-1-609bc026093c@microchip.com>

On 7/28/26 7:03 AM, Ariana Lazar wrote:
> This is the device tree schema for Microchip PAC1711, PAC1721, PAC1811 and
> PAC1821 single-channel power monitor with accumulator. The PAC1711 and
> PAC1721 devices use 12-bit resolution for voltage and current measurements
> and 24 bits for power calculations, while PAC1811 and PAC1821 have 16-bit
> resolution and use 32 bits for power calculations. The 56-bit accumulator
> register accumulates power (energy) or current (Coulomb counter).
> 
> PAC1711 and PAC1811 measure up to 42V Full-Scale Range, respectively 9V for
> PAC1721 and PAC1821.
> 
> Signed-off-by: Ariana Lazar <ariana.lazar@microchip.com>
> ---
>  .../bindings/iio/adc/microchip,pac1711.yaml        | 209 +++++++++++++++++++++
>  MAINTAINERS                                        |   6 +
>  2 files changed, 215 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..846e7801a1667c312c753ab8ee4d5637e258e2e5
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/microchip,pac1711.yaml
> @@ -0,0 +1,209 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/adc/microchip,pac1711.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Microchip PAC1711 Power Monitors with Accumulator
> +
> +maintainers:
> +  - Ariana Lazar <ariana.lazar@microchip.com>
> +
> +description: |
> +  This device is part of the Microchip family of Power Monitors with Accumulator.
> +  Datasheet links:
> +  [PAC1711]: https://ww1.microchip.com/downloads/aemDocuments/documents/MSLD/ProductDocuments/PAC1711-Data-Sheet-DS20007058.pdf
> +  [PAC1721]: https://ww1.microchip.com/downloads/aemDocuments/documents/MSLD/ProductDocuments/PAC1721-Single-Channel-Power-Monitor-with-Accumulator-DS20007088.pdf
> +  [PAC1811]: https://ww1.microchip.com/downloads/aemDocuments/documents/MSLD/ProductDocuments/DataSheets/PAC1811-Data-Sheet-DS20007066.pdf
> +  [PAC1821]: https://ww1.microchip.com/downloads/aemDocuments/documents/MSLD/ProductDocuments/DataSheets/PAC1821-Data-Sheet-DS20007097.pdf
> +
> +  PAC1711, PAC1721, PAC1811 and PAC1821 are Single Channel Power Monitors
> +  with Accumulator, having 12-bit or 16-bit resolution. The devices PAC1711
> +  and PAC1811 can measure up to 42V Full-Scale Range, respectively 9V
> +  Full-Scale Range for PAC1721 and PAC1821.
> +
> +properties:
> +  compatible:
> +    enum:
> +      - microchip,pac1711
> +      - microchip,pac1721
> +      - microchip,pac1811
> +      - microchip,pac1821
> +
> +  reg:
> +    maxItems: 1
> +
> +  vdd-supply: true
> +
> +  "#io-channel-cells":
> +    const: 1
> +
> +  interrupts:
> +    description:
> +      Could be triggered by overvoltage, undervoltage, overcurrent, overpower,
> +      undercurrent, step limit, accumulator overflow and accumulator count
> +      overflow.
> +    minItems: 1
> +
> +  interrupt-names:
> +    items:
> +      - enum: [alert0, alert1]
> +
> +  microchip,gpio0-mode:
> +    $ref: /schemas/types.yaml#/definitions/string
> +    description:
> +      Defines the function of the pin. This is a multifunction gpio digital I/O
> +      pin which can be configured as alert0 interrupt, GPIO digital input, GPIO
> +      digital output or slow. When functioning as SLOW pin pulling the pin high
> +      overrides the programmed sample rate and results in a sample rate of 8 sps
> +      (Slow mode).
> +    enum: [alert0, gpio0_input, gpio0_output, slow]
> +    default: gpio0_input
> +
> +  microchip,gpio1-mode:
> +    $ref: /schemas/types.yaml#/definitions/string
> +    description:
> +      Defines the function of the pin. This is a multifunction gpio digital I/O
> +      pin which can be configured as alert1 interrupt, GPIO digital input, GPIO
> +      digital output or slow. When functioning as SLOW pin pulling the pin high
> +      overrides the programmed sample rate and results in a sample rate of 8 sps
> +      (Slow mode).
> +    enum: [alert1, gpio1_input, gpio1_output, slow]
> +    default: gpio1_input

Maybe I missed something in the previous discussions, but this
seems a bit too restrictive and also a bit redundant.

interrupt-names already tells us if A0 or A1 is used for /ALERT.

And why should we restrict GPIO usage to only input or output?
(Binding should have gpio-controller and #gpio-cells properties
for that too.)

It also isn't clear to me how the slow pin would be useful when
we can also program the sample mode to the same rate over I2C.
So maybe we should defer adding a binding for that until we have
an application that actually requires it.

And this is missing the possibility that the pins can be used
as a conversion trigger as well. Likely that would use a
trigger-sources binding, but as that isn't common, I would defer
adding that until we have a use case.

So I would just leave these properties out.

> +
> +  powerdown-gpios:
> +    description:
> +      Active low puts the device in power-down state. When the PWRDN pin is
> +      pulled high, measurement and accumulation will resume using the default
> +      register settings.
> +    maxItems: 1
> +
> +  shunt-resistor-micro-ohms:
> +    description:
> +      Value in micro Ohms of the shunt resistor connected between
> +      the VSENSEP and VSENSEN inputs, across which the current is measured.

It looks like all of the datasheets say VSENSE+ and VSENSE- rather than P, N.

> +      Value is needed to compute the scaling of the measured current.
> +
> +  label:
> +    description: Unique name to identify which device this is.
> +
> +  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:
> +        - for PAC1711 or PAC1811:
> +            - VBUS has unipolar 0V to 42V FSR (default)
> +            - VBUS has bipolar -42V to 42V FSR
> +            - VBUS has bipolar -21V to 21V FSR
> +        - for PAC1721 or PAC1821:
> +            - VBUS has unipolar 0V to 9V FSR (default)
> +            - VBUS has bipolar -9V to 9V FSR
> +            - VBUS has bipolar -4.5V to 4.5V FSR
> +
> +  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 100 mV FSR (default)
> +        - VSENSE has bipolar -100 mV to 100 mV FSR
> +        - VSENSE has bipolar -50 mV to 50 mV FSR
> +    oneOf:
> +      - items:
> +          - const: 0
> +          - const: 100000
> +      - items:
> +          - const: -100000
> +          - const: 100000
> +      - items:
> +          - const: -50000
> +          - const: 50000
> +
> +  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?

Also datahseet says it can accumulate vbus measurements.

> +
> +required:
> +  - compatible
> +  - reg
> +  - vdd-supply
> +  - shunt-resistor-micro-ohms
> +

  reply	other threads:[~2026-08-01 17:30 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-28 12:03 [PATCH v2 0/2] add support for Microchip PAC1711 Power Monitor Ariana Lazar
2026-07-28 12:03 ` [PATCH v2 1/2] dt-bindings: iio: adc: add support for PAC1711 Ariana Lazar
2026-08-01 17:30   ` David Lechner [this message]
2026-08-01 23:13     ` Jonathan Cameron
2026-08-12 13:34     ` Ariana.Lazar
2026-08-12 13:56       ` David Lechner
2026-07-28 12:03 ` [PATCH v2 2/2] " Ariana Lazar
2026-07-29 12:23   ` Uwe Kleine-König
2026-08-01 18:17   ` David Lechner
2026-08-01 23:38   ` Jonathan Cameron
2026-08-01 23:08 ` [PATCH v2 0/2] add support for Microchip PAC1711 Power Monitor Jonathan Cameron
2026-08-01 23:21 ` Jonathan Cameron

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=a869acd6-9c77-4495-a581-232ca88ac9e2@baylibre.com \
    --to=dlechner@baylibre.com \
    --cc=andy@kernel.org \
    --cc=ariana.lazar@microchip.com \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --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

all inboxes | Powered by JetHome®