archive mirror
 help / color / mirror / Atom feed
From: Lukas Wunner <>
To: Thomas Gleixner <>,
	Jason Cooper <>,
	Marc Zyngier <>,
	"Nicolas Saenz Julienne" <>
Cc: Florian Fainelli <>,
	Ray Jui <>,
	Scott Branden <>,,,,,
	Serge Schneider <>,
	Kristina Brooks <>,
	Stefan Wahren <>,
	Matthias Brugger <>,
	Martin Sperl <>,
	Phil Elwell <>
Subject: [PATCH] irqchip/bcm2835: Quiesce IRQs left enabled by bootloader
Date: Fri, 7 Feb 2020 16:46:41 +0100	[thread overview]
Message-ID: <> (raw)

Customers of our "Revolution Pi" open source PLCs (which are based on
the Raspberry Pi) have reported random lockups as well as jittery eMMC,
UART and SPI latency.  We were able to reproduce the lockups in our lab
and hooked up a JTAG debugger:

It turns out that the USB controller's interrupt is already enabled when
the kernel boots.  All interrupts are disabled when the chip comes out
of power-on reset, according to the spec.  So apparently the bootloader
enables the interrupt but neglects to disable it before handing over
control to the kernel.

The bootloader is a closed source blob provided by the Raspberry Pi
Foundation.  Development of an alternative open source bootloader was
begun by Kristina Brooks but it's not fully functional yet.  Usage of
the blob is thus without alternative for the time being.

The Raspberry Pi Foundation's downstream kernel has a performance-
optimized USB driver (which we use on our Revolution Pi products).
The driver takes advantage of the FIQ fast interrupt.  Because the
regular USB interrupt was left enabled by the bootloader, both the
FIQ and the normal interrupt is enabled once the USB driver probes.

The spec has the following to say on simultaneously enabling the FIQ
and the normal interrupt of a peripheral:

"One interrupt source can be selected to be connected to the ARM FIQ
 input.  An interrupt which is selected as FIQ should have its normal
 interrupt enable bit cleared.  Otherwise a normal and an FIQ interrupt
 will be fired at the same time.  Not a good idea!"
page 110

On a multicore Raspberry Pi, the Foundation's kernel routes all normal
interrupts to CPU 0 and the FIQ to CPU 1.  Because both the FIQ and the
normal interrupt is enabled, a USB interrupt causes CPU 0 to spin in
bcm2836_chained_handle_irq() until the FIQ on CPU 1 has cleared it.
Interrupts with a lower priority than USB are starved as long.

That explains the jittery eMMC, UART and SPI latency:  On one occasion
I've seen CPU 0 blocked for no less than 2.9 msec.  Basically,
everything not USB takes a performance hit:  Whereas eMMC throughput
on a Compute Module 3 remains relatively constant at 23.5 MB/s with
this commit, it irregularly dips to 23.0 MB/s without this commit.

The lockups occur when CPU 0 receives a USB interrupt while holding a
lock which CPU 1 is trying to acquire while the FIQ is temporarily
disabled on CPU 1.

I've tested old releases of the Foundation's bootloader as far back as
1.20160202-1 and they all leave the USB interrupt enabled.  Still older
releases fail to boot a contemporary kernel on a Compute Module 1 or 3,
which are the only Raspberry Pi variants I have at my disposal for

Fix by disabling IRQs left enabled by the bootloader.  Although the
impact is most pronounced on the Foundation's downstream kernel,
it seems prudent to apply the fix to the upstream kernel to guard
against such mistakes in any present and future bootloader.

An alternative, though more convoluted approach would be to clear the
IRQD_IRQ_MASKED flag on all interrupts left enabled on boot.  Then the
first invocation of handle_level_irq() would mask and thereby quiesce
those interrupts.

Signed-off-by: Lukas Wunner <>
Cc: Serge Schneider <>
Cc: Kristina Brooks <>
 drivers/irqchip/irq-bcm2835.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/irqchip/irq-bcm2835.c b/drivers/irqchip/irq-bcm2835.c
index 418245d31921..0d9a5a7ebe2c 100644
--- a/drivers/irqchip/irq-bcm2835.c
+++ b/drivers/irqchip/irq-bcm2835.c
@@ -150,6 +150,13 @@ static int __init armctrl_of_init(struct device_node *node,
 		intc.enable[b] = base + reg_enable[b];
 		intc.disable[b] = base + reg_disable[b];
+		irq = readl(intc.enable[b]);
+		if (irq) {
+			writel(irq, intc.disable[b]);
+			pr_err(FW_BUG "Bootloader left irq enabled: "
+			       "bank %d irq %*pbl\n", b, IRQS_PER_BANK, &irq);
+		}
 		for (i = 0; i < bank_irqs[b]; i++) {
 			irq = irq_create_mapping(intc.domain, MAKE_HWIRQ(b, i));
 			BUG_ON(irq <= 0);

             reply	other threads:[~2020-02-07 15:47 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-02-07 15:46 Lukas Wunner [this message]
2020-02-07 16:11 ` [PATCH] irqchip/bcm2835: Quiesce IRQs left enabled by bootloader Marc Zyngier
2020-02-10  9:52   ` [PATCH v2] " Lukas Wunner
2020-02-12  4:47     ` Florian Fainelli
2020-02-12  8:13     ` Marc Zyngier
2020-02-12 12:36       ` Lukas Wunner
2020-02-12 12:55         ` Nicolas Saenz Julienne
2020-02-23 17:59         ` Stefan Wahren
2020-02-23 18:24           ` Lukas Wunner
2020-02-24  9:21             ` Stefan Wahren
2020-02-25  9:50               ` [PATCH v4] " Lukas Wunner
2020-03-29 20:26                 ` [tip: irq/core] " tip-bot2 for Lukas Wunner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \ \ \ \ \ \ \ \ \ \ \ \ \ \ \ \ \ \ \ \

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).