[PATCH v3 1/1] spi: spi-cadence-quadspi: Fix pm runtime unbalance

[PATCH v3 1/1] spi: spi-cadence-quadspi: Fix pm runtime unbalance

[khairul.anuar.romli]

From: Khairul Anuar Romli <khairul.anuar.romli@altera.com>

Having PM put sync in remove function is causing PM underflow during
remove operation. This is caused by the function, runtime_pm_get_sync,
not being called anywhere during the op. Ensure that calls to
pm_runtime_enable()/pm_runtime_disable() and
pm_runtime_get_sync()/pm_runtime_put_sync() match.

echo 108d2000.spi > /sys/bus/platform/drivers/cadence-qspi/unbind
[   49.644256] Deleting MTD partitions on "108d2000.spi.0":
[   49.649575] Deleting u-boot MTD partition
[   49.684087] Deleting root MTD partition
[   49.724188] cadence-qspi 108d2000.spi: Runtime PM usage count underflow!

Continuous bind/unbind will result in an "Unbalanced pm_runtime_enable" error.
Subsequent unbind attempts will return a "No such device" error, while bind
attempts will return a "Resource temporarily unavailable" error.

[   47.592434] cadence-qspi 108d2000.spi: Runtime PM usage count underflow!
[   49.592233] cadence-qspi 108d2000.spi: detected FIFO depth (1024) different from config (128)
[   53.232309] cadence-qspi 108d2000.spi: Runtime PM usage count underflow!
[   55.828550] cadence-qspi 108d2000.spi: detected FIFO depth (1024) different from config (128)
[   57.940627] cadence-qspi 108d2000.spi: Runtime PM usage count underflow!
[   59.912490] cadence-qspi 108d2000.spi: detected FIFO depth (1024) different from config (128)
[   61.876243] cadence-qspi 108d2000.spi: Runtime PM usage count underflow!
[   61.883000] platform 108d2000.spi: Unbalanced pm_runtime_enable!
[  532.012270] cadence-qspi 108d2000.spi: probe with driver cadence-qspi failed1

Also, change clk_disable_unprepare() to clk_disable() since continuous
bind and unbind operations will trigger a warning indicating that the clock is
already unprepared.

Fixes: 4892b374c9b7 ("mtd: spi-nor: cadence-quadspi: Add runtime PM support")
cc: stable@vger.kernel.org # 6.6+
Signed-off-by: Khairul Anuar Romli <khairul.anuar.romli@altera.com>
Reviewed-by: Matthew Gerlach <matthew.gerlach@altera.com>
---
Changes in v3:
    - v2 was sent without a "Changes in v2" section.

Changes in v2:
    - Remove the runtime_pm variable from the struct, as it’s not needed for the fix.
---
 drivers/spi/spi-cadence-quadspi.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/spi/spi-cadence-quadspi.c b/drivers/spi/spi-cadence-quadspi.c
index c90462783b3f..506a139fbd2c 100644
--- a/drivers/spi/spi-cadence-quadspi.c
+++ b/drivers/spi/spi-cadence-quadspi.c
@@ -1958,10 +1958,10 @@ static int cqspi_probe(struct platform_device *pdev)
 			goto probe_setup_failed;
 	}
 
