| Message ID | 1370656671-41285-1-git-send-email-s-anna@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Jun 7, 2013 at 6:57 PM, Suman Anna <s-anna@ti.com> wrote: > The OMAP mailbox startup code is enabling the interrupt even before > any of the associated mailbox queues are allocated. Any pending > received mailbox message could cause a kernel panic as soon as > the interrupt is enabled due to the dereferencing of non-existing > mailbox queues within the ISR. > > Signed-off-by: Fernando Guzman Lugo <lugo.fernando@gmail.com> > Signed-off-by: Suman Anna <s-anna@ti.com> > --- > arch/arm/plat-omap/mailbox.c | 18 +++++++++--------- > 1 file changed, 9 insertions(+), 9 deletions(-) > > diff --git a/arch/arm/plat-omap/mailbox.c b/arch/arm/plat-omap/mailbox.c > index 5fb4027..e1bd333 100644 > --- a/arch/arm/plat-omap/mailbox.c > +++ b/arch/arm/plat-omap/mailbox.c > @@ -261,13 +261,6 @@ static int omap_mbox_startup(struct omap_mbox *mbox) > } > > if (!mbox->use_count++) { > - ret = request_irq(mbox->irq, mbox_interrupt, IRQF_SHARED, > - mbox->name, mbox); > - if (unlikely(ret)) { > - pr_err("failed to register mailbox interrupt:%d\n", > - ret); > - goto fail_request_irq; > - } > mq = mbox_queue_alloc(mbox, NULL, mbox_tx_tasklet); > if (!mq) { > ret = -ENOMEM; > @@ -282,17 +275,24 @@ static int omap_mbox_startup(struct omap_mbox *mbox) > } > mbox->rxq = mq; > mq->mbox = mbox; > + ret = request_irq(mbox->irq, mbox_interrupt, IRQF_SHARED, > + mbox->name, mbox); > + if (unlikely(ret)) { > + pr_err("failed to register mailbox interrupt:%d\n", > + ret); > + goto fail_request_irq; > + } > > omap_mbox_enable_irq(mbox, IRQ_RX); I can't help but to notice the IRQ unmasking function here. Is there a reason it isn't working? > } > mutex_unlock(&mbox_configured_lock); > return 0; > > +fail_request_irq: > + mbox_queue_free(mbox->rxq); > fail_alloc_rxq: > mbox_queue_free(mbox->txq); > fail_alloc_txq: > - free_irq(mbox->irq, mbox); > -fail_request_irq: > if (mbox->ops->shutdown) > mbox->ops->shutdown(mbox); > mbox->use_count--; > -- > 1.8.2 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-omap" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html
Russ, On 06/08/2013 01:53 PM, Russ Dill wrote: > On Fri, Jun 7, 2013 at 6:57 PM, Suman Anna <s-anna@ti.com> wrote: >> The OMAP mailbox startup code is enabling the interrupt even before >> any of the associated mailbox queues are allocated. Any pending >> received mailbox message could cause a kernel panic as soon as >> the interrupt is enabled due to the dereferencing of non-existing >> mailbox queues within the ISR. >> >> Signed-off-by: Fernando Guzman Lugo <lugo.fernando@gmail.com> >> Signed-off-by: Suman Anna <s-anna@ti.com> >> --- >> arch/arm/plat-omap/mailbox.c | 18 +++++++++--------- >> 1 file changed, 9 insertions(+), 9 deletions(-) >> >> diff --git a/arch/arm/plat-omap/mailbox.c b/arch/arm/plat-omap/mailbox.c >> index 5fb4027..e1bd333 100644 >> --- a/arch/arm/plat-omap/mailbox.c >> +++ b/arch/arm/plat-omap/mailbox.c >> @@ -261,13 +261,6 @@ static int omap_mbox_startup(struct omap_mbox *mbox) >> } >> >> if (!mbox->use_count++) { >> - ret = request_irq(mbox->irq, mbox_interrupt, IRQF_SHARED, >> - mbox->name, mbox); >> - if (unlikely(ret)) { >> - pr_err("failed to register mailbox interrupt:%d\n", >> - ret); >> - goto fail_request_irq; >> - } >> mq = mbox_queue_alloc(mbox, NULL, mbox_tx_tasklet); >> if (!mq) { >> ret = -ENOMEM; >> @@ -282,17 +275,24 @@ static int omap_mbox_startup(struct omap_mbox *mbox) >> } >> mbox->rxq = mq; >> mq->mbox = mbox; >> + ret = request_irq(mbox->irq, mbox_interrupt, IRQF_SHARED, >> + mbox->name, mbox); >> + if (unlikely(ret)) { >> + pr_err("failed to register mailbox interrupt:%d\n", >> + ret); >> + goto fail_request_irq; >> + } >> >> omap_mbox_enable_irq(mbox, IRQ_RX); > > I can't help but to notice the IRQ unmasking function here. Is there a > reason it isn't working? This patch was based on an internal patch needed before the equivalent of 1d8a0e9 "ARM: OMAP: enable mailbox irq per instance" is merged. I will revise the patch description - this is more of a minor cleanup now rather than a fix, would have been a fix without the above patch. Thanks for pointing it out. regards Suman
diff --git a/arch/arm/plat-omap/mailbox.c b/arch/arm/plat-omap/mailbox.c index 5fb4027..e1bd333 100644 --- a/arch/arm/plat-omap/mailbox.c +++ b/arch/arm/plat-omap/mailbox.c @@ -261,13 +261,6 @@ static int omap_mbox_startup(struct omap_mbox *mbox) } if (!mbox->use_count++) { - ret = request_irq(mbox->irq, mbox_interrupt, IRQF_SHARED, - mbox->name, mbox); - if (unlikely(ret)) { - pr_err("failed to register mailbox interrupt:%d\n", - ret); - goto fail_request_irq; - } mq = mbox_queue_alloc(mbox, NULL, mbox_tx_tasklet); if (!mq) { ret = -ENOMEM; @@ -282,17 +275,24 @@ static int omap_mbox_startup(struct omap_mbox *mbox) } mbox->rxq = mq; mq->mbox = mbox; + ret = request_irq(mbox->irq, mbox_interrupt, IRQF_SHARED, + mbox->name, mbox); + if (unlikely(ret)) { + pr_err("failed to register mailbox interrupt:%d\n", + ret); + goto fail_request_irq; + } omap_mbox_enable_irq(mbox, IRQ_RX); } mutex_unlock(&mbox_configured_lock); return 0; +fail_request_irq: + mbox_queue_free(mbox->rxq); fail_alloc_rxq: mbox_queue_free(mbox->txq); fail_alloc_txq: - free_irq(mbox->irq, mbox); -fail_request_irq: if (mbox->ops->shutdown) mbox->ops->shutdown(mbox); mbox->use_count--;