| 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
| ||
|
Message-ID: <99b22453092fc32b751c3e7d0223c7eb@mail.gmail.com>
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