[PATCH] ARM: OMAP2+: fix gpmc request_irq

[PATCH] ARM: OMAP2+: fix gpmc request_irq

[Ming Lei]

The IRQ52 on OMAP2+ is not a shared interrupt line. If IRQF_SHARED
is passed to request_irq and dev_id is set as NULL, request_irq will
return -EINVAL.

This patch just removes the flag of IRQF_SHARED to make the irq
registration successful.

Cc: Kevin Hilman <khilman@ti.com>
Cc: Tony Lindgren <tony@atomide.com>
Signed-off-by: Ming Lei <ming.lei@canonical.com>
---
 arch/arm/mach-omap2/gpmc.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/mach-omap2/gpmc.c b/arch/arm/mach-omap2/gpmc.c
index 2286410..e45d31b 100644
--- a/arch/arm/mach-omap2/gpmc.c
+++ b/arch/arm/mach-omap2/gpmc.c
@@ -769,7 +769,7 @@ static int __init gpmc_init(void)
 		irq++;
 	}
 
-	ret = request_irq(gpmc_irq, gpmc_handle_irq, IRQF_SHARED, "gpmc", NULL);
+	ret = request_irq(gpmc_irq, gpmc_handle_irq, 0, "gpmc", NULL);
 	if (ret)
 		pr_err("gpmc: irq-%d could not claim: err %d\n",
 						gpmc_irq, ret);
-- 
1.7.9.5

[PATCH] ARM: OMAP2+: fix gpmc request_irq

[Shilimkar, Santosh]

+ Afzal who has been doing quite a bit of GMPC work recently.

On Fri, May 18, 2012 at 10:13 AM, Ming Lei <ming.lei@canonical.com> wrote:
> The IRQ52 on OMAP2+ is not a shared interrupt line. If IRQF_SHARED
> is passed to request_irq and dev_id is set as NULL, request_irq will
> return -EINVAL.
>
GPMC IRQ line can shared between the multiple devices you
connect on it.

> This patch just removes the flag of IRQF_SHARED to make the irq
> registration successful.
>
> Cc: Kevin Hilman <khilman@ti.com>
> Cc: Tony Lindgren <tony@atomide.com>
> Signed-off-by: Ming Lei <ming.lei@canonical.com>
> ---
> ?arch/arm/mach-omap2/gpmc.c | ? ?2 +-
> ?1 file changed, 1 insertion(+), 1 deletion(-)
>
Are you sure it fails ?
I just tried booting OMAP4 with 3.4-rc4 and don't see the irq
failure error message. What I am missing ?

Regards
Santosh

[PATCH] ARM: OMAP2+: fix gpmc request_irq

[Mohammed, Afzal]

Hi Santosh,

On Fri, May 18, 2012 at 12:33:16, Shilimkar, Santosh wrote:
> + Afzal who has been doing quite a bit of GMPC work recently.
> 
> On Fri, May 18, 2012 at 10:13 AM, Ming Lei <ming.lei@canonical.com> wrote:
> > The IRQ52 on OMAP2+ is not a shared interrupt line. If IRQF_SHARED
> > is passed to request_irq and dev_id is set as NULL, request_irq will
> > return -EINVAL.
> >
> GPMC IRQ line can shared between the multiple devices you
> connect on it.
:
> Are you sure it fails ?
> I just tried booting OMAP4 with 3.4-rc4 and don't see the irq
> failure error message. What I am missing ?

Issue happens on l-o master, this was mentioned in [1], now I feel
right solution is to keep dev-id.

Regards
Afzal

[1] http://www.mail-archive.com/linux-omap at vger.kernel.org/msg68749.html

[PATCH] ARM: OMAP2+: fix gpmc request_irq

[Shilimkar, Santosh]

