Re: [PATCH 1/3] dt-bindings: iio: dac: Add max22007

[Janani Sunil]

Hi Johnathan,

Thank you for your reviewing the patch.

On 12/19/25 17:40, Jonathan Cameron wrote:
> On Fri, 19 Dec 2025 16:31:15 +0100
> Janani Sunil <janani.sunil@xxxxxxxxxx> wrote:
>
> Hi Janani and welcome to IIO.
>
>
> > Devicetree bindings for MAX22007 4-channel
> > 12-bit DAC that drives a voltage or current
> > output on each channel
> This is a very short wrap. Aim for 75 characters in patch description lines
> (so slightly shorter than 80 chars standard for everything else).

Noted your point. Will update the message.

> > Signed-off-by: Janani Sunil <janani.sunil@xxxxxxxxxx>
> > ---
> >   .../devicetree/bindings/iio/dac/adi,max22007.yaml  | 116 +++++++++++++++++++++
> >   MAINTAINERS                                        |   7 ++
> >   2 files changed, 123 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/iio/dac/adi,max22007.yaml b/Documentation/devicetree/bindings/iio/dac/adi,max22007.yaml
> > new file mode 100644
> > index 000000000000..c2f65d9e42d4
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/iio/dac/adi,max22007.yaml
> ...
>
> > +  vdd-supply:
> > +    description: Low-Voltage Power Supply from +2.7V to +5.5V.
> > +
> > +  hvdd-supply:
> > +    description:
> > +      Positive High-Voltage Power Supply from +8V to (HVSS +24V) for
> > +      the Output Channels.
> > +
> > +  hvss-supply:
> > +    description:
> > +      Negative High-Voltage Power Supply from -2V to 0V for the Output Channels.
> > +
> > +  reset-gpios:
> > +    maxItems: 1
> > +    description:
> > +      GPIO used for hardware reset of the device.
> > +
> > +patternProperties:
> > +  "^channel@[0-3]$":
> > +    allOf:
> > +      - $ref: /schemas/iio/dac/dac.yaml#
> > +      - type: object
> > +        description:
> > +          Represents the external channels which are connected to the DAC.
> > +          Channels not specified in the device tree will be powered off.
> > +
> > +        properties:
> > +          reg:
> > +            description: Channel number
> > +            maxItems: 1
> min / max?

Shall add the values.

> > +
> > +          adi,type:
> > +            description: Channel output type.
> > +            $ref: /schemas/types.yaml#/definitions/string
> > +            enum: [voltage, current]
> This is much more constrained (as only two types of channel) but we do have
> precedence for adi,ch-func in adi,ad74115.yaml and adi,ad74413r.yaml
>
> That's not a particularly pretty binding but we should probably stick to
> it anyway.
>
> adi,type is too vague a name for this property anyway.

Agreed. adi,ch-func seems to be a best fit here. Shall reuse it.

> > +
> > +        required:
> > +          - reg
> > +          - adi,type
> > +
> > +        unevaluatedProperties: false
> > +
> > +required:
> > +  - compatible
> For supplies we document as required any that are needed for the device
> to function, whether or not we happen to need to specify them on
> a given board (given fallbacks that apply on assumption that fixes
> always on supplies are in use).  So I'd expect to see at least some
> of the supplies listed here.

Noted. I shall update the required power supplies.

> > +  - reg
> > +
> > +anyOf:
> > +  - required: [channel@0]
> > +  - required: [channel@1]
> > +  - required: [channel@2]
> > +  - required: [channel@3]
> Interesting. I'm not sure we have never bothered to document this before and there
> are other devices for which some sort of channel config is pretty much needed.
> What is the justification to have this explicitly listed here?

There is no need for the channels to be explicitly mentioned here. I Shall drop them.

> > +
> > +unevaluatedProperties: false
> > +
> > +examples:
> > +  - |
> > +    #include <dt-bindings/gpio/gpio.h>
> > +    spi {
> > +        #address-cells = <1>;
> > +        #size-cells = <0>;
> > +
> > +        dac@0 {
> > +            compatible = "adi,max22007";
> > +            reg = <0>;
> > +            spi-max-frequency = <500000>;
> > +            reset-gpios = <&gpio 19 GPIO_ACTIVE_LOW>;
> > +            #address-cells = <1>;
> > +            #size-cells = <0>;
> > +
> > +            channel@0 {
> > +                reg = <0>;
> > +                adi,type = "voltage";
> > +            };
> > +
> > +            channel@1 {
> > +                reg = <1>;
> > +                adi,type = "current";
> > +            };
> > +        };
> > +    };
> > +...

Thank you,
Janani Sunil