2023-10-07 23:49:29

by Ivan Mikhaylov

[permalink] [raw]
Subject: [PATCH v5 1/2] dt-bindings: adc: provide max34408/9 device tree binding document

The hardware binding for i2c current monitoring device with overcurrent
control.

Signed-off-by: Ivan Mikhaylov <[email protected]>
---
.../bindings/iio/adc/maxim,max34408.yaml | 141 ++++++++++++++++++
1 file changed, 141 insertions(+)
create mode 100644 Documentation/devicetree/bindings/iio/adc/maxim,max34408.yaml

diff --git a/Documentation/devicetree/bindings/iio/adc/maxim,max34408.yaml b/Documentation/devicetree/bindings/iio/adc/maxim,max34408.yaml
new file mode 100644
index 000000000000..9749f1fd1802
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/maxim,max34408.yaml
@@ -0,0 +1,141 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/adc/maxim,max34408.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Two- and four-channel current monitors with overcurrent control
+
+maintainers:
+ - Ivan Mikhaylov <[email protected]>
+
+description: |
+ The MAX34408/MAX34409 are two- and four-channel current monitors that are
+ configured and monitored with a standard I2C/SMBus serial interface. Each
+ unidirectional current sensor offers precision high-side operation with a
+ low full-scale sense voltage. The devices automatically sequence through
+ two or four channels and collect the current-sense samples and average them
+ to reduce the effect of impulse noise. The raw ADC samples are compared to
+ user-programmable digital thresholds to indicate overcurrent conditions.
+ Overcurrent conditions trigger a hardware output to provide an immediate
+ indication to shut down any necessary external circuitry.
+
+ Specifications about the devices can be found at:
+ https://www.analog.com/media/en/technical-documentation/data-sheets/MAX34408-MAX34409.pdf
+
+properties:
+ compatible:
+ enum:
+ - maxim,max34408
+ - maxim,max34409
+
+ "#address-cells":
+ const: 1
+
+ "#size-cells":
+ const: 0
+
+ reg:
+ maxItems: 1
+
+ interrupts:
+ maxItems: 1
+
+ powerdown-gpios:
+ description:
+ Shutdown Output. Open-drain output. This output transitions to high impedance
+ when any of the digital comparator thresholds are exceeded as long as the ENA
+ pin is high.
+ maxItems: 1
+
+ shtdn-enable-gpios:
+ description:
+ SHTDN Enable Input. CMOS digital input. Connect to GND to clear the latch and
+ unconditionally deassert (force low) the SHTDN output and reset the shutdown
+ delay. Connect to VDD to enable normal latch operation of the SHTDN output.
+ maxItems: 1
+
+ vdd-supply: true
+
+patternProperties:
+ "^channel@[0-3]$":
+ $ref: adc.yaml
+ type: object
+ description:
+ Represents the internal channels of the ADC.
+
+ properties:
+ reg:
+ items:
+ minimum: 0
+ maximum: 3
+
+ maxim,rsense-val-micro-ohms:
+ description:
+ Adjust the Rsense value to monitor higher or lower current levels for
+ input.
+ enum: [250, 500, 1000, 5000, 10000, 50000, 100000, 200000, 500000]
+ default: 1000
+
+ required:
+ - reg
+ - maxim,rsense-val-micro-ohms
+
+ unevaluatedProperties: false
+
+required:
+ - compatible
+ - reg
+
+allOf:
+ - if:
+ properties:
+ compatible:
+ contains:
+ const: maxim,max34408
+ then:
+ patternProperties:
+ "^channel@[2-3]$": false
+ "^channel@[0-1]$":
+ properties:
+ reg:
+ minimum: 0
+ maximum: 1
+ else:
+ patternProperties:
+ "^channel@[0-3]$":
+ properties:
+ reg:
+ minimum: 0
+ maximum: 3
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/gpio/gpio.h>
+
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ adc@1e {
+ compatible = "maxim,max34409";
+ reg = <0x1e>;
+ powerdown-gpios = <&gpio0 1 GPIO_ACTIVE_LOW>;
+ shtdn-enable-gpios = <&gpio0 2 GPIO_ACTIVE_HIGH>;
+
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ channel@0 {
+ reg = <0x0>;
+ maxim,rsense-val-micro-ohms = <5000>;
+ };
+
+ channel@1 {
+ reg = <0x1>;
+ maxim,rsense-val-micro-ohms = <10000>;
+ };
+ };
+ };
--
2.42.0


2023-10-10 14:33:27

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH v5 1/2] dt-bindings: adc: provide max34408/9 device tree binding document

On Sun, Oct 08, 2023 at 02:48:37AM +0300, Ivan Mikhaylov wrote:
> The hardware binding for i2c current monitoring device with overcurrent
> control.
>
> Signed-off-by: Ivan Mikhaylov <[email protected]>
> ---
> .../bindings/iio/adc/maxim,max34408.yaml | 141 ++++++++++++++++++
> 1 file changed, 141 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/iio/adc/maxim,max34408.yaml
>
> diff --git a/Documentation/devicetree/bindings/iio/adc/maxim,max34408.yaml b/Documentation/devicetree/bindings/iio/adc/maxim,max34408.yaml
> new file mode 100644
> index 000000000000..9749f1fd1802
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/maxim,max34408.yaml
> @@ -0,0 +1,141 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/adc/maxim,max34408.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Two- and four-channel current monitors with overcurrent control

Maxim MAX34408/MAX34409 current monitors with overcurrent control

