[U-Boot] [PATCH 1/7] dfu:usb: Support for g_dnl composite download gadget.

Marek Vasut
2012-07-03T18:41:24Z

Dear Lukasz Majewski,

> Composite USB download gadget support (g_dnl) for download functions.
> This code works on top of composite gadget.
>
> Signed-off-by: Lukasz Majewski <l.majewski@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> Cc: Marek Vasut <marex@denx.de>
> ---
> drivers/usb/gadget/Makefile | 1 +
> drivers/usb/gadget/g_dnl.c | 235
> +++++++++++++++++++++++++++++++++++++++++++ include/g_dnl.h |
> 33 ++++++
> 3 files changed, 269 insertions(+), 0 deletions(-)
> create mode 100644 drivers/usb/gadget/g_dnl.c
> create mode 100644 include/g_dnl.h
>
> diff --git a/drivers/usb/gadget/Makefile b/drivers/usb/gadget/Makefile
> index 87d1918..2c067c8 100644
> --- a/drivers/usb/gadget/Makefile
> +++ b/drivers/usb/gadget/Makefile
> @@ -29,6 +29,7 @@ LIB := $(obj)libusb_gadget.o
> ifdef CONFIG_USB_GADGET
> COBJS-y += epautoconf.o config.o usbstring.o
> COBJS-$(CONFIG_USB_GADGET_S3C_UDC_OTG) += s3c_udc_otg.o
> +COBJS-$(CONFIG_USBDOWNLOAD_GADGET) += g_dnl.o
> endif
> ifdef CONFIG_USB_ETHER
> COBJS-y += ether.o epautoconf.o config.o usbstring.o
> diff --git a/drivers/usb/gadget/g_dnl.c b/drivers/usb/gadget/g_dnl.c
> new file mode 100644
> index 0000000..7925d0e
> --- /dev/null
> +++ b/drivers/usb/gadget/g_dnl.c
> @@ -0,0 +1,235 @@
> +/*
> + * g_dnl.c -- USB Downloader Gadget
> + *
> + * Copyright (C) 2012 Samsung Electronics
> + * Lukasz Majewski <l.majewski@samsung.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * 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, write to the Free Software
> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307
> USA + */
> +
> +#include <errno.h>
> +#include <common.h>
> +#include <malloc.h>
> +#include <linux/usb/ch9.h>
> +#include <usbdescriptors.h>
> +#include <linux/usb/gadget.h>
> +
> +#include <mmc.h>
> +#include <part.h>
> +
> +#include <g_dnl.h>
> +#include "f_dfu.h"
> +
> +#include "gadget_chips.h"
> +#include "composite.c"
> +
> +/* Samsung's IDs */
> +#define DRIVER_VENDOR_NUM 0x04E8
> +#define DRIVER_PRODUCT_NUM 0x6601

Is it samsung specific? Or can we use some USB IDs that are not bound to
samsung?

> +#define STRING_MANUFACTURER 25
> +#define STRING_PRODUCT 2
> +#define STRING_USBDOWN 2
> +#define CONFIG_USBDOWNLOADER 2
> +
> +#define DRIVER_VERSION "usb_dnl 2.0"
> +
> +static const char shortname[] = "usb_dnl_";
> +static const char product[] = "SLP USB download gadget";
> +static const char manufacturer[] = "Samsung";

DITTO?

