From: Radu Sabau <radu.sabau@analog.com>

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@analog.com>
---
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 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.
---
 drivers/iio/adc/ad4695.c | 31 ++++++++++++++-----------------
 1 file changed, 14 insertions(+), 17 deletions(-)

diff --git a/drivers/iio/adc/ad4695.c b/drivers/iio/adc/ad4695.c
index cda419638d9a..c6721f5ac8af 100644
--- a/drivers/iio/adc/ad4695.c
+++ b/drivers/iio/adc/ad4695.c
@@ -866,24 +866,24 @@ static int ad4695_offload_buffer_postenable(struct iio_dev *indio_dev)
 		return ret;
 
 	/*
-	 * NB: technically, this is part the SPI offload trigger enable, but it
-	 * doesn't work to call it from the offload trigger enable callback
-	 * because it requires accessing the SPI bus. Calling it from the
-	 * trigger enable callback could cause a deadlock.
+	 * NB: these are technically part of the SPI offload trigger enable, but
+	 * they can't be called from the offload trigger enable callback because
+	 * they require accessing the SPI bus. Calling them from the trigger
+	 * enable callback could cause a deadlock.
 	 */
 	ret = regmap_set_bits(st->regmap, AD4695_REG_GP_MODE,
 			      AD4695_REG_GP_MODE_BUSY_GP_EN);
 	if (ret)
 		goto err_unoptimize_message;
 
-	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);
 	if (ret)
-		goto err_offload_trigger_disable;
+		goto err_exit_conversion_mode;
 
 	mutex_lock(&st->cnv_pwm_lock);
 	pwm_get_state(st->cnv_pwm, &state);
@@ -895,22 +895,19 @@ static int ad4695_offload_buffer_postenable(struct iio_dev *indio_dev)
 	ret = pwm_apply_might_sleep(st->cnv_pwm, &state);
 	mutex_unlock(&st->cnv_pwm_lock);
 	if (ret)
-		goto err_offload_exit_conversion_mode;
+		goto err_trigger_disable;
 
 	return 0;
 
-err_offload_exit_conversion_mode:
+err_trigger_disable:
 	/*
-	 * We have to unwind in a different order to avoid triggering offload.
-	 * ad4695_exit_conversion_mode() triggers a conversion, so it has to be
-	 * done after spi_offload_trigger_disable().
+	 * ad4695_exit_conversion_mode() triggers a conversion, so the offload
+	 * must be disabled first to avoid capturing a spurious sample.
 	 */
 	spi_offload_trigger_disable(st->offload, st->offload_trigger);
-	ad4695_exit_conversion_mode(st);
-	goto err_disable_busy_output;
 