> +
> +maintainers:
> + - Ivan Mikhaylov <[email protected]>
> +
> +description: |
> + The MAX34408/MAX34409 are two- and four-channel current monitors that are
> + configured and monitored with a standard I2C/SMBus serial interface. Each
> + unidirectional current sensor offers precision high-side operation with a
> + low full-scale sense voltage. The devices automatically sequence through
> + two or four channels and collect the current-sense samples and average them
> + to reduce the effect of impulse noise. The raw ADC samples are compared to
> + user-programmable digital thresholds to indicate overcurrent conditions.
> + Overcurrent conditions trigger a hardware output to provide an immediate
> + indication to shut down any necessary external circuitry.
> +
> + Specifications about the devices can be found at:
> + https://www.analog.com/media/en/technical-documentation/data-sheets/MAX34408-MAX34409.pdf
> +
> +properties:
> + compatible:
> + enum:
> + - maxim,max34408
> + - maxim,max34409
> +
> + "#address-cells":
> + const: 1
> +
> + "#size-cells":
> + const: 0
> +
> + reg:
> + maxItems: 1
> +
> + interrupts:
> + maxItems: 1
> +
> + powerdown-gpios:
> + description:
> + Shutdown Output. Open-drain output. This output transitions to high impedance
> + when any of the digital comparator thresholds are exceeded as long as the ENA
> + pin is high.
> + maxItems: 1
> +
> + shtdn-enable-gpios:
> + description:
> + SHTDN Enable Input. CMOS digital input. Connect to GND to clear the latch and
> + unconditionally deassert (force low) the SHTDN output and reset the shutdown
> + delay. Connect to VDD to enable normal latch operation of the SHTDN output.
> + maxItems: 1
> +
> + vdd-supply: true
> +
> +patternProperties:
> + "^channel@[0-3]$":
> + $ref: adc.yaml
> + type: object
> + description:
> + Represents the internal channels of the ADC.
> +
> + properties:
> + reg:
> + items:
> + minimum: 0
> + maximum: 3

This allows any number of 'reg' entries. You need this instead:

items:
- minimum: 0
maximum: 3


> +
> + maxim,rsense-val-micro-ohms:
> + description:
> + Adjust the Rsense value to monitor higher or lower current levels for
> + input.
> + enum: [250, 500, 1000, 5000, 10000, 50000, 100000, 200000, 500000]
> + default: 1000
> +
> + required:
> + - reg
> + - maxim,rsense-val-micro-ohms
> +
> + unevaluatedProperties: false
> +
> +required:
> + - compatible
> + - reg
> +
> +allOf:
> + - if:
> + properties:
> + compatible:
> + contains:
> + const: maxim,max34408
> + then:
> + patternProperties:
> + "^channel@[2-3]$": false
> + "^channel@[0-1]$":
> + properties:
> + reg:
> + minimum: 0

0 is already the minimum

> + maximum: 1
> + else:
> + patternProperties:
> + "^channel@[0-3]$":
> + properties:
> + reg:
> + minimum: 0

ditto

> + maximum: 3
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/gpio/gpio.h>
> +
> + i2c {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + adc@1e {
> + compatible = "maxim,max34409";
> + reg = <0x1e>;
> + powerdown-gpios = <&gpio0 1 GPIO_ACTIVE_LOW>;
> + shtdn-enable-gpios = <&gpio0 2 GPIO_ACTIVE_HIGH>;
> +
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + channel@0 {
> + reg = <0x0>;
> + maxim,rsense-val-micro-ohms = <5000>;
> + };
> +
> + channel@1 {
> + reg = <0x1>;
> + maxim,rsense-val-micro-ohms = <10000>;
> + };
> + };
> + };
> --
> 2.42.0
>

2023-10-10 14:40:51

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH v5 1/2] dt-bindings: adc: provide max34408/9 device tree binding document

On Sun, 8 Oct 2023 02:48:37 +0300
Ivan Mikhaylov <[email protected]> wrote:

> The hardware binding for i2c current monitoring device with overcurrent
> control.
>
> Signed-off-by: Ivan Mikhaylov <[email protected]>
> ---
> .../bindings/iio/adc/maxim,max34408.yaml | 141 ++++++++++++++++++
> 1 file changed, 141 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/iio/adc/maxim,max34408.yaml
>
> diff --git a/Documentation/devicetree/bindings/iio/adc/maxim,max34408.yaml b/Documentation/devicetree/bindings/iio/adc/maxim,max34408.yaml
> new file mode 100644
> index 000000000000..9749f1fd1802
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/maxim,max34408.yaml
> @@ -0,0 +1,141 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/adc/maxim,max34408.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Two- and four-channel current monitors with overcurrent control
> +
> +maintainers:
> + - Ivan Mikhaylov <[email protected]>
> +
> +description: |
> + The MAX34408/MAX34409 are two- and four-channel current monitors that are
> + configured and monitored with a standard I2C/SMBus serial interface. Each
> + unidirectional current sensor offers precision high-side operation with a
> + low full-scale sense voltage. The devices automatically sequence through
> + two or four channels and collect the current-sense samples and average them
> + to reduce the effect of impulse noise. The raw ADC samples are compared to
> + user-programmable digital thresholds to indicate overcurrent conditions.
> + Overcurrent conditions trigger a hardware output to provide an immediate
> + indication to shut down any necessary external circuitry.
> +
> + Specifications about the devices can be found at:
> + https://www.analog.com/media/en/technical-documentation/data-sheets/MAX34408-MAX34409.pdf
> +
> +properties:
> + compatible:
> + enum:
> + - maxim,max34408
> + - maxim,max34409
> +
> + "#address-cells":
> + const: 1
> +
> + "#size-cells":
> + const: 0
> +
> + reg:
> + maxItems: 1
> +
> + interrupts:
> + maxItems: 1
> +
> + powerdown-gpios:
> + description:
> + Shutdown Output. Open-drain output. This output transitions to high impedance
> + when any of the digital comparator thresholds are exceeded as long as the ENA
> + pin is high.
> + maxItems: 1
> +
> + shtdn-enable-gpios:

