Ah whoops, I didn't notice your attached patch was new, I assumed it was the original ;-) That patch looks like it will do the trick. I'll try it out. On Wed, Jan 14, 2009 at 3:17 PM, Dan Streetman <ddstreet@xxxxxxxx> wrote: > On Wed, Jan 14, 2009 at 2:55 PM, Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> wrote: >> On Wed, 14 Jan 2009, Dan Streetman wrote: >> >>> The previous patch (as1197), to not reset the data toggle during driver >>> unbinding when an interface is already in altsetting 0, has introduced an >>> intermittent loss of packets due to data toggle mismatch. Specifically, the >>> ehci and ohci drivers do not use the device toggle field to actually track the >>> current toggle; they use hardware to track the device toggle. So avoiding the >>> call to usb_settoggle() in usb_endpoint_enable() isn't enough. >> >> Oho! You are right, although you didn't express the reason very >> clearly. usb_disable_endpoint's call to usb_hcd_disable_endpoint >> changes the hardware state in EHCI and OHCI controllers without >> changing the corresponding state in the device. > > Yep, that is a much better explanation :) > > To clarify further, the call to usb_hcd_disable_endpoint removes the > qh (for ehci) or the ed (for ohci) which has been keeping track of the > toggle. So when the qh or ed is re-created and put back on the > schedule, it defaults to a toggle of 0, which isn't always correct > since the device hasn't reset the toggle (in this case). > >> No, neither of your suggestions is right. The real problem is to >> distinguish between enabling/disabling endpoints just in the usbcore >> data structures or in both the core and the hardware. The reset_toggle >> argument added by as1197 was an attempt to do this, but it targeted >> only the enable-endpoint path. We need a corresponding change for the >> disable_endpoint path as well. > > Ok, you mean in the disable_endpoint path to avoid calling > usb_hcd_disable_endpoint since in some cases it shouldn't really > disable it? And therefore the HCD leaves its qh or ed on the > schedule, and so doesn't lose track of the toggle. > > Will avoiding the call to usb_hcd_disable_endpoint have other > unintended side effects? > >> Does this patch fix everything? > > The patch fixes the case for ehci and ohci (there was no problem in > uhci), but does not touch any of the other host controller > drivers...so if any of those use hardware toggling, they would need a > patch too. > > However it would be easier to just leave the qh or ed on the schedule > by avoiding the call to usb_hcd_disable_endpoint as you mentioned > above, assuming that won't cause any other side effects. > -- 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
