Skip to contentSkip to conversation

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

PATCH [email protected] 2 participants 2 messages Jun 15, 2020

Subsystems (2)
lore
Share & tools
lore

[email protected]

author maintainer
#0 lore raw

The patch below does not apply to the 4.14-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 ca8b19d61e3fce5d2d7790cde27a0b57bcb3f341 Mon Sep 17 00:00:00 2001
From: Lukas Wunner <[email protected]>
Date: Mon, 25 May 2020 14:25:01 +0200
Subject: [PATCH] spi: dw: Fix controller unregister order

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

This order is incorrect: dw_spi_remove_host() shuts down the chip, rendering the SPI bus inaccessible even though the SPI controller is still registered. When the SPI controller is subsequently unregistered, it unbinds all its slave devices. Because their drivers cannot access the SPI bus, e.g. to quiesce interrupts, the slave devices may be left in an improper state.

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

Fix by reverting to the non-devm variant of spi_register_controller().

An alternative approach would be to use device-managed functions for all steps in dw_spi_remove_host(), e.g. by calling devm_add_action_or_reset() on probe. However that approach would add more LoC to the driver and it wouldn't lend itself as well to backporting to stable.

drivers/spi/spi-dw.c +3 -1
index 31e3f866d11a..780ffad64a91 100644
@@ -526,7 +526,7 @@ int dw_spi_add_host(struct device *dev, struct dw_spi *dws)
 		}
 	}
 
-	ret = devm_spi_register_controller(dev, master);
+	ret = spi_register_controller(master);
 	if (ret) {
 		dev_err(&master->dev, "problem registering spi master\n");
 		goto err_dma_exit;
@@ -550,6 +550,8 @@ void dw_spi_remove_host(struct dw_spi *dws)
 {
 	dw_spi_debugfs_remove(dws);
 
+	spi_unregister_controller(dws->master);
+
 	if (dws->dma_ops && dws->dma_ops->dma_exit)
 		dws->dma_ops->dma_exit(dws);
 
maintainer
#1 lore raw
Quoted 40 lines · On Mon, Jun 15, 2020 at 05:20:14PM +0200, [email protected] wrote:

The patch below does not apply to the 4.14-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 ca8b19d61e3fce5d2d7790cde27a0b57bcb3f341 Mon Sep 17 00:00:00 2001
From: Lukas Wunner <[email protected]>
Date: Mon, 25 May 2020 14:25:01 +0200
Subject: [PATCH] spi: dw: Fix controller unregister order

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

This order is incorrect: dw_spi_remove_host() shuts down the chip, rendering the SPI bus inaccessible even though the SPI controller is still registered. When the SPI controller is subsequently unregistered, it unbinds all its slave devices. Because their drivers cannot access the SPI bus, e.g. to quiesce interrupts, the slave devices may be left in an improper state.

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

Fix by reverting to the non-devm variant of spi_register_controller().

An alternative approach would be to use device-managed functions for all steps in dw_spi_remove_host(), e.g. by calling devm_add_action_or_reset() on probe. However that approach would add more LoC to the driver and it wouldn't lend itself as well to backporting to stable.

Quoted 7 lines

I took this additional patch:

66b19d762378 ("spi: dw: fix possible race condition")

And worked around the renames in :

721483e28889 ("spi: dw: Convert to generalized SPI controller API")

And queued these two patches for 4.14, 4.9, and 4.4.

Signature · 2 lines
Thanks,
Sasha