Re: [PATCH v2 2/4] dt-bindings: iio: dac: add adi,ad5710r.yaml

[Conor Dooley <conor@kernel.org>]

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

On Tue, Jul 28, 2026 at 09:44:50PM +0100, Jonathan Cameron wrote:
> On Tue, 28 Jul 2026 16:41:26 +0100
> Conor Dooley <conor@kernel.org> wrote:
> 
> > On Fri, Jul 24, 2026 at 10:57:38PM +0100, Jonathan Cameron wrote:
> > > On Tue, 21 Jul 2026 16:52:10 +0100
> > > Conor Dooley <conor@kernel.org> wrote:
> > >   
> > > > On Tue, Jul 21, 2026 at 04:47:11PM +0800, Kim Seer Paller wrote:  
> > > > > Add device tree bindings for the Analog Devices AD5710R/AD5711R
> > > > > 8-channel 12-/16-bit Configurable IDAC/VDAC.
> > > > > 
> > > > > Signed-off-by: Kim Seer Paller <kimseer.paller@analog.com>
> > > > > ---
> > > > >  .../devicetree/bindings/iio/dac/adi,ad5710r.yaml   | 143 +++++++++++++++++++++    
> > > >   
> > > > > +patternProperties:
> > > > > +  "^channel@[0-7]$":
> > > > > +    $ref: /schemas/iio/dac/dac.yaml#
> > > > > +    type: object
> > > > > +    description:
> > > > > +      Represents the external channels which are connected to the DAC.
> > > > > +
> > > > > +    properties:
> > > > > +      reg:
> > > > > +        description: Channel number
> > > > > +        items:
> > > > > +          minimum: 0
> > > > > +          maximum: 7
> > > > > +
> > > > > +      adi,ch-func:
> > > > > +        description:
> > > > > +          Channel output type. Use CH_FUNC_VOLTAGE_OUTPUT for voltage
> > > > > +          output or CH_FUNC_CURRENT_OUTPUT for current output.
> > > > > +        $ref: /schemas/types.yaml#/definitions/uint32
> > > > > +        enum: [1, 2]    
> > > > 
> > > > Please make this property a string.  
> > > 
> > > It's an existing binding.  I guess it is generic enough that perhaps we
> > > should make it a generic channel property and as part of that we can
> > > flip over to a string.  
> > 
> > Ah, if it is something already established then it can stay as-is.
> > 
> > > We may need to create a top level channels.yaml to cover both ADC and DAC
> > > channels though and reference that from both files.  
> > 
> > I mean yeah, that was one thought I had originally about the
> > spi-device-address property. I'm surprised there's already not more
> > commonality to begin with.
> > 
> > Just how common is some sort of "channel function" property and how
> > many devices are bastardising other properties like diff-channel with a
> > virtual channel to achieve the same result?
> 
> I don't think it is that common to mix different things that can be
> read on the same pin beyond a simple ADC / GPIO that we tend to
> handle by just not having the ADC channel specified.  Still there
> is clearly a non zero number of devices that do.
> 
> I can't immediately recall the virtual channel trick you suggest
> being used except when it is routing from a different place
> in a SoC which to me is a different input via a mux, rather than
> mode of the channel. 

It may be my lack of understanding of the devices, I just recall there
being use made of diff-channels to map to different internal references,
which felt like the kind of thing that someone might see a generic
"channel mode" property and use to achieve that goal instead.
I think the current approach using diff-channels is fine, even if it
isn't a real "channel" represented by a pin, I just don't want to see
two approaches for the same result (not that this particular binding is
doing that at all)

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