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

Ivan Mikhaylov posted 2 patches 2 years, 2 months ago
There is a newer version of this series
[PATCH v5 1/2] dt-bindings: adc: provide max34408/9 device tree binding document
Posted by Ivan Mikhaylov 2 years, 2 months ago
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:
+    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
Re: [PATCH v5 1/2] dt-bindings: adc: provide max34408/9 device tree binding document
Posted by Jonathan Cameron 2 years, 2 months ago
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>;
> +             };
> +        };
> +    };
Re: [PATCH v5 1/2] dt-bindings: adc: provide max34408/9 device tree binding document
Posted by Ivan Mikhaylov 2 years, 2 months ago
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>;
> > +             };
> > +        };
> > +    };
> 

Re: [PATCH v5 1/2] dt-bindings: adc: provide max34408/9 device tree binding document
Posted by Jonathan Cameron 2 years, 2 months ago
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>;
> > > +             };
> > > +        };
> > > +    };  
> >   
> 
Re: [PATCH v5 1/2] dt-bindings: adc: provide max34408/9 device tree binding document
Posted by Ivan Mikhaylov 2 years, 2 months ago
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.
Re: [PATCH v5 1/2] dt-bindings: adc: provide max34408/9 device tree binding document
Posted by Jonathan Cameron 2 years, 2 months ago
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.

Thanks,

Jonathan

> 
> Thanks.
> 
Re: [PATCH v5 1/2] dt-bindings: adc: provide max34408/9 device tree binding document
Posted by Krzysztof Kozlowski 2 years, 2 months ago
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

Re: [PATCH v5 1/2] dt-bindings: adc: provide max34408/9 device tree binding document
Posted by Jonathan Cameron 2 years, 2 months ago
On Fri, 13 Oct 2023 10:35:49 +0200
Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:

> 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.
Ok. Sounds like that what is enable-gpios above should be shutdown-gpios.

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?

Thanks,

Jonathan

> 
> Best regards,
> Krzysztof
> 
> 
Re: [PATCH v5 1/2] dt-bindings: adc: provide max34408/9 device tree binding document
Posted by Krzysztof Kozlowski 2 years, 2 months ago
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

Re: [PATCH v5 1/2] dt-bindings: adc: provide max34408/9 device tree binding document
Posted by Jonathan Cameron 2 years, 2 months ago
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
> 
Re: [PATCH v5 1/2] dt-bindings: adc: provide max34408/9 device tree binding document
Posted by Rob Herring 2 years, 2 months ago
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
>