Re: [PATCH v3 1/2] PCI: dwc: Force L2 link entry on shutdown/reboot without D3cold check

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Forcing PME_Turn_Off and cutting VAUX during shutdown unconditionally violates the PCIe Base Specification and breaks S5 Wake-on-LAN.
--

--- Patch [1]: [PATCH v3 1/2] PCI: dwc: Force L2 link entry on shutdown/reboot without D3cold check ---
commit a2c00716f7d3b58c02f9dc631c72c6f185e30be1
Author: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>

PCI: dwc: Force L2 link entry on shutdown/reboot without D3cold check

This commit modifies dw_pcie_suspend_noirq() to bypass the D3cold
capability check during shutdown or reboot. It unconditionally forces
the link into L2 state to avoid delays or failures when endpoints are
still in D0.

> diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
> index 06722259d2e37..98cd9dd2c7d79 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
[ ... ]
> @@ -1222,16 +1223,29 @@ static int dw_pcie_pme_turn_off(struct dw_pcie *pci)
>  
>  int dw_pcie_suspend_noirq(struct dw_pcie *pci)
>  {
> -	bool pme_capable = false;
> +	bool shutdown = system_state == SYSTEM_HALT ||
> +			system_state == SYSTEM_POWER_OFF ||
> +			system_state == SYSTEM_RESTART;
> +	bool d3cold, pme_capable = false;
>  	int ret = 0;
>  	u32 val;
>  
>  	if (!dw_pcie_link_up(pci))
>  		goto stop_link;
>  
> -	if (!pci_host_common_d3cold_possible(pci->pp.bridge, &pme_capable))
> +	/*
> +	 * During reboot/halt/poweroff the link is going away regardless, so
> +	 * force L2 entry without checking whether endpoints have transitioned
> +	 * to D3hot -- there's no point walking the bus to find out.
> +	 */
> +	if (shutdown)
> +		goto d3cold;

[Severity: High]
Does skipping the D3hot verification and jumping straight to d3cold cause
the controller to broadcast a PME_Turn_Off message to endpoints that might
still be in D0?

Section 5.3.3.2.1 of the PCIe Base Specification mandates that all downstream
devices must be in D3hot or D3cold before software sends this message.

Also, since jumping to d3cold skips the call to
pci_host_common_d3cold_possible(), pme_capable remains false. Will this
cause the driver to incorrectly set skip_pwrctrl_off to false later in the
function:

stop_link:
        ...
        pci->pp.skip_pwrctrl_off = pme_capable;

and unconditionally cut Vaux power to the bus, breaking S5 wake-on-LAN for
devices that were prepared for S5 wakeup?

> +
> +	d3cold = pci_host_common_d3cold_possible(pci->pp.bridge, &pme_capable);
> +	if (!d3cold)
>  		return 0;
>  
> +d3cold:
>  	if (pci->pp.ops->pme_turn_off) {
>  		pci->pp.ops->pme_turn_off(&pci->pp);
>  	} else {

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260824-shutdown-v3-0-81c14bb7a1af@oss.qualcomm.com?part=1