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  4:43 Michael Sokolov
@ 2001-11-07  6:31 ` Dag Nygren
  2001-11-07  7:00   ` Dan Malek
  2001-11-07  6:38 ` Dan Malek
  1 sibling, 1 reply; 24+ messages in thread
From: Dag Nygren @ 2001-11-07  6:31 UTC (permalink / raw)
  To: Michael Sokolov; +Cc: linuxppc-dev, dag

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

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

I think my suggestion for an improvement of the open_pic() interface
could help you in this... I posted it on the linux_embedded list, but got
no comments so far.

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

Yep.

I will attach my port for the SBS VG4 board, it uses the new open_pic() 
interface
and it would be nice if you checked it out.
If someone (?) could add it to the BK tree I would also appreciate it.

BRGDS


-- 
Dag Nygren                               email: dag@newtech.fi
Oy Espoon NewTech Ab                     phone: +358 9 8024910
Träsktorpet 3                              fax: +358 9 8024916
02360 ESBO                              Mobile: +358 400 426312
FINLAND


Here is a copy of the post to linux embedded:

Hi,

here is a working port for the VG4 PPC-board from SBS.

- The diffs are against an almost 1 week old linuxppc_2_4_devel from
BK, but there should be no big problemns there
- This port is using a new scheme for the openpic_init(), proposed
  by me some posts ago, but it was implemented in it's own
  copy of the open_pic code, open_pic2.c, and so it should be
  completely nonintrusive on the existing ports. Anyone doing
  a new port should IMH take a look at open_pic2 as it will
  make your work easier.
  It would of course be nice to get rid of the code duplication there
  is openpic <-> openpic2, preferrably by all ports beeing transferred
  to the new scheme.
- couldn't get the sym53c8xx driver in the BK kernel to work, so I
  downloaded a newer version, and this works fine. It is rather large
  so it is not included here, but if you need it, get it from it's homepage
  or send me a mail.



[-- Attachment #2: vg4.diff.gz --]
[-- Type: application/x-gzip , Size: 1501 bytes --]

[-- Attachment #3: vg4.newfiles.gz --]
[-- Type: application/x-gzip , Size: 16380 bytes --]

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

* Re: RFC: i8259.c cleanup
  2001-11-06 23:15 hollis
@ 2001-11-07  6:37 ` Dan Malek
  2001-11-07  9:31   ` Geert Uytterhoeven
  2001-11-07  9:38 ` Gabriel Paubert
  2001-11-07 20:46 ` Val Henson
  2 siblings, 1 reply; 24+ messages in thread
From: Dan Malek @ 2001-11-07  6:37 UTC (permalink / raw)
  To: hollis; +Cc: linuxppc-dev, davidm, Sven.Dickert


hollis@austin.ibm.com wrote:


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

Ummmmm.....this probably works OK on PReP, but I don't think it is going
to have the desired effect on other systems using an 8259 :-).  You need
to find a way to to make the code you changed unique to PReP.

Thanks.


	-- Dan


** 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  4:43 Michael Sokolov
  2001-11-07  6:31 ` Dag Nygren
@ 2001-11-07  6:38 ` Dan Malek
  1 sibling, 0 replies; 24+ messages in thread
From: Dan Malek @ 2001-11-07  6:38 UTC (permalink / raw)
  To: Michael Sokolov; +Cc: linuxppc-dev


Michael Sokolov 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..........


	-- Dan


** 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  6:31 ` Dag Nygren
@ 2001-11-07  7:00   ` Dan Malek
  2001-11-07  7:09     ` Dag Nygren
  0 siblings, 1 reply; 24+ messages in thread
From: Dan Malek @ 2001-11-07  7:00 UTC (permalink / raw)
  To: Dag Nygren; +Cc: Michael Sokolov, linuxppc-dev


Dag Nygren wrote:


> I think my suggestion for an improvement of the open_pic() interface
> could help you in this... I posted it on the linux_embedded list, but got
> no comments so far.

