Skip to contentSkip to conversation

85XX: Allow 8259 cascade to share an MPIC interrupt line.

RFC Superseded Randy Vinson 3 participants 8 messages Jun 7, 2007

Subsystems (2)

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore
#0 lore raw

RFC 1/1 85XX: Allow 8259 cascade to share an MPIC interrupt line.

The Freescale MPC8555CDS and MPC8548CDS reference hardware has a legacy 8259 interrupt controller pair contained within a VIA VT82C686B Southbridge on the main carrier board. The processor complex plugs into the carrier card using a PCI slot which limits the available interrupts to the INTA-INTD PCI interrupts. The output of the 8259 cascade pair is routed through a gate array and connected to the PCI INTA interrupt line. The normal interrupt chaining hook (set_irq_chained_handler) does not allow sharing of the chained interrupt which prevents the use of PCI INTA by PCI devices. This patch allows the 8259 cascade pair to share their interrupt line with PCI devices.

---
Note that there may very well be a better way of accomplishing this. If someone has a better alternative, I'm open to it. This was just the simplest way I could get this to work.

Also, the addition of the .end routine for the MPIC is not strictly necessary for this patch. It's there so this code will run from within the threaded interrupt context used by the Real Time patch.

 arch/powerpc/platforms/85xx/mpc85xx_cds.c |   32 +++++++++++++++++++++++++---
 arch/powerpc/sysdev/mpic.c                |    1 +
 2 files changed, 29 insertions(+), 4 deletions(-)
arch/powerpc/platforms/85xx/mpc85xx_cds.c +28 -4
index 1490eb3..431aaa2 100644
@@ -127,16 +127,30 @@ static void __init mpc85xx_cds_pcibios_fixup(void)
 }
 
 #ifdef CONFIG_PPC_I8259
-#warning The i8259 PIC support is currently broken
-static void mpc85xx_8259_cascade(unsigned int irq, struct irq_desc *desc)
+static void mpc85xx_8259_cascade_handler(unsigned int irq,
+					 struct irq_desc *desc)
 {
 	unsigned int cascade_irq = i8259_irq();
 
 	if (cascade_irq != NO_IRQ)
+		/* handle an interrupt from the 8259 */
 		generic_handle_irq(cascade_irq);
 
-	desc->chip->eoi(irq);
+	/* check for any interrupts from the shared IRQ line */
+	handle_fasteoi_irq(irq, desc);
 }
+
+static irqreturn_t mpc85xx_8259_cascade_action(int irq, void *dev_id)
+{
+	return IRQ_HANDLED;
+}
+
+static struct irqaction mpc85xxcds_8259_irqaction = {
+	.handler = mpc85xx_8259_cascade_action,
+	.flags = IRQF_SHARED,
+	.mask = CPU_MASK_NONE,
+	.name = "8259 cascade",
+};
 #endif /* PPC_I8259 */
 #endif /* CONFIG_PCI */
 
@@ -216,7 +230,17 @@ static void __init mpc85xx_cds_pic_init(void)
 	i8259_init(cascade_node, 0);
 	of_node_put(cascade_node);
 
-	set_irq_chained_handler(cascade_irq, mpc85xx_8259_cascade);
+	/*
+	 *  Hook the interrupt to make sure desc->action is never NULL.
+	 *  This is required to ensure that the interrupt does not get
+	 *  disabled when the last user of the shared IRQ line frees their
+	 *  interrupt.
+	 */
+	if (setup_irq(cascade_irq, &mpc85xxcds_8259_irqaction))
+		printk(KERN_ERR "Failed to setup cascade interrupt\n");
+	else
+		/* Success. Connect our low-level cascade handler. */
+		set_irq_handler(cascade_irq, mpc85xx_8259_cascade_handler);
 #endif /* CONFIG_PPC_I8259 */
 }
 
