Skip to contentSkip to conversation

spi: fix controller deregistration (part 1/2)

PATCH In review Johan Hovold 3 participants 28 messages Apr 9 – Apr 10

Subsystems (13)
Stable branch Arm/Amlogic Meson SoC support SPI Arm/Microchip (AT91) SoC support +9 more subsystems
lore
Share & tools
lore

Johan Hovold

author
#0 lore raw

cover spi: fix controller deregistration (part 1/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.

Johan

[1] https://lore.kernel.org/lkml/[email protected]/

Johan Hovold (20):
spi: amlogic-spisg: fix controller deregistration
spi: aspeed-smc: fix controller deregistration
spi: at91-usart: fix controller deregistration
spi: atmel: fix controller deregistration
spi: bcm63xx: fix controller deregistration
spi: bcm63xx-hsspi: fix controller deregistration
spi: bcmbca-hsspi: fix controller deregistration
spi: octeon: fix controller deregistration
spi: cavium-thunderx: fix controller deregistration
spi: coldfire-qspi: fix controller deregistration
spi: dln2: fix controller deregistration
spi: ep93xx: fix controller deregistration
spi: fsl-espi: fix controller deregistration
spi: fsl: fix controller deregistration
spi: img-spfi: fix controller deregistration
spi: lantiq-ssc: fix controller deregistration
spi: meson-spicc: fix controller deregistration
spi: microchip-core-qspi: fix controller deregistration
spi: microchip-core-spi: fix controller deregistration
spi: mpfs: fix controller deregistration

 drivers/spi/spi-amlogic-spisg.c       |  4 +++-
 drivers/spi/spi-aspeed-smc.c          |  9 ++++++---
 drivers/spi/spi-at91-usart.c          |  8 +++++++-
 drivers/spi/spi-atmel.c               |  8 +++++++-
 drivers/spi/spi-bcm63xx-hsspi.c       |  8 +++++++-
 drivers/spi/spi-bcm63xx.c             |  8 +++++++-
 drivers/spi/spi-bcmbca-hsspi.c        |  4 +++-
 drivers/spi/spi-cavium-octeon.c       |  8 +++++++-
 drivers/spi/spi-cavium-thunderx.c     |  8 +++++++-
 drivers/spi/spi-coldfire-qspi.c       | 10 ++++++++--
 drivers/spi/spi-dln2.c                |  8 +++++++-
 drivers/spi/spi-ep93xx.c              |  8 +++++++-
 drivers/spi/spi-fsl-espi.c            | 10 +++++++++-
 drivers/spi/spi-fsl-spi.c             |  8 +++++++-
 drivers/spi/spi-img-spfi.c            |  8 +++++++-
 drivers/spi/spi-lantiq-ssc.c          |  8 +++++++-
 drivers/spi/spi-meson-spicc.c         |  8 +++++++-
 drivers/spi/spi-microchip-core-qspi.c | 12 ++++++++----
 drivers/spi/spi-microchip-core-spi.c  |  4 +++-
 drivers/spi/spi-mpfs.c                |  4 +++-
 20 files changed, 127 insertions(+), 26 deletions(-)
Signature · 1 line
2.52.0
author
#1 lore raw

PATCH 1/20 spi: amlogic-spisg: fix controller deregistration

Make sure to deregister the controller before disabling underlying resources like clocks during driver unbind.

Fixes cef9991e04ae ("spi: Add Amlogic SPISG driver")
Cc [email protected] # 6.17: b8db95529979
Cc [email protected] # 6.17
Cc Sunny Luo <[email protected]>
Signed-off-by Johan Hovold <[email protected]>
---
 drivers/spi/spi-amlogic-spisg.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
drivers/spi/spi-amlogic-spisg.c +3 -1
index da8ec35115da..19c5eba412ef 100644
@@ -800,7 +800,7 @@ static int aml_spisg_probe(struct platform_device *pdev)
 		goto out_clk;
 	}
 
-	ret = devm_spi_register_controller(dev, ctlr);
+	ret = spi_register_controller(ctlr);
 	if (ret) {
 		dev_err(&pdev->dev, "spi controller registration failed\n");
 		goto out_clk;

@@ -823,6 +823,8 @@ static void aml_spisg_remove(struct platform_device *pdev)
 {
 	struct spisg_device *spisg = platform_get_drvdata(pdev);
 
+	spi_unregister_controller(spisg->controller);
+
 	if (!pm_runtime_suspended(&pdev->dev)) {
 		pinctrl_pm_select_sleep_state(&spisg->pdev->dev);
 		clk_disable_unprepare(spisg->core);
Signature · 1 line
2.52.0
author
#2 lore raw

PATCH 2/20 spi: aspeed-smc: fix controller deregistration

In reply to Johan Hovold

Make sure to deregister the controller before disabling it to allow SPI device drivers to do I/O during deregistration.

Fixes e3228ed92893 ("spi: spi-mem: Convert Aspeed SMC driver to spi-mem")
Cc [email protected] # 5.19
Cc Cédric Le Goater <[email protected]>
Signed-off-by Johan Hovold <[email protected]>
---
 drivers/spi/spi-aspeed-smc.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)
drivers/spi/spi-aspeed-smc.c +6 -3
index 9c286e534bf0..c21323e07d3c 100644
@@ -972,7 +972,7 @@ static int aspeed_spi_probe(struct platform_device *pdev)
 		return -ENOMEM;
 
 	aspi = spi_controller_get_devdata(ctlr);
-	platform_set_drvdata(pdev, aspi);
+	platform_set_drvdata(pdev, ctlr);
 	aspi->data = data;
 	aspi->dev = dev;
 

@@ -1021,7 +1021,7 @@ static int aspeed_spi_probe(struct platform_device *pdev)
 		return ret;
 	}
 
-	ret = devm_spi_register_controller(dev, ctlr);
+	ret = spi_register_controller(ctlr);
 	if (ret)
 		dev_err(&pdev->dev, "spi_register_controller failed\n");
 

@@ -1030,7 +1030,10 @@ static int aspeed_spi_probe(struct platform_device *pdev)
 
 static void aspeed_spi_remove(struct platform_device *pdev)
 {
-	struct aspeed_spi *aspi = platform_get_drvdata(pdev);
+	struct spi_controller *ctlr = platform_get_drvdata(pdev);
+	struct aspeed_spi *aspi = spi_controller_get_devdata(ctlr);
+
+	spi_unregister_controller(ctlr);
 
 	aspeed_spi_enable(aspi, false);
 }
Signature · 1 line
2.52.0
author
#3 lore raw

PATCH 3/20 spi: at91-usart: fix controller deregistration

In reply to Johan Hovold

Make sure to deregister the controller before disabling and releasing underlying resources like clocks and DMA during driver unbind.

Fixes e1892546ff66 ("spi: at91-usart: Add driver for at91-usart as SPI")
Cc [email protected] # 4.20
Cc Radu Pirea <[email protected]>
Signed-off-by Johan Hovold <[email protected]>
---
 drivers/spi/spi-at91-usart.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)
