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 14:56:30 +0530
From:	Partha Basak <p-basak2@...com>
To:	balbi@...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, 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

>-----Original Message-----
>From: Felipe Balbi [mailto:balbi@...com]
>Sent: Monday, July 04, 2011 1:56 PM
>To: Partha Basak
>Cc: Alan Stern; Keshava Munegowda; linux-usb@...r.kernel.org; linux-
>omap@...r.kernel.org; linux-kernel@...r.kernel.org; Felipe Balbi; Anand
>Gadiyar; sameo@...ux.intel.com; parthab@...ia.ti.com; tony@...mide.com;
>Kevin Hilman; Benoit Cousson; paul@...an.com; johnstul@...ibm.com;
>Vishwanath Sripathy
>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.
>
Refer to the description in Patch(5/6)
<snip>
In fact, the runtime framework takes care the get sync and put sync of the
child
in turn call the get sync and put sync of parent too; but calling get sync
and
put sync of parent is by ASYNC mode;
This mode queues the work item in runtime pm work queue,
which not getting scheduled in case of global suspend path.
<snip>
This approach was tried, but did not work in the Suspend path
static int rpm_suspend(struct device *dev, int rpmflags)
	__releases(&dev->power.lock) __acquires(&dev->power.lock)
{
.
.
.
no_callback:
.
.
.
	/* Maybe the parent is now able to suspend. */
	if (parent && !parent->power.ignore_children &&
!dev->power.irq_safe) {
		spin_unlock(&dev->power.lock);

		spin_lock(&parent->power.lock);
		rpm_idle(parent, RPM_ASYNC);
		spin_unlock(&parent->power.lock);

		spin_lock(&dev->power.lock);
	}
This is the reason of directly calling the parent Runtime PM calls from
the children.
If directly calling Runtime PM APIs with parent dev-pointer isn't
acceptable,
this can be achieved by exporting wrapper APIs from the
parent and calling them from the chidren .suspend/.resume routines.

>Kevin ? Paul ?
>
>--
>balbi
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists