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

[David Lechner <dlechner@baylibre.com>]

On 5/14/26 5:07 AM, Sabau, Radu bogdan wrote:
>> -----Original Message-----
>> From: Jonathan Cameron <jic23@kernel.org>
>> Sent: Tuesday, May 12, 2026 2:13 PM
> 
> ...
> 
>>> Fixes: 132d44dc6966 ("iio: adc: ad_sigma_delta: Check for previous ready
>> signals")
>>> Acked-by: Uwe Kleine-König <u.kleine-koenig@baylibre.com>
>>> Signed-off-by: Radu Sabau <radu.sabau@analog.com>
>>
>> Actually - dropped again. I forgot to check Sashiko.
>> (Note please do this yourself in future and reply to the patch if changes are
>> needed).  It raises some concerns about devices that don't provide
>> the callbacks and hence won't ever use that keep_cs_asserted = false callback.
>>
>> I'm struggling a bit on how the max11205 works at all as there seems
>> to be a status register read on a device which claims to have no registers.
>> ad_sigma_delta_clear_pending_event() as the binding suggests this device
>> doesn't have a drdy_gpio
>>
>> Anyhow, please take a look at the feedback and if it's wrong please provide
>> an explanation of why in this thread.
>>
> 
> Hi Jonathan,
> 
> First of all sorry for forgetting about Sashiko. I had a look at max11205 and it
> seems like the device doesn't have a CS so therefore the concern Sashiko
> raises regarding CS may not be valid, I will make sure to mention
> that in the commit message.
> 
> On the other hand, I see the IC has a shared pin for DRDY and DOUT and
> the bindings specify some interrupt though no specific rdy-gpios are
> mentioned there. The device reads a register that doesn't exist which
> in this case means it blindly clocks out whatever comes on MISO...
> 
> However I may have a fix for this and would appear in a second commit.
> In clear_pending_event where rdy_gpiod is checked there could be yet
> another else branch that could simply return 0 and here is why I think this would work:
> 
> Since the IRQ is requested with IRQF_NO_AUTOEN and IRQ_DISABLE_UNLAZY, the latter
> keeps the IRQ hardware-unmasked even while software-disabled, so any falling edge

This should depend on the IRQ controller. There are some past discussions
on this on the mailing list. The ideal behavior should be that if the
IRQ controller can fully disable the interrupt so that we don't get the
spurious interrupt on enable (from the normal SPI data, not DRDY). If
the interrupt controller can't do this, then it requires the rdy-gpios
to be able to distinguish DRDY from SPI data.

I have used this on ad7173 on a ZedBoard without rdy-gpios and the
interrupt was working correctly. So unless something changed with
the interrupt config in this code since the last time I was using,
I would not expect to see this problem on _all_ interrupt controllers.

> from a completed conversion is latched by the IRQ controller. When ad_sd_enable_irq()
> is subsequently called, the latched edge fires immediately, and ad_sd_read_reg() reads
> the complete stale result without any prior partial clock corruption, so the result is not
> read by mistake as it could currently happen.
> 
> Let me know your thoughts on this,
> Radu
>