drivers/spi/spi-at91-usart.c +7 -1
index 76eb3ba75ab1..79edc1cd13c0 100644
@@ -556,7 +556,7 @@ static int at91_usart_spi_probe(struct platform_device *pdev)
 	spin_lock_init(&aus->lock);
 	init_completion(&aus->xfer_completion);
 
-	ret = devm_spi_register_controller(&pdev->dev, controller);
+	ret = spi_register_controller(controller);
 	if (ret)
 		goto at91_usart_fail_register_controller;
 

@@ -634,8 +634,14 @@ static void at91_usart_spi_remove(struct platform_device *pdev)
 	struct spi_controller *ctlr = platform_get_drvdata(pdev);
 	struct at91_usart_spi *aus = spi_controller_get_devdata(ctlr);
 
+	spi_controller_get(ctlr);
+
+	spi_unregister_controller(ctlr);
+
 	at91_usart_spi_release_dma(ctlr);
 	clk_disable_unprepare(aus->clk);
+
+	spi_controller_put(ctlr);
 }
 
 static const struct dev_pm_ops at91_usart_spi_pm_ops = {
Signature · 1 line
2.52.0
author
#4 lore raw

PATCH 4/20 spi: atmel: fix controller deregistration

In reply to Johan Hovold

Make sure to deregister the controller before disabling underlying resources like clocks during driver unbind.

Fixes 754ce4f29937 ("[PATCH] SPI: atmel_spi driver")
Cc [email protected] # 2.6.21
Signed-off-by Johan Hovold <[email protected]>
---
 drivers/spi/spi-atmel.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)
drivers/spi/spi-atmel.c +7 -1
index 445d645585bf..42db85d7ff8e 100644
@@ -1654,7 +1654,7 @@ static int atmel_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)
 		goto out_free_dma;
 

@@ -1688,8 +1688,12 @@ static void atmel_spi_remove(struct platform_device *pdev)
 	struct spi_controller	*host = platform_get_drvdata(pdev);
 	struct atmel_spi	*as = spi_controller_get_devdata(host);
 
+	spi_controller_get(host);
+
 	pm_runtime_get_sync(&pdev->dev);
 
+	spi_unregister_controller(host);
+
 	/* reset the hardware and block queue progress */
 	if (as->use_dma) {
 		atmel_spi_stop_dma(host);

@@ -1716,6 +1720,8 @@ static void atmel_spi_remove(struct platform_device *pdev)
 
 	pm_runtime_put_noidle(&pdev->dev);
 	pm_runtime_disable(&pdev->dev);
+
+	spi_controller_put(host);
 }
 
 static int atmel_spi_runtime_suspend(struct device *dev)
Signature · 1 line
2.52.0
author
#5 lore raw

PATCH 5/20 spi: bcm63xx: fix controller deregistration

In reply to Johan Hovold

Make sure to deregister the controller before disabling underlying resources like clocks during driver unbind.

Fixes b42dfed83d95 ("spi: add Broadcom BCM63xx SPI controller driver")
Cc [email protected] # 3.4
Cc Florian Fainelli <[email protected]>
Signed-off-by Johan Hovold <[email protected]>
---
 drivers/spi/spi-bcm63xx.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)
drivers/spi/spi-bcm63xx.c +7 -1
index 47266bb23a33..40cd7efc4b54 100644
@@ -602,7 +602,7 @@ static int bcm63xx_spi_probe(struct platform_device *pdev)
 		goto out_clk_disable;
 
 	/* register and we are done */
-	ret = devm_spi_register_controller(dev, host);
+	ret = spi_register_controller(host);
 	if (ret) {
 		dev_err(dev, "spi register failed\n");
 		goto out_clk_disable;

@@ -625,11 +625,17 @@ static void bcm63xx_spi_remove(struct platform_device *pdev)
 	struct spi_controller *host = platform_get_drvdata(pdev);
 	struct bcm63xx_spi *bs = spi_controller_get_devdata(host);
 
+	spi_controller_get(host);
+
+	spi_unregister_controller(host);
+
 	/* reset spi block */
 	bcm_spi_writeb(bs, 0, SPI_INT_MASK);
 
 	/* HW shutdown */
 	clk_disable_unprepare(bs->clk);
+
+	spi_controller_put(host);
 }
 
 static int bcm63xx_spi_suspend(struct device *dev)
