* [U-Boot] [PATCH 0/3] s3c-otg: fastboot: fixes and cleanup @ 2016-04-19 7:16 Roger Quadros 2016-04-19 7:16 ` [U-Boot] [PATCH 1/3] fastboot: Clean up bulk-out logic Roger Quadros ` (2 more replies) 0 siblings, 3 replies; 17+ messages in thread From: Roger Quadros @ 2016-04-19 7:16 UTC (permalink / raw) To: u-boot Hi, These are the patches discussed in the thread [1]. This should fix fastboot on boards using s3c-otg/dwc2 gadget controller. [1] - https://www.mail-archive.com/u-boot at lists.denx.de/msg209740.html NOTE: I haven't tested this extensively on TI platforms other than just a fastboot download command. -- cheers, -roger Roger Quadros (3): fastboot: Clean up bulk-out logic usb: s3c-otg: Fix short packet for request size > ep.maxpacket usb: s3c-otg: Fix remaining bytes in debug messages drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c | 6 ++-- drivers/usb/gadget/f_fastboot.c | 50 ++++++++++++++---------------- 2 files changed, 26 insertions(+), 30 deletions(-) -- 2.5.0 ^ permalink raw reply [flat|nested] 17+ messages in thread
* [U-Boot] [PATCH 1/3] fastboot: Clean up bulk-out logic 2016-04-19 7:16 [U-Boot] [PATCH 0/3] s3c-otg: fastboot: fixes and cleanup Roger Quadros @ 2016-04-19 7:16 ` Roger Quadros 2016-04-19 9:00 ` Lukasz Majewski 2016-04-21 10:03 ` Lukasz Majewski 2016-04-19 7:17 ` [U-Boot] [PATCH 2/3] usb: s3c-otg: Fix short packet for request size > ep.maxpacket Roger Quadros 2016-04-19 7:17 ` [U-Boot] [PATCH 3/3] usb: s3c-otg: Fix remaining bytes in debug messages Roger Quadros 2 siblings, 2 replies; 17+ messages in thread From: Roger Quadros @ 2016-04-19 7:16 UTC (permalink / raw) To: u-boot Just use ep->maxpacket to get the maxpacket size and simplify the bulk-out maxpacket alignment. Signed-off-by: Roger Quadros <rogerq@ti.com> Tested-by: Steve Rae <srae@broadcom.com> --- drivers/usb/gadget/f_fastboot.c | 50 +++++++++++++++++++---------------------- 1 file changed, 23 insertions(+), 27 deletions(-) diff --git a/drivers/usb/gadget/f_fastboot.c b/drivers/usb/gadget/f_fastboot.c index 7961231..28b244a 100644 --- a/drivers/usb/gadget/f_fastboot.c +++ b/drivers/usb/gadget/f_fastboot.c @@ -39,6 +39,11 @@ #define TX_ENDPOINT_MAXIMUM_PACKET_SIZE (0x0040) #define EP_BUFFER_SIZE 4096 +/* + * EP_BUFFER_SIZE must always be an integral multiple of maxpacket size + * (64 or 512 or 1024), else we break on certain controllers like DWC3 + * that expect bulk OUT requests to be divisible by maxpacket size. + */ struct f_fastboot { struct usb_function usb_function; @@ -57,7 +62,6 @@ static struct f_fastboot *fastboot_func; static unsigned int fastboot_flash_session_id; static unsigned int download_size; static unsigned int download_bytes; -static bool is_high_speed; static struct usb_endpoint_descriptor fs_ep_in = { .bLength = USB_DT_ENDPOINT_SIZE, @@ -269,11 +273,6 @@ static int fastboot_set_alt(struct usb_function *f, debug("%s: func: %s intf: %d alt: %d\n", __func__, f->name, interface, alt); - if (gadget->speed == USB_SPEED_HIGH) - is_high_speed = true; - 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) { @@ -455,20 +454,27 @@ static void cb_getvar(struct usb_ep *ep, struct usb_request *req) fastboot_tx_write_str(response); } -static unsigned int rx_bytes_expected(unsigned int maxpacket) +static unsigned int rx_bytes_expected(struct usb_ep *ep) { int rx_remain = download_size - download_bytes; - int rem = 0; - if (rx_remain < 0) + unsigned int rem; + unsigned int maxpacket = ep->maxpacket; + + if (rx_remain <= 0) return 0; - if (rx_remain > EP_BUFFER_SIZE) + else if (rx_remain > EP_BUFFER_SIZE) return EP_BUFFER_SIZE; - if (rx_remain < maxpacket) { - rx_remain = maxpacket; - } else if (rx_remain % maxpacket != 0) { - rem = rx_remain % maxpacket; + + /* + * Some controllers e.g. DWC3 don't like OUT transfers to be + * not ending in maxpacket boundary. So just make them happy by + * always requesting for integral multiple of maxpackets. + * This shouldn't bother controllers that don't care about it. + */ + rem = rx_remain % maxpacket; + if (rem > 0) rx_remain = rx_remain + (maxpacket - rem); - } + return rx_remain; } @@ -480,7 +486,6 @@ static void rx_handler_dl_image(struct usb_ep *ep, struct usb_request *req) const unsigned char *buffer = req->buf; unsigned int buffer_size = req->actual; unsigned int pre_dot_num, now_dot_num; - unsigned int max; if (req->status != 0) { printf("Bad status: %d\n", req->status); @@ -518,11 +523,7 @@ static void rx_handler_dl_image(struct usb_ep *ep, struct usb_request *req) printf("\ndownloading of %d bytes finished\n", download_bytes); } else { - max = is_high_speed ? hs_ep_out.wMaxPacketSize : - fs_ep_out.wMaxPacketSize; - req->length = rx_bytes_expected(max); - if (req->length < ep->maxpacket) - req->length = ep->maxpacket; + req->length = rx_bytes_expected(ep); } req->actual = 0; @@ -533,7 +534,6 @@ static void cb_download(struct usb_ep *ep, struct usb_request *req) { char *cmd = req->buf; char response[FASTBOOT_RESPONSE_LEN]; - unsigned int max; strsep(&cmd, ":"); download_size = simple_strtoul(cmd, NULL, 16); @@ -549,11 +549,7 @@ static void cb_download(struct usb_ep *ep, struct usb_request *req) } else { sprintf(response, "DATA%08x", download_size); req->complete = rx_handler_dl_image; - max = is_high_speed ? hs_ep_out.wMaxPacketSize : - fs_ep_out.wMaxPacketSize; - req->length = rx_bytes_expected(max); - if (req->length < ep->maxpacket) - req->length = ep->maxpacket; + req->length = rx_bytes_expected(ep); } fastboot_tx_write_str(response); } -- 2.5.0 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* [U-Boot] [PATCH 1/3] fastboot: Clean up bulk-out logic 2016-04-19 7:16 ` [U-Boot] [PATCH 1/3] fastboot: Clean up bulk-out logic Roger Quadros @ 2016-04-19 9:00 ` Lukasz Majewski 2016-04-21 10:03 ` Lukasz Majewski 1 sibling, 0 replies; 17+ messages in thread From: Lukasz Majewski @ 2016-04-19 9:00 UTC (permalink / raw) To: u-boot Hi Roger, > Just use ep->maxpacket to get the maxpacket size > and simplify the bulk-out maxpacket alignment. > > Signed-off-by: Roger Quadros <rogerq@ti.com> > Tested-by: Steve Rae <srae@broadcom.com> > --- > drivers/usb/gadget/f_fastboot.c | 50 > +++++++++++++++++++---------------------- 1 file changed, 23 > insertions(+), 27 deletions(-) > > diff --git a/drivers/usb/gadget/f_fastboot.c > b/drivers/usb/gadget/f_fastboot.c index 7961231..28b244a 100644 > --- a/drivers/usb/gadget/f_fastboot.c > +++ b/drivers/usb/gadget/f_fastboot.c > @@ -39,6 +39,11 @@ > #define TX_ENDPOINT_MAXIMUM_PACKET_SIZE (0x0040) > > #define EP_BUFFER_SIZE 4096 > +/* > + * EP_BUFFER_SIZE must always be an integral multiple of maxpacket > size > + * (64 or 512 or 1024), else we break on certain controllers like > DWC3 > + * that expect bulk OUT requests to be divisible by maxpacket size. > + */ > > struct f_fastboot { > struct usb_function usb_function; > @@ -57,7 +62,6 @@ static struct f_fastboot *fastboot_func; > static unsigned int fastboot_flash_session_id; > static unsigned int download_size; > static unsigned int download_bytes; > -static bool is_high_speed; > > static struct usb_endpoint_descriptor fs_ep_in = { > .bLength = USB_DT_ENDPOINT_SIZE, > @@ -269,11 +273,6 @@ static int fastboot_set_alt(struct usb_function > *f, debug("%s: func: %s intf: %d alt: %d\n", > __func__, f->name, interface, alt); > > - if (gadget->speed == USB_SPEED_HIGH) > - is_high_speed = true; > - 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) { > @@ -455,20 +454,27 @@ static void cb_getvar(struct usb_ep *ep, struct > usb_request *req) fastboot_tx_write_str(response); > } > > -static unsigned int rx_bytes_expected(unsigned int maxpacket) > +static unsigned int rx_bytes_expected(struct usb_ep *ep) > { > int rx_remain = download_size - download_bytes; > - int rem = 0; > - if (rx_remain < 0) > + unsigned int rem; > + unsigned int maxpacket = ep->maxpacket; > + > + if (rx_remain <= 0) > return 0; > - if (rx_remain > EP_BUFFER_SIZE) > + else if (rx_remain > EP_BUFFER_SIZE) > return EP_BUFFER_SIZE; > - if (rx_remain < maxpacket) { > - rx_remain = maxpacket; > - } else if (rx_remain % maxpacket != 0) { > - rem = rx_remain % maxpacket; > + > + /* > + * Some controllers e.g. DWC3 don't like OUT transfers to be > + * not ending in maxpacket boundary. So just make them happy > by > + * always requesting for integral multiple of maxpackets. > + * This shouldn't bother controllers that don't care about > it. > + */ > + rem = rx_remain % maxpacket; > + if (rem > 0) > rx_remain = rx_remain + (maxpacket - rem); > - } > + > return rx_remain; > } > > @@ -480,7 +486,6 @@ static void rx_handler_dl_image(struct usb_ep > *ep, struct usb_request *req) const unsigned char *buffer = req->buf; > unsigned int buffer_size = req->actual; > unsigned int pre_dot_num, now_dot_num; > - unsigned int max; > > if (req->status != 0) { > printf("Bad status: %d\n", req->status); > @@ -518,11 +523,7 @@ static void rx_handler_dl_image(struct usb_ep > *ep, struct usb_request *req) > printf("\ndownloading of %d bytes finished\n", > download_bytes); } else { > - max = is_high_speed ? hs_ep_out.wMaxPacketSize : > - fs_ep_out.wMaxPacketSize; > - req->length = rx_bytes_expected(max); > - if (req->length < ep->maxpacket) > - req->length = ep->maxpacket; > + req->length = rx_bytes_expected(ep); > } > > req->actual = 0; > @@ -533,7 +534,6 @@ static void cb_download(struct usb_ep *ep, struct > usb_request *req) { > char *cmd = req->buf; > char response[FASTBOOT_RESPONSE_LEN]; > - unsigned int max; > > strsep(&cmd, ":"); > download_size = simple_strtoul(cmd, NULL, 16); > @@ -549,11 +549,7 @@ static void cb_download(struct usb_ep *ep, > struct usb_request *req) } else { > sprintf(response, "DATA%08x", download_size); > req->complete = rx_handler_dl_image; > - max = is_high_speed ? hs_ep_out.wMaxPacketSize : > - fs_ep_out.wMaxPacketSize; > - req->length = rx_bytes_expected(max); > - if (req->length < ep->maxpacket) > - req->length = ep->maxpacket; > + req->length = rx_bytes_expected(ep); > } > fastboot_tx_write_str(response); > } Reviewed-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] 17+ messages in thread
* [U-Boot] [PATCH 1/3] fastboot: Clean up bulk-out logic 2016-04-19 7:16 ` [U-Boot] [PATCH 1/3] fastboot: Clean up bulk-out logic Roger Quadros 2016-04-19 9:00 ` Lukasz Majewski @ 2016-04-21 10:03 ` Lukasz Majewski 2016-04-22 2:59 ` Steve Rae 1 sibling, 1 reply; 17+ messages in thread From: Lukasz Majewski @ 2016-04-21 10:03 UTC (permalink / raw) To: u-boot Hi Roger, > Just use ep->maxpacket to get the maxpacket size > and simplify the bulk-out maxpacket alignment. > > Signed-off-by: Roger Quadros <rogerq@ti.com> > Tested-by: Steve Rae <srae@broadcom.com> > --- > drivers/usb/gadget/f_fastboot.c | 50 > +++++++++++++++++++---------------------- 1 file changed, 23 > insertions(+), 27 deletions(-) > > diff --git a/drivers/usb/gadget/f_fastboot.c > b/drivers/usb/gadget/f_fastboot.c index 7961231..28b244a 100644 > --- a/drivers/usb/gadget/f_fastboot.c > +++ b/drivers/usb/gadget/f_fastboot.c > @@ -39,6 +39,11 @@ > #define TX_ENDPOINT_MAXIMUM_PACKET_SIZE (0x0040) > > #define EP_BUFFER_SIZE 4096 > +/* > + * EP_BUFFER_SIZE must always be an integral multiple of maxpacket > size > + * (64 or 512 or 1024), else we break on certain controllers like > DWC3 > + * that expect bulk OUT requests to be divisible by maxpacket size. > + */ > > struct f_fastboot { > struct usb_function usb_function; > @@ -57,7 +62,6 @@ static struct f_fastboot *fastboot_func; > static unsigned int fastboot_flash_session_id; > static unsigned int download_size; > static unsigned int download_bytes; > -static bool is_high_speed; > > static struct usb_endpoint_descriptor fs_ep_in = { > .bLength = USB_DT_ENDPOINT_SIZE, > @@ -269,11 +273,6 @@ static int fastboot_set_alt(struct usb_function > *f, debug("%s: func: %s intf: %d alt: %d\n", > __func__, f->name, interface, alt); > > - if (gadget->speed == USB_SPEED_HIGH) > - is_high_speed = true; > - 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) { > @@ -455,20 +454,27 @@ static void cb_getvar(struct usb_ep *ep, struct > usb_request *req) fastboot_tx_write_str(response); > } > > -static unsigned int rx_bytes_expected(unsigned int maxpacket) > +static unsigned int rx_bytes_expected(struct usb_ep *ep) > { > int rx_remain = download_size - download_bytes; > - int rem = 0; > - if (rx_remain < 0) > + unsigned int rem; > + unsigned int maxpacket = ep->maxpacket; > + > + if (rx_remain <= 0) > return 0; > - if (rx_remain > EP_BUFFER_SIZE) > + else if (rx_remain > EP_BUFFER_SIZE) > return EP_BUFFER_SIZE; > - if (rx_remain < maxpacket) { > - rx_remain = maxpacket; > - } else if (rx_remain % maxpacket != 0) { > - rem = rx_remain % maxpacket; > + > + /* > + * Some controllers e.g. DWC3 don't like OUT transfers to be > + * not ending in maxpacket boundary. So just make them happy > by > + * always requesting for integral multiple of maxpackets. > + * This shouldn't bother controllers that don't care about > it. > + */ > + rem = rx_remain % maxpacket; > + if (rem > 0) > rx_remain = rx_remain + (maxpacket - rem); > - } > + > return rx_remain; > } > > @@ -480,7 +486,6 @@ static void rx_handler_dl_image(struct usb_ep > *ep, struct usb_request *req) const unsigned char *buffer = req->buf; > unsigned int buffer_size = req->actual; > unsigned int pre_dot_num, now_dot_num; > - unsigned int max; > > if (req->status != 0) { > printf("Bad status: %d\n", req->status); > @@ -518,11 +523,7 @@ static void rx_handler_dl_image(struct usb_ep > *ep, struct usb_request *req) > printf("\ndownloading of %d bytes finished\n", > download_bytes); } else { > - max = is_high_speed ? hs_ep_out.wMaxPacketSize : > - fs_ep_out.wMaxPacketSize; > - req->length = rx_bytes_expected(max); > - if (req->length < ep->maxpacket) > - req->length = ep->maxpacket; > + req->length = rx_bytes_expected(ep); > } > > req->actual = 0; > @@ -533,7 +534,6 @@ static void cb_download(struct usb_ep *ep, struct > usb_request *req) { > char *cmd = req->buf; > char response[FASTBOOT_RESPONSE_LEN]; > - unsigned int max; > > strsep(&cmd, ":"); > download_size = simple_strtoul(cmd, NULL, 16); > @@ -549,11 +549,7 @@ static void cb_download(struct usb_ep *ep, > struct usb_request *req) } else { > sprintf(response, "DATA%08x", download_size); > req->complete = rx_handler_dl_image; > - max = is_high_speed ? hs_ep_out.wMaxPacketSize : > - fs_ep_out.wMaxPacketSize; > - req->length = rx_bytes_expected(max); > - if (req->length < ep->maxpacket) > - req->length = ep->maxpacket; > + req->length = rx_bytes_expected(ep); > } > fastboot_tx_write_str(response); > } I cannot apply this patch on temporary u-boot-usb/master branch (SHA1: e6c0bc0643e5a4387fecbcf83080d0b796eb067c). Could you apply your changes (from this patch) on top of newest u-boot-usb repo? -- Best regards, Lukasz Majewski Samsung R&D Institute Poland (SRPOL) | Linux Platform Group ^ permalink raw reply [flat|nested] 17+ messages in thread
* [U-Boot] [PATCH 1/3] fastboot: Clean up bulk-out logic 2016-04-21 10:03 ` Lukasz Majewski @ 2016-04-22 2:59 ` Steve Rae 2016-04-22 3:52 ` Lukasz Majewski 0 siblings, 1 reply; 17+ messages in thread From: Steve Rae @ 2016-04-22 2:59 UTC (permalink / raw) To: u-boot On Thu, Apr 21, 2016 at 3:03 AM, Lukasz Majewski <l.majewski@samsung.com> wrote: > Hi Roger, > > > Just use ep->maxpacket to get the maxpacket size > > and simplify the bulk-out maxpacket alignment. > > > > Signed-off-by: Roger Quadros <rogerq@ti.com> > > Tested-by: Steve Rae <srae@broadcom.com> > > --- > > drivers/usb/gadget/f_fastboot.c | 50 > > +++++++++++++++++++---------------------- 1 file changed, 23 > > insertions(+), 27 deletions(-) > > > > diff --git a/drivers/usb/gadget/f_fastboot.c > > b/drivers/usb/gadget/f_fastboot.c index 7961231..28b244a 100644 > > --- a/drivers/usb/gadget/f_fastboot.c > > +++ b/drivers/usb/gadget/f_fastboot.c > > @@ -39,6 +39,11 @@ > > #define TX_ENDPOINT_MAXIMUM_PACKET_SIZE (0x0040) > > > > #define EP_BUFFER_SIZE 4096 > > +/* > > + * EP_BUFFER_SIZE must always be an integral multiple of maxpacket > > size > > + * (64 or 512 or 1024), else we break on certain controllers like > > DWC3 > > + * that expect bulk OUT requests to be divisible by maxpacket size. > > + */ > > > > struct f_fastboot { > > struct usb_function usb_function; > > @@ -57,7 +62,6 @@ static struct f_fastboot *fastboot_func; > > static unsigned int fastboot_flash_session_id; > > static unsigned int download_size; > > static unsigned int download_bytes; > > -static bool is_high_speed; > > > > static struct usb_endpoint_descriptor fs_ep_in = { > > .bLength = USB_DT_ENDPOINT_SIZE, > > @@ -269,11 +273,6 @@ static int fastboot_set_alt(struct usb_function > > *f, debug("%s: func: %s intf: %d alt: %d\n", > > __func__, f->name, interface, alt); > > > > - if (gadget->speed == USB_SPEED_HIGH) > > - is_high_speed = true; > > - 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) { > > @@ -455,20 +454,27 @@ static void cb_getvar(struct usb_ep *ep, struct > > usb_request *req) fastboot_tx_write_str(response); > > } > > > > -static unsigned int rx_bytes_expected(unsigned int maxpacket) > > +static unsigned int rx_bytes_expected(struct usb_ep *ep) > > { > > int rx_remain = download_size - download_bytes; > > - int rem = 0; > > - if (rx_remain < 0) > > + unsigned int rem; > > + unsigned int maxpacket = ep->maxpacket; > > + > > + if (rx_remain <= 0) > > return 0; > > - if (rx_remain > EP_BUFFER_SIZE) > > + else if (rx_remain > EP_BUFFER_SIZE) > > return EP_BUFFER_SIZE; > > - if (rx_remain < maxpacket) { > > - rx_remain = maxpacket; > > - } else if (rx_remain % maxpacket != 0) { > > - rem = rx_remain % maxpacket; > > + > > + /* > > + * Some controllers e.g. DWC3 don't like OUT transfers to be > > + * not ending in maxpacket boundary. So just make them happy > > by > > + * always requesting for integral multiple of maxpackets. > > + * This shouldn't bother controllers that don't care about > > it. > > + */ > > + rem = rx_remain % maxpacket; > > + if (rem > 0) > > rx_remain = rx_remain + (maxpacket - rem); > > - } > > + > > return rx_remain; > > } > > > > @@ -480,7 +486,6 @@ static void rx_handler_dl_image(struct usb_ep > > *ep, struct usb_request *req) const unsigned char *buffer = req->buf; > > unsigned int buffer_size = req->actual; > > unsigned int pre_dot_num, now_dot_num; > > - unsigned int max; > > > > if (req->status != 0) { > > printf("Bad status: %d\n", req->status); > > @@ -518,11 +523,7 @@ static void rx_handler_dl_image(struct usb_ep > > *ep, struct usb_request *req) > > printf("\ndownloading of %d bytes finished\n", > > download_bytes); } else { > > - max = is_high_speed ? hs_ep_out.wMaxPacketSize : > > - fs_ep_out.wMaxPacketSize; > > - req->length = rx_bytes_expected(max); > > - if (req->length < ep->maxpacket) > > - req->length = ep->maxpacket; > > + req->length = rx_bytes_expected(ep); > > } > > > > req->actual = 0; > > @@ -533,7 +534,6 @@ static void cb_download(struct usb_ep *ep, struct > > usb_request *req) { > > char *cmd = req->buf; > > char response[FASTBOOT_RESPONSE_LEN]; > > - unsigned int max; > > > > strsep(&cmd, ":"); > > download_size = simple_strtoul(cmd, NULL, 16); > > @@ -549,11 +549,7 @@ static void cb_download(struct usb_ep *ep, > > struct usb_request *req) } else { > > sprintf(response, "DATA%08x", download_size); > > req->complete = rx_handler_dl_image; > > - max = is_high_speed ? hs_ep_out.wMaxPacketSize : > > - fs_ep_out.wMaxPacketSize; > > - req->length = rx_bytes_expected(max); > > - if (req->length < ep->maxpacket) > > - req->length = ep->maxpacket; > > + req->length = rx_bytes_expected(ep); > > } > > fastboot_tx_write_str(response); > > } > > I cannot apply this patch on temporary u-boot-usb/master branch > (SHA1: e6c0bc0643e5a4387fecbcf83080d0b796eb067c). > > Could you apply your changes (from this patch) on top of newest > u-boot-usb repo? > > Lukasz, (in case Roger doesn't reply soon....) -- this needs to be applied on top of his earlier patches: (1) http://patchwork.ozlabs.org/patch/609417/ [U-Boot,1/2] fastboot: Fix wMaxPacketSize for High-Speed IN endpoint (2) http://patchwork.ozlabs.org/patch/609896/ [U-Boot,v2,2/2] fastboot: Enable the respective speed endpoints at runtime ... then this one will apply (http://patchwork.ozlabs.org/patch/612006/) Thanks, Steve -- > Best regards, > > Lukasz Majewski > > Samsung R&D Institute Poland (SRPOL) | Linux Platform Group > ^ permalink raw reply [flat|nested] 17+ messages in thread
* [U-Boot] [PATCH 1/3] fastboot: Clean up bulk-out logic 2016-04-22 2:59 ` Steve Rae @ 2016-04-22 3:52 ` Lukasz Majewski 0 siblings, 0 replies; 17+ messages in thread From: Lukasz Majewski @ 2016-04-22 3:52 UTC (permalink / raw) To: u-boot On Thu, 21 Apr 2016 19:59:21 -0700 Steve Rae <steve.rae@broadcom.com> wrote: > On Thu, Apr 21, 2016 at 3:03 AM, Lukasz Majewski > <l.majewski@samsung.com> wrote: > > > Hi Roger, > > > > > Just use ep->maxpacket to get the maxpacket size > > > and simplify the bulk-out maxpacket alignment. > > > > > > Signed-off-by: Roger Quadros <rogerq@ti.com> > > > Tested-by: Steve Rae <srae@broadcom.com> > > > --- > > > drivers/usb/gadget/f_fastboot.c | 50 > > > +++++++++++++++++++---------------------- 1 file changed, 23 > > > insertions(+), 27 deletions(-) > > > > > > diff --git a/drivers/usb/gadget/f_fastboot.c > > > b/drivers/usb/gadget/f_fastboot.c index 7961231..28b244a 100644 > > > --- a/drivers/usb/gadget/f_fastboot.c > > > +++ b/drivers/usb/gadget/f_fastboot.c > > > @@ -39,6 +39,11 @@ > > > #define TX_ENDPOINT_MAXIMUM_PACKET_SIZE (0x0040) > > > > > > #define EP_BUFFER_SIZE 4096 > > > +/* > > > + * EP_BUFFER_SIZE must always be an integral multiple of > > > maxpacket size > > > + * (64 or 512 or 1024), else we break on certain controllers like > > > DWC3 > > > + * that expect bulk OUT requests to be divisible by maxpacket > > > size. > > > + */ > > > > > > struct f_fastboot { > > > struct usb_function usb_function; > > > @@ -57,7 +62,6 @@ static struct f_fastboot *fastboot_func; > > > static unsigned int fastboot_flash_session_id; > > > static unsigned int download_size; > > > static unsigned int download_bytes; > > > -static bool is_high_speed; > > > > > > static struct usb_endpoint_descriptor fs_ep_in = { > > > .bLength = USB_DT_ENDPOINT_SIZE, > > > @@ -269,11 +273,6 @@ static int fastboot_set_alt(struct > > > usb_function *f, debug("%s: func: %s intf: %d alt: %d\n", > > > __func__, f->name, interface, alt); > > > > > > - if (gadget->speed == USB_SPEED_HIGH) > > > - is_high_speed = true; > > > - 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) { > > > @@ -455,20 +454,27 @@ static void cb_getvar(struct usb_ep *ep, > > > struct usb_request *req) fastboot_tx_write_str(response); > > > } > > > > > > -static unsigned int rx_bytes_expected(unsigned int maxpacket) > > > +static unsigned int rx_bytes_expected(struct usb_ep *ep) > > > { > > > int rx_remain = download_size - download_bytes; > > > - int rem = 0; > > > - if (rx_remain < 0) > > > + unsigned int rem; > > > + unsigned int maxpacket = ep->maxpacket; > > > + > > > + if (rx_remain <= 0) > > > return 0; > > > - if (rx_remain > EP_BUFFER_SIZE) > > > + else if (rx_remain > EP_BUFFER_SIZE) > > > return EP_BUFFER_SIZE; > > > - if (rx_remain < maxpacket) { > > > - rx_remain = maxpacket; > > > - } else if (rx_remain % maxpacket != 0) { > > > - rem = rx_remain % maxpacket; > > > + > > > + /* > > > + * Some controllers e.g. DWC3 don't like OUT transfers to be > > > + * not ending in maxpacket boundary. So just make them happy > > > by > > > + * always requesting for integral multiple of maxpackets. > > > + * This shouldn't bother controllers that don't care about > > > it. > > > + */ > > > + rem = rx_remain % maxpacket; > > > + if (rem > 0) > > > rx_remain = rx_remain + (maxpacket - rem); > > > - } > > > + > > > return rx_remain; > > > } > > > > > > @@ -480,7 +486,6 @@ static void rx_handler_dl_image(struct usb_ep > > > *ep, struct usb_request *req) const unsigned char *buffer = > > > req->buf; unsigned int buffer_size = req->actual; > > > unsigned int pre_dot_num, now_dot_num; > > > - unsigned int max; > > > > > > if (req->status != 0) { > > > printf("Bad status: %d\n", req->status); > > > @@ -518,11 +523,7 @@ static void rx_handler_dl_image(struct usb_ep > > > *ep, struct usb_request *req) > > > printf("\ndownloading of %d bytes finished\n", > > > download_bytes); } else { > > > - max = is_high_speed ? hs_ep_out.wMaxPacketSize : > > > - fs_ep_out.wMaxPacketSize; > > > - req->length = rx_bytes_expected(max); > > > - if (req->length < ep->maxpacket) > > > - req->length = ep->maxpacket; > > > + req->length = rx_bytes_expected(ep); > > > } > > > > > > req->actual = 0; > > > @@ -533,7 +534,6 @@ static void cb_download(struct usb_ep *ep, > > > struct usb_request *req) { > > > char *cmd = req->buf; > > > char response[FASTBOOT_RESPONSE_LEN]; > > > - unsigned int max; > > > > > > strsep(&cmd, ":"); > > > download_size = simple_strtoul(cmd, NULL, 16); > > > @@ -549,11 +549,7 @@ static void cb_download(struct usb_ep *ep, > > > struct usb_request *req) } else { > > > sprintf(response, "DATA%08x", download_size); > > > req->complete = rx_handler_dl_image; > > > - max = is_high_speed ? hs_ep_out.wMaxPacketSize : > > > - fs_ep_out.wMaxPacketSize; > > > - req->length = rx_bytes_expected(max); > > > - if (req->length < ep->maxpacket) > > > - req->length = ep->maxpacket; > > > + req->length = rx_bytes_expected(ep); > > > } > > > fastboot_tx_write_str(response); > > > } > > > > I cannot apply this patch on temporary u-boot-usb/master branch > > (SHA1: e6c0bc0643e5a4387fecbcf83080d0b796eb067c). > > > > Could you apply your changes (from this patch) on top of newest > > u-boot-usb repo? > > > > Lukasz, (in case Roger doesn't reply soon....) > -- this needs to be applied on top of his earlier patches: > (1) http://patchwork.ozlabs.org/patch/609417/ > [U-Boot,1/2] fastboot: Fix wMaxPacketSize for High-Speed IN > endpoint (2) http://patchwork.ozlabs.org/patch/609896/ > [U-Boot,v2,2/2] fastboot: Enable the respective speed > endpoints at runtime > > ... then this one will apply > (http://patchwork.ozlabs.org/patch/612006/) Steve, thanks for tip. I was missing this information :-) Best regards, ?ukasz > > Thanks, Steve > > -- > > Best regards, > > > > Lukasz Majewski > > > > Samsung R&D Institute Poland (SRPOL) | Linux Platform Group > > > _______________________________________________ > U-Boot mailing list > U-Boot at lists.denx.de > http://lists.denx.de/mailman/listinfo/u-boot -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 181 bytes Desc: OpenPGP digital signature URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160422/fc936c89/attachment.sig> ^ permalink raw reply [flat|nested] 17+ messages in thread
* [U-Boot] [PATCH 2/3] usb: s3c-otg: Fix short packet for request size > ep.maxpacket 2016-04-19 7:16 [U-Boot] [PATCH 0/3] s3c-otg: fastboot: fixes and cleanup Roger Quadros 2016-04-19 7:16 ` [U-Boot] [PATCH 1/3] fastboot: Clean up bulk-out logic Roger Quadros @ 2016-04-19 7:17 ` Roger Quadros 2016-04-19 9:02 ` Lukasz Majewski 2016-04-19 12:20 ` [U-Boot] [PATCH v2 " Roger Quadros 2016-04-19 7:17 ` [U-Boot] [PATCH 3/3] usb: s3c-otg: Fix remaining bytes in debug messages Roger Quadros 2 siblings, 2 replies; 17+ messages in thread From: Roger Quadros @ 2016-04-19 7:17 UTC (permalink / raw) To: u-boot Request size can be greater than ep.packet and still end in a short packet. We need to tackle this case as end of transfer (if short_not_ok is not set) as indicated in USB 2.0 Specification [1], else we get stuck up on certain protocols like fastboot. [1] - USB2.0 Specification, Section 5.3.2 Pipes Reported-by: Steve Rae <steve.rae@broadcom.com> Signed-off-by: Roger Quadros <rogerq@ti.com> Tested-by: Steve Rae <steve.rae@broadcom.com> --- drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c b/drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c index bce9c30..a31d875 100644 --- a/drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c +++ b/drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c @@ -229,7 +229,7 @@ static void complete_rx(struct dwc2_udc *dev, u8 ep_num) ROUND(xfer_size, CONFIG_SYS_CACHELINE_SIZE)); req->req.actual += min(xfer_size, req->req.length - req->req.actual); - is_short = (xfer_size < ep->ep.maxpacket); + is_short = xfer_size % ep->ep.maxpacket; debug_cond(DEBUG_OUT_EP != 0, "%s: RX DMA done : ep = %d, rx bytes = %d/%d, " -- 2.5.0 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* [U-Boot] [PATCH 2/3] usb: s3c-otg: Fix short packet for request size > ep.maxpacket 2016-04-19 7:17 ` [U-Boot] [PATCH 2/3] usb: s3c-otg: Fix short packet for request size > ep.maxpacket Roger Quadros @ 2016-04-19 9:02 ` Lukasz Majewski 2016-04-19 10:21 ` Roger Quadros 2016-04-19 12:20 ` [U-Boot] [PATCH v2 " Roger Quadros 1 sibling, 1 reply; 17+ messages in thread From: Lukasz Majewski @ 2016-04-19 9:02 UTC (permalink / raw) To: u-boot Hi Roger, > Request size can be greater than ep.packet and still end in a > short packet. We need to tackle this case as end of transfer > (if short_not_ok is not set) as indicated in USB 2.0 Specification > [1], else we get stuck up on certain protocols like fastboot. > > [1] - USB2.0 Specification, Section 5.3.2 Pipes > > Reported-by: Steve Rae <steve.rae@broadcom.com> > Signed-off-by: Roger Quadros <rogerq@ti.com> > Tested-by: Steve Rae <steve.rae@broadcom.com> > --- > drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c > b/drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c index bce9c30..a31d875 > 100644 --- a/drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c > +++ b/drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c > @@ -229,7 +229,7 @@ static void complete_rx(struct dwc2_udc *dev, u8 > ep_num) ROUND(xfer_size, CONFIG_SYS_CACHELINE_SIZE)); > > req->req.actual += min(xfer_size, req->req.length - > req->req.actual); > - is_short = (xfer_size < ep->ep.maxpacket); > + is_short = xfer_size % ep->ep.maxpacket; is_short is a flag - so maybe it would be better to write something like: is_short = !!(xfer_size % ep->ep.maxpacket)) ? I'm going to test those patches on my boards. I will share the results ASAP. > > debug_cond(DEBUG_OUT_EP != 0, > "%s: RX DMA done : ep = %d, rx bytes = %d/%d, " -- Best regards, Lukasz Majewski Samsung R&D Institute Poland (SRPOL) | Linux Platform Group ^ permalink raw reply [flat|nested] 17+ messages in thread
* [U-Boot] [PATCH 2/3] usb: s3c-otg: Fix short packet for request size > ep.maxpacket 2016-04-19 9:02 ` Lukasz Majewski @ 2016-04-19 10:21 ` Roger Quadros 2016-04-19 11:25 ` Lukasz Majewski 0 siblings, 1 reply; 17+ messages in thread From: Roger Quadros @ 2016-04-19 10:21 UTC (permalink / raw) To: u-boot Hi Lukasz, On 19/04/16 12:02, Lukasz Majewski wrote: > Hi Roger, > >> Request size can be greater than ep.packet and still end in a >> short packet. We need to tackle this case as end of transfer >> (if short_not_ok is not set) as indicated in USB 2.0 Specification >> [1], else we get stuck up on certain protocols like fastboot. >> >> [1] - USB2.0 Specification, Section 5.3.2 Pipes >> >> Reported-by: Steve Rae <steve.rae@broadcom.com> >> Signed-off-by: Roger Quadros <rogerq@ti.com> >> Tested-by: Steve Rae <steve.rae@broadcom.com> >> --- >> drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c >> b/drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c index bce9c30..a31d875 >> 100644 --- a/drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c >> +++ b/drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c >> @@ -229,7 +229,7 @@ static void complete_rx(struct dwc2_udc *dev, u8 >> ep_num) ROUND(xfer_size, CONFIG_SYS_CACHELINE_SIZE)); >> >> req->req.actual += min(xfer_size, req->req.length - >> req->req.actual); >> - is_short = (xfer_size < ep->ep.maxpacket); >> + is_short = xfer_size % ep->ep.maxpacket; > > is_short is a flag - so maybe it would be better to write something > like: but it is defined as u32 so I thought it might as well print the short packet length instead of just 1/0. > > is_short = !!(xfer_size % ep->ep.maxpacket)) ? > > I'm going to test those patches on my boards. I will share the results > ASAP. > >> >> debug_cond(DEBUG_OUT_EP != 0, >> "%s: RX DMA done : ep = %d, rx bytes = %d/%d, " > > > -- cheers, -roger ^ permalink raw reply [flat|nested] 17+ messages in thread
* [U-Boot] [PATCH 2/3] usb: s3c-otg: Fix short packet for request size > ep.maxpacket 2016-04-19 10:21 ` Roger Quadros @ 2016-04-19 11:25 ` Lukasz Majewski 2016-04-19 11:52 ` Roger Quadros 0 siblings, 1 reply; 17+ messages in thread From: Lukasz Majewski @ 2016-04-19 11:25 UTC (permalink / raw) To: u-boot Hi Roger, > Hi Lukasz, > > On 19/04/16 12:02, Lukasz Majewski wrote: > > Hi Roger, > > > >> Request size can be greater than ep.packet and still end in a > >> short packet. We need to tackle this case as end of transfer > >> (if short_not_ok is not set) as indicated in USB 2.0 Specification > >> [1], else we get stuck up on certain protocols like fastboot. > >> > >> [1] - USB2.0 Specification, Section 5.3.2 Pipes > >> > >> Reported-by: Steve Rae <steve.rae@broadcom.com> > >> Signed-off-by: Roger Quadros <rogerq@ti.com> > >> Tested-by: Steve Rae <steve.rae@broadcom.com> > >> --- > >> drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >> diff --git a/drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c > >> b/drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c index bce9c30..a31d875 > >> 100644 --- a/drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c > >> +++ b/drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c > >> @@ -229,7 +229,7 @@ static void complete_rx(struct dwc2_udc *dev, > >> u8 ep_num) ROUND(xfer_size, CONFIG_SYS_CACHELINE_SIZE)); > >> > >> req->req.actual += min(xfer_size, req->req.length - > >> req->req.actual); > >> - is_short = (xfer_size < ep->ep.maxpacket); > >> + is_short = xfer_size % ep->ep.maxpacket; > > > > is_short is a flag - so maybe it would be better to write something > > like: > > but it is defined as u32 so I thought it might as well print the > short packet length instead of just 1/0. I mean that it looks strange for me in debug message when one see: "is_short: 8" (as it was shown at Steve's output). > > > > > is_short = !!(xfer_size % ep->ep.maxpacket)) ? > > > > I'm going to test those patches on my boards. I will share the > > results ASAP. > > > >> > >> debug_cond(DEBUG_OUT_EP != 0, > >> "%s: RX DMA done : ep = %d, rx bytes = %d/%d, " > > > > > > > > -- > cheers, > -roger > -- Best regards, Lukasz Majewski Samsung R&D Institute Poland (SRPOL) | Linux Platform Group ^ permalink raw reply [flat|nested] 17+ messages in thread
* [U-Boot] [PATCH 2/3] usb: s3c-otg: Fix short packet for request size > ep.maxpacket 2016-04-19 11:25 ` Lukasz Majewski @ 2016-04-19 11:52 ` Roger Quadros 0 siblings, 0 replies; 17+ messages in thread From: Roger Quadros @ 2016-04-19 11:52 UTC (permalink / raw) To: u-boot On 19/04/16 14:25, Lukasz Majewski wrote: > Hi Roger, > >> Hi Lukasz, >> >> On 19/04/16 12:02, Lukasz Majewski wrote: >>> Hi Roger, >>> >>>> Request size can be greater than ep.packet and still end in a >>>> short packet. We need to tackle this case as end of transfer >>>> (if short_not_ok is not set) as indicated in USB 2.0 Specification >>>> [1], else we get stuck up on certain protocols like fastboot. >>>> >>>> [1] - USB2.0 Specification, Section 5.3.2 Pipes >>>> >>>> Reported-by: Steve Rae <steve.rae@broadcom.com> >>>> Signed-off-by: Roger Quadros <rogerq@ti.com> >>>> Tested-by: Steve Rae <steve.rae@broadcom.com> >>>> --- >>>> drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c | 2 +- >>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>> >>>> diff --git a/drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c >>>> b/drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c index bce9c30..a31d875 >>>> 100644 --- a/drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c >>>> +++ b/drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c >>>> @@ -229,7 +229,7 @@ static void complete_rx(struct dwc2_udc *dev, >>>> u8 ep_num) ROUND(xfer_size, CONFIG_SYS_CACHELINE_SIZE)); >>>> >>>> req->req.actual += min(xfer_size, req->req.length - >>>> req->req.actual); >>>> - is_short = (xfer_size < ep->ep.maxpacket); >>>> + is_short = xfer_size % ep->ep.maxpacket; >>> >>> is_short is a flag - so maybe it would be better to write something >>> like: >> >> but it is defined as u32 so I thought it might as well print the >> short packet length instead of just 1/0. > > I mean that it looks strange for me in debug message when one see: > > "is_short: 8" (as it was shown at Steve's output). Agreed. I'll convert it to boolean then. -- cheers, -roger ^ permalink raw reply [flat|nested] 17+ messages in thread
* [U-Boot] [PATCH v2 2/3] usb: s3c-otg: Fix short packet for request size > ep.maxpacket 2016-04-19 7:17 ` [U-Boot] [PATCH 2/3] usb: s3c-otg: Fix short packet for request size > ep.maxpacket Roger Quadros 2016-04-19 9:02 ` Lukasz Majewski @ 2016-04-19 12:20 ` Roger Quadros 2016-04-21 10:04 ` Lukasz Majewski 1 sibling, 1 reply; 17+ messages in thread From: Roger Quadros @ 2016-04-19 12:20 UTC (permalink / raw) To: u-boot Request size can be greater than ep.packet and still end in a short packet. We need to tackle this case as end of transfer (if short_not_ok is not set) as indicated in USB 2.0 Specification [1], else we get stuck up on certain protocols like fastboot. [1] - USB2.0 Specification, Section 5.3.2 Pipes Reported-by: Steve Rae <steve.rae@broadcom.com> Signed-off-by: Roger Quadros <rogerq@ti.com> Tested-by: Steve Rae <steve.rae@broadcom.com> --- drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c b/drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c index bce9c30..9aa0f33 100644 --- a/drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c +++ b/drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c @@ -229,7 +229,7 @@ static void complete_rx(struct dwc2_udc *dev, u8 ep_num) ROUND(xfer_size, CONFIG_SYS_CACHELINE_SIZE)); req->req.actual += min(xfer_size, req->req.length - req->req.actual); - is_short = (xfer_size < ep->ep.maxpacket); + is_short = !!(xfer_size % ep->ep.maxpacket); debug_cond(DEBUG_OUT_EP != 0, "%s: RX DMA done : ep = %d, rx bytes = %d/%d, " -- 2.5.0 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* [U-Boot] [PATCH v2 2/3] usb: s3c-otg: Fix short packet for request size > ep.maxpacket 2016-04-19 12:20 ` [U-Boot] [PATCH v2 " Roger Quadros @ 2016-04-21 10:04 ` Lukasz Majewski 0 siblings, 0 replies; 17+ messages in thread From: Lukasz Majewski @ 2016-04-21 10:04 UTC (permalink / raw) To: u-boot Hi Roger, > Request size can be greater than ep.packet and still end in a > short packet. We need to tackle this case as end of transfer > (if short_not_ok is not set) as indicated in USB 2.0 Specification > [1], else we get stuck up on certain protocols like fastboot. > > [1] - USB2.0 Specification, Section 5.3.2 Pipes > > Reported-by: Steve Rae <steve.rae@broadcom.com> > Signed-off-by: Roger Quadros <rogerq@ti.com> > Tested-by: Steve Rae <steve.rae@broadcom.com> > --- > drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c > b/drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c index bce9c30..9aa0f33 > 100644 --- a/drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c > +++ b/drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c > @@ -229,7 +229,7 @@ static void complete_rx(struct dwc2_udc *dev, u8 > ep_num) ROUND(xfer_size, CONFIG_SYS_CACHELINE_SIZE)); > > req->req.actual += min(xfer_size, req->req.length - > req->req.actual); > - is_short = (xfer_size < ep->ep.maxpacket); > + is_short = !!(xfer_size % ep->ep.maxpacket); > > debug_cond(DEBUG_OUT_EP != 0, > "%s: RX DMA done : ep = %d, rx bytes = %d/%d, " Tested-by: Lukasz Majewski <l.majewski@samsung.com> Test HW: Odroid U3 (Exynos 4412) Test: ./test/py DFU & UMS -- Best regards, Lukasz Majewski Samsung R&D Institute Poland (SRPOL) | Linux Platform Group ^ permalink raw reply [flat|nested] 17+ messages in thread
* [U-Boot] [PATCH 3/3] usb: s3c-otg: Fix remaining bytes in debug messages 2016-04-19 7:16 [U-Boot] [PATCH 0/3] s3c-otg: fastboot: fixes and cleanup Roger Quadros 2016-04-19 7:16 ` [U-Boot] [PATCH 1/3] fastboot: Clean up bulk-out logic Roger Quadros 2016-04-19 7:17 ` [U-Boot] [PATCH 2/3] usb: s3c-otg: Fix short packet for request size > ep.maxpacket Roger Quadros @ 2016-04-19 7:17 ` Roger Quadros 2016-04-21 10:05 ` Lukasz Majewski 2 siblings, 1 reply; 17+ messages in thread From: Roger Quadros @ 2016-04-19 7:17 UTC (permalink / raw) To: u-boot Remaining bytes means bytes that are not yet transferred and not the bytes that were transferred in the last transfer. Reported-by: Lukasz Majewski <l.majewski@samsung.com> Signed-off-by: Roger Quadros <rogerq@ti.com> --- drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c b/drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c index a31d875..0594c3d 100644 --- a/drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c +++ b/drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c @@ -235,7 +235,7 @@ static void complete_rx(struct dwc2_udc *dev, u8 ep_num) "%s: RX DMA done : ep = %d, rx bytes = %d/%d, " "is_short = %d, DOEPTSIZ = 0x%x, remained bytes = %d\n", __func__, ep_num, req->req.actual, req->req.length, - is_short, ep_tsr, xfer_size); + is_short, ep_tsr, req->req.length - req->req.actual); if (is_short || req->req.actual == req->req.length) { if (ep_num == EP0_CON && dev->ep0state == DATA_STATE_RECV) { @@ -292,7 +292,7 @@ static void complete_tx(struct dwc2_udc *dev, u8 ep_num) "%s: TX DMA done : ep = %d, tx bytes = %d/%d, " "is_short = %d, DIEPTSIZ = 0x%x, remained bytes = %d\n", __func__, ep_num, req->req.actual, req->req.length, - is_short, ep_tsr, xfer_size); + is_short, ep_tsr, req->req.length - req->req.actual); if (ep_num == 0) { if (dev->ep0state == DATA_STATE_XMIT) { -- 2.5.0 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* [U-Boot] [PATCH 3/3] usb: s3c-otg: Fix remaining bytes in debug messages 2016-04-19 7:17 ` [U-Boot] [PATCH 3/3] usb: s3c-otg: Fix remaining bytes in debug messages Roger Quadros @ 2016-04-21 10:05 ` Lukasz Majewski 2016-04-22 2:49 ` Steve Rae 0 siblings, 1 reply; 17+ messages in thread From: Lukasz Majewski @ 2016-04-21 10:05 UTC (permalink / raw) To: u-boot Hi Roger, > Remaining bytes means bytes that are not yet transferred > and not the bytes that were transferred in the last transfer. > > Reported-by: Lukasz Majewski <l.majewski@samsung.com> > Signed-off-by: Roger Quadros <rogerq@ti.com> > --- > drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c > b/drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c index a31d875..0594c3d > 100644 --- a/drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c > +++ b/drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c > @@ -235,7 +235,7 @@ static void complete_rx(struct dwc2_udc *dev, u8 > ep_num) "%s: RX DMA done : ep = %d, rx bytes = %d/%d, " > "is_short = %d, DOEPTSIZ = 0x%x, remained bytes = > %d\n", __func__, ep_num, req->req.actual, req->req.length, > - is_short, ep_tsr, xfer_size); > + is_short, ep_tsr, req->req.length - > req->req.actual); > if (is_short || req->req.actual == req->req.length) { > if (ep_num == EP0_CON && dev->ep0state == > DATA_STATE_RECV) { @@ -292,7 +292,7 @@ static void complete_tx(struct > dwc2_udc *dev, u8 ep_num) "%s: TX DMA done : ep = %d, tx bytes = > %d/%d, " "is_short = %d, DIEPTSIZ = 0x%x, remained bytes = %d\n", > __func__, ep_num, req->req.actual, req->req.length, > - is_short, ep_tsr, xfer_size); > + is_short, ep_tsr, req->req.length - req->req.actual); > > if (ep_num == 0) { > if (dev->ep0state == DATA_STATE_XMIT) { 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] 17+ messages in thread
* [U-Boot] [PATCH 3/3] usb: s3c-otg: Fix remaining bytes in debug messages 2016-04-21 10:05 ` Lukasz Majewski @ 2016-04-22 2:49 ` Steve Rae 2016-04-22 3:03 ` Steve Rae 0 siblings, 1 reply; 17+ messages in thread From: Steve Rae @ 2016-04-22 2:49 UTC (permalink / raw) To: u-boot On Thu, Apr 21, 2016 at 3:05 AM, Lukasz Majewski <l.majewski@samsung.com> wrote: > Hi Roger, > > > Remaining bytes means bytes that are not yet transferred > > and not the bytes that were transferred in the last transfer. > > > > Reported-by: Lukasz Majewski <l.majewski@samsung.com> > > Signed-off-by: Roger Quadros <rogerq@ti.com> > > --- > > drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c > > b/drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c index a31d875..0594c3d > > 100644 --- a/drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c > > +++ b/drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c > > @@ -235,7 +235,7 @@ static void complete_rx(struct dwc2_udc *dev, u8 > > ep_num) "%s: RX DMA done : ep = %d, rx bytes = %d/%d, " > > "is_short = %d, DOEPTSIZ = 0x%x, remained bytes = > > %d\n", __func__, ep_num, req->req.actual, req->req.length, > > - is_short, ep_tsr, xfer_size); > > + is_short, ep_tsr, req->req.length - > > req->req.actual); > > if (is_short || req->req.actual == req->req.length) { > > if (ep_num == EP0_CON && dev->ep0state == > > DATA_STATE_RECV) { @@ -292,7 +292,7 @@ static void complete_tx(struct > > dwc2_udc *dev, u8 ep_num) "%s: TX DMA done : ep = %d, tx bytes = > > %d/%d, " "is_short = %d, DIEPTSIZ = 0x%x, remained bytes = %d\n", > > __func__, ep_num, req->req.actual, req->req.length, > > - is_short, ep_tsr, xfer_size); > > + is_short, ep_tsr, req->req.length - req->req.actual); > > > > if (ep_num == 0) { > > if (dev->ep0state == DATA_STATE_XMIT) { > > Acked-by: Lukasz Majewski <l.majewski@samsung.com> > > Tested-by: Steve Rae <srae@broadcom.com> [Test HW: bcm28155_ap board] Thanks, Steve > -- > Best regards, > > Lukasz Majewski > > Samsung R&D Institute Poland (SRPOL) | Linux Platform Group > ^ permalink raw reply [flat|nested] 17+ messages in thread
* [U-Boot] [PATCH 3/3] usb: s3c-otg: Fix remaining bytes in debug messages 2016-04-22 2:49 ` Steve Rae @ 2016-04-22 3:03 ` Steve Rae 0 siblings, 0 replies; 17+ messages in thread From: Steve Rae @ 2016-04-22 3:03 UTC (permalink / raw) To: u-boot On Thu, Apr 21, 2016 at 7:49 PM, Steve Rae <steve.rae@broadcom.com> wrote: > > > On Thu, Apr 21, 2016 at 3:05 AM, Lukasz Majewski <l.majewski@samsung.com> > wrote: > >> Hi Roger, >> >> > Remaining bytes means bytes that are not yet transferred >> > and not the bytes that were transferred in the last transfer. >> > >> > Reported-by: Lukasz Majewski <l.majewski@samsung.com> >> > Signed-off-by: Roger Quadros <rogerq@ti.com> >> > --- >> > drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c | 4 ++-- >> > 1 file changed, 2 insertions(+), 2 deletions(-) >> > >> > diff --git a/drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c >> > b/drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c index a31d875..0594c3d >> > 100644 --- a/drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c >> > +++ b/drivers/usb/gadget/dwc2_udc_otg_xfer_dma.c >> > @@ -235,7 +235,7 @@ static void complete_rx(struct dwc2_udc *dev, u8 >> > ep_num) "%s: RX DMA done : ep = %d, rx bytes = %d/%d, " >> > "is_short = %d, DOEPTSIZ = 0x%x, remained bytes = >> > %d\n", __func__, ep_num, req->req.actual, req->req.length, >> > - is_short, ep_tsr, xfer_size); >> > + is_short, ep_tsr, req->req.length - >> > req->req.actual); >> > if (is_short || req->req.actual == req->req.length) { >> > if (ep_num == EP0_CON && dev->ep0state == >> > DATA_STATE_RECV) { @@ -292,7 +292,7 @@ static void complete_tx(struct >> > dwc2_udc *dev, u8 ep_num) "%s: TX DMA done : ep = %d, tx bytes = >> > %d/%d, " "is_short = %d, DIEPTSIZ = 0x%x, remained bytes = %d\n", >> > __func__, ep_num, req->req.actual, req->req.length, >> > - is_short, ep_tsr, xfer_size); >> > + is_short, ep_tsr, req->req.length - req->req.actual); >> > >> > if (ep_num == 0) { >> > if (dev->ep0state == DATA_STATE_XMIT) { >> >> Acked-by: Lukasz Majewski <l.majewski@samsung.com> >> >> Tested-by: Steve Rae <srae@broadcom.com> > [Test HW: bcm28155_ap board] > > Thanks, Steve > retry because patchworks missed this.... Tested-by: Steve Rae <srae@broadcom.com> [Test HW: bcm28155_ap board] > > >> -- >> Best regards, >> >> Lukasz Majewski >> >> Samsung R&D Institute Poland (SRPOL) | Linux Platform Group >> > > ^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2016-04-22 3:52 UTC | newest] Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-04-19 7:16 [U-Boot] [PATCH 0/3] s3c-otg: fastboot: fixes and cleanup Roger Quadros 2016-04-19 7:16 ` [U-Boot] [PATCH 1/3] fastboot: Clean up bulk-out logic Roger Quadros 2016-04-19 9:00 ` Lukasz Majewski 2016-04-21 10:03 ` Lukasz Majewski 2016-04-22 2:59 ` Steve Rae 2016-04-22 3:52 ` Lukasz Majewski 2016-04-19 7:17 ` [U-Boot] [PATCH 2/3] usb: s3c-otg: Fix short packet for request size > ep.maxpacket Roger Quadros 2016-04-19 9:02 ` Lukasz Majewski 2016-04-19 10:21 ` Roger Quadros 2016-04-19 11:25 ` Lukasz Majewski 2016-04-19 11:52 ` Roger Quadros 2016-04-19 12:20 ` [U-Boot] [PATCH v2 " Roger Quadros 2016-04-21 10:04 ` Lukasz Majewski 2016-04-19 7:17 ` [U-Boot] [PATCH 3/3] usb: s3c-otg: Fix remaining bytes in debug messages Roger Quadros 2016-04-21 10:05 ` Lukasz Majewski 2016-04-22 2:49 ` Steve Rae 2016-04-22 3:03 ` Steve Rae
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.