On Thu, Aug 19, 2010 at 05:41:28PM +0800, Xu, Andiry wrote: > > -----Original Message----- > > From: Alan Stern [mailto:stern@xxxxxxxxxxxxxxxxxxx] > > Sent: Thursday, August 19, 2010 4:32 AM > > To: Sarah Sharp > > Cc: Xu, Andiry; linux-usb@xxxxxxxxxxxxxxx; gregkh@xxxxxxx > > Subject: Re: [PATCH 1/2] xHCI: change xhci_reset_device() to allocate > new > > device > > > > On Wed, 18 Aug 2010, Sarah Sharp wrote: > > > > > > There might be a problem if slot IDs get reused when you start > > reviving > > > > the devices under a controller that has been reset. The > udev->slot_id > > > > value might point to another device's structure. > > > > xhci_discover_or_reset_device will have to check and make sure > that > > the > > > > slot really does match the device. > > > > > > Would it be possible when the failed resume is detected to go > through > > > the list of usb_devices that the xHCI driver has slots for, and > change > > > the slot_id to -1? That would indicate the struct usb_device is > valid, > > > but there is no slot for it. > > > > > > The slot_id field in struct usb_device is an int, although there are > a > > > lot of places it's cast to a unsigned integer. Those places would > have > > > to be looked at, but I suspect that xhci_check_args() would be the > only > > > place that would need to be changed. Then the new > > > xhci_discover_or_reset_device() would need to check udev->slot_id > for -1 > > > instead of checking whether xhci->devs[slot_id] is non-null. > > > > Sure, you could do that if you wanted. But IMO it would be easier to > > store a pointer back to the usb_device in the slot structure, and have > > xhci_discover_or_reset_device check that the pointer points to the > > right device. > > > > I think it's a good idea. And I think xhci_free_dev() should also check > this, in order to avoid freeing the wrong device. Ok, you should probably change xhci_check_args() to check whether the pointers match too, to avoid things like changing bandwidth configurations for a different device. I don't think it's likely to happen, but it's probably better to cover that case too. Sarah Sharp -- 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
