On Fri, Aug 07, 2009 at 05:22:59PM -0400, Alan Stern wrote: > On Fri, 7 Aug 2009, Sarah Sharp wrote: > > > Full speed devices have varying max packet sizes (8, 16, 32, or 64) for > > endpoint 0. The xHCI hardware needs to know the real max packet size > > that the USB core discovers after it fetches the first 8 bytes of the > > device descriptor. > > > > In order to fix this without adding a new hook to host controller drivers, > > the xHCI driver looks for an updated max packet size for control > > endpoints. If it finds an updated size, it issues an evaluate context > > command and waits for that command to finish. This should only happen in > > the initialization and device descriptor fetching steps in the khubd > > thread, so blocking should be fine. > > Did you know that usbcore does a usb_disable_endpoint() for ep0 when > the maxpacket value is updated? In view of this fact, perhaps the > patch isn't needed. The short answer is that the xHCI code that hooks into the usb_disable_endpoint() and usb_enable_endpoint() to update bandwidth calculations on the HW doesn't handle changes to control endpoint 0. It just ignores the USB core removing and re-adding control endpoint 0 during a configuration change. The usb_ep0_reinit() after the 8 byte descriptor fetch would simply be ignored by the xHCI bandwidth code. The slightly longer answer is that the xHCI HW has two commands for updating endpoint information. One is a "Configure Endpoint" command that will add, remove, or update information about any endpoint besides the default control endpoints. The second command is an "Evaluate Context" command that only works on the default control endpoint, and the device information (e.g. is this device a hub). The xHCI bandwidth code builds up an input variable for a Configure Endpoint command with every call to usb_disable_endpoint() and usb_enable_endpoint(). When the USB core calls usb_hcd_check_bandwidth(), the xHCI driver submits that command. If I supported changing the max packet size through usb_disable_endpoint() and usb_enable_endpoint(), I would have to special case this bandwidth code to submit an Evaluate Context command when endpoint 0 changed. The USB core removes and readds the control endpoint 0 on every configuration change, which means I would have to issue an Evaluate Context command when the default endpoint characteristics didn't really change, or further special case this code to check whether the endpoint characteristics actually changed. I'm not really comfortable with special casing the bandwidth code to handle max packet size 0 changes. One slight rant: why does the USB core disable both directions of the control endpoint 0 but only enable one direction? Shouldn't it enable both directions? That's yet another reason why I didn't want to handle control endpoint 0 changes in the bandwidth code. Sarah -- 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
