Commit f522589c authored by Uwe Kleine-König's avatar Uwe Kleine-König Committed by Jonathan Cameron
Browse files

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: af300848 ("iio:adc: Add common code for ADI Sigma Delta devices")
Signed-off-by: default avatarUwe Kleine-König <u.kleine-koenig@baylibre.com>
Link: https://patch.msgid.link/9e6def47e2e773e0e15b7a2c29d22629b53d91b1.1733504533.git.u.kleine-koenig@baylibre.com


Signed-off-by: default avatarJonathan Cameron <Jonathan.Cameron@huawei.com>
parent 90b8b2fe

drivers/iio/adc/ad_sigma_delta.c

+35 −21
Original line number Diff line number Diff line
@@ -202,6 +202,27 @@ int ad_sd_reset(struct ad_sigma_delta *sigma_delta, 
}

EXPORT_SYMBOL_NS_GPL(ad_sd_reset, "IIO_AD_SIGMA_DELTA");



static bool ad_sd_disable_irq(struct ad_sigma_delta *sigma_delta)

{

	guard(spinlock_irqsave)(&sigma_delta->irq_lock);



	/* It's already off, return false to indicate nothing was changed */

	if (sigma_delta->irq_dis)

		return false;



	sigma_delta->irq_dis = true;

	disable_irq_nosync(sigma_delta->irq_line);

	return true;

}



static void ad_sd_enable_irq(struct ad_sigma_delta *sigma_delta)

{

	guard(spinlock_irqsave)(&sigma_delta->irq_lock);



	sigma_delta->irq_dis = false;

	enable_irq(sigma_delta->irq_line);

}



int ad_sd_calibrate(struct ad_sigma_delta *sigma_delta,

	unsigned int mode, unsigned int channel)

{
@@ -221,12 +242,10 @@ int ad_sd_calibrate(struct ad_sigma_delta *sigma_delta, 
	if (ret < 0)

		goto out;



	sigma_delta->irq_dis = false;

	enable_irq(sigma_delta->irq_line);

	ad_sd_enable_irq(sigma_delta);

	time_left = wait_for_completion_timeout(&sigma_delta->completion, 2 * HZ);

	if (time_left == 0) {

		sigma_delta->irq_dis = true;

		disable_irq_nosync(sigma_delta->irq_line);

		ad_sd_disable_irq(sigma_delta);

		ret = -EIO;

	} else {

		ret = 0;
@@ -294,8 +313,7 @@ int ad_sigma_delta_single_conversion(struct iio_dev *indio_dev, 


	ad_sigma_delta_set_mode(sigma_delta, AD_SD_MODE_SINGLE);



	sigma_delta->irq_dis = false;

	enable_irq(sigma_delta->irq_line);

	ad_sd_enable_irq(sigma_delta);

	ret = wait_for_completion_interruptible_timeout(

			&sigma_delta->completion, HZ);
@@ -314,10 +332,7 @@ int ad_sigma_delta_single_conversion(struct iio_dev *indio_dev, 
		&raw_sample);



out:

	if (!sigma_delta->irq_dis) {

		disable_irq_nosync(sigma_delta->irq_line);

		sigma_delta->irq_dis = true;

	}

	ad_sd_disable_irq(sigma_delta);



	sigma_delta->keep_cs_asserted = false;

	ad_sigma_delta_set_mode(sigma_delta, AD_SD_MODE_IDLE);
@@ -396,8 +411,7 @@ static int ad_sd_buffer_postenable(struct iio_dev *indio_dev) 
	if (ret)

		goto err_unlock;



	sigma_delta->irq_dis = false;

	enable_irq(sigma_delta->irq_line);

	ad_sd_enable_irq(sigma_delta);



	return 0;
@@ -414,10 +428,7 @@ static int ad_sd_buffer_postdisable(struct iio_dev *indio_dev) 
	reinit_completion(&sigma_delta->completion);

	wait_for_completion_timeout(&sigma_delta->completion, HZ);



	if (!sigma_delta->irq_dis) {

		disable_irq_nosync(sigma_delta->irq_line);

		sigma_delta->irq_dis = true;

	}

	ad_sd_disable_irq(sigma_delta);



	sigma_delta->keep_cs_asserted = false;

	ad_sigma_delta_set_mode(sigma_delta, AD_SD_MODE_IDLE);
@@ -516,8 +527,7 @@ static irqreturn_t ad_sd_trigger_handler(int irq, void *p) 


irq_handled:

	iio_trigger_notify_done(indio_dev->trig);

	sigma_delta->irq_dis = false;

	enable_irq(sigma_delta->irq_line);

	ad_sd_enable_irq(sigma_delta);



	return IRQ_HANDLED;

}
@@ -551,11 +561,13 @@ static irqreturn_t ad_sd_data_rdy_trig_poll(int irq, void *private) 
	 * So read the MOSI line as GPIO (if available) and only trigger the irq

	 * if the line is active. Without such a GPIO assume this is a valid

	 * interrupt.

	 *

	 * Also as disable_irq_nosync() is used to disable the irq, only act if

	 * the irq wasn't disabled before.

	 */

	if (!sigma_delta->rdy_gpiod || gpiod_get_value(sigma_delta->rdy_gpiod)) {

	if ((!sigma_delta->rdy_gpiod || gpiod_get_value(sigma_delta->rdy_gpiod)) &&

	    ad_sd_disable_irq(sigma_delta)) {

		complete(&sigma_delta->completion);

		disable_irq_nosync(irq);

		sigma_delta->irq_dis = true;

		iio_trigger_poll(sigma_delta->trig);



		return IRQ_HANDLED;
@@ -691,6 +703,8 @@ int ad_sd_init(struct ad_sigma_delta *sigma_delta, struct iio_dev *indio_dev, 
		}

	}



	spin_lock_init(&sigma_delta->irq_lock);



	if (info->irq_line)

		sigma_delta->irq_line = info->irq_line;

	else

include/linux/iio/adc/ad_sigma_delta.h

+1 −0
Original line number Diff line number Diff line
@@ -86,6 +86,7 @@ struct ad_sigma_delta { 


/* private: */

	struct completion	completion;

	spinlock_t		irq_lock; /* protects .irq_dis and irq en/disable state */

	bool			irq_dis;



	bool			bus_locked;