> +
> +static struct usb_device_descriptor device_desc = {
> + .bLength = sizeof device_desc,
> + .bDescriptorType = USB_DT_DEVICE,
> +
> + .bcdUSB = __constant_cpu_to_le16(0x0200),
> + .bDeviceClass = USB_CLASS_COMM,
> + .bDeviceSubClass = 0x02, /*0x02:CDC-modem , 0x00:CDC-serial*/
> +
> + .idVendor = __constant_cpu_to_le16(DRIVER_VENDOR_NUM),
> + .idProduct = __constant_cpu_to_le16(DRIVER_PRODUCT_NUM),
> + .iProduct = STRING_PRODUCT,
> + .bNumConfigurations = 1,
> +};
> +
> +static const struct usb_descriptor_header *otg_desc[] = {
> + (struct usb_descriptor_header *) &(struct usb_otg_descriptor){
> + .bLength = sizeof(struct usb_otg_descriptor),
> + .bDescriptorType = USB_DT_OTG,
> + .bmAttributes = USB_OTG_SRP,
> + },
> + NULL,
> +};
> +
> +/* static strings, in UTF-8 */
> +static struct usb_string odin_string_defs[] = {
> + { 0, manufacturer, },
> + { 1, product, },
> +};
> +
> +static struct usb_gadget_strings odin_string_tab = {
> + .language = 0x0409, /* en-us */
> + .strings = odin_string_defs,
> +};
> +
> +static struct usb_gadget_strings *g_dnl_composite_strings[] = {
> + &odin_string_tab,
> + NULL,
> +};
> +
> +static int g_dnl_unbind(struct usb_composite_dev *cdev)
> +{
> + debug("%s\n", __func__);
> + return 0;
> +}
> +
> +static int g_dnl_do_config(struct usb_configuration *c)
> +{
> + int ret = -1;
> + char *s = (char *) c->cdev->driver->name;
> +
> + debug("%s: c: 0x%p cdev: 0x%p\n", __func__, c, c->cdev);

Can you elaborate in that debug message more? It's not really useful ;-)

> + if (gadget_is_otg(c->cdev->gadget)) {
> + c->descriptors = otg_desc;
> + c->bmAttributes |= USB_CONFIG_ATT_WAKEUP;
> + }
> +
> + printf("GADGET DRIVER: %s\n", s);
> +
> + if (!strcmp(s, "usb_dnl_dfu"))
> + ret = dfu_add(c);

And if this fails, we should set ret= as well, no?

> +
> + return ret;
> +}
> +
> +static int g_dnl_config_register(struct usb_composite_dev *cdev)
> +{
> + debug("%s:\n", __func__);
> + static struct usb_configuration config = {
> + .label = "usb_dnload",
> + .bmAttributes = USB_CONFIG_ATT_ONE | USB_CONFIG_ATT_SELFPOWER,
> + .bConfigurationValue = CONFIG_USBDOWNLOADER,
> + .iConfiguration = STRING_USBDOWN,
> +
> + .bind = g_dnl_do_config,
> + };
> +
> + return usb_add_config(cdev, &config);
> +}
> +
> +static int g_dnl_bind(struct usb_composite_dev *cdev)
> +{
> + int gcnum;
> + int id, ret;
> + struct usb_gadget *gadget = cdev->gadget;
> +
> + debug("%s: gadget: 0x%p cdev: 0x%p\n", __func__, gadget, cdev);
> +
> + id = usb_string_id(cdev);
> +
> + if (id < 0)
> + return id;
> + odin_string_defs[0].id = id;
> + device_desc.iManufacturer = id;
> +
> + id = usb_string_id(cdev);
> + if (id < 0)
> + return id;
> +
> + odin_string_defs[1].id = id;
> + device_desc.iProduct = id;
> +
> + ret = g_dnl_config_register(cdev);
> + if (ret)
> + goto error;
> +
> +

Two newlines

> + gcnum = usb_gadget_controller_number(gadget);
> +
> + debug("gcnum: %d\n", gcnum);
> + if (gcnum >= 0)
> + device_desc.bcdDevice = cpu_to_le16(0x0200 + gcnum);
> + else {
> + debug("%s: controller '%s' not recognized\n",
> + shortname, gadget->name);
> + device_desc.bcdDevice = __constant_cpu_to_le16(0x9999);
> + }
> +
> + return 0;
> +
> + error:
> + g_dnl_unbind(cdev);
> + return -ENOMEM;
> +}
> +
> +static void g_dnl_suspend(struct usb_composite_dev *cdev)
> +{
> + if (cdev->gadget->speed == USB_SPEED_UNKNOWN)
> + return;
> +
> + debug("suspend\n");
> +}
> +
> +static void g_dnl_resume(struct usb_composite_dev *cdev)
> +{
> + debug("resume\n");
> +}

Maybe you should check if suspend/resume are set at all. Then you won't need the
above stubs.

> +static struct usb_composite_driver g_dnl_driver = {
> + .name = NULL,
> + .dev = &device_desc,
> + .strings = g_dnl_composite_strings,
> +
> + .bind = g_dnl_bind,
> + .unbind = g_dnl_unbind,
> +
> + .suspend = g_dnl_suspend,
> + .resume = g_dnl_resume,
> +};
> +
> +int g_dnl_init(char *s)
> +{
> + int ret;
> + static char str[16];
> +

Why use '\0' if you can just use 0 ?

> + memset(str, '\0', sizeof(str));
> +
> + strncpy(str, shortname, sizeof(shortname));
> +
> + if (!strncmp(s, "dfu", sizeof(s))) {
> + strncat(str, s, sizeof(str));
> + } else {
> + printf("%s: unknown command: %s\n", __func__, s);
> + return -EINVAL;
> + }
> +
> + g_dnl_driver.name = str;
> +
> + debug("%s: g_dnl_driver.name: %s\n", __func__, g_dnl_driver.name);
> + ret = usb_composite_register(&g_dnl_driver);
> +
> + if (ret) {
> + printf("%s: failed!, error:%d ", __func__, ret);

Missing newline in printf() (...error:%d ").

> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +void g_dnl_cleanup(void)
> +{
> + usb_composite_unregister(&g_dnl_driver);
> +}
> diff --git a/include/g_dnl.h b/include/g_dnl.h
> new file mode 100644
> index 0000000..5d0e9a5
> --- /dev/null
> +++ b/include/g_dnl.h
> @@ -0,0 +1,33 @@
> +/*
> + * Copyright (C) 2012 Samsung Electronics
> + * Lukasz Majewski <l.majewski@samsung.com>
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation; either version 2 of
> + * the License, or (at your option) any later version.
> + *
> + * 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, write to the Free Software
> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
> + * MA 02111-1307 USA
> + */
> +
> +#ifndef __G_DOWNLOAD_H_
> +#define __G_DOWNLOAD_H_
> +
> +#include <linux/usb/ch9.h>
> +#include <usbdescriptors.h>
> +#include <linux/usb/gadget.h>
> +
> +int g_dnl_init(char *s);
> +void g_dnl_cleanup(void);
> +
> +/* USB initialization declaration - board specific*/
> +void board_usb_init(void);
> +#endif /* __G_DOWNLOAD_H_ */