mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v3 0/2] iio: adc: ad_sigma_delta: fix CS assertion and registerless device handling
@ 2026-05-18  9:59 Radu Sabau via B4 Relay
  2026-05-18  9:59 ` [PATCH v3 1/2] iio: adc: ad_sigma_delta: fix CS held asserted and state leaks Radu Sabau via B4 Relay
  2026-05-18  9:59 ` [PATCH v3 2/2] iio: adc: ad_sigma_delta: fix clear_pending_event for registerless devices Radu Sabau via B4 Relay
  0 siblings, 2 replies; 5+ messages in thread
From: Radu Sabau via B4 Relay @ 2026-05-18  9:59 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, AD7191, AD7780).
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. This partially consumes any pending conversion result, corrupting
the data stream. With num_resetclks = 0 on these devices a further bug
triggers: if the clocked byte has bit 7 clear, a SIZE_MAX memset corrupts
the heap. The fix returns 0 immediately for such devices, relying on
IRQ_DISABLE_UNLAZY to latch any pending edge so it fires cleanly on the
next ad_sd_enable_irq() call.

Signed-off-by: Radu Sabau <radu.sabau@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 | 19 ++++++++++++++-----
 1 file changed, 14 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] 5+ messages in thread

* [PATCH v3 1/2] iio: adc: ad_sigma_delta: fix CS held asserted and state leaks
  2026-05-18  9:59 [PATCH v3 0/2] iio: adc: ad_sigma_delta: fix CS assertion and registerless device handling Radu Sabau via B4 Relay
@ 2026-05-18  9:59 ` Radu Sabau via B4 Relay
  2026-05-18  9:59 ` [PATCH v3 2/2] iio: adc: ad_sigma_delta: fix clear_pending_event for registerless devices Radu Sabau via B4 Relay
  1 sibling, 0 replies; 5+ messages in thread
From: Radu Sabau via B4 Relay @ 2026-05-18  9:59 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 | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/adc/ad_sigma_delta.c b/drivers/iio/adc/ad_sigma_delta.c
index a955556f9ec8..a33a7e8c264f 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,8 @@ static int ad_sd_buffer_postenable(struct iio_dev *indio_dev)
 	return 0;
 
 err_unlock:
+	sigma_delta->keep_cs_asserted = false;
+	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] 5+ messages in thread

* [PATCH v3 2/2] iio: adc: ad_sigma_delta: fix clear_pending_event for registerless devices
  2026-05-18  9:59 [PATCH v3 0/2] iio: adc: ad_sigma_delta: fix CS assertion and registerless device handling Radu Sabau via B4 Relay
  2026-05-18  9:59 ` [PATCH v3 1/2] iio: adc: ad_sigma_delta: fix CS held asserted and state leaks Radu Sabau via B4 Relay
@ 2026-05-18  9:59 ` Radu Sabau via B4 Relay
  2026-05-18 12:55   ` Sabau, Radu bogdan
  1 sibling, 1 reply; 5+ messages in thread
From: Radu Sabau via B4 Relay @ 2026-05-18  9:59 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.

Signed-off-by: Radu Sabau <radu.sabau@analog.com>
---
 drivers/iio/adc/ad_sigma_delta.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/adc/ad_sigma_delta.c b/drivers/iio/adc/ad_sigma_delta.c
index a33a7e8c264f..88dcaa910b2d 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,6 +279,8 @@ 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)
@@ -578,6 +585,7 @@ static int ad_sd_buffer_postenable(struct iio_dev *indio_dev)
 
 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] 5+ messages in thread

* RE: [PATCH v3 2/2] iio: adc: ad_sigma_delta: fix clear_pending_event for registerless devices
  2026-05-18  9:59 ` [PATCH v3 2/2] iio: adc: ad_sigma_delta: fix clear_pending_event for registerless devices Radu Sabau via B4 Relay
@ 2026-05-18 12:55   ` Sabau, Radu bogdan
  2026-05-20 12:23     ` Jonathan Cameron
  0 siblings, 1 reply; 5+ messages in thread
