[v2] spi: bcm2835: enable shared interrupt support

Commit Message

Marc Kleine-Budde July 19, 2022, 10:53 a.m. UTC

From: Martin Sperl <[e-mail]>

BCM2711 shares an interrupt betweem 5 SPI interfaces (0, 3, 4, 5 & 6).
Another interrupt is shared between SPI1, SPI2 and UART1, which also
affects BCM2835/6/7. Acting on an interrupt intended for another
interface ought to be harmless (although potentially inefficient), but
it can cause this driver to crash - presumably because some critical
state is not ready.

Add a test to the spi-bcm2835 interrupt service routine that
interrupts are enabled on this interface to avoid the crash and
improve efficiency.

Signed-off-by: Martin Sperl <[e-mail]>
Link: https://github.com/raspberrypi/linux/issues/5048
Suggested-by: https://github.com/boe-pi
Co-developed-by: Phil Elwell <[e-mail]>
Signed-off-by: Phil Elwell <[e-mail]>
Cc: Mark Brown <[e-mail]>
Cc: Lukas Wunner <[e-mail]>
Signed-off-by: Marc Kleine-Budde <[e-mail]>
---
Hello,

I'm picking up the work of Martin Sperl et al. to bring the shared
interrupt support for the bcm2835 driver to mainline.

The original version of this patch was added in ecfbd3cf3b8b ("spi:
bcm2835: Enable shared interrupt support"), but later reverted as
d62069c22eda ("spi: bcm2835: Remove shared interrupt support").

Here the original version causes transfer timeouts, which lead to
driver crashes. This is fixed by first checking if the interrupts are
actually enabled, before serving them. The fix has been taken from the
rapi downstream repo and squahed into this commit.

The updated version of the patch successfully runs with concurrent SPI
accesses on SPI0 and SPI3 without problems.

regards,
Marc

Changes since v1: https://lore.kernel.org/all/[e-mail]
- check for if interrupts are enabled before serving IRQs

 drivers/spi/spi-bcm2835.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)


base-commit: a3fd35be0eda760610a63e179ad860189b890f0b
--
2.35.1

Comments

Mark Brown July 26, 2022, 11:17 a.m. UTC | #1

On Tue, 19 Jul 2022 12:53:05 +0200, Marc Kleine-Budde wrote:
> From: Martin Sperl <[e-mail]>
> 
> BCM2711 shares an interrupt betweem 5 SPI interfaces (0, 3, 4, 5 & 6).
> Another interrupt is shared between SPI1, SPI2 and UART1, which also
> affects BCM2835/6/7. Acting on an interrupt intended for another
> interface ought to be harmless (although potentially inefficient), but
> it can cause this driver to crash - presumably because some critical
> state is not ready.
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next

Thanks!

[1/1] spi: bcm2835: enable shared interrupt support
      commit: 89fcdd53c2528b8f0ed34553aaf9826fe63848b5

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

Patch

diff --git a/drivers/spi/spi-bcm2835.c b/drivers/spi/spi-bcm2835.c
index 775c0bf2f923..9bdb1b85ae08 100644
--- a/drivers/spi/spi-bcm2835.c
+++ b/drivers/spi/spi-bcm2835.c
@@ -372,6 +372,10 @@  static irqreturn_t bcm2835_spi_interrupt(int irq, void *dev_id)
 	struct bcm2835_spi *bs = dev_id;
 	u32 cs = bcm2835_rd(bs, BCM2835_SPI_CS);

+	/* Bail out early if interrupts are not enabled */
+	if (!(cs & BCM2835_SPI_CS_INTR))
+		return IRQ_NONE;
+
 	/*
 	 * An interrupt is signaled either if DONE is set (TX FIFO empty)
 	 * or if RXR is set (RX FIFO >= ¾ full).
@@ -1365,8 +1369,8 @@  static int bcm2835_spi_probe(struct platform_device *pdev)
 	bcm2835_wr(bs, BCM2835_SPI_CS,
 		   BCM2835_SPI_CS_CLEAR_RX | BCM2835_SPI_CS_CLEAR_TX);

-	err = devm_request_irq(&pdev->dev, bs->irq, bcm2835_spi_interrupt, 0,
-			       dev_name(&pdev->dev), bs);
+	err = devm_request_irq(&pdev->dev, bs->irq, bcm2835_spi_interrupt,
+			       IRQF_SHARED, dev_name(&pdev->dev), bs);
 	if (err) {
 		dev_err(&pdev->dev, "could not request IRQ: %d\n", err);
 		goto out_dma_release;