Signature · 1 line
2.52.0
author
#6 lore raw

PATCH 6/20 spi: bcm63xx-hsspi: fix controller deregistration

In reply to Johan Hovold

Make sure to deregister the controller before disabling underlying resources like interrupts during driver unbind to allow SPI drivers to do I/O during deregistration.

Note that clocks were also disabled before the recent commit e532e21a246d ("spi: bcm63xx-hsspi: Simplify clock handling with devm_clk_get_enabled()").

Fixes 7d255695804f ("spi/bcm63xx-hsspi: use devm_register_master()")
Cc [email protected] # 3.14
Cc Jonas Gorski <[email protected]>
Signed-off-by Johan Hovold <[email protected]>
---
 drivers/spi/spi-bcm63xx-hsspi.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)
drivers/spi/spi-bcm63xx-hsspi.c +7 -1
index 266eabd3715b..e935e8ab9cfd 100644
@@ -857,7 +857,7 @@ static int bcm63xx_hsspi_probe(struct platform_device *pdev)
 	}
 
 	/* register and we are done */
-	ret = devm_spi_register_controller(dev, host);
+	ret = spi_register_controller(host);
 	if (ret)
 		goto out_sysgroup_disable;
 

@@ -880,9 +880,15 @@ static void bcm63xx_hsspi_remove(struct platform_device *pdev)
 	struct spi_controller *host = platform_get_drvdata(pdev);
 	struct bcm63xx_hsspi *bs = spi_controller_get_devdata(host);
 
+	spi_controller_get(host);
+
+	spi_unregister_controller(host);
+
 	/* reset the hardware and block queue progress */
 	__raw_writel(0, bs->regs + HSSPI_INT_MASK_REG);
 	sysfs_remove_group(&pdev->dev.kobj, &bcm63xx_hsspi_group);
+
+	spi_controller_put(host);
 }
 
 #ifdef CONFIG_PM_SLEEP
Signature · 1 line
2.52.0
author
#7 lore raw

PATCH 7/20 spi: bcmbca-hsspi: fix controller deregistration

In reply to Johan Hovold

Make sure to deregister the controller before disabling underlying resources like interrupts during driver unbind to allow SPI drivers to do I/O during deregistration.

Note that clocks were also disabled before the recent commit e532e21a246d ("spi: bcm63xx-hsspi: Simplify clock handling with devm_clk_get_enabled()").

Fixes a38a2233f23b ("spi: bcmbca-hsspi: Add driver for newer HSSPI controller")
Cc [email protected] # 6.3: deb269e0394f
Cc [email protected] # 6.3
Cc William Zhang <[email protected]>
Signed-off-by Johan Hovold <[email protected]>
---
 drivers/spi/spi-bcmbca-hsspi.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
drivers/spi/spi-bcmbca-hsspi.c +3 -1
index 2e22345115fd..09c1472ae4fa 100644
@@ -538,7 +538,7 @@ static int bcmbca_hsspi_probe(struct platform_device *pdev)
 		return dev_err_probe(dev, ret, "couldn't register sysfs group\n");
 
 	/* register and we are done */
-	ret = devm_spi_register_controller(dev, host);
+	ret = spi_register_controller(host);
 	if (ret)
 		goto out_sysgroup_disable;
 

@@ -556,6 +556,8 @@ static void bcmbca_hsspi_remove(struct platform_device *pdev)
 	struct spi_controller *host = platform_get_drvdata(pdev);
 	struct bcmbca_hsspi *bs = spi_controller_get_devdata(host);
 
+	spi_unregister_controller(host);
+
 	/* reset the hardware and block queue progress */
 	__raw_writel(0, bs->regs + HSSPI_INT_MASK_REG);
 	sysfs_remove_group(&pdev->dev.kobj, &bcmbca_hsspi_group);
Signature · 1 line
2.52.0
author
#8 lore raw

PATCH 8/20 spi: octeon: fix controller deregistration

In reply to Johan Hovold

Make sure to deregister the controller before disabling it to avoid hanging or leaking resources associated with the queue when the queue is non-empty.

Fixes 22ad2d8df77d ("spi: octeon: use devm_spi_register_master()")
Cc [email protected] # 3.13
Cc Jingoo Han <[email protected]>
Signed-off-by Johan Hovold <[email protected]>
---
 drivers/spi/spi-cavium-octeon.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)
drivers/spi/spi-cavium-octeon.c +7 -1
index 155085a053a1..b95bfa6a3013 100644
@@ -54,7 +54,7 @@ static int octeon_spi_probe(struct platform_device *pdev)
 	host->bits_per_word_mask = SPI_BPW_MASK(8);
 	host->max_speed_hz = OCTEON_SPI_MAX_CLOCK_HZ;
 