Yes, you did :-).


	-- Dan


** 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  7:00   ` Dan Malek
@ 2001-11-07  7:09     ` Dag Nygren
  0 siblings, 0 replies; 24+ messages in thread
From: Dag Nygren @ 2001-11-07  7:09 UTC (permalink / raw)
  To: Dan Malek; +Cc: Dag Nygren, Michael Sokolov, linuxppc-dev, dag


> Dag Nygren wrote:
>
>
> > I think my suggestion for an improvement of the open_pic() interface
> > could help you in this... I posted it on the linux_embedded list, but got
> > no comments so far.
>
> Yes, you did :-).

Sure, but that was on the i8259 stuff.
It is not in this patch, I left that one out as it
seemed like you did not like that ;-)

The patch I posted will not touch the i8259() !
It only replaces the openpic_init() function with
one that describes the openpic a little better.

Actually I think that this description could easily be
extended to something like the one you purposed.

Dag


** 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  6:37 ` Dan Malek
@ 2001-11-07  9:31   ` Geert Uytterhoeven
  2001-11-07  9:40     ` Gabriel Paubert
  0 siblings, 1 reply; 24+ messages in thread
From: Geert Uytterhoeven @ 2001-11-07  9:31 UTC (permalink / raw)
  To: Dan Malek; +Cc: hollis, Linux/PPC Development, davidm, Sven.Dickert


On Wed, 7 Nov 2001, Dan Malek wrote:
> hollis@austin.ibm.com wrote:
> > The following patch (to linuxppc_2_4_devel) does exactly that - it reads from
> > 0xbffffff0 instead of the 8259 directly.
>
> Ummmmm.....this probably works OK on PReP, but I don't think it is going
> to have the desired effect on other systems using an 8259 :-).  You need
> to find a way to to make the code you changed unique to PReP.

Note that CHRP already has such a thing (chrp_int_ack_special), to be used with
OpenPIC (it's passed to openpic_init()).

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds


** 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-06 23:15 hollis
  2001-11-07  6:37 ` Dan Malek
