On Tue, Jan 13, 2015 at 10:13:32AM -0500, Alan Stern wrote: > On Tue, 13 Jan 2015, Peter Chen wrote: > > > This is an internal API, it can be used to find corresponding udc > > according to usb_gadget_driver. > > > > Signed-off-by: Peter Chen <[email protected]> > > --- > > drivers/usb/gadget/udc/udc-core.c | 33 +++++++++++++++++++++++---------- > > 1 file changed, 23 insertions(+), 10 deletions(-) > > > > diff --git a/drivers/usb/gadget/udc/udc-core.c > > b/drivers/usb/gadget/udc/udc-core.c > > index 0156709..7f8dc5b 100644 > > --- a/drivers/usb/gadget/udc/udc-core.c > > +++ b/drivers/usb/gadget/udc/udc-core.c > > @@ -70,6 +70,22 @@ found: > > return udc; > > } > > > > +static struct usb_udc *udc_driver_find_udc(struct usb_gadget_driver > > *driver) > > +{ > > + struct usb_udc *udc = NULL; > > + > > + mutex_lock(&udc_lock); > > + list_for_each_entry(udc, &udc_list, list) > > + if (udc->driver == driver) > > + goto found; > > + mutex_unlock(&udc_lock); > > + > > + return NULL; > > + > > +found: > > + mutex_unlock(&udc_lock); > > + return udc; > > +} > > /* > > ------------------------------------------------------------------------- */ > > #ifdef CONFIG_HAS_DMA > > > > @@ -469,17 +485,14 @@ int usb_gadget_unregister_driver(struct > > usb_gadget_driver *driver) > > if (!driver || !driver->unbind) > > return -EINVAL; > > > > - mutex_lock(&udc_lock); > > - list_for_each_entry(udc, &udc_list, list) > > - if (udc->driver == driver) { > > - usb_gadget_remove_driver(udc); > > - usb_gadget_set_state(udc->gadget, > > - USB_STATE_NOTATTACHED); > > - ret = 0; > > - break; > > - } > > + udc = udc_driver_find_udc(driver); > > + if (udc) { > > + usb_gadget_remove_driver(udc); > > + usb_gadget_set_state(udc->gadget, > > + USB_STATE_NOTATTACHED); > > + ret = 0; > > + } > > > > - mutex_unlock(&udc_lock); > > return ret; > > } > > EXPORT_SYMBOL_GPL(usb_gadget_unregister_driver); > > This doesn't seem to be an improvement. All it does is increase the > amount of code, because the new subroutine gets used in only one place. > > Also, it means that now you are calling usb_gadget_remove_driver() > without holding the udc_lock mutex, which probably isn't a good idea. > > (In addition, this change will make it more difficult in the future if > we ever allow a single driver to support more than one gadget.) >
Thanks, will drop this patch. Do you have any comments for patch 3/4 and 4/4, they are major changes in this patch set. -- Best Regards, Peter Chen -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
