diff mbox series

[v1,1/2] dt-bindings: iio: adc: adding support for PAC194X

Message ID 20240719173855.53261-2-marius.cristea@microchip.com (mailing list archive)
State Changes Requested
Headers show
Series adding support for Microchip PAC194X Power Monitor | expand

Commit Message

marius.cristea@microchip.com July 19, 2024, 5:38 p.m. UTC
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.

Signed-off-by: Marius Cristea <marius.cristea@microchip.com>
---
 .../bindings/iio/adc/microchip,pac1944.yaml   | 168 ++++++++++++++++++
 1 file changed, 168 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/adc/microchip,pac1944.yaml

Comments

Jonathan Cameron July 20, 2024, 5:09 p.m. UTC | #1
On Fri, 19 Jul 2024 20:38:54 +0300
<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.
Hi Marius

Good to add a tiny bit either here or in below description for why
there are so many part numbers.

Looks like mixture of number of channels and high vs low side monitors.
And the big number is about voltage range?

Other comments inline.

Thanks,
Jonathan

> 
> Signed-off-by: Marius Cristea <marius.cristea@microchip.com>
> ---
>  .../bindings/iio/adc/microchip,pac1944.yaml   | 168 ++++++++++++++++++
>  1 file changed, 168 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..e997a4d536e9
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/microchip,pac1944.yaml
> @@ -0,0 +1,168 @@
> +# 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_1
> +      - microchip,pac1941_2
> +      - microchip,pac1942_1
> +      - microchip,pac1942_2
> +      - microchip,pac1943_1
> +      - microchip,pac1944_1
> +      - microchip,pac1951_1
> +      - microchip,pac1951_2
> +      - microchip,pac1952_1
> +      - microchip,pac1952_2
> +      - microchip,pac1953_1
> +      - microchip,pac1954_1
> +
> +  reg:
> +    maxItems: 1
> +
> +  "#address-cells":
> +    const: 1
> +
> +  "#size-cells":
> +    const: 0
> +
> +  interrupts:
> +    maxItems: 2
Need names as annoyingly people often wire just the second one
(I've never understood why!)

> +
> +  slow-io-gpios:
> +    description:
> +      A GPIO used to trigger a change is sampling rate (lowering the chip power
> +      consumption). If configured in SLOW 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.
> +
> +  microchip,gpio:
> +    type: boolean
> +    description:
> +      In default mode, this pin is a GPIO input pin. It can be
> +      configured to be an output pin, as well as the ALERT2
> +      function. This pin is an open-drain configuration and
> +      needs a pull-up resistor to VDD.

This one is a bit odd.
Can we do that configuration based on whether anyone requests it
or if it is wired as alert2?

So I don't think we should need this binding, but we may need
the stuff to provide a gpio.


Also, oddly short wrap.



> +
> +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-mode:
> +        $ref: /schemas/types.yaml#/definitions/uint32
> +        description:
> +          The VBUS could be configured into the following full scale range (FSR)
Key with these range things is to make it clear why they are a wiring thing
rather than something userspace should control.  Perhaps you can add
some detail on that here?

> +            <0>  -  VBUS has unipolar +32V to 0V FSR (default)
> +            <1>  -  VBUS has bipolar +32V to -32V FSR
> +            <2>  -  VBUS has bipolar +16V to -16V FSR
> +        maximum: 2
> +
> +      microchip,vsense-mode:
> +        $ref: /schemas/types.yaml#/definitions/uint32
> +        description:
> +          The VSENSE could be configured into the following full scale range (FSR)
> +            <0>  -  VSENSE has unipolar +100 mV to 0V FSR (default)
> +            <1>  -  VSENSE has bipolar +100 mV to -100 mV FSR
> +            <2>  -  VSENSE has bipolar +50 mV to -50 mV FSR
> +        maximum: 2
> +
> +      microchip,accumulation-mode:
> +        $ref: /schemas/types.yaml#/definitions/uint32
Why is this a hardware thing? Is it specific to particular wiring?
Not sure this one belong in DT.

> +        description:
> +          The Hardware Accumulator could be configured to accumulate
> +          VPOWER, VSENSE or VBUS
> +            <0>  -  Accumulator accumulates VPOWER (default)
> +            <1>  -  Accumulator accumulates VSENSE
Add some info on what this is for (charge measurement)
Thankfully there is a nice section on why you'd want to do this in the datasheet
as only the power option made sense to me.
There is a nice statement that you might do this as an extreme form
of oversampling as well.

> +            <2>  -  Accumulator accumulates VBUS


> +        maximum: 2
> +
> +    required:
> +      - reg
> +      - shunt-resistor-micro-ohms
> +
> +    unevaluatedProperties: false
> +
> +required:
> +  - compatible
> +  - reg
> +  - "#address-cells"
> +  - "#size-cells"
Basic power supplies should always be here as chips tend not to run without
power.  Note that doesn't mean a dts needs to actually list them if they are always
on as the regulator framework will provide stubs etc.

Jonathan
Krzysztof Kozlowski July 20, 2024, 6:36 p.m. UTC | #2
On 19/07/2024 19:38, 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.
> 
> Signed-off-by: Marius Cristea <marius.cristea@microchip.com>
> ---
>  .../bindings/iio/adc/microchip,pac1944.yaml   | 168 ++++++++++++++++++
>  1 file changed, 168 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..e997a4d536e9
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/microchip,pac1944.yaml
> @@ -0,0 +1,168 @@
> +# 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:

Wrap it according to Linux coding style, so at 80.

> +    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_1

Underscores are not allowed.

What does _1 and _2 mean?

> +      - microchip,pac1941_2
> +      - microchip,pac1942_1
> +      - microchip,pac1942_2
> +      - microchip,pac1943_1
> +      - microchip,pac1944_1
> +      - microchip,pac1951_1
> +      - microchip,pac1951_2
> +      - microchip,pac1952_1
> +      - microchip,pac1952_2
> +      - microchip,pac1953_1
> +      - microchip,pac1954_1
> +
> +  reg:
> +    maxItems: 1
> +
> +  "#address-cells":
> +    const: 1
> +
> +  "#size-cells":
> +    const: 0
> +
> +  interrupts:
> +    maxItems: 2

You need to list and describe the items.

> +
> +  slow-io-gpios:

Missing maxItems

> +    description:
> +      A GPIO used to trigger a change is sampling rate (lowering the chip power
> +      consumption). If configured in SLOW 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.
> +
> +  microchip,gpio:
> +    type: boolean
> +    description:
> +      In default mode, this pin is a GPIO input pin. It can be

Which pin?

> +      configured to be an output pin, as well as the ALERT2

output is also GPIO, so maybe you meant some interrupts? But then you
have interrupts property for that...

> +      function. This pin is an open-drain configuration and
> +      needs a pull-up resistor to VDD.
> +
> +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-mode:
> +        $ref: /schemas/types.yaml#/definitions/uint32
> +        description:
> +          The VBUS could be configured into the following full scale range (FSR)
> +            <0>  -  VBUS has unipolar +32V to 0V FSR (default)
> +            <1>  -  VBUS has bipolar +32V to -32V FSR
> +            <2>  -  VBUS has bipolar +16V to -16V FSR
> +        maximum: 2
> +
> +      microchip,vsense-mode:
> +        $ref: /schemas/types.yaml#/definitions/uint32
> +        description:
> +          The VSENSE could be configured into the following full scale range (FSR)
> +            <0>  -  VSENSE has unipolar +100 mV to 0V FSR (default)
> +            <1>  -  VSENSE has bipolar +100 mV to -100 mV FSR
> +            <2>  -  VSENSE has bipolar +50 mV to -50 mV FSR
> +        maximum: 2
> +
> +      microchip,accumulation-mode:
> +        $ref: /schemas/types.yaml#/definitions/uint32
> +        description:
> +          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
> +
> +    required:
> +      - reg
> +      - shunt-resistor-micro-ohms
> +
> +    unevaluatedProperties: false
> +
> +required:
> +  - compatible
> +  - reg
> +  - "#address-cells"
> +  - "#size-cells"
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    i2c {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        power-monitor@10 {
> +            compatible = "microchip,pac1954_1";
> +            reg = <0x10>;
> +
> +            #address-cells = <1>;
> +            #size-cells = <0>;
> +
> +            status = "okay";

Drop



Best regards,
Krzysztof
diff mbox series

Patch

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..e997a4d536e9
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/microchip,pac1944.yaml
@@ -0,0 +1,168 @@ 
+# 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_1
+      - microchip,pac1941_2
+      - microchip,pac1942_1
+      - microchip,pac1942_2
+      - microchip,pac1943_1
+      - microchip,pac1944_1
+      - microchip,pac1951_1
+      - microchip,pac1951_2
+      - microchip,pac1952_1
+      - microchip,pac1952_2
+      - microchip,pac1953_1
+      - microchip,pac1954_1
+
+  reg:
+    maxItems: 1
+
+  "#address-cells":
+    const: 1
+
+  "#size-cells":
+    const: 0
+
+  interrupts:
+    maxItems: 2
+
+  slow-io-gpios:
+    description:
+      A GPIO used to trigger a change is sampling rate (lowering the chip power
+      consumption). If configured in SLOW 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.
+
+  microchip,gpio:
+    type: boolean
+    description:
+      In default mode, this pin is a GPIO input pin. It can be
+      configured to be an output pin, as well as the ALERT2
+      function. This pin is an open-drain configuration and
+      needs a pull-up resistor to VDD.
+
+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-mode:
+        $ref: /schemas/types.yaml#/definitions/uint32
+        description:
+          The VBUS could be configured into the following full scale range (FSR)
+            <0>  -  VBUS has unipolar +32V to 0V FSR (default)
+            <1>  -  VBUS has bipolar +32V to -32V FSR
+            <2>  -  VBUS has bipolar +16V to -16V FSR
+        maximum: 2
+
+      microchip,vsense-mode:
+        $ref: /schemas/types.yaml#/definitions/uint32
+        description:
+          The VSENSE could be configured into the following full scale range (FSR)
+            <0>  -  VSENSE has unipolar +100 mV to 0V FSR (default)
+            <1>  -  VSENSE has bipolar +100 mV to -100 mV FSR
+            <2>  -  VSENSE has bipolar +50 mV to -50 mV FSR
+        maximum: 2
+
+      microchip,accumulation-mode:
+        $ref: /schemas/types.yaml#/definitions/uint32
+        description:
+          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
+
+    required:
+      - reg
+      - shunt-resistor-micro-ohms
+
+    unevaluatedProperties: false
+
+required:
+  - compatible
+  - reg
+  - "#address-cells"
+  - "#size-cells"
+
+additionalProperties: false
+
+examples:
+  - |
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        power-monitor@10 {
+            compatible = "microchip,pac1954_1";
+            reg = <0x10>;
+
+            #address-cells = <1>;
+            #size-cells = <0>;
+
+            status = "okay";
+
+            channel@1 {
+                reg = <0x1>;
+                shunt-resistor-micro-ohms = <24900>;
+                label = "CPU";
+                microchip,vbus-mode = <0>;
+                microchip,vsense-mode = <0>;
+                microchip,accumulation-mode = <0>;
+            };
+
+            channel@3 {
+                reg = <0x3>;
+                shunt-resistor-micro-ohms = <75000>;
+                label = "MEM";
+                microchip,vbus-mode = <0>;
+                microchip,vsense-mode = <0>;
+                microchip,accumulation-mode = <0>;
+            };
+
+            channel@4 {
+                reg = <0x4>;
+                shunt-resistor-micro-ohms = <100000>;
+                label = "NET";
+                microchip,vbus-mode = <0>;
+                microchip,vsense-mode = <0>;
+                microchip,accumulation-mode = <0>;
+            };
+        };
+    };
+
+...