-	ret = devm_pm_runtime_enable(dev);
-	if (ret) {
-		if (cqspi->rx_chan)
-			dma_release_channel(cqspi->rx_chan);
+	pm_runtime_enable(dev);
+
+	if (cqspi->rx_chan) {
+		dma_release_channel(cqspi->rx_chan);
 		goto probe_setup_failed;
 	}
 
@@ -1981,6 +1981,7 @@ static int cqspi_probe(struct platform_device *pdev)
 	return 0;
 probe_setup_failed:
 	cqspi_controller_enable(cqspi, 0);
+	pm_runtime_disable(dev);
 probe_reset_failed:
 	if (cqspi->is_jh7110)
 		cqspi_jh7110_disable_clk(pdev, cqspi);
@@ -1999,7 +2000,8 @@ static void cqspi_remove(struct platform_device *pdev)
 	if (cqspi->rx_chan)
 		dma_release_channel(cqspi->rx_chan);
 
-	clk_disable_unprepare(cqspi->clk);
+	if (pm_runtime_get_sync(&pdev->dev) >= 0)
+		clk_disable(cqspi->clk);
 
 	if (cqspi->is_jh7110)
 		cqspi_jh7110_disable_clk(pdev, cqspi);
-- 
2.35.3

Re: [PATCH v3 1/1] spi: spi-cadence-quadspi: Fix pm runtime unbalance

[Mark Brown]

On Mon, 16 Jun 2025 09:13:53 +0800, khairul.anuar.romli@altera.com wrote:
> Having PM put sync in remove function is causing PM underflow during
> remove operation. This is caused by the function, runtime_pm_get_sync,
> not being called anywhere during the op. Ensure that calls to
> pm_runtime_enable()/pm_runtime_disable() and
> pm_runtime_get_sync()/pm_runtime_put_sync() match.
> 
> echo 108d2000.spi > /sys/bus/platform/drivers/cadence-qspi/unbind
> [   49.644256] Deleting MTD partitions on "108d2000.spi.0":
> [   49.649575] Deleting u-boot MTD partition
> [   49.684087] Deleting root MTD partition
> [   49.724188] cadence-qspi 108d2000.spi: Runtime PM usage count underflow!
> 
> [...]

Applied to

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

Thanks!

[1/1] spi: spi-cadence-quadspi: Fix pm runtime unbalance
      commit: b07f349d1864abe29436f45e3047da2bdd476462

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

Re: [PATCH v3 1/1] spi: spi-cadence-quadspi: Fix pm runtime unbalance

[Dan Carpenter]

On Mon, Jun 16, 2025 at 09:13:53AM +0800, khairul.anuar.romli@altera.com wrote:
> From: Khairul Anuar Romli <khairul.anuar.romli@altera.com>
> 
> Having PM put sync in remove function is causing PM underflow during
> remove operation. This is caused by the function, runtime_pm_get_sync,
> not being called anywhere during the op. Ensure that calls to
> pm_runtime_enable()/pm_runtime_disable() and
> pm_runtime_get_sync()/pm_runtime_put_sync() match.
> 
> echo 108d2000.spi > /sys/bus/platform/drivers/cadence-qspi/unbind
> [   49.644256] Deleting MTD partitions on "108d2000.spi.0":
> [   49.649575] Deleting u-boot MTD partition
> [   49.684087] Deleting root MTD partition
> [   49.724188] cadence-qspi 108d2000.spi: Runtime PM usage count underflow!
> 
> Continuous bind/unbind will result in an "Unbalanced pm_runtime_enable" error.
> Subsequent unbind attempts will return a "No such device" error, while bind
> attempts will return a "Resource temporarily unavailable" error.
> 
> [   47.592434] cadence-qspi 108d2000.spi: Runtime PM usage count underflow!
> [   49.592233] cadence-qspi 108d2000.spi: detected FIFO depth (1024) different from config (128)
> [   53.232309] cadence-qspi 108d2000.spi: Runtime PM usage count underflow!
> [   55.828550] cadence-qspi 108d2000.spi: detected FIFO depth (1024) different from config (128)
> [   57.940627] cadence-qspi 108d2000.spi: Runtime PM usage count underflow!
> [   59.912490] cadence-qspi 108d2000.spi: detected FIFO depth (1024) different from config (128)
> [   61.876243] cadence-qspi 108d2000.spi: Runtime PM usage count underflow!
> [   61.883000] platform 108d2000.spi: Unbalanced pm_runtime_enable!
> [  532.012270] cadence-qspi 108d2000.spi: probe with driver cadence-qspi failed1
> 
> Also, change clk_disable_unprepare() to clk_disable() since continuous
> bind and unbind operations will trigger a warning indicating that the clock is
> already unprepared.
> 
> Fixes: 4892b374c9b7 ("mtd: spi-nor: cadence-quadspi: Add runtime PM support")
> cc: stable@vger.kernel.org # 6.6+
> Signed-off-by: Khairul Anuar Romli <khairul.anuar.romli@altera.com>
> Reviewed-by: Matthew Gerlach <matthew.gerlach@altera.com>
> ---
> Changes in v3:
>     - v2 was sent without a "Changes in v2" section.
> 
> Changes in v2:
>     - Remove the runtime_pm variable from the struct, as it’s not needed for the fix.
> ---
>  drivers/spi/spi-cadence-quadspi.c | 12 +++++++-----
>  1 file changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/spi/spi-cadence-quadspi.c b/drivers/spi/spi-cadence-quadspi.c
> index c90462783b3f..506a139fbd2c 100644
> --- a/drivers/spi/spi-cadence-quadspi.c
> +++ b/drivers/spi/spi-cadence-quadspi.c
> @@ -1958,10 +1958,10 @@ static int cqspi_probe(struct platform_device *pdev)
>  			goto probe_setup_failed;
>  	}
>  
> -	ret = devm_pm_runtime_enable(dev);
> -	if (ret) {
> -		if (cqspi->rx_chan)
> -			dma_release_channel(cqspi->rx_chan);
> +	pm_runtime_enable(dev);
> +
> +	if (cqspi->rx_chan) {
> +		dma_release_channel(cqspi->rx_chan);
>  		goto probe_setup_failed;
>  	}

This will totally break the driver.  It was supposed to be

	if (ret) {
		if (cqspi->rx_chan)
			dma_release_channel(cqspi->rx_chan);
		goto probe_setup_failed;
  	}

In other words, if we failed there was some slightly complicated
cleanup to do.  But now it will do the cleanup and free things on the
success path if we're in cqspi->use_direct_mode.

regards,
dan carpenter


>  
> @@ -1981,6 +1981,7 @@ static int cqspi_probe(struct platform_device *pdev)
>  	return 0;
>  probe_setup_failed:
>  	cqspi_controller_enable(cqspi, 0);
> +	pm_runtime_disable(dev);
>  probe_reset_failed:
>  	if (cqspi->is_jh7110)
>  		cqspi_jh7110_disable_clk(pdev, cqspi);
> @@ -1999,7 +2000,8 @@ static void cqspi_remove(struct platform_device *pdev)
>  	if (cqspi->rx_chan)
>  		dma_release_channel(cqspi->rx_chan);
>  
> -	clk_disable_unprepare(cqspi->clk);
> +	if (pm_runtime_get_sync(&pdev->dev) >= 0)
> +		clk_disable(cqspi->clk);
>  
>  	if (cqspi->is_jh7110)
>  		cqspi_jh7110_disable_clk(pdev, cqspi);
> -- 
> 2.35.3
> 

Re: [PATCH v3 1/1] spi: spi-cadence-quadspi: Fix pm runtime unbalance

[Dan Carpenter]

On Thu, Jun 26, 2025 at 11:37:53PM -0500, Dan Carpenter wrote:
> On Mon, Jun 16, 2025 at 09:13:53AM +0800, khairul.anuar.romli@altera.com wrote:
> > From: Khairul Anuar Romli <khairul.anuar.romli@altera.com>
> > 
> > Having PM put sync in remove function is causing PM underflow during
> > remove operation. This is caused by the function, runtime_pm_get_sync,
> > not being called anywhere during the op. Ensure that calls to
> > pm_runtime_enable()/pm_runtime_disable() and
> > pm_runtime_get_sync()/pm_runtime_put_sync() match.
> > 
> > echo 108d2000.spi > /sys/bus/platform/drivers/cadence-qspi/unbind
> > [   49.644256] Deleting MTD partitions on "108d2000.spi.0":
> > [   49.649575] Deleting u-boot MTD partition
> > [   49.684087] Deleting root MTD partition
> > [   49.724188] cadence-qspi 108d2000.spi: Runtime PM usage count underflow!
> > 
> > Continuous bind/unbind will result in an "Unbalanced pm_runtime_enable" error.
> > Subsequent unbind attempts will return a "No such device" error, while bind
> > attempts will return a "Resource temporarily unavailable" error.
> > 
> > [   47.592434] cadence-qspi 108d2000.spi: Runtime PM usage count underflow!
> > [   49.592233] cadence-qspi 108d2000.spi: detected FIFO depth (1024) different from config (128)
> > [   53.232309] cadence-qspi 108d2000.spi: Runtime PM usage count underflow!
> > [   55.828550] cadence-qspi 108d2000.spi: detected FIFO depth (1024) different from config (128)
> > [   57.940627] cadence-qspi 108d2000.spi: Runtime PM usage count underflow!
> > [   59.912490] cadence-qspi 108d2000.spi: detected FIFO depth (1024) different from config (128)
> > [   61.876243] cadence-qspi 108d2000.spi: Runtime PM usage count underflow!
> > [   61.883000] platform 108d2000.spi: Unbalanced pm_runtime_enable!
> > [  532.012270] cadence-qspi 108d2000.spi: probe with driver cadence-qspi failed1
> > 
> > Also, change clk_disable_unprepare() to clk_disable() since continuous
> > bind and unbind operations will trigger a warning indicating that the clock is
> > already unprepared.
> > 
> > Fixes: 4892b374c9b7 ("mtd: spi-nor: cadence-quadspi: Add runtime PM support")
> > cc: stable@vger.kernel.org # 6.6+
> > Signed-off-by: Khairul Anuar Romli <khairul.anuar.romli@altera.com>
> > Reviewed-by: Matthew Gerlach <matthew.gerlach@altera.com>
> > ---
> > Changes in v3:
> >     - v2 was sent without a "Changes in v2" section.
> > 
> > Changes in v2:
> >     - Remove the runtime_pm variable from the struct, as it’s not needed for the fix.
> > ---
> >  drivers/spi/spi-cadence-quadspi.c | 12 +++++++-----
> >  1 file changed, 7 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/spi/spi-cadence-quadspi.c b/drivers/spi/spi-cadence-quadspi.c
> > index c90462783b3f..506a139fbd2c 100644
> > --- a/drivers/spi/spi-cadence-quadspi.c
> > +++ b/drivers/spi/spi-cadence-quadspi.c
> > @@ -1958,10 +1958,10 @@ static int cqspi_probe(struct platform_device *pdev)
> >  			goto probe_setup_failed;
> >  	}
> >  
> > -	ret = devm_pm_runtime_enable(dev);
> > -	if (ret) {
> > -		if (cqspi->rx_chan)
> > -			dma_release_channel(cqspi->rx_chan);
> > +	pm_runtime_enable(dev);
> > +
> > +	if (cqspi->rx_chan) {
> > +		dma_release_channel(cqspi->rx_chan);
> >  		goto probe_setup_failed;
> >  	}
> 
> This will totally break the driver.  It was supposed to be
> 
> 	if (ret) {
> 		if (cqspi->rx_chan)
> 			dma_release_channel(cqspi->rx_chan);
> 		goto probe_setup_failed;
>   	}
> 
> In other words, if we failed there was some slightly complicated
> cleanup to do.  But now it will do the cleanup and free things on the
> success path if we're in cqspi->use_direct_mode.
> 

I suck at email.  What I meant was delete the if block:

-	if (cqspi->rx_chan) {
-		dma_release_channel(cqspi->rx_chan);
-		goto probe_setup_failed;
-	}
-

regards,
dan carpenter

Re: [PATCH v3 1/1] spi: spi-cadence-quadspi: Fix pm runtime unbalance

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 582 bytes --]

