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

[Janani Sunil <jan.sun97@gmail.com>]

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@analog.com> 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@analog.com>
>> ---
>>   .../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