On Fri, Sep 23, 2011 at 06:38:05PM +0200, Julian Andres Klode wrote:
> +struct nvec_msg *nvec_write_sync(struct nvec_chip *nvec,
> +             const unsigned char *data, short size)
> +{
> +     mutex_lock(&nvec->sync_write_mutex);
> +
> +     nvec->sync_write_pending = (data[1] << 8) + data[0];
> +     nvec_write_async(nvec, data, size);
> +
> +     dev_dbg(nvec->dev, "nvec_sync_write: 0x%04x\n",
> +                                     nvec->sync_write_pending);
> +     if (!(wait_for_completion_timeout(&nvec->sync_write,
> +                             msecs_to_jiffies(2000)))) {
> +             dev_warn(nvec->dev, "timeout waiting for sync write to 
> complete\n");
> +             mutex_unlock(&nvec->sync_write_mutex);
> +             return NULL;
> +     }
> +
> +     dev_dbg(nvec->dev, "nvec_sync_write: pong!\n");
> +
> +     mutex_unlock(&nvec->sync_write_mutex);
> +
> +     return nvec->last_sync_msg;

The ->sync_write_mutex protects the ->last_sync_msg member but you're
using it outside the lock here.  (Save a local of msg pointer before
you release the lock).

> +}
> +EXPORT_SYMBOL(nvec_write_sync);

> +
> +/* TX worker */
>  static void nvec_request_master(struct work_struct *work)
>  {
>       struct nvec_chip *nvec = container_of(work, struct nvec_chip, tx_work);
> +     unsigned long flags;
> +     struct nvec_msg *msg;
>  
> -     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);

wait_for_completion_interruptible_timeout() returns a negative error
code if it gets interrupted so that case should be handled as well.

regards,
dan carpenter


_______________________________________________
devel mailing list
[email protected]
http://driverdev.linuxdriverproject.org/mailman/listinfo/devel

Reply via email to