[PATCH] u-boot: Fix DFU upload in u-boot
Harald Welte
laforge at openmoko.org
Tue Oct 7 19:20:23 CEST 2008
Fix DFU upload in u-boot
The existing USB DFU upload (read firmware from device to USB host) code
was a big mess and probably only ever worked by accident.
specifically, there were three bugs described in
http://docs.openmoko.org/trac/ticket/1843 :
* when it copies a new blockful of data, it copies it into the same buffer that
urb->buffer was already pointing to, thus overwriting the beginning of the
last buffer before it is sent back to the requestor
* it then calls memcpy() to copy the beginning of the newly-read block to after
the end of the buffer that urb->buffer is pointing to. If ds->nand->erasesize
is the same as the _buf[] array, then this will write past the end of the
_buf[] array and smash some other item in RAM.
* if a requested buffer exactly reaches the end of the block that's been
buffered in ds->buf, then handle_upload() will read a new NAND block into the
buffer even though it is not needed. (The test for (len > remain) should
probably go before the test for ds->ptr, not after?)
So instead of fixing those issues individually, I rewored the logic
for how to deal with DFU upload. Much simpler, and without those bugs.
Signed-off-by: Harald Welte <laforge at openmoko.org>
diff --git a/drivers/usb/usbdfu.c b/drivers/usb/usbdfu.c
index 933f587..855d12e 100644
--- a/drivers/usb/usbdfu.c
+++ b/drivers/usb/usbdfu.c
@@ -259,18 +259,17 @@ static int erase_tail_clean_nand(struct urb *urb, struct dnload_state *ds)
}
/* Read the next erase blcok from NAND into buffer */
-static int read_next_nand(struct urb *urb, struct dnload_state *ds)
+static int read_next_nand(struct urb *urb, struct dnload_state *ds, int len)
{
struct usb_device_instance *dev = urb->device;
int rc;
ds->read_opts.buffer = ds->buf;
- ds->read_opts.length = ds->nand->erasesize;
+ ds->read_opts.length = len;
ds->read_opts.offset = ds->off;
ds->read_opts.quiet = 1;
- debug("Reading 0x%x at 0x%x to 0x%08p\n", ds->nand->erasesize,
- ds->off, ds->buf);
+ debug("Reading 0x%x at 0x%x to 0x%08p\n", len, ds->off, ds->buf);
rc = nand_read_opts(ds->nand, &ds->read_opts);
if (rc) {
debug("Error reading\n");
@@ -278,7 +277,7 @@ static int read_next_nand(struct urb *urb, struct dnload_state *ds)
dev->dfu_status = DFU_STATUS_errWRITE;
return RET_STALL;
}
- ds->off += ds->nand->erasesize;
+ ds->off += len;
ds->ptr = ds->buf;
return RET_NOTHING;
@@ -498,9 +497,6 @@ static int handle_upload(struct urb *urb, u_int16_t val, u_int16_t len, int firs
return -EINVAL;
printf("Starting DFU Upload of partition '%s'\n",
ds->part->name);
- rc = read_next_nand(urb, ds);
- if (rc)
- return -EINVAL;
}
if (len > ds->nand->erasesize) {
@@ -509,27 +505,19 @@ static int handle_upload(struct urb *urb, u_int16_t val, u_int16_t len, int firs
len = ds->nand->erasesize;
}
- remain = ds->nand->erasesize - (ds->ptr - ds->buf);
- if (len < remain)
- remain = len;
+ /* limit length to whatever number of bytes is left in
+ * this partition */
+ remain = (ds->part->offset + ds->part->size) - ds->off;
+ if (len > remain)
+ len = remain;
- debug("copying %u bytes ", remain);
- urb->buffer = ds->ptr;
- ds->ptr += remain;
- urb->actual_length = remain;
+ rc = read_next_nand(urb, ds, len);
+ if (rc)
+ return -EINVAL;
- if (ds->ptr >= ds->buf + ds->nand->erasesize &&
- ds->off < ds->part->offset + ds->part->size) {
- rc = read_next_nand(urb, ds);
- if (rc)
- return -EINVAL;
- if (len > remain) {
- debug("copying another %u bytes ", len - remain);
- memcpy(urb->buffer + remain, ds->ptr, len - remain);
- ds->ptr += (len - remain);
- urb->actual_length += (len - remain);
- }
- }
+ debug("uploading %u bytes ", len);
+ urb->buffer = ds->buf;
+ urb->actual_length = len;
break;
}
--
- Harald Welte <laforge at openmoko.org> http://openmoko.org/
============================================================================
Software for the world's first truly open Free Software mobile phone
More information about the openmoko-kernel
mailing list