2006-11-20 11:04:51

by Stefan Roese

[permalink] [raw]
Subject: [PATCH] serial: Use real irq on UART0 (IRQ = 0) on PPC4xx systems

This patch fixes a problem seen on multiple 4xx platforms, where
the UART0 interrupt number is 0. The macro "is_real_interrupt" lead
on those systems to not use an real interrupt but the timer based
implementation.

This problem was detected and fixed by
Charles Gales <[email protected]> and John Baxter <[email protected]>
from AMCC.

Signed-off-by: Stefan Roese <[email protected]>

---
commit f83094acd3258575c9a5cdb1d65e241c16eff03a
tree 72582c5eeb2a9c8ea57287616d51e42fff8ed641
parent f56f68c4e1977f0e884a304af4c617bed4909417
author Stefan Roese <[email protected]> Sat, 18 Nov 2006 10:28:50 +0100
committer Stefan Roese <[email protected]> Sat, 18 Nov 2006 10:28:50 +0100

drivers/serial/8250.c | 22 ++++++++++++++--------
1 files changed, 14 insertions(+), 8 deletions(-)

diff --git a/drivers/serial/8250.c b/drivers/serial/8250.c
index e34bd03..a51679e 100644
--- a/drivers/serial/8250.c
+++ b/drivers/serial/8250.c
@@ -76,8 +76,14 @@ static unsigned int nr_uarts = CONFIG_SE
* We default to IRQ0 for the "no irq" hack. Some
* machine types want others as well - they're free
* to redefine this in their header file.
+ * NOTE: Some PPC4xx use IRQ0 for a UART Interrupt, so
+ * we will assume that the IRQ is always real
*/
+#ifdef CONFIG_4xx
+#define is_real_interrupt(irq) (1)
+#else
#define is_real_interrupt(irq) ((irq) != 0)
+#endif

#ifdef CONFIG_SERIAL_8250_DETECT_IRQ
#define CONFIG_SERIAL_DETECT_IRQ 1


2006-11-20 11:24:43

by Peter Korsgaard

[permalink] [raw]
Subject: Re: [PATCH] serial: Use real irq on UART0 (IRQ = 0) on PPC4xx systems

>>>>> "Stefan" == Stefan Roese <[email protected]> writes:

Hi,

Stefan> This patch fixes a problem seen on multiple 4xx platforms,
Stefan> where the UART0 interrupt number is 0. The macro
Stefan> "is_real_interrupt" lead on those systems to not use an real
Stefan> interrupt but the timer based implementation.

That's not the proper way to fix this. Instead remap the interrupt
number to make 0 invalid.

I sent a similar patch some months ago and that was the reply I got:

http://marc.theaimsgroup.com/?t=114363521500003&r=1&w=2

--
Bye, Peter Korsgaard

2006-11-20 11:37:04

by Alan

[permalink] [raw]
Subject: Re: [PATCH] serial: Use real irq on UART0 (IRQ = 0) on PPC4xx systems

On Mon, 20 Nov 2006 12:00:36 +0100
Stefan Roese <[email protected]> wrote:

> This patch fixes a problem seen on multiple 4xx platforms, where
> the UART0 interrupt number is 0. The macro "is_real_interrupt" lead
> on those systems to not use an real interrupt but the timer based
> implementation.

NAK.

Zero means "no interrupt" in the Linux space. If you have a physical IRQ
0 remap it to a convenient number (eg map IRQ's + 1, or stick it on the
end). The logical and physical IRQ numbering in Linux don't have to match
up - and given some platforms have IRQ numbering per bus and the like
clearly doesn't in many cases.

2006-11-20 11:55:13

by Stefan Roese

[permalink] [raw]
Subject: Re: [PATCH] serial: Use real irq on UART0 (IRQ = 0) on PPC4xx systems

On Monday 20 November 2006 12:42, Alan wrote:
> On Mon, 20 Nov 2006 12:00:36 +0100
>
> Stefan Roese <[email protected]> wrote:
> > This patch fixes a problem seen on multiple 4xx platforms, where
> > the UART0 interrupt number is 0. The macro "is_real_interrupt" lead
> > on those systems to not use an real interrupt but the timer based
> > implementation.
>
> NAK.

I knew it. ;-)

> Zero means "no interrupt" in the Linux space. If you have a physical IRQ
> 0 remap it to a convenient number (eg map IRQ's + 1, or stick it on the
> end). The logical and physical IRQ numbering in Linux don't have to match
> up - and given some platforms have IRQ numbering per bus and the like
> clearly doesn't in many cases.

Let's see, if I got this right. You mean that on such a platform, where 0 is a
valid physical IRQ, we should assign another value as virtual IRQ number (not
0 and not -1 of course). And then the platform "pic" implementation should
take care of the remapping of these virtual IRQ numbers to the physical
numbers.

Correct?

Best regards,
Stefan

2006-11-20 12:04:26

by Alan

[permalink] [raw]
Subject: Re: [PATCH] serial: Use real irq on UART0 (IRQ = 0) on PPC4xx systems

