mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Jonathan Cameron <jic23@kernel.org>
To: "Sabau, Radu bogdan" <Radu.Sabau@analog.com>
Cc: "Uwe Kleine König" <u.kleine-koenig@baylibre.com>,
	"Radu Sabau via B4 Relay"
	<devnull+radu.sabau.analog.com@kernel.org>,
	"Lars-Peter Clausen" <lars@metafoo.de>,
	"Hennerich, Michael" <Michael.Hennerich@analog.com>,
	"David Lechner" <dlechner@baylibre.com>,
	"Sa, Nuno" <Nuno.Sa@analog.com>,
	"Andy Shevchenko" <andy@kernel.org>,
	"linux-iio@vger.kernel.org" <linux-iio@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"Thomas Gleixner" <tglx@linutronix.de>
Subject: Re: [PATCH v4 2/2] iio: adc: ad_sigma_delta: fix clear_pending_event for registerless devices
Date: Tue, 26 May 2026 13:39:33 +0100	[thread overview]
Message-ID: <20260526133933.61c7444f@jic23-huawei> (raw)
In-Reply-To: <SA5PR03MB84250008796EE6F3B5A124DAF70A2@SA5PR03MB8425.namprd03.prod.outlook.com>

On Mon, 25 May 2026 12:16:14 +0000
"Sabau, Radu bogdan" <Radu.Sabau@analog.com> wrote:

> > -----Original Message-----
> > From: Uwe Kleine-König <u.kleine-koenig@baylibre.com>
> > 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


      reply	other threads:[~2026-05-26 12:39 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-21 10:30 [PATCH v4 0/2] iio: adc: ad_sigma_delta: fix CS assertion and registerless device handling Radu Sabau via B4 Relay
2026-05-21 10:30 ` [PATCH v4 1/2] iio: adc: ad_sigma_delta: fix CS held asserted and state leaks Radu Sabau via B4 Relay
2026-05-21 10:30 ` [PATCH v4 2/2] iio: adc: ad_sigma_delta: fix clear_pending_event for registerless devices Radu Sabau via B4 Relay
2026-05-22 13:51   ` Jonathan Cameron
2026-05-22 15:20     ` Uwe Kleine-König
2026-05-25 12:16       ` Sabau, Radu bogdan
2026-05-26 12:39         ` Jonathan Cameron [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260526133933.61c7444f@jic23-huawei \
    --to=jic23@kernel.org \
    --cc=Michael.Hennerich@analog.com \
    --cc=Nuno.Sa@analog.com \
    --cc=Radu.Sabau@analog.com \
    --cc=andy@kernel.org \
    --cc=devnull+radu.sabau.analog.com@kernel.org \
    --cc=dlechner@baylibre.com \
    --cc=lars@metafoo.de \
    --cc=linux-iio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=tglx@linutronix.de \
    --cc=u.kleine-koenig@baylibre.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®