Re: [PATCH v4] Staging: comedi: convert while loop to timeout inni_mio_common.c

From: Ian Abbott
Date: Wed Jan 15 2014 - 05:30:09 EST


On 2014-01-15 03:58, Greg KH wrote:
On Tue, Jan 14, 2014 at 06:23:05PM -0600, 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>
---

OK, here's another go at it. Hopefully everything looks more correct
this time. Greg, I've followed the pattern you gave me, and I really
appreciate all of the tips! As always, just let me know if there are
still things that need adjusting (especially length of timeout, udelay,
etc.).

Thanks,
Chase Southwood

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.

drivers/staging/comedi/drivers/ni_mio_common.c | 11 ++++++++++-
1 file changed, 10 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..882b249 100644
--- a/drivers/staging/comedi/drivers/ni_mio_common.c
+++ b/drivers/staging/comedi/drivers/ni_mio_common.c
@@ -687,12 +687,21 @@ 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;
+ unsigned long timeout;

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 */
+ timeout = jiffies + msec_to_jiffies(100);
+ while (ni_readl(AIFIFO_Status_6143) & 0x10) {
+ if (time_after(jiffies, timeout)) {
+ comedi_error(dev, "FIFO flush timeout.");
+ break;
+ }
+ udelay(1);

Sleep for at least 10, as I think that's the smallest time delay you can
sleep for anyway (meaning it will be that long no matter what number you
put there less than 10, depending on the hardware used of course.)

udelay() doesn't sleep (though it may get pre-empted) and I think you can use any value you like within reason (up to a few 1000 microseconds).


Other than that, looks much better.

thanks,

greg k-h



--
-=( Ian Abbott @ MEV Ltd. E-mail: <abbotti@xxxxxxxxx> )=-
-=( Tel: +44 (0)161 477 1898 FAX: +44 (0)161 718 3587 )=-
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/