On Tue, Nov 26, 2024 at 03:17:11PM -0800, Brian Norris wrote: > From: Manivannan Sadhasivam <manivannan.sadhasivam@xxxxxxxxxx> > > Unlike ACPI based platforms, there are no known issues with D3Hot for > the PCI bridges in Device Tree based platforms. Can we elaborate on this a little bit? Referring to "known issues with ACPI-based platforms" depends on a lot of domain-specific history that most readers (including me) don't know. I don't think "ACPI-based" or "devicetree-based" are good justifications for changing the behavior because they don't identify any specific reasons. It's like saying "we can enable this feature because the platform spec is written in French." > Past discussions (Link [1]) determined the restrictions around D3 > should be relaxed for all Device Tree systems. This is far too generic a statement for me to sign up to, especially since "all Device Tree systems" doesn't say anything at all about how any particular hardware works or what behavior we're relying on. We need to say something about what D3hot means (i.e., only message and type 0 config requests accepted) and that we know anything below the bridge is inaccessible in D3hot and why that's OK. E.g., maybe we only care about wakeup requests and we know those still work with the bridge in D3hot because XYZ. > So let's allow the PCI bridges to go to D3Hot during runtime. > > To match devm_pci_alloc_host_bridge() -> devm_of_pci_bridge_init(), we > look at the host bridge's parent when determining whether this is a > Device Tree based platform. Not all bridges have their own node, but the > parent (controller) should. > > Link: https://lore.kernel.org/linux-pci/20240227225442.GA249898@bhelgaas/ [1] > Link: https://lore.kernel.org/linux-pci/20240828210705.GA37859@bhelgaas/ [2] > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@xxxxxxxxxx> > [Brian: look at host bridge's parent, not bridge node; rewrite > description] > Signed-off-by: Brian Norris <briannorris@xxxxxxxxxxxx> > --- > Based on prior work by Manivannan Sadhasivam that was part of a bigger > series that stalled: > > [PATCH v5 4/4] PCI: Allow PCI bridges to go to D3Hot on all Devicetree based platforms > https://lore.kernel.org/linux-pci/20240802-pci-bridge-d3-v5-4-2426dd9e8e27@xxxxxxxxxx/ > > I'm resubmitting this single patch, since it's useful and seemingly had > agreement. I massaged it a bit to relax some restrictions on how the > Device Tree should look. > > Changes in v5: > - Pulled out of the larger series, as there were more controversial > changes in there, while this one had agreement (Link [2]). > - Rewritten with a relaxed set of rules, because the above patch > required us to modify many device trees to add bridge nodes. > > drivers/pci/pci.c | 11 +++++++++++ > 1 file changed, 11 insertions(+) > > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c > index e278861684bc..5d898f5ea155 100644 > --- a/drivers/pci/pci.c > +++ b/drivers/pci/pci.c > @@ -3018,6 +3018,8 @@ static const struct dmi_system_id bridge_d3_blacklist[] = { > */ > bool pci_bridge_d3_possible(struct pci_dev *bridge) > { > + struct pci_host_bridge *host_bridge; > + > if (!pci_is_pcie(bridge)) > return false; > > @@ -3038,6 +3040,15 @@ bool pci_bridge_d3_possible(struct pci_dev *bridge) > if (pci_bridge_d3_force) > return true; > > + /* > + * Allow D3 for all Device Tree based systems. We assume a host > + * bridge's parent will have a device node, even if this bridge > + * may not have its own. > + */ > + host_bridge = pci_find_host_bridge(bridge->bus); > + if (dev_of_node(host_bridge->dev.parent)) > + return true; > + > /* Even the oldest 2010 Thunderbolt controller supports D3. */ > if (bridge->is_thunderbolt) > return true; > -- > 2.47.0.338 >
