* [PATCH 3/7] omap: mailbox: call request_irq after mbox queues are allocated
@ 2013-06-08 1:57 Suman Anna
2013-06-08 18:53 ` Russ Dill
0 siblings, 1 reply; 3+ messages in thread
From: Suman Anna @ 2013-06-08 1:57 UTC (permalink / raw)
To: linux-arm-kernel
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);
}
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
^ permalink raw reply related [flat|nested] 3+ messages in thread* [PATCH 3/7] omap: mailbox: call request_irq after mbox queues are allocated
2013-06-08 1:57 [PATCH 3/7] omap: mailbox: call request_irq after mbox queues are allocated Suman Anna
@ 2013-06-08 18:53 ` Russ Dill
2013-06-10 19:14 ` Suman Anna
0 siblings, 1 reply; 3+ messages in thread
From: Russ Dill @ 2013-06-08 18:53 UTC (permalink / raw)
To: linux-arm-kernel
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 at vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 3+ messages in thread* [PATCH 3/7] omap: mailbox: call request_irq after mbox queues are allocated
2013-06-08 18:53 ` Russ Dill
@ 2013-06-10 19:14 ` Suman Anna
0 siblings, 0 replies; 3+ messages in thread
From: Suman Anna @ 2013-06-10 19:14 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2013-06-10 19:14 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-06-08 1:57 [PATCH 3/7] omap: mailbox: call request_irq after mbox queues are allocated Suman Anna
2013-06-08 18:53 ` Russ Dill
2013-06-10 19:14 ` Suman Anna
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).