Re: [PATCH] PCI/portdrv: Disallow runtime suspend when waekup is required but PME service isn't supported
From: Lukas Wunner <lukas@wunner.de>
Date: 2021-08-09 09:47:37
Also in:
lkml
[cc += Mika] On Mon, Aug 09, 2021 at 12:24:12PM +0800, Kai-Heng Feng wrote:
Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=213873
The last comment on this bugzilla says "BIOS will fix this." and the status is RESOLVED WILL_NOT_FIX. Why is the patch still necessary?
Some platforms cannot detect ethernet hotplug once its upstream port is runtime suspended because PME isn't enabled in _OSC.
If PME is not handled natively, why does the NIC runtime suspend? Shouldn't this be fixed in the NIC driver by keeping the device runtime active if PME cannot be used?
Disallow port runtime suspend when any child device requires wakeup, so pci_pme_list_scan() can still read the PME status from the devices behind the port.
pci_pme_list_scan() is for broken devices which fail to signal PME. Is this NIC really among them or does PME fail merely because it's not granted to OSPM?
quoted hunk ↗ jump to hunk
--- a/drivers/pci/pcie/portdrv_pci.c +++ b/drivers/pci/pcie/portdrv_pci.c@@ -59,14 +59,30 @@ static int pcie_port_runtime_suspend(struct device *dev) return pcie_port_device_runtime_suspend(dev); } +static int pcie_port_wakeup_check(struct device *dev, void *data) +{ + struct pci_dev *pdev = to_pci_dev(dev); + + if (!pdev) + return 0; + + return pdev->wakeup_prepared; +} + static int pcie_port_runtime_idle(struct device *dev) { + struct pci_dev *pdev = to_pci_dev(dev); + + if (!pcie_port_find_device(pdev, PCIE_PORT_SERVICE_PME) && + device_for_each_child(dev, NULL, pcie_port_wakeup_check)) + return -EBUSY; + /* * Assume the PCI core has set bridge_d3 whenever it thinks the port * should be good to go to D3. Everything else, including moving * the port to D3, is handled by the PCI core. */ - return to_pci_dev(dev)->bridge_d3 ? 0 : -EBUSY; + return pdev->bridge_d3 ? 0 : -EBUSY;
If an additional check is necessary for this issue, it should be integrated into pci_dev_check_d3cold() instead of pcie_port_runtime_idle(). Thanks, Lukas