[U-Boot] [PATCH v1] usb: gadget: f_dfu: write req->actual bytes
Lukasz Majewski
lukma at denx.de
Tue Feb 21 13:21:32 UTC 2017
Hi.
>
> Hi,
>
> Lukasz Majewski <lukma at denx.de> writes:
> >> Lukasz Majewski <lukma at 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).
If you want, you can add your board to them.
More info ./test/py {dfu, ums} and ./Documentation.
(Or you can wait a bit, so I will share my "fresh" BBB setup).
Best regards,
Łukasz
>
> #!/usr/bin/env ruby
>
> $errors = 0
> $success = 0
> $total = 0
>
> $initial_size = 64
> $max_size = 8 * 1024 * 1024 # 8MiB
> $current_size = $initial_size
>
> def create_file(size)
> `dd if=/dev/zero of=file.bin bs=#{size} count=1 > /dev/null 2>&1`
> end
>
> def send_file(size)
> `dfu-util -v -d 8087:0a99 -D file.bin > /dev/null 2>&1`
> end
>
> def transmit(size)
> create_file(size)
> send_file(size)
> end
>
> def transmit_and_check(size)
> transmit(size)
>
> if $?.success?
> $success += 1
> print "."
> else
> $errors += 1
> print "F"
> end
>
> $total += 1
> end
>
> while $current_size <= $max_size do
> transmit_and_check($current_size - 1)
> transmit_and_check($current_size)
> transmit_and_check($current_size + 1)
>
> $current_size *= 2
> end
>
> puts "\n"
> puts "Test Done"
> puts "Successes: #{$success}"
> puts "Failures: #{$errors}"
> puts "Total: #{$total}"
>
Best regards,
Lukasz Majewski
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 181 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170221/b74f4fc9/attachment.sig>
More information about the U-Boot
mailing list