[U-Boot] [PATCH] dfu: initial implementation

Stefan Schmidt stefan at datenfreihafen.org
Wed Nov 2 21:07:17 CET 2011


Hello.

@Remy: One question I have for you is if the DFU implementation should
be based on the re-written gadget layer from samsung or based on the
current one?

First, and only high level, review for the DFU part.

Against which u-boot tree/branch/version is this patch? I tried to
apply it against HEAD and it failed for me. Anything I miss?

On Wed, 2011-11-02 at 11:00, Andrzej Pietrasiewicz wrote:
> 
> The implementation is split into two parts: the dfu gadget implementation
> and a "flashing backend", the interface between the two being the
> struct flash_entity. The "flashing backend"'s implementation is
> board-specific and is implemented on the Goni target.

I replied to Mike's mail with my ideas on this. Split between dfu and
flashing backends is good and missng in my approach currently. I would
not put it into the board file though but make it generic for other
boards as well and only define the needed informations in the board
file. Please see my other mail for details. Comments appreciated!

> diff --git a/common/Makefile b/common/Makefile
> index ae795e0..de926d9 100644
> --- a/common/Makefile
> +++ b/common/Makefile
> @@ -162,6 +162,7 @@ COBJS-y += cmd_usb.o
>  COBJS-y += usb.o
>  COBJS-$(CONFIG_USB_STORAGE) += usb_storage.o
>  endif
> +COBJS-$(CONFIG_CMD_DEVICE_FIRMWARE_UPGRADE) += cmd_dfu.o

CONFIG_CMD_DFU instead? Looks a bit long to me.

>  COBJS-$(CONFIG_CMD_XIMG) += cmd_ximg.o
>  COBJS-$(CONFIG_YAFFS2) += cmd_yaffs2.o
>  
> diff --git a/common/cmd_dfu.c b/common/cmd_dfu.c
V> new file mode 100644
> index 0000000..c85fbb1
> --- /dev/null
> +++ b/common/cmd_dfu.c
> @@ -0,0 +1,50 @@
> +/*
> + * Copyright (C) 2011 Samsung Electrnoics
> + * author: Andrzej Pietrasiewicz <andrzej.p 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 <common.h>
> +#include <command.h>
> +#include <dfu.h>
> +#include <flash_entity.h>
> +
> +static int do_dfu(cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
> +{
> +	board_dfu_init();
> +	dfu_init();
> +	while (1) {
> +		int irq_res;
> +		/* Handle control-c and timeouts */
> +		if (ctrlc()) {
> +			printf("The remote end did not respond in time.\n");
> +			goto fail;
> +		}
> +
> +		irq_res = usb_gadget_handle_interrupts();
> +	}
> +fail:
> +	dfu_cleanup();
> +	board_dfu_cleanup();
> +	return -1;
> +}
> +
> +U_BOOT_CMD(dfu, CONFIG_SYS_MAXARGS, 1, do_dfu,
> +	"Use the DFU [Device Firmware Upgrade]",
> +	"dfu - device firmware upgrade"
> +);
> +

While I think a dfu command is usefull I don't like the need to
execute it before any DFU interaction can happen. That may be an
option during development but for field upgrades or receovery it is
not.

I already wrote a bit about my approach here. At OpenMoko we used a
button to enter u-boot during startup when pressed. This offered a
usbtty interface as usb gadget as well as the runtime descripto for
DFU. With dfu-util it was possible to iniate the DFU download or
upload procedure while being in the mode. Another option would be to
directly jump into DFU mode when a button is pressed.

