linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
* RFC: i8259.c cleanup
@ 2001-11-06 23:15 hollis
  2001-11-07  6:37 ` Dan Malek
                   ` (2 more replies)
  0 siblings, 3 replies; 24+ messages in thread
From: hollis @ 2001-11-06 23:15 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: davidm, Sven.Dickert

[-- Attachment #1: Type: text/plain, Size: 2254 bytes --]

i8259.c hasn't worked right for a long time on IBM PReP's, which crippled
those that use IDE ("hda: lost interrupt" forever). I sent a note about this a
while back
(http://lists.linuxppc.org/linuxppc-workstation/200109/msg00030.html) but
never got any answers...

Anyways, by reading from 0xbffffff0 we can get the active irq without having
to play with the 8259 directly (which we were doing wrong; see above URL).
This is true on the IBM PReP's because the 8259's are implemented in the Fire
Coral SIO bridge (which is on the PCI bus), so it can respond to the PHB's
interrupt request. (Unless I'm mistaken it's the PHB, in this case MPC105,
which decodes 0xbffffff0 and asks the system interrupt controller to supply
the active irq number.)

The following patch (to linuxppc_2_4_devel) does exactly that - it reads from
0xbffffff0 instead of the 8259 directly. This seems to have completely fixed
the lost irq problems that we (IBM PReP users) have been having for a long
time. This patch has only been tested on a 6050 so far, but that should cover
all the Carolina's.

I'm not positive this trick will work on every board that has an 8259. The
PReP spec calls for it, and anything with an MPC105/106 should support it. I'm
really hoping that covers everyone...

Also, with this trick our i8259.c could be made almost identical to
i386/mips/alpha, which already know the irq vector entering mask_and_ack().
A lot of our boards call i8259_irq() directly though, which would have to be
changed. I only suggest it because the i386 folks apparently know 8259's
better than we do... :) Oh, and alpha uses the rotating priority feature which
sounds cool. :)

The patch also cleans up some whitespace (parts of i8259.c were apparently
copied and pasted from one xterm to another), adds a few comments, and adds
resource_request's for both 8259's.

i8259_irq() (called from a variety of embedded files) is passed a single
argument, int cpu. This argument is never used and should probably be deleted?

I have one other question, regarding spin_lock vs spin_lock_irqsave. We use
both in different places in i8259.c ("_irqsave" is commented out in some
places). i386 and mips use irqsave in their i8259.c's, alpha does not. Which
is appropriate here?

-Hollis