On Fri, May 18, 2012 at 12:46 PM, Mohammed, Afzal <afzal@ti.com> wrote:
> Hi Santosh,
>
> On Fri, May 18, 2012 at 12:33:16, Shilimkar, Santosh wrote:
>> + Afzal who has been doing quite a bit of GMPC work recently.
>>
>> On Fri, May 18, 2012 at 10:13 AM, Ming Lei <ming.lei@canonical.com> wrote:
>> > The IRQ52 on OMAP2+ is not a shared interrupt line. If IRQF_SHARED
>> > is passed to request_irq and dev_id is set as NULL, request_irq will
>> > return -EINVAL.
>> >
>> GPMC IRQ line can shared between the multiple devices you
>> connect on it.
> :
>> Are you sure it fails ?
>> I just tried booting OMAP4 with 3.4-rc4 and don't see the irq
>> failure error message. What I am missing ?
>
> Issue happens on l-o master, this was mentioned in [1], now I feel
> right solution is to keep dev-id.
>
Thanks for the reference. removing SHARED flag is not solution and
yes, you might have to keep the dev-id if that was the issue.

Regards
Santosh

[PATCH] ARM: OMAP2+: fix gpmc request_irq

[Ming Lei]

Hi,

On Fri, May 18, 2012 at 3:19 PM, Shilimkar, Santosh
<santosh.shilimkar@ti.com> wrote:
>> :
>>> Are you sure it fails ?

[    1.778930] GPMC revision 6.0
[    1.782226] gpmc: irq-52 could not claim: err -22


> Thanks for the reference. removing SHARED flag is not solution and
> yes, you might have to keep the dev-id if that was the issue.

Is the gpmc a shared interrupt line? SHARED is not needed at all for
non shared interrupt line in hardware.

Thanks,
--
Ming Lei

[PATCH] ARM: OMAP2+: fix gpmc request_irq

[Shilimkar, Santosh]

On Fri, May 18, 2012 at 1:10 PM, Ming Lei <ming.lei@canonical.com> wrote:
> Hi,
>
> On Fri, May 18, 2012 at 3:19 PM, Shilimkar, Santosh
> <santosh.shilimkar@ti.com> wrote:
>>> :
>>>> Are you sure it fails ?
>
> [ ? ?1.778930] GPMC revision 6.0
> [ ? ?1.782226] gpmc: irq-52 could not claim: err -22
>
Afzal pointed me to the failing tree.

>
>> Thanks for the reference. removing SHARED flag is not solution and
>> yes, you might have to keep the dev-id if that was the issue.
>
> Is the gpmc a shared interrupt line? SHARED is not needed at all for
> non shared interrupt line in hardware.
>
The line is shared.

Regards
Santosh

[PATCH] ARM: OMAP2+: fix gpmc request_irq

[Ming Lei]

On Fri, May 18, 2012 at 3:43 PM, Shilimkar, Santosh
<santosh.shilimkar@ti.com> wrote:

>> Is the gpmc a shared interrupt line? SHARED is not needed at all for
>> non shared interrupt line in hardware.
>>
> The line is shared.

If so, dev_id should be added. But sorry, could you let me know
what other interrupt sources are shared with the line?

>From 17.3.2 "Interrupt Requests to Cortex-A9 MPU INTC" of OMAP4
TRM, GPMC_IRQ is the only source of MA_IRQ_20 for Cortex-A9 MPU
INTC.  Even though GPMC_IRQ is connected with D_IRQ_59(DSP INTC)
and MA_IRQ_20(MPU INTC), this still means it is not a shared line for
MPU INTC.

Also looks the above is similar for OMAP3.

Correct me if the above is wrong.

Thanks,
--
Ming Lei

[PATCH] ARM: OMAP2+: fix gpmc request_irq

[Shilimkar, Santosh]

On Fri, May 18, 2012 at 2:34 PM, Ming Lei <ming.lei@canonical.com> wrote:
> On Fri, May 18, 2012 at 3:43 PM, Shilimkar, Santosh
> <santosh.shilimkar@ti.com> wrote:
>
>>> Is the gpmc a shared interrupt line? SHARED is not needed at all for
>>> non shared interrupt line in hardware.
>>>
>> The line is shared.
>
> If so, dev_id should be added. But sorry, could you let me know
> what other interrupt sources are shared with the line?
>
> From 17.3.2 "Interrupt Requests to Cortex-A9 MPU INTC" of OMAP4
> TRM, GPMC_IRQ is the only source of MA_IRQ_20 for Cortex-A9 MPU
> INTC. ?Even though GPMC_IRQ is connected with D_IRQ_59(DSP INTC)
> and MA_IRQ_20(MPU INTC), this still means it is not a shared line for
> MPU INTC.
>
Generally IRQF_SHARED means, the interrupt line is only one but
multiple devices can generate the interrupts. If you connect
different devices on GMPC using chip selects like Ethernet controller,
Nand, NOR contoller, all of they can generate events which can be
reported by the GPMC. That's the reason I shared the line
is shared.

