linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] PowerPC: 4xx uic updates
@ 2007-11-13 20:15 Valentine Barshak
  2007-11-13 20:25 ` [PATCH 1/2] PowerPC: 4xx uic: add mask_ack callback Valentine Barshak
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Valentine Barshak @ 2007-11-13 20:15 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: dwg

These patches update 4xx uic code. The first one
fixes a minor issue with edge-triggered interrupts,
while the second one makes it use generic level and edge irq
handlers. I've added irq ack'ing to the unmask callback for
level-triggered interrupts, because to de-assert them we have
to do 2 things is the exact order as below:
1. de-assert the external source in the ISR.
2. ack the IRQ on the UIC.
So, ack'ing level interrupts before unmasking them makes possible
to use generic level irq handler and it doesn't hurt, cause
we can never miss a level-triggered interrupt. It always stays
asserted untill the external source is removed and ack'ed on UIC.

These have been tested on Sequoia PowerPC 440EPx board.
Thanks,
Valentine.

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

* [PATCH 1/2] PowerPC: 4xx uic: add mask_ack callback
  2007-11-13 20:15 [PATCH 0/2] PowerPC: 4xx uic updates Valentine Barshak
@ 2007-11-13 20:25 ` Valentine Barshak
  2007-11-14  0:57   ` David Gibson
  2007-11-13 20:27 ` [PATCH 2/2] PowerPC: make 4xx uic use generic edge and level irq handlers Valentine Barshak
  2007-11-14  2:05 ` [PATCH 0/2] PowerPC: 4xx uic updates Josh Boyer
  2 siblings, 1 reply; 11+ messages in thread
From: Valentine Barshak @ 2007-11-13 20:25 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: dwg

This adds uic_mask_ack_irq() callback to PowerPC 4xx uic code 
to avoid kernel crash. It is used for edge-triggered interrupts
by handle_uic_irq().

Signed-off-by: Valentine Barshak <vbarshak@ru.mvista.com>
---
 arch/powerpc/sysdev/uic.c |   18 +++++++++++++++++-
 1 files changed, 17 insertions(+), 1 deletion(-)

--- linux-2.6.orig/arch/powerpc/sysdev/uic.c	2007-11-13 18:42:33.000000000 +0300
+++ linux-2.6/arch/powerpc/sysdev/uic.c	2007-11-13 22:28:01.000000000 +0300
@@ -97,6 +97,22 @@ static void uic_ack_irq(unsigned int vir
 	spin_unlock_irqrestore(&uic->lock, flags);
 }
 
+static void uic_mask_ack_irq(unsigned int virq)
+{
+	struct uic *uic = get_irq_chip_data(virq);
+	unsigned int src = uic_irq_to_hw(virq);
+	unsigned long flags;
+	u32 er, sr;
+
+	sr = 1 << (31-src);
+	spin_lock_irqsave(&uic->lock, flags);
+	er = mfdcr(uic->dcrbase + UIC_ER);
+	er &= ~sr;
+	mtdcr(uic->dcrbase + UIC_ER, er);
+	mtdcr(uic->dcrbase + UIC_SR, sr);
+	spin_unlock_irqrestore(&uic->lock, flags);
+}
+
 static int uic_set_irq_type(unsigned int virq, unsigned int flow_type)
 {
 	struct uic *uic = get_irq_chip_data(virq);
@@ -152,7 +168,7 @@ static struct irq_chip uic_irq_chip = {
 	.typename	= " UIC  ",
 	.unmask		= uic_unmask_irq,
 	.mask		= uic_mask_irq,
-/* 	.mask_ack	= uic_mask_irq_and_ack, */
+ 	.mask_ack	= uic_mask_ack_irq,
 	.ack		= uic_ack_irq,
 	.set_type	= uic_set_irq_type,
 };

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

* [PATCH 2/2] PowerPC: make 4xx uic use generic edge and level irq handlers
  2007-11-13 20:15 [PATCH 0/2] PowerPC: 4xx uic updates Valentine Barshak
  2007-11-13 20:25 ` [PATCH 1/2] PowerPC: 4xx uic: add mask_ack callback Valentine Barshak
