* [PATCH] slimbus: qcom-ngd-ctrl: drop the PM runtime reference after a sync transfer
@ 2026-11-04 2:14 Wentao Liang
2026-11-04 2:21 ` sashiko-bot
2026-11-04 7:41 ` Krzysztof Kozlowski
0 siblings, 2 replies; 3+ messages in thread
From: Wentao Liang @ 2026-11-04 2:14 UTC (permalink / raw)
To: Srinivas Kandagatla; +Cc: linux-arm-msm, linux-sound
qcom_slim_ngd_xfer_msg_sync() takes a runtime PM reference with
pm_runtime_get_sync() and only releases it on the error paths. On a
successful transfer the function returns 0 with the usage counter still
elevated, so the controller can never runtime-suspend again.
Drop the reference on the success path as well.
Fixes: 917809e2280b ("slimbus: ngd: Add qcom SLIMBus NGD driver")
Cc: stable@vger.kernel.org
Assisted-by: DeepSeek-V4.1-Flash and OpenAI Codex
Signed-off-by: Wentao Liang <vulab@iscas.ac.cn>
---
drivers/slimbus/qcom-ngd-ctrl.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/slimbus/qcom-ngd-ctrl.c b/drivers/slimbus/qcom-ngd-ctrl.c
index eb983b6d1815..9bee4b376877 100644
--- a/drivers/slimbus/qcom-ngd-ctrl.c
+++ b/drivers/slimbus/qcom-ngd-ctrl.c
@@ -942,6 +942,9 @@ static int qcom_slim_ngd_xfer_msg_sync(struct slim_controller *ctrl,
ret = -ETIMEDOUT;
goto pm_put;
}
+
+ pm_runtime_put(ctrl->dev);
+
return 0;
pm_put:
--
2.34.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] slimbus: qcom-ngd-ctrl: drop the PM runtime reference after a sync transfer
2026-11-04 2:14 [PATCH] slimbus: qcom-ngd-ctrl: drop the PM runtime reference after a sync transfer Wentao Liang
@ 2026-11-04 2:21 ` sashiko-bot
2026-11-04 7:41 ` Krzysztof Kozlowski
1 sibling, 0 replies; 3+ messages in thread
From: sashiko-bot @ 2026-11-04 2:21 UTC (permalink / raw)
To: Wentao Liang; +Cc: linux-sound
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] slimbus: qcom-ngd-ctrl: runtime PM underflow in qcom_slim_ngd_xfer_msg_sync()
--
commit 95f275b622c76bd63160fe6491c0337ce83609c2
Author: Wentao Liang <vulab@iscas.ac.cn>
slimbus: qcom-ngd-ctrl: drop the PM runtime reference after a sync transfer
This commit adds a pm_runtime_put() call on the success path of
qcom_slim_ngd_xfer_msg_sync() to balance the pm_runtime_get_sync() call made
at the start of the transfer.
> diff --git a/drivers/slimbus/qcom-ngd-ctrl.c b/drivers/slimbus/qcom-ngd-ctrl.c
> index eb983b6d1815f..9bee4b376877e 100644
> --- a/drivers/slimbus/qcom-ngd-ctrl.c
> +++ b/drivers/slimbus/qcom-ngd-ctrl.c
> @@ -942,6 +942,9 @@ static int qcom_slim_ngd_xfer_msg_sync(struct slim_controller *ctrl,
> ret = -ETIMEDOUT;
> goto pm_put;
> }
> +
> + pm_runtime_put(ctrl->dev);
[Severity: High]
Won't this underflow the runtime PM usage counter on ctrl->dev after every
successful synchronous transfer?
Every caller of qcom_slim_ngd_xfer_msg_sync() (qcom_slim_ngd_enable_stream(),
qcom_slim_ngd_disable_stream(), and qcom_slim_ngd_get_laddr()) allocates a
transaction ID via slim_alloc_txn_tid(ctrl, &txn).
When the reply or ACK (SLIM_USR_MC_ADDR_REPLY or SLIM_USR_MC_GENERIC_ACK)
arrives in qcom_slim_ngd_rx(), it invokes slim_msg_response(), which already
completes txn->comp (&done) and drops the runtime PM reference:
drivers/slimbus/qcom-ngd-ctrl.c:qcom_slim_ngd_rx() {
...
if (mc == SLIM_MSG_MC_REPLY_INFORMATION ||
mc == SLIM_MSG_MC_REPLY_VALUE || (mc == SLIM_USR_MC_ADDR_REPLY &&
mt == SLIM_MSG_MT_SRC_REFERRED_USER) ||
(mc == SLIM_USR_MC_GENERIC_ACK &&
mt == SLIM_MSG_MT_SRC_REFERRED_USER)) {
slim_msg_response(&ctrl->ctrl, &buf[4], buf[3], len - 4);
pm_runtime_mark_last_busy(ctrl->ctrl.dev);
}
}
drivers/slimbus/messaging.c:slim_msg_response() {
...
slim_free_txn_tid(ctrl, txn);
memcpy(msg->rbuf, reply, len);
if (txn->comp)
complete(txn->comp);
/* Remove runtime-pm vote now that response was received for TID txn */
pm_runtime_mark_last_busy(ctrl->dev);
pm_runtime_put_autosuspend(ctrl->dev);
}
Because slim_msg_response() drops the runtime PM reference when the response
is received, only the error and timeout paths in qcom_slim_ngd_xfer_msg_sync()
(where slim_msg_response() is not reached, as handled in commit 63c60a226c90)
should call pm_runtime_put(ctrl->dev). Calling pm_runtime_put(ctrl->dev) on
the success path releases the single pm_runtime_get_sync(ctrl->dev) reference
a second time, which can cause premature runtime suspension of the controller.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261104021424.1468172-1-vulab@iscas.ac.cn?part=1
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] slimbus: qcom-ngd-ctrl: drop the PM runtime reference after a sync transfer
2026-11-04 2:14 [PATCH] slimbus: qcom-ngd-ctrl: drop the PM runtime reference after a sync transfer Wentao Liang
2026-11-04 2:21 ` sashiko-bot
@ 2026-11-04 7:41 ` Krzysztof Kozlowski
1 sibling, 0 replies; 3+ messages in thread
From: Krzysztof Kozlowski @ 2026-11-04 7:41 UTC (permalink / raw)
To: Wentao Liang; +Cc: Srinivas Kandagatla, linux-sound, linux-arm-msm
On Wed, 04 Nov 2026 02:14:24 +0000, Wentao Liang wrote:
> qcom_slim_ngd_xfer_msg_sync() takes a runtime PM reference with
> pm_runtime_get_sync() and only releases it on the error paths. On a
> successful transfer the function returns 0 with the usage counter still
> elevated, so the controller can never runtime-suspend again.
>
> Drop the reference on the success path as well.
>
> Fixes: 917809e2280b ("slimbus: ngd: Add qcom SLIMBus NGD driver")
> Cc: stable@vger.kernel.org
> Assisted-by: DeepSeek-V4.1-Flash and OpenAI Codex
> Signed-off-by: Wentao Liang <vulab@iscas.ac.cn>
> ---
> drivers/slimbus/qcom-ngd-ctrl.c | 3 +++
> 1 file changed, 3 insertions(+)
>
Multiple things here:
1. Your team ignored completely previous feedback. Multiple times.
2. You use multiple identities with this email address, thus I doubt that we
speak with an actual person.
3. Your bot does not even bother running checkpatch, right?
4. Nothing of this was reproduced nor tested.
5. Finally, the same feedback:
You sent multiple independent patches, to multiple independent
subsystems. The amount of these patches clearly suggest this was
AI generated and most likely not tested.
More importantly, you sent all this work without properly organizing
relevant patches into patchsets. This makes reviewing difficult
and might cause multiple reviewers to address the same issue.
Replying to the entire set is impossible and requires handling each
patch independently, instead of applying or discarding the set.
Maintainers also won't see the bigger picture of your work. Quite
worrying.
This is on the verge of hostile patch: bomb us with so many
contributions, we won't be able to handle them in efficient manner,
like responding ONCE to ask you to slow down. Considering all this
is untested and LLM generated, I have even more doubts whether this
should be considered for review.
Please read kernel documentation BEFORE posting more work. It will
explain you how to identify subsystems, how to organize your work per
subsystem (so a patchset grouping multiple patches with a short cover
letter), how to document the issue discovery method and what was
or was not tested.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-11-04 7:41 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-11-04 2:14 [PATCH] slimbus: qcom-ngd-ctrl: drop the PM runtime reference after a sync transfer Wentao Liang
2026-11-04 2:21 ` sashiko-bot
2026-11-04 7:41 ` Krzysztof Kozlowski
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox