All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sebastian Reichel <sebastian.reichel@collabora.com>
To: Mark Brown <broonie@kernel.org>
Cc: Alexander Shiyan <eagle.alexander923@gmail.com>,
	 linux-spi@vger.kernel.org, linux-rockchip@lists.infradead.org,
	 Heiko Stuebner <heiko@sntech.de>
Subject: Re: [PATCH] spi: rockchip: Fix runtime PM underflow on CS error paths
Date: Sat, 7 Nov 2026 03:35:51 +0100	[thread overview]
Message-ID: <au6LMNctO1fx_Kcl@venus> (raw)
In-Reply-To: <179397711825.3231166.14531785924256368148.b4-review@b4>


[-- Attachment #1.1: Type: text/plain, Size: 2728 bytes --]

Hi,

On Fri, Nov 06, 2026 at 02:58:38PM +0000, Mark Brown wrote:
> > The Rockchip SPI driver manages runtime PM reference counting around
> > chip select (CS) assertion: pm_runtime_get_sync() is called when CS is
> > asserted, and pm_runtime_put() when deasserted. This balanced pair is
> > expected.
> 
> > @@ -256,6 +257,7 @@ static void rockchip_spi_set_cs(struct spi_device *spi, bool enable)
> >  	if (cs_asserted) {
> >  		/* Keep things powered as long as CS is asserted */
> >  		pm_runtime_get_sync(rs->dev);
> > +		rs->cs_claimed = true;
> 
> This sort of boolean tracking of reference counts is extremely error
> prone, and note that there are two chip selects on the device.  This
> should probably at least be per chip select.

I ran into the same symptom while working on RK3588 system suspend
and came up with a way different root cause.

I haven't found the time to investigate more closely, check for a
proper Fixes references e.t.c., hence it has not yet been send. So
take this with a grain of salt, but it seems to be not Rockchip
specific:

-----

From: Sebastian Reichel <sebastian.reichel@collabora.com>
Date: Sat, 5 Sep 2026 00:26:40 +0200
Subject: [PATCH] spi: Fix runtime PM imbalance on pump transfer message error
Cc: kernel@collabora.com

The current SPI message pumping code may drop the runtime PM twice when
an SPI message cannot be send:

spi_master spi2: noqueue transfer failed
rockchip-spi feb20000.spi: Runtime PM usage count underflow!

Fix the problem by always keeping the runtime PM count increased when
exiting __spi_pump_transfer_message(), as the caller handles decreasing
it.

Note, that the function already keeps the RPM usage counter increased
for the error paths following after the prepare_transfer_hardware step.

Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
 drivers/spi/spi.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 40cda0037ea5..9cd1cabfa0b3 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -1951,7 +1951,6 @@ static int __spi_pump_transfer_message(struct spi_controller *ctlr,
 	if (!was_busy && ctlr->auto_runtime_pm) {
 		ret = pm_runtime_get_sync(ctlr->dev.parent);
 		if (ret < 0) {
-			pm_runtime_put_noidle(ctlr->dev.parent);
 			dev_err(&ctlr->dev, "Failed to power device: %d\n",
 				ret);
 
@@ -1972,9 +1971,6 @@ static int __spi_pump_transfer_message(struct spi_controller *ctlr,
 				"failed to prepare transfer hardware: %d\n",
 				ret);
 
-			if (ctlr->auto_runtime_pm)
-				pm_runtime_put(ctlr->dev.parent);
-
 			msg->status = ret;
 			spi_finalize_current_message(ctlr);
 
-- 
2.53.0


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

[-- Attachment #2: Type: text/plain, Size: 170 bytes --]

_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

WARNING: multiple messages have this Message-ID (diff)
From: Sebastian Reichel <sebastian.reichel@collabora.com>
To: Mark Brown <broonie@kernel.org>
Cc: Alexander Shiyan <eagle.alexander923@gmail.com>,
	 linux-spi@vger.kernel.org, linux-rockchip@lists.infradead.org,
	 Heiko Stuebner <heiko@sntech.de>
Subject: Re: [PATCH] spi: rockchip: Fix runtime PM underflow on CS error paths
Date: Sat, 7 Nov 2026 03:35:51 +0100	[thread overview]
Message-ID: <au6LMNctO1fx_Kcl@venus> (raw)
In-Reply-To: <179397711825.3231166.14531785924256368148.b4-review@b4>

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

Hi,

On Fri, Nov 06, 2026 at 02:58:38PM +0000, Mark Brown wrote:
> > The Rockchip SPI driver manages runtime PM reference counting around
> > chip select (CS) assertion: pm_runtime_get_sync() is called when CS is
> > asserted, and pm_runtime_put() when deasserted. This balanced pair is
> > expected.
> 
> > @@ -256,6 +257,7 @@ static void rockchip_spi_set_cs(struct spi_device *spi, bool enable)
> >  	if (cs_asserted) {
> >  		/* Keep things powered as long as CS is asserted */
> >  		pm_runtime_get_sync(rs->dev);
> > +		rs->cs_claimed = true;
> 
> This sort of boolean tracking of reference counts is extremely error
> prone, and note that there are two chip selects on the device.  This
> should probably at least be per chip select.

I ran into the same symptom while working on RK3588 system suspend
and came up with a way different root cause.

I haven't found the time to investigate more closely, check for a
proper Fixes references e.t.c., hence it has not yet been send. So
take this with a grain of salt, but it seems to be not Rockchip
specific:

-----

From: Sebastian Reichel <sebastian.reichel@collabora.com>
Date: Sat, 5 Sep 2026 00:26:40 +0200
Subject: [PATCH] spi: Fix runtime PM imbalance on pump transfer message error
Cc: kernel@collabora.com

The current SPI message pumping code may drop the runtime PM twice when
an SPI message cannot be send:

spi_master spi2: noqueue transfer failed
rockchip-spi feb20000.spi: Runtime PM usage count underflow!

Fix the problem by always keeping the runtime PM count increased when
exiting __spi_pump_transfer_message(), as the caller handles decreasing
it.

Note, that the function already keeps the RPM usage counter increased
for the error paths following after the prepare_transfer_hardware step.

Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---
 drivers/spi/spi.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 40cda0037ea5..9cd1cabfa0b3 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -1951,7 +1951,6 @@ static int __spi_pump_transfer_message(struct spi_controller *ctlr,
 	if (!was_busy && ctlr->auto_runtime_pm) {
 		ret = pm_runtime_get_sync(ctlr->dev.parent);
 		if (ret < 0) {
-			pm_runtime_put_noidle(ctlr->dev.parent);
 			dev_err(&ctlr->dev, "Failed to power device: %d\n",
 				ret);
 
@@ -1972,9 +1971,6 @@ static int __spi_pump_transfer_message(struct spi_controller *ctlr,
 				"failed to prepare transfer hardware: %d\n",
 				ret);
 
-			if (ctlr->auto_runtime_pm)
-				pm_runtime_put(ctlr->dev.parent);
-
 			msg->status = ret;
 			spi_finalize_current_message(ctlr);
 
-- 
2.53.0


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

  reply	other threads:[~2026-11-07  2:36 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-06 19:18 [PATCH] spi: rockchip: Fix runtime PM underflow on CS error paths Alexander Shiyan
2026-08-06 19:18 ` Alexander Shiyan
2026-11-06 14:58 ` Mark Brown
2026-11-06 14:58   ` Mark Brown
2026-11-07  2:35   ` Sebastian Reichel [this message]
2026-11-07  2:35     ` Sebastian Reichel
2026-11-07 12:33     ` Mark Brown
2026-11-07 12:33       ` Mark Brown

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=au6LMNctO1fx_Kcl@venus \
    --to=sebastian.reichel@collabora.com \
    --cc=broonie@kernel.org \
    --cc=eagle.alexander923@gmail.com \
    --cc=heiko@sntech.de \
    --cc=linux-rockchip@lists.infradead.org \
    --cc=linux-spi@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.