-	err = devm_spi_register_controller(&pdev->dev, host);
+	err = spi_register_controller(host);
 	if (err) {
 		dev_err(&pdev->dev, "register host failed: %d\n", err);
 		goto fail;

@@ -73,8 +73,14 @@ static void octeon_spi_remove(struct platform_device *pdev)
 	struct spi_controller *host = platform_get_drvdata(pdev);
 	struct octeon_spi *p = spi_controller_get_devdata(host);
 
+	spi_controller_get(host);
+
+	spi_unregister_controller(host);
+
 	/* Clear the CSENA* and put everything in a known state. */
 	writeq(0, p->register_base + OCTEON_SPI_CFG(p));
+
+	spi_controller_put(host);
 }
 
 static const struct of_device_id octeon_spi_match[] = {
Signature · 1 line
2.52.0
author
#9 lore raw

PATCH 9/20 spi: cavium-thunderx: fix controller deregistration

In reply to Johan Hovold

Make sure to deregister the controller before disabling it to avoid hanging or leaking resources associated with the queue when the queue non-empty.

Fixes 7347a6c7af8d ("spi: octeon: Add ThunderX driver")
Cc [email protected] # 4.9
Cc Jan Glauber <[email protected]>
Signed-off-by Johan Hovold <[email protected]>
---
 drivers/spi/spi-cavium-thunderx.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)
drivers/spi/spi-cavium-thunderx.c +7 -1
index 99aac40a1bba..f1a9aa696c87 100644
@@ -70,7 +70,7 @@ static int thunderx_spi_probe(struct pci_dev *pdev,
 
 	pci_set_drvdata(pdev, host);
 
-	ret = devm_spi_register_controller(dev, host);
+	ret = spi_register_controller(host);
 	if (ret)
 		goto error;
 

@@ -90,8 +90,14 @@ static void thunderx_spi_remove(struct pci_dev *pdev)
 	if (!p)
 		return;
 
+	spi_controller_get(host);
+
+	spi_unregister_controller(host);
+
 	/* Put everything in a known state. */
 	writeq(0, p->register_base + OCTEON_SPI_CFG(p));
+
+	spi_controller_put(host);
 }
 
 static const struct pci_device_id thunderx_spi_pci_id_table[] = {
Signature · 1 line
2.52.0
author
#10 lore raw

PATCH 10/20 spi: coldfire-qspi: fix controller deregistration

In reply to Johan Hovold

Make sure to deregister the controller before disabling underlying resources like clocks (via runtime pm) during driver unbind.

Fixes 34b8c6617366 ("spi: Add Freescale/Motorola Coldfire QSPI driver")
Cc [email protected] # 2.6.34
Cc Steven King <[email protected]>
Signed-off-by Johan Hovold <[email protected]>
---
 drivers/spi/spi-coldfire-qspi.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)
drivers/spi/spi-coldfire-qspi.c +8 -2
index fdf37636cb9f..b45f44de85dc 100644
@@ -410,9 +410,9 @@ static int mcfqspi_probe(struct platform_device *pdev)
 	platform_set_drvdata(pdev, host);
 	pm_runtime_enable(&pdev->dev);
 
-	status = devm_spi_register_controller(&pdev->dev, host);
+	status = spi_register_controller(host);
 	if (status) {
-		dev_dbg(&pdev->dev, "devm_spi_register_controller failed\n");
+		dev_dbg(&pdev->dev, "failed to register controller\n");
 		goto fail1;
 	}
 

@@ -436,11 +436,17 @@ static void mcfqspi_remove(struct platform_device *pdev)
 	struct spi_controller *host = platform_get_drvdata(pdev);
 	struct mcfqspi *mcfqspi = spi_controller_get_devdata(host);
 
+	spi_controller_get(host);
+
+	spi_unregister_controller(host);
+
 	pm_runtime_disable(&pdev->dev);
 	/* disable the hardware (set the baud rate to 0) */
 	mcfqspi_wr_qmr(mcfqspi, MCFQSPI_QMR_MSTR);
 
 	mcfqspi_cs_teardown(mcfqspi);
+
+	spi_controller_put(host);
 }
 
 #ifdef CONFIG_PM_SLEEP
Signature · 1 line
2.52.0
author
#11 lore raw

PATCH 11/20 spi: dln2: fix controller deregistration

In reply to Johan Hovold

Make sure to deregister the controller before disabling it to allow SPI device drivers to do I/O during deregistration.

Fixes 3d8c0d749da3 ("spi: add support for DLN-2 USB-SPI adapter")
Cc [email protected] # 4.0
Cc Laurentiu Palcu <[email protected]>
Signed-off-by Johan Hovold <[email protected]>
---
 drivers/spi/spi-dln2.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)
drivers/spi/spi-dln2.c +7 -1
index d90282960ab6..392f0d05f508 100644
@@ -758,7 +758,7 @@ static int dln2_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 < 0) {
 		dev_err(&pdev->dev, "Failed to register host\n");
 		goto exit_register;

@@ -783,10 +783,16 @@ static void dln2_spi_remove(struct platform_device *pdev)
 	struct spi_controller *host = platform_get_drvdata(pdev);
 	struct dln2_spi *dln2 = spi_controller_get_devdata(host);
 
+	spi_controller_get(host);
+
+	spi_unregister_controller(host);
+
 	pm_runtime_disable(&pdev->dev);
 
 	if (dln2_spi_enable(dln2, false) < 0)
 		dev_err(&pdev->dev, "Failed to disable SPI module\n");
+
+	spi_controller_put(host);
 }
 
 #ifdef CONFIG_PM_SLEEP
Signature · 1 line
2.52.0
author
#12 lore raw

PATCH 12/20 spi: ep93xx: fix controller deregistration

In reply to Johan Hovold

Make sure to deregister the controller before releasing underlying resources like DMA during driver unbind.

Fixes 011f23a3c2f2 ("spi/ep93xx: implemented driver for Cirrus EP93xx SPI controller")
Cc [email protected] # 2.6.35
Signed-off-by Johan Hovold <[email protected]>
---
 drivers/spi/spi-ep93xx.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)
