Rune Torgersen wrote:
>Hi
>I have a driver that roughly does something like:
>
>int driver_read(int cs, int addr, void *buf, int len)
>{
> hw_done = 0;
> /* init_hw */
>
> if (!hw_done)
> {
> ret = wait_event_interruptible_timeout(inq, hw_done, TIMEOUT);
> if (ret == 0)
> {
> if (hw_done)
> goto hw_finished;
>
> return -EIO;
> }
> }
>hw_finished:
> return len;
>}
>
>static irqreturn_t myinterrupt(int irq, void * dev_id, struct pt_regs *
>regs)
>{
> hw_done = 1;
> schedule_work(&tqueue);
>
> return IRQ_HANDLED;
>}
>
>static void do_softint(void *private_)
>{
> wake_up_interruptible(&inq);
>
>}
>
>I have a problem however with this, because in about 10% of my cases,
>the interrupt triggers very fast, and ends up being served between the
>check for hw_done and the wait_event call. This cause the wait to
>timeout instead of getting waked up.
>
>Is there a better way of doing this?
>I do not want to do a busy wait, because the hardware can take up to
>several 100's of ms to return, but most often returns within 20us.
>
>
You seem to create an unnecessary race condition by checking the hw_done
variable in the read function. You can avoid this by changing it to
int driver_read(int cs, int addr, void *buf, int len)
{
hw_done = 0;
/* init_hw */
if (wait_event_interruptible_timeout(inq, hw_done, TIMEOUT))
return -ERESTARTSYS;
hw_done = 0;
/* copy the data to user space here */
return len;
}
The wait_event_interruptible_timeout() function checks the variable for
you. You still must make sure the read function does the right thing if
it is called simultaneously by more than one application.
