| Message ID | 20180906155020.51700-2-mika.westerberg@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
| Series | PCI: Allow D3cold for PCIe hierarchies | expand |
On Thursday, September 6, 2018 5:50:11 PM CEST Mika Westerberg wrote: > Commit baecc470d5fd ("PCI / PM: Skip bridges in pci_enable_wake()") > changed pci_enable_wake() so that all bridges are skipped when wakeup is > enabled (or disabled) with the reasoning that bridges can only signal > wakeup on behalf of their subordinate devices. > > However, there are bridges that can signal wakeup itself. For example > PCIe downstream and root ports supporting hotplug may signal wakeup upon > hotplug event. > > For this reason change pci_enable_wake() so that it skips all bridges > except those that we power manage (->bridge_d3 is set). Those are the > ones that can go into low power states and may need to signal wakeup. > > Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com> > --- > drivers/pci/pci.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c > index 29ff9619b5fa..074f3f0253f9 100644 > --- a/drivers/pci/pci.c > +++ b/drivers/pci/pci.c > @@ -2134,9 +2134,11 @@ static int __pci_enable_wake(struct pci_dev *dev, pci_power_t state, bool enable > > /* > * Bridges can only signal wakeup on behalf of subordinate devices, > - * but that is set up elsewhere, so skip them. > + * but that is set up elsewhere, so skip them. With the exception > + * of bridges that we power manage. These can signal wake for > + * example on a hotplug event. > */ I would change the comment even more, to something like: "Bridges that are not power-manageable directly only signal wakeup on behalf of subordinate devices which is set up elsewhere, so skip them. However, bridges that are power-manageable may signal wakeup for themselves (for example, on a hotplug event) and they need to be covered here." > - if (pci_has_subordinate(dev)) > + if (!pci_power_manageable(dev)) > return 0; > > /* Don't do the same thing twice in a row for one device. */ >
On Tue, Sep 11, 2018 at 10:47:55AM +0200, Rafael J. Wysocki wrote: > > @@ -2134,9 +2134,11 @@ static int __pci_enable_wake(struct pci_dev *dev, pci_power_t state, bool enable > > > > /* > > * Bridges can only signal wakeup on behalf of subordinate devices, > > - * but that is set up elsewhere, so skip them. > > + * but that is set up elsewhere, so skip them. With the exception > > + * of bridges that we power manage. These can signal wake for > > + * example on a hotplug event. > > */ > > I would change the comment even more, to something like: > > "Bridges that are not power-manageable directly only signal wakeup on behalf > of subordinate devices which is set up elsewhere, so skip them. However, > bridges that are power-manageable may signal wakeup for themselves (for example, > on a hotplug event) and they need to be covered here." OK, I'll update the comment accordingly.
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index 29ff9619b5fa..074f3f0253f9 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -2134,9 +2134,11 @@ static int __pci_enable_wake(struct pci_dev *dev, pci_power_t state, bool enable /* * Bridges can only signal wakeup on behalf of subordinate devices, - * but that is set up elsewhere, so skip them. + * but that is set up elsewhere, so skip them. With the exception + * of bridges that we power manage. These can signal wake for + * example on a hotplug event. */ - if (pci_has_subordinate(dev)) + if (!pci_power_manageable(dev)) return 0; /* Don't do the same thing twice in a row for one device. */
Commit baecc470d5fd ("PCI / PM: Skip bridges in pci_enable_wake()") changed pci_enable_wake() so that all bridges are skipped when wakeup is enabled (or disabled) with the reasoning that bridges can only signal wakeup on behalf of their subordinate devices. However, there are bridges that can signal wakeup itself. For example PCIe downstream and root ports supporting hotplug may signal wakeup upon hotplug event. For this reason change pci_enable_wake() so that it skips all bridges except those that we power manage (->bridge_d3 is set). Those are the ones that can go into low power states and may need to signal wakeup. Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com> --- drivers/pci/pci.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)