Re: [PATCH v10 1/2] dt-bindings: iio: adc: add max14001

[Conor Dooley <conor@kernel.org>]

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

On Tue, Sep 02, 2025 at 09:29:04AM -0500, David Lechner wrote:
> On 9/2/25 8:15 AM, Marilene Andrade Garcia wrote:
> > Add device-tree documentation for MAX14001/MAX14002 ADCs.
> > The MAX14001/MAX14002 are isolated, single-channel analog-to-digital
> > converters with programmable voltage comparators and inrush current
> > control optimized for configurable binary input applications.
> 
> When there are multiple devices, DT maintainers like to know
> what is the difference between the devices.

Looking at the driver, I don't really buy that there even is a
meaningful one, at least at first glance. What even is different between
them other than the name?

> 
> > 
> > Co-developed-by: Marilene Andrade Garcia <marilene.agarcia@gmail.com>
> > Signed-off-by: Marilene Andrade Garcia <marilene.agarcia@gmail.com>
> > Signed-off-by: Kim Seer Paller <kimseer.paller@analog.com>
> 
> Sine the patch is From: M.A.G., according to [1], this should be:
> 
> Co-developed-by: K.S.P.
> Signed-off-by: K.S.P.
> Signed-off-by: M.A.G.
> 
> (hopefully obvious, but don't use the abbreviations - I just did
> that for brevity)

Prob also worth putting a note under the --- line as to why the r-b from
Krzysztof got dropped that was on v9.

> 
> [1]: https://www.kernel.org/doc/html/latest/process/submitting-patches.html#when-to-use-acked-by-cc-and-co-developed-by
> 
> > ---
> >  .../bindings/iio/adc/adi,max14001.yaml        | 79 +++++++++++++++++++
> >  MAINTAINERS                                   |  8 ++
> >  2 files changed, 87 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/iio/adc/adi,max14001.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/iio/adc/adi,max14001.yaml b/Documentation/devicetree/bindings/iio/adc/adi,max14001.yaml
> > new file mode 100644
> > index 000000000000..ff9a41f04300
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/iio/adc/adi,max14001.yaml
> > @@ -0,0 +1,79 @@
> > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > +# Copyright 2023-2025 Analog Devices Inc.
> > +# Copyright 2023 Kim Seer Paller
> > +# Copyright 2025 Marilene Andrade Garcia
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/iio/adc/adi,max14001.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Analog Devices MAX14001-MAX14002 ADC
> > +
> > +maintainers:
> > +  - Kim Seer Paller <kimseer.paller@analog.com>
> > +  - Marilene Andrade Garcia <marilene.agarcia@gmail.com>
> > +
> > +description: |
> > +    Single channel 10 bit ADC with SPI interface.
> > +    Datasheet can be found here
> > +      https://www.analog.com/media/en/technical-documentation/data-sheets/MAX14001-MAX14002.pdf
> > +
> > +$ref: /schemas/spi/spi-peripheral-props.yaml#
> > +
> > +properties:
> > +  compatible:
> > +    enum:
> > +      - adi,max14001
> > +      - adi,max14002
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  spi-max-frequency:
> > +    maximum: 5000000
> > +
> > +  vdd-supply:
> > +    description:
> > +      Isolated DC-DC power supply input voltage.
> > +
> > +  vddl-supply:
> > +    description:
> > +      Logic power supply.
> > +
> > +  vrefin-supply:
> 
> The actual pin name is REFIN, so refin-supply would make more sense.
> 
> > +    description:
> > +      ADC voltage reference supply.
> > +
> > +  interrupts:
> 
> Likely needs `minItems: 1` in case only one interrupt is wired.
> 
> > +    items:
> > +      - description: |
> > +          Interrupt for signaling when conversion results exceed the configured
> > +          upper threshold for ADC readings or fall below the lower threshold for
> > +          them. Interrupt source must be attached to COUT pin.
> 
> We could shorten these descriptions. The important part is which pin
> it is connected to.
> 
> > +      - description: |
> > +          Alert output that asserts low during a number of different error
> > +          conditions. The interrupt source must be attached to FAULT pin.
> > +
> 
> And also `interrupt-names:` makes sense so we know which one is
> is wired if only one is given.

Additionally, v9 had no interrupts at all but only serviced once device.
Do both devices have the interrupts or should these only be permitted on
the 140002?

> 
> > +required:
> > +  - compatible
> > +  - reg
> > +  - vdd-supply
> > +  - vddl-supply
> > +

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