Re: [PATCH v2] PCI: rcar-gen4: Add missing PM ops
From: sashiko-bot@kernel.org
Date: 2026-09-07 15:54:38
Also in:
linux-pci
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [High] Uninitialized variable dw->mode causes suspend/resume callbacks to be silently skipped. - [Low] Missing `static` keyword on `rcar_gen4_pcie_pm_ops` definition. -- commit 0e99d69c4bfd5a0067af46f15d0ef68cd860b192 Author: Marek Vasut [off-list ref] PCI: rcar-gen4: Add missing PM ops The R-Car Gen4 PCIe controller is part of a power domain that is shut down on V4M during suspend, losing state. This patch adds generic suspend/resume noirq ops to tear down and restart the link on suspend/resume.
quoted hunk ↗ jump to hunk
diff --git a/drivers/pci/controller/dwc/pcie-rcar-gen4.c b/drivers/pci/controller/dwc/pcie-rcar-gen4.c index fbe465a29068f..47754aee84278 100644 --- a/drivers/pci/controller/dwc/pcie-rcar-gen4.c +++ b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
[ ... ]
quoted hunk ↗ jump to hunk
@@ -849,6 +849,28 @@ static int rcar_gen4_pcie_ltssm_control(struct rcar_gen4_pcie *rcar, bool enable return 0; } +static int rcar_gen4_pcie_suspend_noirq(struct device *dev) +{ + struct rcar_gen4_pcie *rcar = dev_get_drvdata(dev); + struct dw_pcie *dw = &rcar->dw; + + if (dw->mode != DW_PCIE_RC_TYPE) + return 0;
[Severity: High] Is dw->mode guaranteed to be initialized here when CONFIG_PCIE_DW_DEBUGFS is disabled? It appears dw->mode relies on dwc_pcie_debugfs_init() for initialization. If debugfs is disabled, dwc_pcie_debugfs_init() becomes an empty inline function, leaving dw->mode as 0 (DW_PCIE_UNKNOWN_TYPE). Because 0 != DW_PCIE_RC_TYPE, wouldn't this cause the callbacks to silently bypass the suspend and resume logic entirely on production builds without debugfs, leaving the PCIe link up during suspend and dead after resume?
+
+ return dw_pcie_suspend_noirq(dw);
+}
+
+static int rcar_gen4_pcie_resume_noirq(struct device *dev)
+{
+ struct rcar_gen4_pcie *rcar = dev_get_drvdata(dev);
+ struct dw_pcie *dw = &rcar->dw;
+
+ if (dw->mode != DW_PCIE_RC_TYPE)
+ return 0;[Severity: High] Similar to the suspend callback above, will this also silently skip resuming the link when debugfs is disabled?
+ + return dw_pcie_resume_noirq(dw); +}
[ ... ]
+DEFINE_NOIRQ_DEV_PM_OPS(rcar_gen4_pcie_pm_ops, + rcar_gen4_pcie_suspend_noirq, rcar_gen4_pcie_resume_noirq);
[Severity: Low] Should rcar_gen4_pcie_pm_ops be marked static? The DEFINE_NOIRQ_DEV_PM_OPS macro does not include the static keyword in its expansion. By not explicitly declaring it static here, this creates a global symbol that pollutes the global namespace, even though it is only used locally within this driver file. -- Sashiko AI review · https://sashiko.dev/#/patchset/20260907153711.653861-1-marek.vasut+renesas@mailbox.org?part=1