On Tue, Oct 01, 2013 at 10:01:08PM +0300, Xenia Ragiadakou wrote: > Hi Sarah, > > I read the mail on 'possible conflict between xhci_hcd and a patched > usbhid'. For reference to others: http://marc.info/?l=linux-usb&m=138064948726038&w=2 http://marc.info/?l=linux-usb&m=138065201426880&w=2 > I looked in xhci and the problem arises in xhci_queue_intr_tx() when > if (xhci_interval != ep_interval) { > ... > urb->interval = xhci_interval; > } > > right? Yes. The underlying problem is that the xHCI host sets up the endpoint contexts during the Configure Endpoint command, using the interval from the device's endpoint descriptors. It also uses the endpoint descriptor wMaxPacketSize, which can be wrong as well. If the device driver wants to use a different urb->interval than is in the endpoint descriptor, the xHCI driver will simply ignore it. (I'm Ccing the linux-media list, as I've discussed some of these devices with broken descriptors before.) > When you say a new API, what do you mean? New functions in usbcore > to be used by usb device drivers? Yes. You would export the function in the USB core, and put a prototype in a USB include file (probably in include/linux/usb.h). Let's say that function is called usb_change_ep_bandwidth. Drivers could call into that function when they needed to change either the bInterval or wMaxPacketSize of a particular endpoint. This could be during the driver's probe function, or before switching alternate interface settings, or even after the alt setting is in place, but userspace dictates the driver use a different bandwidth. Drivers should pass usb_change_ep_bandwidth a pointer to the endpoint they need to change, along with the bInterval and wMaxPacketSize values they would like the endpoint to have. Those values could be stored as new values in struct usb_host_endpoint. usb_change_ep_bandwidth would then call into the xHCI driver to drop the endpoint, re-add it, and then issue a bandwidth change. The xHCI driver would have to be changed to look at the new fields in usb_host_endpoint, and set up the endpoint contexts with the interval and packet size from those fields, instead of the endpoint descriptor. We should probably set the new values in usb_host_endpoint to zero after the driver unbinds from the device. Not sure if they should be reset after the driver switches interfaces. I would have to see the use cases in the driver. > Here, it is needed to change the endpoint descriptors with the new > value in urb so that xhci takes the correct value? > I mean the fix should be made in usbcore and xhci shall remain intact? No, we need to fix both the xHCI driver and the USB core. > I have time to work on that but i 'm not sure that i understood. Sure. I would actually suggest you first finish up the patch to issue a configure endpoint if userspace wants to clear a halt, but the endpoint isn't actually halted. Did your most current patch work? I can't remember what the status was. Sarah Sharp -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
