Skip to content

Commit 3e95ba8

Browse files
ukleineknunojsa
authored andcommitted
iio: adc: ad_sigma_delta: Fix a race condition
The ad_sigma_delta driver helper uses irq_disable_nosync(). With that one it is possible that the irq handler still runs after the irq_disable_nosync() function call returns. Also to properly synchronize irq disabling in the different threads proper locking is needed and because it's unclear if the irq handler's irq_disable_nosync() call comes first or the one in the enabler's error path, all code locations that disable the irq must check for .irq_dis first to ensure there is exactly one disable call per enable call. So add a spinlock to the struct ad_sigma_delta and use it to synchronize irq enabling and disabling. Also only act in the irq handler if the irq is still enabled. Fixes: af30084 ("iio:adc: Add common code for ADI Sigma Delta devices") Signed-off-by: Uwe Kleine-König <u.kleine-koenig@baylibre.com> Link: https://patch.msgid.link/9e6def47e2e773e0e15b7a2c29d22629b53d91b1.1733504533.git.u.kleine-koenig@baylibre.com Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> Origin: next-20241216~37^2~20, commit:f522589c139debb8af56dbead0c6e9dfca2d5ce4
1 parent bc8fdf4 commit 3e95ba8

File tree

2 files changed

+37
-23
lines changed

2 files changed

+37
-23
lines changed

drivers/iio/adc/ad_sigma_delta.c

Lines changed: 36 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -203,6 +203,27 @@ int ad_sd_reset(struct ad_sigma_delta *sigma_delta,
203203
}
204204
EXPORT_SYMBOL_NS_GPL(ad_sd_reset, IIO_AD_SIGMA_DELTA);
205205