@ 2001-11-07  9:38 ` Gabriel Paubert
  2001-11-07 17:06   ` hollis
  2001-11-07 20:46 ` Val Henson
  2 siblings, 1 reply; 24+ messages in thread
From: Gabriel Paubert @ 2001-11-07  9:38 UTC (permalink / raw)
  To: hollis; +Cc: linuxppc-dev, davidm, Sven.Dickert


On Tue, 6 Nov 2001 hollis@austin.ibm.com wrote:

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

For example on OpenPIC with cascaded 8259 like Motorola MVME/MTX, the
address is 0xfeff0030 (doesn't even need a PTE if you put a large
enough BAT with CHRP mappings since it's just above PCI I/O space).

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

The question is why the polling code did not work, subtle timing problems
or is the code plain wrong ? I suspect that some 8259 accesses were not
serialized well enough.

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

No, it won't work for everybody.

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

Good, but you should add resource request for the edge/level control
register while you are at it (0x4d0/4d1). And you should not require
32 bytes for the 8259. On my boards, the super I/O control port is
0x2e-ox2f.

BTW, if you have an MTX or MVME board just try to do inl(0x20): instant
lockup, no machine check, nothing. It might lock up other boards too.
Bad hardware design, I find it scary for some of my applications.

> 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?

You should not need irqsave variants, once the interrupt is acked and
masked you can reenable interrupts explicitly with __sti if the interrupt
flags allow this (doing like i386 is probably the best). But you have to
check the interaction with the OpenPIC when cascaded. 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

	Regards,
	Gabriel.


** 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  9:31   ` Geert Uytterhoeven
@ 2001-11-07  9:40     ` Gabriel Paubert
  0 siblings, 0 replies; 24+ messages in thread
From: Gabriel Paubert @ 2001-11-07  9:40 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Dan Malek, hollis, Linux/PPC Development, davidm, Sven.Dickert


On Wed, 7 Nov 2001, Geert Uytterhoeven wrote:

>
> On Wed, 7 Nov 2001, Dan Malek wrote:
> > hollis@austin.ibm.com wrote:
> > > The following patch (to linuxppc_2_4_devel) does exactly that - it reads from
> > > 0xbffffff0 instead of the 8259 directly.
> >
> > Ummmmm.....this probably works OK on PReP, but I don't think it is going
> > to have the desired effect on other systems using an 8259 :-).  You need
> > to find a way to to make the code you changed unique to PReP.
>
> Note that CHRP already has such a thing (chrp_int_ack_special), to be used with
> OpenPIC (it's passed to openpic_init()).

Indeed, what the 8259.c code might do is use the hardware address and
fall back to polling if the address is NULL. But to avoid run time
checking and minmize icache footprint, the two cases could be considered
as different interrupt controllers.

	Regards,
	Gabriel.


** 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  9:38 ` Gabriel Paubert
@ 2001-11-07 17:06   ` hollis
  2001-11-07 17:17     ` Geert Uytterhoeven
  2001-11-07 21:17     ` Gabriel Paubert
  0 siblings, 2 replies; 24+ messages in thread
From: hollis @ 2001-11-07 17:06 UTC (permalink / raw)
  To: Gabriel Paubert; +Cc: linuxppc-dev, davidm, Sven.Dickert


On Wed, Nov 07, 2001 at 10:38:27AM +0100, Gabriel Paubert wrote:
>
> The question is why the polling code did not work, subtle timing problems
> or is the code plain wrong ? I suspect that some 8259 accesses were not
> serialized well enough.

I laid out what I saw in
http://lists.linuxppc.org/linuxppc-workstation/200109/msg00030.html . I could
be wrong, but our code doesn't seem to match the datasheet very well.

I think fixing what we have is the least intrusive, but I tried and failed, so
it was nice to write code that actually worked. :) I guess the PCI intack
stuff is too varied to be useful though...

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

> > 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.
>
> Good, but you should add resource request for the edge/level control
> register while you are at it (0x4d0/4d1). And you should not require
> 32 bytes for the 8259. On my boards, the super I/O control port is
> 0x2e-ox2f.

That's what I forgot... that code requests directly from ioport_resource,
which is what gets us into trouble on vgacon. We should use request_region
instead of request_resource. Unfortunately vgacon is initialized before the
VMM, and since request_region calls kmalloc we die very early. :/ I didn't
check to see if that's a problem here.

The problem with using request_resource(&ioport_resource) in vgacon is that
our PHB's initialize later, try to reserve their own IO regions (eg
0x0->0xffffffff), and fail because vgacon already took 0x3c0 or so.

-Hollis

** 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 17:06   ` hollis
@ 2001-11-07 17:17     ` Geert Uytterhoeven
  2001-11-07 21:17     ` Gabriel Paubert
  1 sibling, 0 replies; 24+ messages in thread
From: Geert Uytterhoeven @ 2001-11-07 17:17 UTC (permalink / raw)
  To: hollis; +Cc: Gabriel Paubert, Linux/PPC Development, davidm, Sven.Dickert


On Wed, 7 Nov 2001 hollis@austin.ibm.com wrote:
> On Wed, Nov 07, 2001 at 10:38:27AM +0100, Gabriel Paubert wrote:
> > The question is why the polling code did not work, subtle timing problems
> > or is the code plain wrong ? I suspect that some 8259 accesses were not
> > serialized well enough.
>
> I laid out what I saw in
> http://lists.linuxppc.org/linuxppc-workstation/200109/msg00030.html . I could
> be wrong, but our code doesn't seem to match the datasheet very well.
>
> I think fixing what we have is the least intrusive, but I tried and failed, so
> it was nice to write code that actually worked. :) I guess the PCI intack
> stuff is too varied to be useful though...
>
> Would you say poking the 8259 directly is more "right" than using 0xbffffff0
> (or equivalent)?

