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