On Thu, Jul 07, 2011 at 05:42:50PM -0700, Sarah Sharp wrote: > On Thu, Jul 07, 2011 at 02:46:31PM -0700, Greg KH wrote: > > On Wed, Jun 29, 2011 at 01:45:05AM +0000, James Bottomley wrote: > > > commit: fccf4e86200b8f5edd9a65da26f150e32ba79808 > > > From: Sarah Sharp <[email protected]> > > > Date: Sun, 5 Jun 2011 23:22:22 -0700 > > > Subject: [PATCH] USB: Free bandwidth when usb_disable_device is called. > > > > > > Tanya ran into an issue when trying to switch a UAS device from the BOT > > > configuration to the UAS configuration via the bConfigurationValue sysfs > > > file. Before installing the UAS configuration, set_bConfigurationValue() > > > calls usb_disable_device(). That function is supposed to remove all host > > > controller resources associated with that device, but it leaves some state > > > in the xHCI host controller. > > > > > > Commit 0791971ba8fbc44e4f476079f856335ed45e6324 > > > usb: allow drivers to use allocated bandwidth until unbound > > > added a call to usb_disable_device() in usb_set_configuration(), before > > > the xHCI bandwidth functions were invoked. That commit fixed a bug, but > > > also introduced a bug that is triggered when a configured device is > > > switched to a new configuration. > > > > > > usb_disable_device() goes through all the motions of unbinding the drivers > > > attached to active interfaces and removing the USB core structures > > > associated with those interfaces, but it doesn't actually remove the > > > endpoints from the internal xHCI host controller bandwidth structures. > > > > > > When usb_disable_device() calls usb_disable_endpoint() with reset_hardware > > > set to true, the entries in udev->ep_out and udev->ep_in will be set to > > > NULL. Usually, when the USB core installs a new configuration, > > > usb_hcd_alloc_bandwidth() will drop all non-NULL endpoints in udev->ep_out > > > and udev->ep_in before adding any new endpoints. However, when the new > > > UAS configuration was added, all those entries were null, so none of the > > > old endpoints in the BOT configuration were dropped. > > > > > > The xHCI driver blindly added the UAS configuration endpoints, and some of > > > the endpoint addresses overlapped with the old BOT configuration > > > endpoints. This caused the xHCI host to reject the Configure Endpoint > > > command. Now that the xHCI driver code is cleaned up to reject a > > > double-add of active endpoints, we need to fix the USB core to properly > > > drop old endpoints in usb_disable_device(). > > > > > > If the host controller driver needs bandwidth checking support, make > > > usb_disable_device() call usb_disable_endpoint() with > > > reset_hardware set to false, drop the endpoints from the xHCI host > > > controller, and then call usb_disable_endpoint() again with > > > reset_hardware set to true. > > > > > > The first call to usb_disable_endpoint() will cancel any pending URBs and > > > wait on them to be freed in usb_hcd_disable_endpoint(), but will keep the > > > pointers in udev->ep_out and udev->ep in intact. Then > > > usb_hcd_alloc_bandwidth() will use those pointers to know which endpoints > > > to drop. > > > > > > The final call to usb_disable_endpoint() will do two things: > > > > > > 1. It will call usb_hcd_disable_endpoint() again, which should be harmless > > > since the ep->urb_list should be empty after the first call to > > > usb_disable_endpoint() returns. > > > > > > 2. It will set the entries in udev->ep_out and udev->ep in to NULL, and > > > call > > > usb_hcd_disable_endpoint(). That call will have no effect, since the xHCI > > > driver doesn't set the endpoint_disable function pointer. > > > > > > Note that usb_disable_device() will now need to be called with > > > hcd->bandwidth_mutex held. > > > > > > This should be backported to kernels as old as 2.6.32. > > > > This doesn't apply to .32, or .33, care to provide a backport? > > I can try, maybe next week?
That's fine, no rush from me, those kernels aren't going anywhere :) > In the meantime, do you really want to add > Alan's patch "USB: fix regression occurring during device removal" to > the 2.6.32 and 2.6.33 trees? I think it's not needed unless this patch > is applied. Hm, good point, how about I just drop those patches from .32 and .33 for now. > Also, do you want to wait until Alan's second bug fix patch > "USB: additional regression fix for device removal" gets into Linus' > tree before applying either this patch or Alan's second patch? I would > hate for a stable tree to be released when we know this patch causes > khubd hangs without both of Alan's patches. I was counting on Linus taking that patch soon enough for me to get it in with these before 2.6.39.3 is released. Hopefully that works out... thanks, greg k-h _______________________________________________ stable mailing list [email protected] http://linux.kernel.org/mailman/listinfo/stable
