Skip to contentSkip to conversation

spi: cadence: runtime PM fixes

PATCH In review Johan Hovold 2 participants 6 messages Apr 21 – Apr 27

Subsystems (2)
Stable branch SPI
lore
Share & tools
lore

Johan Hovold

author
#0 lore raw

cover spi: cadence: runtime PM fixes

This series fixes some runtime PM related issues in the cadence driver.

Included is also a couple of related cleanups.

Johan

Johan Hovold (4):
spi: cadence: fix unclocked access on unbind
spi: cadence: fix clock imbalance on probe failure
spi: cadence: rename probe error labels
spi: cadence: clean up probe return value

 drivers/spi/spi-cadence.c | 37 ++++++++++++++++++++++++-------------
 1 file changed, 24 insertions(+), 13 deletions(-)
Signature · 1 line
2.52.0
author
#1 lore raw

PATCH 1/4 spi: cadence: fix unclocked access on unbind

Make sure that the controller is runtime resumed before disabling it during driver unbind to avoid unclocked register access and unbalanced clock disable.

Also restore the autosuspend setting.

This issue was flagged by Sashiko when reviewing a controller deregistration fix.

Fixes d36ccd9f7ea4 ("spi: cadence: Runtime pm adaptation")
Cc [email protected] # 4.7
Cc Shubhrajyoti Datta <[email protected]>
Link https://sashiko.dev/#/patchset/20260414134319.978196-1-johan%40kernel.org?part=1
Signed-off-by Johan Hovold <[email protected]>
---
 drivers/spi/spi-cadence.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)
drivers/spi/spi-cadence.c +8 -1
index 08d7dabe818d..bf4a7cf6b142 100644
@@ -776,16 +776,23 @@ static void cdns_spi_remove(struct platform_device *pdev)
 {
 	struct spi_controller *ctlr = platform_get_drvdata(pdev);
 	struct cdns_spi *xspi = spi_controller_get_devdata(ctlr);
+	int ret = 0;
+
+	if (!spi_controller_is_target(ctlr))
+		ret = pm_runtime_get_sync(&pdev->dev);
 
 	spi_controller_get(ctlr);
 
 	spi_unregister_controller(ctlr);
 
-	cdns_spi_write(xspi, CDNS_SPI_ER, CDNS_SPI_ER_DISABLE);
+	if (ret >= 0)
+		cdns_spi_write(xspi, CDNS_SPI_ER, CDNS_SPI_ER_DISABLE);
 
 	if (!spi_controller_is_target(ctlr)) {
 		pm_runtime_disable(&pdev->dev);
 		pm_runtime_set_suspended(&pdev->dev);
+		pm_runtime_put_noidle(&pdev->dev);
+		pm_runtime_dont_use_autosuspend(&pdev->dev);
 	}
 
 	spi_controller_put(ctlr);
Signature · 1 line
2.52.0
author
#2 lore raw

PATCH 2/4 spi: cadence: fix clock imbalance on probe failure

In reply to Johan Hovold

Make sure that the controller is active before disabling clocks on probe failure to avoid unbalanced clock disable.

Also drop the usage count before returning (so that the controller can be suspended after a probe deferral) and restore the autosuspend setting.

Fixes d36ccd9f7ea4 ("spi: cadence: Runtime pm adaptation")
Cc [email protected] # 4.7
Cc Shubhrajyoti Datta <[email protected]>
Signed-off-by Johan Hovold <[email protected]>
---
 drivers/spi/spi-cadence.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)
drivers/spi/spi-cadence.c +5 -1
index bf4a7cf6b142..891e2ba36958 100644
@@ -741,7 +741,6 @@ static int cdns_spi_probe(struct platform_device *pdev)
 		/* Set to default valid value */
 		ctlr->max_speed_hz = xspi->clk_rate / 4;
 		xspi->speed_hz = ctlr->max_speed_hz;
-		pm_runtime_put_autosuspend(&pdev->dev);
 	} else {
 		ctlr->mode_bits |= SPI_NO_CS;
 		ctlr->target_abort = cdns_target_abort;

@@ -752,12 +751,17 @@ static int cdns_spi_probe(struct platform_device *pdev)
 		goto clk_dis_all;
 	}
 
+	if (!spi_controller_is_target(ctlr))
+		pm_runtime_put_autosuspend(&pdev->dev);
+
 	return ret;
 
 clk_dis_all:
 	if (!spi_controller_is_target(ctlr)) {
 		pm_runtime_disable(&pdev->dev);
 		pm_runtime_set_suspended(&pdev->dev);
+		pm_runtime_put_noidle(&pdev->dev);
+		pm_runtime_dont_use_autosuspend(&pdev->dev);
 	}
 remove_ctlr:
 	spi_controller_put(ctlr);
Signature · 1 line
2.52.0
author
#3 lore raw

PATCH 3/4 spi: cadence: rename probe error labels

In reply to Johan Hovold

The "clk_dis_all" error label is not used to disable clocks since commit f64b1600f92e ("spi: spi-cadence: Use helper function devm_clk_get_enabled()").

Similarly, "remove_ctlr" drops a reference rather than deregisters the controller.

Rename the labels after what they do.

Signed-off-by Johan Hovold <[email protected]>
---
 drivers/spi/spi-cadence.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)
