Re: [PATCH v3 1/3] dt-bindings: iio: adc: Add AD4134

[Jonathan Cameron <jic23@kernel.org>]

On Fri, 5 Dec 2025 09:50:08 -0300
Marcelo Schmitt <marcelo.schmitt1@gmail.com> wrote:

> On 12/05, Tomas Melin wrote:
> > Hi,
> > 
> > On 02/12/2025 22:55, Marcelo Schmitt wrote:  
> > > Add device tree documentation for AD4134 24-Bit, 4-channel simultaneous
> > > sampling, precision ADC.
> > > 
> > > Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
> > > Signed-off-by: Marcelo Schmitt <marcelo.schmitt@analog.com>
> > > ---
> > > Change log v2 -> v3:
> > > - fixed typo in powerdown-gpios description.
> > > - picked up Conor's review tag. 
> > > 
> > >  .../bindings/iio/adc/adi,ad4134.yaml          | 198 ++++++++++++++++++  
> ...
> > > +description: |
> > > +  The AD4134 is a quad channel, low noise, simultaneous sampling, precision
> > > +  analog-to-digital converter (ADC).
> > > +  Specifications can be found at:
> > > +    https://www.analog.com/media/en/technical-documentation/data-sheets/ad4134.pdf
> > > +
> > > +$ref: /schemas/spi/spi-peripheral-props.yaml#
> > > +
> > > +properties:
> > > +    maxItems: 1
> > > +
> > > +  adi,asrc-mode:
> > > +    $ref: /schemas/types.yaml#/definitions/string
> > > +    description:
> > > +      Asynchronous Sample Rate Converter (ASRC) operation mode control input.
> > > +      Describes whether the MODE pin is set to a high level (for master mode
> > > +      operation) or to a low level (for slave mode operation).
> > > +    enum: [ high, low ]
> > > +    default: low  
> > Since minimim I/O mode is only one currently supported, this should
> > always be low, right? Is the property needed at this point?  
> 
> Correct, it is expected that adi,asrc-mode will always be low for minimum I/O mode.
> The property is not _needed_ but, according to dt-binding guidelines [1], it is
> desired.
> 
> [1]: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/writing-bindings.rst?h=v6.18#n17
> 
> >   
> > > +
> > > +  adi,dclkio:
> > > +    description:
> > > +      DCLK pin I/O direction control for when the device operates in Pin Control
> > > +      Slave Mode or in SPI Control Mode. Describes if DEC0/DCLKIO pin is at a
> > > +      high level (which configures DCLK as an output) or to set to a low level
> > > +      (configuring DCLK for input).
> > > +    enum: [ out, in ]
> > > +    default: in
> > > +
> > > +  adi,dclkmode:
> > > +    description:
> > > +      DCLK mode control for when the device operates in Pin Control Slave Mode
> > > +      or in SPI Control Mode. Describes whether the DEC1/DCLKMODE pin is set to
> > > +      a high level (configuring the DCLK to operate in free running mode) or
> > > +      to a low level (to configure DCLK to operate in gated mode).
> > > +    enum: [ free-running, gated ]
> > > +    default: gated  
> > In minimum I/O mode datasheet mentions this should always be gated.
> > Perhaps this and adi,dclkio can be left out and added when driver gains
> > other support than I/O mode?  
> 
> Yes, that's also correct. A few properties are actually not needed for minimum
> I/O mode (i.e. can have their values inferred from the minimum I/O mode requirements).
> Sure, from developer's perspective it's easier to document only what the driver
> uses. adi,dclkio, adi,dclkmode, adi,asrc-mode, powerdown-gpios, regulators, could
> all be left to a latter time. Fine by me if dt maintainers agree with that.

Add as much as you are sure about to the binding now.  Sometimes
we will add properties later (with defaults) to cover things that are
complex enough that they will delay the initial binding merge.

Jonathan

> 
> With best regards,
> Marcelo