From: Marius Cristea <[email protected]>
This is the device tree schema for iio driver for
Microchip PAC193X series of Power Monitors with Accumulator.
Signed-off-by: Marius Cristea <[email protected]>
---
.../bindings/iio/adc/microchip,pac1934.yaml | 146 ++++++++++++++++++
1 file changed, 146 insertions(+)
create mode 100644 Documentation/devicetree/bindings/iio/adc/microchip,pac1934.yaml
diff --git a/Documentation/devicetree/bindings/iio/adc/microchip,pac1934.yaml b/Documentation/devicetree/bindings/iio/adc/microchip,pac1934.yaml
new file mode 100644
index 000000000000..837053ed8a71
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/microchip,pac1934.yaml
@@ -0,0 +1,146 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/adc/microchip,pac1934.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Microchip PAC1934 Power Monitors with Accumulator
+
+maintainers:
+ - Marius Cristea <[email protected]>
+
+description: |
+ Bindings for the Microchip family of Power Monitors with Accumulator.
+ The datasheet for PAC1931, PAC1932, PAC1933 and PAC1934 can be found here:
+ https://ww1.microchip.com/downloads/aemDocuments/documents/OTH/ProductDocuments/DataSheets/PAC1931-Family-Data-Sheet-DS20005850E.pdf
+
+properties:
+ compatible:
+ enum:
+ - microchip,pac1931
+ - microchip,pac1932
+ - microchip,pac1933
+ - microchip,pac1934
+
+ reg:
+ maxItems: 1
+
+ "#address-cells":
+ const: 1
+
+ "#size-cells":
+ const: 0
+
+ interrupts:
+ description: IRQ line of the ADC
+ maxItems: 1
+
+ drive-open-drain:
+ description: The IRQ signal is configured as open-drain.
+ type: boolean
+ maxItems: 1
+
+ microchip,slow-io:
+ type: boolean
+ description: |
+ A GPIO used to trigger a change is sampling rate (lowering the chip power consumption).
+ In default mode, if this pin is forced high, sampling rate is forced to eight
+ samples/second. When it is forced low, the sampling rate is 1024 samples/second unless
+ a different sample rate has been programmed.
+
+required:
+ - compatible
+ - reg
+ - "#address-cells"
+ - "#size-cells"
+
+patternProperties:
+ "^channel@[1-4]+$":
+ type: object
+ $ref: adc.yaml
+ description: Represents the external channels which are connected to the ADC.
+
+ properties:
+ reg:
+ description: |
+ The channel number.
+ It can have up to 4 channels, numbered from 1 to 4.
+ 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.
+
+ label:
+ $ref: /schemas/types.yaml#/definitions/string
+ description: Name of the monitored power rail.
+
+ bipolar:
+ description: Whether the channel is bi-directional.
+ type: boolean
+
+ required:
+ - reg
+ - shunt-resistor-micro-ohms
+
+ additionalProperties: false
+
+allOf:
+ - if:
+ required:
+ - interrupts
+ then:
+ required:
+ - drive-open-drain
+ else:
+ properties:
+ drive-open-drain: false
+
+additionalProperties: false
+
+examples:
+ - |
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ pac193x: pac193x@10 {
+ compatible = "microchip,pac1934";
+ reg = <0x10>;
+
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ channel@1 {
+ reg = <0x1>;
+ shunt-resistor-micro-ohms = <24900000>;
+ label = "CPU";
+ };
+
+ channel@2 {
+ reg = <0x2>;
+ shunt-resistor-micro-ohms = <49900000>;
+ label = "GPU";
+ };
+
+ channel@3 {
+ reg = <0x3>;
+ shunt-resistor-micro-ohms = <75000000>;
+ label = "MEM";
+ bipolar;
+ };
+
+ channel@4 {
+ reg = <0x4>;
+ shunt-resistor-micro-ohms = <100000000>;
+ label = "NET";
+ bipolar;
+ };
+ };
+ };
+
+...
--
2.34.1
Hey Marius,
On Wed, Oct 25, 2023 at 04:44:03PM +0300, [email protected] wrote:
> From: Marius Cristea <[email protected]>
>
> This is the device tree schema for iio driver for
> Microchip PAC193X series of Power Monitors with Accumulator.
>
> Signed-off-by: Marius Cristea <[email protected]>
> ---
> .../bindings/iio/adc/microchip,pac1934.yaml | 146 ++++++++++++++++++
> 1 file changed, 146 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/iio/adc/microchip,pac1934.yaml
>
> diff --git a/Documentation/devicetree/bindings/iio/adc/microchip,pac1934.yaml b/Documentation/devicetree/bindings/iio/adc/microchip,pac1934.yaml
> new file mode 100644
> index 000000000000..837053ed8a71
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/microchip,pac1934.yaml
> @@ -0,0 +1,146 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/adc/microchip,pac1934.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Microchip PAC1934 Power Monitors with Accumulator
> +
> +maintainers:
> + - Marius Cristea <[email protected]>
> +
> +description: |
> + Bindings for the Microchip family of Power Monitors with Accumulator.
> + The datasheet for PAC1931, PAC1932, PAC1933 and PAC1934 can be found here:
> + https://ww1.microchip.com/downloads/aemDocuments/documents/OTH/ProductDocuments/DataSheets/PAC1931-Family-Data-Sheet-DS20005850E.pdf
> +
> +properties:
> + compatible:
> + enum:
> + - microchip,pac1931
> + - microchip,pac1932
> + - microchip,pac1933
> + - microchip,pac1934
> +
> + reg:
> + maxItems: 1
> +
> + "#address-cells":
> + const: 1
> +
> + "#size-cells":
> + const: 0
> +
> + interrupts:
> + description: IRQ line of the ADC
> + maxItems: 1
> +
> + drive-open-drain:
> + description: The IRQ signal is configured as open-drain.
> + type: boolean
> + maxItems: 1
> +
> + microchip,slow-io:
> + type: boolean
> + description: |
> + A GPIO used to trigger a change is sampling rate (lowering the chip power consumption).
> + In default mode, if this pin is forced high, sampling rate is forced to eight
> + samples/second. When it is forced low, the sampling rate is 1024 samples/second unless
> + a different sample rate has been programmed.
This description doesn't really make sense to me - if a GPIO is used to
drive the pin low or high, why do we need a property? A DT property
implies that this is a static configuration depending on the board, but
reading the description this seems to be something that can be toggled
at runtime.
I do note though, that this GPIO is not documented in the binding, so I
suppose what really needs to happen here is document the gpio so that
the driver can determine at runtime what state this pin is in?
Also, you say "In default mode", but don't mention what the non-default
mode is. What happens in the other mode?
Cheers,
Conor.
Hi Conor,
On Wed, 2023-10-25 at 16:08 +0100, Conor Dooley wrote:
> Hey Marius,
>
> On Wed, Oct 25, 2023 at 04:44:03PM +0300,
> [email protected] wrote:
> > From: Marius Cristea <[email protected]>
> >
> > This is the device tree schema for iio driver for
> > Microchip PAC193X series of Power Monitors with Accumulator.
> >
> > Signed-off-by: Marius Cristea <[email protected]>
> > ---
> > .../bindings/iio/adc/microchip,pac1934.yaml | 146
> > ++++++++++++++++++
> > 1 file changed, 146 insertions(+)
> > create mode 100644
> > Documentation/devicetree/bindings/iio/adc/microchip,pac1934.yaml
> >
> > diff --git
> > a/Documentation/devicetree/bindings/iio/adc/microchip,pac1934.yaml
> > b/Documentation/devicetree/bindings/iio/adc/microchip,pac1934.yaml
> > new file mode 100644
> > index 000000000000..837053ed8a71
> > --- /dev/null
> > +++
> > b/Documentation/devicetree/bindings/iio/adc/microchip,pac1934.yaml
> > @@ -0,0 +1,146 @@
> > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/iio/adc/microchip,pac1934.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Microchip PAC1934 Power Monitors with Accumulator
> > +
> > +maintainers:
> > + - Marius Cristea <[email protected]>
> > +
> > +description: |
> > + Bindings for the Microchip family of Power Monitors with
> > Accumulator.
> > + The datasheet for PAC1931, PAC1932, PAC1933 and PAC1934 can be
> > found here:
> > +
> > https://ww1.microchip.com/downloads/aemDocuments/documents/OTH/ProductDocuments/DataSheets/PAC1931-Family-Data-Sheet-DS20005850E.pdf
> > +
> > +properties:
> > + compatible:
> > + enum:
> > + - microchip,pac1931
> > + - microchip,pac1932
> > + - microchip,pac1933
> > + - microchip,pac1934
> > +
> > + reg:
> > + maxItems: 1
> > +
> > + "#address-cells":
> > + const: 1
> > +
> > + "#size-cells":
> > + const: 0
> > +
> > + interrupts:
> > + description: IRQ line of the ADC
> > + maxItems: 1
> > +
> > + drive-open-drain:
> > + description: The IRQ signal is configured as open-drain.
> > + type: boolean
> > + maxItems: 1
> > +
> > + microchip,slow-io:
> > + type: boolean
> > + description: |
> > + A GPIO used to trigger a change is sampling rate (lowering
> > the chip power consumption).
> > + In default mode, if this pin is forced high, sampling rate
> > is forced to eight
> > + samples/second. When it is forced low, the sampling rate is
> > 1024 samples/second unless
> > + a different sample rate has been programmed.
>
> This description doesn't really make sense to me - if a GPIO is used
> to
> drive the pin low or high, why do we need a property? A DT property
> implies that this is a static configuration depending on the board,
> but
> reading the description this seems to be something that can be
> toggled
> at runtime.
> I do note though, that this GPIO is not documented in the binding, so
> I
> suppose what really needs to happen here is document the gpio so that
> the driver can determine at runtime what state this pin is in?
>
> Also, you say "In default mode", but don't mention what the non-
> default
> mode is. What happens in the other mode?
>
> Cheers,
> Conor.
This is a "double function" pin. On the PAC193x there is the SLOW/ALERT
pin. At runtime this pin could be configured as an input to the PAC and
the functionality will be "SLOW" that means if it is forced high, the
PAC will work in low power mode by changing the sample rate to 8 SPS.
If it's forced low the PAC will work at it's full sample rate.
"SLOW" is the default function of the pin but it may be programmed to
function as ALERT pin (Open Collector when functioning as ALERT,
requires pull-up resistor to VDD I/O). This time the pin will be set as
output from PAC (ALERT functionality) to trigger an interrupt to the
system (this is covered by the interrupts and drive-open-drain).
The system could work fine without this pin. The driver doesn't use
interrupt at this time, but it could be extended.
Thanks,
Marius
On Thu, Oct 26, 2023 at 03:23:46PM +0000, [email protected] wrote:
> Hi Conor,
>
> On Wed, 2023-10-25 at 16:08 +0100, Conor Dooley wrote:
> > Hey Marius,
> >
> > On Wed, Oct 25, 2023 at 04:44:03PM +0300,
> > [email protected]?wrote:
> > > From: Marius Cristea <[email protected]>
> > >
> > > This is the device tree schema for iio driver for
> > > Microchip PAC193X series of Power Monitors with Accumulator.
> > >
> > > Signed-off-by: Marius Cristea <[email protected]>
> > > ---
> > > ?.../bindings/iio/adc/microchip,pac1934.yaml?? | 146
> > > ++++++++++++++++++
> > > ?1 file changed, 146 insertions(+)
> > > ?create mode 100644
> > > Documentation/devicetree/bindings/iio/adc/microchip,pac1934.yaml
> > >
> > > diff --git
> > > a/Documentation/devicetree/bindings/iio/adc/microchip,pac1934.yaml
> > > b/Documentation/devicetree/bindings/iio/adc/microchip,pac1934.yaml
> > > new file mode 100644
> > > index 000000000000..837053ed8a71
> > > --- /dev/null
> > > +++
> > > b/Documentation/devicetree/bindings/iio/adc/microchip,pac1934.yaml
> > > @@ -0,0 +1,146 @@
> > > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/iio/adc/microchip,pac1934.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: Microchip PAC1934 Power Monitors with Accumulator
> > > +
> > > +maintainers:
> > > +? - Marius Cristea <[email protected]>
> > > +
> > > +description: |
> > > +? Bindings for the Microchip family of Power Monitors with
> > > Accumulator.
> > > +? The datasheet for PAC1931, PAC1932, PAC1933 and PAC1934 can be
> > > found here:
> > > +???
> > > https://ww1.microchip.com/downloads/aemDocuments/documents/OTH/ProductDocuments/DataSheets/PAC1931-Family-Data-Sheet-DS20005850E.pdf
> > > +
> > > +properties:
> > > +? compatible:
> > > +??? enum:
> > > +????? - microchip,pac1931
> > > +????? - microchip,pac1932
> > > +????? - microchip,pac1933
> > > +????? - microchip,pac1934
> > > +
> > > +? reg:
> > > +??? maxItems: 1
> > > +
> > > +? "#address-cells":
> > > +??? const: 1
> > > +
> > > +? "#size-cells":
> > > +??? const: 0
> > > +
> > > +? interrupts:
> > > +??? description: IRQ line of the ADC
> > > +??? maxItems: 1
> > > +
> > > +? drive-open-drain:
> > > +??? description: The IRQ signal is configured as open-drain.
> > > +??? type: boolean
> > > +??? maxItems: 1
> > > +
> > > +? microchip,slow-io:
> > > +??? type: boolean
> > > +??? description: |
> > > +????? A GPIO used to trigger a change is sampling rate (lowering
> > > the chip power consumption).
> > > +????? In default mode, if this pin is forced high, sampling rate
> > > is forced to eight
> > > +????? samples/second. When it is forced low, the sampling rate is
> > > 1024 samples/second unless
> > > +????? a different sample rate has been programmed.
> >
> > This description doesn't really make sense to me - if a GPIO is used
> > to
> > drive the pin low or high, why do we need a property? A DT property
> > implies that this is a static configuration depending on the board,
> > but
> > reading the description this seems to be something that can be
> > toggled
> > at runtime.
> > I do note though, that this GPIO is not documented in the binding, so
> > I
> > suppose what really needs to happen here is document the gpio so that
> > the driver can determine at runtime what state this pin is in?
> >
> > Also, you say "In default mode", but don't mention what the non-
> > default
> > mode is. What happens in the other mode?
> This is a "double function" pin. On the PAC193x there is the SLOW/ALERT
> pin. At runtime this pin could be configured as an input to the PAC and
> the functionality will be "SLOW" that means if it is forced high, the
> PAC will work in low power mode by changing the sample rate to 8 SPS.
> If it's forced low the PAC will work at it's full sample rate.
Since this is a runtime thing, it doesn't make sense to have a property
that is set at dts creation time that decides what mode the pin is in.
> "SLOW" is the default function of the pin but it may be programmed to
> function as ALERT pin (Open Collector when functioning as ALERT,
> requires pull-up resistor to VDD I/O). This time the pin will be set as
> output from PAC (ALERT functionality) to trigger an interrupt to the
> system (this is covered by the interrupts and drive-open-drain).
Hmm, at the risk of getting out of my depth with what the GPIO subsystem
is capable of doing, I would expect to see something like
sampling-rate-gpios:
description:
<what you have above>
maxItems: 1
Which would allow the driver to either drive this pin via the gpio
subsystem, or to use the interrupt property to use it as an interrupt
instead.
Perhaps Jonathan etc knows better for these sort of dual mode pins.
> The system could work fine without this pin. The driver doesn't use
> interrupt at this time, but it could be extended.
Cheers,
Conor.
A nit, subject: drop second/last, redundant "bindings". The
"dt-bindings" prefix is already stating that these are bindings.
On 25/10/2023 15:44, [email protected] wrote:
> From: Marius Cristea <[email protected]>
>
> This is the device tree schema for iio driver for
> Microchip PAC193X series of Power Monitors with Accumulator.
>
> Signed-off-by: Marius Cristea <[email protected]>
> ---
> .../bindings/iio/adc/microchip,pac1934.yaml | 146 ++++++++++++++++++
> 1 file changed, 146 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/iio/adc/microchip,pac1934.yaml
>
> diff --git a/Documentation/devicetree/bindings/iio/adc/microchip,pac1934.yaml b/Documentation/devicetree/bindings/iio/adc/microchip,pac1934.yaml
> new file mode 100644
> index 000000000000..837053ed8a71
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/microchip,pac1934.yaml
> @@ -0,0 +1,146 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/adc/microchip,pac1934.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Microchip PAC1934 Power Monitors with Accumulator
> +
> +maintainers:
> + - Marius Cristea <[email protected]>
> +
> +description: |
> + Bindings for the Microchip family of Power Monitors with Accumulator.
Drop "Bindings for" and describe instead the hardware.
> + The datasheet for PAC1931, PAC1932, PAC1933 and PAC1934 can be found here:
> + https://ww1.microchip.com/downloads/aemDocuments/documents/OTH/ProductDocuments/DataSheets/PAC1931-Family-Data-Sheet-DS20005850E.pdf
> +
> +properties:
> + compatible:
> + enum:
> + - microchip,pac1931
> + - microchip,pac1932
> + - microchip,pac1933
> + - microchip,pac1934
> +
> + reg:
> + maxItems: 1
> +
> + "#address-cells":
> + const: 1
> +
> + "#size-cells":
> + const: 0
> +
> + interrupts:
> + description: IRQ line of the ADC
Drop, useless.
> + maxItems: 1
> +
> + drive-open-drain:
> + description: The IRQ signal is configured as open-drain.
> + type: boolean
> + maxItems: 1
> +
> + microchip,slow-io:
> + type: boolean
> + description: |
> + A GPIO used to trigger a change is sampling rate (lowering the chip power consumption).
> + In default mode, if this pin is forced high, sampling rate is forced to eight
> + samples/second. When it is forced low, the sampling rate is 1024 samples/second unless
> + a different sample rate has been programmed.
> +
> +required:
> + - compatible
> + - reg
> + - "#address-cells"
> + - "#size-cells"
> +
> +patternProperties:
patternProperties: follow properties:, not required: block. Reorder.
> + "^channel@[1-4]+$":
> + type: object
> + $ref: adc.yaml
> + description: Represents the external channels which are connected to the ADC.
> +
> + properties:
> + reg:
> + description: |
> + The channel number.
> + It can have up to 4 channels, numbered from 1 to 4.
Drop, redundant, comes with adc.yaml.
> + 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.
> +
> + label:
> + $ref: /schemas/types.yaml#/definitions/string
> + description: Name of the monitored power rail.
Drop property, comes with adc.yaml.
> +
> + bipolar:
> + description: Whether the channel is bi-directional.
> + type: boolean
Drop property, comes with adc.yaml.
> +
> + required:
> + - reg
> + - shunt-resistor-micro-ohms
> +
> + additionalProperties: false
unevaluatedProperties: false
> +
> +allOf:
> + - if:
> + required:
> + - interrupts
> + then:
> + required:
> + - drive-open-drain
> + else:
> + properties:
> + drive-open-drain: false
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + i2c {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + pac193x: pac193x@10 {
Node names should be generic. See also an explanation and list of
examples (not exhaustive) in DT specification:
https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation
> + compatible = "microchip,pac1934";
> + reg = <0x10>;
> +
Best regards,
Krzysztof
On Thu, 26 Oct 2023 17:08:07 +0100
Conor Dooley <[email protected]> wrote:
> On Thu, Oct 26, 2023 at 03:23:46PM +0000, [email protected] wrote:
> > Hi Conor,
> >
> > On Wed, 2023-10-25 at 16:08 +0100, Conor Dooley wrote:
> > > Hey Marius,
> > >
> > > On Wed, Oct 25, 2023 at 04:44:03PM +0300,
> > > [email protected] wrote:
> > > > From: Marius Cristea <[email protected]>
> > > >
> > > > This is the device tree schema for iio driver for
> > > > Microchip PAC193X series of Power Monitors with Accumulator.
> > > >
> > > > Signed-off-by: Marius Cristea <[email protected]>
> > > > ---
> > > > .../bindings/iio/adc/microchip,pac1934.yaml | 146
> > > > ++++++++++++++++++
> > > > 1 file changed, 146 insertions(+)
> > > > create mode 100644
> > > > Documentation/devicetree/bindings/iio/adc/microchip,pac1934.yaml
> > > >
> > > > diff --git
> > > > a/Documentation/devicetree/bindings/iio/adc/microchip,pac1934.yaml
> > > > b/Documentation/devicetree/bindings/iio/adc/microchip,pac1934.yaml
> > > > new file mode 100644
> > > > index 000000000000..837053ed8a71
> > > > --- /dev/null
> > > > +++
> > > > b/Documentation/devicetree/bindings/iio/adc/microchip,pac1934.yaml
> > > > @@ -0,0 +1,146 @@
> > > > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > > > +%YAML 1.2
> > > > +---
> > > > +$id: http://devicetree.org/schemas/iio/adc/microchip,pac1934.yaml#
> > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > +
> > > > +title: Microchip PAC1934 Power Monitors with Accumulator
> > > > +
> > > > +maintainers:
> > > > + - Marius Cristea <[email protected]>
> > > > +
> > > > +description: |
> > > > + Bindings for the Microchip family of Power Monitors with
> > > > Accumulator.
> > > > + The datasheet for PAC1931, PAC1932, PAC1933 and PAC1934 can be
> > > > found here:
> > > > +
> > > > https://ww1.microchip.com/downloads/aemDocuments/documents/OTH/ProductDocuments/DataSheets/PAC1931-Family-Data-Sheet-DS20005850E.pdf
> > > > +
> > > > +properties:
> > > > + compatible:
> > > > + enum:
> > > > + - microchip,pac1931
> > > > + - microchip,pac1932
> > > > + - microchip,pac1933
> > > > + - microchip,pac1934
> > > > +
> > > > + reg:
> > > > + maxItems: 1
> > > > +
> > > > + "#address-cells":
> > > > + const: 1
> > > > +
> > > > + "#size-cells":
> > > > + const: 0
> > > > +
> > > > + interrupts:
> > > > + description: IRQ line of the ADC
> > > > + maxItems: 1
> > > > +
> > > > + drive-open-drain:
> > > > + description: The IRQ signal is configured as open-drain.
> > > > + type: boolean
> > > > + maxItems: 1
> > > > +
> > > > + microchip,slow-io:
> > > > + type: boolean
> > > > + description: |
> > > > + A GPIO used to trigger a change is sampling rate (lowering
> > > > the chip power consumption).
> > > > + In default mode, if this pin is forced high, sampling rate
> > > > is forced to eight
> > > > + samples/second. When it is forced low, the sampling rate is
> > > > 1024 samples/second unless
> > > > + a different sample rate has been programmed.
> > >
> > > This description doesn't really make sense to me - if a GPIO is used
> > > to
> > > drive the pin low or high, why do we need a property? A DT property
> > > implies that this is a static configuration depending on the board,
> > > but
> > > reading the description this seems to be something that can be
> > > toggled
> > > at runtime.
> > > I do note though, that this GPIO is not documented in the binding, so
> > > I
> > > suppose what really needs to happen here is document the gpio so that
> > > the driver can determine at runtime what state this pin is in?
> > >
> > > Also, you say "In default mode", but don't mention what the non-
> > > default
> > > mode is. What happens in the other mode?
>
> > This is a "double function" pin. On the PAC193x there is the SLOW/ALERT
> > pin. At runtime this pin could be configured as an input to the PAC and
> > the functionality will be "SLOW" that means if it is forced high, the
> > PAC will work in low power mode by changing the sample rate to 8 SPS.
> > If it's forced low the PAC will work at it's full sample rate.
>
> Since this is a runtime thing, it doesn't make sense to have a property
> that is set at dts creation time that decides what mode the pin is in.
>
> > "SLOW" is the default function of the pin but it may be programmed to
> > function as ALERT pin (Open Collector when functioning as ALERT,
> > requires pull-up resistor to VDD I/O). This time the pin will be set as
> > output from PAC (ALERT functionality) to trigger an interrupt to the
> > system (this is covered by the interrupts and drive-open-drain).
>
> Hmm, at the risk of getting out of my depth with what the GPIO subsystem
> is capable of doing, I would expect to see something like
>
> sampling-rate-gpios:
> description:
> <what you have above>
> maxItems: 1
>
> Which would allow the driver to either drive this pin via the gpio
> subsystem, or to use the interrupt property to use it as an interrupt
> instead.
>
> Perhaps Jonathan etc knows better for these sort of dual mode pins.
Beyond them being a pain? The fun is they may get wired to interrupt
controllers that are also GPIOs or they may not (and the other way around
with them wired to GPIO pins that aren't interrupt pins).
I don't understand the usecase for the SLOW control.
Given it seems software can override the use for SLOW I'd be tempted to
always do that.
Thus making this pin useable only as an optional interrupt.
If someone hard wires it to high or low that is harmless if we aren't
letting it control anything.
>
> > The system could work fine without this pin. The driver doesn't use
> > interrupt at this time, but it could be extended.
>
> Cheers,
> Conor.
Hi
On Fri, 2023-10-27 at 15:26 +0100, Jonathan Cameron wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you
> know the content is safe
>
> On Thu, 26 Oct 2023 17:08:07 +0100
> Conor Dooley <[email protected]> wrote:
>
> > On Thu, Oct 26, 2023 at 03:23:46PM +0000,
> > [email protected] wrote:
> > > Hi Conor,
> > >
> > > On Wed, 2023-10-25 at 16:08 +0100, Conor Dooley wrote:
> > > > Hey Marius,
> > > >
> > > > On Wed, Oct 25, 2023 at 04:44:03PM +0300,
> > > > [email protected] wrote:
> > > > > From: Marius Cristea <[email protected]>
> > > > >
> > > > > This is the device tree schema for iio driver for
> > > > > Microchip PAC193X series of Power Monitors with Accumulator.
> > > > >
> > > > >
......
> > > > > +
> > > > > + reg:
> > > > > + maxItems: 1
> > > > > +
> > > > > + "#address-cells":
> > > > > + const: 1
> > > > > +
> > > > > + "#size-cells":
> > > > > + const: 0
> > > > > +
> > > > > + interrupts:
> > > > > + description: IRQ line of the ADC
> > > > > + maxItems: 1
> > > > > +
> > > > > + drive-open-drain:
> > > > > + description: The IRQ signal is configured as open-drain.
> > > > > + type: boolean
> > > > > + maxItems: 1
> > > > > +
> > > > > + microchip,slow-io:
> > > > > + type: boolean
> > > > > + description: |
> > > > > + A GPIO used to trigger a change is sampling rate
> > > > > (lowering
> > > > > the chip power consumption).
> > > > > + In default mode, if this pin is forced high, sampling
> > > > > rate
> > > > > is forced to eight
> > > > > + samples/second. When it is forced low, the sampling
> > > > > rate is
> > > > > 1024 samples/second unless
> > > > > + a different sample rate has been programmed.
> > > >
> > > > This description doesn't really make sense to me - if a GPIO is
> > > > used
> > > > to
> > > > drive the pin low or high, why do we need a property? A DT
> > > > property
> > > > implies that this is a static configuration depending on the
> > > > board,
> > > > but
> > > > reading the description this seems to be something that can be
> > > > toggled
> > > > at runtime.
> > > > I do note though, that this GPIO is not documented in the
> > > > binding, so
> > > > I
> > > > suppose what really needs to happen here is document the gpio
> > > > so that
> > > > the driver can determine at runtime what state this pin is in?
> > > >
> > > > Also, you say "In default mode", but don't mention what the
> > > > non-
> > > > default
> > > > mode is. What happens in the other mode?
> >
> > > This is a "double function" pin. On the PAC193x there is the
> > > SLOW/ALERT
> > > pin. At runtime this pin could be configured as an input to the
> > > PAC and
> > > the functionality will be "SLOW" that means if it is forced high,
> > > the
> > > PAC will work in low power mode by changing the sample rate to 8
> > > SPS.
> > > If it's forced low the PAC will work at it's full sample rate.
> >
> > Since this is a runtime thing, it doesn't make sense to have a
> > property
> > that is set at dts creation time that decides what mode the pin is
> > in.
> >
> > > "SLOW" is the default function of the pin but it may be
> > > programmed to
> > > function as ALERT pin (Open Collector when functioning as ALERT,
> > > requires pull-up resistor to VDD I/O). This time the pin will be
> > > set as
> > > output from PAC (ALERT functionality) to trigger an interrupt to
> > > the
> > > system (this is covered by the interrupts and drive-open-drain).
> >
> > Hmm, at the risk of getting out of my depth with what the GPIO
> > subsystem
> > is capable of doing, I would expect to see something like
> >
> > sampling-rate-gpios:
> > description:
> > <what you have above>
> > maxItems: 1
> >
> > Which would allow the driver to either drive this pin via the gpio
> > subsystem, or to use the interrupt property to use it as an
> > interrupt
> > instead.
> >
> > Perhaps Jonathan etc knows better for these sort of dual mode pins.
>
> Beyond them being a pain? The fun is they may get wired to interrupt
> controllers that are also GPIOs or they may not (and the other way
> around
> with them wired to GPIO pins that aren't interrupt pins).
>
> I don't understand the usecase for the SLOW control.
> Given it seems software can override the use for SLOW I'd be tempted
> to
> always do that.
> Thus making this pin useable only as an optional interrupt.
>
> If someone hard wires it to high or low that is harmless if we aren't
> letting it control anything.
>
Here I was trying to define/describe 3 possible situations:
- 1) the pin is not used at all, so it doesn't matter if it's connected
somewhere
- 2) the pin is user configured as "interrupt" and it's connected to
the interrupt controller (this case is not supported in the driver
right now)
- 3) the pin is user configured as "SLOW" (this case is not supported
in the driver right now). That means it should be connected to a GPIO
pin. This function (SLOW control) will automatically change the PAC
internal sampling frequency to lower the PAC internal power
consumption. For example, the PAC could be configured to a sample rate
of 1024 samples/s (it will consume maximum current). Using the SLOW
control, the chip will internally change to 8 samples/s but the math
internally will "behave" as the 1024 samples/s but at a much lower
power consumption. It's very useful in case the system wants to lower
power consumption (we still need to measure battery power consumption
even if the system is put into a low power state). PAC internal power
consumption is proportional to the number of channels used and also the
sampling frequency.
> >
> > > The system could work fine without this pin. The driver doesn't
> > > use
> > > interrupt at this time, but it could be extended.
> >
> > Cheers,
> > Conor.
>
Thanks,
Marius
On Tue, 7 Nov 2023 08:55:48 +0000
<[email protected]> wrote:
> Hi
>
> On Fri, 2023-10-27 at 15:26 +0100, Jonathan Cameron wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you
> > know the content is safe
> >
> > On Thu, 26 Oct 2023 17:08:07 +0100
> > Conor Dooley <[email protected]> wrote:
> >
> > > On Thu, Oct 26, 2023 at 03:23:46PM +0000,
> > > [email protected] wrote:
> > > > Hi Conor,
> > > >
> > > > On Wed, 2023-10-25 at 16:08 +0100, Conor Dooley wrote:
> > > > > Hey Marius,
> > > > >
> > > > > On Wed, Oct 25, 2023 at 04:44:03PM +0300,
> > > > > [email protected] wrote:
> > > > > > From: Marius Cristea <[email protected]>
> > > > > >
> > > > > > This is the device tree schema for iio driver for
> > > > > > Microchip PAC193X series of Power Monitors with Accumulator.
> > > > > >
> > > > > >
> ......
> > > > > > +
> > > > > > + reg:
> > > > > > + maxItems: 1
> > > > > > +
> > > > > > + "#address-cells":
> > > > > > + const: 1
> > > > > > +
> > > > > > + "#size-cells":
> > > > > > + const: 0
> > > > > > +
> > > > > > + interrupts:
> > > > > > + description: IRQ line of the ADC
> > > > > > + maxItems: 1
> > > > > > +
> > > > > > + drive-open-drain:
> > > > > > + description: The IRQ signal is configured as open-drain.
> > > > > > + type: boolean
> > > > > > + maxItems: 1
> > > > > > +
> > > > > > + microchip,slow-io:
> > > > > > + type: boolean
> > > > > > + description: |
> > > > > > + A GPIO used to trigger a change is sampling rate
> > > > > > (lowering
> > > > > > the chip power consumption).
> > > > > > + In default mode, if this pin is forced high, sampling
> > > > > > rate
> > > > > > is forced to eight
> > > > > > + samples/second. When it is forced low, the sampling
> > > > > > rate is
> > > > > > 1024 samples/second unless
> > > > > > + a different sample rate has been programmed.
> > > > >
> > > > > This description doesn't really make sense to me - if a GPIO is
> > > > > used
> > > > > to
> > > > > drive the pin low or high, why do we need a property? A DT
> > > > > property
> > > > > implies that this is a static configuration depending on the
> > > > > board,
> > > > > but
> > > > > reading the description this seems to be something that can be
> > > > > toggled
> > > > > at runtime.
> > > > > I do note though, that this GPIO is not documented in the
> > > > > binding, so
> > > > > I
> > > > > suppose what really needs to happen here is document the gpio
> > > > > so that
> > > > > the driver can determine at runtime what state this pin is in?
> > > > >
> > > > > Also, you say "In default mode", but don't mention what the
> > > > > non-
> > > > > default
> > > > > mode is. What happens in the other mode?
> > >
> > > > This is a "double function" pin. On the PAC193x there is the
> > > > SLOW/ALERT
> > > > pin. At runtime this pin could be configured as an input to the
> > > > PAC and
> > > > the functionality will be "SLOW" that means if it is forced high,
> > > > the
> > > > PAC will work in low power mode by changing the sample rate to 8
> > > > SPS.
> > > > If it's forced low the PAC will work at it's full sample rate.
> > >
> > > Since this is a runtime thing, it doesn't make sense to have a
> > > property
> > > that is set at dts creation time that decides what mode the pin is
> > > in.
> > >
> > > > "SLOW" is the default function of the pin but it may be
> > > > programmed to
> > > > function as ALERT pin (Open Collector when functioning as ALERT,
> > > > requires pull-up resistor to VDD I/O). This time the pin will be
> > > > set as
> > > > output from PAC (ALERT functionality) to trigger an interrupt to
> > > > the
> > > > system (this is covered by the interrupts and drive-open-drain).
> > >
> > > Hmm, at the risk of getting out of my depth with what the GPIO
> > > subsystem
> > > is capable of doing, I would expect to see something like
> > >
> > > sampling-rate-gpios:
> > > description:
> > > <what you have above>
> > > maxItems: 1
> > >
> > > Which would allow the driver to either drive this pin via the gpio
> > > subsystem, or to use the interrupt property to use it as an
> > > interrupt
> > > instead.
> > >
> > > Perhaps Jonathan etc knows better for these sort of dual mode pins.
> >
> > Beyond them being a pain? The fun is they may get wired to interrupt
> > controllers that are also GPIOs or they may not (and the other way
> > around
> > with them wired to GPIO pins that aren't interrupt pins).
> >
> > I don't understand the usecase for the SLOW control.
> > Given it seems software can override the use for SLOW I'd be tempted
> > to
> > always do that.
> > Thus making this pin useable only as an optional interrupt.
> >
> > If someone hard wires it to high or low that is harmless if we aren't
> > letting it control anything.
> >
>
> Here I was trying to define/describe 3 possible situations:
> - 1) the pin is not used at all, so it doesn't matter if it's connected
> somewhere
>
> - 2) the pin is user configured as "interrupt" and it's connected to
> the interrupt controller (this case is not supported in the driver
> right now)
>
> - 3) the pin is user configured as "SLOW" (this case is not supported
> in the driver right now). That means it should be connected to a GPIO
> pin. This function (SLOW control) will automatically change the PAC
> internal sampling frequency to lower the PAC internal power
> consumption. For example, the PAC could be configured to a sample rate
> of 1024 samples/s (it will consume maximum current). Using the SLOW
> control, the chip will internally change to 8 samples/s but the math
> internally will "behave" as the 1024 samples/s but at a much lower
> power consumption. It's very useful in case the system wants to lower
> power consumption (we still need to measure battery power consumption
> even if the system is put into a low power state). PAC internal power
> consumption is proportional to the number of channels used and also the
> sampling frequency.
So far so good, but how does 3 differ from just setting the chip to sample
at 8 samples/s, which I believe we can do from software?
Anyhow, for a DT binding, provide both gpio and interrupt as optional
and the driver can make up it's mind on what to do if both are provided.
Jonathan
>
>
>
> > >
> > > > The system could work fine without this pin. The driver doesn't
> > > > use
> > > > interrupt at this time, but it could be extended.
> > >
> > > Cheers,
> > > Conor.
> >
>
> Thanks,
> Marius
On Fri, 10 Nov 2023 16:27:54 +0000
<[email protected]> wrote:
> Hi Jonathan,
>
> On Fri, 2023-10-27 at 15:26 +0100, Jonathan Cameron wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you
> > know the content is safe
> >
> > On Thu, 26 Oct 2023 17:08:07 +0100
> > Conor Dooley <[email protected]> wrote:
> >
> > > On Thu, Oct 26, 2023 at 03:23:46PM +0000,
> > > [email protected] wrote:
> > > > Hi Conor,
> > > >
> > > > On Wed, 2023-10-25 at 16:08 +0100, Conor Dooley wrote:
> > > > > Hey Marius,
> > > > >
> > > > > On Wed, Oct 25, 2023 at 04:44:03PM +0300,
> > > > > [email protected] wrote:
> > > > > > From: Marius Cristea <[email protected]>
> > > > > >
> .................
> > > > > > +$id:
> > > > > > http://devicetree.org/schemas/iio/adc/microchip,pac1934.yaml#
> > > > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > > > +
> > > > > > +title: Microchip PAC1934 Power Monitors with Accumulator
> > > > > > +
> > > > > > +maintainers:
> > > > > > + - Marius Cristea <[email protected]>
> > > > > > +
> > > > > > +description: |
> > > > > > + Bindings for the Microchip family of Power Monitors with
> > > > > > Accumulator.
> > > > > > + The datasheet for PAC1931, PAC1932, PAC1933 and PAC1934
> > > > > > can be
> > > > > > found here:
> > > > > > +
> > > > > > https://ww1.microchip.com/downloads/aemDocuments/documents/OTH/ProductDocuments/DataSheets/PAC1931-Family-Data-Sheet-DS20005850E.pdf
> > > > > > +
> > > > > > +properties:
> > > > > > + compatible:
> > > > > > + enum:
> > > > > > + - microchip,pac1931
> > > > > > + - microchip,pac1932
> > > > > > + - microchip,pac1933
> > > > > > + - microchip,pac1934
> > > > > > +
> > > > > > + reg:
> > > > > > + maxItems: 1
> > > > > > +
> > > > > > + "#address-cells":
> > > > > > + const: 1
> > > > > > +
> > > > > > + "#size-cells":
> > > > > > + const: 0
> > > > > > +
> > > > > > + interrupts:
> > > > > > + description: IRQ line of the ADC
> > > > > > + maxItems: 1
> > > > > > +
> > > > > > + drive-open-drain:
> > > > > > + description: The IRQ signal is configured as open-drain.
> > > > > > + type: boolean
> > > > > > + maxItems: 1
> > > > > > +
> > > > > > + microchip,slow-io:
> > > > > > + type: boolean
> > > > > > + description: |
> > > > > > + A GPIO used to trigger a change is sampling rate
> > > > > > (lowering
> > > > > > the chip power consumption).
> > > > > > + In default mode, if this pin is forced high, sampling
> > > > > > rate
> > > > > > is forced to eight
> > > > > > + samples/second. When it is forced low, the sampling
> > > > > > rate is
> > > > > > 1024 samples/second unless
> > > > > > + a different sample rate has been programmed.
> > > > >
> > > > > This description doesn't really make sense to me - if a GPIO is
> > > > > used
> > > > > to
> > > > > drive the pin low or high, why do we need a property? A DT
> > > > > property
> > > > > implies that this is a static configuration depending on the
> > > > > board,
> > > > > but
> > > > > reading the description this seems to be something that can be
> > > > > toggled
> > > > > at runtime.
> > > > > I do note though, that this GPIO is not documented in the
> > > > > binding, so
> > > > > I
> > > > > suppose what really needs to happen here is document the gpio
> > > > > so that
> > > > > the driver can determine at runtime what state this pin is in?
> > > > >
> > > > > Also, you say "In default mode", but don't mention what the
> > > > > non-
> > > > > default
> > > > > mode is. What happens in the other mode?
> > >
> > > > This is a "double function" pin. On the PAC193x there is the
> > > > SLOW/ALERT
> > > > pin. At runtime this pin could be configured as an input to the
> > > > PAC and
> > > > the functionality will be "SLOW" that means if it is forced high,
> > > > the
> > > > PAC will work in low power mode by changing the sample rate to 8
> > > > SPS.
> > > > If it's forced low the PAC will work at it's full sample rate.
> > >
> > > Since this is a runtime thing, it doesn't make sense to have a
> > > property
> > > that is set at dts creation time that decides what mode the pin is
> > > in.
> > >
> > > > "SLOW" is the default function of the pin but it may be
> > > > programmed to
> > > > function as ALERT pin (Open Collector when functioning as ALERT,
> > > > requires pull-up resistor to VDD I/O). This time the pin will be
> > > > set as
> > > > output from PAC (ALERT functionality) to trigger an interrupt to
> > > > the
> > > > system (this is covered by the interrupts and drive-open-drain).
> > >
> > > Hmm, at the risk of getting out of my depth with what the GPIO
> > > subsystem
> > > is capable of doing, I would expect to see something like
> > >
> > > sampling-rate-gpios:
> > > description:
> > > <what you have above>
> > > maxItems: 1
> > >
> > > Which would allow the driver to either drive this pin via the gpio
> > > subsystem, or to use the interrupt property to use it as an
> > > interrupt
> > > instead.
> > >
> > > Perhaps Jonathan etc knows better for these sort of dual mode pins.
> >
> > Beyond them being a pain? The fun is they may get wired to interrupt
> > controllers that are also GPIOs or they may not (and the other way
> > around
> > with them wired to GPIO pins that aren't interrupt pins).
> >
> > I don't understand the usecase for the SLOW control.
> > Given it seems software can override the use for SLOW I'd be tempted
> > to
> > always do that.
> > Thus making this pin useable only as an optional interrupt.
> >
>
> I was thinking to have something like interrupt or an equivalent to
> "powerdown-gpios", "richtek,mute-enable", "shutdown-gpios", "mute-
> gpios", "gain-gpios". I think the driver should know (from the Device
> Tree) if the pin is routed to a gpio and it could be used as "SLOW"/low
> power.
If we assume the slow pin is always connected to the host CPU then
simply providing the interrupt and the gpio definition is sufficient.
A comment will do to say they are the same physical pin.
Things get messier if we assume that slow control is coming from something
external to the world Linux knows about. That tends to be when we need
things like mute-enable
Jonathan
>
> > If someone hard wires it to high or low that is harmless if we aren't
> > letting it control anything.
> >
> > >
> > > > The system could work fine without this pin. The driver doesn't
> > > > use
> > > > interrupt at this time, but it could be extended.
> > >
> > > Cheers,
> > > Conor.
> >
> Thanks,
> Marius
>