Hi, On Mon, Jul 04, 2011 at 10:36:54AM +0530, Partha Basak wrote: > >-----Original Message----- > >From: Alan Stern [mailto:stern@xxxxxxxxxxxxxxxxxxx] > >Sent: Saturday, July 02, 2011 12:37 AM > >To: Keshava Munegowda > >Cc: linux-usb@xxxxxxxxxxxxxxx; linux-omap@xxxxxxxxxxxxxxx; linux- > >kernel@xxxxxxxxxxxxxxx; balbi@xxxxxx; gadiyar@xxxxxx; > >sameo@xxxxxxxxxxxxxxx; parthab@xxxxxxxxxxxx; tony@xxxxxxxxxxx; > >khilman@xxxxxx; b-cousson@xxxxxx; paul@xxxxxxxxx; johnstul@xxxxxxxxxx; > >vishwanath.bs@xxxxxx > >Subject: Re: [PATCH 6/6 v2] arm: omap: usb: global Suspend and resume > >support of ehci and ohci > > > >On Sat, 2 Jul 2011, Keshava Munegowda wrote: > > > >> From: Keshava Munegowda <Keshava_mgowda@xxxxxx> > >> > >> The global suspend and resume functions for ehci and ohci > >> drivers are implemented; these functions does the > >> pm_runtime_get_sync and pm_runtime_put_sync of the > >> parent device usbhs core driver respectively. > >> > >> Signed-off-by: Keshava Munegowda <keshava_mgowda@xxxxxx> > >> --- > >> drivers/usb/host/ehci-omap.c | 22 ++++++++++++++++++++-- > >> drivers/usb/host/ohci-omap3.c | 21 +++++++++++++++++++++ > >> 2 files changed, 41 insertions(+), 2 deletions(-) > >> > >> diff --git a/drivers/usb/host/ehci-omap.c b/drivers/usb/host/ehci- > >omap.c > >> index 178f63e..a02a684 100644 > >> --- a/drivers/usb/host/ehci-omap.c > >> +++ b/drivers/usb/host/ehci-omap.c > >> @@ -259,14 +259,32 @@ static void ehci_hcd_omap_shutdown(struct > >platform_device *pdev) > >> hcd->driver->shutdown(hcd); > >> } > >> > >> +static int omap_ehci_resume(struct device *dev) > >> +{ > >> + if (dev->parent) > >> + pm_runtime_get_sync(dev->parent); > >> + return 0; > >> +} > >> + > >> +static int omap_ehci_suspend(struct device *dev) > >> +{ > >> + if (dev->parent) > >> + pm_runtime_put_sync(dev->parent); > >> + return 0; > >> +} > > > >I don't see any point in these routines (and likewise for > >omap_ohci_suspend/resume). When the whole system is going to sleep > >anyway, what reason is there for enabling runtime PM on the parent > >device? > > Both for EHCI & OHCI, the clocks are owned by the parent (uhh-tll). > > Calling pm_runtime_put_sync(dev->parent) within omap_ehci_suspend > will turn-off the parent clocks in the Suspend path. > > Similarly, calling pm_runtime_get_sync(dev->parent) within > omap_ehci_resume > will turn-on the parent clocks in the resume path. > > This way, all reference counting are implicit within the Runtime PM layer > and takes care of all combinations of only EHCI insmoded, OHCI insmoded, > both insmoded etc. > > When both EHCI & OHCI are suspended, parent clocks will actually be > turned OFF and vice-versa. not sure this is necessary. I would expect: pm_runtime_get_sync(dev) to propagate up the parent tree and enable all necessary resources to get the child in a working state. IOW, you shouldn't need to manuall access the parent device. Kevin ? Paul ? -- balbi
Attachment:
signature.asc
Description: Digital signature
