spi: cadence-quadspi: runtime PM fixes
Subsystems (2)
cover spi: cadence-quadspi: runtime PM fixes
This series fixes some runtime PM related issues in the cadence-quadspi driver.
Included is also a couple of related cleanups.
Johan
Johan Hovold (6):
spi: cadence-quadspi: fix runtime pm disable imbalance on probe
failure
spi: cadence-quadspi: fix clock imbalance on probe failure
spi: cadence-quadspi: fix unclocked access on unbind
spi: cadence-quadspi: fix runtime pm and clock imbalance on unbind
spi: cadence-quadspi: clean up disable runtime pm quirk
spi: cadence-quadspi: drop redundant match data lookup
drivers/spi/spi-cadence-quadspi.c | 66 +++++++++++++------------------ 1 file changed, 27 insertions(+), 39 deletions(-)
Signature · 1 line
2.52.0
PATCH 1/6 spi: cadence-quadspi: fix runtime pm disable imbalance on probe failure
A recent attempt to fix the probe error handling introduced a runtime PM disable depth imbalance by incorrectly disabling runtime PM on early failures (e.g. probe deferral).
--- drivers/spi/spi-cadence-quadspi.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-)
drivers/spi/spi-cadence-quadspi.c +5 -6
@@ -1871,7 +1871,7 @@ static int cqspi_probe(struct platform_device *pdev) ret =clk_bulk_prepare_enable (CLK_QSPI_NUM , cqspi->clks);if (ret) {dev_err (dev,"Cannot enable QSPI clocks.\n" ); - goto disable_rpm; +return ret; }/* Obtain QSPI reset control */
@@ -1981,7 +1981,7 @@ static int cqspi_probe(struct platform_device *pdev) ret =cqspi_request_mmap_dma (cqspi);if (ret == -EPROBE_DEFER ) {dev_err_probe (&pdev->dev, ret,"Failed to request mmap DMA\n" ); - goto disable_controller; + goto disable_rpm; } }
@@ -1999,14 +1999,13 @@ static int cqspi_probe(struct platform_device *pdev) release_dma_chan:if (cqspi->rx_chan)dma_release_channel (cqspi->rx_chan); -disable_controller: +disable_rpm: +if (!(ddata && (ddata->quirks &CQSPI_DISABLE_RUNTIME_PM ))) +pm_runtime_disable (dev);cqspi_controller_enable (cqspi,0 ); disable_clks:if (pm_runtime_get_sync (&pdev->dev) >=0 )clk_bulk_disable_unprepare (CLK_QSPI_NUM , cqspi->clks); -disable_rpm: -if (!(ddata && (ddata->quirks &CQSPI_DISABLE_RUNTIME_PM ))) -pm_runtime_disable (dev);return ret; }
Signature · 1 line
2.52.0
Re: [PATCH 1/6] spi: cadence-quadspi: fix runtime pm disable imbalance on probe failure
Hi,
Quoted 8 lines · Johan Hovold
A recent attempt to fix the probe error handling introduced a runtime PM disable depth imbalance by incorrectly disabling runtime PM on early failures (e.g. probe deferral).
Fixes f18c8cfa4f1a ("spi: cadence-qspi: Fix probe error path and remove")Cc [email protected] # 7.0Cc Miquel Raynal (Schneider Electric) <[email protected]>Signed-off-by Johan Hovold <[email protected]>
Reading it again, I probably got confused by the impact of pm_runtime_set_active(), what you propose looks correct.
Thanks,
Miquèl
PATCH 2/6 spi: cadence-quadspi: fix clock imbalance on probe failure
In reply to Johan HovoldDrop the bogus runtime PM get on probe failures that was never needed and that leaks a usage count reference while preventing the clocks from being disabled (as runtime PM has not yet been enabled).
--- drivers/spi/spi-cadence-quadspi.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
drivers/spi/spi-cadence-quadspi.c +1 -2
@@ -2004,8 +2004,7 @@ static int cqspi_probe(struct platform_device *pdev)pm_runtime_disable (dev);cqspi_controller_enable (cqspi,0 ); disable_clks: -if (pm_runtime_get_sync (&pdev->dev) >=0 ) -clk_bulk_disable_unprepare (CLK_QSPI_NUM , cqspi->clks); +clk_bulk_disable_unprepare (CLK_QSPI_NUM , cqspi->clks);return ret; }
Signature · 1 line
2.52.0
PATCH 3/6 spi: cadence-quadspi: fix unclocked access on unbind
In reply to Johan HovoldMake sure that the controller is runtime resumed before disabling it during driver unbind to avoid an unclocked register access.
This issue was flagged by Sashiko when reviewing a controller deregistration fix.
--- drivers/spi/spi-cadence-quadspi.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-)
drivers/spi/spi-cadence-quadspi.c +3 -4
@@ -2028,14 +2028,13 @@ static void cqspi_remove(struct platform_device *pdev)if (cqspi->rx_chan)dma_release_channel (cqspi->rx_chan); -cqspi_controller_enable (cqspi,0 ); - -if (!(ddata && (ddata->quirks &CQSPI_DISABLE_RUNTIME_PM ))) ret =pm_runtime_get_sync (&pdev->dev); -if (ret >=0 ) +if (ret >=0 ) { +cqspi_controller_enable (cqspi,0 );clk_bulk_disable_unprepare (CLK_QSPI_NUM , cqspi->clks); + }if (!(ddata && (ddata->quirks &CQSPI_DISABLE_RUNTIME_PM ))) {pm_runtime_put_sync (&pdev->dev);
Signature · 1 line
2.52.0
PATCH 4/6 spi: cadence-quadspi: fix runtime pm and clock imbalance on unbind
In reply to Johan HovoldMake sure to balance the runtime PM usage count before returning on probe failure (to allow the controller to suspend after a probe deferral) and to only drop the usage count on driver unbind to avoid a clock disable imbalance.
Also restore the autosuspend setting.
--- drivers/spi/spi-cadence-quadspi.c | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-)
drivers/spi/spi-cadence-quadspi.c +10 -7
@@ -1864,10 +1864,6 @@ static int cqspi_probe(struct platform_device *pdev)if (irq <0 )return -ENXIO ; - ret =pm_runtime_set_active (dev); -if (ret) -return ret; - ret =clk_bulk_prepare_enable (CLK_QSPI_NUM , cqspi->clks);if (ret) {dev_err (dev,"Cannot enable QSPI clocks.\n" );
@@ -1966,10 +1962,11 @@ static int cqspi_probe(struct platform_device *pdev) cqspi->sclk =0 ;if (!(ddata && (ddata->quirks &CQSPI_DISABLE_RUNTIME_PM ))) { -pm_runtime_enable (dev);pm_runtime_set_autosuspend_delay (dev,CQSPI_AUTOSUSPEND_TIMEOUT );pm_runtime_use_autosuspend (dev);pm_runtime_get_noresume (dev); +pm_runtime_set_active (dev); +pm_runtime_enable (dev); } host->num_chipselect = cqspi->num_chipselect;
@@ -2000,8 +1997,12 @@ static int cqspi_probe(struct platform_device *pdev)if (cqspi->rx_chan)dma_release_channel (cqspi->rx_chan); disable_rpm: -if (!(ddata && (ddata->quirks &CQSPI_DISABLE_RUNTIME_PM ))) +if (!(ddata && (ddata->quirks &CQSPI_DISABLE_RUNTIME_PM ))) {pm_runtime_disable (dev); +pm_runtime_set_suspended (dev); +pm_runtime_put_noidle (dev); +pm_runtime_dont_use_autosuspend (dev); + }cqspi_controller_enable (cqspi,0 ); disable_clks:clk_bulk_disable_unprepare (CLK_QSPI_NUM , cqspi->clks);
@@ -2037,8 +2038,10 @@ static void cqspi_remove(struct platform_device *pdev) }if (!(ddata && (ddata->quirks &CQSPI_DISABLE_RUNTIME_PM ))) { -pm_runtime_put_sync (&pdev->dev);pm_runtime_disable (&pdev->dev); +pm_runtime_set_suspended (&pdev->dev); +pm_runtime_put_noidle (&pdev->dev); +pm_runtime_dont_use_autosuspend (&pdev->dev); } }
Signature · 1 line
2.52.0
PATCH 5/6 spi: cadence-quadspi: clean up disable runtime pm quirk
In reply to Johan HovoldCommit 30dbc1c8d50f ("spi: cadence-qspi: defer runtime support on socfpga if reset bit is enabled") fixed a warm reset issue on SoCFPGA by disabling runtime PM on that platform.
Clean up the quirk implementation by never dropping the runtime PM usage count on probe instead of sprinkling conditionals throughout the driver which makes the code unnecessarily hard to read and maintain.
--- drivers/spi/spi-cadence-quadspi.c | 47 +++++++++++++------------------ 1 file changed, 19 insertions(+), 28 deletions(-)
drivers/spi/spi-cadence-quadspi.c +19 -28
@@ -1478,7 +1478,6 @@ static int cqspi_exec_mem_op(struct spi_mem *mem, const struct spi_mem_op *op)int ret;struct cqspi_st *cqspi =spi_controller_get_devdata (mem->spi->controller);struct device *dev = &cqspi->pdev->dev; -const struct cqspi_driver_platdata *ddata =of_device_get_match_data (dev);if (refcount_read (&cqspi->inflight_ops) ==0 )return -ENODEV ;
@@ -1494,18 +1493,15 @@ static int cqspi_exec_mem_op(struct spi_mem *mem, const struct spi_mem_op *op)return -EBUSY ; } -if (!(ddata && (ddata->quirks &CQSPI_DISABLE_RUNTIME_PM ))) { - ret =pm_runtime_resume_and_get (dev); -if (ret) { -dev_err (&mem->spi->dev,"resume failed with %d\n" , ret); - goto dec_inflight_refcount; - } + ret =pm_runtime_resume_and_get (dev); +if (ret) { +dev_err (&mem->spi->dev,"resume failed with %d\n" , ret); + goto dec_inflight_refcount; } ret =cqspi_mem_process (mem, op); -if (!(ddata && (ddata->quirks &CQSPI_DISABLE_RUNTIME_PM ))) -pm_runtime_put_autosuspend (dev); +pm_runtime_put_autosuspend (dev);if (ret)dev_err (&mem->spi->dev,"operation failed with %d\n" , ret);
@@ -1961,13 +1957,11 @@ static int cqspi_probe(struct platform_device *pdev) cqspi->current_cs =-1 ; cqspi->sclk =0 ; -if (!(ddata && (ddata->quirks &CQSPI_DISABLE_RUNTIME_PM ))) { -pm_runtime_set_autosuspend_delay (dev,CQSPI_AUTOSUSPEND_TIMEOUT ); -pm_runtime_use_autosuspend (dev); -pm_runtime_get_noresume (dev); -pm_runtime_set_active (dev); -pm_runtime_enable (dev); - } +pm_runtime_set_autosuspend_delay (dev,CQSPI_AUTOSUSPEND_TIMEOUT ); +pm_runtime_use_autosuspend (dev); +pm_runtime_get_noresume (dev); +pm_runtime_set_active (dev); +pm_runtime_enable (dev); host->num_chipselect = cqspi->num_chipselect;
@@ -1997,12 +1991,11 @@ static int cqspi_probe(struct platform_device *pdev)if (cqspi->rx_chan)dma_release_channel (cqspi->rx_chan); disable_rpm: -if (!(ddata && (ddata->quirks &CQSPI_DISABLE_RUNTIME_PM ))) { -pm_runtime_disable (dev); -pm_runtime_set_suspended (dev); -pm_runtime_put_noidle (dev); -pm_runtime_dont_use_autosuspend (dev); - } +pm_runtime_disable (dev); +pm_runtime_set_suspended (dev); +pm_runtime_put_noidle (dev); +pm_runtime_dont_use_autosuspend (dev); +cqspi_controller_enable (cqspi,0 ); disable_clks:clk_bulk_disable_unprepare (CLK_QSPI_NUM , cqspi->clks);
@@ -2037,12 +2030,10 @@ static void cqspi_remove(struct platform_device *pdev)clk_bulk_disable_unprepare (CLK_QSPI_NUM , cqspi->clks); } -if (!(ddata && (ddata->quirks &CQSPI_DISABLE_RUNTIME_PM ))) { -pm_runtime_disable (&pdev->dev); -pm_runtime_set_suspended (&pdev->dev); -pm_runtime_put_noidle (&pdev->dev); -pm_runtime_dont_use_autosuspend (&pdev->dev); - } +pm_runtime_disable (&pdev->dev); +pm_runtime_set_suspended (&pdev->dev); +pm_runtime_put_noidle (&pdev->dev); +pm_runtime_dont_use_autosuspend (&pdev->dev); }static int cqspi_runtime_suspend (struct device *dev)
Signature · 1 line
2.52.0
PATCH 6/6 spi: cadence-quadspi: drop redundant match data lookup
In reply to Johan HovoldUse the OF match data stored at probe instead of looking it up again on driver unbind.
--- drivers/spi/spi-cadence-quadspi.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-)
drivers/spi/spi-cadence-quadspi.c +1 -4
@@ -2005,13 +2005,10 @@ static int cqspi_probe(struct platform_device *pdev)static void cqspi_remove (struct platform_device *pdev) { -const struct cqspi_driver_platdata *ddata;struct cqspi_st *cqspi =platform_get_drvdata (pdev); -struct device *dev = &pdev->dev; +const struct cqspi_driver_platdata *ddata = cqspi->ddata;int ret =0 ; - ddata =of_device_get_match_data (dev); -spi_unregister_controller (cqspi->host);refcount_set (&cqspi->refcount,0 );
Signature · 1 line
2.52.0
Mark Brown
maintainerQuoted 8 lines · Johan Hovold
spi: cadence-quadspi: runtime PM fixes
This series fixes some runtime PM related issues in the cadence-quadspi driver.
Included is also a couple of related cleanups.
[...]
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-7.2
Thanks!
[1/6] spi: cadence-quadspi: fix runtime pm disable imbalance on probe failure
https://git.kernel.org/broonie/spi/c/5ff4d5d1af0c [2/6] spi: cadence-quadspi: fix clock imbalance on probe failure https://git.kernel.org/broonie/spi/c/cba53fe20c18 [3/6] spi: cadence-quadspi: fix unclocked access on unbind https://git.kernel.org/broonie/spi/c/233db2cb14db [4/6] spi: cadence-quadspi: fix runtime pm and clock imbalance on unbind https://git.kernel.org/broonie/spi/c/5e8bb0cc72f1 [5/6] spi: cadence-quadspi: clean up disable runtime pm quirk https://git.kernel.org/broonie/spi/c/edbaae583ead [6/6] spi: cadence-quadspi: drop redundant match data lookup https://git.kernel.org/broonie/spi/c/37c9dfa385db
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