drivers/spi/spi-cadence.c +9 -9
index 891e2ba36958..f27586151ca9 100644
@@ -656,21 +656,21 @@ static int cdns_spi_probe(struct platform_device *pdev)
 	xspi->regs = devm_platform_ioremap_resource(pdev, 0);
 	if (IS_ERR(xspi->regs)) {
 		ret = PTR_ERR(xspi->regs);
-		goto remove_ctlr;
+		goto err_put_ctlr;
 	}
 
 	xspi->pclk = devm_clk_get_enabled(&pdev->dev, "pclk");
 	if (IS_ERR(xspi->pclk)) {
 		dev_err(&pdev->dev, "pclk clock not found.\n");
 		ret = PTR_ERR(xspi->pclk);
-		goto remove_ctlr;
+		goto err_put_ctlr;
 	}
 
 	xspi->rstc = devm_reset_control_get_optional_exclusive(&pdev->dev, "spi");
 	if (IS_ERR(xspi->rstc)) {
 		ret = dev_err_probe(&pdev->dev, PTR_ERR(xspi->rstc),
 				    "Cannot get SPI reset.\n");
-		goto remove_ctlr;
+		goto err_put_ctlr;
 	}
 
 	reset_control_assert(xspi->rstc);

@@ -680,7 +680,7 @@ static int cdns_spi_probe(struct platform_device *pdev)
 	if (IS_ERR(xspi->ref_clk)) {
 		dev_err(&pdev->dev, "ref_clk clock not found.\n");
 		ret = PTR_ERR(xspi->ref_clk);
-		goto remove_ctlr;
+		goto err_put_ctlr;
 	}
 
 	if (!spi_controller_is_target(ctlr)) {

@@ -710,7 +710,7 @@ static int cdns_spi_probe(struct platform_device *pdev)
 	irq = platform_get_irq(pdev, 0);
 	if (irq < 0) {
 		ret = irq;
-		goto clk_dis_all;
+		goto err_disable_rpm;
 	}
 
 	ret = devm_request_irq(&pdev->dev, irq, cdns_spi_irq,

@@ -718,7 +718,7 @@ static int cdns_spi_probe(struct platform_device *pdev)
 	if (ret != 0) {
 		ret = -ENXIO;
 		dev_err(&pdev->dev, "request_irq failed\n");
-		goto clk_dis_all;
+		goto err_disable_rpm;
 	}
 
 	ctlr->use_gpio_descriptors = true;

@@ -748,7 +748,7 @@ static int cdns_spi_probe(struct platform_device *pdev)
 	ret = spi_register_controller(ctlr);
 	if (ret) {
 		dev_err(&pdev->dev, "spi_register_controller failed\n");
-		goto clk_dis_all;
+		goto err_disable_rpm;
 	}
 
 	if (!spi_controller_is_target(ctlr))

@@ -756,14 +756,14 @@ static int cdns_spi_probe(struct platform_device *pdev)
 
 	return ret;
 
-clk_dis_all:
+err_disable_rpm:
 	if (!spi_controller_is_target(ctlr)) {
 		pm_runtime_disable(&pdev->dev);
 		pm_runtime_set_suspended(&pdev->dev);
 		pm_runtime_put_noidle(&pdev->dev);
 		pm_runtime_dont_use_autosuspend(&pdev->dev);
 	}
-remove_ctlr:
+err_put_ctlr:
 	spi_controller_put(ctlr);
 	return ret;
 }
Signature · 1 line
2.52.0
author
#4 lore raw

PATCH 4/4 spi: cadence: clean up probe return value

In reply to Johan Hovold

Drop the redundant initialisation and return explicit zero on successful probe to make the code more readable.

Signed-off-by Johan Hovold <[email protected]>
---
 drivers/spi/spi-cadence.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/spi/spi-cadence.c +2 -2
index f27586151ca9..d108e89fda22 100644
@@ -635,7 +635,7 @@ static int cdns_target_abort(struct spi_controller *ctlr)
  */
 static int cdns_spi_probe(struct platform_device *pdev)
 {
-	int ret = 0, irq;
+	int ret, irq;
 	struct spi_controller *ctlr;
 	struct cdns_spi *xspi;
 	u32 num_cs;

@@ -754,7 +754,7 @@ static int cdns_spi_probe(struct platform_device *pdev)
 	if (!spi_controller_is_target(ctlr))
 		pm_runtime_put_autosuspend(&pdev->dev);
 
-	return ret;
+	return 0;
 
 err_disable_rpm:
 	if (!spi_controller_is_target(ctlr)) {
Signature · 1 line
2.52.0
maintainer
#5 lore raw
In reply to Johan Hovold
Quoted 9 lines · Johan Hovold

spi: cadence: runtime PM fixes

This series fixes some runtime PM related issues in the cadence driver.

Included is also a couple of related cleanups.

Johan

[...]

Applied to

https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-7.2

Thanks!

[1/4] spi: cadence: fix unclocked access on unbind

      https://git.kernel.org/broonie/spi/c/5b1689a41f02
[2/4] spi: cadence: fix clock imbalance on probe failure
      https://git.kernel.org/broonie/spi/c/ecea4f0e9db2
[3/4] spi: cadence: rename probe error labels
      https://git.kernel.org/broonie/spi/c/63f34e35f87f
[4/4] spi: cadence: clean up probe return value
      https://git.kernel.org/broonie/spi/c/bf7b648acd48

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