arch/powerpc/sysdev/mpic.c +1 -0
index 75aad38..14e3d1d 100644
@@ -836,6 +836,7 @@ static struct irq_chip mpic_irq_chip = {
 	.mask		= mpic_mask_irq,
 	.unmask		= mpic_unmask_irq,
 	.eoi		= mpic_end_irq,
+	.end		= mpic_unmask_irq,
 	.set_type	= mpic_set_irq_type,
 };
 
Signature · 1 line
1.5.0.GIT
#1 lore raw
Quoted 12 lines · Randy Vinson

The Freescale MPC8555CDS and MPC8548CDS reference hardware has a legacy 8259 interrupt controller pair contained within a VIA VT82C686B Southbridge on the main carrier board. The processor complex plugs into the carrier card using a PCI slot which limits the available interrupts to the INTA-INTD PCI interrupts. The output of the 8259 cascade pair is routed through a gate array and connected to the PCI INTA interrupt line. The normal interrupt chaining hook (set_irq_chained_handler) does not allow sharing of the chained interrupt which prevents the use of PCI INTA by PCI devices. This patch allows the 8259 cascade pair to share their interrupt line with PCI devices.

Quoted 8 lines

---
Note that there may very well be a better way of accomplishing this. If someone has a better alternative, I'm open to it. This was just the simplest way I could get this to work.

Also, the addition of the .end routine for the MPIC is not strictly necessary for this patch. It's there so this code will run from within the threaded interrupt context used by the Real Time patch.

Hmm. I feel that at least the last paragraph here needs to be above the --- line for future patch readers. Otherwise it's relationship to the rest of the patch details will be intermixed and lost.

Thanks,
jdl

#2 lore raw
Quoted 1 line · Jon Loeliger

On Wed, 2007-06-06 at 19:47, Randy Vinson wrote:

[snippage]

Quoted 11 lines
Quoted 9 lines

---
Note that there may very well be a better way of accomplishing this. If someone has a better alternative, I'm open to it. This was just the simplest way I could get this to work.

Also, the addition of the .end routine for the MPIC is not strictly necessary for this patch. It's there so this code will run from within the threaded interrupt context used by the Real Time patch.

Quoted 4 lines

Hmm. I feel that at least the last paragraph here needs to be above the --- line for future patch readers. Otherwise it's relationship to the rest of the patch details will be intermixed and lost.

OK. That's a good idea. I'll update that for the next round.

Thx,
Randy V.

#3 lore raw
In reply to Randy Vinson

Hello.

Quoted 10 lines · Randy Vinson

The Freescale MPC8555CDS and MPC8548CDS reference hardware has a legacy 8259 interrupt controller pair contained within a VIA VT82C686B Southbridge on the main carrier board. The processor complex plugs into the carrier card using a PCI slot which limits the available interrupts to the INTA-INTD PCI interrupts. The output of the 8259 cascade pair is routed through a gate array and connected to the PCI INTA interrupt line. The normal interrupt chaining hook (set_irq_chained_handler) does not allow sharing of the chained interrupt which prevents the use of PCI INTA by PCI devices. This patch allows the 8259 cascade pair to share their interrupt line with PCI devices.

Hmm, I see you've come up with an interesting solution. :-)

Quoted 9 lines

---
Note that there may very well be a better way of accomplishing this. If someone has a better alternative, I'm open to it. This was just the simplest way I could get this to work.

Also, the addition of the .end routine for the MPIC is not strictly necessary for this patch. It's there so this code will run from within the threaded interrupt context used by the Real Time

Hmmm, why would you need that? Where RT is different from the vanilla? :-O

Quoted 22 lines
 arch/powerpc/platforms/85xx/mpc85xx_cds.c |   32 +++++++++++++++++++++++++---
 arch/powerpc/sysdev/mpic.c                |    1 +
 2 files changed, 29 insertions(+), 4 deletions(-)
