On Fri, 18 Mar 2011, Matthieu CASTET wrote:
> >> --- a/drivers/Makefile
> >> +++ b/drivers/Makefile
> >> @@ -68,7 +68,7 @@ obj-$(CONFIG_USB_OTG_UTILS) += usb/otg/
> >> obj-$(CONFIG_USB) += usb/
> >> obj-$(CONFIG_USB_MUSB_HDRC) += usb/musb/
> >> obj-$(CONFIG_PCI) += usb/
> >> -obj-$(CONFIG_USB_GADGET) += usb/gadget/
> >> +obj-y += usb/gadget/
> >
> > Why was this changed? And why doesn't the spacing match the lines
> > above?
> That was to make core_udc always build in kernel (not a module).
> But it should be possible to make it a module.
Since as the rest of the gadget code can live in modules, this
probably should have that ability too.
> >> +{
> >> + if (!gadget->udc ||
> >> + !gadget->udc->probe_driver || !gadget->udc->unregister_driver)
> >> + return -EINVAL;
> >> +
> >> + if (usb_gadget_udc)
> >> + return -EBUSY;
> >> +
> >> + usb_gadget_udc = gadget;
> >
> > In theory this should be protected by a mutex. But it probably doesn't
> > matter.
> Are there any arch where this is not atomic ?
Exactly what do you mean? The combination of an "if" statement
followed by an assignment is non-atomic on _every_ architecture.
> I could use a local_irq_save/local_irq_restore if needed.
> A mutex seems overkill.
Pretty much anything seems overkill. Still, given that UDC drivers can
be built as modules, is there anything to prevent two different modules
from being loaded at the same time? Or one driver being loaded at the
same time as another is being unloaded?
> >> +{
> >> + if (!usb_gadget_udc || usb_gadget_udc != gadget)
> >> + return;
> >> +
> >> + usb_gadget_udc = NULL;
> >> +
> >> + device_unregister(&gadget->dev);
> >
> > What happens if a gadget driver is currently registered for this
> > gadget? It will have no way to unregister, because
> > usb_gadget_unregister_driver() will return -ENODEV without doing
> > anything.
> That an interesting problem.
> Before symbol deps on usb_gadget_probe_driver make sure udc driver wasn't
> unloaded before gadget driver.
>
> We can use try_module_get in usb_gadget_probe_driver and module_put in
> usb_gadget_unregister_driver to make sure the udc driver don't call
> usb_gadget_unregister before usb_gadget_unregister_driver.
>
> The struct usb_gadget_udc will look like :
> struct usb_gadget_udc {
> struct module *owner;
> int (*probe_driver) (struct usb_gadget_driver *driver, int (*bind)(struct usb_gadget *), struct usb_gadget *gadget);
> int (*unregister_driver) (struct usb_gadget_driver *driver, struct usb_gadget *gadget);
> };
Yes, that's a good solution.
Alan Stern
--
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
