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

From: Robin Murphy <robin.murphy@arm.com>
To: Lukas Wunner <lukas@wunner.de>
Cc: Florian Fainelli <f.fainelli@gmail.com>,
	Mark Brown <broonie@kernel.org>,
	"moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE" 
	<linux-arm-kernel@lists.infradead.org>,
	"open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" 
	<devicetree@vger.kernel.org>,
	Scott Branden <sbranden@broadcom.com>,
	Ray Jui <rjui@broadcom.com>,
	linux-kernel@vger.kernel.org,
	"open list:SPI SUBSYSTEM" <linux-spi@vger.kernel.org>,
	Rob Herring <robh+dt@kernel.org>,
	"maintainer:BROADCOM BCM281XX/BCM11XXX/BCM216XX ARM ARCHITE..." 
	<bcm-kernel-feedback-list@broadcom.com>,
	"moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE" 
	<linux-rpi-kernel@lists.infradead.org>,
	Martin Sperl <kernel@martin.sperl.org>,
	Nicolas Saenz Julienne <nsaenzjulienne@suse.de>
Subject: Re: [PATCH v2] spi: bcm2835: Enable shared interrupt support
Date: Mon, 15 Jun 2020 20:09:48 +0100	[thread overview]
Message-ID: <3d4fd3f9-9bde-90a8-bef5-9fc97cc9b363@arm.com> (raw)
In-Reply-To: <20200608114148.4bau4mdcvwgf25ut@wunner.de>

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.