| Message ID | 1422371952-1126-1-git-send-email-bogdan.purcareata@freescale.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Benjamin Herrenschmidt |
| Headers | show |
On Tue, 2015-01-27 at 15:19 +0000, Bogdan Purcareata wrote: > The readback acts as a synchronization mechanism in handling external > interrupts, making sure the core waits until EOI write completion. This is > required in certain scenarios, such as when the MPIC communicates with a PCI > device in posted write mode. If the device uses legacy interrupts, and the CPU > returns from the interrupt as soon as it fires the EOI write, there is a chance > to receive spurious interrupts because the line isn't deasserted yet. The line was deasserted when IACK was read, before the EOI. The issue is (in theory at least) the timing of the write to the device versus the write to EOI, not the timing of MPIC receiving the EOI versus when the CPU executes rfi. > + - mpic-eoi-no-readback > + Usage: optional > + Value type: <empty> > + Definition: The presence of this property specifies that the > + MPIC will not issue a readback s/will not/does not need to/ At this point, given the previous discussion, does anyone object to removing the readback entirely? Do we have any evidence that reading WHOAMI is effective at addressing whatever problem reading EOI made go away on the relevant platform? > when delivering the EOI for an > + external interrupt. The readback operation is done by reading > + the CPU WHOAMI register after writing to the CPU EOI register. > + Originally, this was required due to the fact that the MPIC > + operates at lower frequencies, Why? -Scott
On Tue, Jan 27, 2015 at 03:57:53PM -0600, Scott Wood wrote: > At this point, given the previous discussion, does anyone object to > removing the readback entirely? Do we have any evidence that reading > WHOAMI is effective at addressing whatever problem reading EOI made go > away on the relevant platform? What platforms were those? Just test there! If no one remembers, they can not (any longer) be important platforms ;-) Just remove the code, and if in twelve months finally someone reports a failure we will at least know what platform it was. Segher
On Wed, 2015-01-28 at 14:15 -0600, Segher Boessenkool wrote: > On Tue, Jan 27, 2015 at 03:57:53PM -0600, Scott Wood wrote: > > At this point, given the previous discussion, does anyone object to > > removing the readback entirely? Do we have any evidence that reading > > WHOAMI is effective at addressing whatever problem reading EOI made go > > away on the relevant platform? > > What platforms were those? Just test there! Ben said "some macs". The only Mac I have is packed away and may not be the one Ben had in mind, so I can't "test there". > If no one remembers, they can not (any longer) be important platforms ;-) > Just remove the code, and if in twelve months finally someone reports a > failure we will at least know what platform it was. This is what I was trying to encourage. :-) -Scott
diff --git a/Documentation/devicetree/bindings/powerpc/fsl/mpic.txt b/Documentation/devicetree/bindings/powerpc/fsl/mpic.txt index dc57446..9789094 100644 --- a/Documentation/devicetree/bindings/powerpc/fsl/mpic.txt +++ b/Documentation/devicetree/bindings/powerpc/fsl/mpic.txt @@ -77,6 +77,19 @@ PROPERTIES in the global feature registers. If specified, this field will override the value read from MPIC_GREG_FEATURE_LAST_SRC. + - mpic-eoi-no-readback + Usage: optional + Value type: <empty> + Definition: The presence of this property specifies that the + MPIC will not issue a readback when delivering the EOI for an + external interrupt. The readback operation is done by reading + the CPU WHOAMI register after writing to the CPU EOI register. + Originally, this was required due to the fact that the MPIC + operates at lower frequencies, or in scenarios where the MPIC + is connected through PCI with write posting. This is not the + case in an emulated environment (e.g. KVM guest), or in scenarios + where interrupts are not handled in a loop of get_irq() calls. + INTERRUPT SPECIFIER DEFINITION Interrupt specifiers consists of 4 cells encoded as diff --git a/arch/powerpc/include/asm/mpic.h b/arch/powerpc/include/asm/mpic.h index 754f93d..e2a4146 100644 --- a/arch/powerpc/include/asm/mpic.h +++ b/arch/powerpc/include/asm/mpic.h @@ -386,6 +386,8 @@ extern struct bus_type mpic_subsys; * from the BRR1 register). */ #define MPIC_FSL_HAS_EIMR 0x00010000 +/* Dont bother with readback after MPIC EOI */ +#define MPIC_EOI_NO_READBACK 0x00020000 /* MPIC HW modification ID */ #define MPIC_REGSET_MASK 0xf0000000 diff --git a/arch/powerpc/sysdev/mpic.c b/arch/powerpc/sysdev/mpic.c index f3e8624..431f68e 100644 --- a/arch/powerpc/sysdev/mpic.c +++ b/arch/powerpc/sysdev/mpic.c @@ -656,7 +656,9 @@ static inline struct mpic * mpic_from_irq_data(struct irq_data *d) static inline void mpic_eoi(struct mpic *mpic) { mpic_cpu_write(MPIC_INFO(CPU_EOI), 0); - (void)mpic_cpu_read(MPIC_INFO(CPU_WHOAMI)); + + if (!(mpic->flags & MPIC_EOI_NO_READBACK)) + (void)mpic_cpu_read(MPIC_INFO(CPU_WHOAMI)); } /* @@ -1290,6 +1292,10 @@ struct mpic * __init mpic_alloc(struct device_node *node, flags |= MPIC_SINGLE_DEST_CPU; if (of_device_is_compatible(node, "fsl,mpic")) flags |= MPIC_FSL | MPIC_LARGE_VECTORS; + if (of_get_property(node, "mpic-eoi-no-readback", NULL)) { + pr_debug("mpic: no readback activated"); + flags |= MPIC_EOI_NO_READBACK; + } mpic = kzalloc(sizeof(struct mpic), GFP_KERNEL); if (mpic == NULL)