I guess the review crossed with you sending v5. There is some feedback on v4 you need
to address here.

> + description:
> + SHTDN Enable Input. CMOS digital input. Connect to GND to clear the latch and
> + unconditionally deassert (force low) the SHTDN output and reset the shutdown
> + delay. Connect to VDD to enable normal latch operation of the SHTDN output.
> + maxItems: 1
> +
> + vdd-supply: true
> +
> +patternProperties:
> + "^channel@[0-3]$":
> + $ref: adc.yaml
> + type: object
> + description:
> + Represents the internal channels of the ADC.
> +
> + properties:
> + reg:
> + items:
> + minimum: 0
> + maximum: 3
> +
> + maxim,rsense-val-micro-ohms:
> + description:
> + Adjust the Rsense value to monitor higher or lower current levels for
> + input.
> + enum: [250, 500, 1000, 5000, 10000, 50000, 100000, 200000, 500000]
> + default: 1000
> +
> + required:
> + - reg
> + - maxim,rsense-val-micro-ohms
> +
> + unevaluatedProperties: false
> +
> +required:
> + - compatible
> + - reg
> +
> +allOf:
> + - if:
> + properties:
> + compatible:
> + contains:
> + const: maxim,max34408
> + then:
> + patternProperties:
> + "^channel@[2-3]$": false
> + "^channel@[0-1]$":
> + properties:
> + reg:
> + minimum: 0
> + maximum: 1
> + else:
> + patternProperties:
> + "^channel@[0-3]$":
> + properties:
> + reg:
> + minimum: 0
> + maximum: 3
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/gpio/gpio.h>
> +
> + i2c {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + adc@1e {
> + compatible = "maxim,max34409";
> + reg = <0x1e>;
> + powerdown-gpios = <&gpio0 1 GPIO_ACTIVE_LOW>;
> + shtdn-enable-gpios = <&gpio0 2 GPIO_ACTIVE_HIGH>;
> +
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + channel@0 {
> + reg = <0x0>;
> + maxim,rsense-val-micro-ohms = <5000>;
> + };
> +
> + channel@1 {
> + reg = <0x1>;
> + maxim,rsense-val-micro-ohms = <10000>;
> + };
> + };
> + };

2023-10-10 20:23:15

by Ivan Mikhaylov

[permalink] [raw]
Subject: Re: [PATCH v5 1/2] dt-bindings: adc: provide max34408/9 device tree binding document

On Tue, 2023-10-10 at 15:40 +0100, Jonathan Cameron wrote:
> On Sun,  8 Oct 2023 02:48:37 +0300
> Ivan Mikhaylov <[email protected]> wrote:
>
> > The hardware binding for i2c current monitoring device with
> > overcurrent
> > control.
> >
> > Signed-off-by: Ivan Mikhaylov <[email protected]>
> > ---
> >  .../bindings/iio/adc/maxim,max34408.yaml      | 141
> > ++++++++++++++++++
> >  1 file changed, 141 insertions(+)
> >  create mode 100644
> > Documentation/devicetree/bindings/iio/adc/maxim,max34408.yaml
> >
> > diff --git
> > a/Documentation/devicetree/bindings/iio/adc/maxim,max34408.yaml
> > b/Documentation/devicetree/bindings/iio/adc/maxim,max34408.yaml
> > new file mode 100644
> > index 000000000000..9749f1fd1802
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/iio/adc/maxim,max34408.yaml
> > @@ -0,0 +1,141 @@
> > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/iio/adc/maxim,max34408.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Two- and four-channel current monitors with overcurrent
> > control
> > +
> > +maintainers:
> > +  - Ivan Mikhaylov <[email protected]>
> > +
> > +description: |
> > +  The MAX34408/MAX34409 are two- and four-channel current monitors
> > that are
> > +  configured and monitored with a standard I2C/SMBus serial
> > interface. Each
> > +  unidirectional current sensor offers precision high-side
> > operation with a
> > +  low full-scale sense voltage. The devices automatically sequence
> > through
> > +  two or four channels and collect the current-sense samples and
> > average them
> > +  to reduce the effect of impulse noise. The raw ADC samples are
> > compared to
> > +  user-programmable digital thresholds to indicate overcurrent
> > conditions.
> > +  Overcurrent conditions trigger a hardware output to provide an
> > immediate
> > +  indication to shut down any necessary external circuitry.
> > +
> > +  Specifications about the devices can be found at:
> > + 
> > https://www.analog.com/media/en/technical-documentation/data-sheets/MAX34408-MAX34409.pdf
> > +
> > +properties:
> > +  compatible:
> > +    enum:
> > +      - maxim,max34408
> > +      - maxim,max34409
> > +
> > +  "#address-cells":
> > +    const: 1
> > +
> > +  "#size-cells":
> > +    const: 0
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  interrupts:
> > +    maxItems: 1
> > +
> > +  powerdown-gpios:
> > +    description:
> > +      Shutdown Output. Open-drain output. This output transitions
> > to high impedance
> > +      when any of the digital comparator thresholds are exceeded
> > as long as the ENA
> > +      pin is high.
> > +    maxItems: 1
> > +
> > +  shtdn-enable-gpios:
>
> I guess the review crossed with you sending v5.  There is some
> feedback on v4 you need
> to address here.