@ 2007-11-13 20:27 ` Valentine Barshak
  2007-11-14  2:13   ` David Gibson
  2007-11-14  2:05 ` [PATCH 0/2] PowerPC: 4xx uic updates Josh Boyer
  2 siblings, 1 reply; 11+ messages in thread
From: Valentine Barshak @ 2007-11-13 20:27 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: dwg

This patch makes PowerPC 4xx UIC use generic edge and level irq handlers
instead of a custom handle_uic_irq() function. Acking a level irq on UIC
has no effect if the interrupt is still asserted by the device, even if
the interrupt is already masked. So, to really de-assert the interrupt
we need to de-assert the external source first *and* ack it on UIC then.
The handle_level_irq() function masks and ack's the interrupt with mask_ack
callback prior to calling the actual ISR and unmasks it at the end.
So, to use it with UIC level interrupts we need to ack in the unmask
callback instead, after the ISR has de-asserted the external interrupt source.
Even if we ack the interrupt that we didn't handle (unmask/ack it at
the end of the handler, while next irq is already pending) it will not
de-assert the irq, untill we de-assert its exteral source.

Signed-off-by: Valentine Barshak <vbarshak@ru.mvista.com>
---
 arch/powerpc/sysdev/uic.c |   89 ++++++++++++----------------------------------
 1 files changed, 24 insertions(+), 65 deletions(-)

--- linux-2.6/arch/powerpc/sysdev/uic.c	2007-11-13 22:28:01.000000000 +0300
+++ linux-2.6.o/arch/powerpc/sysdev/uic.c	2007-11-13 22:27:20.000000000 +0300
@@ -60,14 +60,19 @@ struct uic {
 
 static void uic_unmask_irq(unsigned int virq)
 {
+	struct irq_desc *desc = get_irq_desc(virq);
 	struct uic *uic = get_irq_chip_data(virq);
 	unsigned int src = uic_irq_to_hw(virq);
 	unsigned long flags;
-	u32 er;
+	u32 er, sr;
 
+	sr = 1 << (31-src);
 	spin_lock_irqsave(&uic->lock, flags);
+	/* ack level interrupts here */
+	if (desc->status & IRQ_LEVEL)
+		mtdcr(uic->dcrbase + UIC_SR, sr);
 	er = mfdcr(uic->dcrbase + UIC_ER);
-	er |= 1 << (31 - src);
+	er |= sr;
 	mtdcr(uic->dcrbase + UIC_ER, er);
 	spin_unlock_irqrestore(&uic->lock, flags);
 }
@@ -99,6 +104,7 @@ static void uic_ack_irq(unsigned int vir
 
 static void uic_mask_ack_irq(unsigned int virq)
 {
+	struct irq_desc *desc = get_irq_desc(virq);
 	struct uic *uic = get_irq_chip_data(virq);
 	unsigned int src = uic_irq_to_hw(virq);
 	unsigned long flags;
@@ -109,7 +115,16 @@ static void uic_mask_ack_irq(unsigned in
 	er = mfdcr(uic->dcrbase + UIC_ER);
 	er &= ~sr;
 	mtdcr(uic->dcrbase + UIC_ER, er);
-	mtdcr(uic->dcrbase + UIC_SR, sr);
+ 	/* 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
+	 * we only ack the egde interrupts here, while
+	 * level interrupts are ack'ed after the actual
+	 * isr call in the uic_unmask_irq()
+	 */
+	if (!(desc->status & IRQ_LEVEL))
+		mtdcr(uic->dcrbase + UIC_SR, sr);
 	spin_unlock_irqrestore(&uic->lock, flags);
 }
 
@@ -156,8 +171,11 @@ 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;
+		set_irq_handler(virq, handle_level_irq);
+	} else
+		set_irq_handler(virq, handle_edge_irq);
 
 	spin_unlock_irqrestore(&uic->lock, flags);
 
@@ -173,73 +191,14 @@ 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_map(struct irq_host *h, unsigned int virq,
 			irq_hw_number_t hw)
 {
 	struct uic *uic = h->host_data;
 
 	set_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_uic_irq);
+	/* appropriate irq handler is set by set_irq_type() */
+	set_irq_chip(virq, &uic_irq_chip);
 
 	/* Set default irq type */
 	set_irq_type(virq, IRQ_TYPE_NONE);

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

* Re: [PATCH 1/2] PowerPC: 4xx uic: add mask_ack callback
  2007-11-13 20:25 ` [PATCH 1/2] PowerPC: 4xx uic: add mask_ack callback Valentine Barshak
@ 2007-11-14  0:57   ` David Gibson
  0 siblings, 0 replies; 11+ messages in thread
From: David Gibson @ 2007-11-14  0:57 UTC (permalink / raw)
  To: Valentine Barshak; +Cc: linuxppc-dev

On Tue, Nov 13, 2007 at 11:25:21PM +0300, Valentine Barshak wrote:
> This adds uic_mask_ack_irq() callback to PowerPC 4xx uic code 
> to avoid kernel crash. It is used for edge-triggered interrupts
> by handle_uic_irq().

Oops.  Obviously never caught this obvious bug, because I've never
used an edge-triggered interrupt on the Ebony yet.

> Signed-off-by: Valentine Barshak <vbarshak@ru.mvista.com>

Acked-by: David Gibson <david@gibson.dropbear.id.au>

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

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

* Re: [PATCH 0/2] PowerPC: 4xx uic updates
  2007-11-13 20:15 [PATCH 0/2] PowerPC: 4xx uic updates Valentine Barshak
  2007-11-13 20:25 ` [PATCH 1/2] PowerPC: 4xx uic: add mask_ack callback Valentine Barshak
  2007-11-13 20:27 ` [PATCH 2/2] PowerPC: make 4xx uic use generic edge and level irq handlers Valentine Barshak
@ 2007-11-14  2:05 ` Josh Boyer
  2007-11-14  2:14   ` David Gibson
  2007-11-14  3:40   ` Benjamin Herrenschmidt
  2 siblings, 2 replies; 11+ messages in thread
From: Josh Boyer @ 2007-11-14  2:05 UTC (permalink / raw)
  To: Valentine Barshak; +Cc: linuxppc-dev, dwg

On Tue, 13 Nov 2007 23:15:59 +0300
Valentine Barshak <vbarshak@ru.mvista.com> wrote:

> These patches update 4xx uic code. The first one
> fixes a minor issue with edge-triggered interrupts,
> while the second one makes it use generic level and edge irq
> handlers. I've added irq ack'ing to the unmask callback for
> level-triggered interrupts, because to de-assert them we have
> to do 2 things is the exact order as below:
> 1. de-assert the external source in the ISR.
> 2. ack the IRQ on the UIC.
> So, ack'ing level interrupts before unmasking them makes possible
> to use generic level irq handler and it doesn't hurt, cause
> we can never miss a level-triggered interrupt. It always stays
> asserted untill the external source is removed and ack'ed on UIC.
> 
> These have been tested on Sequoia PowerPC 440EPx board.

Is my mail server slow, or did patch 2 of 2 never make it out?

josh

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

* Re: [PATCH 2/2] PowerPC: make 4xx uic use generic edge and level irq handlers
  2007-11-13 20:27 ` [PATCH 2/2] PowerPC: make 4xx uic use generic edge and level irq handlers Valentine Barshak
@ 2007-11-14  2:13   ` David Gibson
  2007-11-14  3:43     ` Benjamin Herrenschmidt
  0 siblings, 1 reply; 11+ messages in thread
From: David Gibson @ 2007-11-14  2:13 UTC (permalink / raw)
  To: Valentine Barshak; +Cc: linuxppc-dev

On Tue, Nov 13, 2007 at 11:27:31PM +0300, Valentine Barshak wrote:
> This patch makes PowerPC 4xx UIC use generic edge and level irq handlers
> instead of a custom handle_uic_irq() function. Acking a level irq on UIC
> has no effect if the interrupt is still asserted by the device, even if
> the interrupt is already masked. So, to really de-assert the interrupt
> we need to de-assert the external source first *and* ack it on UIC then.
> The handle_level_irq() function masks and ack's the interrupt with mask_ack
> callback prior to calling the actual ISR and unmasks it at the end.
> So, to use it with UIC level interrupts we need to ack in the unmask
> callback instead, after the ISR has de-asserted the external interrupt source.
> Even if we ack the interrupt that we didn't handle (unmask/ack it at
> the end of the handler, while next irq is already pending) it will not
> de-assert the irq, untill we de-assert its exteral source.

Hrm.  I *think* I'm convinced this is safe, although acking in a
callback which doesn't say it acks is rather yucky.  Essentially this
code is trading flow readability (because just reading
handle_level_irq will tell you something other than what it does in
our case) for smaller code size.  I'm not sure if this is a good trade
or not.

There's also one definite problem: according to the discussions I had
with Thomas Gleixner when I wrote uic.c, handle_edge_irq is not what
we want for edge interrupts.

Apparently handle_edge_irq is only for edge interrupts on "broken"
PICs which won't latch new interrupts while the irq is masked.  UIC is
not in this category, so handle_level_irq is actually what we want,
even for an edge irq.

Yes, I thought the naming was more than a little confusing, too.

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

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

* Re: [PATCH 0/2] PowerPC: 4xx uic updates
  2007-11-14  2:05 ` [PATCH 0/2] PowerPC: 4xx uic updates Josh Boyer