arch/powerpc/platforms/85xx/mpc85xx_cds.c +3 -2
index 1490eb3..431aaa2 100644
@@ -127,16 +127,30 @@ static void __init mpc85xx_cds_pcibios_fixup(void)
 }
 
 #ifdef CONFIG_PPC_I8259
-#warning The i8259 PIC support is currently broken
-static void mpc85xx_8259_cascade(unsigned int irq, struct irq_desc *desc)
+static void mpc85xx_8259_cascade_handler(unsigned int irq,
+					 struct irq_desc *desc)
 {
 	unsigned int cascade_irq = i8259_irq();
 
 	if (cascade_irq != NO_IRQ)
+		/* handle an interrupt from the 8259 */
 		generic_handle_irq(cascade_irq);

arch/powerpc/platforms/85xx/mpc85xx_cds.c static void __init mpc85xx_cds_pcibios_fixup(void) · line 146

Quoted 10 lines
arch/powerpc/platforms/85xx/mpc85xx_cds.c +7 -1
 
-	desc->chip->eoi(irq);
+	/* check for any interrupts from the shared IRQ line */
+	handle_fasteoi_irq(irq, desc);
 }
+
+static irqreturn_t mpc85xx_8259_cascade_action(int irq, void *dev_id)
+{
+	return IRQ_HANDLED;
+}

Well, mpc85xx_8259_intr() would probably be more in line with the code
elsewhere... and you could keep the mpc85xx_8259_cascade() name then.

Quoted 33 lines
arch/powerpc/platforms/85xx/mpc85xx_cds.c +17 -1
+static struct irqaction mpc85xxcds_8259_irqaction = {
+	.handler = mpc85xx_8259_cascade_action,
+	.flags = IRQF_SHARED,
+	.mask = CPU_MASK_NONE,
+	.name = "8259 cascade",
+};
 #endif /* PPC_I8259 */
 #endif /* CONFIG_PCI */
 
@@ -216,7 +230,17 @@ static void __init mpc85xx_cds_pic_init(void)
 	i8259_init(cascade_node, 0);
 	of_node_put(cascade_node);
 
-	set_irq_chained_handler(cascade_irq, mpc85xx_8259_cascade);
+	/*
+	 *  Hook the interrupt to make sure desc->action is never NULL.
+	 *  This is required to ensure that the interrupt does not get
+	 *  disabled when the last user of the shared IRQ line frees their
+	 *  interrupt.
+	 */
+	if (setup_irq(cascade_irq, &mpc85xxcds_8259_irqaction))
+		printk(KERN_ERR "Failed to setup cascade interrupt\n");
+	else
+		/* Success. Connect our low-level cascade handler. */
+		set_irq_handler(cascade_irq, mpc85xx_8259_cascade_handler);
 #endif /* CONFIG_PPC_I8259 */
 }
 
arch/powerpc/sysdev/mpic.c +0 -0
index 75aad38..14e3d1d 100644