On the side note, looks like GMPC too needs to be converted
to sparse IRQ since it seems to be creating a dummy irqchip
and dispatching secondary handlers based on chip selects.

Regards
santosh

[PATCH] ARM: OMAP2+: fix gpmc request_irq

[Mohammed, Afzal]

Hi Santosh,

On Fri, May 18, 2012 at 15:43:31, Shilimkar, Santosh wrote:

> On the side note, looks like GMPC too needs to be converted
> to sparse IRQ since it seems to be creating a dummy irqchip
> and dispatching secondary handlers based on chip selects.

GPMC dummy chip is being modified to so that irq descriptors are created
dynamically [1], with a final goal of reaching something similar to [2],
please let me know if any comments on those.

Hi Ming,

Requesting irq is called by driver of IP, while whether it is shared or
not depends on SoC where IP lives, so ideally it seems platform should
inform the driver whether it is shared & driver should do what platform
tells. Or else to be safe, it should be made shared always ?

This may not make much sense now w.r.t gpmc, but would be applicable
once gpmc becomes a driver (undergoing conversion), but may not be that
important as there are no SoCs presently sharing gpmc interrupt (afaik)

Regards
Afzal

[1] http://www.mail-archive.com/linux-omap at vger.kernel.org/msg68745.html
[2] http://www.mail-archive.com/linux-omap at vger.kernel.org/msg67496.html

[PATCH] ARM: OMAP2+: fix gpmc request_irq

[Shilimkar, Santosh]

On Fri, May 18, 2012 at 4:21 PM, Mohammed, Afzal <afzal@ti.com> wrote:
> Hi Santosh,
>
> On Fri, May 18, 2012 at 15:43:31, Shilimkar, Santosh wrote:
>
>> On the side note, looks like GMPC too needs to be converted
>> to sparse IRQ since it seems to be creating a dummy irqchip
>> and dispatching secondary handlers based on chip selects.
>
> GPMC dummy chip is being modified to so that irq descriptors are created
> dynamically [1], with a final goal of reaching something similar to [2],
> please let me know if any comments on those.
>
Cool. So it's already getting cleaned-up.
Thanks for the reference.

Regards
Santosh

[PATCH] ARM: OMAP2+: fix gpmc request_irq

[Ming Lei]

Hi,

On Fri, May 18, 2012 at 6:13 PM, Shilimkar, Santosh
<santosh.shilimkar@ti.com> wrote:
> Generally IRQF_SHARED means, the interrupt line is only one but
> multiple devices can generate the interrupts. If you connect
> different devices on GMPC using chip selects like Ethernet controller,
> Nand, NOR contoller, all of they can generate events which can be
> reported by the GPMC. That's the reason I shared the line
> is shared.

IMO, it depends on if the event handlers of other chips(ethernet, nand, nor..)
share the same irq number(GPMC_IRQ). If so, it should be IRQF_SHARED.
If they are handled in its own irq number(it may convert to irq dispatching),
that shouldn't be IRQF_SHARED.

On Fri, May 18, 2012 at 6:51 PM, Mohammed, Afzal <afzal@ti.com> wrote:
> Hi Ming,
>
> Requesting irq is called by driver of IP, while whether it is shared or
> not depends on SoC where IP lives, so ideally it seems platform should
> inform the driver whether it is shared & driver should do what platform
> tells. Or else to be safe, it should be made shared always ?

Shared or not(IRQF_SHARED) may be determined by hardware and
software handling.

If the same interrupt line is shared by several peripherals and all interrupt
handlers are requested to same interrupt number for handling IRQs from
these peripherals, it should be IRQF_SHARED.

>
> This may not make much sense now w.r.t gpmc, but would be applicable
> once gpmc becomes a driver (undergoing conversion), but may not be that
> important as there are no SoCs presently sharing gpmc interrupt (afaik)

Looks the fix isn't needed if so.

Anyway, thanks your guys for exposing much info about GPMC irq handling.

Thanks,
--
Ming Lei