linux-kernel - Re: [PATCH 6/6 v2] arm: omap: usb: global Suspend and resume support of ehci and ohci

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Mon, 4 Jul 2011 11:25:54 +0300
From:	Felipe Balbi <balbi@...com>
To:	Partha Basak <p-basak2@...com>
Cc:	Alan Stern <stern@...land.harvard.edu>,
	Keshava Munegowda <keshava_mgowda@...com>,
	linux-usb@...r.kernel.org, linux-omap@...r.kernel.org,
	linux-kernel@...r.kernel.org, Felipe Balbi <balbi@...com>,
	Anand Gadiyar <gadiyar@...com>, sameo@...ux.intel.com,
	parthab@...ia.ti.com, tony@...mide.com,
	Kevin Hilman <khilman@...com>,
	Benoit Cousson <b-cousson@...com>, paul@...an.com,
	johnstul@...ibm.com, Vishwanath Sripathy <vishwanath.bs@...com>
Subject: Re: [PATCH 6/6 v2] arm: omap: usb: global Suspend and resume support
 of ehci and ohci

Hi,

On Mon, Jul 04, 2011 at 10:36:54AM +0530, Partha Basak wrote:
> >-----Original Message-----
> >From: Alan Stern [mailto:stern@...land.harvard.edu]
> >Sent: Saturday, July 02, 2011 12:37 AM
> >To: Keshava Munegowda
> >Cc: linux-usb@...r.kernel.org; linux-omap@...r.kernel.org; linux-
> >kernel@...r.kernel.org; balbi@...com; gadiyar@...com;
> >sameo@...ux.intel.com; parthab@...ia.ti.com; tony@...mide.com;
> >khilman@...com; b-cousson@...com; paul@...an.com; johnstul@...ibm.com;
> >vishwanath.bs@...com
> >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@...com>
> >>
> >> 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@...com>
> >> ---
> >>  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

Download attachment "signature.asc" of type "application/pgp-signature" (491 bytes)