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

[Greg KH <gregkh@linuxfoundation.org>]

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@yahoo.com>
> ---
> 
> 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.)

Other than that, looks much better.

thanks,

greg k-h