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
