RE: [PATCH 6/6 v2] arm: omap: usb: global Suspend and resume support of ehci and ohci

[Partha Basak <p-basak2@ti.com>]

>-----Original Message-----
>From: Alan Stern [mailto:stern@rowland.harvard.edu]
>Sent: Saturday, July 02, 2011 12:37 AM
>To: Keshava Munegowda
>Cc: linux-usb@vger.kernel.org; linux-omap@vger.kernel.org; linux-
>kernel@vger.kernel.org; balbi@ti.com; gadiyar@ti.com;
>sameo@linux.intel.com; parthab@india.ti.com; tony@atomide.com;
>khilman@ti.com; b-cousson@ti.com; paul@pwsan.com; johnstul@us.ibm.com;
>vishwanath.bs@ti.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@ti.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@ti.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.

Note that the parent per-se does not have any .suspend & .resume hooked
up.
At the end of the _probe of parent, the clocks are turned OFF.
Subsequently, enabling
the parent clocks are entirely done implicitly by the children get_sync()
in their _probe.

Therefore while .suspend/.resume of children are called they call back
into the parent to turn-off the clocks.

FYI, Keshava is out of office due to some emergency this week,
there will be some delays in replying from his side.

>
>Alan Stern