diff mbox series

[35/40] iio: adc: ti_am335x_adc: Wait the idle state to avoid stalls

Message ID 20210825152518.379386-36-miquel.raynal@bootlin.com
State Superseded
Headers show
Series TI AM437X ADC1 | expand

Commit Message

Miquel Raynal Aug. 25, 2021, 3:25 p.m. UTC
A simple:
$ cat /sys/bus/iio/devices/iio\:deviceX/in_voltage*_raw
can stall forever. It seems that it comes from the fact that the
internal state machine does not have enough time to return to its idle
state in this situation before receiving another request, leading to an
internal stall.

Add a tiadc_wait_idle() helper to ensure no new conversion is requested
while the FSM is still busy.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 drivers/iio/adc/ti_am335x_adc.c | 24 ++++++++++++++++++++++--
 1 file changed, 22 insertions(+), 2 deletions(-)

Comments

Jonathan Cameron Aug. 30, 2021, 2:22 p.m. UTC | #1
On Wed, 25 Aug 2021 17:25:13 +0200
Miquel Raynal <miquel.raynal@bootlin.com> wrote:

> A simple:

> $ cat /sys/bus/iio/devices/iio\:deviceX/in_voltage*_raw

> can stall forever. It seems that it comes from the fact that the

> internal state machine does not have enough time to return to its idle

> state in this situation before receiving another request, leading to an

> internal stall.


Interesting and non obvious... Good work figuring that out.

> 

> Add a tiadc_wait_idle() helper to ensure no new conversion is requested

> while the FSM is still busy.

> 

> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>


Acked-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>


> ---

>  drivers/iio/adc/ti_am335x_adc.c | 24 ++++++++++++++++++++++--

>  1 file changed, 22 insertions(+), 2 deletions(-)

> 

> diff --git a/drivers/iio/adc/ti_am335x_adc.c b/drivers/iio/adc/ti_am335x_adc.c

> index ebf6326af60d..d4619fd6fd54 100644

> --- a/drivers/iio/adc/ti_am335x_adc.c

> +++ b/drivers/iio/adc/ti_am335x_adc.c

> @@ -17,6 +17,7 @@

>  #include <linux/of_device.h>

>  #include <linux/iio/machine.h>

>  #include <linux/iio/driver.h>

> +#include <linux/iopoll.h>

>  

>  #include <linux/mfd/ti_am335x_tscadc.h>

>  #include <linux/iio/buffer.h>

> @@ -94,6 +95,15 @@ static u32 get_adc_step_bit(struct tiadc_device *adc_dev, int chan)

>  	return 1 << adc_dev->channel_step[chan];

>  }

>  

> +static int tiadc_wait_idle(struct tiadc_device *adc_dev)

> +{

> +	u32 val;

> +

> +	return readl_poll_timeout(adc_dev->mfd_tscadc->tscadc_base + REG_ADCFSM,

> +				  val, !(val & SEQ_STATUS), 10,

> +				  IDLE_TIMEOUT_MS * 1000 * adc_dev->channels);

> +}

> +

>  static void tiadc_step_config(struct iio_dev *indio_dev)