@ 2007-11-14  2:14   ` David Gibson
  2007-11-14  3:40   ` Benjamin Herrenschmidt
  1 sibling, 0 replies; 11+ messages in thread
From: David Gibson @ 2007-11-14  2:14 UTC (permalink / raw)
  To: Josh Boyer; +Cc: linuxppc-dev

On Tue, Nov 13, 2007 at 08:05:14PM -0600, Josh Boyer wrote:
> On Tue, 13 Nov 2007 23:15:59 +0300
> Valentine Barshak <vbarshak@ru.mvista.com> wrote:
> 
> > These patches update 4xx uic code. The first one
> > fixes a minor issue with edge-triggered interrupts,
> > while the second one makes it use generic level and edge irq
> > handlers. I've added irq ack'ing to the unmask callback for
> > level-triggered interrupts, because to de-assert them we have
> > to do 2 things is the exact order as below:
> > 1. de-assert the external source in the ISR.
> > 2. ack the IRQ on the UIC.
> > So, ack'ing level interrupts before unmasking them makes possible
> > to use generic level irq handler and it doesn't hurt, cause
> > we can never miss a level-triggered interrupt. It always stays
> > asserted untill the external source is removed and ack'ed on UIC.
> > 
> > These have been tested on Sequoia PowerPC 440EPx board.
> 
> Is my mail server slow, or did patch 2 of 2 never make it out?

It reached me eventually, but only to my ibm address, not via the
list (whereas I got both copies of 0 and 1).

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

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

* Re: [PATCH 0/2] PowerPC: 4xx uic updates
  2007-11-14  2:05 ` [PATCH 0/2] PowerPC: 4xx uic updates Josh Boyer
  2007-11-14  2:14   ` David Gibson
@ 2007-11-14  3:40   ` Benjamin Herrenschmidt
  1 sibling, 0 replies; 11+ messages in thread
From: Benjamin Herrenschmidt @ 2007-11-14  3:40 UTC (permalink / raw)
  To: Josh Boyer; +Cc: linuxppc-dev, dwg


On Tue, 2007-11-13 at 20:05 -0600, Josh Boyer wrote:
> On Tue, 13 Nov 2007 23:15:59 +0300
> Valentine Barshak <vbarshak@ru.mvista.com> wrote:
> 
> > These patches update 4xx uic code. The first one
> > fixes a minor issue with edge-triggered interrupts,
> > while the second one makes it use generic level and edge irq
> > handlers. I've added irq ack'ing to the unmask callback for
> > level-triggered interrupts, because to de-assert them we have
> > to do 2 things is the exact order as below:
> > 1. de-assert the external source in the ISR.
> > 2. ack the IRQ on the UIC.
> > So, ack'ing level interrupts before unmasking them makes possible
> > to use generic level irq handler and it doesn't hurt, cause
> > we can never miss a level-triggered interrupt. It always stays
> > asserted untill the external source is removed and ack'ed on UIC.
> > 
> > These have been tested on Sequoia PowerPC 440EPx board.
> 
> Is my mail server slow, or did patch 2 of 2 never make it out?

It never reached me neither.

Ben

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

* Re: [PATCH 2/2] PowerPC: make 4xx uic use generic edge and level irq handlers
  2007-11-14  2:13   ` David Gibson
@ 2007-11-14  3:43     ` Benjamin Herrenschmidt
  2007-11-14 12:31       ` Valentine Barshak
  0 siblings, 1 reply; 11+ messages in thread
From: Benjamin Herrenschmidt @ 2007-11-14  3:43 UTC (permalink / raw)
  To: David Gibson; +Cc: linuxppc-dev