> diff --git a/drivers/usb/gadget/dfu.c b/drivers/usb/gadget/dfu.c
> new file mode 100644
> index 0000000..535e194
> --- /dev/null
> +++ b/drivers/usb/gadget/dfu.c
> @@ -0,0 +1,920 @@
> +/*
> + * dfu.c -- Device Firmware Update gadget
> + *
> + * Copyright (C) 2011 Samsung Electronics
> + * author: Andrzej Pietrasiewicz <andrzej.p at samsung.com>
> + *
> + * Based on gadget zero:
> + * Copyright (C) 2003-2007 David Brownell
> + * All rights reserved.
> + *
> + * 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
> + */
> +
> +#define VERBOSE_DEBUG
> +#define DEBUG
> +
> +/*
> +#include <linux/kernel.h>
> +#include <linux/utsname.h>
> +#include <linux/device.h>
> +*/
> +
> +#include <common.h>
> +#include <asm-generic/errno.h>
> +#include <linux/usb/ch9.h>
> +#include <linux/usb/gadget.h>
> +
> +#include <flash_entity.h>
> +
> +#include "gadget_chips.h"
> +/* #include "epautoconf.c" */
> +/* #include "config.c" */
> +/* #include "usbstring.c" */

Not needed, can be removed.

> +#include <malloc.h>
> +#include "dfu.h"
> +
> +static struct flash_entity *flash_ents;
> +static int num_flash_ents;
> +
> +static struct usb_device_descriptor device_desc = {
> +	.bLength =		sizeof device_desc,
> +	.bDescriptorType =	USB_DT_DEVICE,
> +	.bcdUSB =		__constant_cpu_to_le16(0x0100),
> +	.bDeviceClass =		USB_CLASS_VENDOR_SPEC,
> +	.idVendor =		__constant_cpu_to_le16(DRIVER_VENDOR_NUM),
> +	.idProduct =		__constant_cpu_to_le16(DRIVER_PRODUCT_NUM),
> +	.iManufacturer =	STRING_MANUFACTURER,
> +	.iProduct =		STRING_PRODUCT,
> +	.iSerialNumber =	STRING_SERIAL,
> +	.bNumConfigurations =	1,
> +};
> +
> +static struct usb_config_descriptor dfu_config = {
> +	.bLength =		sizeof dfu_config,
> +	.bDescriptorType =	USB_DT_CONFIG,
> +	/* compute wTotalLength on the fly */
> +	.bNumInterfaces =	1,
> +	.bConfigurationValue =	DFU_CONFIG_VAL,
> +	.iConfiguration =	STRING_DFU_NAME,
> +	.bmAttributes =		USB_CONFIG_ATT_ONE | USB_CONFIG_ATT_SELFPOWER,
> +	.bMaxPower =		1,	/* self-powered */
> +};
> +
> +static struct usb_otg_descriptor otg_descriptor = {
> +	.bLength =		sizeof otg_descriptor,
> +	.bDescriptorType =	USB_DT_OTG,
> +	.bmAttributes =		USB_OTG_SRP,
> +};
> +
> +static const struct dfu_function_descriptor dfu_func = {
> +	.bLength =		sizeof dfu_func,
> +	.bDescriptorType =	DFU_DT_FUNC,
> +	.bmAttributes =		DFU_BIT_WILL_DETACH |

Here are you setting the WILL_DETACH bit. Is the device really
detaching itself? Its not obvious to me from the code that it does.

> +				DFU_BIT_MANIFESTATION_TOLERANT |
> +				DFU_BIT_CAN_UPLOAD |
> +				DFU_BIT_CAN_DNLOAD,
> +	.wDetachTimeOut =	0,
> +	.wTransferSize =	USB_BUFSIZ,
> +	.bcdDFUVersion =	__constant_cpu_to_le16(0x0110),
> +};
> +
> +static const struct usb_interface_descriptor dfu_intf_runtime = {
> +	.bLength =		sizeof dfu_intf_runtime,
> +	.bDescriptorType =	USB_DT_INTERFACE,
> +	.bNumEndpoints =	0,
> +	.bInterfaceClass =	USB_CLASS_APP_SPEC,
> +	.bInterfaceSubClass =	1,
> +	.bInterfaceProtocol =	1,
> +	.iInterface =		STRING_DFU_NAME,
> +};
> +
> +static const struct usb_descriptor_header *dfu_function_runtime[] = {
> +	(struct usb_descriptor_header *) &otg_descriptor,
> +	(struct usb_descriptor_header *) &dfu_func,
> +	(struct usb_descriptor_header *) &dfu_intf_runtime,
> +	NULL,
> +};
> +
> +static struct usb_qualifier_descriptor dev_qualifier = {
> +	.bLength =		sizeof dev_qualifier,
> +	.bDescriptorType =	USB_DT_DEVICE_QUALIFIER,
> +	.bcdUSB =		__constant_cpu_to_le16(0x0200),
> +	.bDeviceClass =		USB_CLASS_VENDOR_SPEC,
> +	.bNumConfigurations =	1,
> +};
> +
> +static char manufacturer[50];
> +static const char longname[] = "DFU Gadget";
> +/* default serial number takes at least two packets */
> +static const char serial[] = "0123456789.0123456789.012345678";
> +static const char dfu_name[] = "Device Firmware Upgrade";
> +static const char shortname[] = "dfu";