Jonathan, I thought I did, I've changed ena to powerdown-gpios from
Krzysztof's comments but about this one pin I'm still not sure, it
looks like *-enable-gpios (like in *-enable-gpios pins in
iio/frequency/adi,adf4377.yaml) pin or is it not? Or maybe any other
suggestions about naming of this one?

Thanks.

>
> > +    description:
> > +      SHTDN Enable Input. CMOS digital input. Connect to GND to
> > clear the latch and
> > +      unconditionally deassert (force low) the SHTDN output and
> > reset the shutdown
> > +      delay. Connect to VDD to enable normal latch operation of
> > the SHTDN output.
> > +    maxItems: 1
> > +
> > +  vdd-supply: true
> > +
> > +patternProperties:
> > +  "^channel@[0-3]$":
> > +    $ref: adc.yaml
> > +    type: object
> > +    description:
> > +      Represents the internal channels of the ADC.
> > +
> > +    properties:
> > +      reg:
> > +        items:
> > +          minimum: 0
> > +          maximum: 3
> > +
> > +      maxim,rsense-val-micro-ohms:
> > +        description:
> > +          Adjust the Rsense value to monitor higher or lower
> > current levels for
> > +          input.
> > +        enum: [250, 500, 1000, 5000, 10000, 50000, 100000, 200000,
> > 500000]
> > +        default: 1000
> > +
> > +    required:
> > +      - reg
> > +      - maxim,rsense-val-micro-ohms
> > +
> > +    unevaluatedProperties: false
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +
> > +allOf:
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          contains:
> > +            const: maxim,max34408
> > +    then:
> > +      patternProperties:
> > +        "^channel@[2-3]$": false
> > +        "^channel@[0-1]$":
> > +          properties:
> > +            reg:
> > +              minimum: 0
> > +              maximum: 1
> > +    else:
> > +      patternProperties:
> > +        "^channel@[0-3]$":
> > +          properties:
> > +            reg:
> > +              minimum: 0
> > +              maximum: 3
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    #include <dt-bindings/gpio/gpio.h>
> > +
> > +    i2c {
> > +        #address-cells = <1>;
> > +        #size-cells = <0>;
> > +
> > +        adc@1e {
> > +              compatible = "maxim,max34409";
> > +              reg = <0x1e>;
> > +              powerdown-gpios = <&gpio0 1 GPIO_ACTIVE_LOW>;
> > +              shtdn-enable-gpios = <&gpio0 2 GPIO_ACTIVE_HIGH>;
> > +
> > +              #address-cells = <1>;
> > +              #size-cells = <0>;
> > +
> > +              channel@0 {
> > +                  reg = <0x0>;
> > +                  maxim,rsense-val-micro-ohms = <5000>;
> > +              };
> > +
> > +              channel@1 {
> > +                  reg = <0x1>;
> > +                  maxim,rsense-val-micro-ohms = <10000>;
> > +             };
> > +        };
> > +    };
>

2023-10-12 08:14:33

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH v5 1/2] dt-bindings: adc: provide max34408/9 device tree binding document

On Tue, 10 Oct 2023 23:22:48 +0300
Ivan Mikhaylov <[email protected]> wrote:

