Hi Felipe, some minor comments. On 06/14/2011 05:09 PM, Balbi, Felipe wrote:
this class will be used to abstract away several of the duplicated operations scattered among the USB gadget controller drivers. Later, we can add an atomic notifier to tell interested drivers about what's happening with the controller. Notifications such as suspend, resume, enumerated, etc. will be useful, at a minimum, for implementing usb charger detection. As part of the converting process usb_gadget_probe_driver() is no longer part of each udc but pushed into the ->stap() callback. The same for his couterpart. The core is currently set explicit to 'n'. It will be changed to 'y' once all users are converted since it provides functions which clash with other drivers. Signed-off-by: Felipe Balbi<balbi@xxxxxx> Signed-off-by: Sebastian Andrzej Siewior<bigeasy@xxxxxxxxxxxxx> Acked-by: Michal Nazarewicz<mina86@xxxxxxxxxx> --- drivers/usb/gadget/udc-core.c | 412 +++++++++++++++++++++++++++++++++++++++++ include/linux/usb/gadget.h | 7 + 2 files changed, 419 insertions(+), 0 deletions(-) create mode 100644 drivers/usb/gadget/udc-core.c diff --git a/drivers/usb/gadget/udc-core.c b/drivers/usb/gadget/udc-core.c new file mode 100644 index 0000000..534f767 --- /dev/null +++ b/drivers/usb/gadget/udc-core.c @@ -0,0 +1,412 @@ +/** + * udc.c - Core UDC Framework + * + * Copyright (C) 2010 Texas Instruments + * Author: Felipe Balbi<balbi@xxxxxx> + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 of + * the License as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see<http://www.gnu.org/licenses/>. + */ + +#include<linux/kernel.h> +#include<linux/module.h> +#include<linux/device.h> +#include<linux/list.h> +#include<linux/err.h> + +#include<linux/usb/ch9.h> +#include<linux/usb/gadget.h> + +/** + * struct usb_udc - describes one usb device controller + * @driver - the gadget driver pointer. For use by the class code + * @dev - the child device to the actual controller + * @gadget - the gadget. For use by the class code + * @list - for use by the udc class driver + * @name - a human friendly name representation
name is missing in the data structure
+ * + * This represents the internal data structure which is used by the UDC-class + * to hold information about udc driver and gadget together. + */ +struct usb_udc { + struct usb_gadget_driver *driver; + struct usb_gadget *gadget; + struct device dev; + struct list_head list; +}; + +static struct class *udc_class; +static struct device_type udc_device_type; +static LIST_HEAD(udc_list); +static DEFINE_MUTEX(udc_lock); + +/* ------------------------------------------------------------------------- */ + +/** + * usb_gadget_start - tells usb device controller to start up + * @gadget: The device we want to get started
Shouldn't it be called "The gadget" rather than "The device". The device would refer to the UDC device. no? Description for other parameters are missing
+ * + * This call is issued by the UDC Class driver when it's about + * to register a gadget driver to the device controller, before + * calling gadget driver's bind() method. + * + * It allows the controller to be powered off until extrictly
did you mean explicitly?
+ * necessary to have it powered on. + * + * Returns zero on success, else negative errno. + */ +static inline int usb_gadget_start(struct usb_gadget *gadget, + struct usb_gadget_driver *drv, int (*bind)(struct usb_gadget *)) +{ + return gadget->ops->start(drv, bind); +} + +/** + * usb_gadget_stop - tells usb device controller we don't need it anymore + * @gadget: The device we want to stop activity
The device -> The gadget. Description for 2nd parameter is missing
+ * + * This call is issued by the UDC Class driver after calling + * gadget driver's unbind() method. + * + * The details are implementation specific, but it can go as + * far as powering off UDC completely and disable its data + * line pullups. + */ +static inline void usb_gadget_stop(struct usb_gadget *gadget, + struct usb_gadget_driver *driver) +{ + gadget->ops->stop(driver); +} + +/** + * usb_udc_release - release the usb_udc struct + * @dev: the dev member within usb_udc + * + * This is called by driver's core in order to free memory once the last + * reference is released. + */ +static void usb_udc_release(struct device *dev) +{ + struct usb_udc *udc; + + udc = container_of(dev, struct usb_udc, dev); + dev_dbg(dev, "releasing '%s'\n", dev_name(dev)); + kfree(udc); +} + +/** + * usb_add_gadget_udc - adds a new gadget to the udc class driver list + * @parent: the parent device to this udc. Usually the controller + * driver's device. + * @gadget: the gadget to be added to the list + * + * Returns zero on success, negative errno otherwise. + */ +int usb_add_gadget_udc(struct device *parent, struct usb_gadget *gadget) +{ + struct usb_udc *udc; + int ret = -ENOMEM; + + udc = kzalloc(sizeof(*udc), GFP_KERNEL); + if (!udc) + goto err1; + + device_initialize(&udc->dev); + udc->dev.release = usb_udc_release; + udc->dev.class = udc_class; + udc->dev.parent = parent; + ret = dev_set_name(&udc->dev, "%s", kobject_name(&parent->kobj)); + if (ret) + goto err2; + + udc->gadget = gadget; + + mutex_lock(&udc_lock); + list_add_tail(&udc->list,&udc_list); + + ret = device_add(&udc->dev); + if (ret) + goto err3; + + kobject_uevent(&udc->dev.kobj, KOBJ_ADD);
Not necessary. device_add() generates the uevent for you.
+ mutex_unlock(&udc_lock); + return 0; +err3: + list_del(&udc->list); + mutex_unlock(&udc_lock); + +err2: + put_device(&udc->dev);
memory leaks here. need to free udc.
+err1: + return ret; +} +EXPORT_SYMBOL_GPL(usb_add_gadget_udc); + +static void usb_gadget_remove_driver(struct usb_udc *udc) +{ + dev_dbg(&udc->dev, "unregistering UDC driver [%s]\n", + udc->gadget->name); + + kobject_uevent(&udc->dev.kobj, KOBJ_CHANGE);
Do you really need this? It will be immediately followed by a KOBJ_REMOVE since you call device_unregister() soon after.
+ + usb_gadget_stop(udc->gadget, udc->driver); + + udc->driver = NULL; + udc->dev.driver = NULL; +} + +/** + * usb_del_gadget_udc - deletes @udc from udc_list + * @udc: the udc to be removed.
should be @gadget.
+ * + * This, will call usb_gadget_unregister_driver() if + * the @udc is still busy. + */ +void usb_del_gadget_udc(struct usb_gadget *gadget) +{ + struct usb_udc *udc = NULL; + + dev_vdbg(gadget->dev.parent, "unregistering gadget\n");
isn't is better if this print is moved to after found: ?
+ + mutex_lock(&udc_lock); + list_for_each_entry(udc,&udc_list, list) + if (udc->gadget == gadget) + goto found; + + dev_err(gadget->dev.parent, "gadget not registered.\n"); + mutex_unlock(&udc_lock); + return; +found: + list_del(&udc->list); + mutex_unlock(&udc_lock); + + if (udc->driver) + usb_gadget_remove_driver(udc); + + kobject_uevent(&udc->dev.kobj, KOBJ_REMOVE);
Not required. device_del() does it for you when you do a device_unregister() below.
+ device_unregister(&udc->dev); +} +EXPORT_SYMBOL_GPL(usb_del_gadget_udc); + +/* ------------------------------------------------------------------------- */ +
missing description?
+int usb_gadget_probe_driver(struct usb_gadget_driver *driver, + int (*bind)(struct usb_gadget *)) +{ + struct usb_udc *udc = NULL; + int ret; + + if (!driver || !bind || !driver->setup) + return -EINVAL; + + mutex_lock(&udc_lock); + list_for_each_entry(udc,&udc_list, list) { + /* For now we take the first one */ + if (!udc->driver) + goto found; + } + + pr_debug("couldn't find an available UDC\n"); + mutex_unlock(&udc_lock); + return -ENODEV; + +found: + dev_dbg(&udc->dev, "registering UDC driver [%s]\n", + driver->function); + + udc->driver = driver; + udc->dev.driver =&driver->driver; + + ret = usb_gadget_start(udc->gadget, driver, bind); + if (ret) + goto err1; + + kobject_uevent(&udc->dev.kobj, KOBJ_CHANGE); + mutex_unlock(&udc_lock); + return 0; + +err1: + dev_err(&udc->dev, "failed to start %s: %d\n", + udc->driver->function, ret); + udc->driver = NULL; + udc->dev.driver = NULL; + mutex_unlock(&udc_lock); + return ret; +} +EXPORT_SYMBOL_GPL(usb_gadget_probe_driver); +
ditto.
+int usb_gadget_unregister_driver(struct usb_gadget_driver *driver) +{ + struct usb_udc *udc = NULL; + int ret = -ENODEV; + + 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); + ret = 0; + break; + } + + mutex_unlock(&udc_lock); + return ret; +} +EXPORT_SYMBOL_GPL(usb_gadget_unregister_driver); +
cheers, -roger -- 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