________________________________________ >From: Hiroshi DOYU [Hiroshi.DOYU@xxxxxxxxx] >Sent: Wednesday, June 09, 2010 12:07 AM >To: ohad@xxxxxxxxxx >Cc: Guzman Lugo, Fernando; Chitriki Rudramuni, Deepak; Ramirez Luna, Omar; Kanigeri, Hari; linux-omap@xxxxxxxxxxxxxxx >Subject: Re: [PATCH v3 4/4] omap: mailbox: convert block api to kfifo > >From: ext Ohad Ben-Cohen <ohad@xxxxxxxxxx> >Subject: Re: [PATCH v3 4/4] omap: mailbox: convert block api to kfifo >Date: Tue, 8 Jun 2010 04:54:55 +0200 > >> On Mon, Jun 7, 2010 at 6:14 PM, Guzman Lugo, Fernando >> <fernando.lugo@xxxxxx> wrote: >>> I think the best thing to do here is remove the spinlock >> >> I'm afraid we can't do that: we need it to support OMAP4 syslink IPC >> use cases which have multiple simultaneous sending contexts. >> >> >>>, if not, you are preventing that omap_mbox_msg_send be executed from a tasklet or isr context. >> >> >> Right. But this is pretty standard - you can't call every kernel API >> from every possible context - the allowed calling context is part of >> the API. If later we decide we need to add additional calling >> contexts, it's ok - it's just like changing the API. >> >> >>> Why not remove the workqueue at all and let the mailbox module user be the judge for using tasklet, workqueue or do the job in the same isr (if it is small task and need to be fast)? >> >> >> hmm I'm not following here - will you please elaborate on this a bit more ? > >The abvoe could be something like below: > > Modified arch/arm/plat-omap/mailbox.c >diff --git a/arch/arm/plat-omap/mailbox.c b/arch/arm/plat-omap/mailbox.c >index 87e0cde..1b79b32 100644 >--- a/arch/arm/plat-omap/mailbox.c >+++ b/arch/arm/plat-omap/mailbox.c >@@ -188,7 +188,7 @@ static void __mbox_rx_interrupt(struct omap_mbox *mbox) > /* no more messages in the fifo. clear IRQ source. */ > ack_mbox_irq(mbox, IRQ_RX); > nomem: >- queue_work(mboxd, &mbox->rxq->work); >+ mbox->callback(mbox); > } > > static irqreturn_t mbox_interrupt(int irq, void *p) > >The user/client of mbox can register its own callback function in >advance and do its own job(tasklet, workqueue, just func whatever) >later by itself. > >It makes more sense to me than the current workqueue as default. Yes, this is exactly what I was talking about. Regards, Fernando. -- 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
