2023-09-29 21:15:21

by Ivan Mikhaylov

[permalink] [raw]
Subject: [PATCH v2 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 | 101 ++++++++++++++++++
1 file changed, 101 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..cdf89fa4c80e
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/maxim,max34408.yaml
@@ -0,0 +1,101 @@
+# 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
+
+ reg:
+ maxItems: 1
+
+ interrupts:
+ maxItems: 1
+
+ maxim,input1-rsense-val-micro-ohms:
+ description:
+ Adjust the Rsense value to monitor higher or lower current levels for
+ input 1.
+ enum: [250, 500, 1000, 5000, 10000, 50000, 100000, 200000, 500000]
+ default: 1000
+
+ maxim,input2-rsense-val-micro-ohms:
+ description:
+ Adjust the Rsense value to monitor higher or lower current levels for
+ input 2.
+ enum: [250, 500, 1000, 5000, 10000, 50000, 100000, 200000, 500000]
+ default: 1000
+
+ maxim,input3-rsense-val-micro-ohms:
+ description:
+ Adjust the Rsense value to monitor higher or lower current levels for
+ input 3.
+ enum: [250, 500, 1000, 5000, 10000, 50000, 100000, 200000, 500000]
+ default: 1000
+
+ maxim,input4-rsense-val-micro-ohms:
+ description:
+ Adjust the Rsense value to monitor higher or lower current levels for
+ input 4.
+ enum: [250, 500, 1000, 5000, 10000, 50000, 100000, 200000, 500000]
+ default: 1000
+
+ maxim,shtdn:
+ 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.
+ type: boolean
+
+ maxim,ena:
+ 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.
+ type: boolean
+
+ supply-vdd: true
+
+required:
+ - compatible
+ - reg
+
+additionalProperties: false
+
+examples:
+ - |
+ i2c {
+
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ adc@1e {
+ compatible = "maxim,max34409";
+ reg = <0x1e>;
+ maxim,input1-rsense-val-micro-ohms = <5000>;
+ maxim,input2-rsense-val-micro-ohms = <10000>;
+ };
+ };
--
2.42.0


2023-09-29 22:42:40

by Rob Herring

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


On Fri, 29 Sep 2023 23:08:43 +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 | 101 ++++++++++++++++++
> 1 file changed, 101 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/iio/adc/maxim,max34408.yaml
>

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/iio/adc/maxim,max34408.yaml: supply-vdd: missing type definition

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/[email protected]

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.

2023-09-30 09:38:27

by Conor Dooley

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

Hey,

On Fri, Sep 29, 2023 at 11:08:43PM +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 | 101 ++++++++++++++++++
> 1 file changed, 101 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..cdf89fa4c80e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/maxim,max34408.yaml
> @@ -0,0 +1,101 @@
> +# 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
> +
> + reg:
> + maxItems: 1
> +
> + interrupts:
> + maxItems: 1
> +
> + maxim,input1-rsense-val-micro-ohms:
> + description:
> + Adjust the Rsense value to monitor higher or lower current levels for
> + input 1.
> + enum: [250, 500, 1000, 5000, 10000, 50000, 100000, 200000, 500000]
> + default: 1000
> +
> + maxim,input2-rsense-val-micro-ohms:
> + description:
> + Adjust the Rsense value to monitor higher or lower current levels for
> + input 2.
> + enum: [250, 500, 1000, 5000, 10000, 50000, 100000, 200000, 500000]
> + default: 1000
> +
> + maxim,input3-rsense-val-micro-ohms:
> + description:
> + Adjust the Rsense value to monitor higher or lower current levels for
> + input 3.
> + enum: [250, 500, 1000, 5000, 10000, 50000, 100000, 200000, 500000]
> + default: 1000
> +
> + maxim,input4-rsense-val-micro-ohms:
> + description:
> + Adjust the Rsense value to monitor higher or lower current levels for
> + input 4.
> + enum: [250, 500, 1000, 5000, 10000, 50000, 100000, 200000, 500000]
> + default: 1000

Having 4 almost identical properties makes it seem like this should have
some channel nodes, each containing an rsense-micro-ohms type property.

> +
> + maxim,shtdn:
> + 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.
> + type: boolean

I don't understand what this property is used for. The description here,
and below for "ena", read like they are the descriptions in the
datasheet for the pin, rather than how to use the property.

The drivers don't appear to contain users either - what is the point of
these properties?

> +
> + maxim,ena:
> + 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.
> + type: boolean
> +
> + supply-vdd: true

As pointed out by the bot, this is not correct. You need to use a
-supply affix, not a supply-prefix.

Thanks,
Conor.

> +
> +required:
> + - compatible
> + - reg
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + i2c {
> +
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + adc@1e {
> + compatible = "maxim,max34409";
> + reg = <0x1e>;
> + maxim,input1-rsense-val-micro-ohms = <5000>;
> + maxim,input2-rsense-val-micro-ohms = <10000>;
> + };
> + };
> --
> 2.42.0
>


Attachments:
(No filename) (4.78 kB)
signature.asc (235.00 B)
Download all attachments

2023-09-30 14:09:33

by Jonathan Cameron

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

On Sat, 30 Sep 2023 10:37:09 +0100
Conor Dooley <[email protected]> wrote:

> Hey,
>
> On Fri, Sep 29, 2023 at 11:08:43PM +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 | 101 ++++++++++++++++++
> > 1 file changed, 101 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..cdf89fa4c80e
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/iio/adc/maxim,max34408.yaml
> > @@ -0,0 +1,101 @@
> > +# 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
> > +
> > + reg:
> > + maxItems: 1
> > +
> > + interrupts:
> > + maxItems: 1
> > +
> > + maxim,input1-rsense-val-micro-ohms:
> > + description:
> > + Adjust the Rsense value to monitor higher or lower current levels for
> > + input 1.
> > + enum: [250, 500, 1000, 5000, 10000, 50000, 100000, 200000, 500000]
> > + default: 1000
> > +
> > + maxim,input2-rsense-val-micro-ohms:
> > + description:
> > + Adjust the Rsense value to monitor higher or lower current levels for
> > + input 2.
> > + enum: [250, 500, 1000, 5000, 10000, 50000, 100000, 200000, 500000]
> > + default: 1000
> > +
> > + maxim,input3-rsense-val-micro-ohms:
> > + description:
> > + Adjust the Rsense value to monitor higher or lower current levels for
> > + input 3.
> > + enum: [250, 500, 1000, 5000, 10000, 50000, 100000, 200000, 500000]
> > + default: 1000
> > +
> > + maxim,input4-rsense-val-micro-ohms:
> > + description:
> > + Adjust the Rsense value to monitor higher or lower current levels for
> > + input 4.
> > + enum: [250, 500, 1000, 5000, 10000, 50000, 100000, 200000, 500000]
> > + default: 1000
>
> Having 4 almost identical properties makes it seem like this should have
> some channel nodes, each containing an rsense-micro-ohms type property.
Agreed. That is most flexible route if there is any chance of ending up
with more channel specific stuff in future.

There should also be some magic in here to rule out the input3 and input4
for the devices with only two channels. (lots of examples in tree)

Otherwise, in theory this could be an array I guess, but I'd also prefer
channel nodes.

>
> > +
> > + maxim,shtdn:
> > + 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.
> > + type: boolean
>
> I don't understand what this property is used for. The description here,
> and below for "ena", read like they are the descriptions in the
> datasheet for the pin, rather than how to use the property.
>
> The drivers don't appear to contain users either - what is the point of
> these properties?
>
> > +
> > + maxim,ena:
> > + 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.
> > + type: boolean
> > +
> > + supply-vdd: true
>
> As pointed out by the bot, this is not correct. You need to use a
> -supply affix, not a supply-prefix.
My error in earlier review (not enough coffee that day I guess :)

Anyhow it does show that the tests weren't run which isn't a good thing to see.

Jonathan

>
> Thanks,
> Conor.
>
> > +
> > +required:
> > + - compatible
> > + - reg
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > + - |
> > + i2c {
> > +
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > +
> > + adc@1e {
> > + compatible = "maxim,max34409";
> > + reg = <0x1e>;
> > + maxim,input1-rsense-val-micro-ohms = <5000>;
> > + maxim,input2-rsense-val-micro-ohms = <10000>;
> > + };
> > + };
> > --
> > 2.42.0
> >

2023-09-30 22:47:46

by Ivan Mikhaylov

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

On Sat, 2023-09-30 at 10:37 +0100, Conor Dooley wrote:
> Hey,
>
> On Fri, Sep 29, 2023 at 11:08:43PM +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      | 101
> > ++++++++++++++++++
> >  1 file changed, 101 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..cdf89fa4c80e
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/iio/adc/maxim,max34408.yaml
> > @@ -0,0 +1,101 @@
> > +# 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
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  interrupts:
> > +    maxItems: 1
> > +
> > +  maxim,input1-rsense-val-micro-ohms:
> > +    description:
> > +      Adjust the Rsense value to monitor higher or lower current
> > levels for
> > +      input 1.
> > +    enum: [250, 500, 1000, 5000, 10000, 50000, 100000, 200000,
> > 500000]
> > +    default: 1000
> > +
> > +  maxim,input2-rsense-val-micro-ohms:
> > +    description:
> > +      Adjust the Rsense value to monitor higher or lower current
> > levels for
> > +      input 2.
> > +    enum: [250, 500, 1000, 5000, 10000, 50000, 100000, 200000,
> > 500000]
> > +    default: 1000
> > +
> > +  maxim,input3-rsense-val-micro-ohms:
> > +    description:
> > +      Adjust the Rsense value to monitor higher or lower current
> > levels for
> > +      input 3.
> > +    enum: [250, 500, 1000, 5000, 10000, 50000, 100000, 200000,
> > 500000]
> > +    default: 1000
> > +
> > +  maxim,input4-rsense-val-micro-ohms:
> > +    description:
> > +      Adjust the Rsense value to monitor higher or lower current
> > levels for
> > +      input 4.
> > +    enum: [250, 500, 1000, 5000, 10000, 50000, 100000, 200000,
> > 500000]
> > +    default: 1000
>
> Having 4 almost identical properties makes it seem like this should
> have
> some channel nodes, each containing an rsense-micro-ohms type
> property.

Conor, I'll look through.

>
> > +
> > +  maxim,shtdn:
> > +    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.
> > +    type: boolean
>
> I don't understand what this property is used for. The description
> here,
> and below for "ena", read like they are the descriptions in the
> datasheet for the pin, rather than how to use the property.
>
> The drivers don't appear to contain users either - what is the point
> of
> these properties?

ena and shtdn physical pins of hardware, in the previous version
Jonathan asked about adding them into yaml even if it's not used in
code. should I do it in some other way?

>
> > +
> > +  maxim,ena:
> > +    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.
> > +    type: boolean
> > +
> > +  supply-vdd: true
>
> As pointed out by the bot, this is not correct. You need to use a
> -supply affix, not a supply-prefix.

Oops.

Thanks.