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 <peter.chen@xxxxxxxxxxxxx>
> > ---
> > 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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
