All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH 0/2] uboot: fastboot: fix high speed endpoint descriptors
@ 2016-04-12 12:51 Roger Quadros
  2016-04-12 12:51 ` [U-Boot] [PATCH 1/2] fastboot: Fix wMaxPacketSize for High-Speed IN endpoint Roger Quadros
  2016-04-12 12:51 ` [U-Boot] [PATCH 2/2] fastboot: Enable the respective speed endpoints at runtime Roger Quadros
  0 siblings, 2 replies; 11+ messages in thread
From: Roger Quadros @ 2016-04-12 12:51 UTC (permalink / raw)
  To: u-boot

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(-)

-- 
2.5.0

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [U-Boot] [PATCH 1/2] fastboot: Fix wMaxPacketSize for High-Speed IN endpoint
  2016-04-12 12:51 [U-Boot] [PATCH 0/2] uboot: fastboot: fix high speed endpoint descriptors Roger Quadros
@ 2016-04-12 12:51 ` Roger Quadros
  2016-04-12 13:54   ` Lukasz Majewski
  2016-04-13  2:18   ` Steve Rae
  2016-04-12 12:51 ` [U-Boot] [PATCH 2/2] fastboot: Enable the respective speed endpoints at runtime Roger Quadros
  1 sibling, 2 replies; 11+ messages in thread
From: Roger Quadros @ 2016-04-12 12:51 UTC (permalink / raw)
  To: u-boot

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

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [U-Boot] [PATCH 2/2] fastboot: Enable the respective speed endpoints at runtime
  2016-04-12 12:51 [U-Boot] [PATCH 0/2] uboot: fastboot: fix high speed endpoint descriptors Roger Quadros
  2016-04-12 12:51 ` [U-Boot] [PATCH 1/2] fastboot: Fix wMaxPacketSize for High-Speed IN endpoint Roger Quadros
@ 2016-04-12 12:51 ` Roger Quadros
  2016-04-12 13:47   ` Lukasz Majewski
  2016-04-13  8:30   ` [U-Boot] [PATCH v2 " Roger Quadros
  1 sibling, 2 replies; 11+ messages in thread
From: Roger Quadros @ 2016-04-12 12:51 UTC (permalink / raw)
  To: u-boot

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 <rogerq@ti.com>
---
 drivers/usb/gadget/f_fastboot.c | 19 +++++++++++++++----
 1 file changed, 15 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/gadget/f_fastboot.c b/drivers/usb/gadget/f_fastboot.c
index e1038ea..8ab187e 100644
--- a/drivers/usb/gadget/f_fastboot.c
+++ b/drivers/usb/gadget/f_fastboot.c
@@ -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;
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [U-Boot] [PATCH 2/2] fastboot: Enable the respective speed endpoints at runtime
  2016-04-12 12:51 ` [U-Boot] [PATCH 2/2] fastboot: Enable the respective speed endpoints at runtime Roger Quadros
@ 2016-04-12 13:47   ` Lukasz Majewski
  2016-04-13  1:43     ` Steve Rae
  2016-04-13  8:30   ` [U-Boot] [PATCH v2 " Roger Quadros
  1 sibling, 1 reply; 11+ messages in thread
From: Lukasz Majewski @ 2016-04-12 13:47 UTC (permalink / raw)
  To: u-boot

Hi Roger,

> 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 <rogerq@ti.com>
> ---
>  drivers/usb/gadget/f_fastboot.c | 19 +++++++++++++++----
>  1 file changed, 15 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/usb/gadget/f_fastboot.c
> b/drivers/usb/gadget/f_fastboot.c index e1038ea..8ab187e 100644
> --- a/drivers/usb/gadget/f_fastboot.c
> +++ b/drivers/usb/gadget/f_fastboot.c
> @@ -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;
>  	}

Very minor remark - 
Those {} are not needed.


> +
> +	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;


Despite of that:

Acked-by: Lukasz Majewski <l.majewski@samsung.com>

-- 
Best regards,

Lukasz Majewski

Samsung R&D Institute Poland (SRPOL) | Linux Platform Group

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [U-Boot] [PATCH 1/2] fastboot: Fix wMaxPacketSize for High-Speed IN endpoint
  2016-04-12 12:51 ` [U-Boot] [PATCH 1/2] fastboot: Fix wMaxPacketSize for High-Speed IN endpoint Roger Quadros
@ 2016-04-12 13:54   ` Lukasz Majewski
  2016-04-13  1:42     ` Steve Rae
  2016-04-13  2:18   ` Steve Rae
  1 sibling, 1 reply; 11+ messages in thread
From: Lukasz Majewski @ 2016-04-12 13:54 UTC (permalink / raw)
  To: u-boot

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.

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [U-Boot] [PATCH 1/2] fastboot: Fix wMaxPacketSize for High-Speed IN endpoint
  2016-04-12 13:54   ` Lukasz Majewski
@ 2016-04-13  1:42     ` Steve Rae
  2016-04-13  7:19       ` Roger Quadros
  0 siblings, 1 reply; 11+ messages in thread
From: Steve Rae @ 2016-04-13  1:42 UTC (permalink / raw)
  To: u-boot

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

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [U-Boot] [PATCH 2/2] fastboot: Enable the respective speed endpoints at runtime
  2016-04-12 13:47   ` Lukasz Majewski
@ 2016-04-13  1:43     ` Steve Rae
  0 siblings, 0 replies; 11+ messages in thread
From: Steve Rae @ 2016-04-13  1:43 UTC (permalink / raw)
  To: u-boot

Hi Roger,

On Tue, Apr 12, 2016 at 6:47 AM, Lukasz Majewski <l.majewski@samsung.com> wrote:
> Hi Roger,
>
>> 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 <rogerq@ti.com>
>> ---
>>  drivers/usb/gadget/f_fastboot.c | 19 +++++++++++++++----
>>  1 file changed, 15 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/usb/gadget/f_fastboot.c
>> b/drivers/usb/gadget/f_fastboot.c index e1038ea..8ab187e 100644
>> --- a/drivers/usb/gadget/f_fastboot.c
>> +++ b/drivers/usb/gadget/f_fastboot.c
>> @@ -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;
>>       }
>
> Very minor remark -
> Those {} are not needed.
>
>
>> +
>> +     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;
>
>
> Despite of that:
>
> Acked-by: Lukasz Majewski <l.majewski@samsung.com>
>
> --
> Best regards,
>
> Lukasz Majewski
>
> Samsung R&D Institute Poland (SRPOL) | Linux Platform Group

Tested-by: Steve Rae <srae@broadcom.com>
[Test HW: bcm235xx board]

Thanks, Steve

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [U-Boot] [PATCH 1/2] fastboot: Fix wMaxPacketSize for High-Speed IN endpoint
  2016-04-12 12:51 ` [U-Boot] [PATCH 1/2] fastboot: Fix wMaxPacketSize for High-Speed IN endpoint Roger Quadros
  2016-04-12 13:54   ` Lukasz Majewski
@ 2016-04-13  2:18   ` Steve Rae
  2016-04-13  2:34     ` Hong Chen
  1 sibling, 1 reply; 11+ messages in thread
From: Steve Rae @ 2016-04-13  2:18 UTC (permalink / raw)
  To: u-boot

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
>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [U-Boot] [PATCH 1/2] fastboot: Fix wMaxPacketSize for High-Speed IN endpoint
  2016-04-13  2:18   ` Steve Rae
@ 2016-04-13  2:34     ` Hong Chen
  0 siblings, 0 replies; 11+ messages in thread
From: Hong Chen @ 2016-04-13  2:34 UTC (permalink / raw)
  To: u-boot

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 <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
>
_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [U-Boot] [PATCH 1/2] fastboot: Fix wMaxPacketSize for High-Speed IN endpoint
  2016-04-13  1:42     ` Steve Rae
@ 2016-04-13  7:19       ` Roger Quadros
  0 siblings, 0 replies; 11+ messages in thread
From: Roger Quadros @ 2016-04-13  7:19 UTC (permalink / raw)
  To: 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 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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [U-Boot] [PATCH v2 2/2] fastboot: Enable the respective speed endpoints at runtime
  2016-04-12 12:51 ` [U-Boot] [PATCH 2/2] fastboot: Enable the respective speed endpoints at runtime Roger Quadros
  2016-04-12 13:47   ` Lukasz Majewski
@ 2016-04-13  8:30   ` Roger Quadros
  1 sibling, 0 replies; 11+ messages in thread
From: Roger Quadros @ 2016-04-13  8:30 UTC (permalink / raw)
  To: u-boot

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 <rogerq@ti.com>
Acked-by: Lukasz Majewski <l.majewski@samsung.com>
Tested-by: Steve Rae <srae@broadcom.com>
[Test HW: bcm235xx board]
---
v2:
- removed unnecessary braces in if statement.

 drivers/usb/gadget/f_fastboot.c | 24 +++++++++++++++++-------
 1 file changed, 17 insertions(+), 7 deletions(-)

diff --git a/drivers/usb/gadget/f_fastboot.c b/drivers/usb/gadget/f_fastboot.c
index e1038ea..7961231 100644
--- a/drivers/usb/gadget/f_fastboot.c
+++ b/drivers/usb/gadget/f_fastboot.c
@@ -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;
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2016-04-13  8:30 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-04-12 12:51 [U-Boot] [PATCH 0/2] uboot: fastboot: fix high speed endpoint descriptors Roger Quadros
2016-04-12 12:51 ` [U-Boot] [PATCH 1/2] fastboot: Fix wMaxPacketSize for High-Speed IN endpoint Roger Quadros
2016-04-12 13:54   ` Lukasz Majewski
2016-04-13  1:42     ` Steve Rae
2016-04-13  7:19       ` Roger Quadros
2016-04-13  2:18   ` Steve Rae
2016-04-13  2:34     ` Hong Chen
2016-04-12 12:51 ` [U-Boot] [PATCH 2/2] fastboot: Enable the respective speed endpoints at runtime Roger Quadros
2016-04-12 13:47   ` Lukasz Majewski
2016-04-13  1:43     ` Steve Rae
2016-04-13  8:30   ` [U-Boot] [PATCH v2 " Roger Quadros

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.