I remember some interrupt-related problems went away on my LongTrail after I
started using the interrupt acknowledge register on the host bridge instead of
the plain i8259 stuff.

> > > 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.
> >
> > Good, but you should add resource request for the edge/level control
> > register while you are at it (0x4d0/4d1). And you should not require
> > 32 bytes for the 8259. On my boards, the super I/O control port is
> > 0x2e-ox2f.
>
> That's what I forgot... that code requests directly from ioport_resource,
> which is what gets us into trouble on vgacon. We should use request_region
> instead of request_resource. Unfortunately vgacon is initialized before the
> VMM, and since request_region calls kmalloc we die very early. :/ I didn't
> check to see if that's a problem here.

Or we need a request_resource() that doesn't try to attach to the passed
parent, but walks the list (starting from the passed parent) to hook in at the
right spot. (*)

> The problem with using request_resource(&ioport_resource) in vgacon is that
> our PHB's initialize later, try to reserve their own IO regions (eg
> 0x0->0xffffffff), and fail because vgacon already took 0x3c0 or so.

Yes, so either we have to delay the request_resource() and use the scheme above
(*), or we need a request_resource_and_take_over() call to register the PHB
resources, which reshuffles the already allocated legacy resources.

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds


** 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-06 23:15 hollis
  2001-11-07  6:37 ` Dan Malek
  2001-11-07  9:38 ` Gabriel Paubert
@ 2001-11-07 20:46 ` Val Henson
  2 siblings, 0 replies; 24+ messages in thread
From: Val Henson @ 2001-11-07 20:46 UTC (permalink / raw)
  To: hollis; +Cc: linuxppc-dev, davidm, Sven.Dickert


While y'all are mucking around in there, you might have a chance to
fix my pet peeve: if you have an OpenPIC, you don't necessarily have
an i8259 also.  Gemini doesn't have an i8259, but it either has to
ifdef out the i8259 code or include the i8259.o object.

The whole i8259/OpenPIC interface could use some redesign, actually.
But it works, hey...

-VAL

On Tue, Nov 06, 2001 at 05:15:59PM -0600, hollis@austin.ibm.com wrote:
> 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

> ===== 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);
>  }


** 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 17:06   ` hollis
  2001-11-07 17:17     ` Geert Uytterhoeven
@ 2001-11-07 21:17     ` Gabriel Paubert
  2001-11-07 22:01       ` hollis
  1 sibling, 1 reply; 24+ messages in thread
From: Gabriel Paubert @ 2001-11-07 21:17 UTC (permalink / raw)
  To: hollis; +Cc: linuxppc-dev, davidm, Sven.Dickert


On Wed, 7 Nov 2001 hollis@austin.ibm.com wrote:

> I laid out what I saw in
> http://lists.linuxppc.org/linuxppc-workstation/200109/msg00030.html . I could
> be wrong, but our code doesn't seem to match the datasheet very well.

Have you ever see code matching the datasheet, or a chip matching the
datasheet for that matter ? :-)

Oh, and I don't think that there is any integrated 8259 in a chipset which
completely follows the implementation of the discrete part.

>
> I think fixing what we have is the least intrusive, but I tried and failed, so
> it was nice to write code that actually worked. :) I guess the PCI intack
> stuff is too varied to be useful though...
>
> 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.

> > Good, but you should add resource request for the edge/level control
> > register while you are at it (0x4d0/4d1). And you should not require
> > 32 bytes for the 8259. On my boards, the super I/O control port is
> > 0x2e-ox2f.
>
> That's what I forgot... that code requests directly from ioport_resource,
> which is what gets us into trouble on vgacon. We should use request_region
> instead of request_resource. Unfortunately vgacon is initialized before the
> VMM, and since request_region calls kmalloc we die very early. :/ I didn't
> check to see if that's a problem here.
>
> The problem with using request_resource(&ioport_resource) in vgacon is that
> our PHB's initialize later, try to reserve their own IO regions (eg
> 0x0->0xffffffff), and fail because vgacon already took 0x3c0 or so.

I'd say that the problem is that the PHB is initialized too late while it
should be one of the first things to be initialized.

I know that there are interactions with debugging/early console. I did
something like this for my MVME boards, switching the host bridge from
PreP to CHRP mappings. There is a transient period in which any kind of
debugging is impossible: you don't have any I/O accessible. But in any
case this kind of remapping is much better done early, actually before
giving control to the kernel in my bootloader...

	Gabriel.


** 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 21:13 Michael Sokolov
@ 2001-11-07 21:22 ` Geert Uytterhoeven
  0 siblings, 0 replies; 24+ messages in thread