>  {

>  	struct tiadc_device *adc_dev = iio_priv(indio_dev);

> @@ -268,6 +278,11 @@ static int tiadc_buffer_preenable(struct iio_dev *indio_dev)

>  {

>  	struct tiadc_device *adc_dev = iio_priv(indio_dev);

>  	int i, fifo1count;

> +	int ret;

> +

> +	ret = tiadc_wait_idle(adc_dev);

> +	if (ret)

> +		return ret;

>  

>  	tiadc_writel(adc_dev, REG_IRQCLR,

>  		     IRQENB_FIFO1THRES | IRQENB_FIFO1OVRRUN |

> @@ -419,12 +434,12 @@ static int tiadc_read_raw(struct iio_dev *indio_dev,

>  			  long mask)

>  {

>  	struct tiadc_device *adc_dev = iio_priv(indio_dev);

> -	int ret = IIO_VAL_INT;

>  	int i, map_val;

>  	unsigned int fifo1count, read, stepid;

>  	bool found = false;

>  	u32 step_en;

>  	unsigned long timeout;

> +	int ret;

>  

>  	if (iio_buffer_enabled(indio_dev))

>  		return -EBUSY;

> @@ -434,6 +449,11 @@ static int tiadc_read_raw(struct iio_dev *indio_dev,

>  		return -EINVAL;

>  

>  	mutex_lock(&adc_dev->fifo1_lock);

> +

> +	ret = tiadc_wait_idle(adc_dev);

> +	if (ret)

> +		goto err_unlock;

> +

>  	fifo1count = tiadc_readl(adc_dev, REG_FIFO1CNT);

>  	while (fifo1count--)

>  		tiadc_readl(adc_dev, REG_FIFO1);

> @@ -482,7 +502,7 @@ static int tiadc_read_raw(struct iio_dev *indio_dev,

>  

>  err_unlock:

>  	mutex_unlock(&adc_dev->fifo1_lock);

> -	return ret;

> +	return ret ? ret : IIO_VAL_INT;

>  }

>  

>  static const struct iio_info tiadc_info = {
diff mbox series

Patch

diff --git a/drivers/iio/adc/ti_am335x_adc.c b/drivers/iio/adc/ti_am335x_adc.c
index ebf6326af60d..d4619fd6fd54 100644
--- a/drivers/iio/adc/ti_am335x_adc.c
+++ b/drivers/iio/adc/ti_am335x_adc.c
@@ -17,6 +17,7 @@ 
 #include <linux/of_device.h>
 #include <linux/iio/machine.h>
 #include <linux/iio/driver.h>
+#include <linux/iopoll.h>
 
 #include <linux/mfd/ti_am335x_tscadc.h>
 #include <linux/iio/buffer.h>
@@ -94,6 +95,15 @@  static u32 get_adc_step_bit(struct tiadc_device *adc_dev, int chan)
 	return 1 << adc_dev->channel_step[chan];
 }
 
+static int tiadc_wait_idle(struct tiadc_device *adc_dev)
+{
+	u32 val;
+
+	return readl_poll_timeout(adc_dev->mfd_tscadc->tscadc_base + REG_ADCFSM,
+				  val, !(val & SEQ_STATUS), 10,
+				  IDLE_TIMEOUT_MS * 1000 * adc_dev->channels);
+}
+
 static void tiadc_step_config(struct iio_dev *indio_dev)
 {
 	struct tiadc_device *adc_dev = iio_priv(indio_dev);
@@ -268,6 +278,11 @@  static int tiadc_buffer_preenable(struct iio_dev *indio_dev)
 {
 	struct tiadc_device *adc_dev = iio_priv(indio_dev);
 	int i, fifo1count;
+	int ret;
+
+	ret = tiadc_wait_idle(adc_dev);
+	if (ret)
+		return ret;
 
 	tiadc_writel(adc_dev, REG_IRQCLR,
 		     IRQENB_FIFO1THRES | IRQENB_FIFO1OVRRUN |
@@ -419,12 +434,12 @@  static int tiadc_read_raw(struct iio_dev *indio_dev,
 			  long mask)
 {
 	struct tiadc_device *adc_dev = iio_priv(indio_dev);
-	int ret = IIO_VAL_INT;
 	int i, map_val;
 	unsigned int fifo1count, read, stepid;
 	bool found = false;
 	u32 step_en;
 	unsigned long timeout;
+	int ret;
 
 	if (iio_buffer_enabled(indio_dev))
 		return -EBUSY;
@@ -434,6 +449,11 @@  static int tiadc_read_raw(struct iio_dev *indio_dev,
 		return -EINVAL;
 
 	mutex_lock(&adc_dev->fifo1_lock);
+
+	ret = tiadc_wait_idle(adc_dev);
+	if (ret)
+		goto err_unlock;
+
 	fifo1count = tiadc_readl(adc_dev, REG_FIFO1CNT);
 	while (fifo1count--)
 		tiadc_readl(adc_dev, REG_FIFO1);
@@ -482,7 +502,7 @@  static int tiadc_read_raw(struct iio_dev *indio_dev,
 
 err_unlock:
 	mutex_unlock(&adc_dev->fifo1_lock);
-	return ret;
+	return ret ? ret : IIO_VAL_INT;
 }
 
 static const struct iio_info tiadc_info = {