arch/powerpc/sysdev/mpic.c static struct irq_chip mpic_irq_chip = { · line 841

Quoted 6 lines
arch/powerpc/sysdev/mpic.c +1 -0
@@ -836,6 +836,7 @@ static struct irq_chip mpic_irq_chip = {
 	.mask		= mpic_mask_irq,
 	.unmask		= mpic_unmask_irq,
 	.eoi		= mpic_end_irq,
+	.end		= mpic_unmask_irq,
 	.set_type	= mpic_set_irq_type,
 };

WBR, Sergei

#4 lore raw
Quoted 1 line · Sergei Shtylyov

Hello.

Hi.

Quoted 15 lines
Quoted 11 lines · Randy Vinson

The Freescale MPC8555CDS and MPC8548CDS reference hardware has a legacy 8259 interrupt controller pair contained within a VIA VT82C686B Southbridge
on the main carrier board. The processor complex plugs into the carrier card using a PCI slot which limits the available interrupts to the INTA-INTD PCI interrupts. The output of the 8259 cascade pair is routed through a gate array and connected to the PCI INTA interrupt line. The normal interrupt chaining hook (set_irq_chained_handler) does not allow sharing of the chained interrupt which prevents the use of PCI INTA by PCI devices. This patch allows the 8259 cascade pair to share their interrupt line with PCI devices.

Quoted 1 line

Hmm, I see you've come up with an interesting solution. :-)

Well, I'm not sure how interesting it is, but it works :)

Quoted 9 lines
Quoted 9 lines

---
Note that there may very well be a better way of accomplishing this. If someone
has a better alternative, I'm open to it. This was just the simplest way I could
get this to work.

Quoted 8 lines
Quoted 5 lines

Also, the addition of the .end routine for the MPIC is not strictly necessary for
this patch. It's there so this code will run from within the threaded interrupt
context used by the Real Time

Hmmm, why would you need that? Where RT is different from the
vanilla? :-O

The threaded interrupt system has threaded routines for the 4 standard interrupts types used by the generic IRQ system (fasteoi, edge, level and simple), plus a handler for non-generic interrupts (the ones that still use __do_IRQ.) When interrupts are being threaded and a non-cascaded MPIC interrupt occurs, desc->handler normally points to handle_fasteoi_irq which masks the interrupt source, schedules the corresponding IRQ thread, issues an EOI and returns to the interrupted context. At a later time, the scheduler dispatches the IRQ thread which calls a threaded version of the fasteoi handler. The threaded fasteoi routine processes the action chain and calls .unmask to re-enable the interrupt before returning to the calling thread. Once the interrupt has been unmasked, the entire process is free to repeat as needed.

After processing a possible 8259 interrupt, the 8259 cascade handler calls handle_fasteoi_irq to perform the actions noted above. However, with the 8259 cascade handler hooked to the interrupt, the threaded IRQ handler doesn't recognize it as one of the 4 standard generic IRQ types and calls the threaded version of do_irq instead of the threaded fasteoi handler. Once the threaded do_irq handler completes the action list, it uses the .end routine to restart the interrupt flow. Pointing .end to the standard MPIC unmask routine allows it to balance the interrupt mask operation performed by handle_fasteoi_irq before it scheduled the IRQ thread.

Quoted 27 lines
Quoted 27 lines
 arch/powerpc/platforms/85xx/mpc85xx_cds.c |   32
+++++++++++++++++++++++++---
 arch/powerpc/sysdev/mpic.c                |    1 +
 2 files changed, 29 insertions(+), 4 deletions(-)
arch/powerpc/platforms/85xx/mpc85xx_cds.c +3 -2
index 1490eb3..431aaa2 100644
@@ -127,16 +127,30 @@ static void __init mpc85xx_cds_pcibios_fixup(void)
 }
 
 #ifdef CONFIG_PPC_I8259
-#warning The i8259 PIC support is currently broken
-static void mpc85xx_8259_cascade(unsigned int irq, struct irq_desc
 *desc)
+static void mpc85xx_8259_cascade_handler(unsigned int irq,
+                     struct irq_desc *desc)
 {
     unsigned int cascade_irq = i8259_irq();
 
     if (cascade_irq != NO_IRQ)
+        /* handle an interrupt from the 8259 */
         generic_handle_irq(cascade_irq);
 
Quoted 12 lines

arch/powerpc/platforms/85xx/mpc85xx_cds.c static void __init mpc85xx_cds_pcibios_fixup(void) · line 146

Quoted 9 lines
arch/powerpc/platforms/85xx/mpc85xx_cds.c +7 -1
-    desc->chip->eoi(irq);
+    /* check for any interrupts from the shared IRQ line */
+    handle_fasteoi_irq(irq, desc);
 }
+
+static irqreturn_t mpc85xx_8259_cascade_action(int irq, void *dev_id)
+{
+    return IRQ_HANDLED;
+}

Well, mpc85xx_8259_intr() would probably be more in line with the
code elsewhere... and you could keep the mpc85xx_8259_cascade() name then.

I chose the names to show that the two routines run at different levels within the interrupt subsystem. The cascade handler always runs in interrupt context while the cascade action can run in interrupt context or in threaded context. While I was researching this, I kept getting confused between the desc->handler and action->handler fields so I named them different to make it easier for me to keep the differences clear in my head. I can change the names it if you think it's important.

Randy V.

#5 lore raw
Quoted 16 lines · Randy Vinson
Quoted 7 lines
Quoted 6 lines

---
Note that there may very well be a better way of accomplishing this. If someone
has a better alternative, I'm open to it. This was just the simplest way I could
get this to work.

Quoted 8 lines
Quoted 5 lines

Also, the addition of the .end routine for the MPIC is not strictly necessary for
this patch. It's there so this code will run from within the threaded interrupt
context used by the Real Time

Hmmm, why would you need that? Where RT is different from the
vanilla? :-O

Quoted 12 lines

The threaded interrupt system has threaded routines for the 4 standard interrupts types used by the generic IRQ system (fasteoi, edge, level and simple), plus a handler for non-generic interrupts (the ones that still use __do_IRQ.) When interrupts are being threaded and a non-cascaded MPIC interrupt occurs, desc->handler normally points to handle_fasteoi_irq which masks the interrupt source, schedules the corresponding IRQ thread, issues an EOI and returns to the interrupted context. At a later time, the scheduler dispatches the IRQ thread which calls a threaded version of the fasteoi handler. The threaded fasteoi routine processes the action chain and calls .unmask to re-enable the interrupt before returning to the calling thread. Once the interrupt has been unmasked, the entire process is free to repeat as needed.

Oh, you could have really skipped the basics. ;-)

Quoted 6 lines

After processing a possible 8259 interrupt, the 8259 cascade handler calls handle_fasteoi_irq to perform the actions noted above. However, with the 8259 cascade handler hooked to the interrupt, the threaded IRQ handler doesn't recognize it as one of the 4 standard generic IRQ types and calls the threaded version of do_irq instead of the threaded fasteoi handler.

Ah, that's it!

Quoted 5 lines

Once the threaded do_irq handler completes the action list, it uses the .end routine to restart the interrupt flow. Pointing .end to the standard MPIC unmask routine allows it to balance the interrupt mask operation performed by handle_fasteoi_irq before it scheduled the IRQ thread.

Well, but when MPIC's eoi() method is called then? :-O

Quoted 39 lines
Quoted 26 lines
Quoted 26 lines

arch/powerpc/platforms/85xx/mpc85xx_cds.c | 32
+++++++++++++++++++++++++---
arch/powerpc/sysdev/mpic.c | 1 +

2 files changed, 29 insertions(+), 4 deletions(-)
arch/powerpc/platforms/85xx/mpc85xx_cds.c +3 -2
index 1490eb3..431aaa2 100644
@@ -127,16 +127,30 @@ static void __init mpc85xx_cds_pcibios_fixup(void)
 }
 
 #ifdef CONFIG_PPC_I8259
-#warning The i8259 PIC support is currently broken
-static void mpc85xx_8259_cascade(unsigned int irq, struct irq_desc
 *desc)
+static void mpc85xx_8259_cascade_handler(unsigned int irq,
+                     struct irq_desc *desc)
 {
    unsigned int cascade_irq = i8259_irq();
 
    if (cascade_irq != NO_IRQ)
+        /* handle an interrupt from the 8259 */
        generic_handle_irq(cascade_irq);
 
Quoted 12 lines

arch/powerpc/platforms/85xx/mpc85xx_cds.c static void __init mpc85xx_cds_pcibios_fixup(void) · line 146

Quoted 9 lines
arch/powerpc/platforms/85xx/mpc85xx_cds.c +7 -1
-    desc->chip->eoi(irq);
+    /* check for any interrupts from the shared IRQ line */
+    handle_fasteoi_irq(irq, desc);
 }
