diff mbox series

[03/11] staging: iio: adc: ad7606: Use wait-for-completion handler

Message ID 1544705183-13288-4-git-send-email-stefan.popa@analog.com (mailing list archive)
State New, archived
Headers show
Series staging: iio: ad7606: Move out of staging | expand

Commit Message

Stefan Popa Dec. 13, 2018, 12:46 p.m. UTC
This patch replaces the use of wait_event_interruptible() with
wait_for_completion_timeout() when reading the result of a single
conversion. In this way, if the interrupt never occurs, the program will
not remain blocked.

Signed-off-by: Stefan Popa <stefan.popa@analog.com>
---
 drivers/staging/iio/adc/ad7606.c | 14 +++++++-------
 drivers/staging/iio/adc/ad7606.h |  6 ++----
 2 files changed, 9 insertions(+), 11 deletions(-)

Comments

Jonathan Cameron Dec. 16, 2018, 1:41 p.m. UTC | #1
On Thu, 13 Dec 2018 14:46:15 +0200
Stefan Popa <stefan.popa@analog.com> wrote:

> This patch replaces the use of wait_event_interruptible() with
> wait_for_completion_timeout() when reading the result of a single
> conversion. In this way, if the interrupt never occurs, the program will
> not remain blocked.
> 
> Signed-off-by: Stefan Popa <stefan.popa@analog.com>
Looks good and not related to the previous 2 that obviously need
a v2.  I'll cherry pick what I can from the series to minimise
the number that you need to send again.

Applied to the togreg branch of iio.git and pushed out as testing
for the autobuilders to play with it.

Thanks,

Jonathan

