On Fri, Dec 16, 2011 at 3:38 AM, Felipe Balbi <balbi@xxxxxx> wrote: > 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 I tested this. Nothing bad happens (until isp1704_charger actually tries to do something, of course; the counters are messed up). Cheers. -- Felipe Contreras -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