+
+static irqreturn_t mpc85xx_8259_cascade_action(int irq, void *dev_id)
+{
+    return IRQ_HANDLED;
+}

Well, mpc85xx_8259_intr() would probably be more in line with the
code elsewhere... and you could keep the mpc85xx_8259_cascade() name then.

Quoted 7 lines

I chose the names to show that the two routines run at different levels within the interrupt subsystem. The cascade handler always runs in interrupt context while the cascade action can run in interrupt context or in threaded context. While I was researching this, I kept getting confused between the desc->handler and action->handler fields so I named them different to make it easier for me to keep the differences clear in my head. I can change the names it if you think it's important.

I don't insist. :-)

Quoted 1 line

Randy V.

WBR, Sergei

#6 lore raw

Sergei Shtylyov wrote:
[snippage]

Quoted 13 lines
Quoted 4 lines

The threaded interrupt system has threaded routines for the 4 standard interrupts types used by the generic IRQ system (fasteoi, edge, level and simple), plus a handler for non-generic interrupts (the ones that still use __do_IRQ.) When interrupts are being threaded and a

Quoted 8 lines

non-cascaded MPIC interrupt occurs, desc->handler normally points to handle_fasteoi_irq which masks the interrupt source, schedules the corresponding IRQ thread, issues an EOI and returns to the interrupted context. At a later time, the scheduler dispatches the IRQ thread which calls a threaded version of the fasteoi handler. The threaded fasteoi routine processes the action chain and calls .unmask to re-enable the interrupt before returning to the calling thread. Once the interrupt has been unmasked, the entire process is free to repeat as needed.

Quoted 1 line

Oh, you could have really skipped the basics. ;-)

Maybe not. See below.

Quoted 10 lines
Quoted 7 lines

After processing a possible 8259 interrupt, the 8259 cascade handler calls handle_fasteoi_irq to perform the actions noted above. However, with the 8259 cascade handler hooked to the interrupt, the threaded IRQ handler doesn't recognize it as one of the 4 standard generic IRQ types and calls the threaded version of do_irq instead of the threaded fasteoi handler.

Ah, that's it!

Quoted 7 lines
Quoted 5 lines

Once the threaded do_irq handler completes the action list, it uses the .end routine to restart the interrupt flow. Pointing .end to the standard MPIC unmask routine allows it to balance the interrupt mask operation performed by handle_fasteoi_irq before it scheduled the IRQ thread.

Well, but when MPIC's eoi() method is called then? :-O

It's called from handle_fasteoi_irq as I described in the "basics" ;)

Randy V.

#7 lore raw
Quoted 15 lines · Randy Vinson
Quoted 14 lines
Quoted 6 lines

After processing a possible 8259 interrupt, the 8259 cascade handler calls handle_fasteoi_irq to perform the actions noted above. However, with the 8259 cascade handler hooked to the interrupt, the threaded IRQ handler doesn't recognize it as one of the 4 standard generic IRQ types and calls the threaded version of do_irq instead of the threaded fasteoi handler.

Ah, that's it!

Quoted 5 lines

Once the threaded do_irq handler completes the action list, it uses the .end routine to restart the interrupt flow. Pointing .end to the standard MPIC unmask routine allows it to balance the interrupt mask operation performed by handle_fasteoi_irq before it scheduled the IRQ thread.

Quoted 3 lines
Quoted 1 line

Well, but when MPIC's eoi() method is called then? :-O

It's called from handle_fasteoi_irq as I described in the "basics" ;)

Indeed, I somehow thought eoi() is out of the threaded exit path of
handle_fasteoi_irq() -- but it hasn't always been that way! So, it's probably a painful memories of my past quarrel with Ingo over fasteoi breakage that played a trick with me here. ;-)

Quoted 1 line

Randy V.

WBR, Sergei