On Fri, Jun 27, 2025 at 07:39:24AM +0300, Dan Carpenter wrote:
> On Thu, Jun 26, 2025 at 11:37:53PM -0500, Dan Carpenter wrote:

> > In other words, if we failed there was some slightly complicated
> > cleanup to do.  But now it will do the cleanup and free things on the
> > success path if we're in cqspi->use_direct_mode.

> I suck at email.  What I meant was delete the if block:

> -	if (cqspi->rx_chan) {
> -		dma_release_channel(cqspi->rx_chan);
> -		goto probe_setup_failed;
> -	}
> -

Can you submit a fix for this please?  The patch is already applied and
in Linus' tree.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

RE: [PATCH v3 1/1] spi: spi-cadence-quadspi: Fix pm runtime unbalance

[Romli, Khairul Anuar]

> To: Dan Carpenter <dan.carpenter@linaro.org>
> Cc: Romli, Khairul Anuar <khairul.anuar.romli@altera.com>; open list:SPI
> SUBSYSTEM <linux-spi@vger.kernel.org>; open list <linux-
> kernel@vger.kernel.org>; Gerlach, Matthew <matthew.gerlach@altera.com>;
> Romli, Khairul Anuar <khairul.anuar.romli@altera.com>
> Subject: Re: [PATCH v3 1/1] spi: spi-cadence-quadspi: Fix pm runtime
> unbalance
> 
> On Fri, Jun 27, 2025 at 07:39:24AM +0300, Dan Carpenter wrote:
> > On Thu, Jun 26, 2025 at 11:37:53PM -0500, Dan Carpenter wrote:
> 
> > > In other words, if we failed there was some slightly complicated
> > > cleanup to do.  But now it will do the cleanup and free things on
> > > the success path if we're in cqspi->use_direct_mode.
> 
> > I suck at email.  What I meant was delete the if block:
> 
> > -	if (cqspi->rx_chan) {
> > -		dma_release_channel(cqspi->rx_chan);
> > -		goto probe_setup_failed;
> > -	}
> > -
> 
> Can you submit a fix for this please?  The patch is already applied and in Linus'
> tree.

