diff mbox series

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

Message ID 20230917211143.7094-2-fr0st61te@gmail.com
State New
Headers
Series Add maxim max34408/34409 ADC driver and dts |

Commit Message

Ivan Mikhaylov Sept. 17, 2023, 9:11 p.m. UTC 
  The i2c driver with Rsense option for current monitoring.

Signed-off-by: Ivan Mikhaylov <fr0st61te@gmail.com>
---
 .../bindings/iio/adc/maxim,max34408.yaml      | 63 +++++++++++++++++++
 1 file changed, 63 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/adc/maxim,max34408.yaml

Comments

Rob Herring Sept. 18, 2023, 9:14 p.m. UTC | #1 
On Mon, Sep 18, 2023 at 12:11:42AM +0300, Ivan Mikhaylov wrote:
> The i2c driver with Rsense option for current monitoring.

driver? This is a binding for a hardware device.

> 
> Signed-off-by: Ivan Mikhaylov <fr0st61te@gmail.com>
> ---
>  .../bindings/iio/adc/maxim,max34408.yaml      | 63 +++++++++++++++++++
>  1 file changed, 63 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..ae7c6ddb13d8
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/maxim,max34408.yaml
> @@ -0,0 +1,63 @@
> +# 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.

Drop the period.

With those fixes:

Reviewed-by: Rob Herring <robh@kernel.org>
Jonathan Cameron Sept. 24, 2023, 12:53 p.m. UTC | #2 
On Mon, 18 Sep 2023 00:11:42 +0300
Ivan Mikhaylov <fr0st61te@gmail.com> wrote:

> The i2c driver with Rsense option for current monitoring.
> 
> Signed-off-by: Ivan Mikhaylov <fr0st61te@gmail.com>

Hi Ivan,

Welcome to IIO!

Looks good, but there are a few things I'd add to make this describe the device
a little more fully and flexibly.  Ideally we want a binding to fully describe
a device, even if the particular driver for Linux doesn't use all the features.
Some are easy though such as enabling regulators (that are probably turned on
already on your board)

Thanks,

Jonathan

> ---
>  .../bindings/iio/adc/maxim,max34408.yaml      | 63 +++++++++++++++++++
>  1 file changed, 63 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..ae7c6ddb13d8
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/maxim,max34408.yaml
> @@ -0,0 +1,63 @@
> +# 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
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  maxim,rsense-val-micro-ohms:
From the datasheet you link, it looks like this could be different for
the inputs?

> +    description:
> +      Adjust the Rsense value to monitor higher or lower current levels.
> +    enum: [250, 500, 1000, 5000, 10000, 50000, 100000, 200000, 500000]

These come from Table 18 which is example values I think?  Not sure there
is anything limiting us to those particular values given the equation given
just above that table should apply more generally.

> +    default: 1000

Please add regulator definitions.

supply-vdd: true
and add it to the required properties. It might be provided by a stub regulator
but we still list that as required.

Also good to add bindings for the other control pins that might be wired to be
in the binding from the start - no need for the driver to use them though.
Looks like we have SHTDN and ENA here that could be wired to GPIOs on the host.

> +
> +required:
> +  - compatible
> +  - reg
> +  - maxim,rsense-val-micro-ohms
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    i2c {
> +
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        adc@1e {
> +              compatible = "maxim,max34409";
> +              reg = <0x1e>;
> +              maxim,rsense-val-micro-ohms = <1000>;
> +        };
> +    };
Ivan Mikhaylov Sept. 25, 2023, 4:48 p.m. UTC | #3 
On Sun, 2023-09-24 at 13:53 +0100, Jonathan Cameron wrote:
> On Mon, 18 Sep 2023 00:11:42 +0300
> Ivan Mikhaylov <fr0st61te@gmail.com> wrote:
> 
> > The i2c driver with Rsense option for current monitoring.
> > 
> > Signed-off-by: Ivan Mikhaylov <fr0st61te@gmail.com>
> 
> Hi Ivan,
> 
> Welcome to IIO!
> 
> Looks good, but there are a few things I'd add to make this describe
> the device
> a little more fully and flexibly.  Ideally we want a binding to fully
> describe
> a device, even if the particular driver for Linux doesn't use all the
> features.
> Some are easy though such as enabling regulators (that are probably
> turned on
> already on your board)
> 
> Thanks,
> 
> Jonathan
> 
> > ---
> >  .../bindings/iio/adc/maxim,max34408.yaml      | 63
> > +++++++++++++++++++
> >  1 file changed, 63 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..ae7c6ddb13d8
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/iio/adc/maxim,max34408.yaml
> > @@ -0,0 +1,63 @@
> > +# 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
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  interrupts:
> > +    maxItems: 1
> > +
> > +  maxim,rsense-val-micro-ohms:
> From the datasheet you link, it looks like this could be different
> for
> the inputs?

