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

[Conor Dooley <conor@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 4895 bytes --]

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]