This surely should be come from the board configuration?

> +
> +/* static strings, in UTF-8 */
> +static struct usb_string strings_runtime[] = {
> +	{ STRING_MANUFACTURER, manufacturer, },
> +	{ STRING_PRODUCT, longname, },
> +	{ STRING_SERIAL, serial, },
> +	{ STRING_DFU_NAME, dfu_name, },
> +	{  }			/* end of list */
> +};
> +
> +static struct usb_gadget_strings stringtab_runtime = {
> +	.language	= 0x0409,	/* en-us */
> +	.strings	= strings_runtime,
> +};
> +
> +static struct usb_gadget_strings stringtab_dfu = {
> +	.language	= 0x0409,	/* en-us */
> +};
> +
> +static bool is_runtime(struct dfu_dev *dev)
> +{
> +	return dev->dfu_state == DFU_STATE_appIDLE ||
> +		dev->dfu_state == DFU_STATE_appDETACH;
> +}

Hmm, here you are checking if you are in run-time mode. In the
introduction you wrote that it is in DFU mode when the dfu command is
executed. When does it enter the run-time mode? After the first
flashing?

> +static int config_buf(struct usb_gadget *gadget,
> +		u8 *buf, u8 type, unsigned index)
> +{
> +	int hs = 0;
> +	struct dfu_dev *dev = get_gadget_data(gadget);
> +	const struct usb_descriptor_header **function;
> +	int len;
> +
> +	if (index > 0)
> +		return -EINVAL;
> +
> +	if (gadget_is_dualspeed(gadget)) {
> +		hs = (gadget->speed == USB_SPEED_HIGH);
> +		if (type == USB_DT_OTHER_SPEED_CONFIG)
> +			hs = !hs;
> +	}
> +	if (is_runtime(dev))
> +		function = dfu_function_runtime;
> +	else
> +		function = (const struct usb_descriptor_header **)dev->function;
> +
> +	/* for now, don't advertise srp-only devices */
> +	if (!gadget_is_otg(gadget))
> +		function++;
> +
> +	len = usb_gadget_config_buf(&dfu_config,
> +			buf, USB_BUFSIZ, function);
> +	if (len < 0)
> +		return len;
> +	((struct usb_config_descriptor *) buf)->bDescriptorType = type;
> +	return len;
> +}
> +
> +static void dfu_reset_config(struct dfu_dev *dev)
> +{
> +	if (dev->config == 0)
> +		return;
> +
> +	DBG(dev, "reset config\n");
> +
> +	dev->config = 0;
> +}
> +
> +static int dfu_set_config(struct dfu_dev *dev, unsigned number)
> +{
> +	int result = 0;
> +	struct usb_gadget *gadget = dev->gadget;
> +	char *speed;
> +
> +	if (number == dev->config)
> +		return 0;
> +
> +	dfu_reset_config(dev);
> +
> +	if (DFU_CONFIG_VAL != number) {
> +		result = -EINVAL;
> +		return result;
> +	}
> +
> +	switch (gadget->speed) {
> +	case USB_SPEED_LOW:
> +		speed = "low";
> +		break;
> +	case USB_SPEED_FULL:
> +		speed = "full";
> +		break;
> +	case USB_SPEED_HIGH:
> +		speed = "high";
> +		break;
> +	default:
> +		speed = "?";
> +		break;
> +	}
> +
> +	dev->config = number;
> +	INFO(dev, "%s speed config #%d: %s\n", speed, number, dfu_name);
> +	return result;
> +}
> +
> +/*-------------------------------------------------------------------------*/
> +
> +static void empty_complete(struct usb_ep *ep, struct usb_request *req)
> +{
> +	/* intentionally empty */
> +}
> +
> +static void dnload_request_complete(struct usb_ep *ep, struct usb_request *req)
> +{
> +	struct dfu_dev *dev = req->context;
> +	struct flash_entity *fe = &flash_ents[dev->altsetting];
> +
> +	if (dev->not_prepared) {
> +		printf("DOWNLOAD %s\n", fe->name);
> +		fe->prepare(fe->ctx, FLASH_WRITE);
> +		dev->not_prepared = false;
> +	}
> +
> +	if (req->length > 0)
> +		fe->write_block(fe->ctx, req->length, req->buf);
> +	else {
> +		fe->finish(fe->ctx, FLASH_WRITE);
> +		dev->not_prepared = true;
> +	}
> +}
> +
> +static void handle_getstatus(struct usb_request *req)
> +{
> +	struct dfu_status *dstat = (struct dfu_status *)req->buf;
> +	struct dfu_dev *dev = req->context;
> +
> +	switch (dev->dfu_state) {
> +	case DFU_STATE_dfuDNLOAD_SYNC:
> +	case DFU_STATE_dfuDNBUSY:
> +		dev->dfu_state = DFU_STATE_dfuDNLOAD_IDLE;
> +		break;
> +	case DFU_STATE_dfuMANIFEST_SYNC:
> +		break;
> +	default:
> +		break;
> +	}
> +
> +	/* send status response */
> +	dstat->bStatus = dev->dfu_status;
> +	/* FIXME: set dstat->bwPollTimeout */

This really needs to be set. It was a nightmare with dfu-util not
having it set with the initial u-boot implementation. setting the
correct values here is not that easy though. It depends on the flash
layer and if can get the information about the maximal time a flash
write can take.

> +	dstat->bState = dev->dfu_state;
> +	dstat->iString = 0;
> +}
> +
> +static void handle_getstate(struct usb_request *req)
> +{
> +	struct dfu_dev *dev = req->context;
> +
> +	((u8 *)req->buf)[0] = dev->dfu_state & 0xff;
> +	req->actual = sizeof(u8);
> +}
> +
> +static int handle_upload(struct usb_request *req, u16 len)
> +{
> +	struct dfu_dev *dev = req->context;
> +	struct flash_entity *fe = &flash_ents[dev->altsetting];
> +	int n;
> +
> +	if (dev->not_prepared) {
> +		printf("UPLOAD %s\n", fe->name);
> +		fe->prepare(fe->ctx, FLASH_READ);
> +		dev->not_prepared = false;
> +	}
> +	n = fe->read_block(fe->ctx, len, req->buf);
> +
> +	/* no more data to read from this entity */
> +	if (n < len) {
> +		fe->finish(fe->ctx, FLASH_READ);
> +		dev->not_prepared = true;
> +	}
> +
> +	return n;
> +}
> +
> +static int handle_dnload(struct usb_gadget *gadget, u16 len)
> +{
> +	struct dfu_dev *dev = get_gadget_data(gadget);
> +	struct usb_request *req = dev->req;
> +
> +	if (len == 0)
> +		dev->dfu_state = DFU_STATE_dfuMANIFEST_SYNC;
> +
> +	req->complete = dnload_request_complete;
> +
> +	return len;
> +}
> +
> +static int
> +dfu_handle(struct usb_gadget *gadget, const struct usb_ctrlrequest *ctrl)
> +{
> +	struct dfu_dev *dev = get_gadget_data(gadget);
> +	struct usb_request *req = dev->req;
> +	u16 len = le16_to_cpu(ctrl->wLength);
> +	int rc = 0;
> +
> +	switch (dev->dfu_state) {
> +	case DFU_STATE_appIDLE:
> +		switch (ctrl->bRequest) {
> +		case USB_REQ_DFU_GETSTATUS:
> +			handle_getstatus(req);
> +			return RET_STAT_LEN;
> +		case USB_REQ_DFU_GETSTATE:
> +			handle_getstate(req);
> +			break;
> +		case USB_REQ_DFU_DETACH:
> +			dev->dfu_state = DFU_STATE_appDETACH;
> +			dev->dfu_state = DFU_STATE_dfuIDLE;
> +			return RET_ZLP;
> +		default:
> +			return RET_STALL;
> +		}
> +		break;
> +	case DFU_STATE_appDETACH:
> +		switch (ctrl->bRequest) {
> +		case USB_REQ_DFU_GETSTATUS:
> +			handle_getstatus(req);
> +			return RET_STAT_LEN;
> +		case USB_REQ_DFU_GETSTATE:
> +			handle_getstate(req);
> +			break;
> +		default:
> +			dev->dfu_state = DFU_STATE_appIDLE;
> +			return RET_STALL;
> +		}
> +		/* FIXME: implement timer to return to appIDLE */
> +		break;
> +	case DFU_STATE_dfuIDLE:
> +		switch (ctrl->bRequest) {
> +		case USB_REQ_DFU_DNLOAD:
> +			if (len == 0) {
> +				dev->dfu_state = DFU_STATE_dfuERROR;
> +				return RET_STALL;
> +			}
> +			dev->dfu_state = DFU_STATE_dfuDNLOAD_SYNC;
> +			return handle_dnload(gadget, len);
> +		case USB_REQ_DFU_UPLOAD:
> +			dev->dfu_state = DFU_STATE_dfuUPLOAD_IDLE;
> +			return handle_upload(req, len);
> +		case USB_REQ_DFU_ABORT:
> +			/* no zlp? */
> +			return RET_ZLP;
> +		case USB_REQ_DFU_GETSTATUS:
> +			handle_getstatus(req);
> +			return RET_STAT_LEN;
> +		case USB_REQ_DFU_GETSTATE:
> +			handle_getstate(req);
> +			break;
> +		case USB_REQ_DFU_DETACH:
> +			/* Proprietary extension: 'detach' from idle mode and
> +			 * get back to runtime mode in case of USB Reset.  As
> +			 * much as I dislike this, we just can't use every USB
> +			 * bus reset to switch back to runtime mode, since at
> +			 * least the Linux USB stack likes to send a number of
> +			 * resets in a row :(
> +			 */

OK, this makes it clear that you took the DFU state machine from
Haralds implementation I'm also using. :)

It would be nice if the related copyright holder would be stated in
the header of the file.

> +			dev->dfu_state = DFU_STATE_dfuMANIFEST_WAIT_RST;
> +			dev->dfu_state = DFU_STATE_appIDLE;
> +			break;
> +		default:
> +			dev->dfu_state = DFU_STATE_dfuERROR;
> +			return RET_STALL;
> +		}
> +		break;
> +	case DFU_STATE_dfuDNLOAD_SYNC:
> +		switch (ctrl->bRequest) {
> +		case USB_REQ_DFU_GETSTATUS:
> +			handle_getstatus(req);
> +			return RET_STAT_LEN;
> +			/* FIXME: state transition depending
> +			 * on block completeness */
> +		case USB_REQ_DFU_GETSTATE:
> +			handle_getstate(req);
> +			break;
> +		default:
> +			dev->dfu_state = DFU_STATE_dfuERROR;
> +			return RET_STALL;
> +		}
> +		break;
> +	case DFU_STATE_dfuDNBUSY:
> +		switch (ctrl->bRequest) {
> +		case USB_REQ_DFU_GETSTATUS:
> +			/* FIXME: only accept getstatus if bwPollTimeout
> +			 * has elapsed */
> +			handle_getstatus(req);
> +			return RET_STAT_LEN;
> +		default:
> +			dev->dfu_state = DFU_STATE_dfuERROR;
> +			return RET_STALL;
> +		}
> +		break;
> +	case DFU_STATE_dfuDNLOAD_IDLE:
> +		switch (ctrl->bRequest) {
> +		case USB_REQ_DFU_DNLOAD:
> +			dev->dfu_state = DFU_STATE_dfuDNLOAD_SYNC;
> +			return handle_dnload(gadget, len);
> +		case USB_REQ_DFU_ABORT:
> +			dev->dfu_state = DFU_STATE_dfuIDLE;
> +			return RET_ZLP;
> +		case USB_REQ_DFU_GETSTATUS:
> +			handle_getstatus(req);
> +			return RET_STAT_LEN;
> +		case USB_REQ_DFU_GETSTATE:
> +			handle_getstate(req);
> +			break;
> +		default:
> +			dev->dfu_state = DFU_STATE_dfuERROR;
> +			return RET_STALL;
> +		}
> +		break;
> +	case DFU_STATE_dfuMANIFEST_SYNC:
> +		switch (ctrl->bRequest) {
> +		case USB_REQ_DFU_GETSTATUS:
> +			/* We're MainfestationTolerant */
> +			dev->dfu_state = DFU_STATE_dfuIDLE;
> +			handle_getstatus(req);
> +			return RET_STAT_LEN;
> +		case USB_REQ_DFU_GETSTATE:
> +			handle_getstate(req);
> +			break;
> +		default:
> +			dev->dfu_state = DFU_STATE_dfuERROR;
> +			return RET_STALL;
> +		}
> +		break;
> +	case DFU_STATE_dfuMANIFEST:
> +		/* we should never go here */
> +		dev->dfu_state = DFU_STATE_dfuERROR;
> +		return RET_STALL;
> +	case DFU_STATE_dfuMANIFEST_WAIT_RST:
> +		/* we should never go here */
> +		break;
> +	case DFU_STATE_dfuUPLOAD_IDLE:
> +		switch (ctrl->bRequest) {
> +		case USB_REQ_DFU_UPLOAD:
> +			/* state transition if less data then requested */
> +			rc = handle_upload(req, len);
> +			if (rc >= 0 && rc < len)
> +				dev->dfu_state = DFU_STATE_dfuIDLE;
> +			return rc;
> +		case USB_REQ_DFU_ABORT:
> +			dev->dfu_state = DFU_STATE_dfuIDLE;
> +			/* no zlp? */
> +			return RET_ZLP;
> +		case USB_REQ_DFU_GETSTATUS:
> +			handle_getstatus(req);
> +			return RET_STAT_LEN;
> +		case USB_REQ_DFU_GETSTATE:
> +			handle_getstate(req);
> +			break;
> +		default:
> +			dev->dfu_state = DFU_STATE_dfuERROR;
> +			return RET_STALL;
> +		}
> +		break;
> +	case DFU_STATE_dfuERROR:
> +		switch (ctrl->bRequest) {
> +		case USB_REQ_DFU_GETSTATUS:
> +			handle_getstatus(req);
> +			return RET_STAT_LEN;
> +		case USB_REQ_DFU_GETSTATE:
> +			handle_getstate(req);
> +			break;
> +		case USB_REQ_DFU_CLRSTATUS:
> +			dev->dfu_state = DFU_STATE_dfuIDLE;
> +			dev->dfu_status = DFU_STATUS_OK;
> +			/* no zlp? */
> +			return RET_ZLP;
> +		default:
> +			dev->dfu_state = DFU_STATE_dfuERROR;
> +			return RET_STALL;
> +		}
> +		break;
> +	default:
> +		return -1;
> +	}
> +
> +	return 0;
> +}

