Re: [PATCH v4 2/2] iio: adc: ad_sigma_delta: fix clear_pending_event for registerless devices
From: Jonathan Cameron
Date: Tue May 26 2026 - 08:40:51 EST
On Mon, 25 May 2026 12:16:14 +0000
"Sabau, Radu bogdan" <Radu.Sabau@xxxxxxxxxx> wrote:
> > -----Original Message-----
> > From: Uwe Kleine-König <u.kleine-koenig@xxxxxxxxxxxx>
> > Sent: Friday, May 22, 2026 6:21 PM
>
> ...
>
> > > David raised this on v2.
> > >
> > > This sounds suspiciously like we are in implementation defined territory.
> > > IIRC if an irq is disabled, whether the interrupt is latched is down
> > > to whether the irq controller happens to do so. There is no hard rule on this
> > > or way to detect whether it will or not.
> > >
> > >
> > https://lore.kernel.org/all/io53lznz3qp3jd5rohqsjhosnmdzd6d44sdbwu5jcfr
> > s3rz2a2@orquwgflrtyc/
> > > Was I think the most recent detailed thread on this.
> > >
> > > Uwe - you were driving that discussion can you sanity check what we have
> > > here if you have a moment? Thanks!
> >
> > Without having looked at the code but skimmed through the old discussion
> > again, the result back then was that you won't get reliable behaviour
> > without rdy-gpio.
> >
> > I didn't try to understand what's different fro "registerless devices",
> > but I doubt this implements a robust solution.
> >
>
> Hi everyone,
>
> Thank you so much for looking into this. The concern about IRQ latching is valid,
> and after looking more carefully at this I can see the commit message explanation
> is simply wrong.
>
> IRQ_DISABLE_UNLAZY I think actually does the opposite... it causes immediate hw
> masking when disable_irq_nosync() is called, precisely to prevent spurious interrupts
> from MISO.
I'd modify this a tiny bit.
Perhaps read that thread above again. The understanding I took away at the time is that
IRQ_DISABLE_UNLAZY is just a performance trick to get into a useful state if you
think something is likely to happen - it's not a tool to use to make functional
changes or constrain the handling.
> So the claim that it "keeps the hardware IRQ line unmasked while
> software-disabled" was backwards. However, as Jonathan says this is implementation
> defined territory, but perhaps the comment and commit message could still be corrected.
>
> But, I think return 0 could still be safe, devices with rdy-gpios use that, the ones without
> but with registers use the status register and the others which have neither (in our case
> ad7191, ad7780 and max11205 as far as I can tell) could safely use return 0 branch from
> here and this is why I think its safe:
>
> - ad7191: CS is the PDOWN pin - deasserting it powers down and resets the device. By the
> time clear_pending_event() is called at the start of a new conversion, the device has been
> reset and there should not be any stale result to clear.
> - ad7780 (with powrdown GPIO): same - device is powered down between conversions.
> - ad7780 (without powerdown GPIO) and max11205: both are continuously-converting ADCs
> that cycle ~DRDY at their output data rate regardless of whether the previous result was read.
> The next conversion's falling edge fires naturally.
>
> One limitation still remains (beside the risk of missing a sample for the last case): a future
> registerless device (has_registers = false) that holds ~RDY asserted until data is read would
> need either num_resetclks set or a rdy-gpio. Perhaps a comment would be useful here.
Agreed. There is a pitfall here that we should definitely label! I'm fine with the change
I think as long as it's backed by some extra 'breadcrumbs' for anyone wondering how this
all works in future.
Thanks
Jonathan
>
> With Best Regards,
> Radu