[-- Attachment #2: 8259-cleanup.patch --]
[-- Type: text/plain, Size: 6733 bytes --]

===== arch/ppc/kernel/i8259.c 1.10 vs edited =====
--- 1.10/arch/ppc/kernel/i8259.c	Sun Nov  4 04:07:37 2001
+++ edited/arch/ppc/kernel/i8259.c	Tue Nov  6 17:01:49 2001
@@ -4,11 +4,14 @@

 #include <linux/stddef.h>
 #include <linux/init.h>
+#include <linux/ioport.h>
 #include <linux/sched.h>
 #include <linux/signal.h>
 #include <asm/io.h>
 #include <asm/i8259.h>

+static volatile char *pci_intack; /* RO, gives us the irq vector */
+
 unsigned char cached_8259[2] = { 0xff, 0xff };
 #define cached_A1 (cached_8259[0])
 #define cached_21 (cached_8259[1])
@@ -22,30 +25,19 @@
 	int irq;

 	spin_lock/*_irqsave*/(&i8259_lock/*, flags*/);
-        /*
-         * Perform an interrupt acknowledge cycle on controller 1
-         */
-        outb(0x0C, 0x20);
-        irq = inb(0x20) & 7;
-        if (irq == 2)
-        {
-                /*
-                 * Interrupt is cascaded so perform interrupt
-                 * acknowledge on controller 2
-                 */
-                outb(0x0C, 0xA0);
-                irq = (inb(0xA0) & 7) + 8;
-        }
-        else if (irq==7)
-        {
-                /*
-                 * This may be a spurious interrupt
-                 *
-                 * Read the interrupt status register. If the most
-                 * significant bit is not set then there is no valid
+	/*
+	 * Perform an interrupt acknowledge cycle on controller 1
+	 */
+	irq = *pci_intack & 0xff;
+	if (irq == 7) {
+		/*
+		 * This may be a spurious interrupt
+		 *
+		 * Read the interrupt status register. If the most
+		 * significant bit is not set then there is no valid
 		 * interrupt
 		 */
-		outb(0x0b, 0x20);
+		outb(0x0b, 0x20); /* select ISR */
 		if(~inb(0x20)&0x80) {
 			spin_unlock/*_irqrestore*/(&i8259_lock/*, flags*/);
 			return -1;
@@ -60,28 +52,28 @@
 	unsigned long flags;

 	spin_lock_irqsave(&i8259_lock, flags);
-        if ( irq_nr >= i8259_pic_irq_offset )
-                irq_nr -= i8259_pic_irq_offset;
+	if ( irq_nr >= i8259_pic_irq_offset )
+		irq_nr -= i8259_pic_irq_offset;

-        if (irq_nr > 7) {
-                cached_A1 |= 1 << (irq_nr-8);
-                inb(0xA1);      /* DUMMY */
-                outb(cached_A1,0xA1);
-                outb(0x20,0xA0);        /* Non-specific EOI */
-                outb(0x20,0x20);        /* Non-specific EOI to cascade */
-        } else {
-                cached_21 |= 1 << irq_nr;
-                inb(0x21);      /* DUMMY */
-                outb(cached_21,0x21);
-                outb(0x20,0x20);        /* Non-specific EOI */
-        }
+	if (irq_nr > 7) {
+		cached_A1 |= 1 << (irq_nr-8);
+		inb(0xA1); /* DUMMY */
+		outb(cached_A1,0xA1);
+		outb(0x20,0xA0); /* Non-specific EOI */
+		outb(0x20,0x20); /* Non-specific EOI to cascade */
+	} else {
+		cached_21 |= 1 << irq_nr;
+		inb(0x21); /* DUMMY */
+		outb(cached_21,0x21);
+		outb(0x20,0x20); /* Non-specific EOI */
+	}
 	spin_unlock_irqrestore(&i8259_lock, flags);
 }

 static void i8259_set_irq_mask(int irq_nr)
 {
-        outb(cached_A1,0xA1);
-        outb(cached_21,0x21);
+	outb(cached_A1,0xA1);
+	outb(cached_21,0x21);
 }

 static void i8259_mask_irq(unsigned int irq_nr)
@@ -89,13 +81,13 @@
 	unsigned long flags;

 	spin_lock_irqsave(&i8259_lock, flags);
-        if ( irq_nr >= i8259_pic_irq_offset )
-                irq_nr -= i8259_pic_irq_offset;
-        if ( irq_nr < 8 )
-                cached_21 |= 1 << irq_nr;
-        else
-                cached_A1 |= 1 << (irq_nr-8);
-        i8259_set_irq_mask(irq_nr);
+	if ( irq_nr >= i8259_pic_irq_offset )
+		irq_nr -= i8259_pic_irq_offset;
+	if ( irq_nr < 8 )
+		cached_21 |= 1 << irq_nr;
+	else
+		cached_A1 |= 1 << (irq_nr-8);
+	i8259_set_irq_mask(irq_nr);
 	spin_unlock_irqrestore(&i8259_lock, flags);
 }

@@ -104,13 +96,13 @@
 	unsigned long flags;

 	spin_lock_irqsave(&i8259_lock, flags);
-        if ( irq_nr >= i8259_pic_irq_offset )
-                irq_nr -= i8259_pic_irq_offset;
-        if ( irq_nr < 8 )
-                cached_21 &= ~(1 << irq_nr);
-        else
-                cached_A1 &= ~(1 << (irq_nr-8));
-        i8259_set_irq_mask(irq_nr);
+	if ( irq_nr >= i8259_pic_irq_offset )
+		irq_nr -= i8259_pic_irq_offset;
+	if ( irq_nr < 8 )
+		cached_21 &= ~(1 << irq_nr);
+	else
+		cached_A1 &= ~(1 << (irq_nr-8));
+	i8259_set_irq_mask(irq_nr);
 	spin_unlock_irqrestore(&i8259_lock, flags);
 }

@@ -121,14 +113,22 @@
 }

 struct hw_interrupt_type i8259_pic = {
-        " i8259    ",
-        NULL,
-        NULL,
-        i8259_unmask_irq,
-        i8259_mask_irq,
-        i8259_mask_and_ack_irq,
-        i8259_end_irq,
-        NULL
+	" i8259    ",
+	NULL,
+	NULL,
+	i8259_unmask_irq,
+	i8259_mask_irq,
+	i8259_mask_and_ack_irq,
+	i8259_end_irq,
+	NULL
+};
+
+static struct resource pic1_io_resource = {
+	"pic1", 0x20, 0x3f, IORESOURCE_BUSY
+};
+
+static struct resource pic2_io_resource = {
+	"pic2", 0xa0, 0xbf, IORESOURCE_BUSY
 };

 void __init i8259_init(void)