> On Tue, 2023-10-10 at 15:40 +0100, Jonathan Cameron wrote:
> > On Sun,  8 Oct 2023 02:48:37 +0300
> > Ivan Mikhaylov <[email protected]> wrote:
> >
> > > The hardware binding for i2c current monitoring device with
> > > overcurrent
> > > control.
> > >
> > > Signed-off-by: Ivan Mikhaylov <[email protected]>
> > > ---
> > >  .../bindings/iio/adc/maxim,max34408.yaml      | 141
> > > ++++++++++++++++++
> > >  1 file changed, 141 insertions(+)
> > >  create mode 100644
> > > Documentation/devicetree/bindings/iio/adc/maxim,max34408.yaml
> > >
> > > diff --git
> > > a/Documentation/devicetree/bindings/iio/adc/maxim,max34408.yaml
> > > b/Documentation/devicetree/bindings/iio/adc/maxim,max34408.yaml
> > > new file mode 100644
> > > index 000000000000..9749f1fd1802
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/iio/adc/maxim,max34408.yaml
> > > @@ -0,0 +1,141 @@
> > > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/iio/adc/maxim,max34408.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: Two- and four-channel current monitors with overcurrent
> > > control
> > > +
> > > +maintainers:
> > > +  - Ivan Mikhaylov <[email protected]>
> > > +
> > > +description: |
> > > +  The MAX34408/MAX34409 are two- and four-channel current monitors
> > > that are
> > > +  configured and monitored with a standard I2C/SMBus serial
> > > interface. Each
> > > +  unidirectional current sensor offers precision high-side
> > > operation with a
> > > +  low full-scale sense voltage. The devices automatically sequence
> > > through
> > > +  two or four channels and collect the current-sense samples and
> > > average them
> > > +  to reduce the effect of impulse noise. The raw ADC samples are
> > > compared to
> > > +  user-programmable digital thresholds to indicate overcurrent
> > > conditions.
> > > +  Overcurrent conditions trigger a hardware output to provide an
> > > immediate
> > > +  indication to shut down any necessary external circuitry.
> > > +
> > > +  Specifications about the devices can be found at:
> > > + 
> > > https://www.analog.com/media/en/technical-documentation/data-sheets/MAX34408-MAX34409.pdf
> > > +
> > > +properties:
> > > +  compatible:
> > > +    enum:
> > > +      - maxim,max34408
> > > +      - maxim,max34409
> > > +
> > > +  "#address-cells":
> > > +    const: 1
> > > +
> > > +  "#size-cells":
> > > +    const: 0
> > > +
> > > +  reg:
> > > +    maxItems: 1
> > > +
> > > +  interrupts:
> > > +    maxItems: 1
> > > +
> > > +  powerdown-gpios:
> > > +    description:
> > > +      Shutdown Output. Open-drain output. This output transitions
> > > to high impedance
> > > +      when any of the digital comparator thresholds are exceeded
> > > as long as the ENA
> > > +      pin is high.
> > > +    maxItems: 1
> > > +
> > > +  shtdn-enable-gpios:
> >
> > I guess the review crossed with you sending v5.  There is some
> > feedback on v4 you need
> > to address here.
>
> Jonathan, I thought I did, I've changed ena to powerdown-gpios from
> Krzysztof's comments but about this one pin I'm still not sure, it
> looks like *-enable-gpios (like in *-enable-gpios pins in
> iio/frequency/adi,adf4377.yaml) pin or is it not? Or maybe any other
> suggestions about naming of this one?
>
> Thanks.

shutdown-gpios and make the sense (active high / low) such that setting
it results in teh device being shut down.
Or treat it as an enable and enable-gpios

Something that indicates both shutdown and enable is confusing ;)

Jonathan


>
> >
> > > +    description:
> > > +      SHTDN Enable Input. CMOS digital input. Connect to GND to
> > > clear the latch and
> > > +      unconditionally deassert (force low) the SHTDN output and
> > > reset the shutdown
> > > +      delay. Connect to VDD to enable normal latch operation of
> > > the SHTDN output.
> > > +    maxItems: 1
> > > +
> > > +  vdd-supply: true
> > > +
> > > +patternProperties:
> > > +  "^channel@[0-3]$":
> > > +    $ref: adc.yaml
> > > +    type: object
> > > +    description:
> > > +      Represents the internal channels of the ADC.
> > > +
> > > +    properties:
> > > +      reg:
> > > +        items:
> > > +          minimum: 0
> > > +          maximum: 3
> > > +
> > > +      maxim,rsense-val-micro-ohms:
> > > +        description:
> > > +          Adjust the Rsense value to monitor higher or lower
> > > current levels for
> > > +          input.
> > > +        enum: [250, 500, 1000, 5000, 10000, 50000, 100000, 200000,
> > > 500000]
> > > +        default: 1000
> > > +
> > > +    required:
> > > +      - reg
> > > +      - maxim,rsense-val-micro-ohms
> > > +
> > > +    unevaluatedProperties: false
> > > +
> > > +required:
> > > +  - compatible
> > > +  - reg
> > > +
> > > +allOf:
> > > +  - if:
> > > +      properties:
> > > +        compatible:
> > > +          contains:
> > > +            const: maxim,max34408
> > > +    then:
> > > +      patternProperties:
> > > +        "^channel@[2-3]$": false
> > > +        "^channel@[0-1]$":
> > > +          properties:
> > > +            reg:
> > > +              minimum: 0
> > > +              maximum: 1
> > > +    else:
> > > +      patternProperties:
> > > +        "^channel@[0-3]$":
> > > +          properties:
> > > +            reg:
> > > +              minimum: 0
> > > +              maximum: 3
> > > +
> > > +additionalProperties: false
> > > +
> > > +examples:
> > > +  - |
> > > +    #include <dt-bindings/gpio/gpio.h>
> > > +
> > > +    i2c {
> > > +        #address-cells = <1>;
> > > +        #size-cells = <0>;
> > > +
> > > +        adc@1e {
> > > +              compatible = "maxim,max34409";
> > > +              reg = <0x1e>;
> > > +              powerdown-gpios = <&gpio0 1 GPIO_ACTIVE_LOW>;
> > > +              shtdn-enable-gpios = <&gpio0 2 GPIO_ACTIVE_HIGH>;
> > > +
> > > +              #address-cells = <1>;
> > > +              #size-cells = <0>;
> > > +
> > > +              channel@0 {
> > > +                  reg = <0x0>;
> > > +                  maxim,rsense-val-micro-ohms = <5000>;
> > > +              };
> > > +
> > > +              channel@1 {
> > > +                  reg = <0x1>;
> > > +                  maxim,rsense-val-micro-ohms = <10000>;
> > > +             };
> > > +        };
> > > +    };
> >
>

2023-10-12 16:27:46

by Ivan Mikhaylov

[permalink] [raw]
Subject: Re: [PATCH v5 1/2] dt-bindings: adc: provide max34408/9 device tree binding document

