Re: [PATCH v4 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] Incomplete evaluation of `pme_capable` during shutdown breaks Wake-on-LAN from S5 by improperly cutting auxiliary power.
--

commit d9876255ad827b6e1f2d6b4bca95ab65c2ea8123
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 detect the shutdown/reboot
case and forces L2 entry without checking if endpoints have transitioned
to D3hot. It unconditionally calls pci_host_common_d3cold_possible() to set
pme_capable but ignores its return value.

> diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
> index f5a38e6fd8d79..5d07a742faa16 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
[ ... ]
> @@ -1224,14 +1225,25 @@ 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. Call
> +	 * pci_host_common_d3cold_possible() anyway (ignoring its return value
> +	 * in the shutdown case) since it's also how "pme_capable" gets set.
> +	 */
> +	d3cold = pci_host_common_d3cold_possible(pci->pp.bridge, &pme_capable);
> +	if (!d3cold && !shutdown)
>  		return 0;
>  
>  	if (pci->pp.ops->pme_turn_off) {

[Severity: High]
The commit message notes that endpoints are frequently left in D0 during
shutdown. If an endpoint is left in D0, won't pci_walk_bus() abort early
during the pci_host_common_d3cold_possible() call?

Looking at __pci_host_common_d3cold_possible() in
drivers/pci/controller/pci-host-common.c, it returns -EOPNOTSUPP if it
encounters a device not in PCI_D3hot:

drivers/pci/controller/pci-host-common.c:__pci_host_common_d3cold_possible() {
    ...
	if (pdev->current_state != PCI_D3hot)
		goto exit;
    ...
exit:
	*flags &= ~PCI_HOST_D3COLD_ALLOWED;

	return -EOPNOTSUPP;
}

Because pci_walk_bus() aborts immediately on a non-zero return, downstream
PME-capable devices won't be evaluated, leaving pme_capable as false.

This would cause the driver to unconditionally cut auxiliary power. Does this
incomplete evaluation of pme_capable break Wake-on-LAN and other PME wakeup
features from the S5 (poweroff) state when devices are left in D0?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260826-shutdown-v4-0-eb5fe9d454ae@oss.qualcomm.com?part=1