[U-Boot] [PATCH 1/2] fastboot: Fix wMaxPacketSize for High-Speed IN endpoint

[Steve Rae]

Hi Roger,

On Tue, Apr 12, 2016 at 6:54 AM, Lukasz Majewski <l.majewski at 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 <srae at 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 <rogerq at 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