On Thu, 2023-10-12 at 08:40 +0100, Jonathan Cameron wrote:
> On Tue, 10 Oct 2023 23:22:48 +0300
> Ivan Mikhaylov <[email protected]> wrote:
>
> > On Tue, 2023-10-10 at 15:40 +0100, Jonathan Cameron wrote:
> > > On Sun,  8 Oct 2023 02:48:37 +0300
> > > Ivan Mikhaylov <[email protected]> wrote:
> > >  
> > > > The hardware binding for i2c current monitoring device with
> > > > overcurrent
> > > > control.
> > > >
> > > > Signed-off-by: Ivan Mikhaylov <[email protected]>
> > > > ---
> > > >  .../bindings/iio/adc/maxim,max34408.yaml      | 141
> > > > ++++++++++++++++++
> > > >  1 file changed, 141 insertions(+)
> > > >  create mode 100644
> > > > Documentation/devicetree/bindings/iio/adc/maxim,max34408.yaml
> > > >
> > > > diff --git
> > > > a/Documentation/devicetree/bindings/iio/adc/maxim,max34408.yaml
> > > > b/Documentation/devicetree/bindings/iio/adc/maxim,max34408.yaml
> > > > new file mode 100644
> > > > index 000000000000..9749f1fd1802
> > > > --- /dev/null
> > > > +++
> > > > b/Documentation/devicetree/bindings/iio/adc/maxim,max34408.yaml
> > > > @@ -0,0 +1,141 @@
> > > > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > > > +%YAML 1.2
> > > > +---
> > > > +$id:
> > > > http://devicetree.org/schemas/iio/adc/maxim,max34408.yaml#
> > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > +
> > > > +title: Two- and four-channel current monitors with overcurrent
> > > > control
> > > > +
> > > > +maintainers:
> > > > +  - Ivan Mikhaylov <[email protected]>
> > > > +
> > > > +description: |
> > > > +  The MAX34408/MAX34409 are two- and four-channel current
> > > > monitors
> > > > that are
> > > > +  configured and monitored with a standard I2C/SMBus serial
> > > > interface. Each
> > > > +  unidirectional current sensor offers precision high-side
> > > > operation with a
> > > > +  low full-scale sense voltage. The devices automatically
> > > > sequence
> > > > through
> > > > +  two or four channels and collect the current-sense samples
> > > > and
> > > > average them
> > > > +  to reduce the effect of impulse noise. The raw ADC samples
> > > > are
> > > > compared to
> > > > +  user-programmable digital thresholds to indicate overcurrent
> > > > conditions.
> > > > +  Overcurrent conditions trigger a hardware output to provide
> > > > an
> > > > immediate
> > > > +  indication to shut down any necessary external circuitry.
> > > > +
> > > > +  Specifications about the devices can be found at:
> > > > + 
> > > > https://www.analog.com/media/en/technical-documentation/data-sheets/MAX34408-MAX34409.pdf
> > > > +
> > > > +properties:
> > > > +  compatible:
> > > > +    enum:
> > > > +      - maxim,max34408
> > > > +      - maxim,max34409
> > > > +
> > > > +  "#address-cells":
> > > > +    const: 1
> > > > +
> > > > +  "#size-cells":
> > > > +    const: 0
> > > > +
> > > > +  reg:
> > > > +    maxItems: 1
> > > > +
> > > > +  interrupts:
> > > > +    maxItems: 1
> > > > +
> > > > +  powerdown-gpios:
> > > > +    description:
> > > > +      Shutdown Output. Open-drain output. This output
> > > > transitions
> > > > to high impedance
> > > > +      when any of the digital comparator thresholds are
> > > > exceeded
> > > > as long as the ENA
> > > > +      pin is high.
> > > > +    maxItems: 1
> > > > +
> > > > +  shtdn-enable-gpios: 
> > >
> > > I guess the review crossed with you sending v5.  There is some
> > > feedback on v4 you need
> > > to address here. 
> >
> > Jonathan, I thought I did, I've changed ena to powerdown-gpios from
> > Krzysztof's comments but about this one pin I'm still not sure, it
> > looks like *-enable-gpios (like in *-enable-gpios pins in
> > iio/frequency/adi,adf4377.yaml) pin or is it not? Or maybe any
> > other
> > suggestions about naming of this one?
> >
> > Thanks.
>
> shutdown-gpios and make the sense (active high / low) such that
> setting
> it results in teh device being shut down.
> Or treat it as an enable and enable-gpios
>
> Something that indicates both shutdown and enable is confusing ;)
>
> Jonathan


Jonathan, then I make these changes:

powerdown-gpios: -> output-enable:
shtdn-enable-gpios: -> enable-gpios:

Is it ok?

Thanks.

2023-10-13 08:36:32

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v5 1/2] dt-bindings: adc: provide max34408/9 device tree binding document

