Re: [PATCH 1/2] usb: gadget: composite: Race between disconnect/unbind and setup
On Sun, 8 Jul 2012, Kevin Cernekee wrote: On Sun, Jul 8, 2012 at 5:33 PM, Alan Stern st...@rowland.harvard.edu wrote: On Sun, 8 Jul 2012, Kevin Cernekee wrote: usb_gadget_remove_driver() runs through a four-step sequence to shut down the gadget driver. For the case of a composite gadget + at91 UDC, this would look like: udc-driver-disconnect(udc-gadget); // composite_disconnect() usb_gadget_disconnect(udc-gadget); // at91_pullup(gadget, 0) udc-driver-unbind(udc-gadget); // composite_unbind() usb_gadget_udc_stop(udc-gadget, udc-driver); // at91_stop() composite_disconnect() says: if (cdev-config) reset_config(cdev); reset_config() sets cdev-config to NULL. composite_unbind() later tests for this: WARN_ON(cdev-config); But SETUP packets may be sent to the composite driver up until the point when usb_gadget_disconnect() returns. That doesn't sound right. A host can't send SETUP packets to a disconnected port. The packets should stop arriving when udc-driver-disconnect returns -- assuming the UDC driver implements a disconnect method. udc-driver-disconnect points to composite_disconnect(). What is composite_disconnect() doing to tell the UDC driver to disconnect the port from the host? Oops -- you're right and I was wrong. It's the usb_gadget_disconnect() call which turns off the pullup. Sorry about the confusion. What I see is that usb_gadget_disconnect() does tell the UDC driver to stop activity on the port, but that only happens after composite_disconnect() was called. Can you confirm that the order of the calls in usb_gadget_remove_driver() is correct? They might need to be changed. It does seems like a bad idea to tell the gadget driver that a disconnect occurred before the host knows about it. If you change the order of those two calls, does that make your posted patch unnecessary? Alan Stern -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] usb: gadget: composite: Race between disconnect/unbind and setup
They might need to be changed. It does seems like a bad idea to tell the gadget driver that a disconnect occurred before the host knows about it. Yes, it should pull down dp first, then, tell class driver, it has been disconnected. If you change the order of those two calls, does that make your posted patch unnecessary? Alan Stern -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] usb: gadget: composite: Race between disconnect/unbind and setup
On Sun, Jul 8, 2012 at 5:33 PM, Alan Stern st...@rowland.harvard.edu wrote: On Sun, 8 Jul 2012, Kevin Cernekee wrote: usb_gadget_remove_driver() runs through a four-step sequence to shut down the gadget driver. For the case of a composite gadget + at91 UDC, this would look like: udc-driver-disconnect(udc-gadget); // composite_disconnect() usb_gadget_disconnect(udc-gadget); // at91_pullup(gadget, 0) udc-driver-unbind(udc-gadget); // composite_unbind() usb_gadget_udc_stop(udc-gadget, udc-driver); // at91_stop() composite_disconnect() says: if (cdev-config) reset_config(cdev); reset_config() sets cdev-config to NULL. composite_unbind() later tests for this: WARN_ON(cdev-config); But SETUP packets may be sent to the composite driver up until the point when usb_gadget_disconnect() returns. That doesn't sound right. A host can't send SETUP packets to a disconnected port. The packets should stop arriving when udc-driver-disconnect returns -- assuming the UDC driver implements a disconnect method. udc-driver-disconnect points to composite_disconnect(). What is composite_disconnect() doing to tell the UDC driver to disconnect the port from the host? What I see is that usb_gadget_disconnect() does tell the UDC driver to stop activity on the port, but that only happens after composite_disconnect() was called. Can you confirm that the order of the calls in usb_gadget_remove_driver() is correct? Thanks. -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
