Hi, > -----Original Message----- > From: Felipe Balbi [mailto:balbi@xxxxxx] > Sent: Thursday, July 26, 2012 4:13 PM > To: Bhupesh SHARMA > Cc: Laurent Pinchart; balbi@xxxxxx; linux-usb@xxxxxxxxxxxxxxx > Subject: Re: [RFC] usb: gadget: Ensure correct functionality for > gadgets that wish to 'connect' only when directed by user-space > > Hi, > > On Thu, Jul 26, 2012 at 06:28:36PM +0800, Bhupesh SHARMA wrote: > > > I just doesn't work :-) Calling usb_gadget_disconnect() during bind > > > doesn't prevent the device from being visible to the USB host. > > > Bhupesh has analyzed the problem and submitted this RFC as a > > > possible fix. > > how about something like this instead: > > diff --git a/drivers/usb/gadget/composite.c > b/drivers/usb/gadget/composite.c index 3f72110..3390af2 100644 > --- a/drivers/usb/gadget/composite.c > +++ b/drivers/usb/gadget/composite.c > @@ -1640,6 +1640,8 @@ int usb_composite_probe(struct > usb_composite_driver *driver, > composite = driver; > composite_gadget_bind = bind; > > + composite_driver.controls_pullup = driver->controls_pullup; > + > return usb_gadget_probe_driver(&composite_driver, > composite_bind); } > > diff --git a/drivers/usb/gadget/udc-core.c b/drivers/usb/gadget/udc- > core.c index e5e44f8..d8b4dc3 100644 > --- a/drivers/usb/gadget/udc-core.c > +++ b/drivers/usb/gadget/udc-core.c > @@ -263,7 +263,10 @@ static void usb_gadget_remove_driver(struct > usb_udc *udc) > > if (udc_is_newstyle(udc)) { > udc->driver->disconnect(udc->gadget); > - usb_gadget_disconnect(udc->gadget); > + > + if (!udc->driver->controls_pullup) > + usb_gadget_disconnect(udc->gadget); > + > udc->driver->unbind(udc->gadget); > usb_gadget_udc_stop(udc->gadget, udc->driver); > } else { > @@ -347,7 +350,9 @@ found: > driver->unbind(udc->gadget); > goto err1; > } > - usb_gadget_connect(udc->gadget); > + > + if (!driver->controls_pullup) > + usb_gadget_connect(udc->gadget); > } else { > > ret = usb_gadget_start(udc->gadget, driver, bind); diff -- > git a/drivers/usb/gadget/webcam.c b/drivers/usb/gadget/webcam.c index > 120e134..1cfd951 100644 > --- a/drivers/usb/gadget/webcam.c > +++ b/drivers/usb/gadget/webcam.c > @@ -396,6 +396,8 @@ static struct usb_composite_driver webcam_driver = > { > .strings = webcam_device_strings, > .max_speed = USB_SPEED_SUPER, > .unbind = webcam_unbind, > + > + .controls_pullup = true, > }; > > static int __init > diff --git a/include/linux/usb/composite.h > b/include/linux/usb/composite.h index 9d8c3b6..28326e5 100644 > --- a/include/linux/usb/composite.h > +++ b/include/linux/usb/composite.h > @@ -262,6 +262,8 @@ void usb_remove_config(struct usb_composite_dev *, > * @max_speed: Highest speed the driver supports. > * @needs_serial: set to 1 if the gadget needs userspace to provide > * a serial number. If one is not provided, warning will be > printed. > + * @controls_pullup: this driver will control pullup and udc-core > shouldn't > + * enable it by default > * @unbind: Reverses bind; called as a side effect of unregistering > * this driver. > * @disconnect: optional driver disconnect method @@ -290,6 +292,7 @@ > struct usb_composite_driver { > struct usb_gadget_strings **strings; > enum usb_device_speed max_speed; > unsigned needs_serial:1; > + unsigned controls_pullup:1; > > int (*unbind)(struct usb_composite_dev *); > > diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h > index 9517466..eaa50ab 100644 > --- a/include/linux/usb/gadget.h > +++ b/include/linux/usb/gadget.h > @@ -787,6 +787,7 @@ static inline int usb_gadget_disconnect(struct > usb_gadget *gadget) > * @suspend: Invoked on USB suspend. May be called in_interrupt. > * @resume: Invoked on USB resume. May be called in_interrupt. > * @driver: Driver model state for this driver. > + * @controls_pullup: tells udc-core to not enable pullups by default > * > * Devices are disabled till a gadget driver successfully bind()s, > which > * means the driver will handle setup() requests needed to enumerate > (and @@ -844,6 +845,8 @@ struct usb_gadget_driver { > > /* FIXME support safe rmmod */ > struct device_driver driver; > + > + unsigned controls_pullup:1; > }; > > > > All that's missing now is for uvc gadget to call > usb_function_activate/usb_function_deactivate() and the correct > locations. Make sure to call it from your ->disconnect() method as udc- > core won't do that for you by default. > > > Yes. Felipe, plz go through the problem description as mentioned in > > the patch log message which can be seen here: > > http://permalink.gmane.org/gmane.linux.usb.general/66585 > > sure, please see above patch. The difference is that this has no impact > on the controller drivers. Yes. I had sent a modified patch (on top of my original one) that did not affect the controller driver and instead modified the way 'pullup' is called from the udc-core, based on the simple 'is_pulleddown' flag. Plz see it here: http://www.spinics.net/lists/linux-usb/msg66338.html This patch has an added advantage of not even modifying the webcam_driver like composite drivers and has been tested to work well at-least with the webcam gadget. Regards, Bhupesh -- 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
