Re: [PATCH v8] Staging: comedi: convert while loop to timeout in ni_mio_common.c

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2014-01-16 18:27, Chase Southwood wrote:
This patch for ni_mio_common.c changes out a while loop for a timeout,
which is preferred.

Signed-off-by: Chase Southwood <chase.southwood@xxxxxxxxx>
---

Okay, back to v2, basically.  I fixed the checkpatch warning from v2,
and added the error checking that was from v3, but otherwise it is the
same.  Of note, I have used a udelay of 1 here (Greg had suggested to
use 10, but Ian prefers 1 so I have gone with that, and I assume that
cpu_relax is no longer an option.).

2: Changed from simple clean-up to swapping a timeout in for a while loop.

3: Removed extra counter variable, and added error checking.

4: No longer using counter variable, using jiffies instead.

5: udelay for 10u, instead of 1u.

6: Scrap udelay entirely, in favor of cpu_relax. Include asm/processor.h
in order to use cpu_relax.

7: Fix typo (msec vs msecs).

8: Revert back to v2, with some small changes (see above).

  drivers/staging/comedi/drivers/ni_mio_common.c | 12 +++++++++++-
  1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/comedi/drivers/ni_mio_common.c b/drivers/staging/comedi/drivers/ni_mio_common.c
index 457b884..10c27cb 100644
--- a/drivers/staging/comedi/drivers/ni_mio_common.c
+++ b/drivers/staging/comedi/drivers/ni_mio_common.c
@@ -687,12 +687,22 @@ static void ni_clear_ai_fifo(struct comedi_device *dev)
  {
  	const struct ni_board_struct *board = comedi_board(dev);
  	struct ni_private *devpriv = dev->private;
+	static const int timeout = 10000;
+	int i;

  	if (board->reg_type == ni_reg_6143) {
  		/*  Flush the 6143 data FIFO */
  		ni_writel(0x10, AIFIFO_Control_6143);	/*  Flush fifo */
  		ni_writel(0x00, AIFIFO_Control_6143);	/*  Flush fifo */
-		while (ni_readl(AIFIFO_Status_6143) & 0x10) ;	/*  Wait for complete */
+		/*  Wait for complete */
+		for (i = 0; i < timeout; i++) {
+			if (!(ni_readl(AIFIFO_Status_6143) & 0x10))
+				break;
+			udelay(1);
+		}
+		if (i == timeout) {
+			comedi_error(dev, "FIFO flush timeout.");
+		}
  	} else {
  		devpriv->stc_writew(dev, 1, ADC_FIFO_Clear);
  		if (board->reg_type == ni_reg_625x) {


Personally, I'm happy with it. The upper bound on the iterations is quite high for something that could be called on an interrupt, but it's better than an infinite loop, and it only reach the upper bound if the hardware is broken or there's some other bug.

Reviewed-by: Ian Abbott <abbotti@xxxxxxxxx>

--
-=( Ian Abbott @ MEV Ltd.    E-mail: <abbotti@xxxxxxxxx>        )=-
-=( Tel: +44 (0)161 477 1898   FAX: +44 (0)161 718 3587         )=-
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel




[Index of Archives]     [Linux Driver Backports]     [DMA Engine]     [Linux GPIO]     [Linux SPI]     [Video for Linux]     [Linux USB Devel]     [Linux Coverity]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux