uboot: fastboot: fix high speed endpoint descriptors
cover uboot: fastboot: fix high speed endpoint descriptors
Hi,
I've not tested this extensively except running "fastboot devices" and reading the usb device descriptor using lsusb -v.
USB Endpoint descriptors before:
Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x81 EP 1 IN
bmAttributes 2
Transfer Type Bulk
Synch Type None
Usage Type Data
wMaxPacketSize 0x0040 1x 64 bytes
bInterval 0
Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x02 EP 2 OUT
bmAttributes 2
Transfer Type Bulk
Synch Type None
Usage Type Data
wMaxPacketSize 0x0200 1x 512 bytes
bInterval 0
and after:
Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x81 EP 1 IN
bmAttributes 2
Transfer Type Bulk
Synch Type None
Usage Type Data
wMaxPacketSize 0x0200 1x 512 bytes
bInterval 0
Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x02 EP 2 OUT
bmAttributes 2
Transfer Type Bulk
Synch Type None
Usage Type Data
wMaxPacketSize 0x0200 1x 512 bytes
bInterval 0
cheers,
-roger
Roger Quadros (2):
fastboot: Fix wMaxPacketSize for High-Speed IN endpoint
fastboot: Enable the respective speed endpoints at runtime
drivers/usb/gadget/f_fastboot.c | 55 +++++++++++++++++++++++++++++++---------- 1 file changed, 42 insertions(+), 13 deletions(-)
Signature · 1 line
2.5.0
PATCH 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.
--- drivers/usb/gadget/f_fastboot.c | 36 +++++++++++++++++++++++++++--------- 1 file changed, 27 insertions(+), 9 deletions(-)
drivers/usb/gadget/f_fastboot.c +27 -9
@@ -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;
Signature · 1 line
2.5.0
PATCH 1/2 fastboot: Fix wMaxPacketSize for High-Speed IN endpoint
Hi Roger,
Quoted 9 lines
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 fromfastboot_addto tofastboot_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.
Quoted 96 lines
Signed-off-by Roger Quadros <[email protected]>--- drivers/usb/gadget/f_fastboot.c | 36+++++++++++++++++++++++++++--------- 1 file changed, 27 insertions(+), 9 deletions(-)
(quoted diff) +0
-0
b/drivers/usb/gadget/f_fastboot.cindex 2e87fee..e1038ea 100644drivers/usb/gadget/f_fastboot.c +10
-4@@ -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_descriptorhs_ep_out= {drivers/usb/gadget/f_fastboot.c +1
-2@@ -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_descriptorinterface_desc= {drivers/usb/gadget/f_fastboot.c +0
-0@@ -97,9 +102,15 @@ static struct usb_interface_descriptor
interface_desc= { .bInterfaceProtocol =FASTBOOT_INTERFACE_PROTOCOL, };drivers/usb/gadget/f_fastboot.c +11
-2-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 */
drivers/usb/gadget/f_fastboot.c +1
-0+ hs_ep_in.bEndpointAddress =fs_ep_in.bEndpointAddress;
drivers/usb/gadget/f_fastboot.c +1
-0+ hs_ep_out.bEndpointAddress =fs_ep_out.bEndpointAddress;
drivers/usb/gadget/f_fastboot.c +3
-1+/* 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;
Signature · 5 lines
Best regards, Lukasz Majewski Samsung R&D Institute Poland (SRPOL) | Linux Platform Group
PATCH 1/2 fastboot: Fix wMaxPacketSize for High-Speed IN endpoint
Hi Roger,
Quoted 12 lines · Lukasz Majewski
Hi Roger,
Quoted 9 lines
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 fromfastboot_addto tofastboot_bind.
- check for dual speed configuration before setting the high speed descriptors.
Quoted 8 lines
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.....
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:
drivers/usb/gadget/f_fastboot.c +0 -0
@@ -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;}
Quoted 106 lines
Quoted 90 lines
Signed-off-by Roger Quadros <[email protected]>--- drivers/usb/gadget/f_fastboot.c | 36+++++++++++++++++++++++++++--------- 1 file changed, 27 insertions(+), 9 deletions(-)
(quoted diff) +0
-0
b/drivers/usb/gadget/f_fastboot.cindex 2e87fee..e1038ea 100644drivers/usb/gadget/f_fastboot.c +10
-4@@ -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_descriptorhs_ep_out= {drivers/usb/gadget/f_fastboot.c +1
-2@@ -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_descriptorinterface_desc= {drivers/usb/gadget/f_fastboot.c +0
-0@@ -97,9 +102,15 @@ static struct usb_interface_descriptor
interface_desc= { .bInterfaceProtocol =FASTBOOT_INTERFACE_PROTOCOL, };drivers/usb/gadget/f_fastboot.c +11
-2-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;drivers/usb/gadget/f_fastboot.c +3
-0+/* copy HS descriptors */ + f->hs_descriptors = fb_hs_function; + } s =getenv ("serial#" );if (s)drivers/usb/gadget/f_fastboot.c static int fastboot_add(struct usb_configuration · line 326
Quoted 7 lines
drivers/usb/gadget/f_fastboot.c +0
-1@@ -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
PATCH 1/2 fastboot: Fix wMaxPacketSize for High-Speed IN endpoint
Quoted 20 lines · Steve Rae
Hi Roger,
Quoted 17 lines · Lukasz Majewski
Hi Roger,
Quoted 9 lines
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 fromfastboot_addto tofastboot_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:
Quoted 8 lines
Quoted 3 lines
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 <[email protected]>[Test HW: bcm235xx board]
Thanks :)
Quoted 5 lines
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
Quoted 135 lines
drivers/usb/gadget/f_fastboot.c +5
-0@@ -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;}
Quoted 106 lines
Quoted 90 lines
Signed-off-by Roger Quadros <[email protected]>--- drivers/usb/gadget/f_fastboot.c | 36+++++++++++++++++++++++++++--------- 1 file changed, 27 insertions(+), 9 deletions(-)
(quoted diff) +0
-0
b/drivers/usb/gadget/f_fastboot.cindex 2e87fee..e1038ea 100644drivers/usb/gadget/f_fastboot.c +10
-4@@ -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_descriptorhs_ep_out= {drivers/usb/gadget/f_fastboot.c +1
-2@@ -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_descriptorinterface_desc= {drivers/usb/gadget/f_fastboot.c +0
-0@@ -97,9 +102,15 @@ static struct usb_interface_descriptor
interface_desc= { .bInterfaceProtocol =FASTBOOT_INTERFACE_PROTOCOL, };drivers/usb/gadget/f_fastboot.c +11
-2-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;drivers/usb/gadget/f_fastboot.c +3
-0+/* copy HS descriptors */ + f->hs_descriptors = fb_hs_function; + } s =getenv ("serial#" );if (s)drivers/usb/gadget/f_fastboot.c static int fastboot_add(struct usb_configuration · line 326
Quoted 7 lines
drivers/usb/gadget/f_fastboot.c +0
-1@@ -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
PATCH 1/2 fastboot: Fix wMaxPacketSize for High-Speed IN endpoint
In reply to Roger Quadrosoops -- my last reply was interpreted as a new patch:
http://patchwork.ozlabs.org/patch/609843/
so I'll try again:
Quoted 100 lines · Roger Quadros
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 fromfastboot_addto tofastboot_bind.
- check for dual speed configuration before setting the high speed descriptors.Signed-off-by Roger Quadros <[email protected]>--- drivers/usb/gadget/f_fastboot.c | 36 +++++++++++++++++++++++++++--------- 1 file changed, 27 insertions(+), 9 deletions(-)drivers/usb/gadget/f_fastboot.c +27
-10@@ -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
PATCH 1/2 fastboot: Fix wMaxPacketSize for High-Speed IN endpoint
Thank you Steve and Roger for your support.
Regards
Hong
-----Original Message-----
From: U-Boot [mailto:u-boot-bounces at lists.denx.de] On Behalf Of Steve Rae
Sent: Tuesday, April 12, 2016 7:18 PM
To: Roger Quadros <[email protected]>
Cc: Marek Va?ut <[email protected]>; Tom Rini <[email protected]>; Steve Rae <[email protected]>; U-Boot Mailing List <[email protected]>
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:
Quoted 98 lines · Roger Quadros
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 fromfastboot_addto tofastboot_bind.
- check for dual speed configuration before setting the high speed descriptors.Signed-off-by Roger Quadros <[email protected]>--- drivers/usb/gadget/f_fastboot.c | 36 +++++++++++++++++++++++++++--------- 1 file changed, 27 insertions(+), 9 deletions(-)(quoted diff) +0
-0
b/drivers/usb/gadget/f_fastboot.cindex 2e87fee..e1038ea 100644drivers/usb/gadget/f_fastboot.c +1
-2@@ -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_descriptorfs_ep_out= {.bDescriptorType = USB_DT_ENDPOINT, .bEndpointAddress = USB_DIR_OUT, .bmAttributes = USB_ENDPOINT_XFER_BULK,drivers/usb/gadget/f_fastboot.c +9
-2- .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_descriptorhs_ep_out= {.bDescriptorType = USB_DT_ENDPOINT, .bEndpointAddress = USB_DIR_OUT, .bmAttributes = USB_ENDPOINT_XFER_BULK,drivers/usb/gadget/f_fastboot.c +17
-6- .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; --
Quoted 2 lines
2.5.0
_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
PATCH 2/2 fastboot: Enable the respective speed endpoints at runtime
In reply to Roger QuadrosIn a dual speed configuration we need to check at runtime if we want to enable the Full-Speed or High-Speed endpoint.
--- drivers/usb/gadget/f_fastboot.c | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-)
drivers/usb/gadget/f_fastboot.c +15 -4
@@ -115,6 +115,15 @@ static struct usb_descriptor_header *fb_hs_function[] = { NULL, }; +static struct usb_endpoint_descriptor * +fb_ep_desc (struct usb_gadget *g,struct usb_endpoint_descriptor *fs, +struct usb_endpoint_descriptor *hs) +{ +if (gadget_is_dualspeed (g) && g->speed ==USB_SPEED_HIGH ) +return hs; +return fs; +} +/* * static strings, in UTF-8 */
@@ -255,18 +264,19 @@ static int fastboot_set_alt(struct usb_function *f,struct usb_composite_dev *cdev = f->config->cdev;struct usb_gadget *gadget = cdev->gadget;struct f_fastboot *f_fb =func_to_fastboot (f); +const struct usb_endpoint_descriptor *d;debug ("%s: func: %s intf: %d alt: %d\n" , __func__, f->name, interface, alt); -/* make sure we don't enable the ep twice */ if (gadget->speed ==USB_SPEED_HIGH ) { - ret =usb_ep_enable (f_fb->out_ep, &hs_ep_out); is_high_speed = true; }else { - ret =usb_ep_enable (f_fb->out_ep, &fs_ep_out); is_high_speed = false; } + + d =fb_ep_desc (gadget, &fs_ep_out, &hs_ep_out); + ret =usb_ep_enable (f_fb->out_ep, d);if (ret) {puts ("failed to enable out ep\n" );return ret;
@@ -280,7 +290,8 @@ static int fastboot_set_alt(struct usb_function *f, } f_fb->out_req->complete = rx_handler_command; - ret =usb_ep_enable (f_fb->in_ep, &fs_ep_in); + d =fb_ep_desc (gadget, &fs_ep_in, &hs_ep_in); + ret =usb_ep_enable (f_fb->in_ep, d);if (ret) {puts ("failed to enable in ep\n" ); goto err;
Signature · 1 line
2.5.0
PATCH 2/2 fastboot: Enable the respective speed endpoints at runtime
Hi Roger,
Quoted 35 lines
In a dual speed configuration we need to check at runtime if we want to enable the Full-Speed or High-Speed endpoint.
Signed-off-by Roger Quadros <[email protected]>--- drivers/usb/gadget/f_fastboot.c | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-)(quoted diff) +0
-0
b/drivers/usb/gadget/f_fastboot.cindex e1038ea..8ab187e 100644drivers/usb/gadget/f_fastboot.c +10
-0@@ -115,6 +115,15 @@ static struct usb_descriptor_header *fb_hs_function[] = { NULL, }; +static struct usb_endpoint_descriptor * +fb_ep_desc (struct usb_gadget *g,struct usb_endpoint_descriptor *fs, +struct usb_endpoint_descriptor *hs) +{ +if (gadget_is_dualspeed (g) && g->speed ==USB_SPEED_HIGH ) +return hs; +return fs; +} +/* * static strings, in UTF-8 */ @@ -255,18 +264,19 @@ static int fastboot_set_alt(struct usb_function *f, struct usb_composite_dev *cdev = f->config->cdev;struct usb_gadget *gadget = cdev->gadget;struct f_fastboot *f_fb =func_to_fastboot (f); +const struct usb_endpoint_descriptor *d; debug("%s: func: %s intf: %d alt: %d\n",
drivers/usb/gadget/f_fastboot.c static int fastboot_set_alt(struct usb_function · line 276
Quoted 10 lines
drivers/usb/gadget/f_fastboot.c +0
-3__func__, f->name, interface, alt); -/* make sure we don't enable the ep twice */ if (gadget->speed ==USB_SPEED_HIGH ) { - ret =usb_ep_enable (f_fb->out_ep, &hs_ep_out); is_high_speed = true; }else { - ret =usb_ep_enable (f_fb->out_ep, &fs_ep_out); is_high_speed = false; }
Very minor remark -
Those {} are not needed.
Quoted 7 lines
drivers/usb/gadget/f_fastboot.c +3
-0+ + d =fb_ep_desc (gadget, &fs_ep_out, &hs_ep_out); + ret =usb_ep_enable (f_fb->out_ep, d);if (ret) {puts ("failed to enable out ep\n" );return ret;
drivers/usb/gadget/f_fastboot.c static int fastboot_set_alt(struct usb_function · line 297
Quoted 9 lines
drivers/usb/gadget/f_fastboot.c +2
-1@@ -280,7 +290,8 @@ static int fastboot_set_alt(struct usb_function *f, } f_fb->out_req->complete = rx_handler_command; - ret =usb_ep_enable (f_fb->in_ep, &fs_ep_in); + d =fb_ep_desc (gadget, &fs_ep_in, &hs_ep_in); + ret =usb_ep_enable (f_fb->in_ep, d);if (ret) {puts ("failed to enable in ep\n" ); goto err;
Despite of that:
Signature · 5 lines
Best regards, Lukasz Majewski Samsung R&D Institute Poland (SRPOL) | Linux Platform Group
PATCH 2/2 fastboot: Enable the respective speed endpoints at runtime
Hi Roger,
Quoted 72 lines · Lukasz Majewski
Hi Roger,
Quoted 35 lines
In a dual speed configuration we need to check at runtime if we want to enable the Full-Speed or High-Speed endpoint.
Signed-off-by Roger Quadros <[email protected]>--- drivers/usb/gadget/f_fastboot.c | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-)(quoted diff) +0
-0
b/drivers/usb/gadget/f_fastboot.cindex e1038ea..8ab187e 100644drivers/usb/gadget/f_fastboot.c +10
-0@@ -115,6 +115,15 @@ static struct usb_descriptor_header *fb_hs_function[] = { NULL, }; +static struct usb_endpoint_descriptor * +fb_ep_desc (struct usb_gadget *g,struct usb_endpoint_descriptor *fs, +struct usb_endpoint_descriptor *hs) +{ +if (gadget_is_dualspeed (g) && g->speed ==USB_SPEED_HIGH ) +return hs; +return fs; +} +/* * static strings, in UTF-8 */ @@ -255,18 +264,19 @@ static int fastboot_set_alt(struct usb_function *f, struct usb_composite_dev *cdev = f->config->cdev;struct usb_gadget *gadget = cdev->gadget;struct f_fastboot *f_fb =func_to_fastboot (f); +const struct usb_endpoint_descriptor *d; debug("%s: func: %s intf: %d alt: %d\n",drivers/usb/gadget/f_fastboot.c static int fastboot_set_alt(struct usb_function · line 276
Quoted 10 lines
drivers/usb/gadget/f_fastboot.c +0
-3__func__, f->name, interface, alt); -/* make sure we don't enable the ep twice */ if (gadget->speed ==USB_SPEED_HIGH ) { - ret =usb_ep_enable (f_fb->out_ep, &hs_ep_out); is_high_speed = true; }else { - ret =usb_ep_enable (f_fb->out_ep, &fs_ep_out); is_high_speed = false; }Very minor remark -
Those {} are not needed.Quoted 7 lines
drivers/usb/gadget/f_fastboot.c +3
-0+ + d =fb_ep_desc (gadget, &fs_ep_out, &hs_ep_out); + ret =usb_ep_enable (f_fb->out_ep, d);if (ret) {puts ("failed to enable out ep\n" );return ret;drivers/usb/gadget/f_fastboot.c static int fastboot_set_alt(struct usb_function · line 297
Quoted 9 lines
drivers/usb/gadget/f_fastboot.c +2
-1@@ -280,7 +290,8 @@ static int fastboot_set_alt(struct usb_function *f, } f_fb->out_req->complete = rx_handler_command; - ret =usb_ep_enable (f_fb->in_ep, &fs_ep_in); + d =fb_ep_desc (gadget, &fs_ep_in, &hs_ep_in); + ret =usb_ep_enable (f_fb->in_ep, d);if (ret) {puts ("failed to enable in ep\n" ); goto err;Despite of that:
Quoted 8 lines
Acked-by Lukasz Majewski <[email protected]>--
Best regards,Lukasz Majewski
Samsung R&D Institute Poland (SRPOL) | Linux Platform Group
Thanks, Steve
PATCH v2 2/2 fastboot: Enable the respective speed endpoints at runtime
In reply to Roger QuadrosIn a dual speed configuration we need to check at runtime if we want to enable the Full-Speed or High-Speed endpoint.
---
v2:
- removed unnecessary braces in if statement.
drivers/usb/gadget/f_fastboot.c | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-)
drivers/usb/gadget/f_fastboot.c +17 -7
@@ -115,6 +115,15 @@ static struct usb_descriptor_header *fb_hs_function[] = { NULL, }; +static struct usb_endpoint_descriptor * +fb_ep_desc (struct usb_gadget *g,struct usb_endpoint_descriptor *fs, +struct usb_endpoint_descriptor *hs) +{ +if (gadget_is_dualspeed (g) && g->speed ==USB_SPEED_HIGH ) +return hs; +return fs; +} +/* * static strings, in UTF-8 */
@@ -255,18 +264,18 @@ static int fastboot_set_alt(struct usb_function *f,struct usb_composite_dev *cdev = f->config->cdev;struct usb_gadget *gadget = cdev->gadget;struct f_fastboot *f_fb =func_to_fastboot (f); +const struct usb_endpoint_descriptor *d;debug ("%s: func: %s intf: %d alt: %d\n" , __func__, f->name, interface, alt); -/* make sure we don't enable the ep twice */ -if (gadget->speed ==USB_SPEED_HIGH ) { - ret =usb_ep_enable (f_fb->out_ep, &hs_ep_out); +if (gadget->speed ==USB_SPEED_HIGH ) is_high_speed = true; - }else { - ret =usb_ep_enable (f_fb->out_ep, &fs_ep_out); +else is_high_speed = false; - } + + d =fb_ep_desc (gadget, &fs_ep_out, &hs_ep_out); + ret =usb_ep_enable (f_fb->out_ep, d);if (ret) {puts ("failed to enable out ep\n" );return ret;
@@ -280,7 +289,8 @@ static int fastboot_set_alt(struct usb_function *f, } f_fb->out_req->complete = rx_handler_command; - ret =usb_ep_enable (f_fb->in_ep, &fs_ep_in); + d =fb_ep_desc (gadget, &fs_ep_in, &hs_ep_in); + ret =usb_ep_enable (f_fb->in_ep, d);if (ret) {puts ("failed to enable in ep\n" ); goto err;
Signature · 1 line
2.5.0