-err_offload_trigger_disable:
-	spi_offload_trigger_disable(st->offload, st->offload_trigger);
+err_exit_conversion_mode:
+	ad4695_exit_conversion_mode(st);
 
 err_disable_busy_output:
 	regmap_clear_bits(st->regmap, AD4695_REG_GP_MODE,

---
base-commit: 11439c4635edd669ae435eec308f4ab8a0804808
change-id: 20260330-ad4696-fix-186955a8c511

Best regards,
-- 
Radu Sabau <radu.sabau@analog.com>

On 3/30/26 8:34 AM, Radu Sabau via B4 Relay wrote:
> From: Radu Sabau <radu.sabau@analog.com>
> 
> 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@analog.com>
> ---
> 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 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.
> ---
>  drivers/iio/adc/ad4695.c | 31 ++++++++++++++-----------------
>  1 file changed, 14 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/iio/adc/ad4695.c b/drivers/iio/adc/ad4695.c
> index cda419638d9a..c6721f5ac8af 100644
> --- a/drivers/iio/adc/ad4695.c
> +++ b/drivers/iio/adc/ad4695.c
> @@ -866,24 +866,24 @@ static int ad4695_offload_buffer_postenable(struct iio_dev *indio_dev)
>  		return ret;
>  
>  	/*
> -	 * NB: technically, this is part the SPI offload trigger enable, but it
> -	 * doesn't work to call it from the offload trigger enable callback
> -	 * because it requires accessing the SPI bus. Calling it from the
> -	 * trigger enable callback could cause a deadlock.
> +	 * NB: these are technically part of the SPI offload trigger enable, but
> +	 * they can't be called from the offload trigger enable callback because
> +	 * they require accessing the SPI bus. Calling them from the trigger
> +	 * enable callback could cause a deadlock.

I didn't see a explanation for changing the comment here. I think "this" is
still correct instead of "these" because only the BUSY pin is the trigger,
so only the one regmap_set_bits() below would be in the trigger callback
if it was possible.

If we want to improve the comment to make it more clear, I would save that
for a separate patch and specifically mention the BUSY output.

>  	 */
>  	ret = regmap_set_bits(st->regmap, AD4695_REG_GP_MODE,
>  			      AD4695_REG_GP_MODE_BUSY_GP_EN);
>  	if (ret)
>  		goto err_unoptimize_message;
>  
> -	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);
>  	if (ret)
> -		goto err_offload_trigger_disable;
> +		goto err_exit_conversion_mode;
>  
>  	mutex_lock(&st->cnv_pwm_lock);
>  	pwm_get_state(st->cnv_pwm, &state);
> @@ -895,22 +895,19 @@ static int ad4695_offload_buffer_postenable(struct iio_dev *indio_dev)
>  	ret = pwm_apply_might_sleep(st->cnv_pwm, &state);
>  	mutex_unlock(&st->cnv_pwm_lock);
>  	if (ret)
> -		goto err_offload_exit_conversion_mode;
> +		goto err_trigger_disable;
>  
>  	return 0;
>  
> -err_offload_exit_conversion_mode:
> +err_trigger_disable:
>  	/*
> -	 * We have to unwind in a different order to avoid triggering offload.
> -	 * ad4695_exit_conversion_mode() triggers a conversion, so it has to be
> -	 * done after spi_offload_trigger_disable().
> +	 * ad4695_exit_conversion_mode() triggers a conversion, so the offload
> +	 * must be disabled first to avoid capturing a spurious sample.

Since this becomes the natural unwinding order, I don't think we need
the comment at all any more.

There is still a similar comment in ad4695_offload_buffer_predisable()
so we aren't losing this information if we remove it here.

>  	 */
>  	spi_offload_trigger_disable(st->offload, st->offload_trigger);
> -	ad4695_exit_conversion_mode(st);
> -	goto err_disable_busy_output;
>  
> -err_offload_trigger_disable:
> -	spi_offload_trigger_disable(st->offload, st->offload_trigger);
> +err_exit_conversion_mode:
> +	ad4695_exit_conversion_mode(st);
>  
>  err_disable_busy_output:
>  	regmap_clear_bits(st->regmap, AD4695_REG_GP_MODE,
> 
> ---
> base-commit: 11439c4635edd669ae435eec308f4ab8a0804808
> change-id: 20260330-ad4696-fix-186955a8c511
> 
> Best regards,

On Mon, Mar 30, 2026 at 04:34:39PM +0300, Radu Sabau via B4 Relay wrote:

> 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.

Please, provide ~3-5 (the most important) lines of that 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.

-- 
With Best Regards,
Andy Shevchenko

On Mon, Mar 30, 2026 at 04:34:39PM +0300, Radu Sabau wrote:
> 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@analog.com>
> ---
> 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 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.
> ---

LGTM,

Reviewed-by: Nuno Sá <nuno.sa@analog.com>

>  drivers/iio/adc/ad4695.c | 31 ++++++++++++++-----------------
>  1 file changed, 14 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/iio/adc/ad4695.c b/drivers/iio/adc/ad4695.c
> index cda419638d9a..c6721f5ac8af 100644
> --- a/drivers/iio/adc/ad4695.c
> +++ b/drivers/iio/adc/ad4695.c
> @@ -866,24 +866,24 @@ static int ad4695_offload_buffer_postenable(struct iio_dev *indio_dev)
>  		return ret;
>  
>  	/*
> -	 * NB: technically, this is part the SPI offload trigger enable, but it
> -	 * doesn't work to call it from the offload trigger enable callback
> -	 * because it requires accessing the SPI bus. Calling it from the
> -	 * trigger enable callback could cause a deadlock.
> +	 * NB: these are technically part of the SPI offload trigger enable, but
> +	 * they can't be called from the offload trigger enable callback because
> +	 * they require accessing the SPI bus. Calling them from the trigger
> +	 * enable callback could cause a deadlock.
>  	 */
>  	ret = regmap_set_bits(st->regmap, AD4695_REG_GP_MODE,
>  			      AD4695_REG_GP_MODE_BUSY_GP_EN);
>  	if (ret)
>  		goto err_unoptimize_message;
>  
> -	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);
>  	if (ret)
> -		goto err_offload_trigger_disable;
> +		goto err_exit_conversion_mode;
>  
>  	mutex_lock(&st->cnv_pwm_lock);
>  	pwm_get_state(st->cnv_pwm, &state);
> @@ -895,22 +895,19 @@ static int ad4695_offload_buffer_postenable(struct iio_dev *indio_dev)
>  	ret = pwm_apply_might_sleep(st->cnv_pwm, &state);
>  	mutex_unlock(&st->cnv_pwm_lock);
>  	if (ret)
> -		goto err_offload_exit_conversion_mode;
> +		goto err_trigger_disable;
>  
>  	return 0;
>  
> -err_offload_exit_conversion_mode:
> +err_trigger_disable:
>  	/*
> -	 * We have to unwind in a different order to avoid triggering offload.
> -	 * ad4695_exit_conversion_mode() triggers a conversion, so it has to be
> -	 * done after spi_offload_trigger_disable().
> +	 * ad4695_exit_conversion_mode() triggers a conversion, so the offload
> +	 * must be disabled first to avoid capturing a spurious sample.
>  	 */
>  	spi_offload_trigger_disable(st->offload, st->offload_trigger);
> -	ad4695_exit_conversion_mode(st);
> -	goto err_disable_busy_output;
>  
> -err_offload_trigger_disable:
> -	spi_offload_trigger_disable(st->offload, st->offload_trigger);
> +err_exit_conversion_mode:
> +	ad4695_exit_conversion_mode(st);
>  
>  err_disable_busy_output:
>  	regmap_clear_bits(st->regmap, AD4695_REG_GP_MODE,
> 
> ---
> base-commit: 11439c4635edd669ae435eec308f4ab8a0804808
> change-id: 20260330-ad4696-fix-186955a8c511
> 
> Best regards,
> -- 
> Radu Sabau <radu.sabau@analog.com>
> 

On 3/30/26 8:34 AM, Radu Sabau via B4 Relay wrote:
> From: Radu Sabau <radu.sabau@analog.com>
> 
> 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@analog.com>
> ---
> 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?

> 
> 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.
> ---
>  drivers/iio/adc/ad4695.c | 31 ++++++++++++++-----------------
>  1 file changed, 14 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/iio/adc/ad4695.c b/drivers/iio/adc/ad4695.c
> index cda419638d9a..c6721f5ac8af 100644
> --- a/drivers/iio/adc/ad4695.c
> +++ b/drivers/iio/adc/ad4695.c
> @@ -866,24 +866,24 @@ static int ad4695_offload_buffer_postenable(struct iio_dev *indio_dev)
>  		return ret;
>  
>  	/*
> -	 * NB: technically, this is part the SPI offload trigger enable, but it
> -	 * doesn't work to call it from the offload trigger enable callback
> -	 * because it requires accessing the SPI bus. Calling it from the
> -	 * trigger enable callback could cause a deadlock.
> +	 * NB: these are technically part of the SPI offload trigger enable, but
> +	 * they can't be called from the offload trigger enable callback because
> +	 * they require accessing the SPI bus. Calling them from the trigger
> +	 * enable callback could cause a deadlock.
>  	 */
>  	ret = regmap_set_bits(st->regmap, AD4695_REG_GP_MODE,
>  			      AD4695_REG_GP_MODE_BUSY_GP_EN);
>  	if (ret)
>  		goto err_unoptimize_message;
>  
> -	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.


>  	if (ret)
> -		goto err_offload_trigger_disable;
> +		goto err_exit_conversion_mode;
>  
>  	mutex_lock(&st->cnv_pwm_lock);
>  	pwm_get_state(st->cnv_pwm, &state);
> @@ -895,22 +895,19 @@ static int ad4695_offload_buffer_postenable(struct iio_dev *indio_dev)
>  	ret = pwm_apply_might_sleep(st->cnv_pwm, &state);
>  	mutex_unlock(&st->cnv_pwm_lock);
>  	if (ret)
> -		goto err_offload_exit_conversion_mode;
> +		goto err_trigger_disable;
>  
>  	return 0;
>  
> -err_offload_exit_conversion_mode:
> +err_trigger_disable:
>  	/*
> -	 * We have to unwind in a different order to avoid triggering offload.
> -	 * ad4695_exit_conversion_mode() triggers a conversion, so it has to be
> -	 * done after spi_offload_trigger_disable().
> +	 * ad4695_exit_conversion_mode() triggers a conversion, so the offload
> +	 * must be disabled first to avoid capturing a spurious sample.
>  	 */
>  	spi_offload_trigger_disable(st->offload, st->offload_trigger);
> -	ad4695_exit_conversion_mode(st);
> -	goto err_disable_busy_output;
>  
> -err_offload_trigger_disable:
> -	spi_offload_trigger_disable(st->offload, st->offload_trigger);
> +err_exit_conversion_mode:
> +	ad4695_exit_conversion_mode(st);
>  
>  err_disable_busy_output:
>  	regmap_clear_bits(st->regmap, AD4695_REG_GP_MODE,
> 
> ---
> base-commit: 11439c4635edd669ae435eec308f4ab8a0804808
> change-id: 20260330-ad4696-fix-186955a8c511
> 
> Best regards,