206+
static bool ad_sd_disable_irq(struct ad_sigma_delta *sigma_delta)
207+
{
208+
guard(spinlock_irqsave)(&sigma_delta->irq_lock);
209+
210+
/* It's already off, return false to indicate nothing was changed */
211+
if (sigma_delta->irq_dis)
212+
return false;
213+
214+
sigma_delta->irq_dis = true;
215+
disable_irq_nosync(sigma_delta->irq_line);
216+
return true;
217+
}
218+
219+
static void ad_sd_enable_irq(struct ad_sigma_delta *sigma_delta)
220+
{
221+
guard(spinlock_irqsave)(&sigma_delta->irq_lock);
222+
223+
sigma_delta->irq_dis = false;
224+
enable_irq(sigma_delta->irq_line);
225+
}
226+
206227
int ad_sd_calibrate(struct ad_sigma_delta *sigma_delta,
207228
unsigned int mode, unsigned int channel)
208229
{
@@ -222,12 +243,10 @@ int ad_sd_calibrate(struct ad_sigma_delta *sigma_delta,
222243
if (ret < 0)
223244
goto out;
224245

225-
sigma_delta->irq_dis = false;
226-
enable_irq(sigma_delta->irq_line);
246+
ad_sd_enable_irq(sigma_delta);
227247
timeout = wait_for_completion_timeout(&sigma_delta->completion, 2 * HZ);
228248
if (timeout == 0) {
229-
sigma_delta->irq_dis = true;
230-
disable_irq_nosync(sigma_delta->irq_line);
249+
ad_sd_disable_irq(sigma_delta);
231250
ret = -EIO;
232251
} else {
233252
ret = 0;
@@ -295,8 +314,7 @@ int ad_sigma_delta_single_conversion(struct iio_dev *indio_dev,
295314

296315
ad_sigma_delta_set_mode(sigma_delta, AD_SD_MODE_SINGLE);
297316

298-
sigma_delta->irq_dis = false;
299-
enable_irq(sigma_delta->irq_line);
317+
ad_sd_enable_irq(sigma_delta);
300318
ret = wait_for_completion_interruptible_timeout(
301319
&sigma_delta->completion, HZ);
302320

@@ -315,10 +333,7 @@ int ad_sigma_delta_single_conversion(struct iio_dev *indio_dev,
315333
&raw_sample);
316334

317335
out:
318-
if (!sigma_delta->irq_dis) {
319-
disable_irq_nosync(sigma_delta->irq_line);
320-
sigma_delta->irq_dis = true;
321-
}
336+
ad_sd_disable_irq(sigma_delta);
322337

323338
sigma_delta->keep_cs_asserted = false;
324339
ad_sigma_delta_set_mode(sigma_delta, AD_SD_MODE_IDLE);
@@ -451,10 +466,8 @@ static int ad_sd_buffer_postenable(struct iio_dev *indio_dev)
451466
* Differs from upstream because of the spi engine support and we want to enable
452467
* the irq only after setting AD_SD_MODE_CONTINUOUS (as in the upstream lib)
453468
*/
454-
if (iio_device_get_current_mode(indio_dev) != INDIO_BUFFER_HARDWARE) {
455-
sigma_delta->irq_dis = false;
456-
enable_irq(sigma_delta->irq_line);
457-
}
469+
if (iio_device_get_current_mode(indio_dev) != INDIO_BUFFER_HARDWARE)
470+
ad_sd_enable_irq(sigma_delta);
458471

459472
return 0;
460473

@@ -474,10 +487,7 @@ static int ad_sd_buffer_postdisable(struct iio_dev *indio_dev)
474487
reinit_completion(&sigma_delta->completion);
475488
wait_for_completion_timeout(&sigma_delta->completion, HZ);
476489

477-
if (!sigma_delta->irq_dis) {
478-
disable_irq_nosync(sigma_delta->irq_line);
479-
sigma_delta->irq_dis = true;
480-
}
490+
ad_sd_disable_irq(sigma_delta);
481491
}
482492

483493
sigma_delta->keep_cs_asserted = false;
@@ -577,8 +587,7 @@ static irqreturn_t ad_sd_trigger_handler(int irq, void *p)
577587

578588
irq_handled:
579589
iio_trigger_notify_done(indio_dev->trig);
580-
sigma_delta->irq_dis = false;
581-
enable_irq(sigma_delta->irq_line);
590+
ad_sd_enable_irq(sigma_delta);
582591

583592
return IRQ_HANDLED;
584593
}
@@ -612,11 +621,13 @@ static irqreturn_t ad_sd_data_rdy_trig_poll(int irq, void *private)
612621
* So read the MOSI line as GPIO (if available) and only trigger the irq
613622
* if the line is active. Without such a GPIO assume this is a valid
614623
* interrupt.
624+
*
625+
* Also as disable_irq_nosync() is used to disable the irq, only act if
626+
* the irq wasn't disabled before.
615627
*/
616-
if (!sigma_delta->rdy_gpiod || gpiod_get_value(sigma_delta->rdy_gpiod)) {
628+
if ((!sigma_delta->rdy_gpiod || gpiod_get_value(sigma_delta->rdy_gpiod)) &&
629+
ad_sd_disable_irq(sigma_delta)) {
617630
complete(&sigma_delta->completion);
618-
disable_irq_nosync(irq);
619-
sigma_delta->irq_dis = true;
620631
iio_trigger_poll(sigma_delta->trig);
621632

622633
return IRQ_HANDLED;
@@ -750,6 +761,8 @@ int ad_sd_init(struct ad_sigma_delta *sigma_delta, struct iio_dev *indio_dev,
750761
}
751762
}
752763

764+
spin_lock_init(&sigma_delta->irq_lock);
765+
753766
if (info->irq_line)
754767
sigma_delta->irq_line = info->irq_line;
755768
else

include/linux/iio/adc/ad_sigma_delta.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,7 @@ struct ad_sigma_delta {
8484

8585
/* private: */
8686
struct completion completion;
87+
spinlock_t irq_lock; /* protects .irq_dis and irq en/disable state */
8788
bool irq_dis;
8889

8990
bool bus_locked;

0 commit comments

Comments
 (0)