Re: [PATCH] Staging: comedi: add timeouts to while loops in s626.c

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

 



On 2014-02-28 07:35, Chase Southwood wrote:
Smatch located a handful of while loops testing readl calls in s626.c.
Since these while loops depend on readl succeeding, it's safer to make
sure they time out eventually.

Signed-off-by: Chase Southwood <chase.southwood@xxxxxxxxx>
---
Ian and/or Hartley, I'd love your comments on this.  It seems to me that
we want these kinds of while loops properly timed out, but I want to make
sure I'm doing everything properly.  First off, s626_debi_transfer() says
directly that it is called from within critical sections, so I assume
that means that the new comedi_timeout() function is no good here, and
s626_send_dac() looked equally suspicious, so I opted for iterative
timeouts.  Is this correct?  Also, for these timeouts, I used a very
conservative 10000 iterations, would it be better to decrease that?

Well 10000 iterations is an improvement on infinity! If the hardware is working, you'd expect it to go round a lot fewer iterations than that, but if the hardware is broken all bets are off, especially if it is generating interrupts.

Also, do my error strings appear acceptable?

Mostly. There's a type in one of the strings that says "TLS" instead of "TSL".

And finally, are timeouts here even necessary or helpful, or are there
any better ways to do it?

In the case of s626_send_dac(), it doesn't seem to be used in any critical sections, so it could make use of Hartley's comedi_timeout().

Some of the timeout errors could be propagated, especially for s626_send_dac() which is only reachable from very few paths.

There are other infinite loops involving calls to the s626_mc_test() function, but those could be dealt with by other patches.

--
-=( 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