RE: [PATCH] iio: adc: ad_sigma_delta: fix CS held asserted after single conversion

[Sabau, Radu bogdan]

> -----Original Message-----
> From: Sabau, Radu bogdan <Radu.Sabau@xxxxxxxxxx>
> Sent: Thursday, May 7, 2026 3:17 PM

...

> >
> 
> Hi everyone,
> 
> Uhmm..., why was this fix marked as Changes Requested? Is it something I am
> missing?
> 
> Best Regards,
> Radu

I have also had a look here :
https://sashiko.dev/#/patchset/20260428-ad_sigma_delta-fix-v1-1-8e3f925ee8d2%40analog.com

Therefore here is the reply to Sashiko's review:

> For devices that lack the optional disable_one callback,
> ad_sigma_delta_disable_one() is a no-op. In that scenario,
> ad_sigma_delta_set_mode() executes the final SPI transfer for the
> conversion.
>
> Because sigma_delta->keep_cs_asserted is still true when set_mode is
> called, its transfer will carry cs_change = 1. Since disable_one
> does not issue a subsequent transfer with cs_change = 0, will the SPI
> framework skip teardown and leave CS permanently stuck low?
>
> Could sigma_delta->keep_cs_asserted be cleared before calling
> ad_sigma_delta_set_mode() to ensure proper teardown, or should an
> explicit dummy transfer be added?

Good catch, thank you. The previous version only moved disable_one()
after clearing keep_cs_asserted, but left set_mode(AD_SD_MODE_IDLE) in
the out: block where keep_cs_asserted is still true. For devices that
do not implement disable_one, set_mode() issues the last SPI transfer,
so it carries cs_change=1 and CS stays stuck — the same bug reproduced
on a different code path.

Fixed by moving set_mode(IDLE) into the out_unlock: block as well, so
both calls execute after keep_cs_asserted is cleared. This matches the
pattern already used correctly in ad_sd_calibrate(), which clears
keep_cs_asserted before calling set_mode(IDLE) and disable_one() in its
single cleanup block.

...

> I noticed an additional state tracking issue in the error path of
> ad_sd_buffer_postenable() that wasn't introduced by this patch.
> When operations like ad_sigma_delta_clear_pending_event() fail, the error
> path calls spi_bus_unlock() but leaves sigma_delta->bus_locked = true and
> sigma_delta->keep_cs_asserted = true. Since the IIO core does not invoke
> predisable on a postenable failure, this state persists.
>
> Subsequent SPI operations like direct mode reads will observe
> bus_locked == true and incorrectly invoke spi_sync_locked(). As the bus
> is not actually locked at the controller level, this bypasses the
> SPI framework's bus_lock_mutex.
>
> Does this allow concurrent access to the SPI controller from multiple
> threads, potentially corrupting SPI bus data?

Yes, the concern is valid. With bus_locked == true but the controller's
bus_lock_mutex not held, any concurrent spi_sync() from another driver
can proceed unobstructed while our driver issues spi_sync_locked() —
exactly the race bus_lock_mutex is meant to prevent.

Fixed in the same patch by resetting both keep_cs_asserted and
bus_locked to false in err_unlock: before calling spi_bus_unlock(),
consistent with the teardown sequence in predisable().