> ---
>  drivers/staging/iio/adc/ad7606.c | 14 +++++++-------
>  drivers/staging/iio/adc/ad7606.h |  6 ++----
>  2 files changed, 9 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/staging/iio/adc/ad7606.c b/drivers/staging/iio/adc/ad7606.c
> index aa5ab1e..4b1bc20 100644
> --- a/drivers/staging/iio/adc/ad7606.c
> +++ b/drivers/staging/iio/adc/ad7606.c
> @@ -118,12 +118,13 @@ static int ad7606_scan_direct(struct iio_dev *indio_dev, unsigned int ch)
>  	struct ad7606_state *st = iio_priv(indio_dev);
>  	int ret;
>  
> -	st->done = false;
>  	gpiod_set_value(st->gpio_convst, 1);
> -
> -	ret = wait_event_interruptible(st->wq_data_avail, st->done);
> -	if (ret)
> +	ret = wait_for_completion_timeout(&st->completion,
> +					  msecs_to_jiffies(1000));
> +	if (!ret) {
> +		ret = -ETIMEDOUT;
>  		goto error_ret;
> +	}
>  
>  	ret = ad7606_read_samples(st);
>  	if (ret == 0)
> @@ -388,8 +389,7 @@ static irqreturn_t ad7606_interrupt(int irq, void *dev_id)
>  	if (iio_buffer_enabled(indio_dev)) {
>  		schedule_work(&st->poll_work);
>  	} else {
> -		st->done = true;
> -		wake_up_interruptible(&st->wq_data_avail);
> +		complete(&st->completion);
>  	}
>  
>  	return IRQ_HANDLED;
> @@ -473,7 +473,7 @@ int ad7606_probe(struct device *dev, int irq, void __iomem *base_address,
>  	indio_dev->channels = st->chip_info->channels;
>  	indio_dev->num_channels = st->chip_info->num_channels;
>  
> -	init_waitqueue_head(&st->wq_data_avail);
> +	init_completion(&st->completion);
>  
>  	ret = ad7606_reset(st);
>  	if (ret)
> diff --git a/drivers/staging/iio/adc/ad7606.h b/drivers/staging/iio/adc/ad7606.h
> index e365fa0..cf20ca2 100644
> --- a/drivers/staging/iio/adc/ad7606.h
> +++ b/drivers/staging/iio/adc/ad7606.h
> @@ -28,11 +28,9 @@ struct ad7606_chip_info {
>   * @reg		regulator info for the the power supply of the device
>   * @poll_work		work struct for continuously reading data from the device
>   *			into an IIO triggered buffer
> - * @wq_data_avail	wait queue struct for buffer mode
>   * @bops		bus operations (SPI or parallel)
>   * @range		voltage range selection, selects which scale to apply
>   * @oversampling	oversampling selection
> - * @done		marks whether reading data is done
>   * @base_address	address from where to read data in parallel operation
>   * @lock		protect sensor state from concurrent accesses to GPIOs
>   * @gpio_convst	GPIO descriptor for conversion start signal (CONVST)
> @@ -43,6 +41,7 @@ struct ad7606_chip_info {
>   * @gpio_frstdata	GPIO descriptor for reading from device when data
>   *			is being read on the first channel
>   * @gpio_os		GPIO descriptors to control oversampling on the device
> + * @complete		completion to indicate end of conversion
>   * @data		buffer for reading data from the device
>   */
>  
> @@ -51,11 +50,9 @@ struct ad7606_state {
>  	const struct ad7606_chip_info	*chip_info;
>  	struct regulator		*reg;
>  	struct work_struct		poll_work;
> -	wait_queue_head_t		wq_data_avail;
>  	const struct ad7606_bus_ops	*bops;
>  	unsigned int			range;
>  	unsigned int			oversampling;
> -	bool				done;
>  	void __iomem			*base_address;
>  
>  	struct mutex			lock; /* protect sensor state */
> @@ -65,6 +62,7 @@ struct ad7606_state {
>  	struct gpio_desc		*gpio_standby;
>  	struct gpio_desc		*gpio_frstdata;
>  	struct gpio_descs		*gpio_os;
> +	struct completion		completion;
>  
>  	/*
>  	 * DMA (thus cache coherency maintenance) requires the
diff mbox series

Patch

diff --git a/drivers/staging/iio/adc/ad7606.c b/drivers/staging/iio/adc/ad7606.c
index aa5ab1e..4b1bc20 100644
--- a/drivers/staging/iio/adc/ad7606.c
+++ b/drivers/staging/iio/adc/ad7606.c
@@ -118,12 +118,13 @@  static int ad7606_scan_direct(struct iio_dev *indio_dev, unsigned int ch)
 	struct ad7606_state *st = iio_priv(indio_dev);
 	int ret;
 
-	st->done = false;
 	gpiod_set_value(st->gpio_convst, 1);
-
-	ret = wait_event_interruptible(st->wq_data_avail, st->done);
-	if (ret)
+	ret = wait_for_completion_timeout(&st->completion,
+					  msecs_to_jiffies(1000));
+	if (!ret) {
+		ret = -ETIMEDOUT;
 		goto error_ret;
+	}
 
 	ret = ad7606_read_samples(st);
 	if (ret == 0)
@@ -388,8 +389,7 @@  static irqreturn_t ad7606_interrupt(int irq, void *dev_id)
 	if (iio_buffer_enabled(indio_dev)) {
 		schedule_work(&st->poll_work);
 	} else {
-		st->done = true;
-		wake_up_interruptible(&st->wq_data_avail);
+		complete(&st->completion);
 	}
 
 	return IRQ_HANDLED;
@@ -473,7 +473,7 @@  int ad7606_probe(struct device *dev, int irq, void __iomem *base_address,
 	indio_dev->channels = st->chip_info->channels;
 	indio_dev->num_channels = st->chip_info->num_channels;
 
-	init_waitqueue_head(&st->wq_data_avail);
+	init_completion(&st->completion);
 
 	ret = ad7606_reset(st);
 	if (ret)
diff --git a/drivers/staging/iio/adc/ad7606.h b/drivers/staging/iio/adc/ad7606.h
index e365fa0..cf20ca2 100644
--- a/drivers/staging/iio/adc/ad7606.h
+++ b/drivers/staging/iio/adc/ad7606.h
@@ -28,11 +28,9 @@  struct ad7606_chip_info {
  * @reg		regulator info for the the power supply of the device
  * @poll_work		work struct for continuously reading data from the device
  *			into an IIO triggered buffer
- * @wq_data_avail	wait queue struct for buffer mode
  * @bops		bus operations (SPI or parallel)
  * @range		voltage range selection, selects which scale to apply
  * @oversampling	oversampling selection
- * @done		marks whether reading data is done
  * @base_address	address from where to read data in parallel operation
  * @lock		protect sensor state from concurrent accesses to GPIOs
  * @gpio_convst	GPIO descriptor for conversion start signal (CONVST)
@@ -43,6 +41,7 @@  struct ad7606_chip_info {
  * @gpio_frstdata	GPIO descriptor for reading from device when data
  *			is being read on the first channel
  * @gpio_os		GPIO descriptors to control oversampling on the device
+ * @complete		completion to indicate end of conversion
  * @data		buffer for reading data from the device
  */
 
@@ -51,11 +50,9 @@  struct ad7606_state {
 	const struct ad7606_chip_info	*chip_info;
 	struct regulator		*reg;
 	struct work_struct		poll_work;
-	wait_queue_head_t		wq_data_avail;
 	const struct ad7606_bus_ops	*bops;
 	unsigned int			range;
 	unsigned int			oversampling;
-	bool				done;
 	void __iomem			*base_address;
 
 	struct mutex			lock; /* protect sensor state */
@@ -65,6 +62,7 @@  struct ad7606_state {
 	struct gpio_desc		*gpio_standby;
 	struct gpio_desc		*gpio_frstdata;
 	struct gpio_descs		*gpio_os;
+	struct completion		completion;
 
 	/*
 	 * DMA (thus cache coherency maintenance) requires the