I'll submit the fix for this.

Re: [PATCH v3 1/1] spi: spi-cadence-quadspi: Fix pm runtime unbalance

[Michael Walle]

[-- Attachment #1: Type: text/plain, Size: 1037 bytes --]

Hi,

> > > -	ret = devm_pm_runtime_enable(dev);
> > > -	if (ret) {
> > > -		if (cqspi->rx_chan)
> > > -			dma_release_channel(cqspi->rx_chan);
> > > +	pm_runtime_enable(dev);
> > > +
> > > +	if (cqspi->rx_chan) {
> > > +		dma_release_channel(cqspi->rx_chan);
> > >  		goto probe_setup_failed;
> > >  	}
> > 
> > This will totally break the driver.  It was supposed to be

Yeah. I've just stumbled on that.

> > 
> > 	if (ret) {
> > 		if (cqspi->rx_chan)
> > 			dma_release_channel(cqspi->rx_chan);
> > 		goto probe_setup_failed;
> >   	}
> > 
> > In other words, if we failed there was some slightly complicated
> > cleanup to do.  But now it will do the cleanup and free things on the
> > success path if we're in cqspi->use_direct_mode.
> > 
>
> I suck at email.  What I meant was delete the if block:
>
> -	if (cqspi->rx_chan) {
> -		dma_release_channel(cqspi->rx_chan);
> -		goto probe_setup_failed;
> -	}
> -

Shouldn't the DMA channel be freed if spi_register_controller()
fails?

-michael

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 297 bytes --]