On 7/10/2023 3:33 PM, Bjorn Helgaas wrote:
It sounds like there's someplace the hardware designers specify how this should work? Where is that? "Modern Standby" doesn't mean anything to me, but maybe there's some spec that covers it?
https://learn.microsoft.com/en-us/windows-hardware/design/device-experiences/modern-standby It quickly devolves into Microsoft specific stuff though and I can't find anything interesting to our specific issue.
Maybe this is the clue we need. My eyes glaze over when reading that section, but if we can come up with a commit log that starts with a sentence from that section and connects the dots all the way to the platform_pci_power_manageable() implementation, that might be a good argument that 9d26d3a8f1b0 was a little too aggressive.
Yeah.
I like the fact that v5 ([1] for anybody following along at home) is very generic: @@ bool pci_bridge_d3_possible(struct pci_dev *bridge) ... + if (pci_pcie_type(bridge) == PCI_EXP_TYPE_ROOT_PORT && + !platform_pci_power_manageable(bridge)) + return false; My objection was that we didn't have a clear connection to any specs, so even though the code is perfectly generic, the commit log mentioned specific cases (USB keyboard/mouse on xHCI device connected to USB-C on AMD USB4 router). But maybe we *could* make a convincing generic commit log. I guess we'd also have to explain the PCI_EXP_TYPE_ROOT_PORT part of the patch.
OK, I'll take a stab at rewriting the v5 commit message to be more generic as you suggested as a v7. We might be able to drop the PCI_EXP_TYPE_ROOT_PORT part well but I would be more worried about regressions from this.