[Public] > > To fix these situations explicitly check that the ACPI device has a GPE > > allowing the device to generate wakeup signals handled by the platform > > in `acpi_pci_bridge_d3`. > > Which may be orthogonal to the _S0W return value mentioned above. > > Also, I'm not quite sure why acpi_pci_bridge_d3() should require the > root port to have a wake GPE associated with it as an indication that > the hierarchy below it can be put into D3cold. The reason that brought me down the path in this patch was actually acpi_dev_pm_get_state. _S0W isn't actually evaluated unless adev->wakeup.flags.valid is set. > > > > > diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c > > index a42dbf448860..9f8f55ed09d9 100644 > > --- a/drivers/pci/pci-acpi.c > > +++ b/drivers/pci/pci-acpi.c > > @@ -999,6 +999,9 @@ bool acpi_pci_bridge_d3(struct pci_dev *dev) > > if (!adev) > > return false; > > > > + if (!adev->wakeup.flags.valid) > > + return false; > > Minor nit: the two checks above could be combined. OK if we stick to this approach I'll do that. > > Also I would add a comment explaining why exactly wakeup.flags.valid > is checked here, because I can imagine a case in which the wakeup > signaling capability is irrelevant for whether or not the given port > can handle D3cold. Specifically a case that it's a hotplug bridge that has HotPlugSupportInD3 though? In practice I've only seen that in use on USB4 and Thunderbolt bridges "so far". I haven't tried yet but I would think directly evaluating _S0W at this time seems it should also work and would match closer to my original intent of the patch. Would you prefer that? > > > + > > if (acpi_dev_get_property(adev, "HotPlugSupportInD3", > > ACPI_TYPE_INTEGER, &obj) < 0) > > return false; > > --
