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

Roger Quadros
2016-04-13T07:19:51Z

On 13/04/16 04:42, Steve Rae wrote:
> Hi Roger,
>
> On Tue, Apr 12, 2016 at 6:54 AM, Lukasz Majewski <l.majewski@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 at 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@broadcom.com>
> [Test HW: bcm235xx board]

Thanks :)

>
> 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:

Yes, that issue still needs to be fixed correctly. I understood that
Sam is working on it.

cheers,
-roger

>
>
> 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@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