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. > + } > + spin_lock_irqsave(&nvec->tx_lock, flags); > + } > + spin_unlock_irqrestore(&nvec->tx_lock, flags); > } > [snip] > +static void nvec_rx_completed(struct nvec_chip *nvec) > +{ > + unsigned long flags; > > - if (!(status & I2C_SL_IRQ)) { > - dev_warn(nvec->dev, "nvec Spurious IRQ\n"); > - goto handled; > - } > - if (status & END_TRANS && !(status & RCVD)) { > - nvec->state = NVEC_WAIT; > - if (nvec->rx->size > 1) { > - list_add_tail(&nvec->rx->node, &nvec->rx_data); > - schedule_work(&nvec->rx_work); > - } else { > - kfree(nvec->rx->data); > - kfree(nvec->rx); > - } > - return IRQ_HANDLED; > - } else if (status & RNW) { > - if (status & RCVD) > - udelay(3); > + if (nvec->rx->pos != nvec_msg_size(nvec->rx)) > + dev_err(nvec->dev, "RX incomplete: Expected %u bytes, got %u\n", > + (uint) nvec_msg_size(nvec->rx), > + (uint) nvec->rx->pos); > > - if (status & RCVD) > - nvec->state = NVEC_WRITE; > + spin_lock_irqsave(&nvec->rx_lock, flags); > > - if (list_empty(&nvec->tx_data)) { > - dev_err(nvec->dev, "nvec empty tx - sending no-op\n"); > - to_send = 0x8a; > - nvec_write_async(nvec, "\x07\x02", 2); > - } else { > - msg = > - list_first_entry(&nvec->tx_data, struct nvec_msg, > - node); > - if (msg->pos < msg->size) { > - to_send = msg->data[msg->pos]; > - msg->pos++; > - } else { > - dev_err(nvec->dev, "nvec crap! %d\n", > - msg->size); > - to_send = 0x01; > - } > + /* add the received data to the work list > + and move the ring buffer pointer to the next entry */ > + list_add_tail(&nvec->rx->node, &nvec->rx_data); > > - if (msg->pos >= msg->size) { > - list_del_init(&msg->node); > - kfree(msg->data); > - kfree(msg); > - schedule_work(&nvec->tx_work); > - nvec->state = NVEC_WAIT; > - } > - } > - writel(to_send, base + I2C_SL_RCVD); > + spin_unlock_irqrestore(&nvec->rx_lock, flags); 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. > > - gpio_set_value(nvec->gpio, 1); > + nvec->state = 0; > > - dev_dbg(nvec->dev, "nvec sent %x\n", to_send); > + if (!nvec_msg_is_event(nvec->rx)) > + complete(&nvec->ec_transfer); > > - goto handled; > + queue_work(nvec->wq, &nvec->rx_work); > +} > + regards, dan carpenter _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/devel