On Wed, Sep 22, 2010 at 04:24:59AM -0500, MichaÅ Nazarewicz wrote:
On Tue, 21 Sep 2010 11:34:13 +0200, Felipe Balbi <balbi@xxxxxx> wrote:
diff --git a/drivers/usb/gadget/udc-core.c b/drivers/usb/gadget/udc-core.c
+static inline int usb_gadget_start(struct usb_gadget *gadget)
+{
+ if (!gadget->ops->start)
+ return -EOPNOTSUPP;
So EOPNOTSUPP after all? Omission or have you decided to leave it that way
after all?
I have decided to keep it since it's a required method to supply. Or I
can remove the check and let it oops if it's NULL.
+int usb_add_udc(struct device *parent, struct usb_udc *udc)
+{
+ struct device *dev;
+ unsigned long flags;
+ int ret;
+
+ if (!udc->gadget || !udc->gadget->ops ||
+ !udc->gadget->ops->start) {
+ dev_dbg(parent, "unitializaed gadget\n");
+ return -EINVAL;
+ }
+
+ dev_vdbg(parent, "registering UDC\n");
+
+ dev = kzalloc(sizeof(*dev), GFP_KERNEL);
+ if (!dev) {
+ ret = -ENOMEM;
+ goto err1;
+ }
+
+ device_initialize(dev);
+
+ dev->class = udc_class;
+ dev->type = &udc_device_type;
+ dev->parent = parent;
+ dev->release = usb_udc_release;
+ dev_set_drvdata(dev, udc);
+ udc->dev = dev;
+ udc->gadget->dev.parent = udc->dev;
+ udc->gadget->dev.release = usb_udc_nop_release;
+ udc->gadget->dev.dma_mask = parent->dma_mask;
+
+ ret = dev_set_name(dev, "%s", udc->name);
+ if (ret)
+ goto err2;
+
+ ret = dev_set_name(&udc->gadget->dev, udc->name);
Still with no "%s"?
I didn't quite get what you meant here and forgot to reply on previous
mail, you mean:
dev_set_name(&udc->gadget->dev, "%s", udc->name) ??
+void usb_del_udc(struct usb_udc *udc)
+{
+ unsigned long flags;
+
+ dev_vdbg(udc->dev->parent, "unregistering UDC\n");
+
+ spin_lock_irqsave(&udc_lock, flags);
+ list_del(&udc->list);
+ spin_unlock_irqrestore(&udc_lock, flags);
+
+ if (udc->busy)
+ usb_gadget_unregister_driver(udc->driver);
usb_gadget_unregister_driver() won't find the UDC -- it has been removed
from the list after all.
Also, the more I think about it, the more I'm convinced that this needs
some synchronisation. If usb_del_udc() is called at the same time as
usb_gadget_unregister_driver() funky stuff can happen. Funky stuff will
also happen if usb_del_udc() is called at the same time as
usb_gadget_register_driver().
I think, spin lock should be replaced with a mutex and the all of the
operations need to be performed when holding the mutex. Alternatively,
mutex can be used in addition to spin lock and then usb_add_udc() could
work without locking the mutex. Or, per-UDC mutex needs to be created
and registering, unregistering and deleting would be performed while
holding this mutex.
I'll revisit this part carefully, makes sense your concern.
--
balbi
--
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