@@ -136,21 +136,26 @@
 	unsigned long flags;

 	spin_lock_irqsave(&i8259_lock, flags);
-        /* init master interrupt controller */
-        outb(0x11, 0x20); /* Start init sequence */
-        outb(0x00, 0x21); /* Vector base */
-        outb(0x04, 0x21); /* edge tiggered, Cascade (slave) on IRQ2 */
-        outb(0x01, 0x21); /* Select 8086 mode */
-        outb(0xFF, 0x21); /* Mask all */
-        /* init slave interrupt controller */
-        outb(0x11, 0xA0); /* Start init sequence */
-        outb(0x08, 0xA1); /* Vector base */
-        outb(0x02, 0xA1); /* edge triggered, Cascade (slave) on IRQ2 */
-        outb(0x01, 0xA1); /* Select 8086 mode */
-        outb(0xFF, 0xA1); /* Mask all */
-        outb(cached_A1, 0xA1);
-        outb(cached_21, 0x21);
+	/* init master interrupt controller */
+	outb(0x11, 0x20); /* Start init sequence */
+	outb(0x00, 0x21); /* Vector base */
+	outb(0x04, 0x21); /* edge tiggered, Cascade (slave) on IRQ2 */
+	outb(0x01, 0x21); /* Select 8086 mode */
+	outb(0xFF, 0x21); /* Mask all */
+	/* init slave interrupt controller */
+	outb(0x11, 0xA0); /* Start init sequence */
+	outb(0x08, 0xA1); /* Vector base */
+	outb(0x02, 0xA1); /* edge triggered, Cascade (slave) on IRQ2 */
+	outb(0x01, 0xA1); /* Select 8086 mode */
+	outb(0xFF, 0xA1); /* Mask all */
+	outb(cached_A1, 0xA1);
+	outb(cached_21, 0x21);
 	spin_unlock_irqrestore(&i8259_lock, flags);
-        request_irq( i8259_pic_irq_offset + 2, no_action, SA_INTERRUPT,
-                     "82c59 secondary cascade", NULL );
+
+	request_irq( i8259_pic_irq_offset + 2, no_action, SA_INTERRUPT,
+				"82c59 secondary cascade", NULL );
+	request_resource(&ioport_resource, &pic1_io_resource);
+	request_resource(&ioport_resource, &pic2_io_resource);
+
+	pci_intack = ioremap(0xbffffff0, 1);
 }

^ permalink raw reply	[flat|nested] 24+ messages in thread
* Re: RFC: i8259.c cleanup
@ 2001-11-07  4:43 Michael Sokolov
  2001-11-07  6:31 ` Dag Nygren
  2001-11-07  6:38 ` Dan Malek
  0 siblings, 2 replies; 24+ messages in thread
From: Michael Sokolov @ 2001-11-07  4:43 UTC (permalink / raw)
  To: linuxppc-dev


hollis@austin.ibm.com wrote:

> Anyways, by reading from 0xbffffff0 we can get the active irq without having
> to play with the 8259 directly (which we were doing wrong; see above URL).
> This is true on the IBM PReP's because the 8259's are implemented in the Fire
> Coral SIO bridge (which is on the PCI bus), so it can respond to the PHB's
> interrupt request. (Unless I'm mistaken it's the PHB, in this case MPC105,
> which decodes 0xbffffff0 and asks the system interrupt controller to supply
> the active irq number.)

Different host bridges provide different means for generating PCI Interrupt
Acknowledge cycles. None of the bridges I've worked with (IBM CPC700, IBM
CPC710, and Galileo GT64260) does it via BFFFFFF0. CPC710 does it via a
register in the IBM PHB structure for each of its two PCI interfaces, CPC700
has a special register, and GT64260 has special registers for PCI0 and PCI1
Interrupt Acknowledge in its internal registers space.

> The following patch (to linuxppc_2_4_devel) does exactly that - it reads from
> 0xbffffff0 instead of the 8259 directly.

i8259.c is not specific to PREP, it's used by many platforms in the tree. Your
patch as posted will break a lot of platforms in the tree, including all SBS
boards which use one of the above host bridges and 8259s in a south bridge.

MS

** Sent via the linuxppc-dev mail list. See http://lists.linuxppc.org/