On Wed, 2007-11-14 at 13:13 +1100, David Gibson wrote:
> Hrm.  I *think* I'm convinced this is safe, although acking in a
> callback which doesn't say it acks is rather yucky.  Essentially this
> code is trading flow readability (because just reading
> handle_level_irq will tell you something other than what it does in
> our case) for smaller code size.  I'm not sure if this is a good trade
> or not.
> 
> There's also one definite problem: according to the discussions I had
> with Thomas Gleixner when I wrote uic.c, handle_edge_irq is not what
> we want for edge interrupts.
> 
> Apparently handle_edge_irq is only for edge interrupts on "broken"
> PICs which won't latch new interrupts while the irq is masked.  UIC is
> not in this category, so handle_level_irq is actually what we want,
> even for an edge irq.
> 
> Yes, I thought the naming was more than a little confusing, too.

Hrm... handle_edge_irq works for both and you have a small performance
benefit in not masking, and thus using handle_edge_irq, so I don't
totally agree here. Basically, what handle_edge_irq() does is lazy
masking. Now there -is- an issue here is that if you do lazy masking,
you need to be able to re-emit in some convenient way.

Ben.

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

* Re: [PATCH 2/2] PowerPC: make 4xx uic use generic edge and level irq handlers
  2007-11-14  3:43     ` Benjamin Herrenschmidt
@ 2007-11-14 12:31       ` Valentine Barshak
  2007-11-14 14:00         ` [PATCH 2/2] PowerPC: make 4xx uic use generic level irq handler Valentine Barshak
  0 siblings, 1 reply; 11+ messages in thread
From: Valentine Barshak @ 2007-11-14 12:31 UTC (permalink / raw)
  To: benh; +Cc: linuxppc-dev, David Gibson

Benjamin Herrenschmidt wrote:
> On Wed, 2007-11-14 at 13:13 +1100, David Gibson wrote:
>> Hrm.  I *think* I'm convinced this is safe, although acking in a
>> callback which doesn't say it acks is rather yucky.  Essentially this
>> code is trading flow readability (because just reading
>> handle_level_irq will tell you something other than what it does in
>> our case) for smaller code size.  I'm not sure if this is a good trade
>> or not.
>>

It's not just code size. Actually, I was having problems with Ingo's 
real-time patch, that works on all platforms that use generic hard irq 
handlers and doesn't work just on 4xx since we use a custom one here.
And I thought that using generic handlers would be easier to maintain.
I agree that ack'ing in a callback which doesn't say it ack's looks odd, 
but ack'ing level-triggered interrupts is quirky on UIC itself. So, I 
just thought that adding a couple of quirks to mask_ack and unmask 
callbacks was not that bad.

>> There's also one definite problem: according to the discussions I had
>> with Thomas Gleixner when I wrote uic.c, handle_edge_irq is not what
>> we want for edge interrupts.
>>
>> Apparently handle_edge_irq is only for edge interrupts on "broken"
>> PICs which won't latch new interrupts while the irq is masked.  UIC is
>> not in this category, so handle_level_irq is actually what we want,
>> even for an edge irq.
>>
>> Yes, I thought the naming was more than a little confusing, too.
> 
> Hrm... handle_edge_irq works for both and you have a small performance
> benefit in not masking, and thus using handle_edge_irq, so I don't
> totally agree here. Basically, what handle_edge_irq() does is lazy
> masking. Now there -is- an issue here is that if you do lazy masking,
> you need to be able to re-emit in some convenient way.

With the ack quirks added we can use handle_level_irq for edge-triggered 
interrupts. I'll test and resubmit the patch.

> 
> Ben.
> 
> 

Thanks,
Valentine.

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

* [PATCH 2/2] PowerPC: make 4xx uic use generic level irq handler
  2007-11-14 12:31       ` Valentine Barshak
