* Guzman Lugo, Fernando <x0095840@xxxxxx> [100215 23:22]: > > Hi, > > >-----Original Message----- > >From: Hiroshi DOYU [mailto:Hiroshi.DOYU@xxxxxxxxx] > >Sent: Monday, February 15, 2010 7:49 AM > >To: Guzman Lugo, Fernando > >Cc: linux-omap@xxxxxxxxxxxxxxx > >Subject: Re: [PATCH 5/6] Mailbox: sleeping function called from invalid > >context fix > > > >Hi Fernando, > > > >From: "ext Guzman Lugo, Fernando" <x0095840@xxxxxx> > >Subject: [PATCH 5/6] Mailbox: sleeping function called from invalid context > >fix > >Date: Sat, 13 Feb 2010 02:42:16 +0100 > > > >> From e06b2716824f225747c4dc83ed2623d0160ae132 Mon Sep 17 00:00:00 2001 > >> From: Fernando Guzman Lugo <x0095840@xxxxxx> > >> Date: Fri, 29 Jan 2010 17:12:24 -0600 > >> Subject: [PATCH] Mailbox: sleeping function called from invalid context > >fix > >> > >> This patch fixes this bug: > >> BUG: sleeping function called from invalid context > >> Inside omap2_mbox_startup is called clk_get_sys that can sleep, > >> therefore omap2_mbox_startup can sleep but it is call in an atomic > >> context . So the spinlock is change for a semaphore. > > > >"mboxes_lock" is used to maintain the global list of mailbox > >instances, which belong to a single mailbox H/W module, but they are > >logical channels from S/W perspective. Both "->ops->startup()" and > >"->ops->shutdown()" are being executed against the above single H/W > >module, and a mailbox H/W module is totally __independent__ of the > >registration of logical mailboxes, which are (un)registered with > > Yes, they are independent of each other, and can be executed at the same time. I am agreed with your patch; that should be the right solution, so you can drop my patch. Hiroshi & Fernando, if you want me to merge this series, please post it one more time with right patches and ack's from Hiroshi. Please Cc also linux-arm-kernel so it gets reviewed there. The merge window is about to open, so we're running out of time.. Regards, Tony > Thanks and regards, > Fernando. > > >"omap_mbox_(un)register()". IOW, a mbox instance can be registered at > >anytime(before/after) H/W initialization. This H/W initialization is > >taken care of by "mbox_configured" variable. So I might think that the > >right solution is to introduce a new mutex lock __just for__ h/w > >configuration as 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 8e90633..19530de 100644 > >--- a/arch/arm/plat-omap/mailbox.c > >+++ b/arch/arm/plat-omap/mailbox.c > >@@ -32,6 +32,7 @@ static struct omap_mbox *mboxes; > > static DEFINE_RWLOCK(mboxes_lock); > > > > static int mbox_configured; > >+static DEFINE_MUTEX(mbox_configured_lock); > > > > /* Mailbox FIFO handle functions */ > > static inline mbox_msg_t mbox_fifo_read(struct omap_mbox *mbox) > >@@ -247,16 +248,16 @@ static int omap_mbox_startup(struct omap_mbox *mbox) > > struct omap_mbox_queue *mq; > > > > if (likely(mbox->ops->startup)) { > >- write_lock(&mboxes_lock); > >+ mutex_lock(&mbox_configured_lock); > > if (!mbox_configured) > > ret = mbox->ops->startup(mbox); > > > > if (unlikely(ret)) { > >- write_unlock(&mboxes_lock); > >+ mutex_unlock(&mbox_configured_lock); > > return ret; > > } > > mbox_configured++; > >- write_unlock(&mboxes_lock); > >+ mutex_unlock(&mbox_configured_lock); > > } > > > > ret = request_irq(mbox->irq, mbox_interrupt, IRQF_SHARED, > >@@ -302,12 +303,12 @@ static void omap_mbox_fini(struct omap_mbox *mbox) > > free_irq(mbox->irq, mbox); > > > > if (unlikely(mbox->ops->shutdown)) { > >- write_lock(&mboxes_lock); > >+ mutex_lock(&mbox_configured_lock); > > if (mbox_configured > 0) > > mbox_configured--; > > if (!mbox_configured) > > mbox->ops->shutdown(mbox); > >- write_unlock(&mboxes_lock); > >+ mutex_unlock(&mbox_configured_lock); > > } > > } > -- > 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 -- 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