[snap]

> +int __init dfu_init(void)
> +{
> +	return usb_gadget_register_driver(&dfu_driver);
> +}

All this is absed on the re-written gadget layer? It loks as if all
kind of kernel stuff is brought over here. Is this gadget layer
already merged into u-boot mainline?

> diff --git a/drivers/usb/gadget/dfu.h b/drivers/usb/gadget/dfu.h
> new file mode 100644
> index 0000000..5a6386e
> --- /dev/null
> +++ b/drivers/usb/gadget/dfu.h
> @@ -0,0 +1,171 @@
> +/*
> + * dfu.h -- Device Firmware Update gadget
> + *
> + * Copyright (C) 2011 Samsung Electronics
> + * author: Andrzej Pietrasiewicz <andrzej.p 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 DFU_H_
> +#define DFU_H_
> +
> +#include <linux/compiler.h>
> +
> +/*
> + * Linux kernel compatibility layer
> + */
> +#define GFP_ATOMIC				((gfp_t) 0)
> +#define GFP_KERNEL				((gfp_t) 0)
> +#define true					1
> +#define false					0
> +#define dev_dbg(...)				do {} while (0)
> +#define dev_vdbg(...)				do {} while (0)
> +#define dev_err(...)				do {} while (0)
> +#define dev_warn(...)				do {} while (0)
> +#define dev_info(...)				do {} while (0)
> +#define pr_warning(...)				do {} while (0)
> +#define spin_lock_init(lock)			do {} while (0)
> +#define spin_lock(lock)				do {} while (0)
> +#define spin_unlock(lock)			do {} while (0)
> +#define spin_lock_irqsave(lock,flags)		do {flags = 1;} while (0)
> +#define spin_unlock_irqrestore(lock,flags)	do {flags = 0;} while (0)
> +#define kmalloc(x,y)				malloc(x)
> +#define kfree(x)				free(x)
> +#define kzalloc(size,flags)			calloc((size), 1)
> +#define __init
> +#define __exit
> +#define __exit_p(x)				x
> +#define module_init(...)
> +#define module_exit(...)
> +#define min_t					min
> +#define spinlock_t				int
> +#define bool					int
> +/*
> + * end compatibility layer
> + */

