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?  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.

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.

Sarah Sharp

_______________________________________________
stable mailing list
[email protected]
http://linux.kernel.org/mailman/listinfo/stable

Reply via email to