| Message ID | 1408372115-4570-3-git-send-email-kishon@ti.com |
|---|---|
| State | New |
| Headers | show |
Hi Kishon, > In order to support multiple USB device controllers in uboot, > udc-core is needed. Is it? In u-boot at best only one UDC is operational at a time. > udc-core also helps to cleanly link the USB > peripheral driver with the gadget driver. Hence Ported minimal > udc-core from linux kernel. I'd appreciate the exact SHA for this udc-core.c code. And the SHA should be from some already released mainline code (like final 3.16), not any private branch nor linux-next. > > Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com> > --- > drivers/usb/gadget/Makefile | 1 + > drivers/usb/gadget/udc-core.c | 229 > ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 230 > insertions(+) create mode 100644 drivers/usb/gadget/udc-core.c > > diff --git a/drivers/usb/gadget/Makefile b/drivers/usb/gadget/Makefile > index 66becdc..74875a2 100644 > --- a/drivers/usb/gadget/Makefile > +++ b/drivers/usb/gadget/Makefile > @@ -19,6 +19,7 @@ obj-$(CONFIG_USBDOWNLOAD_GADGET) += g_dnl.o > obj-$(CONFIG_DFU_FUNCTION) += f_dfu.o > obj-$(CONFIG_USB_GADGET_MASS_STORAGE) += f_mass_storage.o > obj-$(CONFIG_CMD_FASTBOOT) += f_fastboot.o > +obj-y += udc-core.o > endif > ifdef CONFIG_USB_ETHER > obj-y += ether.o > diff --git a/drivers/usb/gadget/udc-core.c > b/drivers/usb/gadget/udc-core.c new file mode 100644 > index 0000000..bbe919c > --- /dev/null > +++ b/drivers/usb/gadget/udc-core.c > @@ -0,0 +1,229 @@ > +/** > + * udc-core.c - Core UDC Framework > + * > + * Copyright (C) 2014 Texas Instruments Incorporated - > http://www.ti.com > + * > + * Author: Felipe Balbi <balbi@ti.com> > + * > + * Taken from Linux Kernel v3.16 (drivers/usb/gadget/udc-core.c) and > ported > + * to uboot. > + * > + * SPDX-License-Identifier: GPL-2.0+ It seems like the author is the same, but the GPL license is different: Original file ./drivers/usb/gadget/udc/udc-core.c /** * udc.c - Core UDCFramework * * Copyright (C) 2010 TexasInstruments * Author: Felipe Balbi<balbi@ti.com> * * 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. If Felipe don't mind then he should give you the permission to add + to the GPL2. > + */ > + > +#include <linux/compat.h> > +#include <malloc.h> > +#include <asm/cache.h> > +#include <common.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 > + * > + * 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 list_head list; > +}; > + > +static LIST_HEAD(udc_list); > + > +void usb_gadget_set_state(struct usb_gadget *gadget, > + enum usb_device_state state) > +{ > + gadget->state = state; > +} > + > +/** > + * usb_gadget_udc_start - tells usb device controller to start up > + * @gadget: The gadget we want to get started > + * @driver: The driver we want to bind to @gadget > + * > + * 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 strictly > + * necessary to have it powered on. > + * > + * Returns zero on success, else negative errno. > + */ > +static inline int usb_gadget_udc_start(struct usb_gadget *gadget, > + struct usb_gadget_driver *driver) > +{ > + return gadget->ops->udc_start(gadget, driver); > +} > + > +/** > + * usb_gadget_udc_stop - tells usb device controller we don't need > it anymore > + * @gadget: The device we want to stop activity > + * @driver: The driver to unbind from @gadget > + * > + * 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_udc_stop(struct usb_gadget *gadget, > + struct usb_gadget_driver *driver) > +{ > + gadget->ops->udc_stop(gadget, driver); > +} > + > +/** > + * usb_add_gadget_udc_release - 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. > + * @release: a gadget release function. > + * > + * Returns zero on success, negative errno otherwise. > + */ > +int usb_add_gadget_udc_release(struct usb_gadget *gadget) > +{ > + struct usb_udc *udc; > + int ret = -ENOMEM; > + > + udc = kzalloc(sizeof(*udc), GFP_KERNEL); > + if (!udc) > + return ret; > + > + udc->gadget = gadget; > + list_add_tail(&udc->list, &udc_list); > + usb_gadget_set_state(gadget, USB_STATE_NOTATTACHED); > + return 0; > +} > + > +/** > + * 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 usb_gadget *gadget) > +{ > + return usb_add_gadget_udc_release(gadget); > +} > + > +static void usb_gadget_remove_driver(struct usb_udc *udc) > +{ > + dev_dbg(&udc->dev, "unregistering UDC driver [%s]\n", > + udc->gadget->name); > + usb_gadget_disconnect(udc->gadget); > + udc->driver->disconnect(udc->gadget); > + udc->driver->unbind(udc->gadget); > + usb_gadget_udc_stop(udc->gadget, NULL); > + > + udc->driver = NULL; > +} > + > +/** > + * usb_del_gadget_udc - deletes @udc from udc_list > + * @gadget: the gadget to be removed. > + * > + * 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; > + list_for_each_entry(udc, &udc_list, list) > + if (udc->gadget == gadget) > + goto found; > + > + dev_err(gadget->dev.parent, "gadget not registered.\n"); > + return; > + > +found: > + dev_vdbg(gadget->dev.parent, "unregistering gadget\n"); > + > + list_del(&udc->list); > + > + if (udc->driver) > + usb_gadget_remove_driver(udc); > + > + kfree(udc); > +} > + > +static int udc_bind_to_driver(struct usb_udc *udc, struct > usb_gadget_driver *driver) +{ > + int ret; > + > + dev_dbg(&udc->dev, "registering UDC driver [%s]\n", > + driver->function); > + > + udc->driver = driver; > + > + ret = driver->bind(udc->gadget); > + if (ret) > + goto err1; > + > + ret = usb_gadget_udc_start(udc->gadget, driver); > + if (ret) { > + driver->unbind(udc->gadget); > + goto err1; > + } > + usb_gadget_connect(udc->gadget); > + > + return 0; > +err1: > + if (ret != -EISNAM) > + dev_err(&udc->dev, "failed to start %s: %d\n", > + udc->driver->function, ret); > + udc->driver = NULL; > + return ret; > +} > + > +int usb_gadget_register_driver(struct usb_gadget_driver *driver) > +{ > + struct usb_udc *udc = NULL; > + int ret; > + > + if (!driver || !driver->bind || !driver->setup) > + return -EINVAL; > + > + list_for_each_entry(udc, &udc_list, list) { > + /* For now we take the first one */ > + if (!udc->driver) > + goto found; > + } > + > + debug("couldn't find an available UDC\n"); > + return -ENODEV; > +found: > + ret = udc_bind_to_driver(udc, driver); > + return ret; > +} > + > +int usb_gadget_unregister_driver(struct usb_gadget_driver *driver) > +{ > + struct usb_udc *udc = NULL; > + int ret = -ENODEV; > + > + if (!driver || !driver->unbind) > + return -EINVAL; > + > + 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; > + } > + > + return ret; > +}
On Monday 18 August 2014 08:06 PM, Felipe Balbi wrote: > On Mon, Aug 18, 2014 at 07:58:24PM +0530, Kishon Vijay Abraham I wrote: >> In order to support multiple USB device controllers in uboot, >> udc-core is needed. udc-core also helps to cleanly link the USB peripheral >> driver with the gadget driver. Hence Ported minimal udc-core from >> linux kernel. >> >> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com> >> --- >> drivers/usb/gadget/Makefile | 1 + >> drivers/usb/gadget/udc-core.c | 229 ++++++++++++++++++++++++++++++++++++++++++ >> 2 files changed, 230 insertions(+) >> create mode 100644 drivers/usb/gadget/udc-core.c >> >> diff --git a/drivers/usb/gadget/Makefile b/drivers/usb/gadget/Makefile >> index 66becdc..74875a2 100644 >> --- a/drivers/usb/gadget/Makefile >> +++ b/drivers/usb/gadget/Makefile >> @@ -19,6 +19,7 @@ obj-$(CONFIG_USBDOWNLOAD_GADGET) += g_dnl.o >> obj-$(CONFIG_DFU_FUNCTION) += f_dfu.o >> obj-$(CONFIG_USB_GADGET_MASS_STORAGE) += f_mass_storage.o >> obj-$(CONFIG_CMD_FASTBOOT) += f_fastboot.o >> +obj-y += udc-core.o >> endif >> ifdef CONFIG_USB_ETHER >> obj-y += ether.o >> diff --git a/drivers/usb/gadget/udc-core.c b/drivers/usb/gadget/udc-core.c >> new file mode 100644 >> index 0000000..bbe919c >> --- /dev/null >> +++ b/drivers/usb/gadget/udc-core.c >> @@ -0,0 +1,229 @@ >> +/** >> + * udc-core.c - Core UDC Framework >> + * >> + * Copyright (C) 2014 Texas Instruments Incorporated - http://www.ti.com >> + * >> + * Author: Felipe Balbi <balbi@ti.com> >> + * >> + * Taken from Linux Kernel v3.16 (drivers/usb/gadget/udc-core.c) and ported >> + * to uboot. >> + * >> + * SPDX-License-Identifier: GPL-2.0+ > > why are you relicensing this file ? In kernel this is gpl v2 only. ah.. copy paste error. Will fix it. Thanks Kishon >
On Tue, Aug 19, 2014 at 10:52:29AM +0200, Lukasz Majewski wrote: > > diff --git a/drivers/usb/gadget/udc-core.c > > b/drivers/usb/gadget/udc-core.c new file mode 100644 > > index 0000000..bbe919c > > --- /dev/null > > +++ b/drivers/usb/gadget/udc-core.c > > @@ -0,0 +1,229 @@ > > +/** > > + * udc-core.c - Core UDC Framework > > + * > > + * Copyright (C) 2014 Texas Instruments Incorporated - > > http://www.ti.com > > + * > > + * Author: Felipe Balbi <balbi@ti.com> > > + * > > + * Taken from Linux Kernel v3.16 (drivers/usb/gadget/udc-core.c) and > > ported > > + * to uboot. > > + * > > + * SPDX-License-Identifier: GPL-2.0+ > > It seems like the author is the same, but the GPL license is different: > > Original file ./drivers/usb/gadget/udc/udc-core.c > > > /** > * udc.c - Core UDCFramework > * > * Copyright (C) 2010 TexasInstruments > * Author: Felipe Balbi<balbi@ti.com> > * > * 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. > > If Felipe don't mind then he should give you the permission to add + to > the GPL2. I mind :-)
Hi Lukasz, On Tuesday 19 August 2014 02:22 PM, Lukasz Majewski wrote: > Hi Kishon, > >> In order to support multiple USB device controllers in uboot, >> udc-core is needed. > > Is it? In u-boot at best only one UDC is operational at a time. I didn't mean operational at the same time. dra7xx has 4 USB controllers and we should allow the user to use any USB port for DFU. The current DFU code re-initializes the controller every time, but that can be optimized. > >> udc-core also helps to cleanly link the USB >> peripheral driver with the gadget driver. Hence Ported minimal >> udc-core from linux kernel. > > I'd appreciate the exact SHA for this udc-core.c code. And the SHA > should be from some already released mainline code (like final 3.16), > not any private branch nor linux-next. We can't have the exact udc-core.c from linux kernel as it deals with kernel driver model stuff and vfs which is not needed for kernel. Trying to have the exact same code from kernel complicates it. Thanks Kishon > >> >> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com> >> --- >> drivers/usb/gadget/Makefile | 1 + >> drivers/usb/gadget/udc-core.c | 229 >> ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 230 >> insertions(+) create mode 100644 drivers/usb/gadget/udc-core.c >> >> diff --git a/drivers/usb/gadget/Makefile b/drivers/usb/gadget/Makefile >> index 66becdc..74875a2 100644 >> --- a/drivers/usb/gadget/Makefile >> +++ b/drivers/usb/gadget/Makefile >> @@ -19,6 +19,7 @@ obj-$(CONFIG_USBDOWNLOAD_GADGET) += g_dnl.o >> obj-$(CONFIG_DFU_FUNCTION) += f_dfu.o >> obj-$(CONFIG_USB_GADGET_MASS_STORAGE) += f_mass_storage.o >> obj-$(CONFIG_CMD_FASTBOOT) += f_fastboot.o >> +obj-y += udc-core.o >> endif >> ifdef CONFIG_USB_ETHER >> obj-y += ether.o >> diff --git a/drivers/usb/gadget/udc-core.c >> b/drivers/usb/gadget/udc-core.c new file mode 100644 >> index 0000000..bbe919c >> --- /dev/null >> +++ b/drivers/usb/gadget/udc-core.c >> @@ -0,0 +1,229 @@ >> +/** >> + * udc-core.c - Core UDC Framework >> + * >> + * Copyright (C) 2014 Texas Instruments Incorporated - >> http://www.ti.com >> + * >> + * Author: Felipe Balbi <balbi@ti.com> >> + * >> + * Taken from Linux Kernel v3.16 (drivers/usb/gadget/udc-core.c) and >> ported >> + * to uboot. >> + * >> + * SPDX-License-Identifier: GPL-2.0+ > > It seems like the author is the same, but the GPL license is different: > > Original file ./drivers/usb/gadget/udc/udc-core.c > > > /** > * udc.c - Core UDCFramework > * > * Copyright (C) 2010 TexasInstruments > * Author: Felipe Balbi<balbi@ti.com> > * > * 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. > > If Felipe don't mind then he should give you the permission to add + to > the GPL2. > >> + */ >> + >> +#include <linux/compat.h> >> +#include <malloc.h> >> +#include <asm/cache.h> >> +#include <common.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 >> + * >> + * 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 list_head list; >> +}; >> + >> +static LIST_HEAD(udc_list); >> + >> +void usb_gadget_set_state(struct usb_gadget *gadget, >> + enum usb_device_state state) >> +{ >> + gadget->state = state; >> +} >> + >> +/** >> + * usb_gadget_udc_start - tells usb device controller to start up >> + * @gadget: The gadget we want to get started >> + * @driver: The driver we want to bind to @gadget >> + * >> + * 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 strictly >> + * necessary to have it powered on. >> + * >> + * Returns zero on success, else negative errno. >> + */ >> +static inline int usb_gadget_udc_start(struct usb_gadget *gadget, >> + struct usb_gadget_driver *driver) >> +{ >> + return gadget->ops->udc_start(gadget, driver); >> +} >> + >> +/** >> + * usb_gadget_udc_stop - tells usb device controller we don't need >> it anymore >> + * @gadget: The device we want to stop activity >> + * @driver: The driver to unbind from @gadget >> + * >> + * 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_udc_stop(struct usb_gadget *gadget, >> + struct usb_gadget_driver *driver) >> +{ >> + gadget->ops->udc_stop(gadget, driver); >> +} >> + >> +/** >> + * usb_add_gadget_udc_release - 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. >> + * @release: a gadget release function. >> + * >> + * Returns zero on success, negative errno otherwise. >> + */ >> +int usb_add_gadget_udc_release(struct usb_gadget *gadget) >> +{ >> + struct usb_udc *udc; >> + int ret = -ENOMEM; >> + >> + udc = kzalloc(sizeof(*udc), GFP_KERNEL); >> + if (!udc) >> + return ret; >> + >> + udc->gadget = gadget; >> + list_add_tail(&udc->list, &udc_list); >> + usb_gadget_set_state(gadget, USB_STATE_NOTATTACHED); >> + return 0; >> +} >> + >> +/** >> + * 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 usb_gadget *gadget) >> +{ >> + return usb_add_gadget_udc_release(gadget); >> +} >> + >> +static void usb_gadget_remove_driver(struct usb_udc *udc) >> +{ >> + dev_dbg(&udc->dev, "unregistering UDC driver [%s]\n", >> + udc->gadget->name); >> + usb_gadget_disconnect(udc->gadget); >> + udc->driver->disconnect(udc->gadget); >> + udc->driver->unbind(udc->gadget); >> + usb_gadget_udc_stop(udc->gadget, NULL); >> + >> + udc->driver = NULL; >> +} >> + >> +/** >> + * usb_del_gadget_udc - deletes @udc from udc_list >> + * @gadget: the gadget to be removed. >> + * >> + * 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; >> + list_for_each_entry(udc, &udc_list, list) >> + if (udc->gadget == gadget) >> + goto found; >> + >> + dev_err(gadget->dev.parent, "gadget not registered.\n"); >> + return; >> + >> +found: >> + dev_vdbg(gadget->dev.parent, "unregistering gadget\n"); >> + >> + list_del(&udc->list); >> + >> + if (udc->driver) >> + usb_gadget_remove_driver(udc); >> + >> + kfree(udc); >> +} >> + >> +static int udc_bind_to_driver(struct usb_udc *udc, struct >> usb_gadget_driver *driver) +{ >> + int ret; >> + >> + dev_dbg(&udc->dev, "registering UDC driver [%s]\n", >> + driver->function); >> + >> + udc->driver = driver; >> + >> + ret = driver->bind(udc->gadget); >> + if (ret) >> + goto err1; >> + >> + ret = usb_gadget_udc_start(udc->gadget, driver); >> + if (ret) { >> + driver->unbind(udc->gadget); >> + goto err1; >> + } >> + usb_gadget_connect(udc->gadget); >> + >> + return 0; >> +err1: >> + if (ret != -EISNAM) >> + dev_err(&udc->dev, "failed to start %s: %d\n", >> + udc->driver->function, ret); >> + udc->driver = NULL; >> + return ret; >> +} >> + >> +int usb_gadget_register_driver(struct usb_gadget_driver *driver) >> +{ >> + struct usb_udc *udc = NULL; >> + int ret; >> + >> + if (!driver || !driver->bind || !driver->setup) >> + return -EINVAL; >> + >> + list_for_each_entry(udc, &udc_list, list) { >> + /* For now we take the first one */ >> + if (!udc->driver) >> + goto found; >> + } >> + >> + debug("couldn't find an available UDC\n"); >> + return -ENODEV; >> +found: >> + ret = udc_bind_to_driver(udc, driver); >> + return ret; >> +} >> + >> +int usb_gadget_unregister_driver(struct usb_gadget_driver *driver) >> +{ >> + struct usb_udc *udc = NULL; >> + int ret = -ENODEV; >> + >> + if (!driver || !driver->unbind) >> + return -EINVAL; >> + >> + 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; >> + } >> + >> + return ret; >> +} > > >
On Tue, Aug 19, 2014 at 08:48:36PM +0530, Kishon Vijay Abraham I wrote: > Hi Lukasz, > > On Tuesday 19 August 2014 02:22 PM, Lukasz Majewski wrote: > > Hi Kishon, > > > >> In order to support multiple USB device controllers in uboot, > >> udc-core is needed. > > > > Is it? In u-boot at best only one UDC is operational at a time. > > I didn't mean operational at the same time. dra7xx has 4 USB > controllers and we should allow the user to use any USB port for DFU. but from u-boot's perspective, that can (should?) be a build-time choice. That user, certainly, won't need more than one port active for booting, right ? > >> udc-core also helps to cleanly link the USB > >> peripheral driver with the gadget driver. Hence Ported minimal > >> udc-core from linux kernel. > > > > I'd appreciate the exact SHA for this udc-core.c code. And the SHA > > should be from some already released mainline code (like final 3.16), > > not any private branch nor linux-next. > > We can't have the exact udc-core.c from linux kernel as it deals with > kernel driver model stuff and vfs which is not needed for kernel. > Trying to have the exact same code from kernel complicates it. what he means is to point to the SHA1 when you originally forked the code from kernel. Sure udc-core can't be the same, but you based off of a known commit ID frmo the kernel.
Hi Felipe, Kishon > On Tue, Aug 19, 2014 at 08:48:36PM +0530, Kishon Vijay Abraham I > wrote: > > Hi Lukasz, > > > > On Tuesday 19 August 2014 02:22 PM, Lukasz Majewski wrote: > > > Hi Kishon, > > > > > >> In order to support multiple USB device controllers in uboot, > > >> udc-core is needed. > > > > > > Is it? In u-boot at best only one UDC is operational at a time. > > > > I didn't mean operational at the same time. dra7xx has 4 USB > > controllers and we should allow the user to use any USB port for > > DFU. > > but from u-boot's perspective, that can (should?) be a build-time > choice. That user, certainly, won't need more than one port active for > booting, right ? This is my point. We are working with the bootloader. Some things here are set at board configuration and aren't changed afterwards (like only UDC 0 output is wired out). The dfu/ums commands give you the ability to specify controller if you need such option. > > > >> udc-core also helps to cleanly link the USB > > >> peripheral driver with the gadget driver. Hence Ported minimal > > >> udc-core from linux kernel. > > > > > > I'd appreciate the exact SHA for this udc-core.c code. And the SHA > > > should be from some already released mainline code (like final > > > 3.16), not any private branch nor linux-next. > > > > We can't have the exact udc-core.c from linux kernel as it deals > > with kernel driver model stuff and vfs which is not needed for > > kernel. Trying to have the exact same code from kernel complicates > > it. > > what he means is to point to the SHA1 when you originally forked the > code from kernel. Sure udc-core can't be the same, but you based off > of a known commit ID frmo the kernel. > +1 The best way is to grab original source from mailine kernel, and then provide patch which adjust it. In this way we have clean trace from where the code comes.
On Tuesday 19 August 2014 08:58 PM, Felipe Balbi wrote: > On Tue, Aug 19, 2014 at 08:48:36PM +0530, Kishon Vijay Abraham I wrote: >> Hi Lukasz, >> >> On Tuesday 19 August 2014 02:22 PM, Lukasz Majewski wrote: >>> Hi Kishon, >>> >>>> In order to support multiple USB device controllers in uboot, >>>> udc-core is needed. >>> >>> Is it? In u-boot at best only one UDC is operational at a time. >> >> I didn't mean operational at the same time. dra7xx has 4 USB >> controllers and we should allow the user to use any USB port for DFU. > > but from u-boot's perspective, that can (should?) be a build-time > choice. That user, certainly, won't need more than one port active for > booting, right ? hmm.. right now I don't think any one needs more than one port. But then without udc core, we have to maintain global variable in gadget.c. Maybe slightly cleaner way is to use udc.. > >>>> udc-core also helps to cleanly link the USB >>>> peripheral driver with the gadget driver. Hence Ported minimal >>>> udc-core from linux kernel. >>> >>> I'd appreciate the exact SHA for this udc-core.c code. And the SHA >>> should be from some already released mainline code (like final 3.16), >>> not any private branch nor linux-next. >> >> We can't have the exact udc-core.c from linux kernel as it deals with >> kernel driver model stuff and vfs which is not needed for kernel. >> Trying to have the exact same code from kernel complicates it. > > what he means is to point to the SHA1 when you originally forked the > code from kernel. Sure udc-core can't be the same, but you based off of > a known commit ID frmo the kernel. Ah ok. The header of the file gives the gives the kernel tag it is forked from. Maybe add that in commit log to? -Kishon >
Hi Kishon, > > > On Tuesday 19 August 2014 08:58 PM, Felipe Balbi wrote: > > On Tue, Aug 19, 2014 at 08:48:36PM +0530, Kishon Vijay Abraham I > > wrote: > >> Hi Lukasz, > >> > >> On Tuesday 19 August 2014 02:22 PM, Lukasz Majewski wrote: > >>> Hi Kishon, > >>> > >>>> In order to support multiple USB device controllers in uboot, > >>>> udc-core is needed. > >>> > >>> Is it? In u-boot at best only one UDC is operational at a time. > >> > >> I didn't mean operational at the same time. dra7xx has 4 USB > >> controllers and we should allow the user to use any USB port for > >> DFU. > > > > but from u-boot's perspective, that can (should?) be a build-time > > choice. That user, certainly, won't need more than one port active > > for booting, right ? > > hmm.. right now I don't think any one needs more than one port. Please look into the dfu/ums commands interface. There is a possibility to set the controller for transmission. > But then without udc core, we have to maintain global variable in > gadget.c. Maybe slightly cleaner way is to use udc.. I don't mind taking code from linux kernel, but I'm concerned about the complexity and accepting the dead code. The composite.c was taken from 2.6.36 on purpose - it was the smallest working code in the linux kernel. > > > >>>> udc-core also helps to cleanly link the USB > >>>> peripheral driver with the gadget driver. Hence Ported minimal > >>>> udc-core from linux kernel. > >>> > >>> I'd appreciate the exact SHA for this udc-core.c code. And the SHA > >>> should be from some already released mainline code (like final > >>> 3.16), not any private branch nor linux-next. > >> > >> We can't have the exact udc-core.c from linux kernel as it deals > >> with kernel driver model stuff and vfs which is not needed for > >> kernel. Trying to have the exact same code from kernel complicates > >> it. > > > > what he means is to point to the SHA1 when you originally forked the > > code from kernel. Sure udc-core can't be the same, but you based > > off of a known commit ID frmo the kernel. > > Ah ok. The header of the file gives the gives the kernel tag it is > forked from. Maybe add that in commit log to? Please for example refer to: SHA1: dee1d99973 and b4d36f6 > > -Kishon > > >
diff --git a/drivers/usb/gadget/Makefile b/drivers/usb/gadget/Makefile index 66becdc..74875a2 100644 --- a/drivers/usb/gadget/Makefile +++ b/drivers/usb/gadget/Makefile @@ -19,6 +19,7 @@ obj-$(CONFIG_USBDOWNLOAD_GADGET) += g_dnl.o obj-$(CONFIG_DFU_FUNCTION) += f_dfu.o obj-$(CONFIG_USB_GADGET_MASS_STORAGE) += f_mass_storage.o obj-$(CONFIG_CMD_FASTBOOT) += f_fastboot.o +obj-y += udc-core.o endif ifdef CONFIG_USB_ETHER obj-y += ether.o diff --git a/drivers/usb/gadget/udc-core.c b/drivers/usb/gadget/udc-core.c new file mode 100644 index 0000000..bbe919c --- /dev/null +++ b/drivers/usb/gadget/udc-core.c @@ -0,0 +1,229 @@ +/** + * udc-core.c - Core UDC Framework + * + * Copyright (C) 2014 Texas Instruments Incorporated - http://www.ti.com + * + * Author: Felipe Balbi <balbi@ti.com> + * + * Taken from Linux Kernel v3.16 (drivers/usb/gadget/udc-core.c) and ported + * to uboot. + * + * SPDX-License-Identifier: GPL-2.0+ + */ + +#include <linux/compat.h> +#include <malloc.h> +#include <asm/cache.h> +#include <common.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 + * + * 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 list_head list; +}; + +static LIST_HEAD(udc_list); + +void usb_gadget_set_state(struct usb_gadget *gadget, + enum usb_device_state state) +{ + gadget->state = state; +} + +/** + * usb_gadget_udc_start - tells usb device controller to start up + * @gadget: The gadget we want to get started + * @driver: The driver we want to bind to @gadget + * + * 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 strictly + * necessary to have it powered on. + * + * Returns zero on success, else negative errno. + */ +static inline int usb_gadget_udc_start(struct usb_gadget *gadget, + struct usb_gadget_driver *driver) +{ + return gadget->ops->udc_start(gadget, driver); +} + +/** + * usb_gadget_udc_stop - tells usb device controller we don't need it anymore + * @gadget: The device we want to stop activity + * @driver: The driver to unbind from @gadget + * + * 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_udc_stop(struct usb_gadget *gadget, + struct usb_gadget_driver *driver) +{ + gadget->ops->udc_stop(gadget, driver); +} + +/** + * usb_add_gadget_udc_release - 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. + * @release: a gadget release function. + * + * Returns zero on success, negative errno otherwise. + */ +int usb_add_gadget_udc_release(struct usb_gadget *gadget) +{ + struct usb_udc *udc; + int ret = -ENOMEM; + + udc = kzalloc(sizeof(*udc), GFP_KERNEL); + if (!udc) + return ret; + + udc->gadget = gadget; + list_add_tail(&udc->list, &udc_list); + usb_gadget_set_state(gadget, USB_STATE_NOTATTACHED); + return 0; +} + +/** + * 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 usb_gadget *gadget) +{ + return usb_add_gadget_udc_release(gadget); +} + +static void usb_gadget_remove_driver(struct usb_udc *udc) +{ + dev_dbg(&udc->dev, "unregistering UDC driver [%s]\n", + udc->gadget->name); + usb_gadget_disconnect(udc->gadget); + udc->driver->disconnect(udc->gadget); + udc->driver->unbind(udc->gadget); + usb_gadget_udc_stop(udc->gadget, NULL); + + udc->driver = NULL; +} + +/** + * usb_del_gadget_udc - deletes @udc from udc_list + * @gadget: the gadget to be removed. + * + * 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; + list_for_each_entry(udc, &udc_list, list) + if (udc->gadget == gadget) + goto found; + + dev_err(gadget->dev.parent, "gadget not registered.\n"); + return; + +found: + dev_vdbg(gadget->dev.parent, "unregistering gadget\n"); + + list_del(&udc->list); + + if (udc->driver) + usb_gadget_remove_driver(udc); + + kfree(udc); +} + +static int udc_bind_to_driver(struct usb_udc *udc, struct usb_gadget_driver *driver) +{ + int ret; + + dev_dbg(&udc->dev, "registering UDC driver [%s]\n", + driver->function); + + udc->driver = driver; + + ret = driver->bind(udc->gadget); + if (ret) + goto err1; + + ret = usb_gadget_udc_start(udc->gadget, driver); + if (ret) { + driver->unbind(udc->gadget); + goto err1; + } + usb_gadget_connect(udc->gadget); + + return 0; +err1: + if (ret != -EISNAM) + dev_err(&udc->dev, "failed to start %s: %d\n", + udc->driver->function, ret); + udc->driver = NULL; + return ret; +} + +int usb_gadget_register_driver(struct usb_gadget_driver *driver) +{ + struct usb_udc *udc = NULL; + int ret; + + if (!driver || !driver->bind || !driver->setup) + return -EINVAL; + + list_for_each_entry(udc, &udc_list, list) { + /* For now we take the first one */ + if (!udc->driver) + goto found; + } + + debug("couldn't find an available UDC\n"); + return -ENODEV; +found: + ret = udc_bind_to_driver(udc, driver); + return ret; +} + +int usb_gadget_unregister_driver(struct usb_gadget_driver *driver) +{ + struct usb_udc *udc = NULL; + int ret = -ENODEV; + + if (!driver || !driver->unbind) + return -EINVAL; + + 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; + } + + return ret; +}
In order to support multiple USB device controllers in uboot, udc-core is needed. udc-core also helps to cleanly link the USB peripheral driver with the gadget driver. Hence Ported minimal udc-core from linux kernel. Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com> --- drivers/usb/gadget/Makefile | 1 + drivers/usb/gadget/udc-core.c | 229 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 230 insertions(+) create mode 100644 drivers/usb/gadget/udc-core.c