[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 <fr0st61te@gmail.com>
---
 .../bindings/iio/adc/maxim,max34408.yaml      | 141 ++++++++++++++++++
 1 file changed, 141 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/adc/maxim,max34408.yaml
  

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 <fr0st61te@gmail.com>
> ---
>  .../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 <fr0st61te@gmail.com>
> +
> +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
>
  

On Sun,  8 Oct 2023 02:48:37 +0300
Ivan Mikhaylov <fr0st61te@gmail.com> wrote:

> The hardware binding for i2c current monitoring device with overcurrent
> control.
> 
> Signed-off-by: Ivan Mikhaylov <fr0st61te@gmail.com>
> ---
>  .../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 <fr0st61te@gmail.com>
> +
> +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>;
> +             };
> +        };
> +    };
  

On Tue, 2023-10-10 at 15:40 +0100, Jonathan Cameron wrote:
> On Sun,  8 Oct 2023 02:48:37 +0300
> Ivan Mikhaylov <fr0st61te@gmail.com> wrote:
> 
> > The hardware binding for i2c current monitoring device with
> > overcurrent
> > control.
> > 
> > Signed-off-by: Ivan Mikhaylov <fr0st61te@gmail.com>
> > ---
> >  .../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 <fr0st61te@gmail.com>
> > +
> > +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>;
> > +             };
> > +        };
> > +    };
>
  

On Tue, 10 Oct 2023 23:22:48 +0300
Ivan Mikhaylov <fr0st61te@gmail.com> wrote:

> On Tue, 2023-10-10 at 15:40 +0100, Jonathan Cameron wrote:
> > On Sun,  8 Oct 2023 02:48:37 +0300
> > Ivan Mikhaylov <fr0st61te@gmail.com> wrote:
> >   
> > > The hardware binding for i2c current monitoring device with
> > > overcurrent
> > > control.
> > > 
> > > Signed-off-by: Ivan Mikhaylov <fr0st61te@gmail.com>
> > > ---
> > >  .../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 <fr0st61te@gmail.com>
> > > +
> > > +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>;
> > > +             };
> > > +        };
> > > +    };  
> >   
>
  

On Thu, 2023-10-12 at 08:40 +0100, Jonathan Cameron wrote:
> On Tue, 10 Oct 2023 23:22:48 +0300
> Ivan Mikhaylov <fr0st61te@gmail.com> wrote:
> 
> > On Tue, 2023-10-10 at 15:40 +0100, Jonathan Cameron wrote:
> > > On Sun,  8 Oct 2023 02:48:37 +0300
> > > Ivan Mikhaylov <fr0st61te@gmail.com> wrote:
> > >   
> > > > The hardware binding for i2c current monitoring device with
> > > > overcurrent
> > > > control.
> > > > 
> > > > Signed-off-by: Ivan Mikhaylov <fr0st61te@gmail.com>
> > > > ---
> > > >  .../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 <fr0st61te@gmail.com>
> > > > +
> > > > +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.
  

On 13/10/2023 10:19, Jonathan Cameron wrote:
> On Thu, 12 Oct 2023 19:27:33 +0300
> Ivan Mikhaylov <fr0st61te@gmail.com> wrote:
> 
>> On Thu, 2023-10-12 at 08:40 +0100, Jonathan Cameron wrote:
>>> On Tue, 10 Oct 2023 23:22:48 +0300
>>> Ivan Mikhaylov <fr0st61te@gmail.com> wrote:
>>>   
>>>> On Tue, 2023-10-10 at 15:40 +0100, Jonathan Cameron wrote:  
>>>>> On Sun,  8 Oct 2023 02:48:37 +0300
>>>>> Ivan Mikhaylov <fr0st61te@gmail.com> wrote:
>>>>>     
>>>>>> The hardware binding for i2c current monitoring device with
>>>>>> overcurrent
>>>>>> control.
>>>>>>
>>>>>> Signed-off-by: Ivan Mikhaylov <fr0st61te@gmail.com>
>>>>>> ---
>>>>>>  .../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 <fr0st61te@gmail.com>
>>>>>> +
>>>>>> +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
  

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
  

On Fri, 13 Oct 2023 11:53:33 +0200
Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> 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
>