Messages in this thread | | | Date | Wed, 21 May 2014 18:27:01 +0100 | From | Mark Brown <> | Subject | Re: [PATCHv5 2/4] mailbox: Introduce framework for mailbox |
| |
On Thu, May 15, 2014 at 11:41:00AM +0530, Jassi Brar wrote: > Introduce common framework for client/protocol drivers and > controller drivers of Inter-Processor-Communication (IPC).
This looks pretty nice, though I do have a few *very* small nits beyond those Arnd had.
> + if (chan->cl->tx_block && chan->active_req) { > + int ret; > + init_completion(&chan->tx_complete);
reinit_completion().
> + if (!cl->tx_tout) /* wait for ever */ > + cl->tx_tout = msecs_to_jiffies(3600000); > + else > + cl->tx_tout = msecs_to_jiffies(cl->tx_tout);
Is the default wait for ever the best timeout - I'm not sure it's best from a defensiveness point of view. It should be fine either way, it's just a matter of taste.
> + ret = chan->mbox->ops->startup(chan); > + if (ret) { > + pr_err("Unable to startup the chan\n");
Perhaps print the error codes? Might be helpful to users.
> + /* The queued TX requests are simply aborted, no callbacks are made */ > + spin_lock_irqsave(&chan->lock, flags); > + chan->cl = NULL; > + chan->active_req = NULL; > + if (chan->txdone_method == (TXDONE_BY_POLL | TXDONE_BY_ACK)) > + chan->txdone_method = TXDONE_BY_POLL; > + > + module_put(chan->mbox->dev->driver->owner); > + spin_unlock_irqrestore(&chan->lock, flags);
Is the module_put() safe in atomic context? [unhandled content-type:application/pgp-signature] | |