From: Geert Uytterhoeven @ 2001-11-07 21:22 UTC (permalink / raw)
  To: Michael Sokolov
  Cc: Linux/PPC Development, Sven.Dickert, davidm, hollis,
	Gabriel Paubert


On Wed, 7 Nov 2001, Michael Sokolov wrote:
> 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.

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

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

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.

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds


** 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 RFC: i8259.c cleanup Michael Sokolov
@ 2001-11-07 21:26 ` Geert Uytterhoeven
  0 siblings, 0 replies; 24+ messages in thread
From: Geert Uytterhoeven @ 2001-11-07 21:26 UTC (permalink / raw)
  To: Michael Sokolov
  Cc: Linux/PPC Development, Sven.Dickert, davidm, hollis,
	Gabriel Paubert


On Wed, 7 Nov 2001, Michael Sokolov wrote:
> 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.

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

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds


** 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:17     ` Gabriel Paubert
@ 2001-11-07 22:01       ` hollis
  0 siblings, 0 replies; 24+ messages in thread
From: hollis @ 2001-11-07 22:01 UTC (permalink / raw)
  To: Gabriel Paubert; +Cc: linuxppc-dev, davidm, Sven.Dickert


On Wed, Nov 07, 2001 at 10:17:05PM +0100, Gabriel Paubert wrote:
>
> On Wed, 7 Nov 2001 hollis@austin.ibm.com wrote:
>
> > I laid out what I saw in
> > http://lists.linuxppc.org/linuxppc-workstation/200109/msg00030.html . I
> > could be wrong, but our code doesn't seem to match the datasheet very
> > well.
>
> Have you ever see code matching the datasheet, or a chip matching the
> datasheet for that matter ? :-)

To be honest I think I was wrong. Reading the 8259 datasheet yet again has
enlightened me as the the exact nature of "INTA" and polling. What I thought
were ISR/IRR reads were actually INTA's (returning the ISR in the process).

So the stock code is definately funny (there's no need at all to explicitly
switch between the ISR and IRR), but correct AFAICS. Which is unfortunate for
me... :/

I've changed it a little to avoid this ISR selection and it's more robust (I
can boot), but still falls over pretty easily. All routines are spin_lock()
protected though; I'm not sure what sort of timing problem I might look for.

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

I'm worried this is my problem. I'm looking at the "Poll Command" section of
the 8259 docs, and i8259.c seems completely legit...

-Hollis

** 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-07 21:22 RFC: i8259.c cleanup Michael Sokolov
2001-11-07 21:26 ` Geert Uytterhoeven
  -- strict thread matches above, loose matches on Subject: below --
2001-11-07 23:04 Michael Sokolov
2001-11-07 23:00 Michael Sokolov
2001-11-07 22:55 Michael Sokolov
2001-11-07 22:43 Michael Sokolov
2001-11-07 21:13 Michael Sokolov
2001-11-07 21:22 ` Geert Uytterhoeven
2001-11-07 20:31 Michael Sokolov
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-06 23:15 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

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