On Mon, 20 Nov 2006 12:54:32 +0100 (MET)
Stefan Roese <[email protected]> wrote:
> Let's see, if I got this right. You mean that on such a platform, where 0 is a
> valid physical IRQ, we should assign another value as virtual IRQ number (not
> 0 and not -1 of course). And then the platform "pic" implementation should
> take care of the remapping of these virtual IRQ numbers to the physical
> numbers.
>
> Correct?

Absolutely correct in all the detail.

2006-11-20 13:05:04

by Russell King

[permalink] [raw]
Subject: Re: [PATCH] serial: Use real irq on UART0 (IRQ = 0) on PPC4xx systems

On Mon, Nov 20, 2006 at 12:10:15PM +0000, Alan wrote:
> On Mon, 20 Nov 2006 12:54:32 +0100 (MET)
> Stefan Roese <[email protected]> wrote:
> > Let's see, if I got this right. You mean that on such a platform, where 0 is a
> > valid physical IRQ, we should assign another value as virtual IRQ number (not
> > 0 and not -1 of course). And then the platform "pic" implementation should
> > take care of the remapping of these virtual IRQ numbers to the physical
> > numbers.
> >
> > Correct?
>
> Absolutely correct in all the detail.

Since IRQ0 is not valid, can we arrange for the generic interrupt
infrastructure to always fail it's allocation, and then remove the
utterly unused bloatful irq_desc[0] ?

Didn't think so since x86 folk would scream. Wait a moment, x86 can
map IRQ0 to some other number for the timer interrupt, just like
other architectures are being forced to map their UART interrupts.

--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of: 2.6 Serial core

2006-11-20 13:26:40

by Sergei Shtylyov

[permalink] [raw]
Subject: Re: [PATCH] serial: Use real irq on UART0 (IRQ = 0) on PPC4xx systems

Hello.

Stefan Roese wrote:
> This patch fixes a problem seen on multiple 4xx platforms, where
> the UART0 interrupt number is 0. The macro "is_real_interrupt" lead
> on those systems to not use an real interrupt but the timer based
> implementation.

> This problem was detected and fixed by
> Charles Gales <[email protected]> and John Baxter <[email protected]>
> from AMCC.

> Signed-off-by: Stefan Roese <[email protected]>

> ---
> commit f83094acd3258575c9a5cdb1d65e241c16eff03a
> tree 72582c5eeb2a9c8ea57287616d51e42fff8ed641
> parent f56f68c4e1977f0e884a304af4c617bed4909417
> author Stefan Roese <[email protected]> Sat, 18 Nov 2006 10:28:50 +0100
> committer Stefan Roese <[email protected]> Sat, 18 Nov 2006 10:28:50 +0100

> drivers/serial/8250.c | 22 ++++++++++++++--------
> 1 files changed, 14 insertions(+), 8 deletions(-)

> diff --git a/drivers/serial/8250.c b/drivers/serial/8250.c
> index e34bd03..a51679e 100644
> --- a/drivers/serial/8250.c
> +++ b/drivers/serial/8250.c
> @@ -76,8 +76,14 @@ static unsigned int nr_uarts = CONFIG_SE
> * We default to IRQ0 for the "no irq" hack. Some
> * machine types want others as well - they're free
> * to redefine this in their header file.
> + * NOTE: Some PPC4xx use IRQ0 for a UART Interrupt, so
> + * we will assume that the IRQ is always real
> */
> +#ifdef CONFIG_4xx
> +#define is_real_interrupt(irq) (1)
> +#else
> #define is_real_interrupt(irq) ((irq) != 0)
> +#endif

This should have been done in <asm/serial.h> instead, like this:

#undef is_real_interrupt(irq)
#define is_real_interrupt(irq) 1

Not too smart as well,but at least this wasy you won't be pulluting the
generic code...

WBR, Sergei

2006-11-20 21:21:30

by Guennadi Liakhovetski

[permalink] [raw]
Subject: Re: [PATCH] serial: Use real irq on UART0 (IRQ = 0) on PPC4xx systems

On Mon, 20 Nov 2006, Russell King wrote:

> > On Mon, 20 Nov 2006 12:54:32 +0100 (MET)
> > Stefan Roese <[email protected]> wrote:
> > > Let's see, if I got this right. You mean that on such a platform, where 0 is a
> > > valid physical IRQ, we should assign another value as virtual IRQ number (not
> > > 0 and not -1 of course). And then the platform "pic" implementation should
> > > take care of the remapping of these virtual IRQ numbers to the physical
> > > numbers.
>
> Since IRQ0 is not valid, can we arrange for the generic interrupt
> infrastructure to always fail it's allocation, and then remove the
> utterly unused bloatful irq_desc[0] ?
>
> Didn't think so since x86 folk would scream. Wait a moment, x86 can
> map IRQ0 to some other number for the timer interrupt, just like
> other architectures are being forced to map their UART interrupts.

I think, what Russell means, is this:

#define is_real_interrupt(irq) ((irq) != NO_IRQ)

where the NO_IRQ macro has been introduced a LONG time ago specifically
for this purpose, and is conveniently defined on some platforms to
(unsigned int)-1 or similar, including asm-powerpc/irq.h. And yes, this
has been discussed MANY times.

Thanks
Guennadi
---
Guennadi Liakhovetski