Hi,
>-----Original Message-----
>From: Hiroshi DOYU [mailto:[email protected]]
>Sent: Monday, February 15, 2010 7:49 AM
>To: Guzman Lugo, Fernando
>Cc: [email protected]
>Subject: Re: [PATCH 5/6] Mailbox: sleeping function called from invalid
>context fix
>
>Hi Fernando,
>
>From: "ext Guzman Lugo, Fernando" <[email protected]>
>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 <[email protected]>
>> 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.
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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
