Re: [RFC][PATCH] PCI: rcar-gen4: Add missing PM ops

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Unconditional calls to host-specific suspend/resume functions crash when the controller operates in Endpoint (EP) mode.
- [High] Suspend/resume deadlock and unsafe GFP_KERNEL allocation via request_firmware() in the resume_noirq path.
--

commit 5af3966526dc17abac6d2f1a9c20afb491499031
Author: Marek Vasut <marek.vasut+renesas@mailbox.org>

PCI: rcar-gen4: Add missing PM ops

The commit adds suspend/resume PM ops to fix an issue where the controller
loses state during suspend on R-Car V4M. It registers generic suspend and
resume handlers to gracefully tear down and restart the PCIe link across
all R-Car Gen4 platforms.

> diff --git a/drivers/pci/controller/dwc/pcie-rcar-gen4.c b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
> index fbe465a29068f..8c3e41e9a8ba1 100644
> --- a/drivers/pci/controller/dwc/pcie-rcar-gen4.c
> +++ b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
> @@ -849,6 +849,22 @@ 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;
> +
> +	return dw_pcie_suspend_noirq(dw);

[Severity: High]
Does this crash when the controller operates in Endpoint mode?

Since the driver registers rcar_gen4_pcie_pm_ops for all matched devices,
including those in Endpoint mode (e.g., "renesas,rcar-gen4-pcie-ep"), this
unconditionally routes the suspend operation to the host-specific
dw_pcie_suspend_noirq() without verifying the controller mode.

In Endpoint mode, dw_pcie_host_init() is never called, leaving the Root
Port (pci->pp) structures uninitialized. If dw_pcie_suspend_noirq() is
called in Endpoint mode, it will dereference pci->pp.bridge which is
strictly NULL:

drivers/pci/controller/dwc/pcie-designware-host.c:dw_pcie_suspend_noirq() {
    ...
	if (!pci_host_common_d3cold_possible(pci->pp.bridge, &pme_capable))
		return 0;
    ...
}

It will also dereference pci->pp.ops during the link-down fallback path:

drivers/pci/controller/dwc/pcie-designware-host.c:dw_pcie_suspend_noirq() {
    ...
stop_link:
    ...
	if (pci->pp.ops->deinit)
		pci->pp.ops->deinit(&pci->pp);
    ...
}

> +}
> +
> +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;
> +
> +	return dw_pcie_resume_noirq(dw);

[Severity: High]
Can this cause a suspend/resume deadlock due to unsafe GFP_KERNEL allocations?

When dw_pcie_resume_noirq() is called, it restarts the link which
ultimately invokes request_firmware() in the resume_noirq phase:

dw_pcie_resume_noirq()
  dw_pcie_start_link()
    rcar_gen4_pcie_start_link()
      rcar_gen4_pcie_ltssm_control()
        rcar_gen4_pcie_download_phy_firmware()
          request_firmware(&fw, RCAR_GEN4_PCIE_FIRMWARE_NAME, dw->dev);

Executing request_firmware() in a noirq phase is strictly unsafe because
device interrupts (including those for storage controllers) are disabled.
If the firmware is not cached, the request will hang indefinitely waiting
for filesystem I/O that cannot complete without interrupts. Even if
cached, request_firmware() allocates memory using GFP_KERNEL, which can
trigger page reclaim and block on disk I/O, similarly deadlocking the
system.

> +}
> +
>  static struct rcar_gen4_pcie_drvdata drvdata_r8a779f0_pcie = {
>  	.ltssm_control = r8a779f0_pcie_ltssm_control,
>  	.mode = DW_PCIE_RC_TYPE,

[ ... ]

> @@ -892,10 +908,14 @@ static const struct of_device_id rcar_gen4_pcie_of_match[] = {
>  };
>  MODULE_DEVICE_TABLE(of, rcar_gen4_pcie_of_match);
>  
> +DEFINE_NOIRQ_DEV_PM_OPS(rcar_gen4_pcie_pm_ops,
> +			rcar_gen4_pcie_suspend_noirq, rcar_gen4_pcie_resume_noirq);
> +
>  static struct platform_driver rcar_gen4_pcie_driver = {
>  	.driver = {
>  		.name = "pcie-rcar-gen4",
>  		.of_match_table = rcar_gen4_pcie_of_match,
> +		.pm = &rcar_gen4_pcie_pm_ops,
>  		.probe_type = PROBE_PREFER_ASYNCHRONOUS,
>  	},
>  	.probe = rcar_gen4_pcie_probe,

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260906173234.522400-1-marek.vasut+renesas@mailbox.org?part=1