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

[Marek Vasut]

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 at samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park at samsung.com>
> Cc: Marek Vasut <marex at 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 at 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 at 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_ */