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
[email protected]
http://driverdev.linuxdriverproject.org/mailman/listinfo/devel