Hi Hiroshi, > -----Original Message----- > From: Hiroshi DOYU [mailto:Hiroshi.DOYU@xxxxxxxxx] > Sent: Wednesday, September 09, 2009 2:32 PM > To: C.A, Subramaniam > Cc: linux-omap@xxxxxxxxxxxxxxx; tony@xxxxxxxxxxx; > rmk@xxxxxxxxxxxxxxxx; Kanigeri, Hari; Gupta, Ramesh > Subject: Re: [PATCH 10/10] omap mailbox: OMAP4 Mailbox-driver > Patch to support tasklet implementation > > Hi Subbu, > > From: "ext C.A, Subramaniam" <subramaniam.ca@xxxxxx> > Subject: RE: [PATCH 10/10] omap mailbox: OMAP4 Mailbox-driver > Patch to support tasklet implementation > Date: Tue, 8 Sep 2009 19:43:48 +0200 > > [...] > > > > As I described in the comment on [PATCH 8/10], I think > that it would > > > be better to keep "mach-omap2/mailbox.c" simple and to add some > > > additional logic on "plat-omap/mailbox.c". > > > > > > Would it be possbile to move this tasklet implementation to > > > "plat-omap/mailbox.c"? > > > > The implementation of the tasklet itself is maintained in > > plat-omap/mailbox.c Since, I wanted to maintain a separate > tasklet for > > each mailbox instance used to send messages from MPU, I had to > > associate the the tasklet to the mailbox. Hence, the > changes were done > > in mach-omap2/mailbox.c > > > > Please give your comments on this approach. > > Wouldn't just converting work_queue to tasklet work like below? > > (I havne't tested this at all, though...) I will try that out and let you know. Also your approach seems better as you are replacing the work queue implementaion as well. > > diff --git a/arch/arm/plat-omap/include/mach/mailbox.h > b/arch/arm/plat-omap/include/mach/mailbox.h > index b7a6991..1f4e53e 100644 > --- a/arch/arm/plat-omap/include/mach/mailbox.h > +++ b/arch/arm/plat-omap/include/mach/mailbox.h > @@ -52,6 +52,8 @@ struct omap_mbox { > > struct omap_mbox_queue *txq, *rxq; > > + struct tasklet_struct tx_tasklet; > + > struct omap_mbox_ops *ops; > > mbox_msg_t seq_snd, seq_rcv; > diff --git a/arch/arm/plat-omap/mailbox.c > b/arch/arm/plat-omap/mailbox.c index 40424ed..37267ca 100644 > --- a/arch/arm/plat-omap/mailbox.c > +++ b/arch/arm/plat-omap/mailbox.c > @@ -184,22 +184,17 @@ int omap_mbox_msg_send(struct omap_mbox > *mbox, mbox_msg_t msg, void* arg) } > EXPORT_SYMBOL(omap_mbox_msg_send); > > -static void mbox_tx_work(struct work_struct *work) > +static void mbox_tx_tasklet(unsigned long data) > { > int ret; > struct request *rq; > - struct omap_mbox_queue *mq = container_of(work, > - struct omap_mbox_queue, work); > - struct omap_mbox *mbox = mq->queue->queuedata; > + struct omap_mbox *mbox = (struct omap_mbox *)data; > struct request_queue *q = mbox->txq->queue; > > while (1) { > struct omap_msg_tx_data *tx_data; > > - spin_lock(q->queue_lock); > rq = blk_fetch_request(q); > - spin_unlock(q->queue_lock); > - > if (!rq) > break; > > @@ -208,15 +203,10 @@ static void mbox_tx_work(struct > work_struct *work) > ret = __mbox_msg_send(mbox, tx_data->msg, tx_data->arg); > if (ret) { > enable_mbox_irq(mbox, IRQ_TX); > - spin_lock(q->queue_lock); > blk_requeue_request(q, rq); > - spin_unlock(q->queue_lock); > return; > } > - > - spin_lock(q->queue_lock); > __blk_end_request_all(rq, 0); > - spin_unlock(q->queue_lock); > } > } > > @@ -266,7 +256,7 @@ static void __mbox_tx_interrupt(struct > omap_mbox *mbox) { > disable_mbox_irq(mbox, IRQ_TX); > ack_mbox_irq(mbox, IRQ_TX); > - schedule_work(&mbox->txq->work); > + tasklet_schedule(&mbox->tx_tasklet); > } > > static void __mbox_rx_interrupt(struct omap_mbox *mbox) @@ > -434,7 +424,9 @@ static int omap_mbox_init(struct omap_mbox *mbox) > goto fail_request_irq; > } > > - mq = mbox_queue_alloc(mbox, mbox_txq_fn, mbox_tx_work); > + tasklet_init(&mbox->tx_tasklet, mbox_tx_tasklet, > (unsigned long)mbox); > + > + mq = mbox_queue_alloc(mbox, mbox_txq_fn, NULL); > if (!mq) { > ret = -ENOMEM; > goto fail_alloc_txq; > > -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html