On Mon, 10 Aug 2009, Sarah Sharp wrote: > > 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. usbcore doesn't touch endpoint 0 during config changes. It is removed and re-added only during device initialization, in usb_ep0_reinit(). > The usb_ep0_reinit() after the 8 byte > descriptor fetch would simply be ignored by the xHCI bandwidth code. So then how/where do the xHCI HW structures for ep0 get set up initially? In hcd->driver->address_device()? > 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. Yes, that sounds like a reasonable approach. > The USB core removes and readds the control > endpoint 0 on every configuration change, No, it doesn't. > 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. It's just a way of handling ep0 like every other endpoint. Of course, if the xHCI hardware really does handle ep0 specially then it's understandable that you might want to do this in a different way. > 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. Sorry, I don't understand the question (or rant). Where does usbcore enable only one direction of ep0? Are you talking about usb_ep0_reinit()? The difference there is caused by the difference in the arguments for usb_disable_endpoint() and usb_enable_endpoint(). One takes an endpoint address while the other takes a pointer to a struct usb_host_endpoint. Since the structure is for a control endpoint, it naturally refers to both directions. If usb_disable_endpoint() were rewritten to accept a pointer to struct usb_host_endpoint, then usb_ep0_reinit() would have to call it only once. But the loop near the start of usb_disable_device() would have to become more complicated. 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
