Re: [U-Boot] [PATCH] fastboot: Fix OUT transaction length alignment

14 Apr 2016

On Wed, Apr 13, 2016 at 9:56 AM, Sam Protsenko semen.protsenko@linaro.org
wrote:
...
On Wed, Apr 13, 2016 at 3:32 PM, Roger Quadros rogerq@ti.com wrote:
...
Hi,
On 13/04/16 15:01, Semen Protsenko wrote:
...
From: Sam Protsenko semen.protsenko@linaro.org
Some UDC controllers may require buffer size to be aligned to
wMaxPacketSize. It's indicated by gadget->quirk_ep_out_aligned_size
field being set to "true" (in UDC driver code). In that case
rx_bytes_expected must be aligned to wMaxPacket size, otherwise stuck on
transaction will happen. For example, it's required by DWC3 controller
data manual:
section 8.2.3.3 Buffer Size Rules and Zero-Length Packets:

For OUT endpoints, the following rules apply:
- The BUFSIZ field must be ≥ 1 byte.
- The total size of a Buffer Descriptor must be a multiple of
  MaxPacketSize
- A received zero-length packet still requires a MaxPacketSize

buffer.
...
...
  Therefore, if the expected amount of data to be received is a

multiple
...
...
  of MaxPacketSize, software should add MaxPacketSize bytes to the

buffer
...
...
  to sink a possible zero-length packet at the end of the transfer.


But other UDC controllers don't need such alignment, so mentioned field
is set to "false". If buffer size is aligned to wMaxPacketSize, those
controllers may stuck on transaction. The example is DWC2.
This patch checks gadget->quirk_ep_out_aligned_size field and aligns
rx_bytes_expected to wMaxPacketSize only when it's needed.
Signed-off-by: Sam Protsenko semen.protsenko@linaro.org
drivers/usb/gadget/f_fastboot.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/usb/gadget/f_fastboot.c
b/drivers/usb/gadget/f_fastboot.c
...
...
index 2e87fee..54dcce0 100644
--- a/drivers/usb/gadget/f_fastboot.c
+++ b/drivers/usb/gadget/f_fastboot.c
@@ -58,6 +58,7 @@ static unsigned int fastboot_flash_session_id;
 static unsigned int download_size;
 static unsigned int download_bytes;
 static bool is_high_speed;
+static bool quirk_ep_out_aligned_size;
static struct usb_endpoint_descriptor fs_ep_in = {
      .bLength            = USB_DT_ENDPOINT_SIZE,
@@ -240,6 +241,8 @@ static int fastboot_set_alt(struct usb_function *f,
      debug("%s: func: %s intf: %d alt: %d\n",
            __func__, f->name, interface, alt);

quirk_ep_out_aligned_size = gadget->quirk_ep_out_aligned_size;


/* make sure we don't enable the ep twice */
if (gadget->speed == USB_SPEED_HIGH) {
        ret = usb_ep_enable(f_fb->out_ep, &hs_ep_out);



@@ -435,12 +438,18 @@ static unsigned int rx_bytes_expected(unsigned
int maxpacket)
...
...
          return 0;
  if (rx_remain > EP_BUFFER_SIZE)
          return EP_BUFFER_SIZE;



if (!quirk_ep_out_aligned_size)


        goto out;


if (rx_remain < maxpacket) {
        rx_remain = maxpacket;
} else if (rx_remain % maxpacket != 0) {
        rem = rx_remain % maxpacket;
        rx_remain = rx_remain + (maxpacket - rem);
}




+out:
      return rx_remain;
 }
yes -- anything that skips those 6 lines will work on my boards....
...
...
Why do we need a special flag for this driver if other drivers e.g. mass
storage
...
are doing perfectly fine without it?
I don't know how it works in other gadgets, but please see this patch
in kernel: [1]. That patch is doing just the same as I did (and also
in gadget code), using usb_ep_align_maybe() function for alignment.
...
This patch is just covering up the real problem, by bypassing the faulty
code
...
with a flag.
The buffer size is EP_BUFFER_SIZE and is already aligned to
wMaxPacketSize so
...
the problem shouldn't have happened in the first place. But it is
happening
...
indicating something else is wrong.
There is what I'm observing on platform with DWC3 controller:

when doing "fastboot flash xloader MLO":
the whole size of data to send is 70964 bytes
the size of all packets (except of last packet) is 4096 bytes
so those are being sent just fine (in req->complete, which is

rx_handler_dl_image() function)

but last packet has size of 1332 bytes (because 70964 % 4096 = 1332)
when its req->length is aligned to wMaxPacketSize (so it's 1536

bytes): after we send it using usb_ep_queue(), the req->complete
callback is called one last time and we see that transmission is
finished (download_bytes >= download_size)

but when its req->length is not aligned to wMaxPacketSize (so it's

1332 bytes): req->complete callback is not called last time, so
transaction is not finished and we are stuck in "fastboot flash"
So I guess the issue is real and related to DWC3 quirk. If you have
any thoughts regarding other possible causes of this problem -- please
share. I can't predict all possible causes as I'm not USB expert.
...
Why is the code using wMaxPacketSize for the check. why can't it just
use usb_ep->maxpacket?
...
I just reused already existed code. Sure we can use usb_ep->maxpacket
instead, it's also 512 in my case (I believe it's assigned in DWC3
code).
[1]
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=2...
...
cheers,
-roger

    

Re: [U-Boot] [PATCH] fastboot: Fix OUT transaction length alignment

Steve Rae

Signed-off-by: Sam Protsenko semen.protsenko@linaro.org