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.)

Alan Stern

--
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

Reply via email to