Hi,
On Tue, Oct 16, 2012 at 05:10:39PM +0530, Vikas Sajjan wrote:
> Hi Felipe,
>
> On 16 October 2012 15:36, Felipe Balbi <balbi@xxxxxx> wrote:
> > Hi,
> >
> > On Tue, Oct 16, 2012 at 03:15:36PM +0530, Vikas Sajjan wrote:
> >> Adds suspend and resume callbacks as part of the power management
> >> support to DWC3 controller Driver.
> >> This patch facilitates transition of DWC3 controller between D0 and D3
> >> power states during suspend/resume cycles.
> >>
> >> Signed-off-by: Abhilash Kesavan <a.kesavan@xxxxxxxxxxx>
> >> Signed-off-by: Vikas C Sajjan <vikas.sajjan@xxxxxxxxxx>
> >> CC: Doug Anderson <dianders@xxxxxxxxxxxx>
> >> ---
> >> drivers/usb/dwc3/core.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
> >> 1 files changed, 44 insertions(+), 0 deletions(-)
> >>
> >> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> >> index 5db4c76..9f35cf8 100644
> >> --- a/drivers/usb/dwc3/core.c
> >> +++ b/drivers/usb/dwc3/core.c
> >> @@ -621,11 +621,55 @@ static int __devexit dwc3_remove(struct platform_device *pdev)
> >> return 0;
> >> }
> >>
> >> +#ifdef CONFIG_PM_SLEEP
> >> +static int dwc3_resume(struct device *dev)
> >> +{
> >> + struct dwc3 *dwc = dev_get_drvdata(dev);
> >> + int ret;
> >> +
> >> + ret = dwc3_core_init(dwc);
> >> + if (ret < 0)
> >> + return ret;
> >> +
> >> + switch (dwc->mode) {
> >> + case DWC3_MODE_DEVICE:
> >> + dwc3_set_mode(dwc, DWC3_GCTL_PRTCAP_DEVICE);
> >> + break;
> >> + case DWC3_MODE_HOST:
> >> + dwc3_set_mode(dwc, DWC3_GCTL_PRTCAP_HOST);
> >> + break;
> >> + case DWC3_MODE_DRD:
> >> + dwc3_set_mode(dwc, DWC3_GCTL_PRTCAP_OTG);
> >> + }
> >> +
> >> + /* runtime set active to reflect active state. */
> >> + pm_runtime_disable(dev);
> >> + pm_runtime_set_active(dev);
> >> + pm_runtime_enable(dev);
> >> +
> >> + return 0;
> >> +}
> >> +
> >> +static int dwc3_suspend(struct device *dev)
> >> +{
> >> + struct dwc3 *dwc = dev_get_drvdata(dev);
> >> +
> >
> > there is one check you need to do here. If you are playing the
> > peripheral role, you can't allow suspend unless link is in U3 or you're
> > not enumerated.
> >
> > Have you tested running 'echo mem > /sys/power/state' on your device
> > while you're transferring data in a USB session with the Host ?
> >
> we have tested only when DWC3 is in HOST mode.
I'm very sorry but this isn't enough :-( We need to make sure there are
no regressions on peripheral role.
> > I'll ask again, how was this tested ? What did you actually run (which
> > commands, which use cases have you validated) ? I'm not asking only
> > which platform you used, I need to know how you exercised this feature,
> > how did you trigger suspend/resume, in which conditions (enumerated ?
> > bus suspended ? no cable attached ?), etc.
> >
> 1) We have tested by connecting a USB Mouse, USB 2.0 Mass Storage Device and
> USB 3.0 Mass Storage Device on a SS hub connected on USB 3.0 port to
> Exynos5250 board, in which DWC3 is configured for HOST mode.
did you have a transfer going through when you suspended ? If you
didn't, then you haven't stressed well enough.
> 2) We tested by running command
> echo +20 > /sys/class/rtc/rtc0/wakealarm && echo mem > /sys/power/state
that's ok, but try something like:
# dd if=/dev/urandom of=/dev/sdXX bs=1M count=99999999 &
# echo +20 > /sys/class/rtc/rtc0/wakealarm && echo mem > /sys/power/state
Better yet, generate a big (and I really mean big :-) file with a known
pattern (make a sequence from 0x00 to 0xff and back to zero, over and
over again) then write that file to the mass storage device, while
transfer is on the fly, suspend, then resume and see if transfer
continues, then later verify the bytes by reading back the data and
comparing with original source file.
Do this for both Host and Peripheral roles. I have a rather big
suspicion that it won't work, in which case we need to make sure to
prevent suspend while transfers are on the fly or do something else.
--
balbi
Attachment:
signature.asc
Description: Digital signature