Hi Jonathan, "maxim,input1-rsense-val-micro-ohms", "maxim,input2-
rsense-val-micro-ohms" and etc would be better?

> 
> > +    description:
> > +      Adjust the Rsense value to monitor higher or lower current
> > levels.
> > +    enum: [250, 500, 1000, 5000, 10000, 50000, 100000, 200000,
> > 500000]
> 
> These come from Table 18 which is example values I think?  Not sure
> there
> is anything limiting us to those particular values given the equation
> given
> just above that table should apply more generally.
> 
> > +    default: 1000
> 
> Please add regulator definitions.
> 
> supply-vdd: true
> and add it to the required properties. It might be provided by a stub
> regulator
> but we still list that as required.
> 
> Also good to add bindings for the other control pins that might be
> wired to be
> in the binding from the start - no need for the driver to use them
> though.
> Looks like we have SHTDN and ENA here that could be wired to GPIOs on
> the host.
> 
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - maxim,rsense-val-micro-ohms
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    i2c {
> > +
> > +        #address-cells = <1>;
> > +        #size-cells = <0>;
> > +
> > +        adc@1e {
> > +              compatible = "maxim,max34409";
> > +              reg = <0x1e>;
> > +              maxim,rsense-val-micro-ohms = <1000>;
> > +        };
> > +    };
> 

Rob, Jonathan, thanks for review, will do the changes which you asked.
Jonathan Cameron Sept. 30, 2023, 4:04 p.m. UTC | #4 
On Mon, 25 Sep 2023 19:48:39 +0300
Ivan Mikhaylov <fr0st61te@gmail.com> wrote:

> On Sun, 2023-09-24 at 13:53 +0100, Jonathan Cameron wrote:
> > On Mon, 18 Sep 2023 00:11:42 +0300
> > Ivan Mikhaylov <fr0st61te@gmail.com> wrote:
> >   
> > > The i2c driver with Rsense option for current monitoring.
> > > 
> > > Signed-off-by: Ivan Mikhaylov <fr0st61te@gmail.com>  
> > 
> > Hi Ivan,
> > 
> > Welcome to IIO!
> > 
> > Looks good, but there are a few things I'd add to make this describe
> > the device
> > a little more fully and flexibly.  Ideally we want a binding to fully
> > describe
> > a device, even if the particular driver for Linux doesn't use all the
> > features.
> > Some are easy though such as enabling regulators (that are probably
> > turned on
> > already on your board)
> > 
> > Thanks,
> > 
> > Jonathan
> >   
> > > ---
> > >  .../bindings/iio/adc/maxim,max34408.yaml      | 63
> > > +++++++++++++++++++
> > >  1 file changed, 63 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..ae7c6ddb13d8
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/iio/adc/maxim,max34408.yaml
> > > @@ -0,0 +1,63 @@
> > > +# 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
> > > +
> > > +  reg:
> > > +    maxItems: 1
> > > +
> > > +  interrupts:
> > > +    maxItems: 1
> > > +
> > > +  maxim,rsense-val-micro-ohms:  
> > From the datasheet you link, it looks like this could be different
> > for
> > the inputs?  
> 
> Hi Jonathan, "maxim,input1-rsense-val-micro-ohms", "maxim,input2-
> rsense-val-micro-ohms" and etc would be better?

Sorry, missed this during the week (too many emails at work!)

Anyhow, I'd have suggested an array, but a better suggestion was
made anyway in reply to your v2.
diff mbox series

Patch

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..ae7c6ddb13d8
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/maxim,max34408.yaml
@@ -0,0 +1,63 @@ 
+# 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
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  maxim,rsense-val-micro-ohms:
+    description:
+      Adjust the Rsense value to monitor higher or lower current levels.
+    enum: [250, 500, 1000, 5000, 10000, 50000, 100000, 200000, 500000]
+    default: 1000
+
+required:
+  - compatible
+  - reg
+  - maxim,rsense-val-micro-ohms
+
+additionalProperties: false
+
+examples:
+  - |
+    i2c {
+
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        adc@1e {
+              compatible = "maxim,max34409";
+              reg = <0x1e>;
+              maxim,rsense-val-micro-ohms = <1000>;
+        };
+    };