^ permalink raw reply	[flat|nested] 24+ messages in thread
* Re: RFC: i8259.c cleanup
@ 2001-11-07 20:31 Michael Sokolov
  0 siblings, 0 replies; 24+ messages in thread
From: Michael Sokolov @ 2001-11-07 20:31 UTC (permalink / raw)
  To: linuxppc-dev


Dan Malek <dan@mvista.com> wrote:

> > .....None of the bridges I've worked with (IBM CPC700, IBM
> > CPC710, and Galileo GT64260) does it via BFFFFFF0.
>
> That's because those are not PReP bridges..........

Of course they aren't, that was exactly my point, there's more to the world
than PREP.

MS

** Sent via the linuxppc-dev mail list. See http://lists.linuxppc.org/

^ permalink raw reply	[flat|nested] 24+ messages in thread
* Re: RFC: i8259.c cleanup
@ 2001-11-07 21:13 Michael Sokolov
  2001-11-07 21:22 ` Geert Uytterhoeven
  0 siblings, 1 reply; 24+ messages in thread
From: Michael Sokolov @ 2001-11-07 21:13 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Sven.Dickert, davidm, hollis, paubert


Gabriel Paubert <paubert@iram.es> wrote:

> The address is not always 0xbffffff0, it is bridge dependent. However this
> address is normally found in OF device tree, in residual data and as a
> last resort could be made host bridge dependent. It has to be ioremapped
> anyway...

OF device tree, residual data, etc. all assume PREP/CHRP/PMAC mentality. There
are also other PPC boards supported in the tree. Those embody all machine
knowledge in the board port. They already have to have full knowledge of every
chip on the board, including the host bridge.

> I actually wonder
> whether it would be better to split it in two cases:
>
> - 8259 is the only interrupt controller in the system
>
> - 8259 is cascaded interrupt controller on an openpic

These are not the only possibilities. Adirondack, for example, has the PC-style
8259 pair cascaded to the Adirondack interrupt controller (completely original
design), which drives the INT# lines to the two CPUs.

MS

** Sent via the linuxppc-dev mail list. See http://lists.linuxppc.org/

^ permalink raw reply	[flat|nested] 24+ messages in thread
* Re: RFC: i8259.c cleanup
@ 2001-11-07 21:22 Michael Sokolov
  2001-11-07 21:26 ` Geert Uytterhoeven
  0 siblings, 1 reply; 24+ messages in thread
From: Michael Sokolov @ 2001-11-07 21:22 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Sven.Dickert, davidm, hollis, paubert


hollis@austin.ibm.com wrote:

> Would you say poking the 8259 directly is more "right" than using 0xbffffff0
> (or equivalent)?

Both methods are equally right. However, the polling method requires knowledge
only of the 8259s, which is required anyway, while the other method requires
knowledge of the interrupt acknowledge mechanism. It is also possible for
somebody to design a machine with 8259s without any interrupt acknowledge
mechanism at all, the 8259 datasheet will tell you that it has been designed
for such applications as well.

MS

** Sent via the linuxppc-dev mail list. See http://lists.linuxppc.org/

^ permalink raw reply	[flat|nested] 24+ messages in thread
* Re: RFC: i8259.c cleanup
@ 2001-11-07 22:43 Michael Sokolov
  0 siblings, 0 replies; 24+ messages in thread
From: Michael Sokolov @ 2001-11-07 22:43 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Sven.Dickert, davidm, hollis, paubert


Gabriel Paubert <paubert@iram.es> wrote:

> > Would you say poking the 8259 directly is more "right" than using 0xbffffff0
> > (or equivalent)?
>
> Certainly not. Most ISA bridges (which are the part which contains the
> dual 8259 Pathetic Interrupt Controller), are tested in an x86
> environment, which generate PCI interrupt acknowledge cycles. So hitting
> hardware bugs is much less likely than with polling.

In the PPC world, however, this requires the following assumptions:

1. That the 8259 pair is in a PCI-to-ISA south bridge.

2. That the above bridge is connected to the PPC-to-PCI host bridge by an
   uninterrupted PCI bus. PCI-to-PCI bridges in between may not pass Interrupt
   Acknowledge cycles. There is an SBS board coming up that will have a GT64260
   host bridge, a VT82C686B south bridge, and a P2P bridge in between. I don't
   know if the P2P bridge will pass Interrupt Acknowledge cycles, but I don't
   think there is a requirement that it does. If it does, great, but if it
   doesn't, I'm not going to tell the hardware engineers to change their
   design. We've proven the VT82C686B working on the Adirondack, and the
   Adirondack port in the current linuxppc_2_4_devel works beautifully with the
   i8259.c code using polls.

