| Message ID | 1373473405-27589-1-git-send-email-rogerq@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, 10 Jul 2013, Roger Quadros wrote: > Call ehci_suspend/resume() during runtime suspend/resume > as well as system suspend/resume. > > Use a flag "bound" to indicate that the HCD structures are valid. > This is only true between usb_add_hcd() and usb_remove_hcd() calls. > > The flag can be used by omap_ehci_runtime_suspend/resume() handlers > to avoid calling ehci_suspend/resume() when we are no longer bound > to the HCD e.g. during probe() and remove(); > @@ -293,15 +301,67 @@ static const struct of_device_id omap_ehci_dt_ids[] = { > > MODULE_DEVICE_TABLE(of, omap_ehci_dt_ids); > > +static int omap_ehci_suspend(struct device *dev) > +{ > + struct usb_hcd *hcd = dev_get_drvdata(dev); > + > + dev_dbg(dev, "%s\n", __func__); > + > + return ehci_suspend(hcd, true); > +} > + > +static int omap_ehci_resume(struct device *dev) > +{ > + struct usb_hcd *hcd = dev_get_drvdata(dev); > + > + dev_dbg(dev, "%s\n", __func__); > + > + return ehci_resume(hcd, false); > +} There are three problems here. The first is very simple: the wakeup settings are messed up. Wakeup is supposed to be enabled always for runtime suspend, whereas it is controlled by device_may_wakeup() for system suspend. You reversed them. The other two problems are both related to the interaction between system PM and runtime PM. Suppose the controller is already runtime suspended when the system goes to sleep. Because it is runtime suspended, it is enabled for wakeup. But device_may_wakeup() could return false; if this happens then you have to do a runtime-resume in omap_ehci_suspend() before calling ehci_suspend(), so that the controller can be suspended again with wakeups disabled. (Or you could choose an alternative method for accomplishing the same result, such as disabling the wakeup signal from the pad without going through a whole EHCI resume/suspend cycle.) Conversely, if device_may_wakeup() returns true then you shouldn't do anything at all, because the controller is already suspended with the correct wakeup setting. For the third problem, suppose the controller was runtime suspended when the system went to sleep. When the system wakes up, the controller will become active, so you have to inform the runtime PM core about its change of state. Basically, if omap_ehci_resume() sees that ehci_resume() returned 0 then it must do: pm_runtime_disable(dev); pm_runtime_set_active(dev); pm_runtime_enable(dev); All of these issues are discussed (among lots of other material) in Documentation/power/runtime_pm.txt. Alan Stern
On 07/10/2013 10:04 PM, Alan Stern wrote: > On Wed, 10 Jul 2013, Roger Quadros wrote: > >> Call ehci_suspend/resume() during runtime suspend/resume >> as well as system suspend/resume. >> >> Use a flag "bound" to indicate that the HCD structures are valid. >> This is only true between usb_add_hcd() and usb_remove_hcd() calls. >> >> The flag can be used by omap_ehci_runtime_suspend/resume() handlers >> to avoid calling ehci_suspend/resume() when we are no longer bound >> to the HCD e.g. during probe() and remove(); > >> @@ -293,15 +301,67 @@ static const struct of_device_id omap_ehci_dt_ids[] = { >> >> MODULE_DEVICE_TABLE(of, omap_ehci_dt_ids); >> >> +static int omap_ehci_suspend(struct device *dev) >> +{ >> + struct usb_hcd *hcd = dev_get_drvdata(dev); >> + >> + dev_dbg(dev, "%s\n", __func__); >> + >> + return ehci_suspend(hcd, true); >> +} >> + >> +static int omap_ehci_resume(struct device *dev) >> +{ >> + struct usb_hcd *hcd = dev_get_drvdata(dev); >> + >> + dev_dbg(dev, "%s\n", __func__); >> + >> + return ehci_resume(hcd, false); >> +} > > There are three problems here. The first is very simple: the wakeup > settings are messed up. Wakeup is supposed to be enabled always for > runtime suspend, whereas it is controlled by device_may_wakeup() for > system suspend. You reversed them. Indeed. I'll fix that up. However, even if wakeup is disabled during system suspend, the OMAP will still wakeup on any activity on the USB pins. The only way to disable that is to disable IO pad wakeup. This can only be done in mfd/omap-usb-host.c I suppose I can use device_may_wakeup() there and configure the IO pad wakeup accordingly. > > The other two problems are both related to the interaction between > system PM and runtime PM. Suppose the controller is already runtime > suspended when the system goes to sleep. Because it is runtime > suspended, it is enabled for wakeup. But device_may_wakeup() could > return false; if this happens then you have to do a runtime-resume in > omap_ehci_suspend() before calling ehci_suspend(), so that the > controller can be suspended again with wakeups disabled. (Or you could > choose an alternative method for accomplishing the same result, such as > disabling the wakeup signal from the pad without going through a whole > EHCI resume/suspend cycle.) Conversely, if device_may_wakeup() returns > true then you shouldn't do anything at all, because the controller is > already suspended with the correct wakeup setting. I think this case is taken care of by the Runtime PM core at least for the OMAP platform according to the documentation http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/power/runtime_pm.txt#n647 At the end of this mail is the log during system suspend/resume You can notice the following sequence -ehci runtime suspends -system suspend triggered -ehci runtime resumes -ehci suspends (uses new wakeup settings) -system wakeup triggered -ehci resumes -ehci runtime suspends > > For the third problem, suppose the controller was runtime suspended > when the system went to sleep. When the system wakes up, the > controller will become active, so you have to inform the runtime PM > core about its change of state. Basically, if omap_ehci_resume() sees > that ehci_resume() returned 0 then it must do: > > pm_runtime_disable(dev); > pm_runtime_set_active(dev); > pm_runtime_enable(dev); > > All of these issues are discussed (among lots of other material) in > Documentation/power/runtime_pm.txt. Is this still applicable? Documentation claims "During system resume it calls pm_runtime_enable() and pm_runtime_put_sync() for every device right after executing the subsystem-level .resume_early() callback and right after executing the subsystem-level .resume() callback for it, respectively." cheers, -roger --- [ 13.256286] smsc95xx 1-2.1:1.0 eth0: entering SUSPEND2 mode [ 13.264648] usb 1-2.1: usb auto-suspend, wakeup 0 [ 13.281677] hub 1-2:1.0: hub_suspend [ 13.285858] usb 1-2: unlink qh256-0001/dd443880 start 1 [1/0 us] / / # [ 13.296386] usb 1-2: usb auto-suspend, wakeup 1 [ 13.321411] hub 1-0:1.0: hub_suspend [ 13.325225] usb usb1: bus auto-suspend, wakeup 1 [ 13.330139] ehci-omap 48064800.ehci: suspend root hub [ 13.336395] ehci-omap 48064800.ehci: omap_ehci_runtime_suspend / # / # echo mem > /sys/power/state [ 26.821838] PM: Syncing filesystems ... done. [ 26.828491] PM: Preparing system for mem sleep [ 26.841796] Freezing user space processes ... (elapsed 0.000 seconds) done. [ 26.849426] Freezing remaining freezable tasks ... (elapsed 0.002 seconds) done. [ 26.860046] PM: Entering mem sleep [ 26.863769] Suspending console(s) (use no_console_suspend to debug) [ 26.878234] ehci-omap 48064800.ehci: omap_ehci_runtime_resume [ 26.878295] usb usb1: usb auto-resume [ 26.878326] ehci-omap 48064800.ehci: resume root hub [ 26.878417] hub 1-0:1.0: hub_resume [ 26.878570] hub 1-0:1.0: port 2: status 0507 change 0000 [ 26.879089] hub 1-0:1.0: hub_suspend [ 26.879211] usb usb1: bus suspend, wakeup 0 [ 26.879241] ehci-omap 48064800.ehci: suspend root hub [ 26.891845] ehci-omap 48064800.ehci: omap_ehci_suspend may_wakeup 0 [ 26.923797] PM: suspend of devices complete after 47.912 msecs [ 26.929443] PM: late suspend of devices complete after 5.645 msecs [ 26.935882] PM: noirq suspend of devices complete after 6.378 msecs [ 26.935943] Disabling non-boot CPUs ... [ 26.936065] Successfully put all powerdomains to target state [ 26.939636] PM: noirq resume of devices complete after 3.448 msecs [ 26.945190] PM: early resume of devices complete after 4.180 msecs [ 26.954833] ehci-omap 48064800.ehci: omap_ehci_resume [ 26.955383] usb usb1: usb resume [ 26.955383] ehci-omap 48064800.ehci: resume root hub [ 26.955413] hub 1-0:1.0: hub_resume [ 26.955535] hub 1-0:1.0: port 2: status 0507 change 0000 [ 26.955902] usb 1-2: usb resume [ 26.990661] ehci-omap 48064800.ehci: GetStatus port:2 status 001005 0 ACK POWER sig=se0 PE CONNECT [ 27.010528] usb 1-2: finish resume [ 27.010894] hub 1-2:1.0: hub_resume [ 27.011352] hub 1-2:1.0: port 1: status 0507 change 0000 [ 27.012390] ehci-omap 48064800.ehci: reused qh dd443880 schedule [ 27.012390] usb 1-2: link qh256-0001/dd443880 start 1 [1/0 us] [ 27.013244] usb 1-2.1: usb resume [ 27.070800] usb 1-2.1: finish resume [ 27.072998] PM: resume of devices complete after 127.716 msecs [ 27.243438] PM: Finishing wakeup. [ 27.246948] Restarting tasks ... [ 27.251312] hub 1-0:1.0: state 7 ports 3 chg 0000 evt 0000 [ 27.257171] hub 1-2:1.0: state 7 ports 5 chg 0000 evt 0000 done. / # [ 29.074493] smsc95xx 1-2.1:1.0 eth0: entering SUSPEND2 mode [ 29.082458] usb 1-2.1: usb auto-suspend, wakeup 0 [ 29.100891] hub 1-2:1.0: hub_suspend [ 29.104736] usb 1-2: unlink qh256-0001/dd443880 start 1 [1/0 us] [ 29.114288] usb 1-2: usb auto-suspend, wakeup 1 [ 29.130859] hub 1-0:1.0: hub_suspend [ 29.134735] usb usb1: bus auto-suspend, wakeup 1 [ 29.139648] ehci-omap 48064800.ehci: suspend root hub [ 29.145446] ehci-omap 48064800.ehci: omap_ehci_runtime_suspend
On Thu, 11 Jul 2013, Roger Quadros wrote: > > The other two problems are both related to the interaction between > > system PM and runtime PM. Suppose the controller is already runtime > > suspended when the system goes to sleep. Because it is runtime > > suspended, it is enabled for wakeup. But device_may_wakeup() could > > return false; if this happens then you have to do a runtime-resume in > > omap_ehci_suspend() before calling ehci_suspend(), so that the > > controller can be suspended again with wakeups disabled. (Or you could > > choose an alternative method for accomplishing the same result, such as > > disabling the wakeup signal from the pad without going through a whole > > EHCI resume/suspend cycle.) Conversely, if device_may_wakeup() returns > > true then you shouldn't do anything at all, because the controller is > > already suspended with the correct wakeup setting. > > I think this case is taken care of by the Runtime PM core at least for the OMAP > platform according to the documentation > > http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/power/runtime_pm.txt#n647 No; that section refers only to races, not to wakeup settings. > At the end of this mail is the log during system suspend/resume > > You can notice the following sequence > > -ehci runtime suspends > -system suspend triggered > -ehci runtime resumes > -ehci suspends (uses new wakeup settings) > -system wakeup triggered > -ehci resumes > -ehci runtime suspends This is because the root hub was runtime suspended with the wrong wakeup setting. The USB core, which is careful about these things, resumed and re-suspended it with the proper wakeup setting. In the process, the controller had to be runtime resumed as well. Try doing the test over again, but this time with the root hub enabled for wakeup and the controller disabled. (I know this is a bizarre combination, but try it anyway.) Also, after the system wakes up, see whether the root hub and controller get runtime suspended. > > For the third problem, suppose the controller was runtime suspended > > when the system went to sleep. When the system wakes up, the > > controller will become active, so you have to inform the runtime PM > > core about its change of state. Basically, if omap_ehci_resume() sees > > that ehci_resume() returned 0 then it must do: > > > > pm_runtime_disable(dev); > > pm_runtime_set_active(dev); > > pm_runtime_enable(dev); > > > > All of these issues are discussed (among lots of other material) in > > Documentation/power/runtime_pm.txt. > > Is this still applicable? Documentation claims > > "During system resume it calls pm_runtime_enable() and pm_runtime_put_sync() > for every device right after executing the subsystem-level .resume_early() > callback and right after executing the subsystem-level .resume() callback > for it, respectively." Yes, this is applicable, but it is irrelevant to the problem I described. You still have to tell the runtime PM core that the device is now active. Alan Stern
Hi Alan, On 07/11/2013 06:14 PM, Alan Stern wrote: > On Thu, 11 Jul 2013, Roger Quadros wrote: > >>> The other two problems are both related to the interaction between >>> system PM and runtime PM. Suppose the controller is already runtime >>> suspended when the system goes to sleep. Because it is runtime >>> suspended, it is enabled for wakeup. But device_may_wakeup() could >>> return false; if this happens then you have to do a runtime-resume in >>> omap_ehci_suspend() before calling ehci_suspend(), so that the >>> controller can be suspended again with wakeups disabled. (Or you could >>> choose an alternative method for accomplishing the same result, such as >>> disabling the wakeup signal from the pad without going through a whole >>> EHCI resume/suspend cycle.) Conversely, if device_may_wakeup() returns >>> true then you shouldn't do anything at all, because the controller is >>> already suspended with the correct wakeup setting. >> >> I think this case is taken care of by the Runtime PM core at least for the OMAP >> platform according to the documentation >> >> http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/Documentation/power/runtime_pm.txt#n647 > > No; that section refers only to races, not to wakeup settings. > >> At the end of this mail is the log during system suspend/resume >> >> You can notice the following sequence >> >> -ehci runtime suspends >> -system suspend triggered >> -ehci runtime resumes >> -ehci suspends (uses new wakeup settings) >> -system wakeup triggered >> -ehci resumes >> -ehci runtime suspends > > This is because the root hub was runtime suspended with the wrong > wakeup setting. The USB core, which is careful about these things, > resumed and re-suspended it with the proper wakeup setting. In the > process, the controller had to be runtime resumed as well. > > Try doing the test over again, but this time with the root hub enabled > for wakeup and the controller disabled. (I know this is a bizarre > combination, but try it anyway.) Also, after the system wakes up, see > whether the root hub and controller get runtime suspended. > The first part of the test caught the problem were we were trying to access EHCI registers when HW is not accessible. So this was a good test case. >>> For the third problem, suppose the controller was runtime suspended >>> when the system went to sleep. When the system wakes up, the >>> controller will become active, so you have to inform the runtime PM >>> core about its change of state. Basically, if omap_ehci_resume() sees >>> that ehci_resume() returned 0 then it must do: >>> >>> pm_runtime_disable(dev); >>> pm_runtime_set_active(dev); >>> pm_runtime_enable(dev); >>> >>> All of these issues are discussed (among lots of other material) in >>> Documentation/power/runtime_pm.txt. >> >> Is this still applicable? Documentation claims >> >> "During system resume it calls pm_runtime_enable() and pm_runtime_put_sync() >> for every device right after executing the subsystem-level .resume_early() >> callback and right after executing the subsystem-level .resume() callback >> for it, respectively." > > Yes, this is applicable, but it is irrelevant to the problem I > described. You still have to tell the runtime PM core that the device > is now active. Right, I understand it now. How does the below code look? +static int omap_ehci_suspend(struct device *dev) +{ + struct usb_hcd *hcd = dev_get_drvdata(dev); + bool do_wakeup = device_may_wakeup(dev); + int ret; + + dev_dbg(dev, "%s may_wakeup %d\n", __func__, do_wakeup); + + if (pm_runtime_status_suspended(dev)) { + pm_runtime_get_sync(dev); + ehci_resume(hcd, false); + ret = ehci_suspend(hcd, do_wakeup); + pm_runtime_put_sync(dev); + + } else { + ret = ehci_suspend(hcd, do_wakeup); + } + + return ret; +} + +static int omap_ehci_resume(struct device *dev) +{ + struct usb_hcd *hcd = dev_get_drvdata(dev); + int ret; + + dev_dbg(dev, "%s\n", __func__); + + ret = ehci_resume(hcd, false); + if (!ret) { + /* + * Controller was powered ON so reflect state + */ + pm_runtime_disable(dev); + pm_runtime_set_active(dev); + pm_runtime_enable(dev); + } + + return ret; +} + +static int omap_ehci_runtime_suspend(struct device *dev) +{ + struct usb_hcd *hcd = dev_get_drvdata(dev); + struct omap_hcd *omap = (struct omap_hcd *)hcd_to_ehci(hcd)->priv; + + dev_dbg(dev, "%s\n", __func__); + + if (omap->bound) + ehci_suspend(hcd, true); + + return 0; +} + +static int omap_ehci_runtime_resume(struct device *dev) +{ + struct usb_hcd *hcd = dev_get_drvdata(dev); + struct omap_hcd *omap = (struct omap_hcd *)hcd_to_ehci(hcd)->priv; + + dev_dbg(dev, "%s\n", __func__); + + if (omap->bound) + ehci_resume(hcd, false); + + return 0; +} cheers, -roger
On Mon, 22 Jul 2013, Roger Quadros wrote: > Right, I understand it now. How does the below code look? > > +static int omap_ehci_suspend(struct device *dev) > +{ > + struct usb_hcd *hcd = dev_get_drvdata(dev); > + bool do_wakeup = device_may_wakeup(dev); > + int ret; > + > + dev_dbg(dev, "%s may_wakeup %d\n", __func__, do_wakeup); > + > + if (pm_runtime_status_suspended(dev)) { You need to do this only when do_wakeup is false. > + pm_runtime_get_sync(dev); > + ehci_resume(hcd, false); > + ret = ehci_suspend(hcd, do_wakeup); > + pm_runtime_put_sync(dev); It would be better to call pm_runtime_resume(dev) at the start instead of pm_runtime_get_sync(), and eliminate the last two lines. Then the ehci_suspend() below could be moved outside the "if" statement. Alternatively, if you are able to turn off the wakeup setting without going through an entire resume/suspend cycle, that would be preferable. > + > + } else { > + ret = ehci_suspend(hcd, do_wakeup); > + } > + > + return ret; > +} > + > +static int omap_ehci_resume(struct device *dev) > +{ > + struct usb_hcd *hcd = dev_get_drvdata(dev); > + int ret; > + > + dev_dbg(dev, "%s\n", __func__); > + > + ret = ehci_resume(hcd, false); > + if (!ret) { > + /* > + * Controller was powered ON so reflect state > + */ > + pm_runtime_disable(dev); > + pm_runtime_set_active(dev); > + pm_runtime_enable(dev); > + } > + > + return ret; > +} All good. Alan Stern
On 07/22/2013 06:18 PM, Alan Stern wrote: > On Mon, 22 Jul 2013, Roger Quadros wrote: > >> Right, I understand it now. How does the below code look? >> >> +static int omap_ehci_suspend(struct device *dev) >> +{ >> + struct usb_hcd *hcd = dev_get_drvdata(dev); >> + bool do_wakeup = device_may_wakeup(dev); >> + int ret; >> + >> + dev_dbg(dev, "%s may_wakeup %d\n", __func__, do_wakeup); >> + >> + if (pm_runtime_status_suspended(dev)) { > > You need to do this only when do_wakeup is false. Right. Missed that. > >> + pm_runtime_get_sync(dev); >> + ehci_resume(hcd, false); >> + ret = ehci_suspend(hcd, do_wakeup); >> + pm_runtime_put_sync(dev); > > It would be better to call pm_runtime_resume(dev) at the start instead > of pm_runtime_get_sync(), and eliminate the last two lines. Then the > ehci_suspend() below could be moved outside the "if" statement. Why do I find pm_runtime_* so confusing ;) > > Alternatively, if you are able to turn off the wakeup setting without > going through an entire resume/suspend cycle, that would be preferable. > As we don't rely on the EHCI controller's interrupt any more after we power it down, maybe ehci_resume/suspend cycle is not required at all on OMAP, even if the wakeup setting is disabled during system suspend. As the wakeup is controlled entirely by pad wakeup, all we need to do is make sure the IO pad wakeup is configured correctly based on do_wakeup. How this is done is still in transition but it might be turn out to be as simple as irq_set_irq_wake() So IMHO, for ehci-omap this should suffice static int omap_ehci_suspend(struct device *dev) { struct usb_hcd *hcd = dev_get_drvdata(dev); bool do_wakeup = device_may_wakeup(dev); int ret = 0; if (!pm_runtime_status_suspended(dev)) ret = ehci_suspend(hcd, do_wakeup); /* Not tested yet */ irq_set_irq_wake(hcd->irq, do_wakeup); return ret; } What do you think? As the OMAP IO pad wakeup management code [1] is still in transition, I'll wait till that gets settled and then resend this series. cheers, -roger [1] - http://thread.gmane.org/gmane.linux.ports.arm.omap/99010
On Tue, 23 Jul 2013, Roger Quadros wrote: > >> + pm_runtime_get_sync(dev); > >> + ehci_resume(hcd, false); > >> + ret = ehci_suspend(hcd, do_wakeup); > >> + pm_runtime_put_sync(dev); > > > > It would be better to call pm_runtime_resume(dev) at the start instead > > of pm_runtime_get_sync(), and eliminate the last two lines. Then the > > ehci_suspend() below could be moved outside the "if" statement. > > Why do I find pm_runtime_* so confusing ;) It tries to provide every service anyone might want, and ends up being confusingly large. In this case, though, the reasoning is simple. We know that after the system resumes, the device will be back in the active state. Hence there's no point in calling pm_runtime_put_sync here, other than to balance the _get_sync above. Since there's no danger of another thread trying to do a runtime suspend at this point, you don't need to increment the usage counter. Therefore pm_runtime_resume is good enough. > > Alternatively, if you are able to turn off the wakeup setting without > > going through an entire resume/suspend cycle, that would be preferable. > > > > As we don't rely on the EHCI controller's interrupt any more after we > power it down, maybe ehci_resume/suspend cycle is not required at all on OMAP, > even if the wakeup setting is disabled during system suspend. > > As the wakeup is controlled entirely by pad wakeup, all we need to do is make sure > the IO pad wakeup is configured correctly based on do_wakeup. How this is done > is still in transition but it might be turn out to be as simple as irq_set_irq_wake() > > So IMHO, for ehci-omap this should suffice > > static int omap_ehci_suspend(struct device *dev) > { > struct usb_hcd *hcd = dev_get_drvdata(dev); > bool do_wakeup = device_may_wakeup(dev); > int ret = 0; > > if (!pm_runtime_status_suspended(dev)) > ret = ehci_suspend(hcd, do_wakeup); > > /* Not tested yet */ > irq_set_irq_wake(hcd->irq, do_wakeup); > > return ret; > } > > What do you think? Nice and simple. It looks good. > As the OMAP IO pad wakeup management code [1] is still in transition, I'll wait till > that gets settled and then resend this series. Okay. Alan Stern
diff --git a/drivers/usb/host/ehci-omap.c b/drivers/usb/host/ehci-omap.c index 9bd7dfe..6d1fde97 100644 --- a/drivers/usb/host/ehci-omap.c +++ b/drivers/usb/host/ehci-omap.c @@ -69,6 +69,7 @@ static const char hcd_name[] = "ehci-omap"; struct omap_hcd { struct usb_phy *phy[OMAP3_HS_USB_PORTS]; /* one PHY for each port */ int nports; + bool bound; /* HCD structures are valid */ }; static inline void ehci_write(void __iomem *base, u32 reg, u32 val) @@ -159,6 +160,7 @@ static int ehci_hcd_omap_probe(struct platform_device *pdev) hcd->rsrc_start = res->start; hcd->rsrc_len = resource_size(res); hcd->regs = regs; + hcd->has_wakeup_irq = true; hcd_to_ehci(hcd)->caps = regs; omap = (struct omap_hcd *)hcd_to_ehci(hcd)->priv; @@ -216,6 +218,8 @@ static int ehci_hcd_omap_probe(struct platform_device *pdev) goto err_pm_runtime; } + omap->bound = true; + /* * Bring PHYs out of reset for non PHY modes. * Even though HSIC mode is a PHY-less mode, the reset @@ -232,6 +236,8 @@ static int ehci_hcd_omap_probe(struct platform_device *pdev) usb_phy_set_suspend(omap->phy[i], 0); } + pm_runtime_put_sync(dev); + return 0; err_pm_runtime: @@ -264,7 +270,9 @@ static int ehci_hcd_omap_remove(struct platform_device *pdev) struct omap_hcd *omap = (struct omap_hcd *)hcd_to_ehci(hcd)->priv; int i; + pm_runtime_get_sync(dev); usb_remove_hcd(hcd); + omap->bound = false; for (i = 0; i < omap->nports; i++) { if (omap->phy[i]) @@ -293,15 +301,67 @@ static const struct of_device_id omap_ehci_dt_ids[] = { MODULE_DEVICE_TABLE(of, omap_ehci_dt_ids); +static int omap_ehci_suspend(struct device *dev) +{ + struct usb_hcd *hcd = dev_get_drvdata(dev); + + dev_dbg(dev, "%s\n", __func__); + + return ehci_suspend(hcd, true); +} + +static int omap_ehci_resume(struct device *dev) +{ + struct usb_hcd *hcd = dev_get_drvdata(dev); + + dev_dbg(dev, "%s\n", __func__); + + return ehci_resume(hcd, false); +} + +static int omap_ehci_runtime_suspend(struct device *dev) +{ + struct usb_hcd *hcd = dev_get_drvdata(dev); + struct omap_hcd *omap = (struct omap_hcd *)hcd_to_ehci(hcd)->priv; + bool do_wakeup = device_may_wakeup(dev); + + dev_dbg(dev, "%s\n", __func__); + + if (omap->bound) + ehci_suspend(hcd, do_wakeup); + + return 0; +} + +static int omap_ehci_runtime_resume(struct device *dev) +{ + struct usb_hcd *hcd = dev_get_drvdata(dev); + struct omap_hcd *omap = (struct omap_hcd *)hcd_to_ehci(hcd)->priv; + + dev_dbg(dev, "%s\n", __func__); + + if (omap->bound) + ehci_resume(hcd, false); + + return 0; +} + + +static const struct dev_pm_ops omap_ehci_pm_ops = { + .suspend = omap_ehci_suspend, + .resume = omap_ehci_resume, + .runtime_suspend = omap_ehci_runtime_suspend, + .runtime_resume = omap_ehci_runtime_resume, +}; + static struct platform_driver ehci_hcd_omap_driver = { .probe = ehci_hcd_omap_probe, .remove = ehci_hcd_omap_remove, .shutdown = ehci_hcd_omap_shutdown, - /*.suspend = ehci_hcd_omap_suspend, */ - /*.resume = ehci_hcd_omap_resume, */ .driver = { .name = hcd_name, .of_match_table = omap_ehci_dt_ids, + .pm = &omap_ehci_pm_ops, } };
Call ehci_suspend/resume() during runtime suspend/resume as well as system suspend/resume. Use a flag "bound" to indicate that the HCD structures are valid. This is only true between usb_add_hcd() and usb_remove_hcd() calls. The flag can be used by omap_ehci_runtime_suspend/resume() handlers to avoid calling ehci_suspend/resume() when we are no longer bound to the HCD e.g. during probe() and remove(); Signed-off-by: Roger Quadros <rogerq@ti.com> --- drivers/usb/host/ehci-omap.c | 64 ++++++++++++++++++++++++++++++++++++++++- 1 files changed, 62 insertions(+), 2 deletions(-)