Hi,
Lukasz Majewski lukma@denx.de writes:
Lukasz Majewski lukma@denx.de writes:
Lukasz Majewski lukma@denx.de writes:
> >> drivers/usb/gadget/f_dfu.c | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >> diff --git a/drivers/usb/gadget/f_dfu.c > >> b/drivers/usb/gadget/f_dfu.c index 8e7c981657..64cdfa7c98 > >> 100644 --- a/drivers/usb/gadget/f_dfu.c > >> +++ b/drivers/usb/gadget/f_dfu.c > >> @@ -159,7 +159,7 @@ static void > >> dnload_request_complete(struct usb_ep *ep, struct > >> usb_request *req) int ret; > >> > >> ret = dfu_write(dfu_get_entity(f_dfu->altsetting), > >> req->buf, > >> - req->length, f_dfu->blk_seq_num); > >> + req->actual, f_dfu->blk_seq_num); > > DFU driver queues a request to USB controller. Per the gadget > API req->length contains maximum amount of data to be > transmitted. req->actual is written by USB controller with > the actual amount of data that we transmitted. > > In the case of IN (TX), upon completion req->length and > req->actual should always be equal (unless errors show up, > etc) > > In the case of OUT (RX), upon completion req->actual MAY BE > less than req->length and that's not an error. Say host sent > us a short packet which causes early termination of transfer. > > With that in mind, let's consider the situation where we're > receiving data from host using DFU. Let's assume that we have > a 4096 byte buffer for transfers and we're receiving a binary > that's 7679 bytes in size. > > Here's what we will do (pseudo-code): > > int remaining = 7679; > char buf[4096]; > > while (remaining) { > req->length = 4096; > req->buf = buf; > usb_ep_queue(req); > > /* wait for completion */ > > remaining -= req->actual; > > dfu_write(buf, req->length); /* this is the error */ > } > > Can you see here that in the last packet we will write 4096 > bytes when we should write only 3583? > > In principle you are right. I need to check if this change > will not introduce regressions. > > Can you share your use case?
Intel Edison running v2017.03-rc1 + patches (see [1]), flashing u-boot.bin over DFU (see [2] for details). Without $subject, image has to be aligned to 4096 bytes as below:
$ dd if=u-boot.bin of=u-boot-4k.bin bs=4k seek=1 && truncate -s %4096 u-boot-4k.bin
With $subject, I don't need truncate. We still need the 4096 byte of zeroes in the beginning of the image for other reasons (which I really don't know why at this point).
[1] https://github.com/andy-shev/u-boot/tree/edison [2] https://communities.intel.com/message/435516#435516
Ok. I will check this. Thanks for pointing out :-)
Any updates here? I'd like to send Tangier Soc and Intel Edison Board support but I kinda depend on this patch making upstream. I can resend as part of the "add intel edison" series.
Let me know
I'm setting up /test/py/dfu now on BBB. I will let you know by EOD.
Here's what I used for testing:
I do appreciate that you tested it - even better, that with different approach.
However, some time ago Stephen Warren has rewritten tests for DFU, UMS to use some python infrastructure. Those tests (especially DFU, test corner cases - ZLP, +/-1B to packet size, etc).
that's exactly what I tested :-)
If you want, you can add your board to them.
I'll see how to do that and maybe add to the TODO list