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

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.

Signed-off-by: Roger Quadros <rogerq@ti.com>

---
 drivers/usb/gadget/f_fastboot.c | 36 +++++++++++++++++++++++++++---------
 1 file changed, 27 insertions(+), 9 deletions(-)

-- 
2.5.0

_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

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

_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot