From a2199afea169c1e13881ca90a02a28e4c9ffd114 Mon Sep 17 00:00:00 2001 From: Heiko Schocher Date: Tue, 18 Mar 2014 08:09:55 +0100 Subject: [PATCH 1/4] usb, dfu: extract flush code into seperate function move the flushing code into an extra function dfu_flush(), so it can be used from other code. Signed-off-by: Heiko Schocher Cc: Lukasz Majewski Cc: Kyungmin Park Cc: Marek Vasut Cc: Pantelis Antoniou --- drivers/dfu/dfu.c | 42 ++++++++++++++++++++++++------------------ include/dfu.h | 1 + 2 files changed, 25 insertions(+), 18 deletions(-) diff --git a/drivers/dfu/dfu.c b/drivers/dfu/dfu.c index 07011e9..1aced26 100644 --- a/drivers/dfu/dfu.c +++ b/drivers/dfu/dfu.c @@ -126,6 +126,28 @@ static int dfu_write_buffer_drain(struct dfu_entity *dfu) return ret; } +int dfu_flush(struct dfu_entity *dfu, void *buf, int size, int blk_seq_num) +{ + int ret = 0; + + if (dfu->flush_medium) + ret = dfu->flush_medium(dfu); + + printf("\nDFU complete CRC32: 0x%08x\n", dfu->crc); + + /* clear everything */ + dfu_free_buf(); + dfu->crc = 0; + dfu->offset = 0; + dfu->i_blk_seq_num = 0; + dfu->i_buf_start = dfu_buf; + dfu->i_buf_end = dfu_buf; + dfu->i_buf = dfu->i_buf_start; + dfu->inited = 0; + + return ret; +} + int dfu_write(struct dfu_entity *dfu, void *buf, int size, int blk_seq_num) { int ret = 0; @@ -197,24 +219,8 @@ int dfu_write(struct dfu_entity *dfu, void *buf, int size, int blk_seq_num) } /* end? */ - if (size == 0) { - /* Now try and flush to the medium if needed. */ - if (dfu->flush_medium) - ret = dfu->flush_medium(dfu); - printf("\nDFU complete CRC32: 0x%08x\n", dfu->crc); - - /* clear everything */ - dfu_free_buf(); - dfu->crc = 0; - dfu->offset = 0; - dfu->i_blk_seq_num = 0; - dfu->i_buf_start = dfu_buf; - dfu->i_buf_end = dfu_buf; - dfu->i_buf = dfu->i_buf_start; - - dfu->inited = 0; - - } + if (size == 0) + ret = dfu_flush(dfu, buf, size, blk_seq_num); return ret = 0 ? size : ret; } diff --git a/include/dfu.h b/include/dfu.h index f973426..272a245 100644 --- a/include/dfu.h +++ b/include/dfu.h @@ -138,6 +138,7 @@ unsigned long dfu_get_buf_size(void); int dfu_read(struct dfu_entity *de, void *buf, int size, int blk_seq_num); int dfu_write(struct dfu_entity *de, void *buf, int size, int blk_seq_num); +int dfu_flush(struct dfu_entity *de, void *buf, int size, int blk_seq_num); /* Device specific */ #ifdef CONFIG_DFU_MMC extern int dfu_fill_entity_mmc(struct dfu_entity *dfu, char *s); From 001a8319866062cd63b7b62e5bb4b85eacfde4b4 Mon Sep 17 00:00:00 2001 From: Heiko Schocher Date: Tue, 18 Mar 2014 08:09:56 +0100 Subject: [PATCH 2/4] usb: dfu: introduce dfuMANIFEST state on nand flash using ubi, after the download of the new image into the flash, the "rest" of the nand sectors get erased while flushing the medium. With current u-boot version dfu-util may show: Starting download: [##################################################] finished! state(7) = dfuMANIFEST, status(0) = No error condition is present unable to read DFU status as get_status is not answered while erasing sectors, if erasing needs some time. So do the following changes to prevent this: - introduce dfuManifest state According to dfu specification ( http://www.usb.org/developers/devclass_docs/usbdfu10.pdf ) section 7: "the device enters the dfuMANIFEST-SYNC state and awaits the solicitation of the status report by the host. Upon receipt of the anticipated DFU_GETSTATUS, the device enters the dfuMANIFEST state, where it completes its reprogramming operations." - when stepping into dfuManifest state, sending a PollTimeout DFU_MANIFEST_POLL_TIMEOUT in ms, to the host, so the host (dfu-util) waits the PollTimeout before sending a get_status again. Signed-off-by: Heiko Schocher Cc: Lukasz Majewski Cc: Kyungmin Park Cc: Marek Vasut Cc: Pantelis Antoniou --- README | 10 ++++++++++ drivers/dfu/dfu.c | 4 ---- drivers/usb/gadget/f_dfu.c | 48 ++++++++++++++++++++++++++++++++++++++++------ include/dfu.h | 3 +++ 4 files changed, 55 insertions(+), 10 deletions(-) diff --git a/README b/README index 216f0c7..7cb7c4f 100644 --- a/README +++ b/README @@ -1525,6 +1525,16 @@ The following options need to be configured: this to the maximum filesize (in bytes) for the buffer. Default is 4 MiB if undefined. + DFU_DEFAULT_POLL_TIMEOUT + Poll timeout [ms], is the timeout a device can send to the + host. The host must wait for this timeout before sending + a subsequent DFU_GET_STATUS request to the device. + + DFU_MANIFEST_POLL_TIMEOUT + Poll timeout [ms], which the device sends to the host when + entering dfuMANIFEST state. Host waits this timeout, before + sending again an USB request to the device. + - Journaling Flash filesystem support: CONFIG_JFFS2_NAND, CONFIG_JFFS2_NAND_OFF, CONFIG_JFFS2_NAND_SIZE, CONFIG_JFFS2_NAND_DEV diff --git a/drivers/dfu/dfu.c b/drivers/dfu/dfu.c index 1aced26..f94c412 100644 --- a/drivers/dfu/dfu.c +++ b/drivers/dfu/dfu.c @@ -218,10 +218,6 @@ int dfu_write(struct dfu_entity *dfu, void *buf, int size, int blk_seq_num) ret = tret; } - /* end? */ - if (size == 0) - ret = dfu_flush(dfu, buf, size, blk_seq_num); - return ret = 0 ? size : ret; } diff --git a/drivers/usb/gadget/f_dfu.c b/drivers/usb/gadget/f_dfu.c index a045864..de75ff1 100644 --- a/drivers/usb/gadget/f_dfu.c +++ b/drivers/usb/gadget/f_dfu.c @@ -164,9 +164,14 @@ static void dnload_request_complete(struct usb_ep *ep, struct usb_request *req) dfu_write(dfu_get_entity(f_dfu->altsetting), req->buf, req->length, f_dfu->blk_seq_num); +} - if (req->length == 0) - puts("DOWNLOAD ... OK\nCtrl+C to exit ...\n"); +static void dnload_request_flush(struct usb_ep *ep, struct usb_request *req) +{ + struct f_dfu *f_dfu = req->context; + + dfu_flush(dfu_get_entity(f_dfu->altsetting), req->buf, + req->length, f_dfu->blk_seq_num); } static void handle_getstatus(struct usb_request *req) @@ -174,19 +179,22 @@ static void handle_getstatus(struct usb_request *req) struct dfu_status *dstat = (struct dfu_status *)req->buf; struct f_dfu *f_dfu = req->context; + dfu_set_poll_timeout(dstat, 0); + switch (f_dfu->dfu_state) { case DFU_STATE_dfuDNLOAD_SYNC: case DFU_STATE_dfuDNBUSY: f_dfu->dfu_state = DFU_STATE_dfuDNLOAD_IDLE; break; case DFU_STATE_dfuMANIFEST_SYNC: + f_dfu->dfu_state = DFU_STATE_dfuMANIFEST; break; + case DFU_STATE_dfuMANIFEST: + dfu_set_poll_timeout(dstat, DFU_MANIFEST_POLL_TIMEOUT); default: break; } - dfu_set_poll_timeout(dstat, 0); - if (f_dfu->poll_timeout) if (!(f_dfu->blk_seq_num % (dfu_get_buf_size() / DFU_USB_BUFSIZ))) @@ -446,10 +454,11 @@ static int state_dfu_manifest_sync(struct f_dfu *f_dfu, switch (ctrl->bRequest) { case USB_REQ_DFU_GETSTATUS: /* We're MainfestationTolerant */ - f_dfu->dfu_state = DFU_STATE_dfuIDLE; + f_dfu->dfu_state = DFU_STATE_dfuMANIFEST; handle_getstatus(req); f_dfu->blk_seq_num = 0; value = RET_STAT_LEN; + req->complete = dnload_request_flush; break; case USB_REQ_DFU_GETSTATE: handle_getstate(req); @@ -463,6 +472,33 @@ static int state_dfu_manifest_sync(struct f_dfu *f_dfu, return value; } +static int state_dfu_manifest(struct f_dfu *f_dfu, + const struct usb_ctrlrequest *ctrl, + struct usb_gadget *gadget, + struct usb_request *req) +{ + int value = 0; + + switch (ctrl->bRequest) { + case USB_REQ_DFU_GETSTATUS: + /* We're MainfestationTolerant */ + f_dfu->dfu_state = DFU_STATE_dfuIDLE; + handle_getstatus(req); + f_dfu->blk_seq_num = 0; + value = RET_STAT_LEN; + puts("DOWNLOAD ... OK\nCtrl+C to exit ...\n"); + break; + case USB_REQ_DFU_GETSTATE: + handle_getstate(req); + break; + default: + f_dfu->dfu_state = DFU_STATE_dfuERROR; + value = RET_STALL; + break; + } + return value; +} + static int state_dfu_upload_idle(struct f_dfu *f_dfu, const struct usb_ctrlrequest *ctrl, struct usb_gadget *gadget, @@ -539,7 +575,7 @@ static dfu_state_fn dfu_state[] = { state_dfu_dnbusy, /* DFU_STATE_dfuDNBUSY */ state_dfu_dnload_idle, /* DFU_STATE_dfuDNLOAD_IDLE */ state_dfu_manifest_sync, /* DFU_STATE_dfuMANIFEST_SYNC */ - NULL, /* DFU_STATE_dfuMANIFEST */ + state_dfu_manifest, /* DFU_STATE_dfuMANIFEST */ NULL, /* DFU_STATE_dfuMANIFEST_WAIT_RST */ state_dfu_upload_idle, /* DFU_STATE_dfuUPLOAD_IDLE */ state_dfu_error /* DFU_STATE_dfuERROR */ diff --git a/include/dfu.h b/include/dfu.h index 272a245..6c71ecb 100644 --- a/include/dfu.h +++ b/include/dfu.h @@ -80,6 +80,9 @@ static inline unsigned int get_mmc_blk_size(int dev) #ifndef DFU_DEFAULT_POLL_TIMEOUT #define DFU_DEFAULT_POLL_TIMEOUT 0 #endif +#ifndef DFU_MANIFEST_POLL_TIMEOUT +#define DFU_MANIFEST_POLL_TIMEOUT DFU_DEFAULT_POLL_TIMEOUT +#endif struct dfu_entity { char name[DFU_NAME_SIZE]; From 401341d62175650e7966058360683cafeb15be88 Mon Sep 17 00:00:00 2001 From: Heiko Schocher Date: Tue, 18 Mar 2014 08:09:57 +0100 Subject: [PATCH 3/4] am335x, dfu: add DFU_MANIFEST_POLL_TIMEOUT to the siemens boards as the siemens boards use dfu for updating a nand ubi partition add DFU_MANIFEST_POLL_TIMEOUT to them, so dfu host waits after complete transfer of the new image for DFU_MANIFEST_POLL_TIMEOUT ms before sending again an usb request. So the board have enough time to erase rest of the nand sectors. Signed-off-by: Heiko Schocher Reviewed-by: Lukasz Majewski Cc: Kyungmin Park Cc: Marek Vasut Cc: Tom Rini Cc: Pantelis Antoniou --- include/configs/siemens-am33x-common.h | 1 + 1 file changed, 1 insertion(+) diff --git a/include/configs/siemens-am33x-common.h b/include/configs/siemens-am33x-common.h index 98b6e72..721c4e6 100644 --- a/include/configs/siemens-am33x-common.h +++ b/include/configs/siemens-am33x-common.h @@ -265,6 +265,7 @@ #define CONFIG_DFU_NAND #define CONFIG_CMD_DFU #define CONFIG_SYS_DFU_DATA_BUF_SIZE (1 << 20) +#define DFU_MANIFEST_POLL_TIMEOUT 25000 #endif /* CONFIG_SPL_BUILD */ From 7d0b605abb1f2dbccf4abb502f3b1fc9f97f2552 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Majewski?= Date: Fri, 21 Feb 2014 08:23:00 +0100 Subject: [PATCH 4/4] dfu: mmc: Replace calls to u-boot commands with native mmc API For some time we have been using the run_command() with properly crafted string. Such approach turned to be unreliable and error prone. Switch to "native" mmc subsystem API would allow better type checking and shall improve speed. Also, it seems that this API is changing less often than u-boot commands. The approach similar to env operations on the eMMC has been reused. Signed-off-by: Lukasz Majewski --- drivers/dfu/dfu_mmc.c | 31 ++++++++++++++++++++++++------- 1 file changed, 24 insertions(+), 7 deletions(-) diff --git a/drivers/dfu/dfu_mmc.c b/drivers/dfu/dfu_mmc.c index 0816f46..651cfff 100644 --- a/drivers/dfu/dfu_mmc.c +++ b/drivers/dfu/dfu_mmc.c @@ -12,6 +12,7 @@ #include #include #include +#include static unsigned char __aligned(CONFIG_SYS_CACHELINE_SIZE) dfu_file_buf[CONFIG_SYS_DFU_MAX_FILE_SIZE]; @@ -20,8 +21,8 @@ static long dfu_file_buf_len; static int mmc_block_op(enum dfu_op op, struct dfu_entity *dfu, u64 offset, void *buf, long *len) { - char cmd_buf[DFU_CMD_BUF_SIZE]; - u32 blk_start, blk_count; + struct mmc *mmc = find_mmc_device(dfu->dev_num); + u32 blk_start, blk_count, n = 0; /* * We must ensure that we work in lba_blk_size chunks, so ALIGN @@ -38,12 +39,28 @@ static int mmc_block_op(enum dfu_op op, struct dfu_entity *dfu, return -EINVAL; } - sprintf(cmd_buf, "mmc %s %p %x %x", - op == DFU_OP_READ ? "read" : "write", - buf, blk_start, blk_count); + debug("%s: %s dev: %d start: %d cnt: %d buf: 0x%p\n", __func__, + op == DFU_OP_READ ? "MMC READ" : "MMC WRITE", dfu->dev_num, + blk_start, blk_count, buf); + switch (op) { + case DFU_OP_READ: + n = mmc->block_dev.block_read(dfu->dev_num, blk_start, + blk_count, buf); + break; + case DFU_OP_WRITE: + n = mmc->block_dev.block_write(dfu->dev_num, blk_start, + blk_count, buf); + break; + default: + error("Operation not supported\n"); + } - debug("%s: %s 0x%p\n", __func__, cmd_buf, cmd_buf); - return run_command(cmd_buf, 0); + if (n != blk_count) { + error("MMC operation failed"); + return -EIO; + } + + return 0; } static int mmc_file_buffer(struct dfu_entity *dfu, void *buf, long *len)