Skip to contentSkip to conversation

FAILED: patch "[PATCH] spi: bcm2835: Fix controller unregister order" failed to apply to 4.19-stable tree

PATCH [email protected] 3 participants 3 messages Jun 15, 2020 – Jun 16, 2020

Subsystems (4)
Stable branch SPI Broadcom BCM2711/BCM2835 Arm architecture Broadcom BCM281XX/BCM11XXX/BCM216XX Arm architecture
lore
Share & tools
lore

[email protected]

author maintainer
#0 lore raw

The patch below does not apply to the 4.19-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.

thanks,

greg k-h

------------------ original commit in Linus's tree ------------------

From 9dd277ff92d06f6aa95b39936ad83981d781f49b Mon Sep 17 00:00:00 2001
From: Lukas Wunner <[email protected]>
Date: Fri, 15 May 2020 17:58:02 +0200
Subject: [PATCH] spi: bcm2835: Fix controller unregister order

The BCM2835 SPI driver uses devm_spi_register_controller() on bind. As a consequence, on unbind, __device_release_driver() first invokes bcm2835_spi_remove() before unregistering the SPI controller via devres_release_all().

This order is incorrect: bcm2835_spi_remove() tears down the DMA channels and turns off the SPI controller, including its interrupts and clock. The SPI controller is thus no longer usable.

When the SPI controller is subsequently unregistered, it unbinds all its slave devices. If their drivers need to access the SPI bus, e.g. to quiesce their interrupts, unbinding will fail.

As a rule, devm_spi_register_controller() must not be used if the ->remove() hook performs teardown steps which shall be performed after unbinding of slaves.

Fix by using the non-devm variant spi_register_controller(). Note that the struct spi_controller as well as the driver-private data are not freed until after bcm2835_spi_remove() has finished, so accessing them is safe.

Fixes 247263dba208 ("spi: bcm2835: use devm_spi_register_master()")
Signed-off-by Lukas Wunner <[email protected]>
Cc [email protected] # v3.13+
Link https://lore.kernel.org/r/2397dd70cdbe95e0bc4da2b9fca0f31cb94e5aed.1589557526.git.lukas@wunner.de open here
Signed-off-by Mark Brown <[email protected]>
drivers/spi/spi-bcm2835.c +3 -1
index 11c235879bb7..fd887a6492f4 100644
@@ -1347,7 +1347,7 @@ static int bcm2835_spi_probe(struct platform_device *pdev)
 		goto out_dma_release;
 	}
 
-	err = devm_spi_register_controller(&pdev->dev, ctlr);
+	err = spi_register_controller(ctlr);
 	if (err) {
 		dev_err(&pdev->dev, "could not register SPI controller: %d\n",
 			err);

@@ -1374,6 +1374,8 @@ static int bcm2835_spi_remove(struct platform_device *pdev)
 
 	bcm2835_debugfs_remove(bs);
 
+	spi_unregister_controller(ctlr);
+
 	/* Clear FIFOs, and disable the HW block */
 	bcm2835_wr(bs, BCM2835_SPI_CS,
 		   BCM2835_SPI_CS_CLEAR_RX | BCM2835_SPI_CS_CLEAR_TX);
maintainer
#1 lore raw
Quoted 39 lines · On Mon, Jun 15, 2020 at 05:22:00PM +0200, [email protected] wrote:

The patch below does not apply to the 4.19-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.

thanks,

greg k-h

------------------ original commit in Linus's tree ------------------

From 9dd277ff92d06f6aa95b39936ad83981d781f49b Mon Sep 17 00:00:00 2001
From: Lukas Wunner <[email protected]>
Date: Fri, 15 May 2020 17:58:02 +0200
Subject: [PATCH] spi: bcm2835: Fix controller unregister order

The BCM2835 SPI driver uses devm_spi_register_controller() on bind. As a consequence, on unbind, __device_release_driver() first invokes bcm2835_spi_remove() before unregistering the SPI controller via devres_release_all().

This order is incorrect: bcm2835_spi_remove() tears down the DMA channels and turns off the SPI controller, including its interrupts and clock. The SPI controller is thus no longer usable.

When the SPI controller is subsequently unregistered, it unbinds all its slave devices. If their drivers need to access the SPI bus, e.g. to quiesce their interrupts, unbinding will fail.

As a rule, devm_spi_register_controller() must not be used if the ->remove() hook performs teardown steps which shall be performed after unbinding of slaves.

Fix by using the non-devm variant spi_register_controller(). Note that the struct spi_controller as well as the driver-private data are not freed until after bcm2835_spi_remove() has finished, so accessing them is safe.

Quoted 5 lines
Fixes 247263dba208 ("spi: bcm2835: use devm_spi_register_master()")
Signed-off-by Lukas Wunner <[email protected]>
Cc [email protected] # v3.13+
Link https://lore.kernel.org/r/2397dd70cdbe95e0bc4da2b9fca0f31cb94e5aed.1589557526.git.lukas@wunner.de open here
Signed-off-by Mark Brown <[email protected]>

more work around master -> controller rename. Queued for 4.19, 4.14, 4.9, and 4.4.

Signature · 2 lines
Thanks,
Sasha
#2 lore raw
Quoted 5 lines · Sasha Levin
Quoted 4 lines · On Mon, Jun 15, 2020 at 05:22:00PM +0200, [email protected] wrote:

The patch below does not apply to the 4.19-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.

[...]

Quoted 7 lines
Quoted 4 lines

From 9dd277ff92d06f6aa95b39936ad83981d781f49b Mon Sep 17 00:00:00 2001
From: Lukas Wunner <[email protected]>
Date: Fri, 15 May 2020 17:58:02 +0200
Subject: [PATCH] spi: bcm2835: Fix controller unregister order

more work around master -> controller rename. Queued for 4.19, 4.14, 4.9, and 4.4.

Thanks a lot Sasha, that has saved me a ton of work.

FWIW, you may also want to consider queueing bd59b343a9c9 ("irqchip/bcm2835: Quiesce IRQs left enabled by bootloader") which went into v5.7-rc1 and fixes random lockups and jittery interrupt handling if the FIQ is used on the Raspberry Pi.

(It is used by the Raspberry Pi Foundation's kernel, which is based on 4.19-stable and will be based on 5.4-stable in the future.)

Thanks again,

Lukas