All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Mrinmay Sarkar" <mrinmay.sarkar@oss.qualcomm.com>
Cc: linux-pci@vger.kernel.org
Subject: Re: [PATCH] PCI: qcom-ep: Clear runtime PM error after pm_runtime_resume_and_get() fails
Date: Fri, 25 Sep 2026 10:32:54 +0000	[thread overview]
Message-ID: <20260925103254.EC98B1F000FF@smtp.kernel.org> (raw)
In-Reply-To: <20260925-pcie-ep-rpm-fix-v1-1-855d56cce43b@oss.qualcomm.com>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] pm: ineffective pm_runtime_set_suspended() due to false runtime_error assumption
--

commit 3d108ba33ceb10b064e31ea4a0bb76a46a7b974e
Author: Mrinmay Sarkar <mrinmay.sarkar@oss.qualcomm.com>

PCI: qcom-ep: Clear runtime PM error after pm_runtime_resume_and_get() fails

This patch attempts to fix a runtime PM recovery issue in the Qualcomm PCIe
endpoint driver. It adds a call to pm_runtime_set_suspended() on the error
path of pm_runtime_resume_and_get() in qcom_pcie_perst_deassert() to reset
the runtime PM state and allow future resume attempts.

> pm_runtime_resume_and_get() sets dev->power.runtime_error when a runtime
> resume fails. As a result, subsequent runtime PM resume requests fail
> immediately until the error state is cleared.

Is this description accurate?

Looking at the PM core in drivers/base/power/runtime.c, rpm_resume() does not
set dev->power.runtime_error upon failure. Instead, it merely updates
runtime_status to RPM_SUSPENDED. It appears only rpm_suspend() sets
runtime_error when a suspend operation fails.

> diff --git a/drivers/pci/controller/dwc/pcie-qcom-ep.c b/drivers/pci/controller/dwc/pcie-qcom-ep.c
> index 8364696a1b980..5a3dfea702cee 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom-ep.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom-ep.c
> @@ -404,6 +404,7 @@ static int qcom_pcie_perst_deassert(struct dw_pcie *pci)
>  	ret = pm_runtime_resume_and_get(dev);
>  	if (ret < 0) {
>  		dev_err(dev, "Failed to enable device: %d\n", ret);
> +		pm_runtime_set_suspended(dev);

[Severity: Medium]
Will this call to pm_runtime_set_suspended() actually be a silent no-op?

pm_runtime_set_suspended() routes to __pm_runtime_set_status(), which
contains an early check:

    if (dev->power.runtime_error || dev->power.disable_depth)
        dev->power.disable_depth++;
    else
        error = -EAGAIN;
    ...
    if (error)
        return error;

Because runtime_error remains 0 (as the resume failure didn't set it) and
disable_depth is 0 (because runtime PM is enabled), the function returns
-EAGAIN and aborts immediately without clearing any state. Does this leave the
underlying reason for repeated resume failures unfixed?

>  		return ret;
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260925-pcie-ep-rpm-fix-v1-1-855d56cce43b@oss.qualcomm.com?part=1

      reply	other threads:[~2026-09-25 10:32 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-25 10:15 [PATCH] PCI: qcom-ep: Clear runtime PM error after pm_runtime_resume_and_get() fails Mrinmay Sarkar
2026-09-25 10:32 ` sashiko-bot [this message]

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=20260925103254.EC98B1F000FF@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=mrinmay.sarkar@oss.qualcomm.com \
    --cc=sashiko-reviews@lists.linux.dev \
    /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.