All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] powerpc/mpic: Add DT option to skip readback after EOI
@ 2015-01-27 15:19 ` Bogdan Purcareata
  0 siblings, 0 replies; 8+ messages in thread
From: Bogdan Purcareata @ 2015-01-27 15:19 UTC (permalink / raw)
  To: linuxppc-dev, linux-kernel; +Cc: Bogdan Purcareata, Scott Wood

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.

This doesn't happen in an emulated environment, e.g. KVM openpic, therefore the
readback is not required. In order to satisfy both cases, make the readback
optional and configurable through the device tree.

Skipping the readback saves a MMIO trap per interrupt.

v2: updated commit message

Signed-off-by: Scott Wood <scottwood@freescale.com>
[add DT binding, update commit message]
Signed-off-by: Bogdan Purcareata <bogdan.purcareata@freescale.com>
---
 Documentation/devicetree/bindings/powerpc/fsl/mpic.txt | 13 +++++++++++++
 arch/powerpc/include/asm/mpic.h                        |  2 ++
 arch/powerpc/sysdev/mpic.c                             |  8 +++++++-
 3 files changed, 22 insertions(+), 1 deletion(-)

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


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

* [PATCH v2] powerpc/mpic: Add DT option to skip readback after EOI
@ 2015-01-27 15:19 ` Bogdan Purcareata
  0 siblings, 0 replies; 8+ messages in thread
From: Bogdan Purcareata @ 2015-01-27 15:19 UTC (permalink / raw)
  To: linuxppc-dev, linux-kernel; +Cc: Scott Wood, Bogdan Purcareata

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.

This doesn't happen in an emulated environment, e.g. KVM openpic, therefore the
readback is not required. In order to satisfy both cases, make the readback
optional and configurable through the device tree.

Skipping the readback saves a MMIO trap per interrupt.

v2: updated commit message

Signed-off-by: Scott Wood <scottwood@freescale.com>
[add DT binding, update commit message]
Signed-off-by: Bogdan Purcareata <bogdan.purcareata@freescale.com>
---
 Documentation/devicetree/bindings/powerpc/fsl/mpic.txt | 13 +++++++++++++
 arch/powerpc/include/asm/mpic.h                        |  2 ++
 arch/powerpc/sysdev/mpic.c                             |  8 +++++++-
 3 files changed, 22 insertions(+), 1 deletion(-)

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

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

* Re: [PATCH v2] powerpc/mpic: Add DT option to skip readback after EOI
  2015-01-27 15:19 ` Bogdan Purcareata
@ 2015-01-27 21:57   ` Scott Wood
  -1 siblings, 0 replies; 8+ messages in thread
From: Scott Wood @ 2015-01-27 21:57 UTC (permalink / raw)
  To: Bogdan Purcareata; +Cc: linuxppc-dev, linux-kernel

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



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

* Re: [PATCH v2] powerpc/mpic: Add DT option to skip readback after EOI
@ 2015-01-27 21:57   ` Scott Wood
  0 siblings, 0 replies; 8+ messages in thread
From: Scott Wood @ 2015-01-27 21:57 UTC (permalink / raw)
  To: Bogdan Purcareata; +Cc: linuxppc-dev, linux-kernel

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

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

* Re: [PATCH v2] powerpc/mpic: Add DT option to skip readback after EOI
  2015-01-27 21:57   ` Scott Wood
@ 2015-01-28 20:15     ` Segher Boessenkool
  -1 siblings, 0 replies; 8+ messages in thread
From: Segher Boessenkool @ 2015-01-28 20:15 UTC (permalink / raw)
  To: Scott Wood; +Cc: Bogdan Purcareata, linuxppc-dev, linux-kernel

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

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

* Re: [PATCH v2] powerpc/mpic: Add DT option to skip readback after EOI
@ 2015-01-28 20:15     ` Segher Boessenkool
  0 siblings, 0 replies; 8+ messages in thread
From: Segher Boessenkool @ 2015-01-28 20:15 UTC (permalink / raw)
  To: Scott Wood; +Cc: linuxppc-dev, Bogdan Purcareata, linux-kernel

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

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

* Re: [PATCH v2] powerpc/mpic: Add DT option to skip readback after EOI
  2015-01-28 20:15     ` Segher Boessenkool
@ 2015-01-28 23:03       ` Scott Wood
  -1 siblings, 0 replies; 8+ messages in thread
From: Scott Wood @ 2015-01-28 23:03 UTC (permalink / raw)
  To: Segher Boessenkool; +Cc: Bogdan Purcareata, linuxppc-dev, linux-kernel

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



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

* Re: [PATCH v2] powerpc/mpic: Add DT option to skip readback after EOI
@ 2015-01-28 23:03       ` Scott Wood
  0 siblings, 0 replies; 8+ messages in thread
From: Scott Wood @ 2015-01-28 23:03 UTC (permalink / raw)
  To: Segher Boessenkool; +Cc: linuxppc-dev, Bogdan Purcareata, linux-kernel

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

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

end of thread, other threads:[~2015-01-29  1:37 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-01-27 15:19 [PATCH v2] powerpc/mpic: Add DT option to skip readback after EOI Bogdan Purcareata
2015-01-27 15:19 ` Bogdan Purcareata
2015-01-27 21:57 ` Scott Wood
2015-01-27 21:57   ` Scott Wood
2015-01-28 20:15   ` Segher Boessenkool
2015-01-28 20:15     ` Segher Boessenkool
2015-01-28 23:03     ` Scott Wood
2015-01-28 23:03       ` Scott Wood

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.