[PATCH] Staging: comedi: add timeouts to while loops in s626.c
Ian Abbott
abbotti at mev.co.uk
Fri Feb 28 17:18:03 UTC 2014
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 at yahoo.com>
> ---
> 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 at mev.co.uk> )=-
-=( Tel: +44 (0)161 477 1898 FAX: +44 (0)161 718 3587 )=-
More information about the devel
mailing list