Re: [PATCH V2] usb: musb: Fix unstable init of OTG_INTERFSEL.
On Fri, Nov 29, 2013 at 10:34 AM, Andreas Naumann wrote: >> Nice find, I think I'm also affected by this. However this crashes on >> OMAP3530 pandora because you now access INTERFSEL before interface >> clock is enabled.. This is not a problem on DM37xx because it uses >> different interconnects. You should probably use "context_valid" >> variable in omap2430_glue or similar to decide to write to register or >> not. >> >> Grazvydas > > > You mean introducing a "context_value"? I'm not sure if i want to add more > code. What I meant is to have a flag that would be set after omap2430_runtime_suspend() is called, and only write to INTERFSEL in omap2430_runtime_resume() if that flag is set. Drivers like drivers/tty/serial/omap-serial.c or drivers/gpio/gpio-omap.c use get_context_loss_count() callbacks for this, but it looks unnecessarily complicated.. Maybe Felipe has some ideas on this? > Do you think simple removing the first read is an option? So far we read > back 0 anyway because the first write to INTERFSEL in resume() set it to > uninitialized / 0 anyway. It should be fine but I don't know if all OMAPs only have PHY interface bits, there could be some that have some more bits there.. Yes current code is broken and writes 0 anyway, so any solution is better, but ideally it should read-modify-write the register and only change PHYSEL bits at probe. -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V2] usb: musb: Fix unstable init of OTG_INTERFSEL.
Nice find, I think I'm also affected by this. However this crashes on OMAP3530 pandora because you now access INTERFSEL before interface clock is enabled.. This is not a problem on DM37xx because it uses different interconnects. You should probably use "context_valid" variable in omap2430_glue or similar to decide to write to register or not. Grazvydas You mean introducing a "context_value"? I'm not sure if i want to add more code. Do you think simple removing the first read is an option? So far we read back 0 anyway because the first write to INTERFSEL in resume() set it to uninitialized / 0 anyway. -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V2] usb: musb: Fix unstable init of OTG_INTERFSEL.
On Thu, Nov 28, 2013 at 9:51 AM, wrote: > From: Andreas Naumann > > This is a hard to reproduce problem which leads to non-functional > USB-OTG port in 0.1%-1% of all boots. Tracked it down to commit > e25bec160158abe86c276d7d206264afc3646281, which introduces save/restore > of OTG_INTERFSEL over suspend. > Since the resume function is also called early in driver init, it uses a > non-initialized value (which is 0 and a non-supported setting in DM37xx > for INTERFSEL). Shortly after the correct value is set. Apparently this > works most time, but not always. > > The fix is to initialize the value, BEFORE being written in the resume > function. Nice find, I think I'm also affected by this. However this crashes on OMAP3530 pandora because you now access INTERFSEL before interface clock is enabled.. This is not a problem on DM37xx because it uses different interconnects. You should probably use "context_valid" variable in omap2430_glue or similar to decide to write to register or not. GraÅžvydas -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