drivers/spi/spi-ep93xx.c +7 -1
index 90d5f3ea6508..db50018050e5 100644
@@ -689,7 +689,7 @@ static int ep93xx_spi_probe(struct platform_device *pdev)
 	/* make sure that the hardware is disabled */
 	writel(0, espi->mmio + SSPCR1);
 
-	error = devm_spi_register_controller(&pdev->dev, host);
+	error = spi_register_controller(host);
 	if (error) {
 		dev_err(&pdev->dev, "failed to register SPI host\n");
 		goto fail_free_dma;

@@ -713,7 +713,13 @@ static void ep93xx_spi_remove(struct platform_device *pdev)
 	struct spi_controller *host = platform_get_drvdata(pdev);
 	struct ep93xx_spi *espi = spi_controller_get_devdata(host);
 
+	spi_controller_get(host);
+
+	spi_unregister_controller(host);
+
 	ep93xx_spi_release_dma(espi);
+
+	spi_controller_put(host);
 }
 
 static const struct of_device_id ep93xx_spi_of_ids[] = {
Signature · 1 line
2.52.0
author
#13 lore raw

PATCH 13/20 spi: fsl-espi: fix controller deregistration

In reply to Johan Hovold

Make sure to deregister the controller before disabling runtime PM (which can leave the controller disabled) to allow SPI device drivers to do I/O during deregistration.

Fixes e9abb4db8d10 ("spi: fsl-espi: add runtime PM")
Cc [email protected] # 4.3
Cc Heiner Kallweit <[email protected]>
Signed-off-by Johan Hovold <[email protected]>
---
 drivers/spi/spi-fsl-espi.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)
drivers/spi/spi-fsl-espi.c +9 -1
index 56270f8fdc17..45b9974ae911 100644
@@ -718,7 +718,7 @@ static int fsl_espi_probe(struct device *dev, struct resource *mem,
 	pm_runtime_enable(dev);
 	pm_runtime_get_sync(dev);
 
-	ret = devm_spi_register_controller(dev, host);
+	ret = spi_register_controller(host);
 	if (ret < 0)
 		goto err_pm;
 

@@ -782,7 +782,15 @@ static int of_fsl_espi_probe(struct platform_device *ofdev)
 
 static void of_fsl_espi_remove(struct platform_device *dev)
 {
+	struct spi_controller *host = platform_get_drvdata(dev);
+
+	spi_controller_get(host);
+
+	spi_unregister_controller(host);
+
 	pm_runtime_disable(&dev->dev);
+
+	spi_controller_put(host);
 }
 
 #ifdef CONFIG_PM_SLEEP
Signature · 1 line
2.52.0
author
#14 lore raw

PATCH 14/20 spi: fsl: fix controller deregistration

In reply to Johan Hovold

Make sure to deregister the controller before releasing underlying resources like DMA during driver unbind.

Fixes 4178b6b1b595 ("spi: fsl-(e)spi: migrate to using devm_ functions to simplify cleanup")
Cc [email protected] # 4.3
Cc Heiner Kallweit <[email protected]>
Signed-off-by Johan Hovold <[email protected]>
---
 drivers/spi/spi-fsl-spi.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)
drivers/spi/spi-fsl-spi.c +7 -1
index bf3fc3ce0cc2..2681ed8daf2b 100644
@@ -614,7 +614,7 @@ static struct spi_controller *fsl_spi_probe(struct device *dev,
 
 	mpc8xxx_spi_write_reg(&reg_base->mode, regval);
 
-	ret = devm_spi_register_controller(dev, host);
+	ret = spi_register_controller(host);
 	if (ret < 0)
 		goto err_probe;
 

@@ -751,7 +751,13 @@ static void plat_mpc8xxx_spi_remove(struct platform_device *pdev)
 	struct spi_controller *host = platform_get_drvdata(pdev);
 	struct mpc8xxx_spi *mpc8xxx_spi = spi_controller_get_devdata(host);
 
+	spi_controller_get(host);
+
+	spi_unregister_controller(host);
+
 	fsl_spi_cpm_free(mpc8xxx_spi);
+
+	spi_controller_put(host);
 }
 
 MODULE_ALIAS("platform:mpc8xxx_spi");
Signature · 1 line
2.52.0
author
#15 lore raw

Re: [PATCH 14/20] spi: fsl: fix controller deregistration

Quoted 19 lines · Johan Hovold

Make sure to deregister the controller before releasing underlying resources like DMA during driver unbind.

Fixes 4178b6b1b595 ("spi: fsl-(e)spi: migrate to using devm_ functions to simplify cleanup")
Cc [email protected] # 4.3
Cc Heiner Kallweit <[email protected]>
Signed-off-by Johan Hovold <[email protected]>
drivers/spi/spi-fsl-spi.c +1 -1
@@ -614,7 +614,7 @@ static struct spi_controller *fsl_spi_probe(struct device *dev,
 
 	mpc8xxx_spi_write_reg(&reg_base->mode, regval);
 
-	ret = devm_spi_register_controller(dev, host);
+	ret = spi_register_controller(host);
 	if (ret < 0)
 		goto err_probe;
 

@@ -751,7 +751,13 @@ static void plat_mpc8xxx_spi_remove(struct platform_device *pdev)
 	struct spi_controller *host = platform_get_drvdata(pdev);

drivers/spi/spi-fsl-spi.c static void plat_mpc8xxx_spi_remove(struct platform_device *pdev) · line 761

Quoted 10 lines
drivers/spi/spi-fsl-spi.c +6 -0
 	struct mpc8xxx_spi *mpc8xxx_spi = spi_controller_get_devdata(host);
 
+	spi_controller_get(host);
+
+	spi_unregister_controller(host);
+
 	fsl_spi_cpm_free(mpc8xxx_spi);
+
+	spi_controller_put(host);
 }

