* Michal Nazarewicz | 2011-05-13 13:24:23 [+0200]:
>On Fri, 13 May 2011 13:05:34 +0200, Felipe Balbi <balbi@xxxxxx> wrote:
>>@@ -43,6 +43,12 @@ menuconfig USB_GADGET
>>if USB_GADGET
>>+config USB_UDC_CORE
>>+ bool "USB UDC Core Support"
>>+ help
>>+ This is the new style UDC Core Layer. If your driver has been
>>+ converted to this new layer, then you should say Y or M here.
>>+
>
>How user is supposed to know? Shouldn't the drivers themselves select it
>when needed?
It should be always on if gadget is on. It will be probably if it
remains invisble to the user.
The complete gadget framework can be compiled as modules but since the
udc-class provides a release function it has to remain in kernel.
>>diff --git a/drivers/usb/gadget/udc-core.c
>>b/drivers/usb/gadget/udc-core.c
>>new file mode 100644
>>index 0000000..dedf3adb
>>--- /dev/null
>>+++ b/drivers/usb/gadget/udc-core.c
...
>>+void usb_del_gadget(struct usb_gadget *gadget)
>>+{
>>+ struct usb_udc *udc = NULL;
>>+ unsigned long flags;
>>+
>>+ dev_vdbg(gadget->dev.parent, "unregistering gadget\n");
>>+
>>+ spin_lock_irqsave(&udc_lock, flags);
>>+ list_for_each_entry(udc, &udc_list, list) {
>>+ if (udc->gadget == gadget)
>>+ break;
>>+ }
>>+
>>+ if (!udc)
>>+ return;
>
>Spinlock remains locked.
Oh.
>
>>+ if (udc->gadget != gadget) {
>>+ dev_err(gadget->dev.parent, "gadget not registered.\n");
>>+ return;
>
>Spinlock remains locked.
Oh.
>>+ }
>
>Also, if you ask me, I'd rewrite the above as:
>
> spin_lock_irqsave(&udc_lock, flags);
> list_for_each_entry(udc, &udc_list, list) {
> if (udc->gadget == gadget)
> goto found;
> }
>
> dev_err(gadget->dev.parent, "gadget not registered.\n");
> spin_unlock_irqrestore(&udc_lock, flags);
> return;
>
>found:
yep. Simpler.
>>+
>>+ list_del(&udc->list);
>
>Do we need to keep the spinlock after this point?
>
>>+
>>+ if (udc->driver) {
>>+ spin_unlock_irqrestore(&udc_lock, flags);
>>+ usb_gadget_unregister_driver(udc->driver);
>
>usb_gadget_unregister_driver() will once again traverse the list
>to look for the UDC and it *will* *fail* since we already have
>removed the UDC from the list. We need to have another function
>which takes udc and unregisteres the driver.
Yes, let me look at this.
>
>>+ spin_lock_irqsave(&udc_lock, flags);
>>+ }
>>+
>>+ device_unregister(&gadget->dev);
>>+ spin_unlock_irqrestore(&udc_lock, flags);
>>+}
>>+EXPORT_SYMBOL_GPL(usb_del_gadget);
>>+
Sebastian
--
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
