* [PATCH v4 0/2] iio: adc: ad_sigma_delta: fix CS assertion and registerless device handling
@ 2026-05-21 10:30 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
0 siblings, 2 replies; 7+ messages in thread
From: Radu Sabau via B4 Relay @ 2026-05-21 10:30 UTC (permalink / raw)
To: Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron,
David Lechner, Nuno Sá,
Andy Shevchenko, Uwe Kleine-König
Cc: linux-iio, linux-kernel, Radu Sabau, Jonathan Cameron
This series fixes two independent bugs in the ad_sigma_delta framework.
Patch 1 fixes CS being left permanently asserted after single conversion
and in the error path of ad_sd_buffer_postenable(). In
ad_sigma_delta_single_conversion(), set_mode(AD_SD_MODE_IDLE) and
disable_one() were executing while keep_cs_asserted was still true,
causing any SPI transfer they issued to carry cs_change=1. The
postenable() error path also failed to call set_mode(AD_SD_MODE_IDLE),
leaving the device in continuous conversion mode with bus_locked
incorrectly set, opening a window for concurrent SPI access.
Patch 2 fixes ad_sigma_delta_clear_pending_event() for devices with
has_registers = false and no rdy_gpiod (currently MAX11205 and AD7191).
These devices fall through to the status register read path, but since
has_registers is false, ad_sd_read_reg() transmits no address byte and
blindly clocks raw MISO bytes — indistinguishable from reading conversion
data, partially consuming any pending result and corrupting the stream.
With num_resetclks = 0 on these devices a further hazard exists: if
pending_event is set, the drain path attempts memset of SIZE_MAX bytes,
corrupting the heap. The same heap corruption can be triggered on any
device with rdy_gpiod set but num_resetclks = 0, so an explicit
data_read_len == 0 guard is added independently of the register check.
Signed-off-by: Radu Sabau <radu.sabau@analog.com>
---
Changes in v4:
- set_mode(AD_SD_MODE_IDLE) was accidentally placed in patch 2
in v3; moved to the correct commit via rebase.
- add data_read_len == 0 guard to cover the heap corruption path
reachable via rdy_gpiod on devices with num_resetclks = 0;
set_mode(AD_SD_MODE_IDLE) moved to patch 1.
- Link to v3: https://lore.kernel.org/r/20260518-ad_sigma_delta-fix-v3-0-a2a92b0c36f3@analog.com
Changes in v3:
- add ad_sigma_delta_set_mode(AD_SD_MODE_IDLE) to the err_unlock
path in ad_sd_buffer_postenable() to revert the device
from continuous mode and deassert CS; previously only the flag resets
were added. Update commit message to remove the inaccurate "in all
cases" claim and note that CS-less devices such as MAX11205 are
unaffected since no physical line is toggled.
- Patch 2: new patch fixing ad_sigma_delta_clear_pending_event() for
devices with has_registers = false and no rdy_gpiod, where the
existing status register read path blindly clocks raw MISO bytes,
partially consuming pending conversion data and potentially
corrupting the heap via a SIZE_MAX memset.
- Link to v2: https://lore.kernel.org/r/20260507-ad_sigma_delta-fix-v2-1-ec86eb0463bd@analog.com
Changes in v2:
- Move set_mode(AD_SD_MODE_IDLE) into out_unlock: as well, not only
disable_one(); v1 left set_mode() above the label where
keep_cs_asserted is still true, so devices without the optional
disable_one callback still had CS stuck after that transfer.
- Fix pre-existing state leak in ad_sd_buffer_postenable() err_unlock:
reset bus_locked and keep_cs_asserted before spi_bus_unlock() to
prevent spi_sync_locked() being called on an unlocked controller.
- Link to v1: https://lore.kernel.org/r/20260428-ad_sigma_delta-fix-v1-1-8e3f925ee8d2@analog.com
---
Radu Sabau (2):
iio: adc: ad_sigma_delta: fix CS held asserted and state leaks
iio: adc: ad_sigma_delta: fix clear_pending_event for registerless devices
drivers/iio/adc/ad_sigma_delta.c | 29 ++++++++++++++++++++++++-----
1 file changed, 24 insertions(+), 5 deletions(-)
---
base-commit: 3b3bea6d4b9c162f9e555905d96b8c1da67ecd5b
change-id: 20260428-ad_sigma_delta-fix-bb65d56ccbb0
Best regards,
--
Radu Sabau <radu.sabau@analog.com>
^ permalink raw reply [flat|nested] 7+ messages in thread* [PATCH v4 1/2] iio: adc: ad_sigma_delta: fix CS held asserted and state leaks
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 ` 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
1 sibling, 0 replies; 7+ messages in thread
From: Radu Sabau via B4 Relay @ 2026-05-21 10:30 UTC (permalink / raw)
To: Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron,
David Lechner, Nuno Sá,
Andy Shevchenko, Uwe Kleine-König
Cc: linux-iio, linux-kernel, Radu Sabau, Jonathan Cameron
From: Radu Sabau <radu.sabau@analog.com>
In ad_sigma_delta_single_conversion(), set_mode(AD_SD_MODE_IDLE) and
disable_one() were called from the out: block while keep_cs_asserted
was still true. This caused any SPI transfer issued by those callbacks
to carry cs_change=1, leaving CS permanently asserted after the
conversion. Fix by moving both calls into the out_unlock: block, after
keep_cs_asserted is cleared, matching the pattern already used in
ad_sd_calibrate().
In the error path of ad_sd_buffer_postenable(), if an operation fails
after set_mode(AD_SD_MODE_CONTINUOUS) has already succeeded (e.g.
spi_offload_trigger_enable()), the device is left in continuous
conversion mode with CS physically asserted. Additionally,
bus_locked remaining true after spi_bus_unlock() causes subsequent
SPI operations to call spi_sync_locked() without the bus lock actually
held, allowing concurrent SPI access.
Fix the error path by clearing keep_cs_asserted first, then calling
set_mode(AD_SD_MODE_IDLE) to revert the device mode and deassert CS,
then clearing bus_locked before releasing the bus.
For devices that implement neither set_mode nor disable_one (such as
MAX11205, which has no physical CS pin), no SPI transfer is issued
during cleanup and the cs_change flag has no effect on any physical
line.
Signed-off-by: Radu Sabau <radu.sabau@analog.com>
---
drivers/iio/adc/ad_sigma_delta.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/iio/adc/ad_sigma_delta.c b/drivers/iio/adc/ad_sigma_delta.c
index a955556f9ec8..651ade67ad2e 100644
--- a/drivers/iio/adc/ad_sigma_delta.c
+++ b/drivers/iio/adc/ad_sigma_delta.c
@@ -441,11 +441,10 @@ int ad_sigma_delta_single_conversion(struct iio_dev *indio_dev,
out:
ad_sd_disable_irq(sigma_delta);
- ad_sigma_delta_set_mode(sigma_delta, AD_SD_MODE_IDLE);
- ad_sigma_delta_disable_one(sigma_delta, chan->address);
-
out_unlock:
sigma_delta->keep_cs_asserted = false;
+ ad_sigma_delta_set_mode(sigma_delta, AD_SD_MODE_IDLE);
+ ad_sigma_delta_disable_one(sigma_delta, chan->address);
sigma_delta->bus_locked = false;
spi_bus_unlock(sigma_delta->spi->controller);
out_release:
@@ -578,6 +577,9 @@ static int ad_sd_buffer_postenable(struct iio_dev *indio_dev)
return 0;
err_unlock:
+ sigma_delta->keep_cs_asserted = false;
+ ad_sigma_delta_set_mode(sigma_delta, AD_SD_MODE_IDLE);
+ sigma_delta->bus_locked = false;
spi_bus_unlock(sigma_delta->spi->controller);
spi_unoptimize_message(&sigma_delta->sample_msg);
--
2.43.0
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v4 2/2] iio: adc: ad_sigma_delta: fix clear_pending_event for registerless devices
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 ` Radu Sabau via B4 Relay
2026-05-22 13:51 ` Jonathan Cameron
1 sibling, 1 reply; 7+ messages in thread
From: Radu Sabau via B4 Relay @ 2026-05-21 10:30 UTC (permalink / raw)
To: Lars-Peter Clausen, Michael Hennerich, Jonathan Cameron,
David Lechner, Nuno Sá,
Andy Shevchenko, Uwe Kleine-König
Cc: linux-iio, linux-kernel, Radu Sabau, Jonathan Cameron
From: Radu Sabau <radu.sabau@analog.com>
ad_sigma_delta_clear_pending_event() falls through to the status register
read path for devices with has_registers = false and no rdy_gpiod. For
such devices, ad_sd_read_reg() skips the address byte entirely and clocks
raw MISO bytes with no address phase — making it byte-for-byte identical
to reading conversion data. If a pending conversion result is present,
this partially consumes it and corrupts the data stream for the subsequent
ad_sd_read_reg() call in ad_sigma_delta_single_conversion().
Furthermore, with num_resetclks = 0 on these devices, data_read_len
evaluates to 0. If the clocked byte has bit 7 clear, pending_event is set
and the code attempts memset(data + 2, 0xff, 0 - 1), overflowing to
SIZE_MAX and corrupting the heap.
Fix by returning 0 immediately when neither rdy_gpiod nor has_registers
is set. This is safe because the IRQ is requested with IRQF_NO_AUTOEN and
IRQ_DISABLE_UNLAZY, which keeps the hardware IRQ line unmasked even while
software-disabled. Any falling edge from a completed conversion is latched
by the IRQ controller. When ad_sd_enable_irq() is subsequently called the
latched edge fires immediately, and the existing ad_sd_read_reg() call in
ad_sigma_delta_single_conversion() reads the complete stale result from
the beginning with no prior partial clock corruption.
The same heap corruption is reachable on any device with rdy_gpiod set
but num_resetclks = 0: if the GPIO indicates a pending event, the drain
path executes memset(data + 2, 0xff, 0 - 1) regardless of has_registers.
Add an explicit data_read_len == 0 guard after the pending event check to
cover this path.
Signed-off-by: Radu Sabau <radu.sabau@analog.com>
---
drivers/iio/adc/ad_sigma_delta.c | 21 +++++++++++++++++++--
1 file changed, 19 insertions(+), 2 deletions(-)
diff --git a/drivers/iio/adc/ad_sigma_delta.c b/drivers/iio/adc/ad_sigma_delta.c
index 651ade67ad2e..d4ded8d6b211 100644
--- a/drivers/iio/adc/ad_sigma_delta.c
+++ b/drivers/iio/adc/ad_sigma_delta.c
@@ -262,11 +262,16 @@ static int ad_sigma_delta_clear_pending_event(struct ad_sigma_delta *sigma_delta
/*
* Read R̅D̅Y̅ pin (if possible) or status register to check if there is an
- * old event.
+ * old event. For devices with neither an RDY GPIO nor registers,
+ * ad_sd_read_reg() transmits no address byte and clocks raw MISO bytes,
+ * which is indistinguishable from reading conversion data and would
+ * partially consume a pending result. Skip the check for such devices;
+ * IRQ_DISABLE_UNLAZY ensures any pending falling edge is latched and
+ * fires naturally on the next ad_sd_enable_irq() call.
*/
if (sigma_delta->rdy_gpiod) {
pending_event = gpiod_get_value(sigma_delta->rdy_gpiod);
- } else {
+ } else if (sigma_delta->info->has_registers) {
unsigned int status_reg;
ret = ad_sd_read_reg(sigma_delta, AD_SD_REG_STATUS, 1, &status_reg);
@@ -274,11 +279,23 @@ static int ad_sigma_delta_clear_pending_event(struct ad_sigma_delta *sigma_delta
return ret;
pending_event = !(status_reg & AD_SD_REG_STATUS_RDY);
+ } else {
+ return 0;
}
if (!pending_event)
return 0;
+ /*
+ * With num_resetclks = 0, data_read_len is 0 and the drain sequence
+ * below would compute memset(data + 2, 0xff, 0 - 1), underflowing to
+ * SIZE_MAX and corrupting the heap. There is no safe way to drain the
+ * stale result without knowing the data register size, so return 0 and
+ * let the latched IRQ edge fire on the next ad_sd_enable_irq() call.
+ */
+ if (!data_read_len)
+ return 0;
+
/*
* In general the size of the data register is unknown. It varies from
* device to device, might be one byte longer if CONTROL.DATA_STATUS is
--
2.43.0
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH v4 2/2] iio: adc: ad_sigma_delta: fix clear_pending_event for registerless devices
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
0 siblings, 1 reply; 7+ messages in thread
From: Jonathan Cameron @ 2026-05-22 13:51 UTC (permalink / raw)
To: Radu Sabau via B4 Relay
Cc: radu.sabau, Lars-Peter Clausen, Michael Hennerich, David Lechner,
Nuno Sá,
Andy Shevchenko, Uwe Kleine-König, linux-iio, linux-kernel
On Thu, 21 May 2026 13:30:49 +0300
Radu Sabau via B4 Relay <devnull+radu.sabau.analog.com@kernel.org> wrote:
> From: Radu Sabau <radu.sabau@analog.com>
>
> ad_sigma_delta_clear_pending_event() falls through to the status register
> read path for devices with has_registers = false and no rdy_gpiod. For
> such devices, ad_sd_read_reg() skips the address byte entirely and clocks
> raw MISO bytes with no address phase — making it byte-for-byte identical
> to reading conversion data. If a pending conversion result is present,
> this partially consumes it and corrupts the data stream for the subsequent
> ad_sd_read_reg() call in ad_sigma_delta_single_conversion().
>
> Furthermore, with num_resetclks = 0 on these devices, data_read_len
> evaluates to 0. If the clocked byte has bit 7 clear, pending_event is set
> and the code attempts memset(data + 2, 0xff, 0 - 1), overflowing to
> SIZE_MAX and corrupting the heap.
>
> Fix by returning 0 immediately when neither rdy_gpiod nor has_registers
> is set. This is safe because the IRQ is requested with IRQF_NO_AUTOEN and
> IRQ_DISABLE_UNLAZY, which keeps the hardware IRQ line unmasked even while
> software-disabled. Any falling edge from a completed conversion is latched
> by the IRQ controller.
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/io53lznz3qp3jd5rohqsjhosnmdzd6d44sdbwu5jcfrs3rz2a2@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!
> When ad_sd_enable_irq() is subsequently called the
> latched edge fires immediately, and the existing ad_sd_read_reg() call in
> ad_sigma_delta_single_conversion() reads the complete stale result from
> the beginning with no prior partial clock corruption.
>
> The same heap corruption is reachable on any device with rdy_gpiod set
> but num_resetclks = 0: if the GPIO indicates a pending event, the drain
> path executes memset(data + 2, 0xff, 0 - 1) regardless of has_registers.
> Add an explicit data_read_len == 0 guard after the pending event check to
> cover this path.
>
> Signed-off-by: Radu Sabau <radu.sabau@analog.com>
> ---
> drivers/iio/adc/ad_sigma_delta.c | 21 +++++++++++++++++++--
> 1 file changed, 19 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/iio/adc/ad_sigma_delta.c b/drivers/iio/adc/ad_sigma_delta.c
> index 651ade67ad2e..d4ded8d6b211 100644
> --- a/drivers/iio/adc/ad_sigma_delta.c
> +++ b/drivers/iio/adc/ad_sigma_delta.c
> @@ -262,11 +262,16 @@ static int ad_sigma_delta_clear_pending_event(struct ad_sigma_delta *sigma_delta
>
> /*
> * Read R̅D̅Y̅ pin (if possible) or status register to check if there is an
> - * old event.
> + * old event. For devices with neither an RDY GPIO nor registers,
> + * ad_sd_read_reg() transmits no address byte and clocks raw MISO bytes,
> + * which is indistinguishable from reading conversion data and would
> + * partially consume a pending result. Skip the check for such devices;
> + * IRQ_DISABLE_UNLAZY ensures any pending falling edge is latched and
> + * fires naturally on the next ad_sd_enable_irq() call.
> */
> if (sigma_delta->rdy_gpiod) {
> pending_event = gpiod_get_value(sigma_delta->rdy_gpiod);
> - } else {
> + } else if (sigma_delta->info->has_registers) {
> unsigned int status_reg;
>
> ret = ad_sd_read_reg(sigma_delta, AD_SD_REG_STATUS, 1, &status_reg);
> @@ -274,11 +279,23 @@ static int ad_sigma_delta_clear_pending_event(struct ad_sigma_delta *sigma_delta
> return ret;
>
> pending_event = !(status_reg & AD_SD_REG_STATUS_RDY);
> + } else {
> + return 0;
> }
>
> if (!pending_event)
> return 0;
>
> + /*
> + * With num_resetclks = 0, data_read_len is 0 and the drain sequence
> + * below would compute memset(data + 2, 0xff, 0 - 1), underflowing to
> + * SIZE_MAX and corrupting the heap. There is no safe way to drain the
> + * stale result without knowing the data register size, so return 0 and
> + * let the latched IRQ edge fire on the next ad_sd_enable_irq() call.
> + */
> + if (!data_read_len)
> + return 0;
> +
> /*
> * In general the size of the data register is unknown. It varies from
> * device to device, might be one byte longer if CONTROL.DATA_STATUS is
>
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH v4 2/2] iio: adc: ad_sigma_delta: fix clear_pending_event for registerless devices
2026-05-22 13:51 ` Jonathan Cameron
@ 2026-05-22 15:20 ` Uwe Kleine-König
2026-05-25 12:16 ` Sabau, Radu bogdan
0 siblings, 1 reply; 7+ messages in thread
From: Uwe Kleine-König @ 2026-05-22 15:20 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Radu Sabau via B4 Relay, radu.sabau, Lars-Peter Clausen,
Michael Hennerich, David Lechner, Nuno Sá,
Andy Shevchenko, linux-iio, linux-kernel, Thomas Gleixner
[-- Attachment #1: Type: text/plain, Size: 2389 bytes --]
Hello,
[adding Thomas to the discussion as he also participated in the old
linked discussion]
On Fri, May 22, 2026 at 02:51:43PM +0100, Jonathan Cameron wrote:
> On Thu, 21 May 2026 13:30:49 +0300
> Radu Sabau via B4 Relay <devnull+radu.sabau.analog.com@kernel.org> wrote:
>
> > From: Radu Sabau <radu.sabau@analog.com>
> >
> > ad_sigma_delta_clear_pending_event() falls through to the status register
> > read path for devices with has_registers = false and no rdy_gpiod. For
> > such devices, ad_sd_read_reg() skips the address byte entirely and clocks
> > raw MISO bytes with no address phase — making it byte-for-byte identical
> > to reading conversion data. If a pending conversion result is present,
> > this partially consumes it and corrupts the data stream for the subsequent
> > ad_sd_read_reg() call in ad_sigma_delta_single_conversion().
> >
> > Furthermore, with num_resetclks = 0 on these devices, data_read_len
> > evaluates to 0. If the clocked byte has bit 7 clear, pending_event is set
> > and the code attempts memset(data + 2, 0xff, 0 - 1), overflowing to
> > SIZE_MAX and corrupting the heap.
> >
> > Fix by returning 0 immediately when neither rdy_gpiod nor has_registers
> > is set. This is safe because the IRQ is requested with IRQF_NO_AUTOEN and
> > IRQ_DISABLE_UNLAZY, which keeps the hardware IRQ line unmasked even while
> > software-disabled. Any falling edge from a completed conversion is latched
> > by the IRQ controller.
>
> 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/io53lznz3qp3jd5rohqsjhosnmdzd6d44sdbwu5jcfrs3rz2a2@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.
Best regards
Uwe
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* RE: [PATCH v4 2/2] iio: adc: ad_sigma_delta: fix clear_pending_event for registerless devices
2026-05-22 15:20 ` Uwe Kleine-König
@ 2026-05-25 12:16 ` Sabau, Radu bogdan
2026-05-26 12:39 ` Jonathan Cameron
0 siblings, 1 reply; 7+ messages in thread
From: Sabau, Radu bogdan @ 2026-05-25 12:16 UTC (permalink / raw)
To: Uwe Kleine König, Jonathan Cameron
Cc: Radu Sabau via B4 Relay, Lars-Peter Clausen, Hennerich, Michael,
David Lechner, Sa, Nuno, Andy Shevchenko, linux-iio,
linux-kernel, Thomas Gleixner
> -----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. 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.
With Best Regards,
Radu
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v4 2/2] iio: adc: ad_sigma_delta: fix clear_pending_event for registerless devices
2026-05-25 12:16 ` Sabau, Radu bogdan
@ 2026-05-26 12:39 ` Jonathan Cameron
0 siblings, 0 replies; 7+ messages in thread
From: Jonathan Cameron @ 2026-05-26 12:39 UTC (permalink / raw)
To: Sabau, Radu bogdan
Cc: Uwe Kleine König, Radu Sabau via B4 Relay,
Lars-Peter Clausen, Hennerich, Michael, David Lechner, Sa, Nuno,
Andy Shevchenko, linux-iio, linux-kernel, Thomas Gleixner
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
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2026-05-26 12:39 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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 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®