@ 2007-11-14 14:00         ` Valentine Barshak
  0 siblings, 0 replies; 11+ messages in thread
From: Valentine Barshak @ 2007-11-14 14:00 UTC (permalink / raw)
  To: linuxppc-dev

This patch makes PowerPC 4xx UIC use generic level irq handler instead
of a custom handle_uic_irq() function. We ack only edge irqs in mask_ack
callback, since acking a level irq on UIC has no effect if the interrupt
is still asserted by the device, even if the interrupt is already masked.
So, to really de-assert the interrupt we need to de-assert the external
source first *and* ack it on UIC then. The handle_level_irq() function
masks and ack's the interrupt with mask_ack callback prior to calling
the actual ISR and unmasks it at the end. So, to use it with UIC interrupts
we need to ack level irqs in the unmask callback instead, after the ISR
has de-asserted the external interrupt source. Even if we ack the interrupt
that we didn't handle (unmask/ack it at the end of the handler, while
next irq is already pending) it will not de-assert the irq, untill we
de-assert its exteral source.

Signed-off-by: Valentine Barshak <vbarshak@ru.mvista.com>
---
 arch/powerpc/sysdev/uic.c |   81 ++++++++++------------------------------------
 1 files changed, 19 insertions(+), 62 deletions(-)

--- linux-2.6.orig/arch/powerpc/sysdev/uic.c	2007-11-14 15:57:37.000000000 +0300
+++ linux-2.6/arch/powerpc/sysdev/uic.c	2007-11-14 15:59:18.000000000 +0300
@@ -60,14 +60,19 @@ struct uic {
 
 static void uic_unmask_irq(unsigned int virq)
 {
+	struct irq_desc *desc = get_irq_desc(virq);
 	struct uic *uic = get_irq_chip_data(virq);
 	unsigned int src = uic_irq_to_hw(virq);
 	unsigned long flags;
-	u32 er;
+	u32 er, sr;
 
+	sr = 1 << (31-src);
 	spin_lock_irqsave(&uic->lock, flags);
+	/* ack level-triggered interrupts here */
+	if (desc->status & IRQ_LEVEL)
+		mtdcr(uic->dcrbase + UIC_SR, sr);
 	er = mfdcr(uic->dcrbase + UIC_ER);
-	er |= 1 << (31 - src);
+	er |= sr;
 	mtdcr(uic->dcrbase + UIC_ER, er);
 	spin_unlock_irqrestore(&uic->lock, flags);
 }
@@ -99,6 +104,7 @@ static void uic_ack_irq(unsigned int vir
 
 static void uic_mask_ack_irq(unsigned int virq)
 {
+	struct irq_desc *desc = get_irq_desc(virq);
 	struct uic *uic = get_irq_chip_data(virq);
 	unsigned int src = uic_irq_to_hw(virq);
 	unsigned long flags;
@@ -109,7 +115,16 @@ static void uic_mask_ack_irq(unsigned in
 	er = mfdcr(uic->dcrbase + UIC_ER);
 	er &= ~sr;
 	mtdcr(uic->dcrbase + UIC_ER, er);
-	mtdcr(uic->dcrbase + UIC_SR, sr);
+ 	/* 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
+	 * we only ack the egde interrupts here, while
+	 * level interrupts are ack'ed after the actual
+	 * isr call in the uic_unmask_irq()
+	 */
+	if (!(desc->status & IRQ_LEVEL))
+		mtdcr(uic->dcrbase + UIC_SR, sr);
 	spin_unlock_irqrestore(&uic->lock, flags);
 }
 
@@ -173,64 +188,6 @@ 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_map(struct irq_host *h, unsigned int virq,
 			irq_hw_number_t hw)
 {
@@ -239,7 +196,7 @@ static int uic_host_map(struct irq_host 
 	set_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_uic_irq);
+	set_irq_chip_and_handler(virq, &uic_irq_chip, handle_level_irq);
 
 	/* Set default irq type */
 	set_irq_type(virq, IRQ_TYPE_NONE);

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

end of thread, other threads:[~2007-11-14 14:02 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-11-13 20:15 [PATCH 0/2] PowerPC: 4xx uic updates Valentine Barshak
2007-11-13 20:25 ` [PATCH 1/2] PowerPC: 4xx uic: add mask_ack callback Valentine Barshak
2007-11-14  0:57   ` David Gibson
2007-11-13 20:27 ` [PATCH 2/2] PowerPC: make 4xx uic use generic edge and level irq handlers Valentine Barshak
2007-11-14  2:13   ` David Gibson
2007-11-14  3:43     ` Benjamin Herrenschmidt
2007-11-14 12:31       ` Valentine Barshak
2007-11-14 14:00         ` [PATCH 2/2] PowerPC: make 4xx uic use generic level irq handler Valentine Barshak
2007-11-14  2:05 ` [PATCH 0/2] PowerPC: 4xx uic updates Josh Boyer
2007-11-14  2:14   ` David Gibson
2007-11-14  3:40   ` Benjamin Herrenschmidt

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