Unverified Commit 81c9cdb1 authored by Mark Brown's avatar Mark Brown
Browse files

spi: fix controller deregistration (part 2/2) 

Johan Hovold <johan@kernel.org> says:

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 1f8fd949 ("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/20260325145319.1132072-1-johan@kernel.org/
parents 5b94c94c c9c01270

drivers/spi/spi-mt65xx.c

+3 −1
Original line number Diff line number Diff line
@@ -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);

drivers/spi/spi-mtk-nor.c

+3 −1
Original line number Diff line number Diff line
@@ -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);

drivers/spi/spi-mxs.c

+7 −1
Original line number Diff line number Diff line
@@ -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 = {

drivers/spi/spi-npcm-pspi.c

+7 −1
Original line number Diff line number Diff line
@@ -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[] = {

drivers/spi/spi-omap2-mcspi.c

+7 −1
Original line number Diff line number Diff line
@@ -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 */