Re: [PATCH 1/4] dt-bindings: iio: adc: add bindings for AD4691 family

[Jonathan Cameron]

On Sat, 7 Mar 2026 12:48:39 -0600
David Lechner <dlechner@xxxxxxxxxxxx> wrote:

> On 3/6/26 5:55 AM, Sabau, Radu bogdan wrote:
> > 
> >   
> >> -----Original Message-----
> >> From: Jonathan Cameron <jic23@xxxxxxxxxx>
> >> Sent: Thursday, March 5, 2026 7:46 PM
> >> To: Radu Sabau via B4 Relay <devnull+radu.sabau.analog.com@xxxxxxxxxx>
> >> Cc: Sabau, Radu bogdan <Radu.Sabau@xxxxxxxxxx>; Lars-Peter Clausen <lars@xxxxxxxxxx>; Hennerich, Michael
> >> <Michael.Hennerich@xxxxxxxxxx>; David Lechner <dlechner@xxxxxxxxxxxx>; Sa, Nuno <Nuno.Sa@xxxxxxxxxx>; Andy Shevchenko
> >> <andy@xxxxxxxxxx>; Rob Herring <robh@xxxxxxxxxx>; Krzysztof Kozlowski <krzk+dt@xxxxxxxxxx>; Conor Dooley <conor+dt@xxxxxxxxxx>;
> >> Uwe Kleine-König <ukleinek@xxxxxxxxxx>; Liam Girdwood <lgirdwood@xxxxxxxxx>; Mark Brown <broonie@xxxxxxxxxx>; Linus Walleij
> >> <linusw@xxxxxxxxxx>; Bartosz Golaszewski <brgl@xxxxxxxxxx>; linux-iio@xxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx; linux-
> >> kernel@xxxxxxxxxxxxxxx; linux-pwm@xxxxxxxxxxxxxxx; linux-gpio@xxxxxxxxxxxxxxx
> >> Subject: Re: [PATCH 1/4] dt-bindings: iio: adc: add bindings for AD4691 family
> >>
> >> [External]
> >>
> >> On Thu, 05 Mar 2026 14:23:27 +0200
> >> Radu Sabau via B4 Relay <devnull+radu.sabau.analog.com@xxxxxxxxxx> wrote:
> >>  
> >>> From: Radu Sabau <radu.sabau@xxxxxxxxxx>
> >>>
> >>> Add YAML bindings and dt-bindings header for the Analog Devices AD4691
> >>> family of multichannel SAR ADCs (AD4691, AD4692, AD4693, AD4694).
> >>>
> >>> The binding describes five operating modes selectable via the
> >>> adi,spi-mode property, optional PWM/clock for CNV Clock and CNV Burst
> >>> modes, GPIO pins, voltage supplies and the trigger-source interface for
> >>> SPI Engine offload operation.
> >>>
> >>> Signed-off-by: Radu Sabau <radu.sabau@xxxxxxxxxx>  
> >>
> >> Hi Radu, I'm going to focus on mode... Mostly because things called
> >> mode are usually a sign of mixing up different aspects of the board
> >> design...
> >>  
> > Hi Jonathan, Krysztof,
> > 
> > Thank you guys so much for your review.
> > 
> > Regarding 'mode', I agree that it should be something that could be modified
> > at run-time, especially since all register modes (CNV_CLOCK, CNV_BURST,
> > AUTONOMOUS and SPI_BURST) rely on the same principles of reading the
> > ADC result from the registers, the main difference being that PWM on the
> > CNV pin is required for CNV_CLOCK and CNV_BURST, but the board design
> > stays the same. Perhaps this PWM can be initialized at start-time and only
> > be used when CNV modes are being used. This would mean mode can
> > become an IIO attribute that could be set by the user at run-time.  
> 
> More likely, it would be two different ways of doing a buffered read,
> so maybe two different buffers? Or just pick the "best" one and only
> implement that mode.

I 'think' burst mode is really an oversampling thing as you read back from
either the output of an averaging filter or an accumulator. 
I doubt there is reason to support both the oversampled and raw
readings at the same time.

> 
> > 
> > However for MANUAL, modifications of jumper resistors on the physical
> > board is required for proper functionality, since the CNV pin needs to be
> > tied to CS in this mode. Would it be preferred if bindings would have a
> > 'register-mode' attribute (the name could be better) which can have values
> > like 1(register modes are used) and 1(manual mode is used), and for
> > register modes, have a global IIO attribute that can switch between
> > them?
> >   
> 
> The binding should describe how the chip is wired up. So rather than thinking
> about modes, try thinking in terms of connections. Based on what the devicetree
> says is connected, the driver can then infer which modes are actually possible.
> 
> Bringing back some context that was trimmed:
> 
> +  adi,spi-mode:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    enum: [0, 1, 2, 3, 4]
> +    description: |
> +      Selects the ADC operating mode:
> +        0 - CNV Clock Mode: External PWM drives CNV pin, samples at PWM rate.
> +        1 - CNV Burst Mode: PWM triggers burst cycles, internal oscillator
> +            drives conversions within each burst.
> +        2 - Autonomous Mode: Internal oscillator drives conversions, software
> +            starts/stops via register write.
> +        3 - SPI Burst Mode: Similar to Autonomous Mode but optimized for
> +            SPI burst reads.
> +        4 - Manual Mode: CNV is directly tied to SPI CS. Each SPI transfer
> +            triggers a conversion and returns previous result (pipelined).
> 
> 
> It sounds like there are 3 ways that the CNV pin could be wired up:
> 
> 1. Wired to PWM
> 2. Not connected
> 3. Wired to CS
> 
> On some other chips we've seen where CNV could be wired up different ways,
> "not connected" was not an option. In those cases, we could infer that if
> that no other properties indicated what CNV was connected to, then we would
> assume CNV was connected to SPI CS.
> 
> In this case, if "not connected" is an option, we might need a bool/flag
> property adi,cnv-is-cs to describe that the CNV pin is wired to the CS pin.
> And we already have the pwms property to know when CNV is connected to a
> PWM.
> 
> 
> > Please let me know your thoughts on this before addressing the other
> > Comments and preparing other patches.
> > 
> > Best regards,
> > Radu
> >