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

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, 28 Jan 2013 14:29:02 +0200
From:	Felipe Balbi <balbi@...com>
To:	Vivek Gautam <gautamvivek1987@...il.com>
CC:	<balbi@...com>, Vivek Gautam <gautam.vivek@...sung.com>,
	<linux-usb@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
	<linux-omap@...r.kernel.org>, <linux-samsung-soc@...r.kernel.org>,
	<gregkh@...uxfoundation.org>, <sarah.a.sharp@...ux.intel.com>,
	<kgene.kim@...sung.com>, <dianders@...omium.org>,
	<sylvester.nawrocki@...il.com>, <tomasz.figa@...il.com>
Subject: Re: [PATCH 3/4] usb: dwc3: exynos: Enable runtime power management

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@...com> 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

Download attachment "signature.asc" of type "application/pgp-signature" (837 bytes)