3. The host bridge must allow you to generate Interrupt Acknowledge cycles, and
   you have to know how it does it, as all host bridges do it differently.

MS

** Sent via the linuxppc-dev mail list. See http://lists.linuxppc.org/

^ permalink raw reply	[flat|nested] 24+ messages in thread
* Re: RFC: i8259.c cleanup
@ 2001-11-07 22:55 Michael Sokolov
  0 siblings, 0 replies; 24+ messages in thread
From: Michael Sokolov @ 2001-11-07 22:55 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Sven.Dickert, davidm, geert, hollis, paubert


Geert Uytterhoeven <geert@linux-m68k.org> wrote:

> That address is passed by board-specific code to the generic OpenPIC code
> during OpenPIC initialization.

What does it have to do with OpenPIC? It's the 8259 code, if it's changed to
use interrupt acknowledge, that will need this information.

> So s/an openpic/another interrupt controller/.
>
> There's no single reason why you can't pass a magic interrupt acknowledge
> address to the initialization code for the Adirondack interrupt controller.
>
> Well, this assumes you have generic `building block' interrupt controller code
> that can be cascaded arbitrarily by board-specific code.

Well, if/when you change the 8259 code to use interrupt acknowledge, you'll
have to provide an interface allowing ports to tell it how to do it. When that
happens I'll make the Adirondack and K2 ports provide this information.

MS

** Sent via the linuxppc-dev mail list. See http://lists.linuxppc.org/

^ permalink raw reply	[flat|nested] 24+ messages in thread
* Re: RFC: i8259.c cleanup
@ 2001-11-07 23:00 Michael Sokolov
  0 siblings, 0 replies; 24+ messages in thread
From: Michael Sokolov @ 2001-11-07 23:00 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Sven.Dickert, davidm, geert, hollis, paubert


Geert Uytterhoeven <geert@linux-m68k.org> wrote:

> But because you have 2 cascaded i8259s it's more complex to poll both i8259s
> than to read the single magic interrupt acknowledge register.

True, an Interrupt Acknowledge cycle is more efficient than polling. I guess we
could make two versions of the 8259 code, one using the more efficient
Interrupt Acknowledge cycle and the other using polling for machines that don't
support interrupt acknowledge.

MS

** Sent via the linuxppc-dev mail list. See http://lists.linuxppc.org/

^ permalink raw reply	[flat|nested] 24+ messages in thread
* Re: RFC: i8259.c cleanup
@ 2001-11-07 23:04 Michael Sokolov
  0 siblings, 0 replies; 24+ messages in thread
From: Michael Sokolov @ 2001-11-07 23:04 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Sven.Dickert, davidm, hollis, paubert


hollis@austin.ibm.com wrote:

> I'm looking at the "Poll Command" section of
> the 8259 docs, and i8259.c seems completely legit...

Exactly.

MS

** Sent via the linuxppc-dev mail list. See http://lists.linuxppc.org/

^ permalink raw reply	[flat|nested] 24+ messages in thread

end of thread, other threads:[~2001-11-07 23:04 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2001-11-06 23:15 RFC: i8259.c cleanup hollis
2001-11-07  6:37 ` Dan Malek
2001-11-07  9:31   ` Geert Uytterhoeven
2001-11-07  9:40     ` Gabriel Paubert
2001-11-07  9:38 ` Gabriel Paubert
2001-11-07 17:06   ` hollis
2001-11-07 17:17     ` Geert Uytterhoeven
2001-11-07 21:17     ` Gabriel Paubert
2001-11-07 22:01       ` hollis
2001-11-07 20:46 ` Val Henson
  -- strict thread matches above, loose matches on Subject: below --
2001-11-07  4:43 Michael Sokolov
2001-11-07  6:31 ` Dag Nygren
2001-11-07  7:00   ` Dan Malek
2001-11-07  7:09     ` Dag Nygren
2001-11-07  6:38 ` Dan Malek
2001-11-07 20:31 Michael Sokolov
2001-11-07 21:13 Michael Sokolov
2001-11-07 21:22 ` Geert Uytterhoeven
2001-11-07 21:22 Michael Sokolov
2001-11-07 21:26 ` Geert Uytterhoeven
2001-11-07 22:43 Michael Sokolov
2001-11-07 22:55 Michael Sokolov
2001-11-07 23:00 Michael Sokolov
2001-11-07 23:04 Michael Sokolov

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).