Hi, On Fri, Dec 16, 2011 at 03:13:13AM +0200, Felipe Contreras wrote: > On Fri, Dec 16, 2011 at 1:50 AM, Felipe Balbi <balbi@xxxxxx> wrote: > > On Fri, Dec 16, 2011 at 01:31:02AM +0200, Felipe Contreras wrote: > >> On Fri, Dec 16, 2011 at 1:01 AM, Felipe Balbi <balbi@xxxxxx> wrote: > >> > On Fri, Dec 16, 2011 at 12:42:14AM +0200, Felipe Contreras wrote: > >> >> --- a/drivers/usb/musb/musb_core.c > >> >> +++ b/drivers/usb/musb/musb_core.c > >> >> @@ -2012,8 +2012,6 @@ musb_init_controller(struct device *dev, int nIrq, void __iomem *ctrl) > >> >> if (status < 0) > >> >> goto fail3; > >> >> > >> >> - pm_runtime_put(musb->controller); > >> > > >> > To me the real fix would be add the missing pm_runtime_get_sync(). On > >> > probe() we're actually accessing MUSB's address space which needs it's > >> > clocks turned on. I guess it's only working now by chance, probably > >> > because glue layer calls pm_runtime_get_sync() to access it's own > >> > address space and that uses the same clocks. > >> > >> Are you sure it's "musb-hdrc", and not "musb-omap2430" the one > >> accessing the relevant address-space? From the runtime_pm > >> documentation it looks like only the probe function should deal with > >> this. > >> > >> If "musb-hdrc" was truly accessing these registers, then I would get > >> the same failure because the clocks are turned off, but I don't... > > > > see musb_core_init(); You don't see any problems when accessing those > > addresses because musb_platform_init() will fall into > > omap2430_musb_init() which calls pm_runtime_get_sync(), and the same > > clock actually enables both address spaces (musb-omap2430 and > > musb-hdrc). > > That's true, but how would I go test this theory? Call > pm_runtime_put_sync() at the end of omap2430_musb_init()? sounds like a plan... Not sure if it will work always though. If I remember correctly, pm_runtime_put_sync() will only be synchronous to the current device, but will go up the parent tree asynchronously. Which means that dev->parent will be see a scheduled runtime_put > Also, I think most pm_runtime_disable() calls shouldn't be there... > That would tell PM to activate power for the device, which is not what > we want. The core drivers code will take care of truly disabling the > pm_runtime stuff _without_ activating the device. true, sounds correct. -- balbi
Attachment:
signature.asc
Description: Digital signature