From: Sabau, Radu bogdan @ 2026-05-18 12:55 UTC (permalink / raw)
  To: Sabau, Radu bogdan, Lars-Peter Clausen, Hennerich, Michael,
	Jonathan Cameron, David Lechner, Sa, Nuno, Andy Shevchenko,
	Uwe Kleine König
  Cc: linux-iio, linux-kernel

Hi everyone, I am responding to Sashiko's review here:

> -----Original Message-----
> From: Radu Sabau via B4 Relay <devnull+radu.sabau.analog.com@kernel.org>
> Sent: Monday, May 18, 2026 12:59 PM

...

> +	} 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,6 +279,8 @@ 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;
>  	}

He is right about this one, the heap corruption may not be fully solved as a
registerless device with num_resetclks = 0 confiugred with rdy-gpios could
still have data_read_len = 0 further if GPIO indicates a pending event.

Perhaps I data_read_len == 0 should indeed be checked before attempting the
dummy SPI transfer as suggested, right?

> 
>  	if (!pending_event)
> @@ -578,6 +585,7 @@ static int ad_sd_buffer_postenable(struct iio_dev
> *indio_dev)
> 
>  err_unlock:
>  	sigma_delta->keep_cs_asserted = false;
> +	ad_sigma_delta_set_mode(sigma_delta, AD_SD_MODE_IDLE);

Also right about this one, it belongs in previous patch...

>  	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] 5+ messages in thread

* Re: [PATCH v3 2/2] iio: adc: ad_sigma_delta: fix clear_pending_event for registerless devices
  2026-05-18 12:55   ` Sabau, Radu bogdan
@ 2026-05-20 12:23     ` Jonathan Cameron
  0 siblings, 0 replies; 5+ messages in thread
From: Jonathan Cameron @ 2026-05-20 12:23 UTC (permalink / raw)
  To: Sabau, Radu bogdan
  Cc: Lars-Peter Clausen, Hennerich, Michael, David Lechner, Sa, Nuno,
	Andy Shevchenko, Uwe Kleine König, linux-iio, linux-kernel

On Mon, 18 May 2026 12:55:33 +0000
"Sabau, Radu bogdan" <Radu.Sabau@analog.com> wrote:

> Hi everyone, I am responding to Sashiko's review here:
> 
> > -----Original Message-----
> > From: Radu Sabau via B4 Relay <devnull+radu.sabau.analog.com@kernel.org>
> > Sent: Monday, May 18, 2026 12:59 PM  
> 
> ...
> 
> > +	} 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,6 +279,8 @@ 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;
> >  	}  
> 
> He is right about this one, the heap corruption may not be fully solved as a
> registerless device with num_resetclks = 0 confiugred with rdy-gpios could
> still have data_read_len = 0 further if GPIO indicates a pending event.
> 
> Perhaps I data_read_len == 0 should indeed be checked before attempting the
> dummy SPI transfer as suggested, right?
Indeed seems valid.   Make sure to leave some comments around on the
'why' of the extra check

> 
> > 
> >  	if (!pending_event)
> > @@ -578,6 +585,7 @@ static int ad_sd_buffer_postenable(struct iio_dev
> > *indio_dev)
> > 
> >  err_unlock:
> >  	sigma_delta->keep_cs_asserted = false;
> > +	ad_sigma_delta_set_mode(sigma_delta, AD_SD_MODE_IDLE);  
> 
> Also right about this one, it belongs in previous patch...
> 
> >  	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] 5+ messages in thread

end of thread, other threads:[~2026-05-20 12:23 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-05-18  9:59 [PATCH v3 0/2] iio: adc: ad_sigma_delta: fix CS assertion and registerless device handling Radu Sabau via B4 Relay
2026-05-18  9:59 ` [PATCH v3 1/2] iio: adc: ad_sigma_delta: fix CS held asserted and state leaks Radu Sabau via B4 Relay
2026-05-18  9:59 ` [PATCH v3 2/2] iio: adc: ad_sigma_delta: fix clear_pending_event for registerless devices Radu Sabau via B4 Relay
2026-05-18 12:55   ` Sabau, Radu bogdan
2026-05-20 12:23     ` 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®