diff mbox series

[RESEND,v2,01/10] PCI: Do not skip power managed bridges in pci_enable_wake()

Message ID 20180914083134.27217-1-mika.westerberg@linux.intel.com (mailing list archive)
State Not Applicable
Headers show
Series [RESEND,v2,01/10] PCI: Do not skip power managed bridges in pci_enable_wake() | expand

Commit Message

Mika Westerberg Sept. 14, 2018, 8:31 a.m. UTC
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>
Reviewed-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
Resending this because for some reason it did not hit the mailing lists.
Added tag from Rafael as well.

 drivers/pci/pci.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

Comments

Bjorn Helgaas Sept. 18, 2018, 9:38 p.m. UTC | #1
On Fri, Sep 14, 2018 at 11:31:34AM +0300, 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>
> Reviewed-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Applied to pci/enumeration for v4.20, thanks!

> ---
> Resending this because for some reason it did not hit the mailing lists.
> Added tag from Rafael as well.
> 
>  drivers/pci/pci.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index 29ff9619b5fa..1af6f1887986 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -2133,10 +2133,13 @@ static int __pci_enable_wake(struct pci_dev *dev, pci_power_t state, bool enable
>  	int ret = 0;
>  
>  	/*
> -	 * Bridges can only signal wakeup on behalf of subordinate devices,
> -	 * but that is set up elsewhere, so skip them.
> +	 * 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. */
> -- 
> 2.18.0
>
diff mbox series

Patch

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 29ff9619b5fa..1af6f1887986 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -2133,10 +2133,13 @@  static int __pci_enable_wake(struct pci_dev *dev, pci_power_t state, bool enable
 	int ret = 0;
 
 	/*
-	 * Bridges can only signal wakeup on behalf of subordinate devices,
-	 * but that is set up elsewhere, so skip them.
+	 * 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. */