On Tue, Feb 04, 2014 at 06:59:43PM +0000, Hartley Sweeten wrote:
> Hello all,
>
> I'm in the process of cleaning up all the "timeout" code in the
> comedi drivers. The general form of the timeout code in all
> the drivers is:
>
> static int foo_eoc(struct comedi_device *dev, unsigned long timeout)
> {
> unsigned int status;
> while (timeout--) {
> status = inb(dev->iobase + STATUS_REG);
> if (status & EOC)
> return 0;
> udelay(1);
> }
> return -ETIMEDOUT;
> }
>
> My question is about the 'udelay()'. Is this the preferred way to
> delay between each check of the status register of would
> something like a 'cpu_relax()' be better?
udelay is fine, if you want really "tiny" sleep calls. What about
msleep() to really delay?
And the whole "this is the number of loops" is just crazy, how about
moving everything to use a real time value to timeout with instead?
Then use the proper jiffies comparison functions so it's obvious as to
what is being done here.
> Also, the 'timeout' values used in the comedi drivers vary a lot.
> I have seen value of 3, 5, 10, 100, 1000, 10000, 40000, and 100000.
> Usually the smaller values have the udelay(1) in the loop, the
> larger values don't. Assuming cpu_relax() would be a better way
> to handle the delays, would it be a good idea to standardize the
> timeout wait to a set number of loops?
Not number of loops, but real time values, that's easier to audit for
and understand.
thanks,
greg k-h
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
