From ef8a185f6f1a2e91c55a2c1dfc986629e028fc15 Mon Sep 17 00:00:00 2001 From: Sasha Levin Date: Mon, 15 Jun 2020 21:55:39 -0400 Subject: [PATCH] Fixes for 4.4 Signed-off-by: Sasha Levin --- queue-4.4/series | 6 ++ ...2835-fix-controller-unregister-order.patch | 66 ++++++++++++++++ ...i-dw-fix-controller-unregister-order.patch | 69 +++++++++++++++++ .../spi-dw-fix-possible-race-condition.patch | 55 ++++++++++++++ .../spi-fix-controller-unregister-order.patch | 52 +++++++++++++ ...ssign-dummy-value-in-spi_unregister_.patch | 52 +++++++++++++ ...a2xx-fix-controller-unregister-order.patch | 76 +++++++++++++++++++ 7 files changed, 376 insertions(+) create mode 100644 queue-4.4/spi-bcm2835-fix-controller-unregister-order.patch create mode 100644 queue-4.4/spi-dw-fix-controller-unregister-order.patch create mode 100644 queue-4.4/spi-dw-fix-possible-race-condition.patch create mode 100644 queue-4.4/spi-fix-controller-unregister-order.patch create mode 100644 queue-4.4/spi-no-need-to-assign-dummy-value-in-spi_unregister_.patch create mode 100644 queue-4.4/spi-pxa2xx-fix-controller-unregister-order.patch diff --git a/queue-4.4/series b/queue-4.4/series index 1617a12dc3b..bf7652fb176 100644 --- a/queue-4.4/series +++ b/queue-4.4/series @@ -23,3 +23,9 @@ x86-speculation-change-misspelled-stipb-to-stibp.patch x86-speculation-add-support-for-stibp-always-on-pref.patch x86-speculation-avoid-force-disabling-ibpb-based-on-.patch x86-speculation-pr_spec_force_disable-enforcement-fo.patch +spi-dw-fix-possible-race-condition.patch +spi-dw-fix-controller-unregister-order.patch +spi-no-need-to-assign-dummy-value-in-spi_unregister_.patch +spi-fix-controller-unregister-order.patch +spi-pxa2xx-fix-controller-unregister-order.patch +spi-bcm2835-fix-controller-unregister-order.patch diff --git a/queue-4.4/spi-bcm2835-fix-controller-unregister-order.patch b/queue-4.4/spi-bcm2835-fix-controller-unregister-order.patch new file mode 100644 index 00000000000..cafc3192ee8 --- /dev/null +++ b/queue-4.4/spi-bcm2835-fix-controller-unregister-order.patch @@ -0,0 +1,66 @@ +From 96624f7bb8d16a983433f4da7f42d18a6acfae12 Mon Sep 17 00:00:00 2001 +From: Sasha Levin +Date: Fri, 15 May 2020 17:58:02 +0200 +Subject: spi: bcm2835: Fix controller unregister order + +From: Lukas Wunner + +[ Upstream commit 9dd277ff92d06f6aa95b39936ad83981d781f49b ] + +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 +Signed-off-by: Sasha Levin +--- + 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 25daebd6f410..27680b336454 100644 +--- a/drivers/spi/spi-bcm2835.c ++++ b/drivers/spi/spi-bcm2835.c +@@ -798,7 +798,7 @@ static int bcm2835_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; +@@ -818,6 +818,8 @@ static int bcm2835_spi_remove(struct platform_device *pdev) + struct spi_master *master = platform_get_drvdata(pdev); + struct bcm2835_spi *bs = spi_master_get_devdata(master); + ++ spi_unregister_master(master); ++ + /* Clear FIFOs, and disable the HW block */ + bcm2835_wr(bs, BCM2835_SPI_CS, + BCM2835_SPI_CS_CLEAR_RX | BCM2835_SPI_CS_CLEAR_TX); +-- +2.25.1 + diff --git a/queue-4.4/spi-dw-fix-controller-unregister-order.patch b/queue-4.4/spi-dw-fix-controller-unregister-order.patch new file mode 100644 index 00000000000..2a41fe60707 --- /dev/null +++ b/queue-4.4/spi-dw-fix-controller-unregister-order.patch @@ -0,0 +1,69 @@ +From cb170a54702886bf8a9e9f16103dd233dd93d63d Mon Sep 17 00:00:00 2001 +From: Sasha Levin +Date: Mon, 25 May 2020 14:25:01 +0200 +Subject: spi: dw: Fix controller unregister order + +From: Lukas Wunner + +[ Upstream commit ca8b19d61e3fce5d2d7790cde27a0b57bcb3f341 ] + +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. + +Fixes: 04f421e7b0b1 ("spi: dw: use managed resources") +Signed-off-by: Lukas Wunner +Reviewed-by: Andy Shevchenko +Cc: stable@vger.kernel.org # v3.14+ +Cc: Baruch Siach +Link: https://lore.kernel.org/r/3fff8cb8ae44a9893840d0688be15bb88c090a14.1590408496.git.lukas@wunner.de +Signed-off-by: Mark Brown +Signed-off-by: Sasha Levin +--- + drivers/spi/spi-dw.c | 4 +++- + 1 file changed, 3 insertions(+), 1 deletion(-) + +diff --git a/drivers/spi/spi-dw.c b/drivers/spi/spi-dw.c +index 61a951549eba..295249843e7c 100644 +--- a/drivers/spi/spi-dw.c ++++ b/drivers/spi/spi-dw.c +@@ -534,7 +534,7 @@ int dw_spi_add_host(struct device *dev, struct dw_spi *dws) + } + } + +- ret = devm_spi_register_master(dev, master); ++ ret = spi_register_master(master); + if (ret) { + dev_err(&master->dev, "problem registering spi master\n"); + goto err_dma_exit; +@@ -558,6 +558,8 @@ void dw_spi_remove_host(struct dw_spi *dws) + { + dw_spi_debugfs_remove(dws); + ++ spi_unregister_master(dws->master); ++ + if (dws->dma_ops && dws->dma_ops->dma_exit) + dws->dma_ops->dma_exit(dws); + +-- +2.25.1 + diff --git a/queue-4.4/spi-dw-fix-possible-race-condition.patch b/queue-4.4/spi-dw-fix-possible-race-condition.patch new file mode 100644 index 00000000000..3f4009cb4ff --- /dev/null +++ b/queue-4.4/spi-dw-fix-possible-race-condition.patch @@ -0,0 +1,55 @@ +From b56b480bd67ed200b45ff15e74e07ecb51e259ae Mon Sep 17 00:00:00 2001 +From: Sasha Levin +Date: Mon, 15 Jun 2020 17:28:08 -0400 +Subject: spi: dw: fix possible race condition + +[ Upstream commit 66b19d762378785d1568b5650935205edfeb0503 ] + +It is possible to get an interrupt as soon as it is requested. dw_spi_irq +does spi_controller_get_devdata(master) and expects it to be different than +NULL. However, spi_controller_set_devdata() is called after request_irq(), +resulting in the following crash: + +CPU 0 Unable to handle kernel paging request at virtual address 00000030, epc == 8058e09c, ra == 8018ff90 +[...] +Call Trace: +[<8058e09c>] dw_spi_irq+0x8/0x64 +[<8018ff90>] __handle_irq_event_percpu+0x70/0x1d4 +[<80190128>] handle_irq_event_percpu+0x34/0x8c +[<801901c4>] handle_irq_event+0x44/0x80 +[<801951a8>] handle_level_irq+0xdc/0x194 +[<8018f580>] generic_handle_irq+0x38/0x50 +[<804c6924>] ocelot_irq_handler+0x104/0x1c0 + +Signed-off-by: Alexandre Belloni +Reviewed-by: Andy Shevchenko +Signed-off-by: Mark Brown +Signed-off-by: Sasha Levin +--- + drivers/spi/spi-dw.c | 3 ++- + 1 file changed, 2 insertions(+), 1 deletion(-) + +diff --git a/drivers/spi/spi-dw.c b/drivers/spi/spi-dw.c +index 5688591e9cd3..61a951549eba 100644 +--- a/drivers/spi/spi-dw.c ++++ b/drivers/spi/spi-dw.c +@@ -501,6 +501,8 @@ int dw_spi_add_host(struct device *dev, struct dw_spi *dws) + snprintf(dws->name, sizeof(dws->name), "dw_spi%d", dws->bus_num); + spin_lock_init(&dws->buf_lock); + ++ spi_master_set_devdata(master, dws); ++ + ret = request_irq(dws->irq, dw_spi_irq, IRQF_SHARED, dws->name, master); + if (ret < 0) { + dev_err(dev, "can not get IRQ\n"); +@@ -532,7 +534,6 @@ int dw_spi_add_host(struct device *dev, struct dw_spi *dws) + } + } + +- spi_master_set_devdata(master, dws); + ret = devm_spi_register_master(dev, master); + if (ret) { + dev_err(&master->dev, "problem registering spi master\n"); +-- +2.25.1 + diff --git a/queue-4.4/spi-fix-controller-unregister-order.patch b/queue-4.4/spi-fix-controller-unregister-order.patch new file mode 100644 index 00000000000..f864b9dd5b5 --- /dev/null +++ b/queue-4.4/spi-fix-controller-unregister-order.patch @@ -0,0 +1,52 @@ +From af9adff04d45f726c49bc1be4a401877e627adf3 Mon Sep 17 00:00:00 2001 +From: Sasha Levin +Date: Fri, 15 May 2020 17:58:01 +0200 +Subject: spi: Fix controller unregister order + +From: Lukas Wunner + +[ Upstream commit 84855678add8aba927faf76bc2f130a40f94b6f7 ] + +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 +Signed-off-by: Sasha Levin +--- + 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 e5460d84ed08..57001f8f727a 100644 +--- a/drivers/spi/spi.c ++++ b/drivers/spi/spi.c +@@ -1922,11 +1922,12 @@ void spi_unregister_master(struct spi_master *master) + dev_err(&master->dev, "queue remove failed\n"); + } + ++ device_for_each_child(&master->dev, NULL, __unregister); ++ + mutex_lock(&board_lock); + list_del(&master->list); + mutex_unlock(&board_lock); + +- device_for_each_child(&master->dev, NULL, __unregister); + device_unregister(&master->dev); + } + EXPORT_SYMBOL_GPL(spi_unregister_master); +-- +2.25.1 + diff --git a/queue-4.4/spi-no-need-to-assign-dummy-value-in-spi_unregister_.patch b/queue-4.4/spi-no-need-to-assign-dummy-value-in-spi_unregister_.patch new file mode 100644 index 00000000000..9d95ec9089c --- /dev/null +++ b/queue-4.4/spi-no-need-to-assign-dummy-value-in-spi_unregister_.patch @@ -0,0 +1,52 @@ +From 8b6fa39365e006a84b184c900286feba7ec65ead Mon Sep 17 00:00:00 2001 +From: Sasha Levin +Date: Sat, 15 Jun 2019 20:41:35 +0300 +Subject: spi: No need to assign dummy value in spi_unregister_controller() +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +From: Andy Shevchenko + +[ Upstream commit ebc37af5e0a134355ea2b62ed4141458bdbd5389 ] + +The device_for_each_child() doesn't require the returned value to be checked. +Thus, drop the dummy variable completely and have no warning anymore: + +drivers/spi/spi.c: In function ‘spi_unregister_controller’: +drivers/spi/spi.c:2480:6: warning: variable ‘dummy’ set but not used [-Wunused-but-set-variable] + int dummy; + ^~~~~ + +Signed-off-by: Andy Shevchenko +Signed-off-by: Mark Brown +Signed-off-by: Sasha Levin +--- + drivers/spi/spi.c | 4 +--- + 1 file changed, 1 insertion(+), 3 deletions(-) + +diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c +index c132c676df3a..e5460d84ed08 100644 +--- a/drivers/spi/spi.c ++++ b/drivers/spi/spi.c +@@ -1917,8 +1917,6 @@ static int __unregister(struct device *dev, void *null) + */ + void spi_unregister_master(struct spi_master *master) + { +- int dummy; +- + if (master->queued) { + if (spi_destroy_queue(master)) + dev_err(&master->dev, "queue remove failed\n"); +@@ -1928,7 +1926,7 @@ void spi_unregister_master(struct spi_master *master) + list_del(&master->list); + mutex_unlock(&board_lock); + +- dummy = device_for_each_child(&master->dev, NULL, __unregister); ++ device_for_each_child(&master->dev, NULL, __unregister); + device_unregister(&master->dev); + } + EXPORT_SYMBOL_GPL(spi_unregister_master); +-- +2.25.1 + diff --git a/queue-4.4/spi-pxa2xx-fix-controller-unregister-order.patch b/queue-4.4/spi-pxa2xx-fix-controller-unregister-order.patch new file mode 100644 index 00000000000..e46124fb939 --- /dev/null +++ b/queue-4.4/spi-pxa2xx-fix-controller-unregister-order.patch @@ -0,0 +1,76 @@ +From 500c06bb92ed48419f7d32500d54244908e6b902 Mon Sep 17 00:00:00 2001 +From: Sasha Levin +Date: Mon, 25 May 2020 14:25:02 +0200 +Subject: spi: pxa2xx: Fix controller unregister order + +From: Lukas Wunner + +[ Upstream commit 32e5b57232c0411e7dea96625c415510430ac079 ] + +The PXA2xx SPI driver uses devm_spi_register_controller() on bind. +As a consequence, on unbind, __device_release_driver() first invokes +pxa2xx_spi_remove() before unregistering the SPI controller via +devres_release_all(). + +This order is incorrect: pxa2xx_spi_remove() disables 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 pxa2xx_spi_remove(), 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. + +The improper use of devm_spi_register_controller() was introduced in 2013 +by commit a807fcd090d6 ("spi: pxa2xx: use devm_spi_register_master()"), +but all earlier versions of the driver going back to 2006 were likewise +broken because they invoked spi_unregister_master() at the end of +pxa2xx_spi_remove(), rather than at the beginning. + +Fixes: e0c9905e87ac ("[PATCH] SPI: add PXA2xx SSP SPI Driver") +Signed-off-by: Lukas Wunner +Reviewed-by: Andy Shevchenko +Cc: stable@vger.kernel.org # v2.6.17+ +Cc: Tsuchiya Yuto +Link: https://bugzilla.kernel.org/show_bug.cgi?id=206403#c1 +Link: https://lore.kernel.org/r/834c446b1cf3284d2660f1bee1ebe3e737cd02a9.1590408496.git.lukas@wunner.de +Signed-off-by: Mark Brown +Signed-off-by: Sasha Levin +--- + drivers/spi/spi-pxa2xx.c | 4 +++- + 1 file changed, 3 insertions(+), 1 deletion(-) + +diff --git a/drivers/spi/spi-pxa2xx.c b/drivers/spi/spi-pxa2xx.c +index 96ed01cb6489..cfcc5a9a5cc9 100644 +--- a/drivers/spi/spi-pxa2xx.c ++++ b/drivers/spi/spi-pxa2xx.c +@@ -1605,7 +1605,7 @@ static int pxa2xx_spi_probe(struct platform_device *pdev) + + /* Register with the SPI framework */ + platform_set_drvdata(pdev, drv_data); +- status = devm_spi_register_master(&pdev->dev, master); ++ status = spi_register_master(master); + if (status != 0) { + dev_err(&pdev->dev, "problem registering spi master\n"); + goto out_error_clock_enabled; +@@ -1635,6 +1635,8 @@ static int pxa2xx_spi_remove(struct platform_device *pdev) + + pm_runtime_get_sync(&pdev->dev); + ++ spi_unregister_master(drv_data->master); ++ + /* Disable the SSP at the peripheral and SOC level */ + pxa2xx_spi_write(drv_data, SSCR0, 0); + clk_disable_unprepare(ssp->clk); +-- +2.25.1 + -- 2.47.3