Re: [PATCH 3/4] usb: dwc3: exynos: Enable runtime power management

[Felipe Balbi]

On Mon, Jan 28, 2013 at 05:57:04PM +0530, Vivek Gautam wrote:
> On Mon, Jan 28, 2013 at 5:42 PM, Felipe Balbi <balbi@xxxxxx> wrote:
> > Hi,
> >
> > On Mon, Jan 28, 2013 at 05:28:30PM +0530, Vivek Gautam wrote:
> >> >> +static int dwc3_exynos_runtime_resume(struct device *dev)
> >> >> +{
> >> >> +     struct dwc3_exynos      *exynos = dev_get_drvdata(dev);
> >> >> +     struct platform_device  *pdev_dwc = exynos->dwc3;
> >> >> +     struct dwc3             *dwc = NULL;
> >> >> +
> >> >> +     dwc = platform_get_drvdata(pdev_dwc);
> >> >> +
> >> >> +     clk_enable(exynos->clk);
> >> >> +
> >> >> +     if (!dwc)
> >> >> +             return 0;
> >> >> +
> >> >> +     pm_runtime_get_sync(dwc->usb3_phy->dev);
> >> >
> >> > dude, this is wrong :-)
> >> >
> >> > look at this:
> >> >
> >> > pm_runtime_get() -> dwc3_exynos_runtime_resume() ->
> >> > pm_runtime_get_sync() -> dwc3_exynos_runtime_resume() -> ...
> >> >
> >> > only your clock enalbe should do
> >> >
> >>
> >> We want to wake up "dwc->usb3_phy" so tried to call pm_runtime_get_sync()
> >> with "dwc->usb3_phy->dev".
> >> Missing something ? :-(
> >
> > oh, my bad. That's the PHY... But we can't really do that for samsung
> > only. It needs to be done generically for the entire dwc3 core driver,
> > and for that we need to introduce usb_phy_autopm_get(),
> > usb_phy_autopm_get_sync() and friends.
> >
> 
> aah!! Ok. I definitely missed that part. :-(
> 
> > Then, from dwc_probe() we call:
> >
> > phy = usb_get_phy();
> > usb_phy_autopm_enable(phy);
> > usb_phy_autopm_get_sync(phy);
> >
> > or something similar ;-) Bottom line, you shouldn't fiddle with phy->dev
> > directly.
> >
> 
> Ok, the core should actually be handling the 'phy' not the glue layers.
> Right ?

that's right, thanks for doing this ;-)

> Will try putting these helper functions in place and come up with a
> solution. :-)

awesome ;-)

cheers

-- 
balbi

Attachment: signature.asc
Description: Digital signature