lkml.org 
[lkml]   [2014]   [Jan]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v8] Staging: comedi: convert while loop to timeout in ni_mio_common.c
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@yahoo.com>
> ---
>
> 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@mev.co.uk>

--
-=( Ian Abbott @ MEV Ltd. E-mail: <abbotti@mev.co.uk> )=-
-=( Tel: +44 (0)161 477 1898 FAX: +44 (0)161 718 3587 )=-


\
 
 \ /
  Last update: 2014-01-17 15:41    [W:0.100 / U:0.068 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site