Re: [PATCH v2] spi: bcm2835: Enable shared interrupt support

From: Robin Murphy
Date: Mon Jun 15 2020 - 15:09:55 EST


On 2020-06-08 12:41, Lukas Wunner wrote:
On Mon, Jun 08, 2020 at 12:11:11PM +0100, Robin Murphy wrote:
And all in code that has at least one obvious inefficiency left on
the table either way.

Care to submit a patch to overcome that inefficiency?

I'll have a quick go, but without any way to measure performance impact (or even test for correctness) I don't fancy going too deep based purely on disassembly and ARM11 cycle timings.

This thread truly epitomises Knuth's "premature optimisation" quote... ;)

The thread came about because it can be determined at compile time
whether the interrupt is going to be shared:

...which is exactly my point - "because it can be" is anything but proof that avoiding a trivial check makes enough measurable difference to justify putting in the effort to do so.

On the BCM2835 (Raspberry Pi 1), CONFIG_ARCH_MULTI_V6 is set and this
SoC doesn't have multiple bcm2835-spi instances, so no shared interrupt.

The question is how to discern BCM2836/BCM2837 (Raspberry Pi 2/3), which
do not have multiple instances, and BCM2711 (Raspberry Pi 4) which does.

Hmm, how much relative importance does that have? On a 700MHz ARM11 it's obviously desirable to spend as little time in the IRQ handler as possible in order to have time left to do anything else, but on the other SoCs even if the IRQ remains permanently asserted it can still only consume 25% of the available CPU capacity, at which point the impact of 2-3 cycles either way at 1GHz+ seems pretty much immeasurable.

The Raspberry Pi Foundation compiles BCM2711 kernels with CONFIG_ARM_LPAE=y,
but Florian considered that kludgy as a discriminator and opted for
runtime-detection via the compatible string instead. If you've got
a better idea please come forward.

Is "optimize shared IRQ support away if IS_ENABLED(CONFIG_ARCH_MULTI_V6),
else leave it in" the best we can do?

In all honesty I'm starting to think it seriously might be :)

Robin.