Re: [PATCH] PCI: qcom: Skip system suspend/resume for firmware-managed PCIe

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Redundant OF match lookup in latency-sensitive noirq suspend/resume fastpaths
--

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

PCI: qcom: Skip system suspend/resume for firmware-managed PCIe

This commit skips host-managed suspend and resume operations for PCIe
controllers that are managed by firmware. It relies on the generic power
domain framework to handle the necessary transitions instead.

> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> --- a/drivers/pci/controller/dwc/pcie-qcom.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> @@ -2341,6 +2341,10 @@ static int qcom_pcie_suspend_noirq(struct device *dev)
>  {
>  	struct qcom_pcie *pcie;
>  	int ret = 0;
> +	const struct qcom_pcie_cfg *pcie_cfg = of_device_get_match_data(dev);
> +
> +	if (pcie_cfg && pcie_cfg->firmware_managed)
> +		return 0;

[Severity: Low]
Is this new OF match lookup redundant?

Since qcom_pcie_probe() bypasses setting drvdata for firmware-managed
controllers, dev_get_drvdata() already natively returns NULL for these
devices. The pre-existing check just below this addition handles that:

    pcie = dev_get_drvdata(dev);
    if (!pcie)
        return 0;

Because this executes in the noirq phase where local CPU interrupts are
disabled and latency is critical, does it make sense to avoid the overhead
of of_device_get_match_data() string comparisons and just rely on the
existing NULL check?

>  
>  	pcie = dev_get_drvdata(dev);
>  	if (!pcie)

[ ... ]

> @@ -2399,6 +2403,10 @@ static int qcom_pcie_resume_noirq(struct device *dev)
>  {
>  	struct qcom_pcie *pcie;
>  	int ret;
> +	const struct qcom_pcie_cfg *pcie_cfg = of_device_get_match_data(dev);
> +
> +	if (pcie_cfg && pcie_cfg->firmware_managed)
> +		return 0;

[Severity: Low]
Similarly for the resume path, does this introduce unnecessary overhead in
the noirq phase?

The identical dev_get_drvdata() check is present just below this addition,
which already safely catches firmware-managed controllers and returns 0.

>  
>  	pcie = dev_get_drvdata(dev);
>  	if (!pcie)

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260831-skip_suspend-v1-1-ba2d877e22b1@oss.qualcomm.com?part=1