I missed that this module registers two platform drivers so this patch will need another spin (reported by Sashiko).

Johan

author
#16 lore raw

PATCH 15/20 spi: img-spfi: fix controller deregistration

In reply to Johan Hovold

Make sure to deregister the controller before disabling and releasing underlying resources like clocks and DMA during driver unbind.

Fixes deba25800a12 ("spi: Add driver for IMG SPFI controller")
Cc [email protected] # 3.19
Cc Andrew Bresticker <[email protected]>
Signed-off-by Johan Hovold <[email protected]>
---
 drivers/spi/spi-img-spfi.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)
drivers/spi/spi-img-spfi.c +7 -1
index 902fb64815c9..57625a3ce2f2 100644
@@ -643,7 +643,7 @@ static int img_spfi_probe(struct platform_device *pdev)
 	pm_runtime_set_active(spfi->dev);
 	pm_runtime_enable(spfi->dev);
 
-	ret = devm_spi_register_controller(spfi->dev, host);
+	ret = spi_register_controller(host);
 	if (ret)
 		goto disable_pm;
 

@@ -669,6 +669,10 @@ static void img_spfi_remove(struct platform_device *pdev)
 	struct spi_controller *host = platform_get_drvdata(pdev);
 	struct img_spfi *spfi = spi_controller_get_devdata(host);
 
+	spi_controller_get(host);
+
+	spi_unregister_controller(host);
+
 	if (spfi->tx_ch)
 		dma_release_channel(spfi->tx_ch);
 	if (spfi->rx_ch)

@@ -679,6 +683,8 @@ static void img_spfi_remove(struct platform_device *pdev)
 		clk_disable_unprepare(spfi->spfi_clk);
 		clk_disable_unprepare(spfi->sys_clk);
 	}
+
+	spi_controller_put(host);
 }
 
 #ifdef CONFIG_PM
Signature · 1 line
2.52.0
author
#17 lore raw

PATCH 16/20 spi: lantiq-ssc: fix controller deregistration

In reply to Johan Hovold

Make sure to deregister the controller before releasing underlying resources like clocks during driver unbind.

Fixes 17f84b793c01 ("spi: lantiq-ssc: add support for Lantiq SSC SPI controller")
Cc [email protected] # 4.11
Cc Hauke Mehrtens <[email protected]>
Signed-off-by Johan Hovold <[email protected]>
---
 drivers/spi/spi-lantiq-ssc.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)
drivers/spi/spi-lantiq-ssc.c +7 -1
index f83cb63c9d0c..75b9af8cb5db 100644
@@ -994,7 +994,7 @@ static int lantiq_ssc_probe(struct platform_device *pdev)
 		"Lantiq SSC SPI controller (Rev %i, TXFS %u, RXFS %u, DMA %u)\n",
 		revision, spi->tx_fifo_size, spi->rx_fifo_size, supports_dma);
 
-	err = devm_spi_register_controller(dev, host);
+	err = spi_register_controller(host);
 	if (err) {
 		dev_err(dev, "failed to register spi host\n");
 		goto err_wq_destroy;

@@ -1016,6 +1016,10 @@ static void lantiq_ssc_remove(struct platform_device *pdev)
 {
 	struct lantiq_ssc_spi *spi = platform_get_drvdata(pdev);
 
+	spi_controller_get(spi->host);
+
+	spi_unregister_controller(spi->host);
+
 	lantiq_ssc_writel(spi, 0, LTQ_SPI_IRNEN);
 	lantiq_ssc_writel(spi, 0, LTQ_SPI_CLC);
 	rx_fifo_flush(spi);

@@ -1024,6 +1028,8 @@ static void lantiq_ssc_remove(struct platform_device *pdev)
 
 	destroy_workqueue(spi->wq);
 	clk_put(spi->fpi_clk);
+
+	spi_controller_put(spi->host);
 }
 
 static struct platform_driver lantiq_ssc_driver = {
Signature · 1 line
2.52.0
author
#18 lore raw

PATCH 17/20 spi: meson-spicc: fix controller deregistration

In reply to Johan Hovold

Make sure to deregister the controller before disabling it to allow SPI device drivers to do I/O during deregistration.

Fixes 454fa271bc4e ("spi: Add Meson SPICC driver")
Cc [email protected] # 4.13
Cc Neil Armstrong <[email protected]>
Signed-off-by Johan Hovold <[email protected]>
---
 drivers/spi/spi-meson-spicc.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)
drivers/spi/spi-meson-spicc.c +7 -1
index 57768da3205d..b80f9f457b66 100644
@@ -1081,7 +1081,7 @@ static int meson_spicc_probe(struct platform_device *pdev)
 		}
 	}
 
-	ret = devm_spi_register_controller(&pdev->dev, host);
+	ret = spi_register_controller(host);
 	if (ret) {
 		dev_err(&pdev->dev, "spi registration failed\n");
 		goto out_host;

@@ -1099,8 +1099,14 @@ static void meson_spicc_remove(struct platform_device *pdev)
 {
 	struct meson_spicc_device *spicc = platform_get_drvdata(pdev);
 
+	spi_controller_get(spicc->host);
+
+	spi_unregister_controller(spicc->host);
+
 	/* Disable SPI */
 	writel(0, spicc->base + SPICC_CONREG);
+
+	spi_controller_put(spicc->host);
 }
 
 static const struct meson_spicc_data meson_spicc_gx_data = {
Signature · 1 line
2.52.0
author
#19 lore raw

PATCH 18/20 spi: microchip-core-qspi: fix controller deregistration

In reply to Johan Hovold

Make sure to deregister the controller before disabling underlying resources like interrupts during driver unbind.

Fixes 8596124c4c1b ("spi: microchip-core-qspi: Add support for microchip fpga qspi controllers")
Cc [email protected] # 6.1
Cc Naga Sureshkumar Relli <[email protected]>
Signed-off-by Johan Hovold <[email protected]>
---
 drivers/spi/spi-microchip-core-qspi.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)
drivers/spi/spi-microchip-core-qspi.c +8 -4
index aafe6cbf2aea..eab059fb0bc2 100644
@@ -692,7 +692,7 @@ static int mchp_coreqspi_probe(struct platform_device *pdev)
 		return -ENOMEM;
 
 	qspi = spi_controller_get_devdata(ctlr);
-	platform_set_drvdata(pdev, qspi);
+	platform_set_drvdata(pdev, ctlr);
 
 	qspi->regs = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(qspi->regs))

@@ -732,7 +732,7 @@ static int mchp_coreqspi_probe(struct platform_device *pdev)
 	ctlr->num_chipselect = 2;
 	ctlr->use_gpio_descriptors = true;
 
-	ret = devm_spi_register_controller(&pdev->dev, ctlr);
+	ret = spi_register_controller(ctlr);
 	if (ret)
 		return dev_err_probe(&pdev->dev, ret,
 				     "spi_register_controller failed\n");

