| Message ID | 1460465509-2731-2-git-send-email-rogerq@ti.com |
|---|---|
| State | Accepted, archived |
| Commit | 718156ad0ab076f94fbfc5f2628e6b43d1f5a127 |
| Delegated to: | Łukasz Majewski |
| Headers | show |
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. > > 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;
oops -- my last reply was interpreted as a new patch: http://patchwork.ozlabs.org/patch/609843/ so I'll try again: Tested-by: Steve Rae <srae@broadcom.com> [Test HW: bcm235xx board] On Tue, Apr 12, 2016 at 5:51 AM, Roger Quadros <rogerq@ti.com> wrote: > 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(-) > > 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; > -- > 2.5.0 >
Thank you Steve and Roger for your support. Regards Hong -----Original Message----- From: U-Boot [mailto:u-boot-bounces@lists.denx.de] On Behalf Of Steve Rae Sent: Tuesday, April 12, 2016 7:18 PM To: Roger Quadros <rogerq@ti.com> Cc: Marek Vašut <marex@denx.de>; Tom Rini <trini@konsulko.com>; Steve Rae <srae@broadcom.com>; U-Boot Mailing List <u-boot@lists.denx.de> Subject: Re: [U-Boot] [PATCH 1/2] fastboot: Fix wMaxPacketSize for High-Speed IN endpoint oops -- my last reply was interpreted as a new patch: http://patchwork.ozlabs.org/patch/609843/ so I'll try again: Tested-by: Steve Rae <srae@broadcom.com> [Test HW: bcm235xx board] On Tue, Apr 12, 2016 at 5:51 AM, Roger Quadros <rogerq@ti.com> wrote: > 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(-) > > 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; > -- > 2.5.0 >
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;
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(-)