[2/6] USB / PCI / PM: Allow the PCI core to do the resume cleanup
diff mbox

Message ID 1565826.hIbT8kZpDn@aspire.rjw.lan
State Superseded, archived
Headers show

Commit Message

Rafael J. Wysocki June 8, 2017, 12:02 a.m. UTC
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

hcd_pci_resume_noirq() used as a universal _resume_noirq handler for
PCI USB controllers calls pci_back_from_sleep() which is unnecessary
and may become problematic.

It is unnecessary, because the PCI bus type carries out post-suspend
cleanup of all PCI devices during resume and that covers all things
done by the pci_back_from_sleep().  There is no reason why USB cannot
follow all of the other PCI devices in that respect.

It will become problematic after subsequent changes that make it
possible to go back to sleep again after executing dpm_resume_noirq()
if no valid system wakeup events have been detected at that point.
Namely, calling pci_back_from_sleep() at the _resume_noirq stage
will cause the wakeup status of the devices in question to be cleared
and if any of them has triggered system wakeup, that event may be
missed then.

For the above reasons, drop the pci_back_from_sleep() invocation
from hcd_pci_resume_noirq().

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/usb/core/hcd-pci.c |    7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)


--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Alan Stern June 8, 2017, 3:24 p.m. UTC | #1
On Thu, 8 Jun 2017, Rafael J. Wysocki wrote:

> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> hcd_pci_resume_noirq() used as a universal _resume_noirq handler for
> PCI USB controllers calls pci_back_from_sleep() which is unnecessary
> and may become problematic.
> 
> It is unnecessary, because the PCI bus type carries out post-suspend
> cleanup of all PCI devices during resume and that covers all things
> done by the pci_back_from_sleep().  There is no reason why USB cannot
> follow all of the other PCI devices in that respect.
> 
> It will become problematic after subsequent changes that make it
> possible to go back to sleep again after executing dpm_resume_noirq()
> if no valid system wakeup events have been detected at that point.
> Namely, calling pci_back_from_sleep() at the _resume_noirq stage
> will cause the wakeup status of the devices in question to be cleared
> and if any of them has triggered system wakeup, that event may be
> missed then.
> 
> For the above reasons, drop the pci_back_from_sleep() invocation
> from hcd_pci_resume_noirq().
> 
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
>  drivers/usb/core/hcd-pci.c |    7 +------
>  1 file changed, 1 insertion(+), 6 deletions(-)
> 
> Index: linux-pm/drivers/usb/core/hcd-pci.c
> ===================================================================
> --- linux-pm.orig/drivers/usb/core/hcd-pci.c
> +++ linux-pm/drivers/usb/core/hcd-pci.c
> @@ -584,12 +584,7 @@ static int hcd_pci_suspend_noirq(struct
>  
>  static int hcd_pci_resume_noirq(struct device *dev)
>  {
> -	struct pci_dev		*pci_dev = to_pci_dev(dev);
> -
> -	powermac_set_asic(pci_dev, 1);
> -
> -	/* Go back to D0 and disable remote wakeup */
> -	pci_back_from_sleep(pci_dev);
> +	powermac_set_asic(to_pci_dev(dev), 1);
>  	return 0;
>  }

Acked-by: Alan Stern <stern@rowland.harvard.edu>

--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Rafael J. Wysocki June 8, 2017, 11:01 p.m. UTC | #2
On Thursday, June 08, 2017 11:24:55 AM Alan Stern wrote:
> On Thu, 8 Jun 2017, Rafael J. Wysocki wrote:
> 
> > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > 
> > hcd_pci_resume_noirq() used as a universal _resume_noirq handler for
> > PCI USB controllers calls pci_back_from_sleep() which is unnecessary
> > and may become problematic.
> > 
> > It is unnecessary, because the PCI bus type carries out post-suspend
> > cleanup of all PCI devices during resume and that covers all things
> > done by the pci_back_from_sleep().  There is no reason why USB cannot
> > follow all of the other PCI devices in that respect.
> > 
> > It will become problematic after subsequent changes that make it
> > possible to go back to sleep again after executing dpm_resume_noirq()
> > if no valid system wakeup events have been detected at that point.
> > Namely, calling pci_back_from_sleep() at the _resume_noirq stage
> > will cause the wakeup status of the devices in question to be cleared
> > and if any of them has triggered system wakeup, that event may be
> > missed then.
> > 
> > For the above reasons, drop the pci_back_from_sleep() invocation
> > from hcd_pci_resume_noirq().
> > 
> > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > ---
> >  drivers/usb/core/hcd-pci.c |    7 +------
> >  1 file changed, 1 insertion(+), 6 deletions(-)
> > 
> > Index: linux-pm/drivers/usb/core/hcd-pci.c
> > ===================================================================
> > --- linux-pm.orig/drivers/usb/core/hcd-pci.c
> > +++ linux-pm/drivers/usb/core/hcd-pci.c
> > @@ -584,12 +584,7 @@ static int hcd_pci_suspend_noirq(struct
> >  
> >  static int hcd_pci_resume_noirq(struct device *dev)
> >  {
> > -	struct pci_dev		*pci_dev = to_pci_dev(dev);
> > -
> > -	powermac_set_asic(pci_dev, 1);
> > -
> > -	/* Go back to D0 and disable remote wakeup */
> > -	pci_back_from_sleep(pci_dev);
> > +	powermac_set_asic(to_pci_dev(dev), 1);
> >  	return 0;
> >  }
> 
> Acked-by: Alan Stern <stern@rowland.harvard.edu>

Thanks!

--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch
diff mbox

Index: linux-pm/drivers/usb/core/hcd-pci.c
===================================================================
--- linux-pm.orig/drivers/usb/core/hcd-pci.c
+++ linux-pm/drivers/usb/core/hcd-pci.c
@@ -584,12 +584,7 @@  static int hcd_pci_suspend_noirq(struct
 
 static int hcd_pci_resume_noirq(struct device *dev)
 {
-	struct pci_dev		*pci_dev = to_pci_dev(dev);
-
-	powermac_set_asic(pci_dev, 1);
-
-	/* Go back to D0 and disable remote wakeup */
-	pci_back_from_sleep(pci_dev);
+	powermac_set_asic(to_pci_dev(dev), 1);
 	return 0;
 }