From: Marius Cristea <marius.cristea@microchip.com>
This is the device tree schema for iio driver for Microchip PAC194X
and PAC195X series of Power Monitors with Accumulator. The PAC194X
family supports 9V Full-Scale Range and the PAC195X supports 32V
Full-Scale Range.
There are two versions of the PAC194X/5X: the PAC194X/5X-1 devices
are for high-side current sensing and the PAC194X/5X-2 devices are
for low-side current sensing or floating VBUS applications.
The PAC194X/5X-1 is named shortly PAC194X/5X.
Signed-off-by: Marius Cristea <marius.cristea@microchip.com>
---
.../bindings/iio/adc/microchip,pac1944.yaml | 204 ++++++++++++++++++
1 file changed, 204 insertions(+)
create mode 100644 Documentation/devicetree/bindings/iio/adc/microchip,pac1944.yaml
diff --git a/Documentation/devicetree/bindings/iio/adc/microchip,pac1944.yaml b/Documentation/devicetree/bindings/iio/adc/microchip,pac1944.yaml
new file mode 100644
index 000000000000..4a2cf6b64055
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/microchip,pac1944.yaml
@@ -0,0 +1,204 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/adc/microchip,pac1944.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Microchip PAC1944 and PAC1954 Power Monitors with Accumulator
+
+maintainers:
+ - Marius Cristea <marius.cristea@microchip.com>
+
+description: |
+ This device is part of the Microchip family of Power Monitors with
+ Accumulator. The datasheet for PAC1941-1, PAC1941-1, PAC1942-1, PAC1942-2,
+ PAC1943-1 and PAC1944-1 can be found here:
+ https://ww1.microchip.com/downloads/aemDocuments/documents/MSLD/ProductDocuments/DataSheets/PAC194X-Family-Data-Sheet-DS20006543.pdf
+ The datasheet for PAC1951-1, PAC1951-1, PAC1952-1, PAC1952-2, PAC1953-1 and
+ PAC1954-1 can be found here:
+ https://ww1.microchip.com/downloads/aemDocuments/documents/MSLD/ProductDocuments/DataSheets/PAC195X-Family-Data-Sheet-DS20006539.pdf
+
+properties:
+ compatible:
+ enum:
+ - microchip,pac1941
+ - microchip,pac19412
+ - microchip,pac1942
+ - microchip,pac19422
+ - microchip,pac1943
+ - microchip,pac1944
+ - microchip,pac1951
+ - microchip,pac19512
+ - microchip,pac1952
+ - microchip,pac19522
+ - microchip,pac1953
+ - microchip,pac1954
+
+ reg:
+ maxItems: 1
+
+ vdd-supply: true
+
+ "#address-cells":
+ const: 1
+
+ "#size-cells":
+ const: 0
+
+ interrupts:
+ maxItems: 2
+
+ interrupt-names:
+ description:
+ alert1 indicates a HIGH or LOW limit was exceeded.
+ alert2 indicates a THERM limit was exceeded.
+ items:
+ - const: alert1
+ - const: alert2
+
+patternProperties:
+ "^channel@[1-4]+$":
+ type: object
+ $ref: adc.yaml
+ description:
+ Represents the external channels which are connected to the ADC.
+
+ properties:
+ reg:
+ items:
+ minimum: 1
+ maximum: 4
+
+ shunt-resistor-micro-ohms:
+ description:
+ Value in micro Ohms of the shunt resistor connected between
+ the SENSE+ and SENSE- inputs, across which the current is measured.
+ Value is needed to compute the scaling of the measured current.
+
+ microchip,vbus-half-range:
+ $ref: /schemas/types.yaml#/definitions/flag
+ description: |
+ In order to increase measurement resolution and keeping the same
+ number the of bits the device has a configurable VBUS full range scale
+ (FSR). The range should be set by hardware design and it should not be
+ changed during runtime. The bipolar capability for VBUS enables
+ accurate offset measurement and correction.
+ The VBUS could be configured into the following full scale range:
+ - VBUS has unipolar 0V to 32V FSR (default) for PAC195X or 0V to 9V
+ (default) for PAC194X.
+ - VBUS has bipolar -32V to 32V FSR for PAC195X or -9V to 9V for
+ PAC194X. The actual range is limited to about -200 mV due to the
+ impact of the ESD structures.
+ - VBUS has bipolar -16V to 16V FSR for PAC195X or -4.5V to 4.5V for
+ PAC194X. The actual range is limited to about -200 mV due to the
+ impact of the ESD structures.
+
+ microchip,vbus-bipolar:
+ $ref: /schemas/types.yaml#/definitions/flag
+ description:
+ If provided, the channel is to be used in bipolar mode. The
+ actual range is limited to about -200 mV due to the impact of the ESD
+ structures.
+
+ microchip,vsense-half-range:
+ $ref: /schemas/types.yaml#/definitions/flag
+ description: |
+ In order to decrease the power dissipation on the shunt resistor and
+ in the same time to increase measurement resolution by keeping the
+ same number the of bits the device has a configurable VSENSE full
+ range scale (FSR). The range should be set 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 0V to 100 mV FSR (default)
+ - VSENSE has bipolar -100 mV to 100 mV FSR
+ - VSENSE has bipolar -50 mV to 50 mV FSR
+
+ microchip,vsense-bipolar:
+ $ref: /schemas/types.yaml#/definitions/flag
+ description:
+ If provided, the channel is to be used in bipolar mode.
+
+ 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. This allows the
+ accumulator to be used as a coulomb counter. For either VSENSE or
+ VBUS, many samples may be accumulated on chip and the result collected
+ by the host and divided by the accumulator counter count value to
+ yield an average value with a very long integration time to reduce
+ noise. This feature is also very useful for system calibration,
+ allowing many averages to be accumulated for fast averaging/noise
+ reduction.
+ This functionality needs to be setup once and must not be changed
+ during the runtime, just in case the user wants to measure the charge
+ or the energy consumed from board power up till the user has control
+ or during a reboot of the system.
+ The Hardware Accumulator could be configured to accumulate
+ VPOWER, VSENSE or VBUS
+ <0> - Accumulator accumulates VPOWER (default)
+ <1> - Accumulator accumulates VSENSE
+ <2> - Accumulator accumulates VBUS
+ maximum: 2
+ default: 0
+
+ required:
+ - reg
+ - shunt-resistor-micro-ohms
+
+ unevaluatedProperties: false
+
+required:
+ - compatible
+ - reg
+ - vdd-supply
+ - "#address-cells"
+ - "#size-cells"
+
+additionalProperties: false
+
+examples:
+ - |
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ power-monitor@10 {
+ compatible = "microchip,pac1954";
+ reg = <0x10>;
+ vdd-supply = <&vdd>;
+
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ channel@1 {
+ reg = <0x1>;
+ shunt-resistor-micro-ohms = <24900>;
+ label = "CPU";
+ microchip,vsense-half-range;
+ microchip,vsense-bipolar;
+ };
+
+ channel@3 {
+ reg = <0x3>;
+ shunt-resistor-micro-ohms = <75000>;
+ label = "MEM";
+ microchip,vbus-half-range;
+ microchip,vbus-bipolar;
+ microchip,vsense-half-range;
+ };
+
+ channel@4 {
+ reg = <0x4>;
+ shunt-resistor-micro-ohms = <100000>;
+ label = "NET";
+ microchip,vbus-bipolar;
+ };
+ };
+ };
+
+...
--
2.48.1On Fri, Jun 06, 2025 at 12:39:28PM +0300, marius.cristea@microchip.com wrote:
David had a bunch of comments, so much to say, but..
> + interrupt-names:
> + description:
> + alert1 indicates a HIGH or LOW limit was exceeded.
> + alert2 indicates a THERM limit was exceeded.
You should be able to merge this description into the items list,
> + items:
> + - const: alert1
by adding description: into these
> + - const: alert2
e.g.
items:
- const: sdmmc-3v3
description: pad configuration for 3.3 V
- const: sdmmc-1v8
description: pad configuration for 1.8 V
- const: sdmmc-3v3-drv
description: pull-up/down configuration for 3.3 V
- const: sdmmc-1v8-drv
description: pull-up/down configuration for 1.8 V
On 6/6/25 4:39 AM, marius.cristea@microchip.com wrote:
> From: Marius Cristea <marius.cristea@microchip.com>
>
> This is the device tree schema for iio driver for Microchip PAC194X
> and PAC195X series of Power Monitors with Accumulator. The PAC194X
> family supports 9V Full-Scale Range and the PAC195X supports 32V
> Full-Scale Range.
>
> There are two versions of the PAC194X/5X: the PAC194X/5X-1 devices
> are for high-side current sensing and the PAC194X/5X-2 devices are
> for low-side current sensing or floating VBUS applications.
>
> The PAC194X/5X-1 is named shortly PAC194X/5X.
>
> Signed-off-by: Marius Cristea <marius.cristea@microchip.com>
> ---
> .../bindings/iio/adc/microchip,pac1944.yaml | 204 ++++++++++++++++++
> 1 file changed, 204 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/iio/adc/microchip,pac1944.yaml
>
> diff --git a/Documentation/devicetree/bindings/iio/adc/microchip,pac1944.yaml b/Documentation/devicetree/bindings/iio/adc/microchip,pac1944.yaml
> new file mode 100644
> index 000000000000..4a2cf6b64055
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/microchip,pac1944.yaml
> @@ -0,0 +1,204 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/adc/microchip,pac1944.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Microchip PAC1944 and PAC1954 Power Monitors with Accumulator
> +
> +maintainers:
> + - Marius Cristea <marius.cristea@microchip.com>
> +
> +description: |
> + This device is part of the Microchip family of Power Monitors with
> + Accumulator. The datasheet for PAC1941-1, PAC1941-1, PAC1942-1, PAC1942-2,
> + PAC1943-1 and PAC1944-1 can be found here:
> + https://ww1.microchip.com/downloads/aemDocuments/documents/MSLD/ProductDocuments/DataSheets/PAC194X-Family-Data-Sheet-DS20006543.pdf
> + The datasheet for PAC1951-1, PAC1951-1, PAC1952-1, PAC1952-2, PAC1953-1 and
> + PAC1954-1 can be found here:
> + https://ww1.microchip.com/downloads/aemDocuments/documents/MSLD/ProductDocuments/DataSheets/PAC195X-Family-Data-Sheet-DS20006539.pdf
> +
> +properties:
> + compatible:
> + enum:
> + - microchip,pac1941
> + - microchip,pac19412
> + - microchip,pac1942
> + - microchip,pac19422
> + - microchip,pac1943
> + - microchip,pac1944
> + - microchip,pac1951
> + - microchip,pac19512
> + - microchip,pac1952
> + - microchip,pac19522
> + - microchip,pac1953
> + - microchip,pac1954
> +
> + reg:
> + maxItems: 1
> +
> + vdd-supply: true
> +
> + "#address-cells":
> + const: 1
> +
> + "#size-cells":
> + const: 0
> +
> + interrupts:
> + maxItems: 2
> +
> + interrupt-names:
Needs minItems: 1 if we want to allow a single named interrupt.
> + description:
> + alert1 indicates a HIGH or LOW limit was exceeded.
> + alert2 indicates a THERM limit was exceeded.
> + items:
> + - const: alert1
> + - const: alert2
> +
I am having deja vu. I just commented on an identical interrupts binding
in a different series [1]. In this case though, alert1 and alert2 are
the actual pin names, so that is fine. But each pin can be programmed
to indicate lots of different things, so drop descriptions or change
them to describe the pin, not an arbitrary function. I don't even
see THERM in the datasheet, so I'm guessing that was just a copy/
paste from something else anyway.
[1]: https://lore.kernel.org/linux-iio/0f68e3f9-cba5-4df3-8e56-2cccbccf35ce@baylibre.com/
---
Even if the driver doesn't use them (yet), we could consider adding
gpio-controller and #gpio-cells properties since these chips have pins
that can operate as GPIOs.
And we could add a powerdown-gpios property for the /PWRDN pin.
We want to try to make the bindings as complete as possible, if we can [2].
[2]: https://docs.kernel.org/devicetree/bindings/writing-bindings.html
> +patternProperties:
> + "^channel@[1-4]+$":
Drop the +. Only 1 to 4 are allowed, not 11, 111, etc.
Also, we could further restrict things based on the actual number of
channels on a chip like this:
allOf:
- if:
properties:
compatible:
pattern: "^pac19[45]1"
then:
properties:
channel@1:
reg:
items:
maximum: 1
patternProperties:
^channel@[2-4]$": false
- if:
properties:
compatible:
pattern: "^pac19[45]2"
then:
patternProperties:
^channel@[1-2]$":
reg:
items:
maximum: 2
patternProperties:
^channel@[3-4]$": false
- if:
properties:
compatible:
pattern: "^pac19[45]3"
then:
patternProperties:
^channel@[1-3]$":
reg:
items:
maximum: 3
properties:
channel@4: false
> + type: object
> + $ref: adc.yaml
> + description:
> + Represents the external channels which are connected to the ADC.
> +
> + properties:
> + reg:
> + items:
> + minimum: 1
> + maximum: 4
> +
> + shunt-resistor-micro-ohms:
> + description:
> + Value in micro Ohms of the shunt resistor connected between
> + the SENSE+ and SENSE- inputs, across which the current is measured.
> + Value is needed to compute the scaling of the measured current.
> +
> + microchip,vbus-half-range:
> + $ref: /schemas/types.yaml#/definitions/flag
> + description: |
> + In order to increase measurement resolution and keeping the same
> + number the of bits the device has a configurable VBUS full range scale
> + (FSR). The range should be set by hardware design and it should not be
> + changed during runtime. The bipolar capability for VBUS enables
> + accurate offset measurement and correction.
> + The VBUS could be configured into the following full scale range:
> + - VBUS has unipolar 0V to 32V FSR (default) for PAC195X or 0V to 9V
> + (default) for PAC194X.
> + - VBUS has bipolar -32V to 32V FSR for PAC195X or -9V to 9V for
> + PAC194X. The actual range is limited to about -200 mV due to the
> + impact of the ESD structures.
> + - VBUS has bipolar -16V to 16V FSR for PAC195X or -4.5V to 4.5V for
> + PAC194X. The actual range is limited to about -200 mV due to the
> + impact of the ESD structures.
> +
> + microchip,vbus-bipolar:
> + $ref: /schemas/types.yaml#/definitions/flag
> + description:
> + If provided, the channel is to be used in bipolar mode. The
> + actual range is limited to about -200 mV due to the impact of the ESD
> + structures.
> +
Using Jonathan's suggestion from v2 to just have a single property with 3 different
ranges to chose from seems simpler that this. It would only require one property
and would be self-documenting. The description could be shortened to just a couple
of lines.
Otherwise, we also need to add:
- if:
required:
microchip,vbus-half-range
then:
required:
microchip,vbus-bipolar
to validate that that there are only 3 possibilities.
Also, swapping the word order to range-half would be more consistent with
the existing adi,range-double property that serves a similar purpose.
Same applies to vsense.
> + microchip,vsense-half-range:
> + $ref: /schemas/types.yaml#/definitions/flag
> + description: |
> + In order to decrease the power dissipation on the shunt resistor and
> + in the same time to increase measurement resolution by keeping the
> + same number the of bits the device has a configurable VSENSE full
> + range scale (FSR). The range should be set 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 0V to 100 mV FSR (default)
> + - VSENSE has bipolar -100 mV to 100 mV FSR
> + - VSENSE has bipolar -50 mV to 50 mV FSR
> +
> + microchip,vsense-bipolar:
> + $ref: /schemas/types.yaml#/definitions/flag
> + description:
> + If provided, the channel is to be used in bipolar mode.
> +
> + 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. This allows the
> + accumulator to be used as a coulomb counter. For either VSENSE or
> + VBUS, many samples may be accumulated on chip and the result collected
> + by the host and divided by the accumulator counter count value to
> + yield an average value with a very long integration time to reduce
> + noise. This feature is also very useful for system calibration,
> + allowing many averages to be accumulated for fast averaging/noise
> + reduction.
> + This functionality needs to be setup once and must not be changed
> + during the runtime,
Why not? The datasheet says there is a REFRESH command to allow changing it
at runtime.
> just in case the user wants to measure the charge
> + or the energy consumed from board power up till the user has control
> + or during a reboot of the system.
> + The Hardware Accumulator could be configured to accumulate
> + VPOWER, VSENSE or VBUS
> + <0> - Accumulator accumulates VPOWER (default)
> + <1> - Accumulator accumulates VSENSE
> + <2> - Accumulator accumulates VBUS
> + maximum: 2
> + default: 0
> +
> + required:
> + - reg
> + - shunt-resistor-micro-ohms
> +
> + unevaluatedProperties: false
> +
> +required:
> + - compatible
> + - reg
> + - vdd-supply
> + - "#address-cells"
> + - "#size-cells"
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + i2c {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + power-monitor@10 {
> + compatible = "microchip,pac1954";
> + reg = <0x10>;
> + vdd-supply = <&vdd>;
> +
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + channel@1 {
> + reg = <0x1>;
> + shunt-resistor-micro-ohms = <24900>;
> + label = "CPU";
> + microchip,vsense-half-range;
> + microchip,vsense-bipolar;
> + };
Seems odd to leave a channel unconfigured since the shunt resistor
value is required and there is a 3 channel version of the chip that
could be used if only 3 channels were wired up.
> +
> + channel@3 {
> + reg = <0x3>;
> + shunt-resistor-micro-ohms = <75000>;
> + label = "MEM";
> + microchip,vbus-half-range;
> + microchip,vbus-bipolar;
> + microchip,vsense-half-range;
> + };
> +
> + channel@4 {
> + reg = <0x4>;
> + shunt-resistor-micro-ohms = <100000>;
> + label = "NET";
> + microchip,vbus-bipolar;
> + };
> + };
> + };
> +
> +...
Hi David,
Thank you for the feedback. Please see my comments below...
On Fri, 2025-06-06 at 10:53 -0500, David Lechner wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you
> know the content is safe
>
> On 6/6/25 4:39 AM, marius.cristea@microchip.com wrote:
> > From: Marius Cristea <marius.cristea@microchip.com>
> >
> > This is the device tree schema for iio driver for Microchip PAC194X
> > and PAC195X series of Power Monitors with Accumulator. The PAC194X
> > family supports 9V Full-Scale Range and the PAC195X supports 32V
> > Full-Scale Range.
> >
> > There are two versions of the PAC194X/5X: the PAC194X/5X-1 devices
> > are for high-side current sensing and the PAC194X/5X-2 devices are
> > for low-side current sensing or floating VBUS applications.
> >
> > The PAC194X/5X-1 is named shortly PAC194X/5X.
> >
> > Signed-off-by: Marius Cristea <marius.cristea@microchip.com>
> > ---
> > .../bindings/iio/adc/microchip,pac1944.yaml | 204
> > ++++++++++++++++++
> > 1 file changed, 204 insertions(+)
> > create mode 100644
> > Documentation/devicetree/bindings/iio/adc/microchip,pac1944.yaml
> >
> > diff --git
> > a/Documentation/devicetree/bindings/iio/adc/microchip,pac1944.yaml
> > b/Documentation/devicetree/bindings/iio/adc/microchip,pac1944.yaml
> > new file mode 100644
> > index 000000000000..4a2cf6b64055
> > --- /dev/null
> > +++
> > b/Documentation/devicetree/bindings/iio/adc/microchip,pac1944.yaml
> > @@ -0,0 +1,204 @@
> > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/iio/adc/microchip,pac1944.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Microchip PAC1944 and PAC1954 Power Monitors with
> > Accumulator
> > +
> > +maintainers:
> > + - Marius Cristea <marius.cristea@microchip.com>
> > +
> > +description: |
> > + This device is part of the Microchip family of Power Monitors
> > with
> > + Accumulator. The datasheet for PAC1941-1, PAC1941-1, PAC1942-1,
> > PAC1942-2,
> > + PAC1943-1 and PAC1944-1 can be found here:
> > +
> > https://ww1.microchip.com/downloads/aemDocuments/documents/MSLD/ProductDocuments/DataSheets/PAC194X-Family-Data-Sheet-DS20006543.pdf
> > + The datasheet for PAC1951-1, PAC1951-1, PAC1952-1, PAC1952-2,
> > PAC1953-1 and
> > + PAC1954-1 can be found here:
> > +
> > https://ww1.microchip.com/downloads/aemDocuments/documents/MSLD/ProductDocuments/DataSheets/PAC195X-Family-Data-Sheet-DS20006539.pdf
> > +
> > +properties:
> > + compatible:
> > + enum:
> > + - microchip,pac1941
> > + - microchip,pac19412
> > + - microchip,pac1942
> > + - microchip,pac19422
> > + - microchip,pac1943
> > + - microchip,pac1944
> > + - microchip,pac1951
> > + - microchip,pac19512
> > + - microchip,pac1952
> > + - microchip,pac19522
> > + - microchip,pac1953
> > + - microchip,pac1954
> > +
> > + reg:
> > + maxItems: 1
> > +
> > + vdd-supply: true
> > +
> > + "#address-cells":
> > + const: 1
> > +
> > + "#size-cells":
> > + const: 0
> > +
> > + interrupts:
> > + maxItems: 2
> > +
> > + interrupt-names:
>
> Needs minItems: 1 if we want to allow a single named interrupt.
>
the driver as it is right now it doesn't support any interrupt. I was
thinking to add them here, just in case there will be a request to be
added later.
> > + description:
> > + alert1 indicates a HIGH or LOW limit was exceeded.
> > + alert2 indicates a THERM limit was exceeded.
> > + items:
> > + - const: alert1
> > + - const: alert2
> > +
>
> I am having deja vu. I just commented on an identical interrupts
> binding
> in a different series [1]. In this case though, alert1 and alert2 are
> the actual pin names, so that is fine. But each pin can be programmed
> to indicate lots of different things, so drop descriptions or change
> them to describe the pin, not an arbitrary function. I don't even
> see THERM in the datasheet, so I'm guessing that was just a copy/
> paste from something else anyway.
>
sorry here, I will fix it.
> [1]:
> https://lore.kernel.org/linux-iio/0f68e3f9-cba5-4df3-8e56-2cccbccf35ce@baylibre.com/
>
>
> ---
>
> Even if the driver doesn't use them (yet), we could consider adding
> gpio-controller and #gpio-cells properties since these chips have
> pins
> that can operate as GPIOs.
>
> And we could add a powerdown-gpios property for the /PWRDN pin.
>
> We want to try to make the bindings as complete as possible, if we
> can [2].
>
> [2]:
> https://docs.kernel.org/devicetree/bindings/writing-bindings.html
>
> > +patternProperties:
> > + "^channel@[1-4]+$":
>
> Drop the +. Only 1 to 4 are allowed, not 11, 111, etc.
>
> Also, we could further restrict things based on the actual number of
> channels on a chip like this:
>
> allOf:
> - if:
> properties:
> compatible:
> pattern: "^pac19[45]1"
> then:
> properties:
> channel@1:
> reg:
> items:
> maximum: 1
> patternProperties:
> ^channel@[2-4]$": false
> - if:
> properties:
> compatible:
> pattern: "^pac19[45]2"
> then:
> patternProperties:
> ^channel@[1-2]$":
> reg:
> items:
> maximum: 2
> patternProperties:
> ^channel@[3-4]$": false
> - if:
> properties:
> compatible:
> pattern: "^pac19[45]3"
> then:
> patternProperties:
> ^channel@[1-3]$":
> reg:
> items:
> maximum: 3
> properties:
> channel@4: false
>
>
>
> > + type: object
> > + $ref: adc.yaml
> > + description:
> > + Represents the external channels which are connected to the
> > ADC.
> > +
> > + properties:
> > + reg:
> > + items:
> > + minimum: 1
> > + maximum: 4
> > +
> > + shunt-resistor-micro-ohms:
> > + description:
> > + Value in micro Ohms of the shunt resistor connected
> > between
> > + the SENSE+ and SENSE- inputs, across which the current
> > is measured.
> > + Value is needed to compute the scaling of the measured
> > current.
> > +
> > + microchip,vbus-half-range:
> > + $ref: /schemas/types.yaml#/definitions/flag
> > + description: |
> > + In order to increase measurement resolution and keeping
> > the same
> > + number the of bits the device has a configurable VBUS
> > full range scale
> > + (FSR). The range should be set by hardware design and it
> > should not be
> > + changed during runtime. The bipolar capability for VBUS
> > enables
> > + accurate offset measurement and correction.
> > + The VBUS could be configured into the following full
> > scale range:
> > + - VBUS has unipolar 0V to 32V FSR (default) for
> > PAC195X or 0V to 9V
> > + (default) for PAC194X.
> > + - VBUS has bipolar -32V to 32V FSR for PAC195X or -9V
> > to 9V for
> > + PAC194X. The actual range is limited to about -200
> > mV due to the
> > + impact of the ESD structures.
> > + - VBUS has bipolar -16V to 16V FSR for PAC195X or -
> > 4.5V to 4.5V for
> > + PAC194X. The actual range is limited to about -200
> > mV due to the
> > + impact of the ESD structures.
> > +
> > + microchip,vbus-bipolar:
> > + $ref: /schemas/types.yaml#/definitions/flag
> > + description:
> > + If provided, the channel is to be used in bipolar mode.
> > The
> > + actual range is limited to about -200 mV due to the
> > impact of the ESD
> > + structures.
> > +
>
> Using Jonathan's suggestion from v2 to just have a single property
> with 3 different
> ranges to chose from seems simpler that this. It would only require
> one property
> and would be self-documenting. The description could be shortened to
> just a couple
> of lines.
I was thinking to add the range for this property, but it looks (for me
at least) more complicated from the checking point of view. The driver
is supporting two family of devices that has, each, 3 different voltage
range as an input.
>
> Otherwise, we also need to add:
>
> - if:
> required:
> microchip,vbus-half-range
> then:
> required:
> microchip,vbus-bipolar
>
> to validate that that there are only 3 possibilities.
Those properties was just an alternative solution to the range. If we
agree that this is the way forward, I will add the check.
>
> Also, swapping the word order to range-half would be more consistent
> with
> the existing adi,range-double property that serves a similar purpose.
>
> Same applies to vsense.
>
> > + microchip,vsense-half-range:
> > + $ref: /schemas/types.yaml#/definitions/flag
> > + description: |
> > + In order to decrease the power dissipation on the shunt
> > resistor and
> > + in the same time to increase measurement resolution by
> > keeping the
> > + same number the of bits the device has a configurable
> > VSENSE full
> > + range scale (FSR). The range should be set 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 0V to 100 mV FSR (default)
> > + - VSENSE has bipolar -100 mV to 100 mV FSR
> > + - VSENSE has bipolar -50 mV to 50 mV FSR
> > +
> > + microchip,vsense-bipolar:
> > + $ref: /schemas/types.yaml#/definitions/flag
> > + description:
> > + If provided, the channel is to be used in bipolar mode.
> > +
> > + 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.
> > This allows the
> > + accumulator to be used as a coulomb counter. For either
> > VSENSE or
> > + VBUS, many samples may be accumulated on chip and the
> > result collected
> > + by the host and divided by the accumulator counter count
> > value to
> > + yield an average value with a very long integration time
> > to reduce
> > + noise. This feature is also very useful for system
> > calibration,
> > + allowing many averages to be accumulated for fast
> > averaging/noise
> > + reduction.
> > + This functionality needs to be setup once and must not
> > be changed
> > + during the runtime,
>
> Why not? The datasheet says there is a REFRESH command to allow
> changing it
> at runtime.
Yes sure, from the data sheet point of view it is possible to change
the accumulation mode during the runtime. From the customer point of
view once the channel has a certain functionality the operation mode
will not be changed. E.g. once a channel is used as a coulomb counter
nobody will change this operation mode in order not to loose any
charging or discharging current for that channel. Also being only one
hardware register per channel it will be hard to have enabled/available
into the sysfs all the functionality and to "guess" the one that is
working at a point in time.
>
> > just in case the user wants to measure the charge
> > + or the energy consumed from board power up till the user
> > has control
> > + or during a reboot of the system.
> > + The Hardware Accumulator could be configured to
> > accumulate
> > + VPOWER, VSENSE or VBUS
> > + <0> - Accumulator accumulates VPOWER (default)
> > + <1> - Accumulator accumulates VSENSE
> > + <2> - Accumulator accumulates VBUS
> > + maximum: 2
> > + default: 0
> > +
> > + required:
> > + - reg
> > + - shunt-resistor-micro-ohms
> > +
> > + unevaluatedProperties: false
> > +
> > +required:
> > + - compatible
> > + - reg
> > + - vdd-supply
> > + - "#address-cells"
> > + - "#size-cells"
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > + - |
> > + i2c {
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > +
> > + power-monitor@10 {
> > + compatible = "microchip,pac1954";
> > + reg = <0x10>;
> > + vdd-supply = <&vdd>;
> > +
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > +
> > + channel@1 {
> > + reg = <0x1>;
> > + shunt-resistor-micro-ohms = <24900>;
> > + label = "CPU";
> > + microchip,vsense-half-range;
> > + microchip,vsense-bipolar;
> > + };
>
> Seems odd to leave a channel unconfigured since the shunt resistor
> value is required and there is a 3 channel version of the chip that
> could be used if only 3 channels were wired up.
Yes, indeed. The reason I was given this example was because, sometimes
the client will use a chip with more channels that are needed in order
to develop some other functions later but on the same hardware. E.g. a
board that could monitor a battery charge/discharge current, but the
same hardware/board on a more expensive product could also support a
second battery that has a second charging monitoring channel.
The parts are pin compatible and sometime the clients will buy the part
with more channels and use only part of those channels. In order to
save some power each channel could be enabled or disabled.
>
> > +
> > + channel@3 {
> > + reg = <0x3>;
> > + shunt-resistor-micro-ohms = <75000>;
> > + label = "MEM";
> > + microchip,vbus-half-range;
> > + microchip,vbus-bipolar;
> > + microchip,vsense-half-range;
> > + };
> > +
> > + channel@4 {
> > + reg = <0x4>;
> > + shunt-resistor-micro-ohms = <100000>;
> > + label = "NET";
> > + microchip,vbus-bipolar;
> > + };
> > + };
> > + };
> > +
> > +...
On 6/10/25 9:46 AM, Marius.Cristea@microchip.com wrote: > Hi David, > > Thank you for the feedback. Please see my comments below... > ... >>> + interrupts: >>> + maxItems: 2 >>> + >>> + interrupt-names: >> >> Needs minItems: 1 if we want to allow a single named interrupt. >> > the driver as it is right now it doesn't support any interrupt. I was > thinking to add them here, just in case there will be a request to be > added later. > Making the bindings complete even if the driver isn't using it yet is the right thing to do. :-) I meant allowing just a single interrupt wired up though. So it doesn't matter how the driver would handle it. > >>> + description: >>> + alert1 indicates a HIGH or LOW limit was exceeded. >>> + alert2 indicates a THERM limit was exceeded. >>> + items: >>> + - const: alert1 >>> + - const: alert2 >>> + >> ... >>> + >>> + microchip,vbus-half-range: >>> + $ref: /schemas/types.yaml#/definitions/flag >>> + description: | >>> + In order to increase measurement resolution and keeping >>> the same >>> + number the of bits the device has a configurable VBUS >>> full range scale >>> + (FSR). The range should be set by hardware design and it >>> should not be >>> + changed during runtime. The bipolar capability for VBUS >>> enables >>> + accurate offset measurement and correction. >>> + The VBUS could be configured into the following full >>> scale range: >>> + - VBUS has unipolar 0V to 32V FSR (default) for >>> PAC195X or 0V to 9V >>> + (default) for PAC194X. >>> + - VBUS has bipolar -32V to 32V FSR for PAC195X or -9V >>> to 9V for >>> + PAC194X. The actual range is limited to about -200 >>> mV due to the >>> + impact of the ESD structures. >>> + - VBUS has bipolar -16V to 16V FSR for PAC195X or - >>> 4.5V to 4.5V for >>> + PAC194X. The actual range is limited to about -200 >>> mV due to the >>> + impact of the ESD structures. >>> + >>> + microchip,vbus-bipolar: >>> + $ref: /schemas/types.yaml#/definitions/flag >>> + description: >>> + If provided, the channel is to be used in bipolar mode. >>> The >>> + actual range is limited to about -200 mV due to the >>> impact of the ESD >>> + structures. >>> + >> >> Using Jonathan's suggestion from v2 to just have a single property >> with 3 different >> ranges to chose from seems simpler that this. It would only require >> one property >> and would be self-documenting. The description could be shortened to >> just a couple >> of lines. > > I was thinking to add the range for this property, but it looks (for me > at least) more complicated from the checking point of view. The driver > is supporting two family of devices that has, each, 3 different voltage > range as an input. > Usually, having a consistent binding for the same thing among similar devices is more important than how easy it is to implement in the driver. Since this seems to be a common pattern, we could probably justify an iio_property_match_ranges() helper function that would simplify the implementation in drivers that would need to use such a property. Then in each driver it would just be a matter of making a static const array lookup table of ranges for each device and calling the helper function.
On Tue, 2025-06-10 at 10:22 -0500, David Lechner wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you > know the content is safe > > On 6/10/25 9:46 AM, Marius.Cristea@microchip.com wrote: > > Hi David, > > > > Thank you for the feedback. Please see my comments below... > > > > ... > ... > > > > > + > > > > + microchip,vbus-half-range: > > > > + $ref: /schemas/types.yaml#/definitions/flag > > > > + description: | > > > > + In order to increase measurement resolution and > > > > keeping > > > > the same > > > > + number the of bits the device has a configurable > > > > VBUS > > > > full range scale > > > > + (FSR). The range should be set by hardware design > > > > and it > > > > should not be > > > > + changed during runtime. The bipolar capability for > > > > VBUS > > > > enables > > > > + accurate offset measurement and correction. > > > > + The VBUS could be configured into the following full > > > > scale range: > > > > + - VBUS has unipolar 0V to 32V FSR (default) for > > > > PAC195X or 0V to 9V > > > > + (default) for PAC194X. > > > > + - VBUS has bipolar -32V to 32V FSR for PAC195X or > > > > -9V > > > > to 9V for > > > > + PAC194X. The actual range is limited to about - > > > > 200 > > > > mV due to the > > > > + impact of the ESD structures. > > > > + - VBUS has bipolar -16V to 16V FSR for PAC195X or > > > > - > > > > 4.5V to 4.5V for > > > > + PAC194X. The actual range is limited to about - > > > > 200 > > > > mV due to the > > > > + impact of the ESD structures. > > > > + > > > > + microchip,vbus-bipolar: > > > > + $ref: /schemas/types.yaml#/definitions/flag > > > > + description: > > > > + If provided, the channel is to be used in bipolar > > > > mode. > > > > The > > > > + actual range is limited to about -200 mV due to the > > > > impact of the ESD > > > > + structures. > > > > + > > > > > > Using Jonathan's suggestion from v2 to just have a single > > > property > > > with 3 different > > > ranges to chose from seems simpler that this. It would only > > > require > > > one property > > > and would be self-documenting. The description could be shortened > > > to > > > just a couple > > > of lines. > > > > I was thinking to add the range for this property, but it looks > > (for me > > at least) more complicated from the checking point of view. The > > driver > > is supporting two family of devices that has, each, 3 different > > voltage > > range as an input. > > > > Usually, having a consistent binding for the same thing among similar > devices is more important than how easy it is to implement in the > driver. > > Since this seems to be a common pattern, we could probably justify an > iio_property_match_ranges() helper function that would simplify the > implementation in drivers that would need to use such a property. > Then > in each driver it would just be a matter of making a static const > array > lookup table of ranges for each device and calling the helper > function. Sorry for not explaining very well. I have implemented the range into the driver and I was working well, but I had issues defining the range into the device binding and the checker was failing. That was the reason that I've dropped the range from the binding. Also I had some issues enforcing a certain "available" ranges for a particular part into the binding.
On 6/10/25 11:04 AM, Marius.Cristea@microchip.com wrote:
> On Tue, 2025-06-10 at 10:22 -0500, David Lechner wrote:
>> EXTERNAL EMAIL: Do not click links or open attachments unless you
>> know the content is safe
>>
>> On 6/10/25 9:46 AM, Marius.Cristea@microchip.com wrote:
>>> Hi David,
>>>
>>> Thank you for the feedback. Please see my comments below...
>>>
...
>>>>
>>>> Using Jonathan's suggestion from v2 to just have a single
>>>> property
>>>> with 3 different
>>>> ranges to chose from seems simpler that this. It would only
>>>> require
>>>> one property
>>>> and would be self-documenting. The description could be shortened
>>>> to
>>>> just a couple
>>>> of lines.
>>>
>>> I was thinking to add the range for this property, but it looks
>>> (for me
>>> at least) more complicated from the checking point of view. The
>>> driver
>>> is supporting two family of devices that has, each, 3 different
>>> voltage
>>> range as an input.
>>>
>>
>> Usually, having a consistent binding for the same thing among similar
>> devices is more important than how easy it is to implement in the
>> driver.
>>
>> Since this seems to be a common pattern, we could probably justify an
>> iio_property_match_ranges() helper function that would simplify the
>> implementation in drivers that would need to use such a property.
>> Then
>> in each driver it would just be a matter of making a static const
>> array
>> lookup table of ranges for each device and calling the helper
>> function.
>
> Sorry for not explaining very well. I have implemented the range into
> the driver and I was working well, but I had issues defining the range
> into the device binding and the checker was failing. That was the
> reason that I've dropped the range from the binding. Also I had some
> issues enforcing a certain "available" ranges for a particular part
> into the binding.
What did you try?
The usual way is to define all possibilities and then limit it by compatible.
I think something like this should work:
patternProperties:
"^channel@[1-4]$":
properties:
microchip,input-range-microvolt:
items:
- enum: [-32000000, -16000000, -9000000, -4500000, 0]
- enum: [4500000, 9000000, 16000000, 32000000]
allOf:
- if:
properties:
compatible:
pattern: "^microchip,pac194"
then:
patternProperties:
"^channel@[1-4]$":
properties:
microchip,input-range-microvolt:
oneOf:
- items:
- const: 0
- const: 9000000
- items:
- const: -9000000
- const: 9000000
- items:
- const: -4500000
- const: 4500000
default:
items:
- const: 0
- const: 9000000
- if:
properties:
compatible:
pattern: "^microchip,pac195"
then:
patternProperties:
"^channel@[1-4]$":
properties:
microchip,input-range-microvolt:
oneOf:
- items:
- const: 0
- const: 32000000
- items:
- const: -32000000
- const: 32000000
- items:
- const: -16000000
- const: 16000000
default:
items:
- const: 0
- const: 32000000
Hi David, Thanks for the suggestion. //Marius On Tue, 2025-06-10 at 11:35 -0500, David Lechner wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you > know the content is safe > > On 6/10/25 11:04 AM, Marius.Cristea@microchip.com wrote: > > On Tue, 2025-06-10 at 10:22 -0500, David Lechner wrote: > > > EXTERNAL EMAIL: Do not click links or open attachments unless you > > > know the content is safe > > > > > > On 6/10/25 9:46 AM, Marius.Cristea@microchip.com wrote: > > > > Hi David, > > > > > > > > Thank you for the feedback. Please see my comments below... > > > > > > ... > > > > > > > > > > > Using Jonathan's suggestion from v2 to just have a single > > > > > property > > > > > with 3 different > > > > > ranges to chose from seems simpler that this. It would only > > > > > require > > > > > one property > > > > > and would be self-documenting. The description could be > > > > > shortened > > > > > to > > > > > just a couple > > > > > of lines. > > > > > > > > I was thinking to add the range for this property, but it looks > > > > (for me > > > > at least) more complicated from the checking point of view. The > > > > driver > > > > is supporting two family of devices that has, each, 3 different > > > > voltage > > > > range as an input. > > > > > > > > > > Usually, having a consistent binding for the same thing among > > > similar > > > devices is more important than how easy it is to implement in the > > > driver. > > > > > > Since this seems to be a common pattern, we could probably > > > justify an > > > iio_property_match_ranges() helper function that would simplify > > > the > > > implementation in drivers that would need to use such a property. > > > Then > > > in each driver it would just be a matter of making a static const > > > array > > > lookup table of ranges for each device and calling the helper > > > function. > > > > Sorry for not explaining very well. I have implemented the range > > into > > the driver and I was working well, but I had issues defining the > > range > > into the device binding and the checker was failing. That was the > > reason that I've dropped the range from the binding. Also I had > > some > > issues enforcing a certain "available" ranges for a particular part > > into the binding. > > What did you try? > > The usual way is to define all possibilities and then limit it by > compatible. > I think something like this should work: > > patternProperties: > "^channel@[1-4]$": > properties: > microchip,input-range-microvolt: > items: > - enum: [-32000000, -16000000, -9000000, -4500000, 0] > - enum: [4500000, 9000000, 16000000, 32000000] > > > allOf: > - if: > properties: > compatible: > pattern: "^microchip,pac194" > then: > patternProperties: > "^channel@[1-4]$": > properties: > microchip,input-range-microvolt: > oneOf: > - items: > - const: 0 > - const: 9000000 > - items: > - const: -9000000 > - const: 9000000 > - items: > - const: -4500000 > - const: 4500000 > default: > items: > - const: 0 > - const: 9000000 > - if: > properties: > compatible: > pattern: "^microchip,pac195" > then: > patternProperties: > "^channel@[1-4]$": > properties: > microchip,input-range-microvolt: > oneOf: > - items: > - const: 0 > - const: 32000000 > - items: > - const: -32000000 > - const: 32000000 > - items: > - const: -16000000 > - const: 16000000 > default: > items: > - const: 0 > - const: 32000000 >
© 2016 - 2025 Red Hat, Inc.