On Sat, Sep 24, 2011 at 01:30:44PM +0300, Dan Carpenter wrote: > On Fri, Sep 23, 2011 at 06:38:05PM +0200, Julian Andres Klode wrote: > > - if (!list_empty(&nvec->tx_data)) > > - gpio_set_value(nvec->gpio, 0); > > + spin_lock_irqsave(&nvec->tx_lock, flags); > > + while (!list_empty(&nvec->tx_data)) { > > + msg = list_first_entry(&nvec->tx_data, struct nvec_msg, node); > > + spin_unlock_irqrestore(&nvec->tx_lock, flags); > > + nvec_gpio_set_value(nvec, 0); > > + if (!(wait_for_completion_interruptible_timeout( > > + &nvec->ec_transfer, msecs_to_jiffies(5000)))) { > > + dev_warn(nvec->dev, "timeout waiting for ec transfer\n"); > > + nvec_gpio_set_value(nvec, 1); > > + msg->pos = 0; > > + } else { > > + list_del_init(&msg->node); > > + nvec_msg_free(nvec, msg); > > This should be under the ->tx_lock lock. OK. > > +static void nvec_rx_completed(struct nvec_chip *nvec) [...] > The irqsave and irqrestore aren't needed because this function is > only called from the irq handler. Otherwise the nvec->state on the > next line would have to be protected by the lock. OK. This probably also means that the local_irq_save() and local_irq_restore() in nvec_interrupt() are unneeded as well, right? -- Julian Andres Klode - Debian Developer, Ubuntu Member See http://wiki.debian.org/JulianAndresKlode and http://jak-linux.org/. _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/devel