[U-Boot] [PATCH] dfu: initial implementation
Signed-off-by: Andrzej Pietrasiewicz andrzej.p@samsung.com Signed-off-by: Kyungmin Park kyungmin.park@samsung.com ---
Dear All,
This is Device Firmware Upgrade (DFU) implementation which supports data upload and download function to devices which are equipped with a UDC.
Device Firmware Upgrade (DFU) is an extension to the USB specification. As of the time of this writing it is documented at
http://www.usb.org/developers/devclass_docs/DFU_1.1.pdf
The aim of DFU is to allow transferring data to/from selected areas of interest within a compliant device. In the DFU nomenclature the host->device direction is called download and the device->host direction is called upload. The areas are defined by the compliant device. In the DFU nomenclature the area of interest within the device is called an altsetting. The device is connected to the host through USB. On the host side compliant software must be used to initiate the data transfer. Example piece of such software is dfu-util package from the OpenMoko project:
http://wiki.openmoko.org/wiki/Dfu-util.
Please refer to dfu-util documentation for details. The below ASCII-art presents a connection between the host and the device.
+-----------------+ <--- DFU ---> +-------------------------+ | e.g. dfu-util | <=== USB ===> | / altsetting 0 | | host +---------------------+ device - altsetting 1 | | file / | | \ ... | +-----------------+ +-------------------------+
The DFU implementation on the device side remains in one of the two modes: the Run-Time mode and the DFU mode. The USB descriptor sets exported by the device depend on which mode is in force. While in DFU mode the device exports the descriptors corresponding to each available altsetting. An example dfu-util session when the device is in the DFU mode looks similar to this:
# dfu-util -l dfu-util - (C) 2007-2008 by OpenMoko Inc. This program is Free Software and has ABSOLUTELY NO WARRANTY
Found DFU: [0x0525:0xffff] devnum=46, cfg=0, intf=0, alt=0, name="bootldr" Found DFU: [0x0525:0xffff] devnum=46, cfg=0, intf=0, alt=1, name="kernel"
In the above example there are two altsettings, altsetting 0 with a human-readable name "bootldr" and altsetting 1 with a human-readable name "kernel"
To initiate data transfer the user at the host side must specify the altsetting to/from which the data transfer is to be performed. In case of the dfu-util it is specified with a "-a" option followed by either a number or a human-readable name. The user also needs to specify the direction of the data transfer and the file on the host from/to which data are to be transfered. An example download command line looks similar to this:
# dfu-util -D vmlinux -a kernel
In the above command line the contents of the file vmlinux is to be downloaded into the altsetting named kernel.
An example upload command line looks similar to this:
# dfu-util -U vmlinux -a kernel
In the above command line the contents of the altsetting named kernel is to be uploaded into the file vmlinux.
This u-boot implementation introduces a parameter-less dfu command. After the user finishes with dfu they can Ctrl-C to return to u-boot main menu.
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.
Patch summary:
Andrzej Pietrasiewicz (1): dfu: initial implementation
board/samsung/goni/Makefile | 2 + board/samsung/goni/flash.c | 634 +++++++++++++++++++++++++++++ board/samsung/goni/flash.h | 28 ++ board/samsung/goni/goni.c | 17 + common/Makefile | 1 + common/cmd_dfu.c | 50 +++ drivers/usb/gadget/Makefile | 1 + drivers/usb/gadget/dfu.c | 920 +++++++++++++++++++++++++++++++++++++++++++ drivers/usb/gadget/dfu.h | 171 ++++++++ include/configs/s5p_goni.h | 6 + include/dfu.h | 31 ++ include/flash_entity.h | 39 ++ include/mbr.h | 49 +++ 13 files changed, 1949 insertions(+), 0 deletions(-) create mode 100644 board/samsung/goni/flash.c create mode 100644 board/samsung/goni/flash.h create mode 100644 common/cmd_dfu.c create mode 100644 drivers/usb/gadget/dfu.c create mode 100644 drivers/usb/gadget/dfu.h create mode 100644 include/dfu.h create mode 100644 include/flash_entity.h create mode 100644 include/mbr.h
diff --git a/board/samsung/goni/Makefile b/board/samsung/goni/Makefile index ecde7a7..c07b0a2 100644 --- a/board/samsung/goni/Makefile +++ b/board/samsung/goni/Makefile @@ -31,6 +31,8 @@ LIB = $(obj)lib$(BOARD).o COBJS-y := goni.o onenand.o SOBJS := lowlevel_init.o
+COBJS-$(CONFIG_CMD_DEVICE_FIRMWARE_UPGRADE) += flash.o + SRCS := $(SOBJS:.o=.S) $(COBJS-y:.o=.c) OBJS := $(addprefix $(obj),$(COBJS-y)) SOBJS := $(addprefix $(obj),$(SOBJS)) diff --git a/board/samsung/goni/flash.c b/board/samsung/goni/flash.c new file mode 100644 index 0000000..a6dd7d2 --- /dev/null +++ b/board/samsung/goni/flash.c @@ -0,0 +1,634 @@ +/* + * flash.c -- board flashing routines + * + * Copyright (C) 2011 Samsung Electronics + * author: Andrzej Pietrasiewicz andrzej.p@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 <malloc.h> +#include <mbr.h> +#include <mmc.h> +#include <fat.h> +#include <flash_entity.h> +#include <linux/types.h> +#include <jffs2/load_kernel.h> + +#define MMC_BLOCK_SZ (256 * 1024) +#define MMC_FAT_BLOCK_SZ (4 * 1024 * 1024) +#define MMC_SECTOR_SZ 512 +#define MMC_UBOOT_OFFSET 128 +#define MMC_UBOOT_SZ 512 +#define PARAM_LEN 12 +#define SHORT_PART_NAME 15 +#define LONG_PART_NAME 20 +#define SIGNATURE ((unsigned short) 0xAA55) +#define CALLOC_STRUCT(n, type) (struct type *) calloc(n, sizeof(struct type)) +#define DEFAULT_MMC_PART_NAME "mmc-default-part" + +/* partition IDs counted from 0, eg. mmc0-pri-1's ID is 0 */ +#define UIMAGE_PART_ID 1 +#define EXTENDED_PART_ID 3 +#define UMS_PART_ID 7 +#define UIMAGE_PART_NAME "mmc0-pri-2" + +#define USE_MMC_UBOOT +#define USE_MMC + +typedef int (*rw_op)(void *buf, unsigned int len, unsigned long from); +typedef int (*erase_op)(unsigned int len, unsigned long from); + +struct flash_entity_ctx { + unsigned long offset; /* offset into the device */ + unsigned long length; /* size of the entity */ + u8 *buf; /* buffer for one chunk */ + unsigned long buf_len; /* one chunk length */ + unsigned int buffered; /* # available bytes in buf */ + unsigned int num_done; /* total bytes handled */ + rw_op read; /* chunk read op for this medium */ + rw_op write; /* chunk write op for this medium */ + erase_op erase; /* erase op for this medium or NULL */ + struct flash_entity *this_entity; /* the containing entity */ + void *associated; /* related entity, if any */ +}; + +struct mbr_part_data { + unsigned long offset; /* #sectors from mmc begin */ + unsigned long length; /* #sectors in this partition*/ + u8 primary; /* != 0 if primary, 0 if extended */ +}; + +/* + * MMC u-boot partitions + */ +static struct mbr_part_data *uboot_pdata; + +static u8 uboot_part_num; +static u8 used_uboot_parts; + +int use_uboot(struct mbr_part_data *pdata, u8 i) +{ + /* + * Use i and pdata[i] members to decide if the partition is used + */ + return 1; +} + +char *alloc_uboot_name(u8 i) +{ + char *name = calloc(SHORT_PART_NAME, 1); + + if (name) { + sprintf(name, "mmc-u-boot"); + return name; + } + + return DEFAULT_MMC_PART_NAME; +} + +/* + * MMC partitions and MMC operations + */ +struct mmc *mmc; + +static struct mbr_part_data *mmc_pdata; + +static u8 mmc_part_num; +static u8 used_mmc_parts; + +static u8 mmc_buf[MMC_BLOCK_SZ]; + +static int extended_lba; + +static int mmc_mbr_dev; + +static u8 pri_count; +static u8 ext_count = 4; + +/* + * Define files available in the UIMAGE partition which has FAT on it. + * Only flat structure without subdirectories is supported. + */ +static char *uImage_part_files[] = { + "uImage", +}; +#define UIMAGE_PART_NUM_FILES ARRAY_SIZE(uImage_part_files) + +static int read_ebr(struct mmc *mmc, struct mbr_partition *mp, + int ebr_next, struct mbr_part_data *pd, int parts) +{ + struct mbr *ebr; + struct mbr_partition *p; + char buf[512]; + int ret, i; + int lba = 0; + + if (ebr_next) + lba = extended_lba; + + lba += mp->lba; + ret = mmc->block_dev.block_read(mmc_mbr_dev, lba, 1, buf); + if (ret != 1) + return 0; + + ebr = (struct mbr *) buf; + + if (ebr->signature != SIGNATURE) { + printf("Signature error 0x%x\n", ebr->signature); + return 0; + } + + for (i = 0; i < 2; i++) { + p = (struct mbr_partition *) &ebr->parts[i]; + + if (i == 0) { + lba += p->lba; + if (p->partition_type == 0x83) { + if (pd) { + pd[parts].offset = lba; + pd[parts].length = p->nsectors; + pd[parts].primary = 0; + } + parts++; + } + } + } + + if (p->lba && p->partition_type == 0x5) + parts = read_ebr(mmc, p, 1, pd, parts); + + return parts; +} + +static int read_mbr(struct mmc *mmc, struct mbr_part_data *pd) +{ + struct mbr_partition *mp; + struct mbr *mbr; + char buf[512]; + int ret, i; + int parts = 0; + + ret = mmc->block_dev.block_read(mmc_mbr_dev, 0, 1, buf); + if (ret != 1) + return 0; + + mbr = (struct mbr *) buf; + + if (mbr->signature != SIGNATURE) { + printf("Signature error 0x%x\n", mbr->signature); + return 0; + } + + for (i = 0; i < 4; i++) { + mp = (struct mbr_partition *) &mbr->parts[i]; + + if (!mp->partition_type) + continue; + + if (mp->partition_type == 0x83) { + if (pd) { + pd[parts].offset = mp->lba; + pd[parts].length = mp->nsectors; + pd[parts].primary = 1; + } + parts++; + } + + if (mp->lba && mp->partition_type == 0x5) { + extended_lba = mp->lba; + parts = read_ebr(mmc, mp, 0, pd, parts); + } + } + + return parts; +} + +static int rw_mmc(void *buf, char *op, unsigned int len, unsigned long from) +{ + char ram_addr[PARAM_LEN]; + char offset[PARAM_LEN]; + char length[PARAM_LEN]; + char *argv[] = {"mmc", op, ram_addr, offset, length}; + + sprintf(ram_addr, "0x%lx", (unsigned long)buf); + sprintf(offset, "0x%lx", from / MMC_SECTOR_SZ); /* guaranteed integer */ + sprintf(length, "0x%x", (len + MMC_SECTOR_SZ - 1) / MMC_SECTOR_SZ); + + return do_mmcops(NULL, 0, 6, argv); +} + +static inline int read_mmc(void *buf, unsigned int len, unsigned long from) +{ + return rw_mmc(buf, "read", len, from); +} + +static inline int write_mmc(void *buf, unsigned int len, unsigned long from) +{ + return rw_mmc(buf, "write", len, from); +} + +/* + * Return number of flash entities per this partition + */ +u8 use_mmc(struct mbr_part_data *pdata, u8 i) +{ + /* + * Use i and pdata[i] members to decide if the partition is used + */ + if (i == UIMAGE_PART_ID) + return UIMAGE_PART_NUM_FILES; + if (i == EXTENDED_PART_ID) + return 0; /* do not expose the extended partition as a whole */ + if (i == UMS_PART_ID) + return 0; /* do not expose UMS; there is a separate command */ + return 1; +} + +char *alloc_mmc_name(struct mbr_part_data *pdata, u8 i, u8 l) +{ + char *name = calloc(PARAM_LEN, 1); + + if (name) { + sprintf(name, "mmc0-"); + if (pdata[i].primary) + sprintf(name + strlen(name), "pri-%d", + l ? pri_count : ++pri_count); + else + sprintf(name + strlen(name), "ext-%d", + l ? ext_count : ++ext_count); + + return name; + } + + return DEFAULT_MMC_PART_NAME; +} + +/* + * FAT operations + */ +static u8 fat_buf[MMC_FAT_BLOCK_SZ]; + +static char *fat_filename; +static int fat_part_num; + +static int read_fat(void *buf, unsigned int len, unsigned long from) +{ + int ret; + + ret = fat_register_device(&mmc->block_dev, fat_part_num); + if (ret < 0) { + printf("error : fat_register_device\n"); + return 0; + } + printf("read up to %d B ", MMC_FAT_BLOCK_SZ); + return file_fat_read(fat_filename, buf, len); +} + +static int write_fat(void *buf, unsigned int len, unsigned long from) +{ +#ifdef CONFIG_FAT_WRITE + int ret; + + ret = fat_register_device(&mmc->block_dev, fat_part_num); + if (ret < 0) { + printf("error : fat_register_divce\n"); + return 0; + } + + printf("write up to %d B ", MMC_FAT_BLOCK_SZ); + ret = file_fat_write(fat_filename, buf, len); + + /* format and write again */ + if (ret == 1) { + printf("formatting\n"); + if (mkfs_vfat(&mmc->block_dev, fat_part_num)) { + printf("error formatting device\n"); + return 0; + } + ret = file_fat_write(fat_filename, buf, len); + } + + if (ret < 0) { + printf("error : writing %s\n", fat_filename); + return 0; + } +#else + printf("error : FAT write not supported\n"); + return 0; +#endif + return len; +} + +/* + * Entity-specific prepare and finish + */ +static void reset_ctx(struct flash_entity_ctx *ctx) +{ + ctx->buffered = 0; + ctx->num_done = 0; +} + +static int generic_prepare(void *ctx, u8 mode) +{ + struct flash_entity_ctx *ct = ctx; + + reset_ctx(ct); + memset(ct->buf, 0, ct->buf_len); + if (mode == FLASH_WRITE) { + if (ct->erase) { + printf("Erase entity: %s ", ct->this_entity->name); + ct->erase(ct->length, ct->offset); + } + printf("Write entity: %s ", ct->this_entity->name); + } else if (mode == FLASH_READ) { + printf("Read entity: %s ", ct->this_entity->name); + } + return 0; +} + +static int generic_finish(void *ctx, u8 mode) +{ + struct flash_entity_ctx *ct = ctx; + + if (mode == FLASH_WRITE && ct->buffered > 0) + ct->write(ct->buf, ct->buffered, ct->offset + ct->num_done); + + return 0; +} + +static int prepare_fat(void *ctx, u8 mode) +{ + struct flash_entity_ctx *ct = ctx; + + fat_filename = ct->associated; + if (!strncmp(ct->this_entity->name, UIMAGE_PART_NAME, + strlen(UIMAGE_PART_NAME))) + fat_part_num = UIMAGE_PART_ID + 1; + return generic_prepare(ctx, mode); +} + +/* + * Transport layer to storage adaptation + */ + +/* + * Adapt transport layer buffer size to storage chunk size + * + * return < n to indicate no more data to read + */ +static int read_block(void *ctx, unsigned int n, void *buf) +{ + struct flash_entity_ctx *ct = ctx; + unsigned int nread = 0; + + if (n == 0) + return n; + + while (nread < n) { + unsigned int copy; + + if (ct->num_done >= ct->length) + break; + if (ct->buffered == 0) { + ct->read(ct->buf, ct->buf_len, + ct->offset + ct->num_done); + ct->buffered = ct->buf_len; + } + copy = min(n - nread, ct->buffered); + + memcpy(buf + nread, ct->buf + ct->buf_len - ct->buffered, copy); + nread += copy; + ct->buffered -= copy; + ct->num_done += copy; + } + + return nread; +} + +/* + * Adapt transport layer buffer size to storage chunk size + */ +static int write_block(void *ctx, unsigned int n, void *buf) +{ + struct flash_entity_ctx *ct = ctx; + unsigned int nwritten = 0; + + if (n == 0) + return n; + + while (nwritten < n) { + unsigned int copy; + + if (ct->num_done >= ct->length) + break; + if (ct->buffered >= ct->buf_len) { + ct->write(ct->buf, ct->buf_len, + ct->offset + ct->num_done); + ct->buffered = 0; + ct->num_done += ct->buf_len; + if (ct->num_done >= ct->length) + break; + } + copy = min(n - nwritten, ct->buf_len - ct->buffered); + + memcpy(ct->buf + ct->buffered, buf + nwritten, copy); + nwritten += copy; + ct->buffered += copy; + } + + return nwritten; +} + +/* + * Flash entities definitions for this board + */ +static struct flash_entity *flash_ents; + +void customize_entities(struct flash_entity *fe, u8 n_ents) +{ + int i; + for (i = 0; i < n_ents; ++i) { + /* i counts all entities, not just mmc entities */ + /* add similar "if" blocks for other customizable entities */ + if (!strcmp(fe[i].name, UIMAGE_PART_NAME)) { + struct flash_entity_ctx *ctx; + char *name, file_number; + + fe[i].prepare = prepare_fat; + ctx = fe[i].ctx; + ctx->length = min(ctx->length, MMC_FAT_BLOCK_SZ); + ctx->buf = fat_buf; + ctx->buf_len = ctx->length; + ctx->read = read_fat; + ctx->write = write_fat; + file_number = (char)ctx->associated; + /* by design file_number cannot exceed array size */ + ctx->associated = uImage_part_files[file_number]; + + name = calloc(LONG_PART_NAME, 1); + if (name) { + sprintf(name, "%s-%s", fe[i].name, + (char *)ctx->associated); + free(fe[i].name); + fe[i].name = name; + } + + continue; + } + } +} + +static inline void generic_flash_entity(struct flash_entity *fe) +{ + fe->prepare = generic_prepare; + fe->finish = generic_finish; + fe->read_block = read_block; + fe->write_block = write_block; +} + +static inline void generic_flash_entity_ctx(struct flash_entity_ctx *ctx, + struct flash_entity *fe) +{ + ctx->buf = mmc_buf; + ctx->buf_len = MMC_BLOCK_SZ; + ctx->read = read_mmc; + ctx->write = write_mmc; + ctx->erase = NULL; + ctx->this_entity = fe; + fe->ctx = ctx; +} + +void register_flash_areas(void) +{ + u8 i, j; + +#ifdef USE_MMC + mmc = find_mmc_device(mmc_mbr_dev); + if (mmc && !mmc_init(mmc)) { + mmc_part_num = read_mbr(mmc, NULL); + if (mmc_part_num) { + mmc_pdata = CALLOC_STRUCT(mmc_part_num, + mbr_part_data); + if (mmc_pdata) + if (!read_mbr(mmc, mmc_pdata)) { + free(mmc_pdata); + mmc_pdata = NULL; + } + } + } + used_mmc_parts = 0; + for (i = 0; mmc_pdata && i < mmc_part_num; ++i) + used_mmc_parts += use_mmc(mmc_pdata, i); + pri_count = 0; + ext_count = 4; +#endif + +#ifdef USE_MMC_UBOOT + if (mmc) { + uboot_part_num = 1; + if (uboot_part_num) { + uboot_pdata = CALLOC_STRUCT(uboot_part_num, + mbr_part_data); + if (uboot_pdata) + for (i = 0; i < uboot_part_num; ++i) { + uboot_pdata[i].offset = + MMC_UBOOT_OFFSET; + uboot_pdata[i].length = + MMC_UBOOT_SZ; + uboot_pdata[i].primary = 0; + } + } + } + used_uboot_parts = 0; + for (i = 0; uboot_pdata && i < uboot_part_num; ++i) + used_uboot_parts += use_uboot(uboot_pdata, i); +#endif + + flash_ents = CALLOC_STRUCT(used_uboot_parts + used_mmc_parts, + flash_entity); + if (!flash_ents) + goto partinfo_alloc_rollback; + + j = 0; + for (i = 0; i < uboot_part_num; ++i) + if (use_uboot(uboot_pdata, i)) { + struct flash_entity *fe; + struct flash_entity_ctx *ctx; + + fe = &flash_ents[j++]; + fe->name = alloc_uboot_name(i); + generic_flash_entity(fe); + + ctx = CALLOC_STRUCT(1, flash_entity_ctx); + if (!ctx) + goto flash_ents_alloc_rollback; + generic_flash_entity_ctx(ctx, fe); + ctx->offset = uboot_pdata[i].offset * MMC_SECTOR_SZ; + ctx->length = uboot_pdata[i].length * MMC_SECTOR_SZ; + } + uboot_part_num = used_uboot_parts; + + for (i = 0; i < mmc_part_num; ++i) { + u8 k = use_mmc(mmc_pdata, i); + u8 l; + for (l = 0; l < k; ++l) { + struct flash_entity *fe; + struct flash_entity_ctx *ctx; + + fe = &flash_ents[j++]; + fe->name = alloc_mmc_name(mmc_pdata, i, l); + generic_flash_entity(fe); + + ctx = CALLOC_STRUCT(1, flash_entity_ctx); + if (!ctx) + goto flash_ents_alloc_rollback; + generic_flash_entity_ctx(ctx, fe); + ctx->offset = mmc_pdata[i].offset * MMC_SECTOR_SZ; + ctx->length = mmc_pdata[i].length * MMC_SECTOR_SZ; + ctx->associated = (void *)l; + } + } + mmc_part_num = used_mmc_parts; + customize_entities(flash_ents, uboot_part_num + mmc_part_num); + register_flash_entities(flash_ents, uboot_part_num + mmc_part_num); + + return; + +flash_ents_alloc_rollback: + while (j--) { + free(flash_ents[j].name); + free(flash_ents[j].ctx); + } + free(flash_ents); + +partinfo_alloc_rollback: + free(uboot_pdata); + free(mmc_pdata); +} + +void unregister_flash_areas(void) +{ + int j = uboot_part_num + mmc_part_num; + while (j--) { + free(flash_ents[j].name); + free(flash_ents[j].ctx); + } + free(flash_ents); + free(uboot_pdata); + free(mmc_pdata); +} + diff --git a/board/samsung/goni/flash.h b/board/samsung/goni/flash.h new file mode 100644 index 0000000..ffe4d6d --- /dev/null +++ b/board/samsung/goni/flash.h @@ -0,0 +1,28 @@ +/* + * flash.h -- board flashing routines interface + * + * Copyright (C) 2011 Samsung Electronics + * author: Andrzej Pietrasiewicz andrzej.p@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 FLASH_GONI_H_ +#define FLASH_GONI_H_ + +void register_flash_areas(void); +void unregister_flash_areas(void); + +#endif diff --git a/board/samsung/goni/goni.c b/board/samsung/goni/goni.c index c4fc3e9..da9afd0 100644 --- a/board/samsung/goni/goni.c +++ b/board/samsung/goni/goni.c @@ -29,6 +29,9 @@ #include <asm/arch/hs_otg.h> #include <asm/arch/cpu.h> #include <max8998_pmic.h> + +#include "flash.h" + DECLARE_GLOBAL_DATA_PTR;
static struct s5pc110_gpio *s5pc110_gpio; @@ -144,4 +147,18 @@ struct s3c_plat_otg_data s5pc110_otg_data = { .regs_phy = S5PC110_PHY_BASE, .regs_otg = S5PC110_OTG_BASE, }; + +#ifdef CONFIG_CMD_DEVICE_FIRMWARE_UPGRADE +void board_dfu_init(void) +{ + register_flash_areas(); + s3c_udc_probe(&s5pc110_otg_data); +} + +void board_dfu_cleanup(void) +{ + unregister_flash_areas(); +} +#endif + #endif 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 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 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@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" +); + diff --git a/drivers/usb/gadget/Makefile b/drivers/usb/gadget/Makefile index cd22bbe..4b173e2 100644 --- a/drivers/usb/gadget/Makefile +++ b/drivers/usb/gadget/Makefile @@ -28,6 +28,7 @@ LIB := $(obj)libusb_gadget.o # new USB gadget layer dependencies ifdef CONFIG_USB_GADGET COBJS-y += epautoconf.o config.o usbstring.o +COBJS-$(CONFIG_DFU_GADGET) += dfu.o COBJS-$(CONFIG_USB_GADGET_S3C_UDC_OTG) += s3c_udc_otg.o endif ifdef CONFIG_USB_ETHER 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@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" */ + +#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 | + 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"; + +/* 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; +} + +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 */ + 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 :( + */ + 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; +} + +static int +dfu_setup(struct usb_gadget *gadget, const struct usb_ctrlrequest *ctrl) +{ + struct dfu_dev *dev = get_gadget_data(gadget); + struct usb_request *req = dev->req; + int value = -EOPNOTSUPP; + u16 w_index = le16_to_cpu(ctrl->wIndex); + u16 w_value = le16_to_cpu(ctrl->wValue); + u16 w_length = le16_to_cpu(ctrl->wLength); + + req->zero = 0; + req->complete = empty_complete; + if (!(ctrl->bRequestType & USB_TYPE_CLASS)) + switch (ctrl->bRequest) { + + case USB_REQ_GET_DESCRIPTOR: + if (ctrl->bRequestType != USB_DIR_IN) + goto unknown; + switch (w_value >> 8) { + + case USB_DT_DEVICE: + value = min(w_length, (u16) sizeof device_desc); + memcpy(req->buf, &device_desc, value); + break; + case USB_DT_DEVICE_QUALIFIER: + if (!gadget_is_dualspeed(gadget)) + break; + value = min(w_length, + (u16) sizeof dev_qualifier); + memcpy(req->buf, &dev_qualifier, value); + break; + + case USB_DT_OTHER_SPEED_CONFIG: + if (!gadget_is_dualspeed(gadget)) + break; + /* FALLTHROUGH */ + case USB_DT_CONFIG: + value = config_buf(gadget, req->buf, + w_value >> 8, + w_value & 0xff); + if (value >= 0) + value = min_t(w_length, (u16) value); + break; + + case USB_DT_STRING: + /* wIndex == language code. */ + value = usb_gadget_get_string( + is_runtime(dev) ? &stringtab_runtime : + &stringtab_dfu, + w_value & 0xff, req->buf); + if (value >= 0) + value = min_t(w_length, (u16) value); + break; + case DFU_DT_FUNC: + value = min(w_length, (u16) sizeof dfu_func); + memcpy(req->buf, &dfu_func, value); + break; + } + break; + + case USB_REQ_SET_CONFIGURATION: + if (ctrl->bRequestType != 0) + goto unknown; + if (gadget->a_hnp_support) + DBG(dev, "HNP available\n"); + else if (gadget->a_alt_hnp_support) + DBG(dev, "HNP needs a different root port\n"); + else + VDBG(dev, "HNP inactive\n"); + spin_lock(&dev->lock); + value = dfu_set_config(dev, w_value); + spin_unlock(&dev->lock); + break; + case USB_REQ_GET_CONFIGURATION: + if (ctrl->bRequestType != USB_DIR_IN) + goto unknown; + *(u8 *)req->buf = dev->config; + value = min(w_length, (u16) 1); + break; + + /* until we add altsetting support, or other interfaces, + * only 0/0 are possible. pxa2xx only supports 0/0 (poorly) + * and already killed pending endpoint I/O. + */ + case USB_REQ_SET_INTERFACE: + if (ctrl->bRequestType != USB_RECIP_INTERFACE) + goto unknown; + spin_lock(&dev->lock); + if (dev->config && w_index == 0) { + u8 config = dev->config; + + /* resets interface configuration, forgets about + * previous transaction state (queued bufs, etc) + * and re-inits endpoint state (toggle etc) + * no response queued, just zero + * status == success. + * if we had more than one interface we couldn't + * use this "reset the config" shortcut. + */ + dfu_reset_config(dev); + dfu_set_config(dev, config); + dev->altsetting = w_value; + value = 0; + } + spin_unlock(&dev->lock); + break; + case USB_REQ_GET_INTERFACE: + if (ctrl->bRequestType != + (USB_DIR_IN|USB_RECIP_INTERFACE)) + goto unknown; + if (!dev->config) + break; + if (w_index != 0) { + value = -EDOM; + break; + } + *(u8 *)req->buf = 0; + value = min(w_length, (u16) 1); + break; + + default: +unknown: + VDBG(dev, + "unknown control req%02x.%02x v%04x i%04x l%d\n", + ctrl->bRequestType, ctrl->bRequest, + w_value, w_index, w_length); + } + else + value = dfu_handle(gadget, ctrl); + + /* respond with data transfer before status phase? */ + if (value >= 0) { + req->length = value; + req->zero = value < w_length; + value = usb_ep_queue(gadget->ep0, req, GFP_ATOMIC); + if (value < 0) { + DBG(dev, "ep_queue --> %d\n", value); + req->status = 0; + } + } + + /* device either stalls (value < 0) or reports success */ + return value; +} + +/*-------------------------------------------------------------------------*/ + +static void dfu_disconnect(struct usb_gadget *gadget) +{ + struct dfu_dev *dev = get_gadget_data(gadget); + unsigned long flags; + + spin_lock_irqsave(&dev->lock, flags); + dfu_reset_config(dev); + spin_unlock_irqrestore(&dev->lock, flags); +} + +static int dfu_prepare_function(struct dfu_dev *dev, int n) +{ + struct usb_interface_descriptor *d; + int i = 0; + + dev->function = kzalloc((ALTSETTING_BASE + n + 1) * + sizeof(struct usb_descriptor_header *), + GFP_KERNEL); + if (!dev->function) + goto enomem; + + dev->function[0] = (struct usb_descriptor_header *)&otg_descriptor; + dev->function[1] = (struct usb_descriptor_header *)&dfu_func; + + for (i = 0; i < n; ++i) { + d = kzalloc(sizeof(*d), GFP_KERNEL); + if (!d) + goto enomem; + + d->bLength = sizeof(*d); + d->bDescriptorType = USB_DT_INTERFACE; + d->bAlternateSetting = i; + d->bNumEndpoints = 0; + d->bInterfaceClass = USB_CLASS_APP_SPEC; + d->bInterfaceSubClass = 1; + d->bInterfaceProtocol = 2; + d->iInterface = DFU_STR_BASE + i; + + dev->function[ALTSETTING_BASE + i] = + (struct usb_descriptor_header *)d; + } + dev->function[ALTSETTING_BASE + i] = NULL; + + return 1; + +enomem: + while (i) { + kfree(dev->function[--i + ALTSETTING_BASE]); + dev->function[i + ALTSETTING_BASE] = NULL; + } + kfree(dev->function); + + return 0; +} + +static int +dfu_prepare_strings(struct dfu_dev *dev, struct flash_entity *f, int n) +{ + int i = 0; + + dev->strings = kzalloc((STRING_ALTSETTING_BASE + n + 1) * + sizeof(struct usb_string), + GFP_KERNEL); + if (!dev->strings) + goto enomem; + + dev->strings[0].id = STRING_MANUFACTURER; + dev->strings[0].s = manufacturer; + dev->strings[1].id = STRING_PRODUCT; + dev->strings[1].s = longname; + dev->strings[2].id = STRING_SERIAL; + dev->strings[2].s = serial; + dev->strings[3].id = STRING_DFU_NAME; + dev->strings[3].s = dfu_name; + + for (i = 0; i < n; ++i) { + char *s; + + dev->strings[STRING_ALTSETTING_BASE + i].id = DFU_STR_BASE + i; + s = kzalloc(strlen(f[i].name) + 1, GFP_KERNEL); + if (!s) + goto enomem; + + strcpy(s, f[i].name); + dev->strings[STRING_ALTSETTING_BASE + i].s = s; + } + dev->strings[STRING_ALTSETTING_BASE + i].id = 0; + dev->strings[STRING_ALTSETTING_BASE + i].s = NULL; + + return 1; + +enomem: + while (i) { + kfree((void *)dev->strings[--i + STRING_ALTSETTING_BASE].s); + dev->strings[i + STRING_ALTSETTING_BASE].s = NULL; + } + kfree(dev->strings); + + return 0; +} + +static void dfu_unbind(struct usb_gadget *gadget) +{ + struct dfu_dev *dev = get_gadget_data(gadget); + int i; + + DBG(dev, "unbind\n"); + + if (dev->strings) { + i = num_flash_ents; + while (i) { + kfree((void *) + dev->strings[--i + STRING_ALTSETTING_BASE].s); + dev->strings[i + STRING_ALTSETTING_BASE].s = NULL; + } + kfree(dev->strings); + } + if (dev->function) { + i = num_flash_ents; + while (i) { + kfree(dev->function[--i + ALTSETTING_BASE]); + dev->function[i + ALTSETTING_BASE] = NULL; + } + kfree(dev->function); + } + /* we've already been disconnected ... no i/o is active */ + if (dev->req) { + dev->req->length = USB_BUFSIZ; + kfree(dev->req->buf); + usb_ep_free_request(gadget->ep0, dev->req); + } + kfree(dev); + set_gadget_data(gadget, NULL); +} + +static int __init dfu_bind(struct usb_gadget *gadget) +{ + struct dfu_dev *dev; + int gcnum; + + usb_ep_autoconfig_reset(gadget); + + gcnum = usb_gadget_controller_number(gadget); + if (gcnum >= 0) + device_desc.bcdDevice = cpu_to_le16(0x0200 + gcnum); + else { + pr_warning("%s: controller '%s' not recognized\n", + shortname, gadget->name); + device_desc.bcdDevice = __constant_cpu_to_le16(0x9999); + } + + dev = kzalloc(sizeof(*dev), GFP_KERNEL); + if (!dev) + return -ENOMEM; + spin_lock_init(&dev->lock); + dev->gadget = gadget; + set_gadget_data(gadget, dev); + + dev->req = usb_ep_alloc_request(gadget->ep0, GFP_KERNEL); + if (!dev->req) + goto enomem; + dev->req->buf = kmalloc(USB_BUFSIZ, GFP_KERNEL); + if (!dev->req->buf) + goto enomem; + + dev->req->complete = empty_complete; + + device_desc.bMaxPacketSize0 = gadget->ep0->maxpacket; + + if (gadget_is_dualspeed(gadget)) + dev_qualifier.bMaxPacketSize0 = device_desc.bMaxPacketSize0; + + if (gadget_is_otg(gadget)) { + otg_descriptor.bmAttributes |= USB_OTG_HNP, + dfu_config.bmAttributes |= USB_CONFIG_ATT_WAKEUP; + } + + usb_gadget_set_selfpowered(gadget); + + dev->dfu_state = DFU_STATE_appIDLE; + dev->dfu_status = DFU_STATUS_OK; + dev->not_prepared = true; + + if (!dfu_prepare_function(dev, num_flash_ents)) + goto enomem; + + if (!dfu_prepare_strings(dev, flash_ents, num_flash_ents)) + goto enomem; + stringtab_dfu.strings = dev->strings; + + gadget->ep0->driver_data = dev; + dev->req->context = dev; + + INFO(dev, "%s, version: " DRIVER_VERSION "\n", longname); + + /* snprintf(manufacturer, sizeof manufacturer, "%s %s with %s", + init_utsname()->sysname, init_utsname()->release, + gadget->name); */ + + return 0; + +enomem: + dfu_unbind(gadget); + return -ENOMEM; +} + +static void dfu_suspend(struct usb_gadget *gadget) +{ + if (gadget->speed == USB_SPEED_UNKNOWN) + return; + + DBG(dev, "suspend\n"); +} + +static void dfu_resume(struct usb_gadget *gadget) +{ + DBG(dev, "resume\n"); +} + +static struct usb_gadget_driver dfu_driver = { +#ifdef CONFIG_USB_GADGET_DUALSPEED + .speed = USB_SPEED_HIGH, +#else + .speed = USB_SPEED_FULL, +#endif + /*.function = (char *) longname,*/ + .bind = dfu_bind, + .unbind = __exit_p(dfu_unbind), + + .setup = dfu_setup, + .disconnect = dfu_disconnect, + + .suspend = dfu_suspend, + .resume = dfu_resume, + + /* + .driver = { + .name = (char *) shortname, + .owner = THIS_MODULE, + },*/ +}; + +void register_flash_entities(struct flash_entity *flents, int n) +{ + flash_ents = flents; + num_flash_ents = n; +} + +int __init dfu_init(void) +{ + return usb_gadget_register_driver(&dfu_driver); +} +module_init(dfu_init); + +void __exit dfu_cleanup(void) +{ + usb_gadget_unregister_driver(&dfu_driver); +} +module_exit(cleanup); + 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@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 + */ + +#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" + +/* Thanks to NetChip Technologies for donating this product ID. */ +#define DRIVER_VENDOR_NUM 0x0525 /* NetChip */ +#define DRIVER_PRODUCT_NUM 0xffff /* DFU */ + +#define STRING_MANUFACTURER 0 +#define STRING_PRODUCT 1 +#define STRING_SERIAL 2 +#define STRING_DFU_NAME 49 +#define DFU_STR_BASE 50 + +#define DFU_CONFIG_VAL 1 +#define DFU_DT_FUNC 0x21 + +#define DFU_BIT_WILL_DETACH (0x1 << 3) +#define DFU_BIT_MANIFESTATION_TOLERANT (0x1 << 2) +#define DFU_BIT_CAN_UPLOAD (0x1 << 1) +#define DFU_BIT_CAN_DNLOAD 0x1 + +/* big enough to hold our biggest descriptor */ +#define USB_BUFSIZ 4096 + +#define USB_REQ_DFU_DETACH 0x00 +#define USB_REQ_DFU_DNLOAD 0x01 +#define USB_REQ_DFU_UPLOAD 0x02 +#define USB_REQ_DFU_GETSTATUS 0x03 +#define USB_REQ_DFU_CLRSTATUS 0x04 +#define USB_REQ_DFU_GETSTATE 0x05 +#define USB_REQ_DFU_ABORT 0x06 + +#define DFU_STATUS_OK 0x00 +#define DFU_STATUS_errTARGET 0x01 +#define DFU_STATUS_errFILE 0x02 +#define DFU_STATUS_errWRITE 0x03 +#define DFU_STATUS_errERASE 0x04 +#define DFU_STATUS_errCHECK_ERASED 0x05 +#define DFU_STATUS_errPROG 0x06 +#define DFU_STATUS_errVERIFY 0x07 +#define DFU_STATUS_errADDRESS 0x08 +#define DFU_STATUS_errNOTDONE 0x09 +#define DFU_STATUS_errFIRMWARE 0x0a +#define DFU_STATUS_errVENDOR 0x0b +#define DFU_STATUS_errUSBR 0x0c +#define DFU_STATUS_errPOR 0x0d +#define DFU_STATUS_errUNKNOWN 0x0e +#define DFU_STATUS_errSTALLEDPKT 0x0f + +#define RET_STALL -1 +#define RET_ZLP 0 +#define RET_STAT_LEN 6 + +#define ALTSETTING_BASE 2 +#define STRING_ALTSETTING_BASE 4 + +enum dfu_state { + DFU_STATE_appIDLE = 0, + DFU_STATE_appDETACH = 1, + DFU_STATE_dfuIDLE = 2, + DFU_STATE_dfuDNLOAD_SYNC = 3, + DFU_STATE_dfuDNBUSY = 4, + DFU_STATE_dfuDNLOAD_IDLE = 5, + DFU_STATE_dfuMANIFEST_SYNC = 6, + DFU_STATE_dfuMANIFEST = 7, + DFU_STATE_dfuMANIFEST_WAIT_RST = 8, + DFU_STATE_dfuUPLOAD_IDLE = 9, + DFU_STATE_dfuERROR = 10, +}; + +struct dfu_status { + __u8 bStatus; + __u8 bwPollTimeout[3]; + __u8 bState; + __u8 iString; +} __packed; + +struct dfu_function_descriptor { + __u8 bLength; + __u8 bDescriptorType; + __u8 bmAttributes; + __le16 wDetachTimeOut; + __le16 wTransferSize; + __le16 bcdDFUVersion; +} __packed; + +struct dfu_dev { + spinlock_t lock; + struct usb_gadget *gadget; + struct usb_request *req; /* for control responses */ + + /* when configured, we have one config */ + u8 config; + u8 altsetting; + enum dfu_state dfu_state; + unsigned int dfu_status; + struct usb_descriptor_header **function; + struct usb_string *strings; + bool not_prepared; +}; + +#endif /* DFU_H_ */ diff --git a/include/configs/s5p_goni.h b/include/configs/s5p_goni.h index df97802..186da76 100644 --- a/include/configs/s5p_goni.h +++ b/include/configs/s5p_goni.h @@ -86,6 +86,8 @@ #define CONFIG_CMD_ONENAND #define CONFIG_CMD_MTDPARTS #define CONFIG_CMD_MMC +#define CONFIG_CMD_FAT +#define CONFIG_CMD_DEVICE_FIRMWARE_UPGRADE
#define CONFIG_BOOTDELAY 1 #define CONFIG_ZERO_BOOTDELAY_CHECK @@ -241,4 +243,8 @@ #define CONFIG_USB_GADGET_S3C_UDC_OTG 1 #define CONFIG_USB_GADGET_DUALSPEED 1
+#if defined (CONFIG_CMD_DEVICE_FIRMWARE_UPGRADE) +#define CONFIG_DFU_GADGET +#endif + #endif /* __CONFIG_H */ diff --git a/include/dfu.h b/include/dfu.h new file mode 100644 index 0000000..977ca91 --- /dev/null +++ b/include/dfu.h @@ -0,0 +1,31 @@ +/* + * dfu.h - Device Firmware Upgrade + * + * copyright (c) 2011 samsung electronics + * author: andrzej pietrasiewicz andrzej.p@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__ + +extern void board_dfu_init(void); +extern int dfu_init(void); +extern int dfu_cleanup(void); +extern int board_dfu_cleanup(void); +extern int usb_gadget_handle_interrupts(void); + +#endif diff --git a/include/flash_entity.h b/include/flash_entity.h new file mode 100644 index 0000000..daa90ee --- /dev/null +++ b/include/flash_entity.h @@ -0,0 +1,39 @@ +/* + * flash_entity.h - flashable area description + * + * Copyright (C) 2011 Samsung Electronics + * author: Andrzej Pietrasiewicz andrzej.p@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 FLASH_ENTITY_H_ +#define FLASH_ENTITY_H_ + +#define FLASH_READ 0 +#define FLASH_WRITE 1 + +struct flash_entity { + char *name; + void *ctx; + int (*prepare)(void *ctx, u8 mode); + int (*read_block)(void *ctx, unsigned int n, void *buf); + int (*write_block)(void *ctx, unsigned int n, void *buf); + int (*finish)(void *ctx, u8 mode); +}; + +void register_flash_entities(struct flash_entity *flents, int n); + +#endif /* FLASH_ENTITY_H_ */ diff --git a/include/mbr.h b/include/mbr.h new file mode 100644 index 0000000..9cbeb2e --- /dev/null +++ b/include/mbr.h @@ -0,0 +1,49 @@ +/* + * Copyright (C) 2010 Samsung Electrnoics + * + * 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 <linux/compiler.h> + +#define MBR_RESERVED_SIZE 0x8000 +#define MBR_START_OFS_BLK (0x500000 / 512) + +struct mbr_partition { + char status; + char f_chs[3]; + char partition_type; + char l_chs[3]; + int lba; + int nsectors; +} __packed; + +struct mbr { + char code_area[440]; + char disk_signature[4]; + char nulls[2]; + struct mbr_partition parts[4]; + unsigned short signature; +}; + +extern unsigned int mbr_offset[16]; +extern unsigned int mbr_parts; + +void set_mbr_dev(int dev); +void set_mbr_info(char *ramaddr, unsigned int len, unsigned int reserved); +void set_mbr_table(unsigned int start_addr, int parts, + unsigned int *blocks, unsigned int *part_offset); +int get_mbr_table(unsigned int *part_offset);
Hello.
This really comes as surprise to me as I'm working on exactly the same at the moment. I just realised that I only mentioned it here inside the fastboot patches thread.
I started from the old patches Harald Welte wrote for u-boot during his work for OpenMoko. I worked with him during this time at OpenMoko and I'm the current maintainer of dfu-util. (Just as background).
You seem to have started from scratch with this DFU implementation instead of using the older but available patches. Was there a specific reason for this?
I found it not that complicated to update them on a current u-boot and adjust it accordingly. I have it working here with a beagleboard. Was about to send them out next week but you have been a small tick faster here. :)
From a quick look I see some stuff missing in your implementation that
is working in mine so there is some ground for working together on this. The main question is which of both implementation to use to build up from. I will give you some feedback on the design high level aspects further below. A complete review of the code and testing on the beagleboard will hopefully happen over the next days. Testing might be problematic as you flashing part is forced to be rewritten for every board here it seems. Need to investigate further before I can really comment on this. More comment inline.
On Wed, 2011-11-02 at 11:00, Andrzej Pietrasiewicz wrote:
Have you fully implemented 1.1? With the detahc logic inside the device implementation?
The aim of DFU is to allow transferring data to/from selected areas of interest within a compliant device. In the DFU nomenclature the host->device direction is called download and the device->host direction is called upload. The areas are defined by the compliant device. In the DFU nomenclature the area of interest within the device is called an altsetting. The device is connected to the host through USB. On the host side compliant software must be used to initiate the data transfer. Example piece of such software is dfu-util package from the OpenMoko project:
Better to use the official homepage here: http://dfu-util.gnumonks.org/
It misses some examples but has the current informations while the wiki is outdated.
The DFU implementation on the device side remains in one of the two modes: the Run-Time mode and the DFU mode. The USB descriptor sets exported by the device depend on which mode is in force. While in DFU mode the device exports the descriptors corresponding to each available altsetting. An example dfu-util session when the device is in the DFU mode looks similar to this:
# dfu-util -l dfu-util - (C) 2007-2008 by OpenMoko Inc. This program is Free Software and has ABSOLUTELY NO WARRANTY
Found DFU: [0x0525:0xffff] devnum=46, cfg=0, intf=0, alt=0, name="bootldr" Found DFU: [0x0525:0xffff] devnum=46, cfg=0, intf=0, alt=1, name="kernel"
Just curious. What version of dfu-util your are using for your tests? I released 0.5 earlier today.
This u-boot implementation introduces a parameter-less dfu command. After the user finishes with dfu they can Ctrl-C to return to u-boot main menu.
Hmm, that sounds like you only implemented the DFU mode but not the run-time mode yet? No addtional DFU descripto in the normal run-time operation mode like usbtty? If yes this is a real limitation as the update would only be possible when the user has serial access to start the dfu command and then flash the device. While the original purpose of DFU was to make updating USB device easy enough for end-users as well. The implementation from OpenMoko i forward ported and cleanup has this functionality and it works well enough. Did so on the Freerunner from OM already.
In the end I would like to see different options available:
1) Entering DFU mode directly when a button is pressed on startup. An update mode if you want to call it like this.
2) Normal startup, but with DFU descripton to allow dfu-util detaching and entering the DFU mode for flashing.
3) Starting dfu mode form the command line as you did implement it.
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.
What is your reasoning behind this split? I have it working here with the normal nand_flashing routines taking care of bad blocks, etc.
I agree that there are several aspects that are board specific. Partions can be read out dynamically and the alt setting generated though. Soemthing what bothers me is the different underlaying medias that can be flashed. Right now I only cover nand, but eMMC, USB and other are valid as well. That is not covered in my implementation at all right now.
To make it clear, I'm happy to see somebody else working on this as well as my primary goal is to have good DFU support in U-Boot here not if my implementation is merged or yours. I will clean up my patches a bit more and send them in the next days. Also will go through your code to understand what and how you are doing things. From there we can go and decide how we merge our work and bring it into U-Boot mainline. Comment from Scott for nand and Remy for USB related parts will be helpful here.
Looking forward to work with you on this until we have good DFU support in U-Boot.
regards Stefan Schmidt
Hello Stefan,
On Wednesday, November 02, 2011 3:29 PM Stefan Shmidt wrote:
Have you fully implemented 1.1? With the detahc logic inside the device implementation?
As you noticed in another post, the state machine is reused.
Just curious. What version of dfu-util your are using for your tests? I released 0.5 earlier today.
I used what was available in my desktop's Ubuntu 10.04 LTS, and it happens to be version 0.1+svn :O It works, though.
Hmm, that sounds like you only implemented the DFU mode but not the run-time mode yet?
This conclusion is not correct. After issuing the dfu command my implementation begins in runtime mode. Only after using dfu-util from the host is the mode changed to DFU mode. Actually, as far as usage of dfu-util is concerned, I find it inconvenient when the device is in runtime mode, because there (at least with my version of dfu-util) is no way for the user to guess what altsettings are actually available. I personally do dfu-util -U <file> -a 0 or a similar command to kick the device into DFU mode, then dfu-util -l to discover what altsettings there are. So, it could be tempting to omit the runtime mode at all and start in DFU mode, but, to be standard compliant, I start in runtime mode. How do you discover with dfu-util what altsettings there are in the device?
While the original purpose of DFU was to make updating USB device easy enough for end-users as well.
True, that's what the standard says about the very purpose of DFU. However, pressing keys to bring the device into a specific mode is probably very much device-specific. It would be nice if we are able to come up with a generic solution which only delegates the necessary details to board-specific files.
In the end I would like to see different options available:
- Entering DFU mode directly when a button is pressed on startup. An
update mode if you want to call it like this.
Please see my comment above.
- Normal startup, but with DFU descripton to allow dfu-util detaching
and entering the DFU mode for flashing.
- Starting dfu mode form the command line as you did implement it.
I think 2 and 3 don't differ too much - it seems that enriching a setup without DFU with additional USB descriptors will do the job. But 2 and 3 probably should not be merged, though.
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.
What is your reasoning behind this split? I have it working here with the normal nand_flashing routines taking care of bad blocks, etc.
The reason is to factor out board-specific stuff from the dfu gadget implementation. However, in what I factored out probably there are parts which are generic enough not to be board-specific. Since there seems to be considerable interest in DFU implementation for u-boot, this could be reworked.
To make it clear, I'm happy to see somebody else working on this
"Whassssuuuup! --- Oh, man, we are not alone!" ;) I am happy to hear from someone else who works on this, too.
Regards,
Andrzej
Hello.
On Thu, 2011-11-03 at 09:12, Andrzej Pietrasiewicz wrote:
On Wednesday, November 02, 2011 3:29 PM Stefan Shmidt wrote:
Have you fully implemented 1.1? With the detahc logic inside the device implementation?
As you noticed in another post, the state machine is reused.
Yes, sorry about this. I was fouled into thinking you did not as it was splitted out and no mentioning of Harald copyright. Having the same state machine is a good base for further cooperation on this. :)
Just curious. What version of dfu-util your are using for your tests? I released 0.5 earlier today.
I used what was available in my desktop's Ubuntu 10.04 LTS, and it happens to be version 0.1+svn :O It works, though.
Ah, latest Ubuntu has 0.3 and hopefully they will update as well. Already pinged the maintainer.
Hmm, that sounds like you only implemented the DFU mode but not the run-time mode yet?
This conclusion is not correct.
Indeed. I stand corrected.
After issuing the dfu command my implementation begins in runtime mode. Only after using dfu-util from the host is the mode changed to DFU mode. Actually, as far as usage of dfu-util is concerned, I find it inconvenient when the device is in runtime mode, because there (at least with my version of dfu-util) is no way for the user to guess what altsettings are actually available.
Correct. Sadly this is due to the standard only allowing the functional descriptor in runtime mode. On the other hand the exposed alt settings in run-time mode would clutter the USB device settings.
I personally do dfu-util -U <file> -a 0 or a similar command to kick the device into DFU mode, then dfu-util -l to discover what altsettings there are.
I did run exactly in the same problem. :) I added a detach command to dfu-util to switch between the different modes. From run-time to dfu and from dfu to run-time. I wanted to get the release out before bringing it in. Pushed it to the master branch of dfu-util now. Feel free to test and let me know if it works for you.
So, it could be tempting to omit the runtime mode at all and start in DFU mode, but, to be standard compliant, I start in runtime mode. How do you discover with dfu-util what altsettings there are in the device?
See above. Not available in run-time mode. :/
While the original purpose of DFU was to make updating USB device easy enough for end-users as well.
True, that's what the standard says about the very purpose of DFU. However, pressing keys to bring the device into a specific mode is probably very much device-specific. It would be nice if we are able to come up with a generic solution which only delegates the necessary details to board-specific files.
Agreed. This should not be handled in the dfu implementation at all. Your split between dfu and flashing backend is a good step in the right direction here. Invoking the dfu mode through a command is also a nice addition.
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.
What is your reasoning behind this split? I have it working here with the normal nand_flashing routines taking care of bad blocks, etc.
The reason is to factor out board-specific stuff from the dfu gadget implementation. However, in what I factored out probably there are parts which are generic enough not to be board-specific. Since there seems to be considerable interest in DFU implementation for u-boot, this could be reworked.
After thinking more about it I agree with you here. The split makes sense and it will allow for more flexibility when adding more flashing backend to the implementation.
To make it clear, I'm happy to see somebody else working on this
"Whassssuuuup! --- Oh, man, we are not alone!" ;) I am happy to hear from someone else who works on this, too.
Good to know. :)
Brainstorming about a way going forward. Please comment if with your view on this.
o I will send out my not ready for mainline patch to give you and others an impression how it is tackled in my patch.
o I like your split between dfu and flashing and also the addition of the dfu command. Could you split this part from the rest and send it as self conatining patch without other dependencies?
o And then one patch with the mmc/fat flashing backend in moved out of the board file (no idea what the best place is, maybe just another file for now) and a last patch with all the board specific changes?
o I will then rebase my code on yours and prepare patches against the dfu implementations as needed and also flashing backend for NAND.
o After this we need to review was is missing and bring it over.
Not all details covered in the plan but a start. Comments?
regards Stefan Schmidt
Hello,
On Thursday, November 03, 2011 3:01 PM Stefan Schmidt wrote:
o I will send out my not ready for mainline patch to give you and others an impression how it is tackled in my patch.
o I like your split between dfu and flashing and also the addition of the dfu command. Could you split this part from the rest and send it as self conatining patch without other dependencies?
o And then one patch with the mmc/fat flashing backend in moved out of the board file (no idea what the best place is, maybe just another file for now) and a last patch with all the board specific changes?
I assume splitting into 4 parts: (1) dfu gadget, (2) board specific code, (3) flashing backend generic code, (4) dfu command.
As I today discovered, after latest merges in u-boot master the mmc subsystem in Goni does not work properly - the detected capacity is 0, and the number of available partitions is 0. This took me some time to discover. I hope the responsible person will fix this soon. So, as far as the splitting of my dfu implementation into separate patches is concerned, I think I will come up with it early next week.
o I will then rebase my code on yours and prepare patches against the dfu implementations as needed and also flashing backend for NAND.
o After this we need to review was is missing and bring it over.
Not all details covered in the plan but a start. Comments?
In the beginning I intend to split the code into respective parts, to give you something to work with. Then I intend to clean the code up, taking into account your and Mike's review.
Regards,
Andrzej
On Wednesday 02 November 2011 06:00:07 Andrzej Pietrasiewicz wrote:
Signed-off-by: Andrzej Pietrasiewicz andrzej.p@samsung.com Signed-off-by: Kyungmin Park kyungmin.park@samsung.com
Dear All,
This is Device Firmware Upgrade (DFU) implementation which supports data upload and download function to devices which are equipped with a UDC.
this information belongs in the changelog (above the "---" marker)
are you working with the elinux.org guys ? http://elinux.org/Merge_DFU_support_into_mainline_U-Boot
board/samsung/goni/Makefile | 2 + board/samsung/goni/flash.c | 634 board/samsung/goni/flash.h | 28 ++ board/samsung/goni/goni.c | 17 + common/Makefile | 1 + common/cmd_dfu.c | 50 +++ drivers/usb/gadget/Makefile | 1 + drivers/usb/gadget/dfu.c | 920 drivers/usb/gadget/dfu.h | 171 include/configs/s5p_goni.h | 6 + include/dfu.h | 31 ++ include/flash_entity.h | 39 ++ include/mbr.h | 49 +++
this should be split up into at least the dfu core and board-specific changes. although i'd wonder how much of the board/samsung/ stuff is really board specific and couldn't be generalized ...
--- /dev/null +++ b/include/dfu.h
+extern int usb_gadget_handle_interrupts(void);
this doesn't belong in the dfu header
--- /dev/null +++ b/include/flash_entity.h
+struct flash_entity {
- char *name;
should probably be const
--- /dev/null +++ b/include/mbr.h
+/*
- Copyright (C) 2010 Samsung Electrnoics
- 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 <linux/compiler.h>
missing ifdef protection against multiple inclusion
this also should be split out of the dfu core ... although it seems weird that u-boot doesn't already have mbr parsing code ... -mike
Hello.
On Wed, 2011-11-02 at 11:16, Mike Frysinger wrote:
On Wednesday 02 November 2011 06:00:07 Andrzej Pietrasiewicz wrote:
are you working with the elinux.org guys ? http://elinux.org/Merge_DFU_support_into_mainline_U-Boot
That would be me. As I stated in my other mail I was surprised seeing this patch coming in. I did not communicate my work on this broadly either. Wanted to show up with soem real code not only promises. Same as Andrzej it seems. :)
A quick comparison shows me that there are areas that don't touch each other at all (mmc and images on fat as backend vs. raw nand flash as backend) and areas that are duplicated and needed to get sorted out. The DFU protocol implementation in this case. I hope Andrzej is willing to work toegtehr with me on a final implementation that contains pieces of both versions. Mine is missing a code split between DFU protocol and flashing backend for example but feature other parts missing here.
board/samsung/goni/Makefile | 2 + board/samsung/goni/flash.c | 634 board/samsung/goni/flash.h | 28 ++ board/samsung/goni/goni.c | 17 + common/Makefile | 1 + common/cmd_dfu.c | 50 +++ drivers/usb/gadget/Makefile | 1 + drivers/usb/gadget/dfu.c | 920 drivers/usb/gadget/dfu.h | 171 include/configs/s5p_goni.h | 6 + include/dfu.h | 31 ++ include/flash_entity.h | 39 ++ include/mbr.h | 49 +++
this should be split up into at least the dfu core and board-specific changes. although i'd wonder how much of the board/samsung/ stuff is really board specific and couldn't be generalized ...
Agreed. The eMMC flashing with files on FAT is nothing goni specific. Others should be able to use this as well. I see three different parts here that can be separated:
1) The DFU protocol implementation which lives in usb gadget with an interface for flashing backends
2) The flashing backends (no idea where to put them best). At the moment that would be the implementation here with eMMC and files on FAT, mine with raw NAND devices and in the future maybe others. Each flashing backend should be generic enough to allow different boards using it.
3) Board specific information like partitions or hints for the flashing backend. The information itself should be in the board file here and only used by the flashing backends.
How does this sound to you? Andrzej?
regards Stefan Schmidt
Hello Stefan,
On Wednesday, November 02, 2011 8:30 PM Stefan Schmidt writes:
Agreed. The eMMC flashing with files on FAT is nothing goni specific. Others should be able to use this as well. I see three different parts here that can be separated:
I agree. Since there is interest in DFU implementation this can and should be generalized.
- The DFU protocol implementation which lives in usb gadget with an
interface for flashing backends
- The flashing backends (no idea where to put them best). At the
moment that would be the implementation here with eMMC and files on FAT, mine with raw NAND devices and in the future maybe others. Each flashing backend should be generic enough to allow different boards using it.
- Board specific information like partitions or hints for the flashing
backend. The information itself should be in the board file here and only used by the flashing backends.
How does this sound to you? Andrzej?
Sounds good to me. In my implementation the interface between dfu gadget and flashing backend is around the struct flash_entity. It contains a character string intended to provide a human-readable name, a void * context which is not interpreted by the gadget code, but is passed to the flashing backend and understood by it. The struct flash_entity also contains prepare-finish methods to be called before and after read/write operations, and the read_block- write_block pair. What do you think?
As far as generalization is concerned, in my flashing backend implementation I see these parts as candidates for generalization:
1) mbr/ebr reading 2) reading/writing mmc 3) read/write fat 4) generic prepare/finish; not sure if fat-specific prepare can be generalized 5) read/write_block 6) some more work should be put to create an interface between the board initialization routine, the flashing backend and the DFU gadget implementation. In my implementation the board initialization routine calls board-specific register_flash_areas, which, in turn, calls DFU gadget's register_flash_entities. What's your opinion?
Thanks,
Andrzej
Hello.
On Thu, 2011-11-03 at 09:44, Andrzej Pietrasiewicz wrote:
On Wednesday, November 02, 2011 8:30 PM Stefan Schmidt writes:
Agreed. The eMMC flashing with files on FAT is nothing goni specific. Others should be able to use this as well. I see three different parts here that can be separated:
I agree. Since there is interest in DFU implementation this can and should be generalized.
Great!
- The DFU protocol implementation which lives in usb gadget with an
interface for flashing backends
- The flashing backends (no idea where to put them best). At the
moment that would be the implementation here with eMMC and files on FAT, mine with raw NAND devices and in the future maybe others. Each flashing backend should be generic enough to allow different boards using it.
- Board specific information like partitions or hints for the flashing
backend. The information itself should be in the board file here and only used by the flashing backends.
How does this sound to you? Andrzej?
Sounds good to me. In my implementation the interface between dfu gadget and flashing backend is around the struct flash_entity. It contains a character string intended to provide a human-readable name, a void * context which is not interpreted by the gadget code, but is passed to the flashing backend and understood by it. The struct flash_entity also contains prepare-finish methods to be called before and after read/write operations, and the read_block- write_block pair. What do you think?
I would need to use it for my NAND backend before I really can comment on it. I can only tell if I'm happy with and interface if I actually used it. :)
Will do this when you send it a separate patch with only the DFU implementation with the flash entity as interface. See my roadmap proposal in the other mail.
As far as generalization is concerned, in my flashing backend implementation I see these parts as candidates for generalization:
- mbr/ebr reading
- reading/writing mmc
- read/write fat
Agreed. That can be used by all kind of devices.
- generic prepare/finish; not sure if fat-specific prepare can be
generalized
Would be good to get soem comments on the fs custodians around here.
- read/write_block
Agreed.
- some more work should be put to create an interface between the board
initialization routine, the flashing backend and the DFU gadget implementation. In my implementation the board initialization routine calls board-specific register_flash_areas, which, in turn, calls DFU gadget's register_flash_entities. What's your opinion?
The interface between the configuration in the board file and the actual flashing backend is something I haven't wraped my mind around yet. Need to think about it.
regards Stefan Schmidt
Hello Mike,
Thank you for your review. Please see my comments inline.
On Wednesday, November 02, 2011 4:16 PM Mike Frysinger wrote:
Dear All,
This is Device Firmware Upgrade (DFU) implementation which supports data upload and download function to devices which are equipped with
a UDC.
this information belongs in the changelog (above the "---" marker)
I generally agree to remarks related to coding style and implementation's distribution into respective patches.
are you working with the elinux.org guys ? http://elinux.org/Merge_DFU_support_into_mainline_U-Boot
That's not me.
this should be split up into at least the dfu core and board-specific changes. although i'd wonder how much of the board/samsung/ stuff is really board specific and couldn't be generalized ...
You are right, probably the "flashing backend" part contains some code which can be generalized.
Regards,
Andrzej
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@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@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@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
Hello Stefan,
Thank you for your review. Please see comments inline.
On Wednesday, November 02, 2011 9:07 PM Stefan Schmidt wrote:
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?
Sorry about that. I forgot to mention the reference. It is http://patchwork.ozlabs.org/patch/122080
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!
Comments sent in other mails;) I agree that since there is interest in implementing DFU in u-boot the code which in fact is not board specific can and should be generalized. We need to think of an appropriate place in the tree to store it, though.
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.
ACK. No problem with changing that.
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.
Please see my comment in the other mail. A generic way to handle "a button pressed" seems necessary. Actually in my opinion just providing the "dfu" command is enough. If, in some hardware, initiating the runtime mode is desired by pressing some keys, then the board initialization code should check for such an event and if it happens then just programmatically run the dfu command. What do you think?
+/* #include "usbstring.c" */
Not needed, can be removed.
I agree to coding style/reduntant code comments; will not specifically comment on them unless I disagree.
+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.
It seems it does not. The dfu-util session says "Resetting USB", so I assume that resetting is initiated by the host, not by the device. To be removed.
+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?
Yes and no - depends on whether we want to version the gadget itself or the flashing backend, or both. As probably the last option is best, the answer is probably "yes". We need some interface then.
+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?
I think this is over-interpretation of what I wrote in introduction. I gave an example dfu-util session when the device is already in DFU mode. I didn't say that it only has DFU mode. It has both.
- /* 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.
Not sure what to do at this moment, either.
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 everyUSB
* bus reset to switch back to runtime mode, since
at
* least the Linux USB stack likes to send a numberof
* 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.
You are absolutely right. My gadget implementation is also based on Gadget Zero from Linux and I did mention that, but missed including appropriate information regarding the DFU state machine.
+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?
I think that the gadget framework works well and can be successfully reused in u-boot. What other interface would you see to talk to UDC? Also, USB mass storage implementation can be easy done with usb gadget.
Actually, the original intention was to implement the dfu both in u-boot and in kernel. I did post my kernel implementation to linux-usb and did give example of being useful. Even though this kind of a driver could seem controversial, there are other drivers based on similar philosophy which are in the mainline kernel. Anyway, Felipe Balbi, the usb maintainer in Linux, did not approve because he says this kind of things belongs to userspace. Still, I see the gadget framework suitable to do the job for us here.
+/*
- 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.
Please see the comment above. In such circumstances, we probably don't.
+#define DRIVER_VERSION "Marzanna"
Maybe that helps you internal but it does not make much sense for people outside Samsung. :)
The code I started to work on had an equally nice name here, which was like "Victory Day" or something. "Marzanna" is one of female names in Polish and it was Marzanna's name day when I started work on dfu. Self explanatory ;)
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.
Absolutely yes.
How would you like to proceed in getting our stuff merged and finally integrated into mainline?
You mentioned that your DFU implementation does things which mine doesn't. Can you please tell something more? As far as I understand what you wrote, there is something in your DFU implementation which does more, and also we use different media as backends.
Thank you,
Andrzej
Hello.
On Thu, 2011-11-03 at 11:33, Andrzej Pietrasiewicz wrote:
On Wednesday, November 02, 2011 9:07 PM Stefan Schmidt wrote:
On Wed, 2011-11-02 at 11:00, Andrzej Pietrasiewicz wrote:
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.
ACK. No problem with changing that.
OK
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.
Please see my comment in the other mail. A generic way to handle "a button pressed" seems necessary. Actually in my opinion just providing the "dfu" command is enough. If, in some hardware, initiating the runtime mode is desired by pressing some keys, then the board initialization code should check for such an event and if it happens then just programmatically run the dfu command. What do you think?
Lets go with your dfu implementation and the dfu command. I will prepare patches against is if I see something missing from my patch.
+/* #include "usbstring.c" */
Not needed, can be removed.
I agree to coding style/reduntant code comments; will not specifically comment on them unless I disagree.
Thanks.
+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.
It seems it does not. The dfu-util session says "Resetting USB", so I assume that resetting is initiated by the host, not by the device. To be removed.
So far I also put this aside. Its only a 1.1 feature which can be added with another patch when the basic stuff is working and merged.
+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?
Yes and no - depends on whether we want to version the gadget itself or the flashing backend, or both. As probably the last option is best, the answer is probably "yes". We need some interface then.
As I already stated this part is not worked out in my head yet. Feel free to propose something or we will fix it while working on it.
+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?
I think this is over-interpretation of what I wrote in introduction. I gave an example dfu-util session when the device is already in DFU mode. I didn't say that it only has DFU mode. It has both.
Yeah, sorry for that. I indeed misread this part of your implementation.
- /* 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.
Not sure what to do at this moment, either.
If we don't have a back channel from the actual flashing subsystem we need to oriantate the values on the hardware spec on own measurements I fear. Soemthing that need to be set in the board file then.
case USB_REQ_DFU_DETACH:/* Proprietary extension: 'detach' from idle modeand
* get back to runtime mode in case of USB Reset.As
* much as I dislike this, we just can't use everyUSB
* bus reset to switch back to runtime mode, sinceat
* least the Linux USB stack likes to send a numberof
* 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.
You are absolutely right. My gadget implementation is also based on Gadget Zero from Linux and I did mention that, but missed including appropriate information regarding the DFU state machine.
OK. Would be good to add this for the next round.
+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?
I think that the gadget framework works well and can be successfully reused in u-boot. What other interface would you see to talk to UDC? Also, USB mass storage implementation can be easy done with usb gadget.
I don't wanted to argue against a good gadget framework. I was just wondering if it already is in mainline and if not what is blocking it.
My patch has some additions to usbtty for the runtime descriptor to show up and all other parts are handled over ep0. The gadget framework would make this easier and better abstracted to work toegther with different usb gadget drivers. The only fear I have is to depend on soemthing not yet in u-boot mainline will make it harder to get the dfu implementation merged.
+#define DRIVER_VERSION "Marzanna"
Maybe that helps you internal but it does not make much sense for people outside Samsung. :)
The code I started to work on had an equally nice name here, which was like "Victory Day" or something. "Marzanna" is one of female names in Polish and it was Marzanna's name day when I started work on dfu. Self explanatory ;)
Hehe :)
How would you like to proceed in getting our stuff merged and finally integrated into mainline?
You mentioned that your DFU implementation does things which mine doesn't. Can you please tell something more? As far as I understand what you wrote, there is something in your DFU implementation which does more, and also we use different media as backends.
One thin was the runtime and dfu mode thing. I was wrong there as yours also do both. For the flashing backend I have the alt settings dynamically generated from available mtdparts and use the new nand write functions skipping over bad blocks. For the core DFU protocol I'm not sure about the differences yet. But as I said, prepare I stand alone patch of yours and I will add send patches to add missing stuff of the DFU implementation, if any.
My patch should be on the list later today or tomorrow to let you see what I have.
regards Stefan Schmidt
On Thursday, November 03, 2011 11:33 AM Andrzej Pietrasiewicz wrote:
Sorry about that. I forgot to mention the reference. It is http://patchwork.ozlabs.org/patch/122080
Sorry again. I meant http://patchwork.ozlabs.org/patch/122079
Andrzej
Hello.
On Thu, 2011-11-03 at 13:47, Andrzej Pietrasiewicz wrote:
On Thursday, November 03, 2011 11:33 AM Andrzej Pietrasiewicz wrote:
Sorry about that. I forgot to mention the reference. It is http://patchwork.ozlabs.org/patch/122080
Sorry again. I meant http://patchwork.ozlabs.org/patch/122079
Hmm, applied this one but the DFU patch stills fails to apply in goni.c and goni.h. Seems there is something else applied that is not in mainline yet.
I can't test the goni part anyway as I don't have the hardware so I will split it out of the patch and test the dfu command and its implementation.
regards Stefan Schmidt
Hello Stefan,
On Thursday, November 03, 2011 2:32 PM Stefan Schmidt wrote:
Sorry about that. I forgot to mention the reference. It is http://patchwork.ozlabs.org/patch/122080
Sorry again. I meant http://patchwork.ozlabs.org/patch/11122079
Hmm, applied this one but the DFU patch stills fails to apply in goni.c and goni.h. Seems there is something else applied that is not in mainline yet.
Ok, here is the full story: this http://patchwork.ozlabs.org/patch/11122079 is the second patch in a series, the first being http://patchwork.ozlabs.org/patch/122080 and you actually need both.
I pulled the latest git.denx.de tree and had to also apply a patch which Defines CONFIG_SYS_CACHELINE_SIZE for Goni target. It can be found here: http://article.gmane.org/gmane.comp.boot-loaders.u-boot/112755/match= I know this is confusing. Sorry about that.
Andrzej
Dear Stefan Schmidt,
In message 20111102200717.GP17069@excalibur.local you wrote:
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.
Yes, a command is the natural way in U-Boot to start any activities.
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
This is just another way to start a command.
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.
NAK. Let's stick to standard interfaces.
Best regards,
Wolfgang Denk
Hello.
On Sat, 2011-11-05 at 16:31, Wolfgang Denk wrote:
In message 20111102200717.GP17069@excalibur.local you wrote:
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.
Yes, a command is the natural way in U-Boot to start any activities.
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
This is just another way to start a command.
So bounding a key press to the command would be the way to achieve what some people want. Fine with me.
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.
NAK. Let's stick to standard interfaces.
What do you mean here? You don't want the DFU descripto available with USBTTY or you don't want to start DFU directly in DFU mode?
regards Stefan Schmidt
Dear Stefan Schmidt,
In message 20111106163605.GA20104@excalibur.local you wrote:
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.
NAK. Let's stick to standard interfaces.
What do you mean here? You don't want the DFU descripto available with USBTTY or you don't want to start DFU directly in DFU mode?
I don't want random transfers of control between unrelated sub-systems. If you want to run a command, then run a command.
Best regards,
Wolfgang Denk
Dear Andrzej Pietrasiewicz,
In message 1320228007-8947-1-git-send-email-andrzej.p@samsung.com you wrote:
Device Firmware Upgrade (DFU) is an extension to the USB specification. As of the time of this writing it is documented at
...
Please refer to dfu-util documentation for details. The below ASCII-art presents a connection between the host and the device.
+-----------------+ <--- DFU ---> +-------------------------+ | e.g. dfu-util | <=== USB ===> | / altsetting 0 | | host +---------------------+ device - altsetting 1 | | file / | | \ ... | +-----------------+ +-------------------------+
Sorry for asking some probably really dumb questions...
If I understand correctly, there is no fundamental reason why the DFU protocol would only be possible to implement over a USB link. Or am I missing something here?
Eventually it should be possible to run this protocol over Ethernet or even over a serial line?
If my assumption is correct, then what would it take to split off protocol part and make it independent of the actual driver interface?
Best regards,
Wolfgang Denk
Dear Wolfgang Denk,
Please see my comments inline.
On Thursday, November 03, 2011 7:14 PM Wolfgang Denk wrote:
...
DFU is part of USB; an extension to be precise, but an extension bound so tightly to the design and philosophy of USB that it is rather inconceivable to separate the two.
If I understand correctly, there is no fundamental reason why the DFU protocol would only be possible to implement over a USB link. Or am I missing something here?
Eventually it should be possible to run this protocol over Ethernet or even over a serial line?
Of course there is no such a reason, provided we lay USB over Ethernet or serial line first ;) Seriously speaking, in view of ties between DFU and USB IMHO it is impossible, or, at least, highly impractical.
If my assumption is correct, then what would it take to split off protocol part and make it independent of the actual driver interface?
I guess that in the situation given it would be of little use.
Regards,
Andrzej
Dear Andrzej Pietrasiewicz,
In message 000601cc9abe$4f544bd0$edfce370$%p@samsung.com you wrote:
DFU is part of USB; an extension to be precise, but an extension bound so tightly to the design and philosophy of USB that it is rather inconceivable to separate the two.
Could you please be so kind and explain which exact issues you see for such a separation?
Eventually it should be possible to run this protocol over Ethernet or even over a serial line?
Of course there is no such a reason, provided we lay USB over Ethernet or serial line first ;)
This is of course not intended. I was thinking about a plain standard UDP based link.
Seriously speaking, in view of ties between DFU and USB IMHO it is impossible, or, at least, highly impractical.
Can you please support this statement with a few facts?
If my assumption is correct, then what would it take to split off protocol part and make it independent of the actual driver interface?
I guess that in the situation given it would be of little use.
What do you think would be of little use?
Best regards,
Wolfgang Denk
Hello.
On Sat, 2011-11-05 at 16:33, Wolfgang Denk wrote:
In message 000601cc9abe$4f544bd0$edfce370$%p@samsung.com you wrote:
DFU is part of USB; an extension to be precise, but an extension bound so tightly to the design and philosophy of USB that it is rather inconceivable to separate the two.
Could you please be so kind and explain which exact issues you see for such a separation?
As Andrzej pointed out the DFU spec is written by the USB forum and one can see that there target are USB devices. Let me bring in some facts from the spec.
One part that is not covered here is the DFU suffix mandatory for all firmware files. It gets stripped from the flashing tool before sending it to the device code and thus it was not covered here so far. This suffix holds, among others, fields with the PID of VID of the USB device. Only allowing to be flashed if the device has the correct IDs. How would assign the USB IDs to a network based approach for example?
The core part, I understand, is the DFU state machine you are referring to. This sate machine relies on the USB descriptors in run-time or DFU mode. It is hardcoded wot USB descriptors using bDeviceClass, bDeviceSubClass, idVendor and idProduct to name a few. (See p. 14, 15 of the 1.1 spec).
In the reconfiguration phase the spec mandates the host to issue a DFU_DETACH request on the USB EP0 and then issuing an USB reset. (p. 17)
All bound to the USB spec and hardware for using the spec in a standard compliant way.
Eventually it should be possible to run this protocol over Ethernet or even over a serial line?
Of course there is no such a reason, provided we lay USB over Ethernet or serial line first ;)
This is of course not intended. I was thinking about a plain standard UDP based link.
As always in computers one could try to come up with solutions to replace the USB hardcoded parts with something else for a UDP based solution. Its just now longer compliant with the DFU spec. All DFU flashing utils out there are only working over USB. All the windows flash utils as well as dfu-programmer, a small dfu util from the bluez project and also dfu-util. The last ones are using libusb for this.
The main point is that if you replace the USB related parts with something else not much of DFU stays anymore. It would be a different flashing procedure. And for systems with a network interface available there are way better ways to flash it then DFU. Even TFP would be more convenient for them. You are way more flexible with Ethernet here.
DFU was designed for really small devices with minimal software originally. Best examples are Bluetooth and WiFi USB dongles which can be flashed this way if they. Its just that it becomes more popular as many consumer electronics device are coming with a USB port but no Ethernet these days.
regards Stefan Schmidt
Dear Stefan Schmidt,
In message 20111106170219.GB20104@excalibur.local you wrote:
Could you please be so kind and explain which exact issues you see for such a separation?
As Andrzej pointed out the DFU spec is written by the USB forum and one can see that there target are USB devices. Let me bring in some facts from the spec.
...
All bound to the USB spec and hardware for using the spec in a standard compliant way.
Yes, indeed.
But I feel you stick way too close to this spec, and are not trying to abstract awway the low level details a bit.
Yes, this spec was written by the USB forum, and they did not even attempt to look over the rim of their plates. But does that mean we all have to do the same? I'm not willing to accept such a narrow view.
Let's - just as a gedankenexperiment - assume our task was to implement a solution that provides similar capabilities as DFU, but must work over serial line, Ethernet, Bluetooth, Infrared, whatever. Assume we thing DFU could be used as base, but we need to find a way to move the USB related things into the USB layer, while things like the state machine, the requests, device status, state transitions, etc. are generic enough.
Yes, the DFU File Suffix contains fields that map nicely to USB devices - but what prevents us to provide similar data on non-USB media, too? We just need to make sure that th code which handles this allows to plug in handlers for other transfer media.
As always in computers one could try to come up with solutions to replace the USB hardcoded parts with something else for a UDP based solution. Its just now longer compliant with the DFU spec. All DFU
I'm not sure what exactly you want to tell me here.
My idea is that of an implementation that is split into a generic and a device specific part. If implemented corectly, the combination of the generic part with the USB specific part would be strictly conformant to the DFU spec.
Of course, the combination of for example the DFU generic part with an Ethernet network interface would not be conformant to the DFU spec, but it would be something that works in a very similar way. And that would most probably be better than inventing yet another download method.
flashing utils out there are only working over USB. All the windows flash utils as well as dfu-programmer, a small dfu util from the bluez project and also dfu-util. The last ones are using libusb for this.
That's the status quo. Agreed. Bit this can be changed, extended. Or not?
The main point is that if you replace the USB related parts with something else not much of DFU stays anymore. It would be a different
I disagree here. DFU is different in a number of aspects, and you know this very well.
flashing procedure. And for systems with a network interface available there are way better ways to flash it then DFU. Even TFP would be more convenient for them. You are way more flexible with Ethernet here.
I'm not sure what your exact argument is here. I'm not the one asking for DFU support. I would be happy to use TFTP over Ethenret over USB. We're actually doing this in some products.
DFU was designed for really small devices with minimal software originally. Best examples are Bluetooth and WiFi USB dongles which can be flashed this way if they. Its just that it becomes more popular as many consumer electronics device are coming with a USB port but no Ethernet these days.
Agreed. And if it's becoming more popular, it might even be a good idea to think about a less restricted implementation, which opens new oportunites.
Yes, I think we should strive for such a separation of USB transport layer and protocol implementation / state machine / command interface.
Best regards,
Wolfgang Denk
Dear Wolfgang Denk,
Please see my comments inline.
DFU is part of USB; an extension to be precise, but an extension
bound
so tightly to the design and philosophy of USB that it is rather inconceivable to separate the two.
Could you please be so kind and explain which exact issues you see for such a separation?
First of all, I think that it is more proper to speak of communications part and state machine part. They are bound together by the standard. For example, in the dfuIDLE state, the receipt of DFU_DNLOAD request with wLength == 0 triggers a "device stalls the control pipe" action. This kind of stuff is very USB specific - I am not sure what would "the control pipe" be for serial/Ethernet, nor what "stall the control pipe" would mean for either of them. Another example, in the dfuMANIFEST state, when the status poll timeout occurs and bitManifestationTolerant == 1 then "device can still communicate via the USB".
The communications part is of course the very USB itself, so the only thing which could be extracted is the state machine part. But, then, in order not to be bound to USB but reusable, it should be abstract. Being abstract, it would not be DFU anymore, but some "Generic Update Protocol" that you have on your mind, but we don't know of. Is there any standard for such a protocol?
Seriously speaking, in view of ties between DFU and USB IMHO it is impossible, or, at least, highly impractical.
Can you please support this statement with a few facts?
Please see my comment above.
If my assumption is correct, then what would it take to split off protocol part and make it independent of the actual driver
interface?
I guess that in the situation given it would be of little use.
What do you think would be of little use?
As far as I understand you correctly, you would like the state machine code extracted from the initial DFU implementation posted on this mailing list. And then you would like the DFU implemented in terms of this abstract state machine. This means actually more code, probably not a desired thing. The generic state machine would also require some accompanying generic data structures, and this would also mean code to translate back and forth between USB/DFU (DFU _is_ USB) and state machine's data structures. All in all, this means adding more complexity and code. This looks like a premature optimization (which is widely known to be the root of all evil), while at the moment, there are no use cases to justify it. Should the use cases appear, the code can be reworked based on the needs of both USB/DFU and new state machine users.
Regards,
Andrzej
Hi Andrzej,
[...]
If my assumption is correct, then what would it take to split off protocol part and make it independent of the actual driver
interface?
I guess that in the situation given it would be of little use.
What do you think would be of little use?
As far as I understand you correctly, you would like the state machine code extracted from the initial DFU implementation posted on this mailing list. And then you would like the DFU implemented in terms of this abstract state machine. This means actually more code, probably not a desired thing. The generic state machine would also require some accompanying generic data structures, and this would also mean code to translate back and forth between USB/DFU (DFU _is_ USB) and state machine's data structures. All in all, this means adding more complexity and code. This looks like a premature optimization (which is widely known to be the root of all evil), while at the moment, there are no use cases to justify it. Should the use cases appear, the code can be reworked based on the needs of both USB/DFU and new state machine users.
Just to add my personal opinion here - I am occassionally lobbying for a long time on this mailing list that mainline U-Boot gains DFU support, so please let's get this working with the existing tools and in the existing contexts, i.e. USB. Personally I consider it to be completely out of scope to dis-entangle DFU from its specification and abstract it into something which it _could_ be, but really _isn't_.
When the implementation for USB is here, we all have much better grounds to discuss possible generalizations (although I doubt that there are many).
Cheers Detlev
participants (5)
-
Andrzej Pietrasiewicz -
Detlev Zundel -
Mike Frysinger -
Stefan Schmidt -
Wolfgang Denk