spi: fix controller deregistration (part 2/2)
Subsystems (17)
cover spi: fix controller deregistration (part 2/2)
Device managed registration generally only works if all involved resources are managed as otherwise resources may be disabled or freed while they are still in use.
This series fixes the SPI controller drivers that get this wrong by disabling resources such as clocks, DMA and interrupts while the controller (and its devices) are still registered, which can lead to issues like system errors due to unclocked accesses, NULL-pointer dereferences, hangs or just prevent SPI device drivers from doing I/O during during deregistration (e.g. to power down devices).
I decided to split these fixes in two parts consisting of 20 and 26 patches respectively in order not to spam the lists too much.
I've also prepared a follow-on series to convert the drivers here that do not yet use device managed controller allocation (which avoids taking extra references during deregistration).
After that it should be possible to change the SPI API so that it no longer drops a reference during deregistration without too much effort (cf. [1]).
Note that this series is based on spi/for-next which specifically has commit 1f8fd9490e31 ("spi: zynq-qspi: Simplify clock handling with devm_clk_get_enabled()") (which is not in the for-7.1 branch).
Johan
[1]
https://lore.kernel.org/lkml/[email protected]/
Johan Hovold (26):
spi: mt65xx: fix controller deregistration
spi: mtk-nor: fix controller deregistration
spi: mxs: fix controller deregistration
spi: npcm-pspi: fix controller deregistration
spi: omap2-mcspi: fix controller deregistration
spi: pic32: fix controller deregistration
spi: pic32-sqi: fix controller deregistration
spi: pl022: fix controller deregistration
spi: qup: fix controller deregistration
spi: rspi: fix controller deregistration
spi: s3c64xx: fix controller deregistration
spi: sh-hspi: fix controller deregistration
spi: sh-msiof: fix controller deregistration
spi: sifive: fix controller deregistration
spi: slave-mt27xx: fix controller deregistration
spi: sprd: fix controller deregistration
spi: st-ssc4: fix controller deregistration
spi: sun4i: fix controller deregistration
spi: sun6i: fix controller deregistration
spi: syncuacer: fix controller deregistration
spi: tegra114: fix controller deregistration
spi: tegra20-sflash: fix controller deregistration
spi: ti-qspi: fix controller deregistration
spi: uniphier: fix controller deregistration
spi: zynqmp-gqspi: fix controller deregistration
spi: zynq-qspi: fix controller deregistration
drivers/spi/spi-mt65xx.c | 4 +++- drivers/spi/spi-mtk-nor.c | 4 +++- drivers/spi/spi-mxs.c | 8 +++++++- drivers/spi/spi-npcm-pspi.c | 8 +++++++- drivers/spi/spi-omap2-mcspi.c | 8 +++++++- drivers/spi/spi-pic32-sqi.c | 8 +++++++- drivers/spi/spi-pic32.c | 11 ++++++++--- drivers/spi/spi-pl022.c | 8 +++++++- drivers/spi/spi-qup.c | 8 +++++++- drivers/spi/spi-rspi.c | 10 ++++++++-- drivers/spi/spi-s3c64xx.c | 4 +++- drivers/spi/spi-sh-hspi.c | 10 ++++++++-- drivers/spi/spi-sh-msiof.c | 10 ++++++++-- drivers/spi/spi-sifive.c | 8 +++++++- drivers/spi/spi-slave-mt27xx.c | 10 +++++++++- drivers/spi/spi-sprd.c | 8 ++++++-- drivers/spi/spi-st-ssc4.c | 8 +++++++- drivers/spi/spi-sun4i.c | 10 +++++++++- drivers/spi/spi-sun6i.c | 8 +++++++- drivers/spi/spi-synquacer.c | 8 +++++++- drivers/spi/spi-tegra114.c | 8 +++++++- drivers/spi/spi-tegra20-sflash.c | 8 +++++++- drivers/spi/spi-ti-qspi.c | 14 ++++++-------- drivers/spi/spi-uniphier.c | 8 +++++++- drivers/spi/spi-zynq-qspi.c | 15 +++++++++++---- drivers/spi/spi-zynqmp-gqspi.c | 4 +++- 26 files changed, 176 insertions(+), 42 deletions(-)
Signature · 1 line
2.52.0
PATCH 1/26 spi: mt65xx: fix controller deregistration
Make sure to deregister the controller before disabling underlying resources like clocks during driver unbind.
--- drivers/spi/spi-mt65xx.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
drivers/spi/spi-mt65xx.c +3 -1
@@ -1325,7 +1325,7 @@ static int mtk_spi_probe(struct platform_device *pdev)pm_runtime_enable (dev); - ret =devm_spi_register_controller (dev, host); + ret =spi_register_controller (host);if (ret) {pm_runtime_disable (dev);return dev_err_probe (dev, ret,"failed to register host\n" );
@@ -1340,6 +1340,8 @@ static void mtk_spi_remove(struct platform_device *pdev)struct mtk_spi *mdata =spi_controller_get_devdata (host);int ret; +spi_unregister_controller (host); +cpu_latency_qos_remove_request (&mdata->qos_request);if (mdata->use_spimem && !completion_done (&mdata->spimem_done))complete (&mdata->spimem_done);
Signature · 1 line
2.52.0
PATCH 2/26 spi: mtk-nor: fix controller deregistration
In reply to Johan HovoldMake sure to deregister the controller before disabling underlying resources like clocks during driver unbind.
--- drivers/spi/spi-mtk-nor.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
drivers/spi/spi-mtk-nor.c +3 -1
@@ -913,7 +913,7 @@ static int mtk_nor_probe(struct platform_device *pdev)pm_runtime_enable (&pdev->dev);pm_runtime_get_noresume (&pdev->dev); - ret =devm_spi_register_controller (&pdev->dev, ctlr); + ret =spi_register_controller (ctlr);if (ret <0 ) goto err_probe;
@@ -938,6 +938,8 @@ static void mtk_nor_remove(struct platform_device *pdev)struct spi_controller *ctlr =dev_get_drvdata (&pdev->dev);struct mtk_nor *sp =spi_controller_get_devdata (ctlr); +spi_unregister_controller (ctlr); +pm_runtime_disable (&pdev->dev);pm_runtime_set_suspended (&pdev->dev);pm_runtime_dont_use_autosuspend (&pdev->dev);
Signature · 1 line
2.52.0
PATCH 3/26 spi: mxs: fix controller deregistration
In reply to Johan HovoldMake sure to deregister the controller before releasing underlying resources like DMA during driver unbind.
--- drivers/spi/spi-mxs.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)
drivers/spi/spi-mxs.c +7 -1
@@ -619,7 +619,7 @@ static int mxs_spi_probe(struct platform_device *pdev)if (ret) goto out_pm_runtime_put; - ret =devm_spi_register_controller (&pdev->dev, host); + ret =spi_register_controller (host);if (ret) {dev_err (&pdev->dev,"Cannot register SPI host, %d\n" , ret); goto out_pm_runtime_put;
@@ -650,11 +650,17 @@ static void mxs_spi_remove(struct platform_device *pdev) spi =spi_controller_get_devdata (host); ssp = &spi->ssp; +spi_controller_get (host); + +spi_unregister_controller (host); +pm_runtime_disable (&pdev->dev);if (!pm_runtime_status_suspended (&pdev->dev))mxs_spi_runtime_suspend (&pdev->dev);dma_release_channel (ssp->dmach); + +spi_controller_put (host); }static struct platform_driver mxs_spi_driver = {
Signature · 1 line
2.52.0
PATCH 4/26 spi: npcm-pspi: fix controller deregistration
In reply to Johan HovoldMake sure to deregister the controller before disabling underlying resources like clocks during driver unbind.
--- drivers/spi/spi-npcm-pspi.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)
drivers/spi/spi-npcm-pspi.c +7 -1
@@ -413,7 +413,7 @@ static int npcm_pspi_probe(struct platform_device *pdev)/* set to default clock rate */ npcm_pspi_set_baudrate (priv,NPCM_PSPI_DEFAULT_CLK ); - ret =devm_spi_register_controller (&pdev->dev, host); + ret =spi_register_controller (host);if (ret) goto out_disable_clk;
@@ -434,8 +434,14 @@ static void npcm_pspi_remove(struct platform_device *pdev)struct spi_controller *host =platform_get_drvdata (pdev);struct npcm_pspi *priv =spi_controller_get_devdata (host); +spi_controller_get (host); + +spi_unregister_controller (host); +npcm_pspi_reset_hw (priv);clk_disable_unprepare (priv->clk); + +spi_controller_put (host); }static const struct of_device_id npcm_pspi_match[] = {
Signature · 1 line
2.52.0
PATCH 5/26 spi: omap2-mcspi: fix controller deregistration
In reply to Johan HovoldMake sure to deregister the controller before releasing underlying resources like DMA during driver unbind.
--- drivers/spi/spi-omap2-mcspi.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)
drivers/spi/spi-omap2-mcspi.c +7 -1
@@ -1592,7 +1592,7 @@ static int omap2_mcspi_probe(struct platform_device *pdev)if (status <0 ) goto disable_pm; - status =devm_spi_register_controller (&pdev->dev, ctlr); + status =spi_register_controller (ctlr);if (status <0 ) goto disable_pm;
@@ -1613,11 +1613,17 @@ static void omap2_mcspi_remove(struct platform_device *pdev)struct spi_controller *ctlr =platform_get_drvdata (pdev);struct omap2_mcspi *mcspi =spi_controller_get_devdata (ctlr); +spi_controller_get (ctlr); + +spi_unregister_controller (ctlr); +omap2_mcspi_release_dma (ctlr);pm_runtime_dont_use_autosuspend (mcspi->dev);pm_runtime_put_sync (mcspi->dev);pm_runtime_disable (&pdev->dev); + +spi_controller_put (ctlr); }/* work with hotplug and coldplug */
Signature · 1 line
2.52.0
PATCH 6/26 spi: pic32: fix controller deregistration
In reply to Johan HovoldMake sure to deregister the controller before releasing underlying resources like DMA during driver unbind.
--- drivers/spi/spi-pic32.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-)
drivers/spi/spi-pic32.c +8 -3
@@ -821,7 +821,7 @@ static int pic32_spi_probe(struct platform_device *pdev) }/* register host */ - ret =devm_spi_register_controller (&pdev->dev, host); + ret =spi_register_controller (host);if (ret) {dev_err (&host->dev,"failed registering spi host\n" ); goto err_bailout;
@@ -840,11 +840,16 @@ static int pic32_spi_probe(struct platform_device *pdev)static void pic32_spi_remove (struct platform_device *pdev) { -struct pic32_spi *pic32s; +struct pic32_spi *pic32s =platform_get_drvdata (pdev); + +spi_controller_get (pic32s->host); + +spi_unregister_controller (pic32s->host); - pic32s =platform_get_drvdata (pdev);pic32_spi_disable (pic32s);pic32_spi_dma_unprep (pic32s); + +spi_controller_put (pic32s->host); }static const struct of_device_id pic32_spi_of_match[] = {
Signature · 1 line
2.52.0
PATCH 7/26 spi: pic32-sqi: fix controller deregistration
In reply to Johan HovoldMake sure to deregister the controller before releasing underlying resources like DMA during driver unbind.
--- drivers/spi/spi-pic32-sqi.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)
drivers/spi/spi-pic32-sqi.c +7 -1
@@ -642,7 +642,7 @@ static int pic32_sqi_probe(struct platform_device *pdev) host->prepare_transfer_hardware = pic32_sqi_prepare_hardware; host->unprepare_transfer_hardware = pic32_sqi_unprepare_hardware; - ret =devm_spi_register_controller (&pdev->dev, host); + ret =spi_register_controller (host);if (ret) {dev_err (&host->dev,"failed registering spi host\n" );free_irq (sqi->irq, sqi);
@@ -665,9 +665,15 @@ static void pic32_sqi_remove(struct platform_device *pdev) {struct pic32_sqi *sqi =platform_get_drvdata (pdev); +spi_controller_get (sqi->host); + +spi_unregister_controller (sqi->host); +/* release resources */ free_irq (sqi->irq, sqi);ring_desc_ring_free (sqi); + +spi_controller_put (sqi->host); }static const struct of_device_id pic32_sqi_of_ids[] = {
Signature · 1 line
2.52.0
PATCH 8/26 spi: pl022: fix controller deregistration
In reply to Johan HovoldMake sure to deregister the controller before releasing underlying resources like DMA during driver unbind.
--- drivers/spi/spi-pl022.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)
drivers/spi/spi-pl022.c +7 -1
@@ -1956,7 +1956,7 @@ static int pl022_probe(struct amba_device *adev, const struct amba_id *id)/* Register with the SPI framework */ amba_set_drvdata (adev, pl022); - status =devm_spi_register_controller (&adev->dev, host); + status =spi_register_controller (host);if (status !=0 ) {dev_err_probe (&adev->dev, status,"problem registering spi host\n" );
@@ -1997,6 +1997,10 @@ pl022_remove(struct amba_device *adev)if (!pl022)return ; +spi_controller_get (pl022->host); + +spi_unregister_controller (pl022->host); +/* * undo pm_runtime_put() in probe. I assume that we're not * accessing the primecell here.
@@ -2008,6 +2012,8 @@ pl022_remove(struct amba_device *adev)pl022_dma_remove (pl022);amba_release_regions (adev); + +spi_controller_put (pl022->host); }#ifdef CONFIG_PM_SLEEP
Signature · 1 line
2.52.0
PATCH 9/26 spi: qup: fix controller deregistration
In reply to Johan HovoldMake sure to deregister the controller before disabling underlying resources like clocks during driver unbind.
--- drivers/spi/spi-qup.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)
drivers/spi/spi-qup.c +7 -1
@@ -1193,7 +1193,7 @@ static int spi_qup_probe(struct platform_device *pdev)pm_runtime_set_active (dev);pm_runtime_enable (dev); - ret =devm_spi_register_controller (dev, host); + ret =spi_register_controller (host);if (ret) goto disable_pm;
@@ -1320,6 +1320,10 @@ static void spi_qup_remove(struct platform_device *pdev)struct spi_qup *controller =spi_controller_get_devdata (host);int ret; +spi_controller_get (host); + +spi_unregister_controller (host); + ret =pm_runtime_get_sync (&pdev->dev);if (ret >=0 ) {
@@ -1339,6 +1343,8 @@ static void spi_qup_remove(struct platform_device *pdev)pm_runtime_put_noidle (&pdev->dev);pm_runtime_disable (&pdev->dev); + +spi_controller_put (host); }static const struct of_device_id spi_qup_dt_match[] = {
Signature · 1 line
2.52.0
PATCH 10/26 spi: rspi: fix controller deregistration
In reply to Johan HovoldMake sure to deregister the controller before releasing underlying resources like DMA during driver unbind.
--- drivers/spi/spi-rspi.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)
drivers/spi/spi-rspi.c +8 -2
@@ -1171,8 +1171,14 @@ static void rspi_remove(struct platform_device *pdev) {struct rspi_data *rspi =platform_get_drvdata (pdev); +spi_controller_get (rspi->ctlr); + +spi_unregister_controller (rspi->ctlr); +rspi_release_dma (rspi->ctlr);pm_runtime_disable (&pdev->dev); + +spi_controller_put (rspi->ctlr); }static const struct spi_ops rspi_ops = {
@@ -1376,9 +1382,9 @@ static int rspi_probe(struct platform_device *pdev)if (ret <0 )dev_warn (&pdev->dev,"DMA not available, using PIO\n" ); - ret =devm_spi_register_controller (&pdev->dev, ctlr); + ret =spi_register_controller (ctlr);if (ret <0 ) { -dev_err (&pdev->dev,"devm_spi_register_controller error.\n" ); +dev_err (&pdev->dev,"failed to register controller\n" ); goto error3; }
Signature · 1 line
2.52.0
PATCH 11/26 spi: s3c64xx: fix controller deregistration
In reply to Johan HovoldMake sure to deregister the controller before releasing underlying resources like DMA during driver unbind.
--- drivers/spi/spi-s3c64xx.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
drivers/spi/spi-s3c64xx.c +3 -1
@@ -1369,7 +1369,7 @@ static int s3c64xx_spi_probe(struct platform_device *pdev)S3C64XX_SPI_INT_TX_OVERRUN_EN |S3C64XX_SPI_INT_TX_UNDERRUN_EN , sdd->regs + S3C64XX_SPI_INT_EN); - ret =devm_spi_register_controller (&pdev->dev, host); + ret =spi_register_controller (host);if (ret !=0 ) {dev_err (&pdev->dev,"cannot register SPI host: %d\n" , ret); goto err_pm_put;
@@ -1399,6 +1399,8 @@ static void s3c64xx_spi_remove(struct platform_device *pdev)pm_runtime_get_sync (&pdev->dev); +spi_unregister_controller (host); +writel (0 , sdd->regs +S3C64XX_SPI_INT_EN );if (!is_polling (sdd)) {
Signature · 1 line
2.52.0
PATCH 12/26 spi: sh-hspi: fix controller deregistration
In reply to Johan HovoldMake sure to deregister the controller before releasing underlying resources like clocks during driver unbind.
--- drivers/spi/spi-sh-hspi.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)
drivers/spi/spi-sh-hspi.c +8 -2
@@ -257,9 +257,9 @@ static int hspi_probe(struct platform_device *pdev) ctlr->transfer_one_message = hspi_transfer_one_message; ctlr->bits_per_word_mask =SPI_BPW_MASK (8 ); - ret =devm_spi_register_controller (&pdev->dev, ctlr); + ret =spi_register_controller (ctlr);if (ret <0 ) { -dev_err (&pdev->dev,"devm_spi_register_controller error.\n" ); +dev_err (&pdev->dev,"failed to register controller\n" ); goto error2; }
@@ -279,9 +279,15 @@ static void hspi_remove(struct platform_device *pdev) {struct hspi_priv *hspi =platform_get_drvdata (pdev); +spi_controller_get (hspi->ctlr); + +spi_unregister_controller (hspi->ctlr); +pm_runtime_disable (&pdev->dev);clk_put (hspi->clk); + +spi_controller_put (hspi->ctlr); }static const struct of_device_id hspi_of_match[] = {
Signature · 1 line
2.52.0
PATCH 13/26 spi: sh-msiof: fix controller deregistration
In reply to Johan HovoldMake sure to deregister the controller before releasing underlying resources like DMA during driver unbind.
--- drivers/spi/spi-sh-msiof.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)
drivers/spi/spi-sh-msiof.c +8 -2
@@ -1289,9 +1289,9 @@ static int sh_msiof_spi_probe(struct platform_device *pdev)if (ret <0 )dev_warn (dev,"DMA not available, using PIO\n" ); - ret =devm_spi_register_controller (dev, ctlr); + ret =spi_register_controller (ctlr);if (ret <0 ) { -dev_err (dev,"devm_spi_register_controller error.\n" ); +dev_err (dev,"failed to register controller\n" ); goto err2; }
@@ -1309,8 +1309,14 @@ static void sh_msiof_spi_remove(struct platform_device *pdev) {struct sh_msiof_spi_priv *p =platform_get_drvdata (pdev); +spi_controller_get (p->ctlr); + +spi_unregister_controller (p->ctlr); +sh_msiof_release_dma (p);pm_runtime_disable (&pdev->dev); + +spi_controller_put (p->ctlr); }static const struct platform_device_id spi_driver_ids[] = {
Signature · 1 line
2.52.0
PATCH 14/26 spi: sifive: fix controller deregistration
In reply to Johan HovoldMake sure to deregister the controller before disabling underlying resources like interrupts during driver unbind.
Note that clocks were also disabled before the recent commit 140039c23aca ("spi: sifive: Simplify clock handling with devm_clk_get_enabled()").
--- drivers/spi/spi-sifive.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)
drivers/spi/spi-sifive.c +7 -1
@@ -392,7 +392,7 @@ static int sifive_spi_probe(struct platform_device *pdev)dev_info (&pdev->dev,"mapped; irq=%d, cs=%d\n" , irq, host->num_chipselect); - ret =devm_spi_register_controller (&pdev->dev, host); + ret =spi_register_controller (host);if (ret <0 ) {dev_err (&pdev->dev,"spi_register_host failed\n" ); goto put_host;
@@ -411,8 +411,14 @@ static void sifive_spi_remove(struct platform_device *pdev)struct spi_controller *host =platform_get_drvdata (pdev);struct sifive_spi *spi =spi_controller_get_devdata (host); +spi_controller_get (host); + +spi_unregister_controller (host); +/* Disable all the interrupts just in case */ sifive_spi_write (spi,SIFIVE_SPI_REG_IE ,0 ); + +spi_controller_put (host); }static int sifive_spi_suspend (struct device *dev)
Signature · 1 line
2.52.0
PATCH 15/26 spi: slave-mt27xx: fix controller deregistration
In reply to Johan HovoldMake sure to deregister the controller before disabling underlying resources like clocks (by disabling runtime PM) during driver unbind.
--- drivers/spi/spi-slave-mt27xx.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)
drivers/spi/spi-slave-mt27xx.c +9 -1
@@ -453,7 +453,7 @@ static int mtk_spi_slave_probe(struct platform_device *pdev)pm_runtime_enable (&pdev->dev); - ret =devm_spi_register_controller (&pdev->dev, ctlr); + ret =spi_register_controller (ctlr);clk_disable_unprepare (mdata->spi_clk);if (ret) { dev_err(&pdev->dev,
@@ -473,7 +473,15 @@ static int mtk_spi_slave_probe(struct platform_device *pdev)static void mtk_spi_slave_remove (struct platform_device *pdev) { +struct spi_controller *ctlr =platform_get_drvdata (pdev); + +spi_controller_get (ctlr); + +spi_unregister_controller (ctlr); +pm_runtime_disable (&pdev->dev); + +spi_controller_put (ctlr); }#ifdef CONFIG_PM_SLEEP
Signature · 1 line
2.52.0
PATCH 16/26 spi: sprd: fix controller deregistration
In reply to Johan HovoldMake sure to deregister the controller before disabling underlying resources like clocks during driver unbind.
Note that the controller is suspended before disabling and releasing resources since commit de082d866cce ("spi: sprd: Add the SPI irq function for the SPI DMA mode") which avoids issues like unclocked accesses but prevents SPI device drivers from doing I/O during deregistration.
--- drivers/spi/spi-sprd.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)
drivers/spi/spi-sprd.c +6 -2
@@ -977,7 +977,7 @@ static int sprd_spi_probe(struct platform_device *pdev) goto err_rpm_put; } - ret =devm_spi_register_controller (&pdev->dev, sctlr); + ret =spi_register_controller (sctlr);if (ret) goto err_rpm_put;
@@ -1008,7 +1008,9 @@ static void sprd_spi_remove(struct platform_device *pdev)if (ret <0 )dev_err (ss->dev,"failed to resume SPI controller\n" ); -spi_controller_suspend (sctlr); +spi_controller_get (sctlr); + +spi_unregister_controller (sctlr);if (ret >=0 ) {if (ss->dma.enable)
@@ -1017,6 +1019,8 @@ static void sprd_spi_remove(struct platform_device *pdev) }pm_runtime_put_noidle (&pdev->dev);pm_runtime_disable (&pdev->dev); + +spi_controller_put (sctlr); }static int __maybe_unused sprd_spi_runtime_suspend (struct device *dev)
Signature · 1 line
2.52.0
PATCH 17/26 spi: st-ssc4: fix controller deregistration
In reply to Johan HovoldMake sure to deregister the controller before disabling underlying resources like clocks during driver unbind.
--- drivers/spi/spi-st-ssc4.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)
drivers/spi/spi-st-ssc4.c +7 -1
@@ -349,7 +349,7 @@ static int spi_st_probe(struct platform_device *pdev)platform_set_drvdata (pdev, host); - ret =devm_spi_register_controller (&pdev->dev, host); + ret =spi_register_controller (host);if (ret) {dev_err (&pdev->dev,"Failed to register host\n" ); goto rpm_disable;
@@ -371,10 +371,16 @@ static void spi_st_remove(struct platform_device *pdev)struct spi_controller *host =platform_get_drvdata (pdev);struct spi_st *spi_st =spi_controller_get_devdata (host); +spi_controller_get (host); + +spi_unregister_controller (host); +pm_runtime_disable (&pdev->dev);clk_disable_unprepare (spi_st->clk); +spi_controller_put (host); +pinctrl_pm_select_sleep_state (&pdev->dev); }
Signature · 1 line
2.52.0
PATCH 18/26 spi: sun4i: fix controller deregistration
In reply to Johan HovoldMake sure to deregister the controller before disabling underlying resources like clocks during driver unbind.
--- drivers/spi/spi-sun4i.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)
drivers/spi/spi-sun4i.c +9 -1
@@ -504,7 +504,7 @@ static int sun4i_spi_probe(struct platform_device *pdev)pm_runtime_enable (&pdev->dev);pm_runtime_idle (&pdev->dev); - ret =devm_spi_register_controller (&pdev->dev, host); + ret =spi_register_controller (host);if (ret) {dev_err (&pdev->dev,"cannot register SPI host\n" ); goto err_pm_disable;
@@ -522,7 +522,15 @@ static int sun4i_spi_probe(struct platform_device *pdev)static void sun4i_spi_remove (struct platform_device *pdev) { +struct spi_controller *host =platform_get_drvdata (pdev); + +spi_controller_get (host); + +spi_unregister_controller (host); +pm_runtime_force_suspend (&pdev->dev); + +spi_controller_put (host); }static const struct of_device_id sun4i_spi_match[] = {
Signature · 1 line
2.52.0
PATCH 19/26 spi: sun6i: fix controller deregistration
In reply to Johan HovoldMake sure to deregister the controller before disabling underlying resources like clocks during driver unbind.
--- drivers/spi/spi-sun6i.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)
drivers/spi/spi-sun6i.c +7 -1
@@ -742,7 +742,7 @@ static int sun6i_spi_probe(struct platform_device *pdev)pm_runtime_set_active (&pdev->dev);pm_runtime_enable (&pdev->dev); - ret =devm_spi_register_controller (&pdev->dev, host); + ret =spi_register_controller (host);if (ret) {dev_err (&pdev->dev,"cannot register SPI host\n" ); goto err_pm_disable;
@@ -768,12 +768,18 @@ static void sun6i_spi_remove(struct platform_device *pdev) {struct spi_controller *host =platform_get_drvdata (pdev); +spi_controller_get (host); + +spi_unregister_controller (host); +pm_runtime_force_suspend (&pdev->dev);if (host->dma_tx)dma_release_channel (host->dma_tx);if (host->dma_rx)dma_release_channel (host->dma_rx); + +spi_controller_put (host); }static const struct sun6i_spi_cfg sun6i_a31_spi_cfg = {
Signature · 1 line
2.52.0
PATCH 20/26 spi: syncuacer: fix controller deregistration
In reply to Johan HovoldMake sure to deregister the controller before disabling underlying resources like clocks during driver unbind.
--- drivers/spi/spi-synquacer.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)
drivers/spi/spi-synquacer.c +7 -1
@@ -716,7 +716,7 @@ static int synquacer_spi_probe(struct platform_device *pdev)pm_runtime_set_active (sspi->dev);pm_runtime_enable (sspi->dev); - ret =devm_spi_register_controller (sspi->dev, host); + ret =spi_register_controller (host);if (ret) goto disable_pm;
@@ -737,9 +737,15 @@ static void synquacer_spi_remove(struct platform_device *pdev)struct spi_controller *host =platform_get_drvdata (pdev);struct synquacer_spi *sspi =spi_controller_get_devdata (host); +spi_controller_get (host); + +spi_unregister_controller (host); +pm_runtime_disable (sspi->dev);clk_disable_unprepare (sspi->clk); + +spi_controller_put (host); }static int __maybe_unused synquacer_spi_suspend (struct device *dev)
Signature · 1 line
2.52.0
PATCH 21/26 spi: tegra114: fix controller deregistration
In reply to Johan HovoldMake sure to deregister the controller before disabling underlying resources like clocks during driver unbind.
--- drivers/spi/spi-tegra114.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)
drivers/spi/spi-tegra114.c +7 -1
@@ -1415,7 +1415,7 @@ static int tegra_spi_probe(struct platform_device *pdev) goto exit_pm_disable; } - ret =devm_spi_register_controller (&pdev->dev, host); + ret =spi_register_controller (host);if (ret <0 ) {dev_err (&pdev->dev,"can not register to host err %d\n" , ret); goto exit_free_irq;
@@ -1441,6 +1441,10 @@ static void tegra_spi_remove(struct platform_device *pdev)struct spi_controller *host =platform_get_drvdata (pdev);struct tegra_spi_data *tspi =spi_controller_get_devdata (host); +spi_controller_get (host); + +spi_unregister_controller (host); +free_irq (tspi->irq, tspi);if (tspi->tx_dma_chan)
@@ -1452,6 +1456,8 @@ static void tegra_spi_remove(struct platform_device *pdev)pm_runtime_disable (&pdev->dev);if (!pm_runtime_status_suspended (&pdev->dev))tegra_spi_runtime_suspend (&pdev->dev); + +spi_controller_put (host); }#ifdef CONFIG_PM_SLEEP
Signature · 1 line
2.52.0
PATCH 22/26 spi: tegra20-sflash: fix controller deregistration
In reply to Johan HovoldMake sure to deregister the controller before disabling underlying resources like clocks during driver unbind.
--- drivers/spi/spi-tegra20-sflash.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)
drivers/spi/spi-tegra20-sflash.c +7 -1
@@ -505,7 +505,7 @@ static int tegra_sflash_probe(struct platform_device *pdev)tegra_sflash_writel (tsd, tsd->def_command_reg,SPI_COMMAND );pm_runtime_put (&pdev->dev); - ret =devm_spi_register_controller (&pdev->dev, host); + ret =spi_register_controller (host);if (ret <0 ) {dev_err (&pdev->dev,"can not register to host err %d\n" , ret); goto exit_pm_disable;
@@ -528,11 +528,17 @@ static void tegra_sflash_remove(struct platform_device *pdev)struct spi_controller *host =platform_get_drvdata (pdev);struct tegra_sflash_data *tsd =spi_controller_get_devdata (host); +spi_controller_get (host); + +spi_unregister_controller (host); +free_irq (tsd->irq, tsd);pm_runtime_disable (&pdev->dev);if (!pm_runtime_status_suspended (&pdev->dev))tegra_sflash_runtime_suspend (&pdev->dev); + +spi_controller_put (host); }#ifdef CONFIG_PM_SLEEP
Signature · 1 line
2.52.0
PATCH 23/26 spi: ti-qspi: fix controller deregistration
In reply to Johan HovoldMake sure to deregister the controller before disabling underlying resources like clocks during driver unbind.
Note that the controller is suspended before disabling and releasing resources since commit 3ac066e2227c ("spi: spi-ti-qspi: Suspend the queue before removing the device") which avoids issues like unclocked accesses but prevents SPI device drivers from doing I/O during deregistration.
--- drivers/spi/spi-ti-qspi.c | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-)
drivers/spi/spi-ti-qspi.c +6 -8
@@ -888,7 +888,7 @@ static int ti_qspi_probe(struct platform_device *pdev) qspi->mmap_enabled = false; qspi->current_cs =-1 ; - ret =devm_spi_register_controller (&pdev->dev, host); + ret =spi_register_controller (host);if (!ret)return 0 ;
@@ -903,19 +903,17 @@ static int ti_qspi_probe(struct platform_device *pdev)static void ti_qspi_remove (struct platform_device *pdev) {struct ti_qspi *qspi =platform_get_drvdata (pdev); -int rc; - rc =spi_controller_suspend (qspi->host); -if (rc) { -dev_alert (&pdev->dev,"spi_controller_suspend() failed (%pe)\n" , -ERR_PTR (rc)); -return ; - } +spi_controller_get (qspi->host); + +spi_unregister_controller (qspi->host);pm_runtime_put_sync (&pdev->dev);pm_runtime_disable (&pdev->dev);ti_qspi_dma_cleanup (qspi); + +spi_controller_put (qspi->host); }static const struct dev_pm_ops ti_qspi_pm_ops = {
Signature · 1 line
2.52.0
PATCH 24/26 spi: uniphier: fix controller deregistration
In reply to Johan HovoldMake sure to deregister the controller before releasing underlying resources like DMA during driver unbind.
Note that clocks were also disabled before the recent commit fdca270f8f87 ("spi: uniphier: Simplify clock handling with devm_clk_get_enabled()").
--- drivers/spi/spi-uniphier.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)
drivers/spi/spi-uniphier.c +7 -1
@@ -746,7 +746,7 @@ static int uniphier_spi_probe(struct platform_device *pdev) host->max_dma_len =min (dma_tx_burst, dma_rx_burst); - ret =devm_spi_register_controller (&pdev->dev, host); + ret =spi_register_controller (host);if (ret) goto out_release_dma;
@@ -771,10 +771,16 @@ static void uniphier_spi_remove(struct platform_device *pdev) {struct spi_controller *host =platform_get_drvdata (pdev); +spi_controller_get (host); + +spi_unregister_controller (host); +if (host->dma_tx)dma_release_channel (host->dma_tx);if (host->dma_rx)dma_release_channel (host->dma_rx); + +spi_controller_put (host); }static const struct of_device_id uniphier_spi_match[] = {
Signature · 1 line
2.52.0
PATCH 25/26 spi: zynqmp-gqspi: fix controller deregistration
In reply to Johan HovoldMake sure to deregister the controller before disabling underlying resources like clocks during driver unbind.
--- drivers/spi/spi-zynqmp-gqspi.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
drivers/spi/spi-zynqmp-gqspi.c +3 -1
@@ -1324,7 +1324,7 @@ static int zynqmp_qspi_probe(struct platform_device *pdev) ctlr->dev.of_node = np; ctlr->auto_runtime_pm = true; - ret =devm_spi_register_controller (&pdev->dev, ctlr); + ret =spi_register_controller (ctlr);if (ret) {dev_err (&pdev->dev,"spi_register_controller failed\n" ); goto clk_dis_all;
@@ -1362,6 +1362,8 @@ static void zynqmp_qspi_remove(struct platform_device *pdev)pm_runtime_get_sync (&pdev->dev); +spi_unregister_controller (xqspi->ctlr); +zynqmp_gqspi_write (xqspi,GQSPI_EN_OFST ,0x0 );pm_runtime_disable (&pdev->dev);
Signature · 1 line
2.52.0
PATCH 26/26 spi: zynq-qspi: fix controller deregistration
In reply to Johan HovoldMake sure to deregister the controller before disabling it during driver unbind.
Note that clocks were also disabled before the recent commit 1f8fd9490e31 ("spi: zynq-qspi: Simplify clock handling with devm_clk_get_enabled()").
--- drivers/spi/spi-zynq-qspi.c | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-)
drivers/spi/spi-zynq-qspi.c +11 -4
@@ -643,7 +643,7 @@ static int zynq_qspi_probe(struct platform_device *pdev) xqspi =spi_controller_get_devdata (ctlr); xqspi->dev = dev; -platform_set_drvdata (pdev, xqspi); +platform_set_drvdata (pdev, ctlr); xqspi->regs =devm_platform_ioremap_resource (pdev,0 );if (IS_ERR (xqspi->regs)) { ret =PTR_ERR (xqspi->regs);
@@ -702,9 +702,9 @@ static int zynq_qspi_probe(struct platform_device *pdev)/* QSPI controller initializations */ zynq_qspi_init_hw (xqspi, ctlr->num_chipselect); - ret =devm_spi_register_controller (&pdev->dev, ctlr); + ret =spi_register_controller (ctlr);if (ret) { -dev_err (&pdev->dev,"devm_spi_register_controller failed\n" ); +dev_err (&pdev->dev,"failed to register controller\n" ); goto remove_ctlr; }
@@ -728,9 +728,16 @@ static int zynq_qspi_probe(struct platform_device *pdev)*/ static void zynq_qspi_remove (struct platform_device *pdev) { -struct zynq_qspi *xqspi =platform_get_drvdata (pdev); +struct spi_controller *ctlr =platform_get_drvdata (pdev); +struct zynq_qspi *xqspi =spi_controller_get_devdata (ctlr); + +spi_controller_get (ctlr); + +spi_unregister_controller (ctlr);zynq_qspi_write (xqspi,ZYNQ_QSPI_ENABLE_OFFSET ,0 ); + +spi_controller_put (ctlr); }static const struct of_device_id zynq_qspi_of_match[] = {
Signature · 1 line
2.52.0
Mark Brown
maintainerQuoted 6 lines · Johan Hovold
spi: fix controller deregistration (part 2/2)
Device managed registration generally only works if all involved resources are managed as otherwise resources may be disabled or freed while they are still in use.
Quoted 8 lines
This series fixes the SPI controller drivers that get this wrong by disabling resources such as clocks, DMA and interrupts while the controller (and its devices) are still registered, which can lead to issues like system errors due to unclocked accesses, NULL-pointer dereferences, hangs or just prevent SPI device drivers from doing I/O during during deregistration (e.g. to power down devices).
[...]
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-7.1
Thanks!
[01/26] spi: mt65xx: fix controller deregistration
https://git.kernel.org/broonie/spi/c/2ad30599cccc [02/26] spi: mtk-nor: fix controller deregistration https://git.kernel.org/broonie/spi/c/76336f249346 [03/26] spi: mxs: fix controller deregistration https://git.kernel.org/broonie/spi/c/8b0d0011af20 [04/26] spi: npcm-pspi: fix controller deregistration https://git.kernel.org/broonie/spi/c/ebd81199e00e [05/26] spi: omap2-mcspi: fix controller deregistration https://git.kernel.org/broonie/spi/c/fb45f95c377e [06/26] spi: pic32: fix controller deregistration https://git.kernel.org/broonie/spi/c/6b627bfe0c44 [07/26] spi: pic32-sqi: fix controller deregistration https://git.kernel.org/broonie/spi/c/420df79d1a61 [08/26] spi: pl022: fix controller deregistration https://git.kernel.org/broonie/spi/c/994b33366be9 [09/26] spi: qup: fix controller deregistration https://git.kernel.org/broonie/spi/c/443e3a0005a4 [10/26] spi: rspi: fix controller deregistration https://git.kernel.org/broonie/spi/c/9944fa6726af [11/26] spi: s3c64xx: fix controller deregistration https://git.kernel.org/broonie/spi/c/c1446b61e472 [12/26] spi: sh-hspi: fix controller deregistration https://git.kernel.org/broonie/spi/c/e63982e6392e [13/26] spi: sh-msiof: fix controller deregistration https://git.kernel.org/broonie/spi/c/45170f67a08b [14/26] spi: sifive: fix controller deregistration https://git.kernel.org/broonie/spi/c/0f25236694a2 [15/26] spi: slave-mt27xx: fix controller deregistration https://git.kernel.org/broonie/spi/c/ab840cbda4fe [16/26] spi: sprd: fix controller deregistration https://git.kernel.org/broonie/spi/c/123d17dbc5f0 [17/26] spi: st-ssc4: fix controller deregistration https://git.kernel.org/broonie/spi/c/19857374010d [18/26] spi: sun4i: fix controller deregistration https://git.kernel.org/broonie/spi/c/42108a2f03e0 [19/26] spi: sun6i: fix controller deregistration https://git.kernel.org/broonie/spi/c/d874a1c33aee [20/26] spi: syncuacer: fix controller deregistration https://git.kernel.org/broonie/spi/c/75d849c3452e [21/26] spi: tegra114: fix controller deregistration https://git.kernel.org/broonie/spi/c/9c9c27ff2058 [22/26] spi: tegra20-sflash: fix controller deregistration https://git.kernel.org/broonie/spi/c/ad7310e98332 [23/26] spi: ti-qspi: fix controller deregistration https://git.kernel.org/broonie/spi/c/0c18a1bacbb1 [24/26] spi: uniphier: fix controller deregistration https://git.kernel.org/broonie/spi/c/0245435f7772 [25/26] spi: zynqmp-gqspi: fix controller deregistration https://git.kernel.org/broonie/spi/c/6895fc4faafc [26/26] spi: zynq-qspi: fix controller deregistration https://git.kernel.org/broonie/spi/c/c9c012706c9f
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