On Fri, 26 Oct 2012, Alan Stern wrote: > On Fri, 26 Oct 2012, Sarah Sharp wrote: > > > On Fri, Oct 26, 2012 at 03:01:32PM -0700, Sarah Sharp wrote: > > > The USB core isn't dropping the endpoints before it calls > > > xhci_check_bandwidth. I remember running into this bug a while back, > > > and I even started on a fix, but then couldn't reproduce the problem. > > > I found the branch with the old fix on it, but it still needs a bit of > > > work. I'll send you a patch on Monday. > > > > Matthias, can you try the attached patch? You should be able to echo 1 > > to the configuration file after this is applied. > > It seems that this patch should go farther than it does. > > Basically, what you are doing amounts to splitting > usb_hcd_alloc_bandwidth() up into two separate routines: one for > altsetting changes and one for config changes. But that's not how the > new code is structured and as a result it ends up looking rather > tortuous. > > Also, I don't like the way we repeat that "find altsetting 0 or use the > first altsetting" logic in several places. We should have a single > routine that determines which alternate interfaces will be installed as > part of a new config. After more thought, it seems that the problem goes deeper. When changing bandwidth settings, you need to figure out which endpoints to remove and which to add. (The code for doing this really should go message.c, not hcd.c, but never mind that for now.) This is complicated by the fact that we are dealing with two different views of the set of active endpoints: the device's view (which is pretty much the same as usbcore's) and the HC's view. It is inevitable that the two views will get out of sync from time to time, because there's no way to update the bandwidth allocation while simultaneously telling the device to switch configs. That's the underlying cause of the original problem. When usb_hcd_alloc_bandwidth sees that a config is being removed, it uses the udev->ep_in and udev->ep_out arrays to figure out which endpoints are going away. This means it is using usbcore's view. But it is sending that information to xhci-hcd, which uses the HC's view. When the two views disagree, it doesn't work. In particular, the views disagree when a Set-Config request fails. The ep_in and ep_out arrays don't get updated until later, when usb_enable_interface runs, but the bandwidth has already been allocated. The easiest fix is probably to move the whole "Initialize the new interface structures..." loop in usb_set_configuration up above the usb_control_msg call. The error recovery would have to be updated accordingly, of course. Alan Stern -- 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