On 13/10/2023 10:19, Jonathan Cameron wrote:
> On Thu, 12 Oct 2023 19:27:33 +0300
> Ivan Mikhaylov <[email protected]> wrote:
>
>> On Thu, 2023-10-12 at 08:40 +0100, Jonathan Cameron wrote:
>>> On Tue, 10 Oct 2023 23:22:48 +0300
>>> Ivan Mikhaylov <[email protected]> wrote:
>>>
>>>> On Tue, 2023-10-10 at 15:40 +0100, Jonathan Cameron wrote:
>>>>> On Sun,  8 Oct 2023 02:48:37 +0300
>>>>> Ivan Mikhaylov <[email protected]> wrote:
>>>>>  
>>>>>> The hardware binding for i2c current monitoring device with
>>>>>> overcurrent
>>>>>> control.
>>>>>>
>>>>>> Signed-off-by: Ivan Mikhaylov <[email protected]>
>>>>>> ---
>>>>>>  .../bindings/iio/adc/maxim,max34408.yaml      | 141
>>>>>> ++++++++++++++++++
>>>>>>  1 file changed, 141 insertions(+)
>>>>>>  create mode 100644
>>>>>> Documentation/devicetree/bindings/iio/adc/maxim,max34408.yaml
>>>>>>
>>>>>> diff --git
>>>>>> a/Documentation/devicetree/bindings/iio/adc/maxim,max34408.yaml
>>>>>> b/Documentation/devicetree/bindings/iio/adc/maxim,max34408.yaml
>>>>>> new file mode 100644
>>>>>> index 000000000000..9749f1fd1802
>>>>>> --- /dev/null
>>>>>> +++
>>>>>> b/Documentation/devicetree/bindings/iio/adc/maxim,max34408.yaml
>>>>>> @@ -0,0 +1,141 @@
>>>>>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>>>>>> +%YAML 1.2
>>>>>> +---
>>>>>> +$id:
>>>>>> http://devicetree.org/schemas/iio/adc/maxim,max34408.yaml#
>>>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>>>>> +
>>>>>> +title: Two- and four-channel current monitors with overcurrent
>>>>>> control
>>>>>> +
>>>>>> +maintainers:
>>>>>> +  - Ivan Mikhaylov <[email protected]>
>>>>>> +
>>>>>> +description: |
>>>>>> +  The MAX34408/MAX34409 are two- and four-channel current
>>>>>> monitors
>>>>>> that are
>>>>>> +  configured and monitored with a standard I2C/SMBus serial
>>>>>> interface. Each
>>>>>> +  unidirectional current sensor offers precision high-side
>>>>>> operation with a
>>>>>> +  low full-scale sense voltage. The devices automatically
>>>>>> sequence
>>>>>> through
>>>>>> +  two or four channels and collect the current-sense samples
>>>>>> and
>>>>>> average them
>>>>>> +  to reduce the effect of impulse noise. The raw ADC samples
>>>>>> are
>>>>>> compared to
>>>>>> +  user-programmable digital thresholds to indicate overcurrent
>>>>>> conditions.
>>>>>> +  Overcurrent conditions trigger a hardware output to provide
>>>>>> an
>>>>>> immediate
>>>>>> +  indication to shut down any necessary external circuitry.
>>>>>> +
>>>>>> +  Specifications about the devices can be found at:
>>>>>> + 
>>>>>> https://www.analog.com/media/en/technical-documentation/data-sheets/MAX34408-MAX34409.pdf
>>>>>> +
>>>>>> +properties:
>>>>>> +  compatible:
>>>>>> +    enum:
>>>>>> +      - maxim,max34408
>>>>>> +      - maxim,max34409
>>>>>> +
>>>>>> +  "#address-cells":
>>>>>> +    const: 1
>>>>>> +
>>>>>> +  "#size-cells":
>>>>>> +    const: 0
>>>>>> +
>>>>>> +  reg:
>>>>>> +    maxItems: 1
>>>>>> +
>>>>>> +  interrupts:
>>>>>> +    maxItems: 1
>>>>>> +
>>>>>> +  powerdown-gpios:
>>>>>> +    description:
>>>>>> +      Shutdown Output. Open-drain output. This output
>>>>>> transitions
>>>>>> to high impedance
>>>>>> +      when any of the digital comparator thresholds are
>>>>>> exceeded
>>>>>> as long as the ENA
>>>>>> +      pin is high.
>>>>>> +    maxItems: 1
>>>>>> +
>>>>>> +  shtdn-enable-gpios: 
>>>>>
>>>>> I guess the review crossed with you sending v5.  There is some
>>>>> feedback on v4 you need
>>>>> to address here. 
>>>>
>>>> Jonathan, I thought I did, I've changed ena to powerdown-gpios from
>>>> Krzysztof's comments but about this one pin I'm still not sure, it
>>>> looks like *-enable-gpios (like in *-enable-gpios pins in
>>>> iio/frequency/adi,adf4377.yaml) pin or is it not? Or maybe any
>>>> other
>>>> suggestions about naming of this one?
>>>>
>>>> Thanks.
>>>
>>> shutdown-gpios and make the sense (active high / low) such that
>>> setting
>>> it results in teh device being shut down.
>>> Or treat it as an enable and enable-gpios
>>>
>>> Something that indicates both shutdown and enable is confusing ;)
>>>
>>> Jonathan
>>
>>
>> Jonathan, then I make these changes:
>>
>> powerdown-gpios: -> output-enable:
> Needs to retain the gpios bit as we want the standard gpio stuff to pick
> them up. I'm not that keen on output-enable-gpios though. The activity
> here is very much 'shutdown because of error or not enabled' I think.
> So perhaps we flip the sense and document that it needs to be active low?
>
>> shtdn-enable-gpios: -> enable-gpios:
>>
>> Is it ok?
>
> Conor, Rob, Krzysztof - you probably have a better insight into this than
> I do.
>

"enable-gpios" are for turning on a specific feature, not powering
on/off entire device. For example to enable regulator output.

"powerdown-gpios" are for turning device on/off.

I don't know what do you have in your device.

Best regards,
Krzysztof

2023-10-13 09:53:53

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v5 1/2] dt-bindings: adc: provide max34408/9 device tree binding document

