* [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: Tony Lindgren
Cc: Ohad Ben-Cohen, Omar Ramirez Luna, Jassi Brar, Loic Pallardy,
linux-omap, linux-arm-kernel, Suman Anna, Fernando Guzman Lugo
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* Re: [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: Suman Anna
Cc: Tony Lindgren, Ohad Ben-Cohen, Omar Ramirez Luna, Jassi Brar,
Loic Pallardy, linux-omap, linux-arm-kernel, Fernando Guzman Lugo
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
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [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: Russ Dill
Cc: Tony Lindgren, Ohad Ben-Cohen, Omar Ramirez Luna, Jassi Brar,
Loic Pallardy, linux-omap, linux-arm-kernel, Fernando Guzman Lugo
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:20 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).