@@ -742,9 +742,13 @@ static int mchp_coreqspi_probe(struct platform_device *pdev)
 
 static void mchp_coreqspi_remove(struct platform_device *pdev)
 {
-	struct mchp_coreqspi *qspi = platform_get_drvdata(pdev);
-	u32 control = readl_relaxed(qspi->regs + REG_CONTROL);
+	struct spi_controller *ctlr = platform_get_drvdata(pdev);
+	struct mchp_coreqspi *qspi = spi_controller_get_devdata(ctlr);
+	u32 control;
 
+	spi_unregister_controller(ctlr);
+
+	control = readl_relaxed(qspi->regs + REG_CONTROL);
 	mchp_coreqspi_disable_ints(qspi);
 	control &= ~CONTROL_ENABLE;
 	writel_relaxed(control, qspi->regs + REG_CONTROL);
Signature · 1 line
2.52.0
Acked-by on v1 18/20
#20 lore raw

Re: [PATCH 18/20] spi: microchip-core-qspi: fix controller deregistration

Quoted 6 lines · Johan Hovold

Make sure to deregister the controller before disabling underlying resources like interrupts during driver unbind.

Fixes 8596124c4c1b ("spi: microchip-core-qspi: Add support for microchip fpga qspi controllers")
Cc [email protected] # 6.1
Cc Naga Sureshkumar Relli <[email protected]>

Where are you getting these CCs from? I am listed as maintainer for this driver but didn't get CCed, only seeing this because I am CCed on another patch in the set. Please use get_maintainer.pl.

Quoted 1 line
Signed-off-by Johan Hovold <[email protected]>
Acked-by Conor Dooley <[email protected]>
maintainer
#21 lore raw

Re: [PATCH 18/20] spi: microchip-core-qspi: fix controller deregistration

Quoted 11 lines · Conor Dooley
Quoted 6 lines · Johan Hovold

Make sure to deregister the controller before disabling underlying resources like interrupts during driver unbind.

Fixes 8596124c4c1b ("spi: microchip-core-qspi: Add support for microchip fpga qspi controllers")
Cc [email protected] # 6.1
Cc Naga Sureshkumar Relli <[email protected]>

Where are you getting these CCs from? I am listed as maintainer for this driver but didn't get CCed, only seeing this because I am CCed on another patch in the set. Please use get_maintainer.pl.

They're the author of the commit referenced in the Fixes: tag.

author
#22 lore raw

Re: [PATCH 18/20] spi: microchip-core-qspi: fix controller deregistration

In reply to Conor Dooley
Quoted 11 lines · Conor Dooley
Quoted 6 lines · Johan Hovold

Make sure to deregister the controller before disabling underlying resources like interrupts during driver unbind.

Fixes 8596124c4c1b ("spi: microchip-core-qspi: Add support for microchip fpga qspi controllers")
Cc [email protected] # 6.1
Cc Naga Sureshkumar Relli <[email protected]>

Where are you getting these CCs from? I am listed as maintainer for this driver but didn't get CCed, only seeing this because I am CCed on another patch in the set. Please use get_maintainer.pl.

As Mark pointed out, the explicit CC above is for the author of the commit in the Fixes tag.

I do use get_maintainer.pl, but it seems I trimmed the result a bit too much this time. I sometimes drop what appears to be fall-back entries (e.g. using pattern matching), but here the driver is indeed explicitly listed in the platform support entry.

Johan

author
#23 lore raw

PATCH 19/20 spi: microchip-core-spi: fix controller deregistration

In reply to Johan Hovold

Make sure to deregister the controller before disabling underlying resources like interrupts during driver unbind.

Fixes 059f545832be ("spi: add support for microchip "soft" spi controller")
Cc [email protected] # 6.19
Cc Prajna Rajendra Kumar <[email protected]>
Signed-off-by Johan Hovold <[email protected]>
---
 drivers/spi/spi-microchip-core-spi.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
drivers/spi/spi-microchip-core-spi.c +3 -1
index a4c128ae391b..be01c178e2b0 100644
@@ -384,7 +384,7 @@ static int mchp_corespi_probe(struct platform_device *pdev)
 
 	mchp_corespi_init(host, spi);
 
-	ret = devm_spi_register_controller(dev, host);
+	ret = spi_register_controller(host);
 	if (ret) {
 		mchp_corespi_disable_ints(spi);
 		mchp_corespi_disable(spi);

@@ -399,6 +399,8 @@ static void mchp_corespi_remove(struct platform_device *pdev)
 	struct spi_controller *host = platform_get_drvdata(pdev);
 	struct mchp_corespi *spi = spi_controller_get_devdata(host);
 
+	spi_unregister_controller(host);
+
 	mchp_corespi_disable_ints(spi);
 	mchp_corespi_disable(spi);
 }
Signature · 1 line
2.52.0
Acked-by on v1 19/20
#24 lore raw

Re: [PATCH 19/20] spi: microchip-core-spi: fix controller deregistration

Quoted 7 lines · Johan Hovold

Make sure to deregister the controller before disabling underlying resources like interrupts during driver unbind.

Fixes 059f545832be ("spi: add support for microchip "soft" spi controller")
Cc [email protected] # 6.19
Cc Prajna Rajendra Kumar <[email protected]>
Signed-off-by Johan Hovold <[email protected]>
Acked-by Conor Dooley <[email protected]>
author
#25 lore raw

PATCH 20/20 spi: mpfs: fix controller deregistration

In reply to Johan Hovold

Make sure to deregister the controller before disabling underlying resources like interrupts during driver unbind.

Fixes 9ac8d17694b6 ("spi: add support for microchip fpga spi controllers")
Cc [email protected] # 6.0
Cc Conor Dooley <[email protected]>
Signed-off-by Johan Hovold <[email protected]>
---
 drivers/spi/spi-mpfs.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
drivers/spi/spi-mpfs.c +3 -1
index 64d15a6188ac..989a379b0700 100644
@@ -574,7 +574,7 @@ static int mpfs_spi_probe(struct platform_device *pdev)
 
 	mpfs_spi_init(host, spi);
 
-	ret = devm_spi_register_controller(&pdev->dev, host);
+	ret = spi_register_controller(host);
 	if (ret) {
 		mpfs_spi_disable_ints(spi);
 		mpfs_spi_disable(spi);

@@ -592,6 +592,8 @@ static void mpfs_spi_remove(struct platform_device *pdev)
 	struct spi_controller *host  = platform_get_drvdata(pdev);
 	struct mpfs_spi *spi = spi_controller_get_devdata(host);
 
+	spi_unregister_controller(host);
+
 	mpfs_spi_disable_ints(spi);
 	mpfs_spi_disable(spi);
 }
Signature · 1 line
2.52.0
maintainer
#27 lore raw

Re: (subset) [PATCH 00/20] spi: fix controller deregistration (part 1/2)

In reply to Johan Hovold
Quoted 6 lines · Johan Hovold

spi: fix controller deregistration (part 1/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/20] spi: amlogic-spisg: fix controller deregistration

        https://git.kernel.org/broonie/spi/c/84d31bb1f625
[02/20] spi: aspeed-smc: fix controller deregistration
        https://git.kernel.org/broonie/spi/c/1044e5a4ccd5
[03/20] spi: at91-usart: fix controller deregistration
        https://git.kernel.org/broonie/spi/c/9acecc9bcff0
[04/20] spi: atmel: fix controller deregistration
        https://git.kernel.org/broonie/spi/c/8d4de97e8352
[05/20] spi: bcm63xx: fix controller deregistration
        https://git.kernel.org/broonie/spi/c/c39e65a4e3b8
[06/20] spi: bcm63xx-hsspi: fix controller deregistration
        https://git.kernel.org/broonie/spi/c/ab837c51899d
[07/20] spi: bcmbca-hsspi: fix controller deregistration
        https://git.kernel.org/broonie/spi/c/c3d97c3320b9
[08/20] spi: octeon: fix controller deregistration
        https://git.kernel.org/broonie/spi/c/3c49a4d8799b
[09/20] spi: cavium-thunderx: fix controller deregistration
        https://git.kernel.org/broonie/spi/c/dbb6b01267c0
[10/20] spi: coldfire-qspi: fix controller deregistration
        https://git.kernel.org/broonie/spi/c/e7c510e192ff
[11/20] spi: dln2: fix controller deregistration
        https://git.kernel.org/broonie/spi/c/c353020fbfa8
[12/20] spi: ep93xx: fix controller deregistration
        https://git.kernel.org/broonie/spi/c/f4838934b695
[13/20] spi: fsl-espi: fix controller deregistration
        https://git.kernel.org/broonie/spi/c/e506a700a7ad
[15/20] spi: img-spfi: fix controller deregistration
        https://git.kernel.org/broonie/spi/c/fc3a83b0d9c1
[16/20] spi: lantiq-ssc: fix controller deregistration
        https://git.kernel.org/broonie/spi/c/b99206710d03
[17/20] spi: meson-spicc: fix controller deregistration
        https://git.kernel.org/broonie/spi/c/77953c76bec9
[18/20] spi: microchip-core-qspi: fix controller deregistration
        https://git.kernel.org/broonie/spi/c/e6464140d439
[19/20] spi: microchip-core-spi: fix controller deregistration
        https://git.kernel.org/broonie/spi/c/d00d722ebad4
[20/20] spi: mpfs: fix controller deregistration
        https://git.kernel.org/broonie/spi/c/573c7db8fce9

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