On 13/10/2023 11:09, Jonathan Cameron wrote:
>>>>>>>> +  shtdn-enable-gpios: 
>>>>>>>
>>>>>>> I guess the review crossed with you sending v5.  There is some
>>>>>>> feedback on v4 you need
>>>>>>> to address here. 
>>>>>>
>>>>>> Jonathan, I thought I did, I've changed ena to powerdown-gpios from
>>>>>> Krzysztof's comments but about this one pin I'm still not sure, it
>>>>>> looks like *-enable-gpios (like in *-enable-gpios pins in
>>>>>> iio/frequency/adi,adf4377.yaml) pin or is it not? Or maybe any
>>>>>> other
>>>>>> suggestions about naming of this one?
>>>>>>
>>>>>> Thanks.
>>>>>
>>>>> shutdown-gpios and make the sense (active high / low) such that
>>>>> setting
>>>>> it results in teh device being shut down.
>>>>> Or treat it as an enable and enable-gpios
>>>>>
>>>>> Something that indicates both shutdown and enable is confusing ;)
>>>>>
>>>>> Jonathan
>>>>
>>>>
>>>> Jonathan, then I make these changes:
>>>>
>>>> powerdown-gpios: -> output-enable:
>>> Needs to retain the gpios bit as we want the standard gpio stuff to pick
>>> them up. I'm not that keen on output-enable-gpios though. The activity
>>> here is very much 'shutdown because of error or not enabled' I think.
>>> So perhaps we flip the sense and document that it needs to be active low?
>>>
>>>> shtdn-enable-gpios: -> enable-gpios:
>>>>
>>>> Is it ok?
>>>
>>> Conor, Rob, Krzysztof - you probably have a better insight into this than
>>> I do.
>>>
>>
>> "enable-gpios" are for turning on a specific feature, not powering
>> on/off entire device. For example to enable regulator output.
>>
>> "powerdown-gpios" are for turning device on/off.
>>
>> I don't know what do you have in your device.
> Ok. Sounds like that what is enable-gpios above should be shutdown-gpios.

shutdown-gpios sounds exactly the same as powerdown-gpios and it is
already used in exactly same context.

> The other case is a device output indicating whether the device is
> shutdown. That can happen because it was told to do so (via the other gpio),
> or because it is in an error state. What's a good naming convention for that?

There is no convention and I did not see such case so far.
powerdown-status-gpios? powerdown-state-gpios?



Best regards,
Krzysztof

2023-10-13 18:12:16

by Jonathan Cameron

[permalink] [raw]
Subject: Re: [PATCH v5 1/2] dt-bindings: adc: provide max34408/9 device tree binding document

On Fri, 13 Oct 2023 11:53:33 +0200
Krzysztof Kozlowski <[email protected]> wrote:

> On 13/10/2023 11:09, Jonathan Cameron wrote:
> >>>>>>>> +  shtdn-enable-gpios: 
> >>>>>>>
> >>>>>>> I guess the review crossed with you sending v5.  There is some
> >>>>>>> feedback on v4 you need
> >>>>>>> to address here. 
> >>>>>>
> >>>>>> Jonathan, I thought I did, I've changed ena to powerdown-gpios from
> >>>>>> Krzysztof's comments but about this one pin I'm still not sure, it
> >>>>>> looks like *-enable-gpios (like in *-enable-gpios pins in
> >>>>>> iio/frequency/adi,adf4377.yaml) pin or is it not? Or maybe any
> >>>>>> other
> >>>>>> suggestions about naming of this one?
> >>>>>>
> >>>>>> Thanks.
> >>>>>
> >>>>> shutdown-gpios and make the sense (active high / low) such that
> >>>>> setting
> >>>>> it results in teh device being shut down.
> >>>>> Or treat it as an enable and enable-gpios
> >>>>>
> >>>>> Something that indicates both shutdown and enable is confusing ;)
> >>>>>
> >>>>> Jonathan
> >>>>
> >>>>
> >>>> Jonathan, then I make these changes:
> >>>>
> >>>> powerdown-gpios: -> output-enable:
> >>> Needs to retain the gpios bit as we want the standard gpio stuff to pick
> >>> them up. I'm not that keen on output-enable-gpios though. The activity
> >>> here is very much 'shutdown because of error or not enabled' I think.
> >>> So perhaps we flip the sense and document that it needs to be active low?
> >>>
> >>>> shtdn-enable-gpios: -> enable-gpios:
> >>>>
> >>>> Is it ok?
> >>>
> >>> Conor, Rob, Krzysztof - you probably have a better insight into this than
> >>> I do.
> >>>
> >>
> >> "enable-gpios" are for turning on a specific feature, not powering
> >> on/off entire device. For example to enable regulator output.
> >>
> >> "powerdown-gpios" are for turning device on/off.
> >>
> >> I don't know what do you have in your device.
> > Ok. Sounds like that what is enable-gpios above should be shutdown-gpios.
>
> shutdown-gpios sounds exactly the same as powerdown-gpios and it is
> already used in exactly same context.
Oops. Yup. powerdown-gpios seems appropriate.
>
> > The other case is a device output indicating whether the device is
> > shutdown. That can happen because it was told to do so (via the other gpio),
> > or because it is in an error state. What's a good naming convention for that?
>
> There is no convention and I did not see such case so far.
> powerdown-status-gpios? powerdown-state-gpios?
Either seems reasonable.

Thanks,

J
>
>
>
> Best regards,
> Krzysztof
>