* [PATCH] qe_ic: Do a sync when masking interrupts.
@ 2006-10-19 18:03 Scott Wood
2006-10-20 3:03 ` Li Yang-r58472
2006-10-23 3:59 ` Paul Mackerras
0 siblings, 2 replies; 16+ messages in thread
From: Scott Wood @ 2006-10-19 18:03 UTC (permalink / raw)
To: linuxppc-dev
This patch causes a sync do be done after masking a QE interrupt, to
ensure that the masking has completed before interrupts are enabled.
This allows the masking of the cascade IRQ to be removed without causing
spurious interrupts.
The mask_and_ack function is also removed and set to the mask function,
as the two are identical.
Signed-off-by: Scott Wood <scottwood@freescale.com>
---
arch/powerpc/sysdev/qe_lib/qe_ic.c | 34 +++++++---------------------------
1 files changed, 7 insertions(+), 27 deletions(-)
diff --git a/arch/powerpc/sysdev/qe_lib/qe_ic.c b/arch/powerpc/sysdev/qe_lib/qe_ic.c
index 6995f51..d96e48d 100644
--- a/arch/powerpc/sysdev/qe_lib/qe_ic.c
+++ b/arch/powerpc/sysdev/qe_lib/qe_ic.c
@@ -222,24 +222,12 @@ static void qe_ic_mask_irq(unsigned int
temp = qe_ic_read(qe_ic->regs, qe_ic_info[src].mask_reg);
qe_ic_write(qe_ic->regs, qe_ic_info[src].mask_reg,
temp & ~qe_ic_info[src].mask);
-
- spin_unlock_irqrestore(&qe_ic_lock, flags);
-}
-
-static void qe_ic_mask_irq_and_ack(unsigned int virq)
-{
- struct qe_ic *qe_ic = qe_ic_from_irq(virq);
- unsigned int src = virq_to_hw(virq);
- unsigned long flags;
- u32 temp;
-
- spin_lock_irqsave(&qe_ic_lock, flags);
-
- temp = qe_ic_read(qe_ic->regs, qe_ic_info[src].mask_reg);
- qe_ic_write(qe_ic->regs, qe_ic_info[src].mask_reg,
- temp & ~qe_ic_info[src].mask);
-
- /* There is nothing to do for ack here, ack is handled in ISR */
+
+ /* Flush the above write before enabling interrupts;
+ * otherwise, spurious interrupts will sometimes
+ * happen
+ */
+ mb();
spin_unlock_irqrestore(&qe_ic_lock, flags);
}
@@ -248,7 +236,7 @@ static struct irq_chip qe_ic_irq_chip =
.typename = " QEIC ",
.unmask = qe_ic_unmask_irq,
.mask = qe_ic_mask_irq,
- .mask_ack = qe_ic_mask_irq_and_ack,
+ .mask_ack = qe_ic_mask_irq,
};
static int qe_ic_host_match(struct irq_host *h, struct device_node *node)
@@ -331,8 +319,6 @@ unsigned int qe_ic_get_high_irq(struct q
return irq_linear_revmap(qe_ic->irqhost, irq);
}
-/* FIXME: We mask all the QE Low interrupts while handling. We should
- * let other interrupt come in, but BAD interrupts are generated */
void fastcall qe_ic_cascade_low(unsigned int irq, struct irq_desc *desc)
{
struct qe_ic *qe_ic = desc->handler_data;
@@ -340,14 +326,10 @@ void fastcall qe_ic_cascade_low(unsigned
unsigned int cascade_irq = qe_ic_get_low_irq(qe_ic);
- chip->mask_ack(irq);
if (cascade_irq != NO_IRQ)
generic_handle_irq(cascade_irq);
- chip->unmask(irq);
}
-/* FIXME: We mask all the QE High interrupts while handling. We should
- * let other interrupt come in, but BAD interrupts are generated */
void fastcall qe_ic_cascade_high(unsigned int irq, struct irq_desc *desc)
{
struct qe_ic *qe_ic = desc->handler_data;
@@ -355,10 +337,8 @@ void fastcall qe_ic_cascade_high(unsigne
unsigned int cascade_irq = qe_ic_get_high_irq(qe_ic);
- chip->mask_ack(irq);
if (cascade_irq != NO_IRQ)
generic_handle_irq(cascade_irq);
- chip->unmask(irq);
}
void __init qe_ic_init(struct device_node *node, unsigned int flags)
--
1.4.2.3
^ permalink raw reply related [flat|nested] 16+ messages in thread
* RE: [PATCH] qe_ic: Do a sync when masking interrupts.
2006-10-19 18:03 Scott Wood
@ 2006-10-20 3:03 ` Li Yang-r58472
2006-10-23 3:59 ` Paul Mackerras
1 sibling, 0 replies; 16+ messages in thread
From: Li Yang-r58472 @ 2006-10-20 3:03 UTC (permalink / raw)
To: Wood Scott-B07421, linuxppc-dev
> -----Original Message-----
> From: linuxppc-dev-bounces+leoli=3Dfreescale.com@ozlabs.org
> [mailto:linuxppc-dev-bounces+leoli=3Dfreescale.com@ozlabs.org] On =
Behalf
Of Scott
> Wood
> Sent: Friday, October 20, 2006 2:03 AM
> To: linuxppc-dev@ozlabs.org
> Subject: [PATCH] qe_ic: Do a sync when masking interrupts.
>=20
> This patch causes a sync do be done after masking a QE interrupt, to
> ensure that the masking has completed before interrupts are enabled.
> This allows the masking of the cascade IRQ to be removed without
causing
> spurious interrupts.
>=20
> The mask_and_ack function is also removed and set to the mask
function,
> as the two are identical.
>=20
> Signed-off-by: Scott Wood <scottwood@freescale.com>
Acked-by: Li Yang <leoli@freescale.com>
> ---
> arch/powerpc/sysdev/qe_lib/qe_ic.c | 34
+++++++---------------------------
> 1 files changed, 7 insertions(+), 27 deletions(-)
>=20
> diff --git a/arch/powerpc/sysdev/qe_lib/qe_ic.c
> b/arch/powerpc/sysdev/qe_lib/qe_ic.c
> index 6995f51..d96e48d 100644
> --- a/arch/powerpc/sysdev/qe_lib/qe_ic.c
> +++ b/arch/powerpc/sysdev/qe_lib/qe_ic.c
> @@ -222,24 +222,12 @@ static void qe_ic_mask_irq(unsigned int
> temp =3D qe_ic_read(qe_ic->regs, qe_ic_info[src].mask_reg);
> qe_ic_write(qe_ic->regs, qe_ic_info[src].mask_reg,
> temp & ~qe_ic_info[src].mask);
> -
> - spin_unlock_irqrestore(&qe_ic_lock, flags);
> -}
> -
> -static void qe_ic_mask_irq_and_ack(unsigned int virq)
> -{
> - struct qe_ic *qe_ic =3D qe_ic_from_irq(virq);
> - unsigned int src =3D virq_to_hw(virq);
> - unsigned long flags;
> - u32 temp;
> -
> - spin_lock_irqsave(&qe_ic_lock, flags);
> -
> - temp =3D qe_ic_read(qe_ic->regs, qe_ic_info[src].mask_reg);
> - qe_ic_write(qe_ic->regs, qe_ic_info[src].mask_reg,
> - temp & ~qe_ic_info[src].mask);
> -
> - /* There is nothing to do for ack here, ack is handled in ISR */
> +
> + /* Flush the above write before enabling interrupts;
> + * otherwise, spurious interrupts will sometimes
> + * happen
> + */
> + mb();
>=20
> spin_unlock_irqrestore(&qe_ic_lock, flags);
> }
> @@ -248,7 +236,7 @@ static struct irq_chip qe_ic_irq_chip =3D
> .typename =3D " QEIC ",
> .unmask =3D qe_ic_unmask_irq,
> .mask =3D qe_ic_mask_irq,
> - .mask_ack =3D qe_ic_mask_irq_and_ack,
> + .mask_ack =3D qe_ic_mask_irq,
> };
>=20
> static int qe_ic_host_match(struct irq_host *h, struct device_node
*node)
> @@ -331,8 +319,6 @@ unsigned int qe_ic_get_high_irq(struct q
> return irq_linear_revmap(qe_ic->irqhost, irq);
> }
>=20
> -/* FIXME: We mask all the QE Low interrupts while handling. We
should
> - * let other interrupt come in, but BAD interrupts are generated */
> void fastcall qe_ic_cascade_low(unsigned int irq, struct irq_desc
*desc)
> {
> struct qe_ic *qe_ic =3D desc->handler_data;
> @@ -340,14 +326,10 @@ void fastcall qe_ic_cascade_low(unsigned
>=20
> unsigned int cascade_irq =3D qe_ic_get_low_irq(qe_ic);
>=20
> - chip->mask_ack(irq);
> if (cascade_irq !=3D NO_IRQ)
> generic_handle_irq(cascade_irq);
> - chip->unmask(irq);
> }
>=20
> -/* FIXME: We mask all the QE High interrupts while handling. We
should
> - * let other interrupt come in, but BAD interrupts are generated */
> void fastcall qe_ic_cascade_high(unsigned int irq, struct irq_desc
*desc)
> {
> struct qe_ic *qe_ic =3D desc->handler_data;
> @@ -355,10 +337,8 @@ void fastcall qe_ic_cascade_high(unsigne
>=20
> unsigned int cascade_irq =3D qe_ic_get_high_irq(qe_ic);
>=20
> - chip->mask_ack(irq);
> if (cascade_irq !=3D NO_IRQ)
> generic_handle_irq(cascade_irq);
> - chip->unmask(irq);
> }
>=20
> void __init qe_ic_init(struct device_node *node, unsigned int flags)
> --
> 1.4.2.3
>=20
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@ozlabs.org
> https://ozlabs.org/mailman/listinfo/linuxppc-dev
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] qe_ic: Do a sync when masking interrupts.
2006-10-19 18:03 Scott Wood
2006-10-20 3:03 ` Li Yang-r58472
@ 2006-10-23 3:59 ` Paul Mackerras
2006-10-23 9:35 ` Li Yang-r58472
2006-10-23 15:19 ` Scott Wood
1 sibling, 2 replies; 16+ messages in thread
From: Paul Mackerras @ 2006-10-23 3:59 UTC (permalink / raw)
To: Scott Wood; +Cc: linuxppc-dev
Scott Wood writes:
> This patch causes a sync do be done after masking a QE interrupt, to
> ensure that the masking has completed before interrupts are enabled.
> This allows the masking of the cascade IRQ to be removed without causing
> spurious interrupts.
Hmmm. In general a sync having completed doesn't mean that previous
MMIO stores have actually got to the device. Reading from a device
register generally does ensure that previous writes have actually got
to the device though - could you do that instead? I'm concerned that
adding the sync is not a robust fix and is possibly only working due
to fortuitous timing.
Paul.
^ permalink raw reply [flat|nested] 16+ messages in thread
* RE: [PATCH] qe_ic: Do a sync when masking interrupts.
2006-10-23 3:59 ` Paul Mackerras
@ 2006-10-23 9:35 ` Li Yang-r58472
2006-10-23 15:19 ` Scott Wood
1 sibling, 0 replies; 16+ messages in thread
From: Li Yang-r58472 @ 2006-10-23 9:35 UTC (permalink / raw)
To: Paul Mackerras, Wood Scott-B07421; +Cc: linuxppc-dev
> -----Original Message-----
> From: linuxppc-dev-bounces+leoli=3Dfreescale.com@ozlabs.org
> [mailto:linuxppc-dev-bounces+leoli=3Dfreescale.com@ozlabs.org] On =
Behalf
Of Paul
> Mackerras
> Sent: Monday, October 23, 2006 12:00 PM
> To: Wood Scott-B07421
> Cc: linuxppc-dev@ozlabs.org
> Subject: Re: [PATCH] qe_ic: Do a sync when masking interrupts.
>=20
> Scott Wood writes:
>=20
> > This patch causes a sync do be done after masking a QE interrupt, to
> > ensure that the masking has completed before interrupts are enabled.
> > This allows the masking of the cascade IRQ to be removed without
causing
> > spurious interrupts.
>=20
> Hmmm. In general a sync having completed doesn't mean that previous
> MMIO stores have actually got to the device. Reading from a device
> register generally does ensure that previous writes have actually got
> to the device though - could you do that instead? I'm concerned that
> adding the sync is not a robust fix and is possibly only working due
> to fortuitous timing.
But an i/o read will be considerably slower than a sync, and it is in
the critical path of interrupt. I have tested the patch under
relatively heavy Ethernet load, and there is no spurious interrupt.
Maybe it is because the device is an SOC device and MMIO store completes
faster. I'm wondering if there is a standard test method to show if the
faster approach is sufficient or not.
- Leo=20
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] qe_ic: Do a sync when masking interrupts.
2006-10-23 3:59 ` Paul Mackerras
2006-10-23 9:35 ` Li Yang-r58472
@ 2006-10-23 15:19 ` Scott Wood
2006-10-23 15:29 ` Scott Wood
2006-10-23 17:52 ` Kumar Gala
1 sibling, 2 replies; 16+ messages in thread
From: Scott Wood @ 2006-10-23 15:19 UTC (permalink / raw)
To: Paul Mackerras; +Cc: linuxppc-dev
Paul Mackerras wrote:
> Hmmm. In general a sync having completed doesn't mean that previous
> MMIO stores have actually got to the device. Reading from a device
> register generally does ensure that previous writes have actually got
> to the device though - could you do that instead? I'm concerned that
> adding the sync is not a robust fix and is possibly only working due
> to fortuitous timing.
Possibly -- but the only problem on the rare occasions where the timing
is not fortuitous is a spurious interrupt; the only reason the sync
needs to be there at all is to avoid the overhead of the extra interrupt
(and to avoid user complaints that they're getting "BAD" interrupts).
Note that many/most interrupt controller drivers don't even do that
much; they just have more fortuitous timing than the QE (and/or the
spurious interrupt is non-cascaded, and thus gets caught in
handle_level_irq and doesn't increment the spurious interrupt count).
-Scott
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] qe_ic: Do a sync when masking interrupts.
2006-10-23 15:19 ` Scott Wood
@ 2006-10-23 15:29 ` Scott Wood
2006-10-23 17:52 ` Kumar Gala
1 sibling, 0 replies; 16+ messages in thread
From: Scott Wood @ 2006-10-23 15:29 UTC (permalink / raw)
To: Paul Mackerras; +Cc: linuxppc-dev
Scott Wood wrote:
> Note that many/most interrupt controller drivers don't even do that
> much; they just have more fortuitous timing than the QE (and/or the
> spurious interrupt is non-cascaded, and thus gets caught in
> handle_level_irq and doesn't increment the spurious interrupt count).
Ignore that last bit about non-cascaded interrupts; if it makes a
difference at all, it's because of added delays (getting back to the
fortuitous timing thing).
-Scott
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: RE: [PATCH] qe_ic: Do a sync when masking interrupts.
@ 2006-10-23 15:30 Michael R. Zucca
2006-10-23 16:49 ` Segher Boessenkool
0 siblings, 1 reply; 16+ messages in thread
From: Michael R. Zucca @ 2006-10-23 15:30 UTC (permalink / raw)
To: Li Yang-r58472, Paul Mackerras, Wood Scott-B07421; +Cc: linuxppc-dev
>From: Li Yang-r58472 <LeoLi@freescale.com>
>
>But an i/o read will be considerably slower than a sync, and it is in
>the critical path of interrupt. I have tested the patch under
>relatively heavy Ethernet load, and there is no spurious interrupt.
>Maybe it is because the device is an SOC device and MMIO store completes
>faster. I'm wondering if there is a standard test method to show if the
>faster approach is sufficient or not.
All a sync tells you is that an I/O made it out of the CPU. The problem is, there may be other places a write could get hung up. For instance, sometimes devices sit behind a bridge with a write FIFO. In such a scenario, you can't be sure a write has made it to the device until you do a read to flush the FIFO.
If you're trying to figure out the minimum thing to do (eieio, sync, read-back, etc.) you have to understand what your system is doing between the store and the bits going into the register.
It may be that a sync is enough, but you won't know until you fully understand the system's bus/bridge topolgy between the CPU and the device.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] qe_ic: Do a sync when masking interrupts.
2006-10-23 15:30 RE: [PATCH] qe_ic: Do a sync when masking interrupts Michael R. Zucca
@ 2006-10-23 16:49 ` Segher Boessenkool
2006-10-23 18:27 ` Scott Wood
0 siblings, 1 reply; 16+ messages in thread
From: Segher Boessenkool @ 2006-10-23 16:49 UTC (permalink / raw)
To: mrz5149; +Cc: linuxppc-dev, Paul Mackerras
> ll a sync tells you is that an I/O made it out of the CPU. The
> problem is, there may be other places a write could get hung up.
> For instance, sometimes devices sit behind a bridge with a write
> FIFO. In such a scenario, you can't be sure a write has made it to
> the device until you do a read to flush the FIFO.
It's not enough that a write made it to the device even -- you
have to make sure the device has acted on it.
> If you're trying to figure out the minimum thing to do (eieio,
> sync, read-back, etc.) you have to understand what your system is
> doing between the store and the bits going into the register.
What the system is doing, and also what exactly you want to
accomplish (what ordering and what completion you depend on).
> It may be that a sync is enough, but you won't know until you fully
> understand the system's bus/bridge topolgy between the CPU and the
> device.
If a sync after an MMIO write is enough, then (in almost all
cases) so is an eieio.
Segher
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] qe_ic: Do a sync when masking interrupts.
2006-10-23 15:19 ` Scott Wood
2006-10-23 15:29 ` Scott Wood
@ 2006-10-23 17:52 ` Kumar Gala
2006-10-23 18:22 ` Scott Wood
1 sibling, 1 reply; 16+ messages in thread
From: Kumar Gala @ 2006-10-23 17:52 UTC (permalink / raw)
To: Scott Wood; +Cc: linuxppc-dev, Paul Mackerras
On Oct 23, 2006, at 10:19 AM, Scott Wood wrote:
> Paul Mackerras wrote:
>> Hmmm. In general a sync having completed doesn't mean that previous
>> MMIO stores have actually got to the device. Reading from a device
>> register generally does ensure that previous writes have actually got
>> to the device though - could you do that instead? I'm concerned that
>> adding the sync is not a robust fix and is possibly only working due
>> to fortuitous timing.
>
> Possibly -- but the only problem on the rare occasions where the
> timing
> is not fortuitous is a spurious interrupt; the only reason the sync
> needs to be there at all is to avoid the overhead of the extra
> interrupt
> (and to avoid user complaints that they're getting "BAD" interrupts).
Why wouldn't the read accomplish the same thing in a more robust way
than the sync?
- k
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] qe_ic: Do a sync when masking interrupts.
2006-10-23 17:52 ` Kumar Gala
@ 2006-10-23 18:22 ` Scott Wood
0 siblings, 0 replies; 16+ messages in thread
From: Scott Wood @ 2006-10-23 18:22 UTC (permalink / raw)
To: Kumar Gala; +Cc: linuxppc-dev, Paul Mackerras
Kumar Gala wrote:
>
> On Oct 23, 2006, at 10:19 AM, Scott Wood wrote:
>> Possibly -- but the only problem on the rare occasions where the timing
>> is not fortuitous is a spurious interrupt; the only reason the sync
>> needs to be there at all is to avoid the overhead of the extra interrupt
>> (and to avoid user complaints that they're getting "BAD" interrupts).
>
>
> Why wouldn't the read accomplish the same thing in a more robust way
> than the sync?
It would. However, it also adds a small amount of overhead to every QE
interrupt, and the only thing that that overhead buys is avoiding
possible but empirically very rare spurious interrupts; it'd cost more
than simply accepting that a spurious interrupt might happen once in a
great while.
Without any type of sync, spurious interrupts happen fairly regularly
(about 5-10% of legitimate interrupts), so adding the sync should be a
net gain over doing nothing.
If the consensus is that a read should be done anyway, I can resumbit
the patch that way; I just think it's overkill given that a 100%
guarantee isn't required for correctness.
-Scott
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] qe_ic: Do a sync when masking interrupts.
2006-10-23 16:49 ` Segher Boessenkool
@ 2006-10-23 18:27 ` Scott Wood
2006-10-23 18:46 ` Segher Boessenkool
2006-10-24 7:17 ` Li Yang-r58472
0 siblings, 2 replies; 16+ messages in thread
From: Scott Wood @ 2006-10-23 18:27 UTC (permalink / raw)
To: Segher Boessenkool; +Cc: Paul Mackerras, linuxppc-dev
Segher Boessenkool wrote:
> If a sync after an MMIO write is enough, then (in almost all
> cases) so is an eieio.
In this case, the spurious interrupts still happen with eieio, but not
with sync. It's probably synchronizing the MMIO write with some
unrelated load from memory (such as reading the stack frame to return
from the mask function, or reading action->flags to determine whether to
check for IRQ_DISABLED).
-Scott
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] qe_ic: Do a sync when masking interrupts.
2006-10-23 18:27 ` Scott Wood
@ 2006-10-23 18:46 ` Segher Boessenkool
2006-10-24 7:17 ` Li Yang-r58472
1 sibling, 0 replies; 16+ messages in thread
From: Segher Boessenkool @ 2006-10-23 18:46 UTC (permalink / raw)
To: Scott Wood; +Cc: Paul Mackerras, linuxppc-dev
>> If a sync after an MMIO write is enough, then (in almost all
>> cases) so is an eieio.
>
> In this case, the spurious interrupts still happen with eieio, but
> not with sync. It's probably synchronizing the MMIO write with
> some unrelated load from memory (such as reading the stack frame to
> return from the mask function, or reading action->flags to
> determine whether to check for IRQ_DISABLED).
Yes, it sure sounds like the only reason the sync insn helps
is that it causes a (small) delay. If you document this (and
also that it isn't required for correctness) in the code, I
don't think anyone will complain / be curious about it anymore.
Segher
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH] qe_ic: Do a sync when masking interrupts.
@ 2006-10-23 18:53 Scott Wood
0 siblings, 0 replies; 16+ messages in thread
From: Scott Wood @ 2006-10-23 18:53 UTC (permalink / raw)
To: linuxppc-dev
This patch causes a sync do be done after masking a QE interrupt, to
ensure that the masking has completed before interrupts are enabled.
This allows the masking of the cascade IRQ to be removed without causing
spurious interrupts.
The mask_and_ack function is also removed and set to the mask function,
as the two are identical.
This is the second version of this patch; this version clarifies why a
sync was chosen rather than a read-back, and it fixes unused variable
warnings where the cascade mask was removed.
Signed-off-by: Scott Wood <scottwood@freescale.com>
---
arch/powerpc/sysdev/qe_lib/qe_ic.c | 40 +++++++++---------------------------
1 files changed, 10 insertions(+), 30 deletions(-)
diff --git a/arch/powerpc/sysdev/qe_lib/qe_ic.c b/arch/powerpc/sysdev/qe_lib/qe_ic.c
index 6995f51..74e48d9 100644
--- a/arch/powerpc/sysdev/qe_lib/qe_ic.c
+++ b/arch/powerpc/sysdev/qe_lib/qe_ic.c
@@ -223,23 +223,15 @@ static void qe_ic_mask_irq(unsigned int
qe_ic_write(qe_ic->regs, qe_ic_info[src].mask_reg,
temp & ~qe_ic_info[src].mask);
- spin_unlock_irqrestore(&qe_ic_lock, flags);
-}
-
-static void qe_ic_mask_irq_and_ack(unsigned int virq)
-{
- struct qe_ic *qe_ic = qe_ic_from_irq(virq);
- unsigned int src = virq_to_hw(virq);
- unsigned long flags;
- u32 temp;
-
- spin_lock_irqsave(&qe_ic_lock, flags);
-
- temp = qe_ic_read(qe_ic->regs, qe_ic_info[src].mask_reg);
- qe_ic_write(qe_ic->regs, qe_ic_info[src].mask_reg,
- temp & ~qe_ic_info[src].mask);
-
- /* There is nothing to do for ack here, ack is handled in ISR */
+ /* Flush the above write before enabling interrupts; otherwise,
+ * spurious interrupts will sometimes happen. To be 100% sure
+ * that the write has reached the device before interrupts are
+ * enabled, the mask register would have to be read back; however,
+ * this is not required for correctness, only to avoid wasting
+ * time on a large number of spurious interrupts. In testing,
+ * a sync reduced the observed spurious interrupts to zero.
+ */
+ mb();
spin_unlock_irqrestore(&qe_ic_lock, flags);
}
@@ -248,7 +240,7 @@ static struct irq_chip qe_ic_irq_chip =
.typename = " QEIC ",
.unmask = qe_ic_unmask_irq,
.mask = qe_ic_mask_irq,
- .mask_ack = qe_ic_mask_irq_and_ack,
+ .mask_ack = qe_ic_mask_irq,
};
static int qe_ic_host_match(struct irq_host *h, struct device_node *node)
@@ -331,34 +323,22 @@ unsigned int qe_ic_get_high_irq(struct q
return irq_linear_revmap(qe_ic->irqhost, irq);
}
-/* FIXME: We mask all the QE Low interrupts while handling. We should
- * let other interrupt come in, but BAD interrupts are generated */
void fastcall qe_ic_cascade_low(unsigned int irq, struct irq_desc *desc)
{
struct qe_ic *qe_ic = desc->handler_data;
- struct irq_chip *chip = irq_desc[irq].chip;
-
unsigned int cascade_irq = qe_ic_get_low_irq(qe_ic);
- chip->mask_ack(irq);
if (cascade_irq != NO_IRQ)
generic_handle_irq(cascade_irq);
- chip->unmask(irq);
}
-/* FIXME: We mask all the QE High interrupts while handling. We should
- * let other interrupt come in, but BAD interrupts are generated */
void fastcall qe_ic_cascade_high(unsigned int irq, struct irq_desc *desc)
{
struct qe_ic *qe_ic = desc->handler_data;
- struct irq_chip *chip = irq_desc[irq].chip;
-
unsigned int cascade_irq = qe_ic_get_high_irq(qe_ic);
- chip->mask_ack(irq);
if (cascade_irq != NO_IRQ)
generic_handle_irq(cascade_irq);
- chip->unmask(irq);
}
void __init qe_ic_init(struct device_node *node, unsigned int flags)
--
1.4.2.3
^ permalink raw reply related [flat|nested] 16+ messages in thread
* RE: [PATCH] qe_ic: Do a sync when masking interrupts.
2006-10-23 18:27 ` Scott Wood
2006-10-23 18:46 ` Segher Boessenkool
@ 2006-10-24 7:17 ` Li Yang-r58472
2006-10-25 3:51 ` Benjamin Herrenschmidt
1 sibling, 1 reply; 16+ messages in thread
From: Li Yang-r58472 @ 2006-10-24 7:17 UTC (permalink / raw)
To: Wood Scott-B07421, Segher Boessenkool; +Cc: Paul Mackerras, linuxppc-dev
> -----Original Message-----
> From: Scott Wood [mailto:scottwood@freescale.com]
> Sent: Tuesday, October 24, 2006 2:27 AM
> To: Segher Boessenkool
> Cc: mrz5149@acm.org; Li Yang-r58472; Paul Mackerras;
linuxppc-dev@ozlabs.org
> Subject: Re: [PATCH] qe_ic: Do a sync when masking interrupts.
>=20
> Segher Boessenkool wrote:
> > If a sync after an MMIO write is enough, then (in almost all
> > cases) so is an eieio.
>=20
> In this case, the spurious interrupts still happen with eieio, but not
> with sync. =20
eieio on e300 core is just a no-op.
- Leo
^ permalink raw reply [flat|nested] 16+ messages in thread
* RE: [PATCH] qe_ic: Do a sync when masking interrupts.
2006-10-24 7:17 ` Li Yang-r58472
@ 2006-10-25 3:51 ` Benjamin Herrenschmidt
2006-10-25 4:47 ` Liu Dave-r63238
0 siblings, 1 reply; 16+ messages in thread
From: Benjamin Herrenschmidt @ 2006-10-25 3:51 UTC (permalink / raw)
To: Li Yang-r58472; +Cc: linuxppc-dev, Paul Mackerras
On Tue, 2006-10-24 at 15:17 +0800, Li Yang-r58472 wrote:
> > -----Original Message-----
> > From: Scott Wood [mailto:scottwood@freescale.com]
> > Sent: Tuesday, October 24, 2006 2:27 AM
> > To: Segher Boessenkool
> > Cc: mrz5149@acm.org; Li Yang-r58472; Paul Mackerras;
> linuxppc-dev@ozlabs.org
> > Subject: Re: [PATCH] qe_ic: Do a sync when masking interrupts.
> >
> > Segher Boessenkool wrote:
> > > If a sync after an MMIO write is enough, then (in almost all
> > > cases) so is an eieio.
> >
> > In this case, the spurious interrupts still happen with eieio, but not
> > with sync.
>
> eieio on e300 core is just a no-op.
It's not sent to the bus on non-cahed storage (like the PCI bridge ?)
That's pretty bad ... there is quite a bit of code that assumes that
eieio's will prevent write combining...
Ben.
^ permalink raw reply [flat|nested] 16+ messages in thread
* RE: [PATCH] qe_ic: Do a sync when masking interrupts.
2006-10-25 3:51 ` Benjamin Herrenschmidt
@ 2006-10-25 4:47 ` Liu Dave-r63238
0 siblings, 0 replies; 16+ messages in thread
From: Liu Dave-r63238 @ 2006-10-25 4:47 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Li Yang-r58472; +Cc: linuxppc-dev, Paul Mackerras
> > > Segher Boessenkool wrote:
> > > > If a sync after an MMIO write is enough, then (in almost all
> > > > cases) so is an eieio.
> > >=20
> > > In this case, the spurious interrupts still happen with=20
> eieio, but=20
> > > not with sync.
> >=20
> > eieio on e300 core is just a no-op.
>=20
> It's not sent to the bus on non-cahed storage (like the PCI=20
> bridge ?) That's pretty bad ... there is quite a bit of code=20
> that assumes that eieio's will prevent write combining...
Due to the LSU, the e300 cored doesn't reorder non-cached
stroage, and doesn't implementation of write commbining.
So, actually its behavior likes eieio.
-Dave
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2006-10-25 4:47 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2006-10-23 15:30 RE: [PATCH] qe_ic: Do a sync when masking interrupts Michael R. Zucca
2006-10-23 16:49 ` Segher Boessenkool
2006-10-23 18:27 ` Scott Wood
2006-10-23 18:46 ` Segher Boessenkool
2006-10-24 7:17 ` Li Yang-r58472
2006-10-25 3:51 ` Benjamin Herrenschmidt
2006-10-25 4:47 ` Liu Dave-r63238
-- strict thread matches above, loose matches on Subject: below --
2006-10-23 18:53 Scott Wood
2006-10-19 18:03 Scott Wood
2006-10-20 3:03 ` Li Yang-r58472
2006-10-23 3:59 ` Paul Mackerras
2006-10-23 9:35 ` Li Yang-r58472
2006-10-23 15:19 ` Scott Wood
2006-10-23 15:29 ` Scott Wood
2006-10-23 17:52 ` Kumar Gala
2006-10-23 18:22 ` Scott Wood
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).