Re: [PATCH v2 3/4] dt-bindings: iio: dac: Add adi,ad3530r.yaml

[Jonathan Cameron <jic23@kernel.org>]

On Mon, 31 Mar 2025 01:31:10 +0000
"Paller, Kim Seer" <KimSeer.Paller@analog.com> wrote:

> > -----Original Message-----
> > From: Jonathan Cameron <jic23@kernel.org>
> > Sent: Friday, March 28, 2025 5:04 PM
> > To: Paller, Kim Seer <KimSeer.Paller@analog.com>
> > Cc: Lars-Peter Clausen <lars@metafoo.de>; Hennerich, Michael
> > <Michael.Hennerich@analog.com>; Rob Herring <robh@kernel.org>; Krzysztof
> > Kozlowski <krzk+dt@kernel.org>; Conor Dooley <conor+dt@kernel.org>; linux-
> > iio@vger.kernel.org; linux-kernel@vger.kernel.org; devicetree@vger.kernel.org
> > Subject: Re: [PATCH v2 3/4] dt-bindings: iio: dac: Add adi,ad3530r.yaml
> > 
> > [External]
> > 
> > On Mon, 24 Mar 2025 19:22:57 +0800
> > Kim Seer Paller <kimseer.paller@analog.com> wrote:
> >   
> > > Document the AD3530R/AD3530, an 8-Channel, 16-bit Voltage Output DAC,
> > > while the AD3531R/AD3531 is a 4-Channel, 16-Bit Voltage Output DAC.
> > > These devices include software-programmable gain controls that provide
> > > full-scale output spans of 2.5V or 5V for reference voltages of 2.5V.
> > > They operate from a single supply voltage range of 2.7V to 5.5V and are
> > > guaranteed to be monotonic by design. Additionally, these devices
> > > features a 2.5V, 5ppm/°C internal reference, which is disabled by default.
> > >
> > > Signed-off-by: Kim Seer Paller <kimseer.paller@analog.com>
> > > ---
> > >  .../devicetree/bindings/iio/dac/adi,ad3530r.yaml   | 91  
> > ++++++++++++++++++++++  
> > >  MAINTAINERS                                        |  1 +
> > >  2 files changed, 92 insertions(+)
> > >
> > > diff --git a/Documentation/devicetree/bindings/iio/dac/adi,ad3530r.yaml  
> > b/Documentation/devicetree/bindings/iio/dac/adi,ad3530r.yaml  
> > > new file mode 100644
> > > index  
> > 0000000000000000000000000000000000000000..e581472b50048bedda742
> > 2748035423b9b020382  
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/iio/dac/adi,ad3530r.yaml
> > > @@ -0,0 +1,91 @@
> > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > +%YAML 1.2
> > > +---
> > > +$id:  
> > https://urldefense.com/v3/__http://devicetree.org/schemas/iio/dac/adi,ad353
> > 0r.yaml*__;Iw!!A3Ni8CS0y2Y!-bEqh-bKz-a-
> > ZzfbJZwXi9KWwKlsk_Pcaj5XQLeLaDBpO8MaryRedRHaL0GUFfNz35tDsFaJkV45P
> > fyEzA$  
> > > +$schema: https://urldefense.com/v3/__http://devicetree.org/meta-  
> > schemas/core.yaml*__;Iw!!A3Ni8CS0y2Y!-bEqh-bKz-a-
> > ZzfbJZwXi9KWwKlsk_Pcaj5XQLeLaDBpO8MaryRedRHaL0GUFfNz35tDsFaJkV5zI4
> > zMaQ$  
> > > +
> > > +title: Analog Devices AD3530R and Similar DACs
> > > +
> > > +maintainers:
> > > +  - Kim Seer Paller <kimseer.paller@analog.com>
> > > +
> > > +description: |
> > > +  The AD3530/AD3530R are low power, 8-channel, 16-bit, buffered voltage  
> > output,  
> > > +  digital-to-analog converters (DACs) that include software-programmable  
> > gain  
> > > +  controls that result in full-scale output spans of 2.5V or 5V for reference
> > > +  voltages of 2.5V. The devices operate from single, 2.7V to 5.5V supply  
> > ranges  
> > > +  and are guaranteed monotonic by design. The AD3530R also offers a 2.5V,
> > > +  5ppm/°C internal reference that is disabled by default.
> > > +  Datasheet can be found here:
> > > +  https://www.analog.com/media/en/technical-documentation/data-  
> > sheets/ad3530_ad530r.pdf  
> > > +
> > > +properties:
> > > +  compatible:
> > > +    enum:
> > > +      - adi,ad3530r  
> > 
> > You mention this one as well as a variant without the r postfix in the
> > 'description'.
> > So why not compatible for that?  If it's software compatible with the r version
> > than
> > a fallback compatible makes sense.  We probably still want to have separate
> > compatibles though in case we get an errata that only applies to one of them.
> > 
> > If they are the same silicon, perhaps with different ratings then make that clear
> > in the description and perhaps it is fine to not have both compatibles listed.  
> 
> The main difference is that the r variant supports both internal and external references,
> while the non-r variant supports only external references. I agree with having separate
> compatibles. Well, I'm thinking of adding a parameter in chip_info into the driver to
> identify internal reference support. What do you think?
Yes, combination of separate compatibles and a parameter in chip_info to make the
code simple sounds like the right solution to me.

Jonathan

> 
> >   
> > > +      - adi,ad3531r  
> > 
> > This isn't mentioned in the description text.
> >   
> > > +
> > > +  reg:
> > > +    maxItems: 1  
> > 
> > Thanks,
> > 
> > Jonathan