diff mbox

IRQ #0 broken on ARM

Message ID 87egsw5jen.fsf@free.fr (mailing list archive)
State New, archived
Headers show

Commit Message

Robert Jarzmik Nov. 21, 2014, 9:32 p.m. UTC
Marc Zyngier <marc.zyngier@arm.com> writes:
>> Linus has decreed it to not be a valid IRQ number, and that's basically
>> the end of the discussion.  Generic code, and drivers, will increasingly
>> decide that IRQ0 is not valid, and objecting to it has, and will continue
>> to elicit a response of "fix ARM".
>
> I'm fine with that.
For pxa, why not do something like that [1] ?

Cheers.

--
Robert

[1]
---8>---

From 551eaf75934bd84939a40781470ed3c04d17507a Mon Sep 17 00:00:00 2001
From: Robert Jarzmik <robert.jarzmik@free.fr>
Date: Fri, 21 Nov 2014 22:11:42 +0100
Subject: [PATCH] ARM: pxa: arbitrarily set first interrupt number

As IRQ0, the legacy timer interrupt should not be used as an interrupt
number, shift the interrupts by a fixed number.

As we had in a special case a shift of 16 when ISA bus was used on a
PXA, use that value as the first interrupt number, regardless of ISA or
not.

Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr>
---
 arch/arm/mach-pxa/include/mach/irqs.h | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

Comments

Dmitry Baryshkov Nov. 21, 2014, 10:20 p.m. UTC | #1
2014-11-22 0:32 GMT+03:00 Robert Jarzmik <robert.jarzmik@free.fr>:
> Marc Zyngier <marc.zyngier@arm.com> writes:
>>> Linus has decreed it to not be a valid IRQ number, and that's basically
>>> the end of the discussion.  Generic code, and drivers, will increasingly
>>> decide that IRQ0 is not valid, and objecting to it has, and will continue
>>> to elicit a response of "fix ARM".
>>
>> I'm fine with that.
> For pxa, why not do something like that [1] ?
>
> Cheers.
>
> --
> Robert
>
> [1]
> ---8>---
>
> From 551eaf75934bd84939a40781470ed3c04d17507a Mon Sep 17 00:00:00 2001
> From: Robert Jarzmik <robert.jarzmik@free.fr>
> Date: Fri, 21 Nov 2014 22:11:42 +0100
> Subject: [PATCH] ARM: pxa: arbitrarily set first interrupt number
>
> As IRQ0, the legacy timer interrupt should not be used as an interrupt
> number, shift the interrupts by a fixed number.
>
> As we had in a special case a shift of 16 when ISA bus was used on a
> PXA, use that value as the first interrupt number, regardless of ISA or
> not.

This will shift the issue from PXA_SSP3 interrupt to ISA IRQ0.
On the other hand ISA IRQs are used only on viper and zeus boards.
And those boards explicitly mark IRQ0 as unused (because it is not
routed through CPLD).

I have another question. As for me, those "ISA" interrupts being placed
in front of PXA interrupts look like some kind of legacy stuff. Do we
still require for "ISA" (well, PC/104) interrupts to be the first ones?
Rob Herring Nov. 21, 2014, 10:27 p.m. UTC | #2
On Fri, Nov 21, 2014 at 3:32 PM, Robert Jarzmik <robert.jarzmik@free.fr> wrote:
> Marc Zyngier <marc.zyngier@arm.com> writes:
>>> Linus has decreed it to not be a valid IRQ number, and that's basically
>>> the end of the discussion.  Generic code, and drivers, will increasingly
>>> decide that IRQ0 is not valid, and objecting to it has, and will continue
>>> to elicit a response of "fix ARM".
>>
>> I'm fine with that.
> For pxa, why not do something like that [1] ?
>
> Cheers.
>
> --
> Robert
>
> [1]
> ---8>---
>
> From 551eaf75934bd84939a40781470ed3c04d17507a Mon Sep 17 00:00:00 2001
> From: Robert Jarzmik <robert.jarzmik@free.fr>
> Date: Fri, 21 Nov 2014 22:11:42 +0100
> Subject: [PATCH] ARM: pxa: arbitrarily set first interrupt number
>
> As IRQ0, the legacy timer interrupt should not be used as an interrupt
> number, shift the interrupts by a fixed number.
>
> As we had in a special case a shift of 16 when ISA bus was used on a
> PXA, use that value as the first interrupt number, regardless of ISA or
> not.
>
> Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr>
> ---
>  arch/arm/mach-pxa/include/mach/irqs.h | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
>
> diff --git a/arch/arm/mach-pxa/include/mach/irqs.h b/arch/arm/mach-pxa/include/mach/irqs.h
> index 48c2fd8..9d8983f 100644
> --- a/arch/arm/mach-pxa/include/mach/irqs.h
> +++ b/arch/arm/mach-pxa/include/mach/irqs.h
> @@ -14,12 +14,9 @@
>
>  #ifdef CONFIG_PXA_HAVE_ISA_IRQS

