From 84855678add8aba927faf76bc2f130a40f94b6f7 Mon Sep 17 00:00:00 2001 From: Lukas Wunner Date: Fri, 15 May 2020 17:58:01 +0200 Subject: [PATCH 1/3] spi: Fix controller unregister order When an SPI controller unregisters, it unbinds all its slave devices. For this, their drivers may need to access the SPI bus, e.g. to quiesce interrupts. However since commit ffbbdd21329f ("spi: create a message queueing infrastructure"), spi_destroy_queue() is executed before unbinding the slaves. It sets ctlr->running = false, thereby preventing SPI bus access and causing unbinding of slave devices to fail. Fix by unbinding slaves before calling spi_destroy_queue(). Fixes: ffbbdd21329f ("spi: create a message queueing infrastructure") Signed-off-by: Lukas Wunner Cc: stable@vger.kernel.org # v3.4+ Cc: Linus Walleij Link: https://lore.kernel.org/r/8aaf9d44c153fe233b17bc2dec4eb679898d7e7b.1589557526.git.lukas@wunner.de Signed-off-by: Mark Brown --- drivers/spi/spi.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c index 531d1de85f7f..25eb96d329ab 100644 --- a/drivers/spi/spi.c +++ b/drivers/spi/spi.c @@ -2763,6 +2763,8 @@ void spi_unregister_controller(struct spi_controller *ctlr) struct spi_controller *found; int id = ctlr->bus_num; + device_for_each_child(&ctlr->dev, NULL, __unregister); + /* First make sure that this controller was ever added */ mutex_lock(&board_lock); found = idr_find(&spi_master_idr, id); @@ -2775,7 +2777,6 @@ void spi_unregister_controller(struct spi_controller *ctlr) list_del(&ctlr->list); mutex_unlock(&board_lock); - device_for_each_child(&ctlr->dev, NULL, __unregister); device_unregister(&ctlr->dev); /* free bus id */ mutex_lock(&board_lock); From 9dd277ff92d06f6aa95b39936ad83981d781f49b Mon Sep 17 00:00:00 2001 From: Lukas Wunner Date: Fri, 15 May 2020 17:58:02 +0200 Subject: [PATCH 2/3] 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 Cc: stable@vger.kernel.org # v3.13+ Link: https://lore.kernel.org/r/2397dd70cdbe95e0bc4da2b9fca0f31cb94e5aed.1589557526.git.lukas@wunner.de Signed-off-by: Mark Brown --- drivers/spi/spi-bcm2835.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/spi/spi-bcm2835.c b/drivers/spi/spi-bcm2835.c index 11c235879bb7..fd887a6492f4 100644 --- a/drivers/spi/spi-bcm2835.c +++ b/drivers/spi/spi-bcm2835.c @@ -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); From b9dd3f6d417258ad0beeb292a1bc74200149f15d Mon Sep 17 00:00:00 2001 From: Lukas Wunner Date: Fri, 15 May 2020 17:58:03 +0200 Subject: [PATCH 3/3] spi: bcm2835aux: Fix controller unregister order The BCM2835aux SPI driver uses devm_spi_register_master() on bind. As a consequence, on unbind, __device_release_driver() first invokes bcm2835aux_spi_remove() before unregistering the SPI controller via devres_release_all(). This order is incorrect: bcm2835aux_spi_remove() 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_master() 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_master(). Note that the struct spi_master as well as the driver-private data are not freed until after bcm2835aux_spi_remove() has finished, so accessing them is safe. Fixes: 1ea29b39f4c8 ("spi: bcm2835aux: add bcm2835 auxiliary spi device driver") Signed-off-by: Lukas Wunner Cc: stable@vger.kernel.org # v4.4+ Cc: Martin Sperl Link: https://lore.kernel.org/r/32f27f4d8242e4d75f9a53f7e8f1f77483b08669.1589557526.git.lukas@wunner.de Signed-off-by: Mark Brown --- drivers/spi/spi-bcm2835aux.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/spi/spi-bcm2835aux.c b/drivers/spi/spi-bcm2835aux.c index a2162ff56a12..c331efd6e86b 100644 --- a/drivers/spi/spi-bcm2835aux.c +++ b/drivers/spi/spi-bcm2835aux.c @@ -569,7 +569,7 @@ static int bcm2835aux_spi_probe(struct platform_device *pdev) goto out_clk_disable; } - err = devm_spi_register_master(&pdev->dev, master); + err = spi_register_master(master); if (err) { dev_err(&pdev->dev, "could not register SPI master: %d\n", err); goto out_clk_disable; @@ -593,6 +593,8 @@ static int bcm2835aux_spi_remove(struct platform_device *pdev) bcm2835aux_debugfs_remove(bs); + spi_unregister_master(master); + bcm2835aux_spi_reset_hw(bs); /* disable the HW block by releasing the clock */