UIC fixes
cover UIC fixes
This series fixes several small problems with the driver for the UIC interrupt controller found in PowerPC 4xx embedded CPUs.
Signature · 4 lines
David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson
PATCH 1/3 Fix setting of irq trigger type in UIC driver
The UIC (interrupt controller in 4xx embedded CPUs) driver currently missets the IRQ_lEVEL flag in desc->status, due to a thinko. This patch fixes the bug.
Currently this is only a cosmetic problem (affects the output in /proc/interrupts), however subsequent patches will use the IRQ_LEVEL flag to affect flow handling.
--- arch/powerpc/sysdev/uic.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
arch/powerpc/sysdev/uic.c +1 -1
@@ -142,7 +142,7 @@ static int uic_set_irq_type(unsigned int desc->status &= ~(IRQ_TYPE_SENSE_MASK |IRQ_LEVEL ); desc->status |= flow_type &IRQ_TYPE_SENSE_MASK ; -if (trigger) +if (!trigger) desc->status |=IRQ_LEVEL ;spin_unlock_irqrestore (&uic->lock, flags);
Re: [PATCH 1/3] Fix setting of irq trigger type in UIC driver
Quoted 10 lines · On Tue, 14 Aug 2007 13:52:42 +1000 (EST) David Gibson…
The UIC (interrupt controller in 4xx embedded CPUs) driver currently missets the
IRQ_lEVELflag indesc->status, due to a thinko. This patch fixes the bug.Currently this is only a cosmetic problem (affects the output in
/proc/interrupts), however subsequent patches will use theIRQ_LEVELflag to affect flow handling.Signed-off-by Valentine Barshak <[email protected]>Signed-off-by David Gibson <[email protected]>
josh
PATCH 2/3 Fix irq flow handler for 4xx UIC
In reply to David GibsonAt present the driver for the UIC (the embedded interrupt controller in 4xx chips) uses the handle_level_irq() flow handler. It turns out this does not correctly handle level triggered interrupts on the UIC.
Specifically, acknowledging an irq on the UIC (i.e. clearing the relevant bit in UIC_SR) will have no effect for a level interrupt which is still asserted by the external device, even if the irq is already masked. Therefore, unlike handle_level_irq() we must ack the interrupt after invoking the ISR (which should cause the device to stop asserting the irq) instead of acking it when we mask it, before the ISR.
This patch implements this change, in a new handle_uic_irq(), a customised irq flow handler for the UIC. For edge triggered interrupts, handle_uic_irq() still uses the old flow - we must ack edge triggered interrupt before the ISR not after, or we could miss a second event which occurred between invoking the ISR and acking the irq.
--- arch/powerpc/sysdev/uic.c | 61 +++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 60 insertions(+), 1 deletion(-)
arch/powerpc/sysdev/uic.c +60 -1
@@ -24,6 +24,7 @@#include <linux/spinlock.h> #include <linux/irq.h> #include <linux/interrupt.h> +#include <linux/kernel_stat.h> #include <asm/irq.h> #include <asm/io.h> #include <asm/prom.h>
@@ -159,6 +160,64 @@ static struct irq_chip uic_irq_chip = { .set_type = uic_set_irq_type, }; +/** +* handle_uic_irq - irq flow handler for UIC +* @irq: the interrupt number +* @desc: the interrupt description structure for this irq +* +* This is modified version of the generic handle_level_irq() suitable +* for the UIC. On the UIC, acking (i.e. clearing the SR bit) a level +* irq will have no effect if the interrupt is still asserted by the +* device, even if the interrupt is already masked. Therefore, unlike +* the standard handle_level_irq(), we must ack the interrupt *after* +* invoking the ISR (which should have de-asserted the interrupt in +* the external source). For edge interrupts we ack at the beginning +* instead of the end, to keep the window in which we can miss an +* interrupt as small as possible. +*/ +void fastcall handle_uic_irq(unsigned int irq, struct irq_desc *desc) +{ +unsigned int cpu =smp_processor_id (); +struct irqaction *action; +irqreturn_t action_ret; + +spin_lock (&desc->lock); +if (desc->status &IRQ_LEVEL ) + desc->chip->mask (irq); +else + desc->chip->mask_ack (irq); + +if (unlikely (desc->status &IRQ_INPROGRESS )) + goto out_unlock; + desc->status &= ~(IRQ_REPLAY |IRQ_WAITING ); +kstat_cpu (cpu).irqs[irq]++; + +/* +* If its disabled or no action available +* keep it masked and get out of here +*/ + action = desc->action; +if (unlikely (!action || (desc->status &IRQ_DISABLED ))) { + desc->status |=IRQ_PENDING ; + goto out_unlock; + } + + desc->status |=IRQ_INPROGRESS ; + desc->status &= ~IRQ_PENDING ; +spin_unlock (&desc->lock); + + action_ret =handle_IRQ_event (irq, action); + +spin_lock (&desc->lock); + desc->status &= ~IRQ_INPROGRESS ; +if (desc->status &IRQ_LEVEL ) + desc->chip->ack (irq); +if (!(desc->status &IRQ_DISABLED ) && desc->chip->unmask) + desc->chip->unmask (irq); +out_unlock: +spin_unlock (&desc->lock); +} +static int uic_host_match (struct irq_host *h,struct device_node *node) {struct uic *uic = h->host_data;
@@ -173,7 +232,7 @@ static int uic_host_map(struct irq_hostset_irq_chip_data (virq, uic);/* Despite the name, handle_level_irq() works for both level * and edge irqs on UIC. FIXME: check this is correct */ -set_irq_chip_and_handler (virq, &uic_irq_chip, handle_level_irq); +set_irq_chip_and_handler (virq, &uic_irq_chip, handle_uic_irq);/* Set default irq type */ set_irq_type (virq,IRQ_TYPE_NONE );
Re: [PATCH 2/3] Fix irq flow handler for 4xx UIC
Quoted 12 lines · On Tue, 14 Aug 2007 13:52:42 +1000 (EST) David Gibson…
At present the driver for the UIC (the embedded interrupt controller in 4xx chips) uses the
handle_level_irq()flow handler. It turns out this does not correctly handle level triggered interrupts on the UIC.Specifically, acknowledging an irq on the UIC (i.e. clearing the relevant bit in
UIC_SR) will have no effect for a level interrupt which is still asserted by the external device, even if the irq is already masked. Therefore, unlikehandle_level_irq()we must ack the interrupt after invoking the ISR (which should cause the device to stop asserting the irq) instead of acking it when we mask it, before the ISR.
Quoted 8 lines
This patch implements this change, in a new
handle_uic_irq(), a customised irq flow handler for the UIC. For edge triggered interrupts,handle_uic_irq()still uses the old flow - we must ack edge triggered interrupt before the ISR not after, or we could miss a second event which occurred between invoking the ISR and acking the irq.Signed-off-by David Gibson <[email protected]>
josh
PATCH 3/3 Improve robustness of the UIC cascade handler
In reply to David GibsonAt present the cascade interrupt handler for the UIC (interrupt controller on 4xx embedded chips) will misbehave badly if it is called spuriously - that is if the handler is invoked when no interrupts are asserted in the child UIC.
Although spurious interrupts shouldn't happen, it's good to behave robustly if they do. This patch does so by checking for and ignoring spurious interrupts.
--- arch/powerpc/sysdev/uic.c | 3 +++ 1 file changed, 3 insertions(+)
arch/powerpc/sysdev/uic.c +3 -0
@@ -266,6 +266,9 @@ irqreturn_t uic_cascade(int virq, void *int subvirq; msr =mfdcr (uic->dcrbase +UIC_MSR ); +if (!msr)/* spurious interrupt */ +return IRQ_HANDLED ; + src =32 -ffs (msr); subvirq =irq_linear_revmap (uic->irqhost, src);
Re: [PATCH 3/3] Improve robustness of the UIC cascade handler
Quoted 21 lines · On Tue, 14 Aug 2007 13:52:42 +1000 (EST) David Gibson…
At present the cascade interrupt handler for the UIC (interrupt controller on 4xx embedded chips) will misbehave badly if it is called spuriously - that is if the handler is invoked when no interrupts are asserted in the child UIC.
Although spurious interrupts shouldn't happen, it's good to behave robustly if they do. This patch does so by checking for and ignoring spurious interrupts.
Signed-off-by Valentine Barshak <[email protected]>Signed-off-by David Gibson <[email protected]>--- arch/powerpc/sysdev/uic.c | 3 +++ 1 file changed, 3 insertions(+)Index:
working-2.6/arch/powerpc/sysdev/uic.c
===================================================================
arch/powerpc/sysdev/uic.c irqreturn_t uic_cascade(int virq, void * · line 270
Quoted 5 lines
arch/powerpc/sysdev/uic.c +2
-0@@ -266,6 +266,9 @@ irqreturn_t uic_cascade(int virq, void *int subvirq; msr =mfdcr (uic->dcrbase +UIC_MSR ); +if (!msr)/* spurious interrupt */ +return IRQ_HANDLED ;
Hm. Is there was a way we could have this case increment ppc_spurious_interrupts so that the BAD entry in /proc/interrupts would be updated with these? Not a huge deal, but it might be nice to have. Otherwise the patch looks fine.
josh