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: > >> In musb_init_controller() there's a pm_runtime_put(), but there's no > >> pm_runtime_get(), which creates a mismatch that causes the driver to > >> sleep when it shouldn't. > >> > >> This was introduced in 7acc619, but it wasn't triggered until 18a2689 > >> was merged to Linus' branch at point 6899608. > > > > you need to add the commit description (whatever was the mail's subject) > > here too. And you should put in Cc the author or those commits too, > > otherwise we can't poke into their brains to understand what they were > > thinking when they originally wrote those patches. > > True, but code is code, and even if you don't know what he was > thinking, it's clear there's something wrong. > > >> However, it seems most of the time this is used in a way that keeps the > >> counter above 0, so nobody noticed. Also, it seems to depend on the > >> configuration used. > >> > >> I found the problem by loading isp1704_charger before any usb gadgets: > >> http://article.gmane.org/gmane.linux.kernel/1226122 > >> > >> All versions after 2.6.39 are affected. > >> > >> Cc: stable@xxxxxxxxxxxxxxx > >> Signed-off-by: Felipe Contreras <felipe.contreras@xxxxxxxxx> > >> --- > >> drivers/usb/musb/musb_core.c | 2 -- > >> 1 files changed, 0 insertions(+), 2 deletions(-) > >> > >> diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c > >> index b63ab15..920f04e 100644 > >> --- 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). (now, this was all from memory, so it could be wrong ;-) -- balbi
Attachment:
signature.asc
Description: Digital signature
