RE: [PATCH] iio: adc: ad4695: Fix call ordering in offload buffer postenable

[Sabau, Radu bogdan]

> -----Original Message-----
> From: David Lechner <dlechner@xxxxxxxxxxxx>
> Sent: Monday, March 30, 2026 5:11 PM
> To: Sabau, Radu bogdan <Radu.Sabau@xxxxxxxxxx>; Lars-Peter Clausen
> <lars@xxxxxxxxxx>; Hennerich, Michael <Michael.Hennerich@xxxxxxxxxx>;
> Sa, Nuno <Nuno.Sa@xxxxxxxxxx>; Jonathan Cameron <jic23@xxxxxxxxxx>;
> Andy Shevchenko <andy@xxxxxxxxxx>
> Cc: linux-iio@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx
> Subject: Re: [PATCH] iio: adc: ad4695: Fix call ordering in offload buffer
> postenable
> 
> [External]
> 
> On 3/30/26 8:34 AM, Radu Sabau via B4 Relay wrote:
> > From: Radu Sabau <radu.sabau@xxxxxxxxxx>
> >
> > ad4695_enter_advanced_sequencer_mode() was called after
> > spi_offload_trigger_enable(), meaning a regular SPI transfer could be in
> > flight while the offload was already active. When the offload fires its
> > completion interrupt concurrently with the regular transfer, the SPI
> > engine interrupt handler is not designed to handle both at once, leading
> > to a kernel panic.
> >
> > Fix this by calling ad4695_enter_advanced_sequencer_mode() before
> > spi_offload_trigger_enable(), ensuring all SPI bus accesses are complete
> > before the offload becomes active. This is consistent with the same
> > constraint that already applies to the BUSY_GP_EN write above it.
> >
> > Update the error unwind labels accordingly: add err_exit_conversion_mode
> > so that a failure of spi_offload_trigger_enable() correctly exits
> > conversion mode before clearing BUSY_GP_EN.
> >
> > Fixes: f09f140e3ea8 ("iio: adc: ad4695: Add support for SPI offload")
> > Signed-off-by: Radu Sabau <radu.sabau@xxxxxxxxxx>
> > ---
> > When enabling the IIO buffer for SPI offload operation on AD4695/AD4696,
> > ad4695_enter_advanced_sequencer_mode() was called after
> > spi_offload_trigger_enable(), resulting in a regular SPI transfer being
> > in flight while the offload was already active. This caused a kernel
> > panic in the SPI engine interrupt handler.
> 
> This used to work (I spend many hours testing it to arrive at the current
> state). Are we sure there hasn't been a bug introduced in the AXI SPI Engine
> instead?
> 

Hi David,

I also tried looking in the AXI SPI Engine and find something related first
but couldn't find anything, though I am happy to be corrected.

> >
> > This series fixes the call ordering so all SPI bus accesses complete
> > before the offload becomes active, consistent with the constraint that
> > was already documented for the BUSY_GP_EN write in the same function.

...

> > -	ret = spi_offload_trigger_enable(st->offload, st->offload_trigger,
> > -					 &config);
> > +	ret = ad4695_enter_advanced_sequencer_mode(st, num_slots);
> >  	if (ret)
> >  		goto err_disable_busy_output;
> >
> > -	ret = ad4695_enter_advanced_sequencer_mode(st, num_slots);
> > +	ret = spi_offload_trigger_enable(st->offload, st->offload_trigger,
> > +					 &config);
> 
> Swapping the order here introduces a race condition.
> 
> ad4695_enter_advanced_sequencer_mode() starts sampling (triggered from
> the
> internal clock in the ADC) and the first sample could be completed before
> the SPI offload is enabled. If the first sample is missed, then all of the
> data will be off by one index on the buffer.
> 

I thought this too at the first glance, but couldn’t be sure so I tried testing it
anyway using the FMCZ evaluation board and a Zedboard, and everything
worked as expected.
I have plotted buffered data and it showed exactly what the Signal Generator
was outputting and on the right channel, so no index was off, if that's what
you meant.

Best Regards,
Radu