Urgs, is this all needed? Do we really want to have all this around.
This is not the kernel. U-Boots debug infrastrcuture should be used.

> +#define DBG(d, fmt, args...) \
> +	dev_dbg(&(d)->gadget->dev , fmt , ## args)
> +#define VDBG(d, fmt, args...) \
> +	dev_vdbg(&(d)->gadget->dev , fmt , ## args)
> +#define ERROR(d, fmt, args...) \
> +	dev_err(&(d)->gadget->dev , fmt , ## args)
> +#define WARN(d, fmt, args...) \
> +	dev_warn(&(d)->gadget->dev , fmt , ## args)
> +#define INFO(d, fmt, args...) \
> +	dev_info(&(d)->gadget->dev , fmt , ## args)
> +
> +#define DRIVER_VERSION			"Marzanna"

Maybe that helps you internal but it does not make much sense for
people outside Samsung. :)

It was good to see that the DFU state machine was taken from Haralds
patches. That should make it a lot easier to find a common ground
between the implementations. And I agree that splitting out the DFU
protocol from the flashing parts makes sense.

I would like to get some initila feedback from Remy if this should be
based on the new gadget stuff or not.

How would you like to proceed in getting our stuff merged and finally
integrated into mainline?

regards
Stefan Schmidt


More information about the U-Boot mailing list