You can get rid of this ifdef and kconfig symbol. It is only used here.

>  #define PXA_ISA_IRQ(x) (x)
> -#define PXA_ISA_IRQ_NUM        (16)
> -#else
> -#define PXA_ISA_IRQ_NUM        (0)
>  #endif
>
> -#define PXA_IRQ(x)     (PXA_ISA_IRQ_NUM + (x))
> +#define PXA_IRQ(x)     (16 + (x))

Perhaps use NR_IRQS_LEGACY here.

>  #define IRQ_SSP3       PXA_IRQ(0)      /* SSP3 service request */
>  #define IRQ_MSL                PXA_IRQ(1)      /* MSL Interface interrupt */
> --
> 2.1.0
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Robert Jarzmik Nov. 22, 2014, 12:18 p.m. UTC | #3
Rob Herring <robherring2@gmail.com> writes:

>>  #ifdef CONFIG_PXA_HAVE_ISA_IRQS
>
> You can get rid of this ifdef and kconfig symbol. It is only used here.
Right.

>> -#define PXA_IRQ(x)     (PXA_ISA_IRQ_NUM + (x))
>> +#define PXA_IRQ(x)     (16 + (x))
>
> Perhaps use NR_IRQS_LEGACY here.
Ah yes, good idea.

I'll sent a proper patch next time, not an attached piece of code.

Cheers.
Dmitry Baryshkov Nov. 22, 2014, 12:40 p.m. UTC | #4
2014-11-22 15:18 GMT+03:00 Robert Jarzmik <robert.jarzmik@free.fr>:
> Rob Herring <robherring2@gmail.com> writes:
>
>>>  #ifdef CONFIG_PXA_HAVE_ISA_IRQS
>>
>> You can get rid of this ifdef and kconfig symbol. It is only used here.
> Right.
>
>>> -#define PXA_IRQ(x)     (PXA_ISA_IRQ_NUM + (x))
>>> +#define PXA_IRQ(x)     (16 + (x))
>>
>> Perhaps use NR_IRQS_LEGACY here.
> Ah yes, good idea.

What about using NUM_ISA_INTERRUPTS? This would be logical
if viper & zeus were converted to call irq_domain_add_legacy_isa.
Robert Jarzmik Nov. 22, 2014, 12:55 p.m. UTC | #5
Dmitry Eremin-Solenikov <dbaryshkov@gmail.com> writes:

>>> Perhaps use NR_IRQS_LEGACY here.
>> Ah yes, good idea.
>
> What about using NUM_ISA_INTERRUPTS? This would be logical
> if viper & zeus were converted to call irq_domain_add_legacy_isa.
That would make include/mach/irqs.h depend on include/linux/irqdomain.h.

That's something that makes me fell uneasy, as this irqs.h could be very well
used in assembler files, and I must check it.

On the other hand, NR_IRQS_LEGACY is defined in include/asm/irq.h, and that one
looks a better candidate for inclusion in mach/irqs.h.

Cheers.
diff mbox

Patch

diff --git a/arch/arm/mach-pxa/include/mach/irqs.h b/arch/arm/mach-pxa/include/mach/irqs.h
index 48c2fd8..9d8983f 100644
--- a/arch/arm/mach-pxa/include/mach/irqs.h
+++ b/arch/arm/mach-pxa/include/mach/irqs.h
@@ -14,12 +14,9 @@ 
 
 #ifdef CONFIG_PXA_HAVE_ISA_IRQS
 #define PXA_ISA_IRQ(x)	(x)
-#define PXA_ISA_IRQ_NUM	(16)
-#else
-#define PXA_ISA_IRQ_NUM	(0)
 #endif
 
-#define PXA_IRQ(x)	(PXA_ISA_IRQ_NUM + (x))
+#define PXA_IRQ(x)	(16 + (x))
 
 #define IRQ_SSP3	PXA_IRQ(0)	/* SSP3 service request */
 #define IRQ_MSL		PXA_IRQ(1)	/* MSL Interface interrupt */