On Thu, Jan 16, 2014 at 8:35 AM, Sarah Sharp <sarah.a.sharp@xxxxxxxxxxxxxxx> wrote: > On Tue, Jan 07, 2014 at 12:30:00PM -0800, Dan Williams wrote: >> The port pm_runtime implementation unconditionally clears FEAT_C_ENABLE >> after clearing PORT_POWER, but the bit is reserved on usb3 hub ports. >> >> It also takes steps to re-disable the port if it fails to reconnect >> which again is broken on usb3 ports and unnecessary on usb2 ports. > > The subject is misleading, since you don't just change when you clear > FEAT_C_ENABLE for USB 2.0 hubs; you also change it for USB 3.0 hubs. > Suggest: > > "usb: Clear FEAT_C_ENABLE on port suspend for usb2, not usb3" > > Actually, that's still not clear, because the patch really fixes two > distinct bugs: > > 1. Don't clear FEAT_C_ENABLE for USB 3.0 hubs at all. > 2. Don't clear FEAT_C_ENABLE for USB 2.0 ports on failed resume. > > That really suggests it should be two separate patches, in case there's > say, a bug in patch #2 and we really do want to clear FEAT_C_ENABLE on > USB 2.0 port resume. > > Can you split this patch up? > Of course, will do. Given how fragile port recovery is in the current kernel I do not see how 2/ could cause a regression especially since it will be cleared as a matter of course by khubd. In fact, we want khubd to service this and not hide it with an opportunistic clearing in usb_port_runtime_resume(). I'll say this in the split patch change log. -- Dan -- 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
