Hi Roger, On Tue, Apr 12, 2016 at 6:54 AM, Lukasz Majewski <l.majew...@samsung.com> wrote: > Hi Roger, > >> wMaxPacketSize for IN endpoing in High-Speed must be 512 and not 64. >> While fixing that we do some clean ups like >> >> - use cpu_to_le16(decimal_length) instead of hexadecimal length. >> - No need to initialize bInterval to 0. Static variables are 0 >> initialized. >> - Move descriptor setting from fastboot_add to to fastboot_bind. >> - check for dual speed configuration before setting the high speed >> descriptors. > > Fixes are correct, but I think that there would be a problem with > Broadcomm boards when you change EP IN max packet from 64 to 512B. > > We had such discussion previously: > > https://www.mail-archive.com/u-boot@lists.denx.de/msg201596.html > > I think that Steve would say more about this.
That was a different issue -- nothing to do with the size of the endpoints..... Tested-by: Steve Rae <s...@broadcom.com> [Test HW: bcm235xx board] Thanks, Steve PS. however, it does NOT solve the issue is the other thread, I still need to patch with the following in order to get the download to not stall on the last packet: diff --git a/drivers/usb/gadget/f_fastboot.c b/drivers/usb/gadget/f_fastboot.c index eddc6b9..2633503 100644 --- a/drivers/usb/gadget/f_fastboot.c +++ b/drivers/usb/gadget/f_fastboot.c @@ -456,12 +456,17 @@ static unsigned int rx_bytes_expected(unsigned int maxpacket) return 0; if (rx_remain > EP_BUFFER_SIZE) return EP_BUFFER_SIZE; + +rem = rx_remain % maxpacket; +printf("\n\n **** SRAE: code removed: rx_remain=%d, maxpacket=%d, rem=%d\n", rx_remain, maxpacket, rem); +#if 0 if (rx_remain < maxpacket) { rx_remain = maxpacket; } else if (rx_remain % maxpacket != 0) { rem = rx_remain % maxpacket; rx_remain = rx_remain + (maxpacket - rem); } +#endif return rx_remain; } > >> >> Signed-off-by: Roger Quadros <rog...@ti.com> >> --- >> drivers/usb/gadget/f_fastboot.c | 36 >> +++++++++++++++++++++++++++--------- 1 file changed, 27 >> insertions(+), 9 deletions(-) >> >> diff --git a/drivers/usb/gadget/f_fastboot.c >> b/drivers/usb/gadget/f_fastboot.c index 2e87fee..e1038ea 100644 >> --- a/drivers/usb/gadget/f_fastboot.c >> +++ b/drivers/usb/gadget/f_fastboot.c >> @@ -64,8 +64,7 @@ static struct usb_endpoint_descriptor fs_ep_in = { >> .bDescriptorType = USB_DT_ENDPOINT, >> .bEndpointAddress = USB_DIR_IN, >> .bmAttributes = USB_ENDPOINT_XFER_BULK, >> - .wMaxPacketSize = TX_ENDPOINT_MAXIMUM_PACKET_SIZE, >> - .bInterval = 0x00, >> + .wMaxPacketSize = cpu_to_le16(64), >> }; >> >> static struct usb_endpoint_descriptor fs_ep_out = { >> @@ -73,8 +72,15 @@ static struct usb_endpoint_descriptor fs_ep_out = { >> .bDescriptorType = USB_DT_ENDPOINT, >> .bEndpointAddress = USB_DIR_OUT, >> .bmAttributes = USB_ENDPOINT_XFER_BULK, >> - .wMaxPacketSize = >> RX_ENDPOINT_MAXIMUM_PACKET_SIZE_1_1, >> - .bInterval = 0x00, >> + .wMaxPacketSize = cpu_to_le16(64), >> +}; >> + >> +static struct usb_endpoint_descriptor hs_ep_in = { >> + .bLength = USB_DT_ENDPOINT_SIZE, >> + .bDescriptorType = USB_DT_ENDPOINT, >> + .bEndpointAddress = USB_DIR_IN, >> + .bmAttributes = USB_ENDPOINT_XFER_BULK, >> + .wMaxPacketSize = cpu_to_le16(512), >> }; >> >> static struct usb_endpoint_descriptor hs_ep_out = { >> @@ -82,8 +88,7 @@ static struct usb_endpoint_descriptor hs_ep_out = { >> .bDescriptorType = USB_DT_ENDPOINT, >> .bEndpointAddress = USB_DIR_OUT, >> .bmAttributes = USB_ENDPOINT_XFER_BULK, >> - .wMaxPacketSize = >> RX_ENDPOINT_MAXIMUM_PACKET_SIZE_2_0, >> - .bInterval = 0x00, >> + .wMaxPacketSize = cpu_to_le16(512), >> }; >> >> static struct usb_interface_descriptor interface_desc = { >> @@ -97,9 +102,15 @@ static struct usb_interface_descriptor >> interface_desc = { .bInterfaceProtocol = >> FASTBOOT_INTERFACE_PROTOCOL, }; >> >> -static struct usb_descriptor_header *fb_runtime_descs[] = { >> +static struct usb_descriptor_header *fb_fs_function[] = { >> (struct usb_descriptor_header *)&interface_desc, >> (struct usb_descriptor_header *)&fs_ep_in, >> + (struct usb_descriptor_header *)&fs_ep_out, >> +}; >> + >> +static struct usb_descriptor_header *fb_hs_function[] = { >> + (struct usb_descriptor_header *)&interface_desc, >> + (struct usb_descriptor_header *)&hs_ep_in, >> (struct usb_descriptor_header *)&hs_ep_out, >> NULL, >> }; >> @@ -177,7 +188,15 @@ static int fastboot_bind(struct >> usb_configuration *c, struct usb_function *f) return -ENODEV; >> f_fb->out_ep->driver_data = c->cdev; >> >> - hs_ep_out.bEndpointAddress = fs_ep_out.bEndpointAddress; >> + f->descriptors = fb_fs_function; >> + >> + if (gadget_is_dualspeed(gadget)) { >> + /* Assume endpoint addresses are the same for both >> speeds */ >> + hs_ep_in.bEndpointAddress = >> fs_ep_in.bEndpointAddress; >> + hs_ep_out.bEndpointAddress = >> fs_ep_out.bEndpointAddress; >> + /* copy HS descriptors */ >> + f->hs_descriptors = fb_hs_function; >> + } >> >> s = getenv("serial#"); >> if (s) >> @@ -302,7 +321,6 @@ static int fastboot_add(struct usb_configuration >> *c) } >> >> f_fb->usb_function.name = "f_fastboot"; >> - f_fb->usb_function.hs_descriptors = fb_runtime_descs; >> f_fb->usb_function.bind = fastboot_bind; >> f_fb->usb_function.unbind = fastboot_unbind; >> f_fb->usb_function.set_alt = fastboot_set_alt; > > > > -- > Best regards, > > Lukasz Majewski > > Samsung R&D Institute Poland (SRPOL) | Linux Platform Group _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot