usb: gadget: fastboot: Request status and length check in rx handler
PATCH 1/2 usb: gadget: fastboot: Request status and length check in rx handler
This avoids handling requests that have an error status or no data. In particular, this avoids showing unnecessary error messages when the USB gadget gets disconnected (e.g. with fastboot continue) and the fastboot USB gadget driver sends an error back to the host (that has disconnected already).
--- drivers/usb/gadget/f_fastboot.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-)
drivers/usb/gadget/f_fastboot.c +6 -5
@@ -635,6 +635,9 @@ static void rx_handler_command(struct usb_ep *ep, struct usb_request *req)void (*func_cb)(struct usb_ep *ep,struct usb_request *req) =NULL ;int i; +if (req->status !=0 || req->length ==0 ) +return ; +for (i =0 ; i <ARRAY_SIZE (cmd_dispatch_info); i++) {if (!strcmp_l1 (cmd_dispatch_info[i].cmd, cmdbuf)) { func_cb = cmd_dispatch_info[i].cb;
@@ -656,9 +659,7 @@ static void rx_handler_command(struct usb_ep *ep, struct usb_request *req) } } -if (req->status ==0 ) { - *cmdbuf ='\0' ; - req->actual =0 ; -usb_ep_queue (ep, req,0 ); - } + *cmdbuf ='\0' ; + req->actual =0 ; +usb_ep_queue (ep, req,0 ); }
Signature · 1 line
1.9.1
PATCH 2/2 usb: gadget: fastboot: Dequeue the previous IN request for the current request
Recent versions of the fastboot tool will query the partition type before doing an operation on a partition (such as erase, flash, etc). It will then submit the operation as soon as the response for the partition type is received.
Usually, the MUSB controller will see that the partition type request return status was read by the host at the very same time as the actual operation request is submitted by the host. However, the operation will be read first (int_rx is handled first in musb_interrupt) and after it is completed, the fastboot USB gadget driver will send another return status. Hence, this happens before the musb gadget framework has had a chance to handle the previous acknowledgement that the host read the return status and dequeue the request.
The host will then usually empty the FIFO by the time musb_interrupt gets around handling the return status acknowledgement (for the previous request, this is still on the same musb_interrupt call), so no other interrupt is generated and the most recent return status acknowledgement remains unaccounted for.
It will then be used as a response for the next command, and the proper response for it will be delayed to the next command, and so on.
Dequeuing the previous IN request in the fastboot code ensures that no previous return status remains. It is acceptable to do it since there is no callback to it anyways.
--- drivers/usb/gadget/f_fastboot.c | 3 +++ 1 file changed, 3 insertions(+)
drivers/usb/gadget/f_fastboot.c +3 -0
@@ -311,6 +311,9 @@ static int fastboot_tx_write(const char *buffer, unsigned int buffer_size)memcpy (in_req->buf, buffer, buffer_size); in_req->length = buffer_size; + +usb_ep_dequeue (fastboot_func->in_ep, in_req); + ret =usb_ep_queue (fastboot_func->in_ep, in_req,0 );if (ret)printf ("Error %d on queue\n" , ret);
Signature · 1 line
1.9.1
PATCH 2/2 usb: gadget: fastboot: Dequeue the previous IN request for the current request
Hi Paul,
Quoted 38 lines
Recent versions of the fastboot tool will query the partition type before doing an operation on a partition (such as erase, flash, etc). It will then submit the operation as soon as the response for the partition type is received.
Usually, the MUSB controller will see that the partition type request return status was read by the host at the very same time as the actual operation request is submitted by the host. However, the operation will be read first (
int_rxis handled first inmusb_interrupt) and after it is completed, the fastboot USB gadget driver will send another return status. Hence, this happens before the musb gadget framework has had a chance to handle the previous acknowledgement that the host read the return status and dequeue the request.The host will then usually empty the FIFO by the time
musb_interruptgets around handling the return status acknowledgement (for the previous request, this is still on the samemusb_interruptcall), so no other interrupt is generated and the most recent return status acknowledgement remains unaccounted for.It will then be used as a response for the next command, and the proper response for it will be delayed to the next command, and so on.
Dequeuing the previous IN request in the fastboot code ensures that no previous return status remains. It is acceptable to do it since there is no callback to it anyways.
Signed-off-by Paul Kocialkowski <[email protected]>--- drivers/usb/gadget/f_fastboot.c | 3 +++ 1 file changed, 3 insertions(+)(quoted diff) +0
-0
b/drivers/usb/gadget/f_fastboot.cindex b9a9099..60c846d 100644
drivers/usb/gadget/f_fastboot.c static int fastboot_tx_write(const char *buffer, · line 319
Quoted 9 lines
drivers/usb/gadget/f_fastboot.c +3
-0@@ -311,6 +311,9 @@ static int fastboot_tx_write(const char *buffer, unsigned int buffer_size) memcpy(in_req->buf, buffer, buffer_size); in_req->length = buffer_size; + +usb_ep_dequeue (fastboot_func->in_ep, in_req); + ret =usb_ep_queue (fastboot_func->in_ep, in_req,0 );if (ret)printf ("Error %d on queue\n" , ret);
Comment the same as for [PATCH 1/2]
Signature · 5 lines
Best regards, Lukasz Majewski Samsung R&D Institute Poland (SRPOL) | Linux Platform Group
[U-Boot] Subject
In reply to Paul KocialkowskiHi Marek, Paul, cc: Alex
jFYI/FWIW, reverting [1-2] allows getting rid of below warnings on R-Car3, when running basic fastboot commands (e.g. fastboot getvar):
[ 8.035764] status: -104 ep 'ep1' trans: 0 [ 18.744354] status: -104 ep 'ep1' trans: 28 [ 18.748950] status: -104 ep 'ep1' trans: 9 [ 18.753370] status: -104 ep 'ep1' trans: 28 [ 26.064761] status: -104 ep 'ep1' trans: 28
This has been pointed out by Dmytro (To:) in one of his patches which reached us via official Renesas release. Since R-Car USB gadget driver is not yet present in mainline [3], the behavior cannot be reproduced with vanilla U-Boot. In case, at some point in time, you hit the same problem and come to the same or an alternative solution, please share. TIA!
[1] https://patchwork.ozlabs.org/patch/491249/ ("[U-Boot,2/2] usb: gadget: fastboot: Dequeue the previous IN request for the current request")
[2] http://git.denx.de/?p=u-boot.git;a=commitdiff;h=bc9071c9f318 ("usb: gadget: fastboot: Dequeue the previous IN request for the current request")
[3] https://patchwork.ozlabs.org/patch/978026/#2107803
Best regards,
Eugeniu.
[U-Boot] Subject
Hi,
Quoted 10 lines · Eugeniu Rosca
Hi Marek, Paul, cc: Alex
jFYI/FWIW, reverting [1-2] allows getting rid of below warnings on R-Car3, when running basic fastboot commands (e.g. fastboot getvar):
[ 8.035764] status: -104 ep 'ep1' trans: 0 [ 18.744354] status: -104 ep 'ep1' trans: 28 [ 18.748950] status: -104 ep 'ep1' trans: 9 [ 18.753370] status: -104 ep 'ep1' trans: 28 [ 26.064761] status: -104 ep 'ep1' trans: 28
I don't think reverting this patch in mainline U-Boot would be a good idea, as it is required to get fastboot to work at all on sunxi platforms with the MUSB controller.
Did you investigate the issue to see what is happening here precisely?
Cheers,
Paul
Quoted 13 lines
This has been pointed out by Dmytro (To:) in one of his patches which reached us via official Renesas release. Since R-Car USB gadget driver is not yet present in mainline [3], the behavior cannot be reproduced with vanilla U-Boot. In case, at some point in time, you hit the same problem and come to the same or an alternative solution, please share. TIA!
[1] https://patchwork.ozlabs.org/patch/491249/ ("[U-Boot,2/2] usb: gadget: fastboot: Dequeue the previous IN request for the current request")
[2] http://git.denx.de/?p=u-boot.git;a=commitdiff;h=bc9071c9f318 ("usb: gadget: fastboot: Dequeue the previous IN request for the current request")
[3] https://patchwork.ozlabs.org/patch/978026/#2107803Best regards,
Eugeniu.
[U-Boot] Subject
Hi Paul, hello Marek,
Quoted 14 lines · Paul Kocialkowski
Hi,
Quoted 10 lines · Eugeniu Rosca
Hi Marek, Paul, cc: Alex
jFYI/FWIW, reverting [1-2] allows getting rid of below warnings on R-Car3, when running basic fastboot commands (e.g. fastboot getvar):
[ 8.035764] status: -104 ep 'ep1' trans: 0 [ 18.744354] status: -104 ep 'ep1' trans: 28 [ 18.748950] status: -104 ep 'ep1' trans: 9 [ 18.753370] status: -104 ep 'ep1' trans: 28 [ 26.064761] status: -104 ep 'ep1' trans: 28
Quoted 5 lines
I don't think reverting this patch in mainline U-Boot would be a good idea, as it is required to get fastboot to work at all on sunxi platforms with the MUSB controller.
Did you investigate the issue to see what is happening here precisely?
I attempted to record some traces to visualize the functions called during my use-case, but it seems like lib/trace.c and cmd/trace.c are not usable on arm64 (no U-Boot console output on my H3-ULCB-KF). Anyone with a hint in this direction?
Anyway, given that I use an out-of-tree USB gadget driver, any logs I can share will only be of limited/little help, I assume. I expect the discussion to make more sense during upstreaming of USBHS R-Car Linux driver. Now, it is more of a heads up.
Best regards,
Eugeniu.
PATCH 2/2 usb: gadget: fastboot: Dequeue the previous IN request for the current request
-bouncing e-mails
cc: Łukasz
Quoted 35 lines · Eugeniu Rosca
Hi Paul, hello Marek,
Quoted 14 lines · Paul Kocialkowski
Hi,
Quoted 10 lines · Eugeniu Rosca
Hi Marek, Paul, cc: Alex
jFYI/FWIW, reverting [1-2] allows getting rid of below warnings on R-Car3, when running basic fastboot commands (e.g. fastboot getvar):
[ 8.035764] status: -104 ep 'ep1' trans: 0 [ 18.744354] status: -104 ep 'ep1' trans: 28 [ 18.748950] status: -104 ep 'ep1' trans: 9 [ 18.753370] status: -104 ep 'ep1' trans: 28 [ 26.064761] status: -104 ep 'ep1' trans: 28Quoted 5 lines
I don't think reverting this patch in mainline U-Boot would be a good idea, as it is required to get fastboot to work at all on sunxi platforms with the MUSB controller.
Did you investigate the issue to see what is happening here precisely?
I attempted to record some traces to visualize the functions called during my use-case, but it seems like
lib/trace.candcmd/trace.care not usable on arm64 (no U-Boot console output on my H3-ULCB-KF). Anyone with a hint in this direction?Anyway, given that I use an out-of-tree USB gadget driver, any logs I can share will only be of limited/little help, I assume. I expect the discussion to make more sense during upstreaming of USBHS R-Car Linux driver. Now, it is more of a heads up.
Best regards,
Eugeniu.
[U-Boot] Subject
In reply to Eugeniu RoscaQuoted 1 line · Eugeniu Rosca
Hi Marek, Paul, cc: Alex
Hi,
Quoted 2 lines
jFYI/FWIW, reverting [1-2] allows getting rid of below warnings on R-Car3, when running basic fastboot commands (e.g. fastboot getvar):
Maybe a more constructive approach would be to send a patch fixing the issue instead of reverting patches :)
Quoted 6 lines
[ 8.035764] status: -104 ep 'ep1' trans: 0 [ 18.744354] status: -104 ep 'ep1' trans: 28 [ 18.748950] status: -104 ep 'ep1' trans: 9 [ 18.753370] status: -104 ep 'ep1' trans: 28 [ 26.064761] status: -104 ep 'ep1' trans: 28
Quoted 6 lines
This has been pointed out by Dmytro (To:) in one of his patches which reached us via official Renesas release. Since R-Car USB gadget driver is not yet present in mainline [3], the behavior cannot be reproduced with vanilla U-Boot. In case, at some point in time, you hit the same problem and come to the same or an alternative solution, please share. TIA!
Since this is gadget code, CCing U-Boot USB gadget maintainer.
Quoted 7 lines
[1] https://patchwork.ozlabs.org/patch/491249/ ("[U-Boot,2/2] usb: gadget: fastboot: Dequeue the previous IN request for the current request")
[2] http://git.denx.de/?p=u-boot.git;a=commitdiff;h=bc9071c9f318 ("usb: gadget: fastboot: Dequeue the previous IN request for the current request")
[3] https://patchwork.ozlabs.org/patch/978026/#2107803Best regards,
Eugeniu.
Signature · 2 lines
Best regards, Marek Vasut
PATCH 1/2 usb: gadget: fastboot: Request status and length check in rx handler
In reply to Paul KocialkowskiHi Paul,
Quoted 28 lines
This avoids handling requests that have an error status or no data. In particular, this avoids showing unnecessary error messages when the USB gadget gets disconnected (e.g. with fastboot continue) and the fastboot USB gadget driver sends an error back to the host (that has disconnected already).
Signed-off-by Paul Kocialkowski <[email protected]>--- drivers/usb/gadget/f_fastboot.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-)(quoted diff) +0
-0
b/drivers/usb/gadget/f_fastboot.cindex 206b6d1..b9a9099 100644drivers/usb/gadget/f_fastboot.c +0
-0@@ -635,6 +635,9 @@ static void rx_handler_command(struct usb_ep *ep,struct
usb_request*req) void (*func_cb)(structusb_ep*ep, structusb_request*req) = NULL; int i;drivers/usb/gadget/f_fastboot.c +3
-0+if (req->status !=0 || req->length ==0 ) +return ; +for (i =0 ; i <ARRAY_SIZE (cmd_dispatch_info); i++) {if (!strcmp_l1 (cmd_dispatch_info[i].cmd, cmdbuf)) { func_cb = cmd_dispatch_info[i].cb;@@ -656,9 +659,7 @@ static void rx_handler_command(struct usb_ep *ep, struct usb_request *req) } }
drivers/usb/gadget/f_fastboot.c static void rx_handler_command(struct usb_ep *ep, · line 665
Quoted 10 lines
drivers/usb/gadget/f_fastboot.c +3
-5-if (req->status ==0 ) { - *cmdbuf ='\0' ; - req->actual =0 ; -usb_ep_queue (ep, req,0 ); - } + *cmdbuf ='\0' ; + req->actual =0 ; +usb_ep_queue (ep, req,0 ); }
Let's wait for some more time before pulling this patch to u-boot-dfu tree. I hope that other fastboot users will also review this patch.
Signature · 5 lines
Best regards, Lukasz Majewski Samsung R&D Institute Poland (SRPOL) | Linux Platform Group
PATCH 1/2 usb: gadget: fastboot: Request status and length check in rx handler
Le lundi 06 juillet 2015 ? 12:28 +0200, Lukasz Majewski a ?crit :
Quoted 31 lines
Hi Paul,
Quoted 29 lines
This avoids handling requests that have an error status or no data. In particular, this avoids showing unnecessary error messages when the USB gadget gets disconnected (e.g. with fastboot continue) and the fastboot USB gadget driver sends an error back to the host (that has disconnected already).
Signed-off-by Paul Kocialkowski <[email protected]>--- drivers/usb/gadget/f_fastboot.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-)(quoted diff) +0
-0
b/drivers/usb/gadget/f_fastboot.cindex 206b6d1..b9a9099 100644drivers/usb/gadget/f_fastboot.c +0
-0@@ -635,6 +635,9 @@ static void rx_handler_command(struct usb_ep *ep,struct
usb_request*req) void (*func_cb)(structusb_ep*ep, structusb_request*req) = NULL; int i;drivers/usb/gadget/f_fastboot.c +3
-0+if (req->status !=0 || req->length ==0 ) +return ; +for (i =0 ; i <ARRAY_SIZE (cmd_dispatch_info); i++) {if (!strcmp_l1 (cmd_dispatch_info[i].cmd, cmdbuf)) { func_cb = cmd_dispatch_info[i].cb;@@ -656,9 +659,7 @@ static void rx_handler_command(struct usb_ep *ep, struct usb_request *req) } }
Quoted 11 lines
drivers/usb/gadget/f_fastboot.c static void rx_handler_command(struct usb_ep *ep, struct usb_request *req) · line 665
Quoted 9 lines
drivers/usb/gadget/f_fastboot.c +3
-5-if (req->status ==0 ) { - *cmdbuf ='\0' ; - req->actual =0 ; -usb_ep_queue (ep, req,0 ); - } + *cmdbuf ='\0' ; + req->actual =0 ; +usb_ep_queue (ep, req,0 ); }Reviewed-by Lukasz Majewski <[email protected]>
Thanks!
Quoted 2 lines
Let's wait for some more time before pulling this patch to u-boot-dfu tree. I hope that other fastboot users will also review this patch.
That would be nice, but it should only affect people using a recent version of fastboot. I have also tested this with other (older) versions (for which everything worked fine already).
However, I certainly haven't covered all use cases, so more testing is indeed welcome!
Signature · 16 lines
Paul Kocialkowski, Replicant developer Replicant is a fully free Android distribution running on several devices, a free software mobile operating system putting the emphasis on freedom and privacy/security. Website: http://www.replicant.us/ Blog: http://blog.replicant.us/ Wiki/tracker/forums: http://redmine.replicant.us/ -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: This is a digitally signed message part URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150706/1fe9b008/attachment.sig>
PATCH 1/2 usb: gadget: fastboot: Request status and length check in rx handler
In reply to Lukasz MajewskiLe lundi 06 juillet 2015 ? 12:28 +0200, Lukasz Majewski a ?crit :
Quoted 43 lines
Hi Paul,
Quoted 28 lines
This avoids handling requests that have an error status or no data. In particular, this avoids showing unnecessary error messages when the USB gadget gets disconnected (e.g. with fastboot continue) and the fastboot USB gadget driver sends an error back to the host (that has disconnected already).
Signed-off-by Paul Kocialkowski <[email protected]>--- drivers/usb/gadget/f_fastboot.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-)(quoted diff) +0
-0
b/drivers/usb/gadget/f_fastboot.cindex 206b6d1..b9a9099 100644drivers/usb/gadget/f_fastboot.c +0
-0@@ -635,6 +635,9 @@ static void rx_handler_command(struct usb_ep *ep,struct
usb_request*req) void (*func_cb)(structusb_ep*ep, structusb_request*req) = NULL; int i;drivers/usb/gadget/f_fastboot.c +3
-0+if (req->status !=0 || req->length ==0 ) +return ; +for (i =0 ; i <ARRAY_SIZE (cmd_dispatch_info); i++) {if (!strcmp_l1 (cmd_dispatch_info[i].cmd, cmdbuf)) { func_cb = cmd_dispatch_info[i].cb;@@ -656,9 +659,7 @@ static void rx_handler_command(struct usb_ep *ep, struct usb_request *req) } }drivers/usb/gadget/f_fastboot.c static void rx_handler_command(struct usb_ep *ep, · line 665
Quoted 10 lines
drivers/usb/gadget/f_fastboot.c +3
-5-if (req->status ==0 ) { - *cmdbuf ='\0' ; - req->actual =0 ; -usb_ep_queue (ep, req,0 ); - } + *cmdbuf ='\0' ; + req->actual =0 ; +usb_ep_queue (ep, req,0 ); }Reviewed-by Lukasz Majewski <[email protected]>
Quoted 2 lines
Let's wait for some more time before pulling this patch to u-boot-dfu tree. I hope that other fastboot users will also review this patch.
Oh and by the way, in the fastboot protocol description (doc/README.android-fastboot-protocol), it clearly says:
"zero-length packets are ignored".
Signature · 16 lines
Paul Kocialkowski, Replicant developer Replicant is a fully free Android distribution running on several devices, a free software mobile operating system putting the emphasis on freedom and privacy/security. Website: http://www.replicant.us/ Blog: http://blog.replicant.us/ Wiki/tracker/forums: http://redmine.replicant.us/ -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: This is a digitally signed message part URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150706/2f9b296d/attachment.sig>
PATCH 1/2 usb: gadget: fastboot: Request status and length check in rx handler
In reply to Paul KocialkowskiHi Paul,
Quoted 28 lines
This avoids handling requests that have an error status or no data. In particular, this avoids showing unnecessary error messages when the USB gadget gets disconnected (e.g. with fastboot continue) and the fastboot USB gadget driver sends an error back to the host (that has disconnected already).
Signed-off-by Paul Kocialkowski <[email protected]>--- drivers/usb/gadget/f_fastboot.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-)(quoted diff) +0
-0
b/drivers/usb/gadget/f_fastboot.cindex 206b6d1..b9a9099 100644drivers/usb/gadget/f_fastboot.c +0
-0@@ -635,6 +635,9 @@ static void rx_handler_command(struct usb_ep *ep,struct
usb_request*req) void (*func_cb)(structusb_ep*ep, structusb_request*req) = NULL; int i;drivers/usb/gadget/f_fastboot.c +3
-0+if (req->status !=0 || req->length ==0 ) +return ; +for (i =0 ; i <ARRAY_SIZE (cmd_dispatch_info); i++) {if (!strcmp_l1 (cmd_dispatch_info[i].cmd, cmdbuf)) { func_cb = cmd_dispatch_info[i].cb;@@ -656,9 +659,7 @@ static void rx_handler_command(struct usb_ep *ep, struct usb_request *req) } }
drivers/usb/gadget/f_fastboot.c static void rx_handler_command(struct usb_ep *ep, · line 665
Quoted 10 lines
drivers/usb/gadget/f_fastboot.c +3
-5-if (req->status ==0 ) { - *cmdbuf ='\0' ; - req->actual =0 ; -usb_ep_queue (ep, req,0 ); - } + *cmdbuf ='\0' ; + req->actual =0 ; +usb_ep_queue (ep, req,0 ); }
Any more comments to those patches? I mean 1/2 and 2/2.
Signature · 5 lines
Best regards, Lukasz Majewski Samsung R&D Institute Poland (SRPOL) | Linux Platform Group
PATCH 1/2 usb: gadget: fastboot: Request status and length check in rx handler
Le mardi 14 juillet 2015 ? 12:22 +0200, Lukasz Majewski a ?crit :
Quoted 31 lines
Hi Paul,
Quoted 29 lines
This avoids handling requests that have an error status or no data. In particular, this avoids showing unnecessary error messages when the USB gadget gets disconnected (e.g. with fastboot continue) and the fastboot USB gadget driver sends an error back to the host (that has disconnected already).
Signed-off-by Paul Kocialkowski <[email protected]>--- drivers/usb/gadget/f_fastboot.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-)(quoted diff) +0
-0
b/drivers/usb/gadget/f_fastboot.cindex 206b6d1..b9a9099 100644drivers/usb/gadget/f_fastboot.c +0
-0@@ -635,6 +635,9 @@ static void rx_handler_command(struct usb_ep *ep,struct
usb_request*req) void (*func_cb)(structusb_ep*ep, structusb_request*req) = NULL; int i;drivers/usb/gadget/f_fastboot.c +3
-0+if (req->status !=0 || req->length ==0 ) +return ; +for (i =0 ; i <ARRAY_SIZE (cmd_dispatch_info); i++) {if (!strcmp_l1 (cmd_dispatch_info[i].cmd, cmdbuf)) { func_cb = cmd_dispatch_info[i].cb;@@ -656,9 +659,7 @@ static void rx_handler_command(struct usb_ep *ep, struct usb_request *req) } }
Quoted 11 lines
drivers/usb/gadget/f_fastboot.c static void rx_handler_command(struct usb_ep *ep, struct usb_request *req) · line 665
Quoted 9 lines
drivers/usb/gadget/f_fastboot.c +3
-5-if (req->status ==0 ) { - *cmdbuf ='\0' ; - req->actual =0 ; -usb_ep_queue (ep, req,0 ); - } + *cmdbuf ='\0' ; + req->actual =0 ; +usb_ep_queue (ep, req,0 ); }Any more comments to those patches? I mean 1/2 and 2/2.
I could test them on OMAP3 and OMAP5 if you'd like. Together with sunxi (that I developped this on), I think a fair share of platforms using musb-new would be covered. Testing on am33xx and omap4 would be nice, too.
Signature · 16 lines
Paul Kocialkowski, Replicant developer Replicant is a fully free Android distribution running on several devices, a free software mobile operating system putting the emphasis on freedom and privacy/security. Website: http://www.replicant.us/ Blog: http://blog.replicant.us/ Wiki/tracker/forums: http://redmine.replicant.us/ -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: This is a digitally signed message part URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150714/35efec3d/attachment.sig>
PATCH 1/2 usb: gadget: fastboot: Request status and length check in rx handler
Hi Paul,
Quoted 44 lines
Le mardi 14 juillet 2015 ? 12:22 +0200, Lukasz Majewski a ?crit :
Quoted 31 lines
Hi Paul,
Quoted 29 lines
This avoids handling requests that have an error status or no data. In particular, this avoids showing unnecessary error messages when the USB gadget gets disconnected (e.g. with fastboot continue) and the fastboot USB gadget driver sends an error back to the host (that has disconnected already).
Signed-off-by Paul Kocialkowski <[email protected]>--- drivers/usb/gadget/f_fastboot.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-)(quoted diff) +0
-0
b/drivers/usb/gadget/f_fastboot.cindex 206b6d1..b9a9099 100644drivers/usb/gadget/f_fastboot.c +3
-0@@ -635,6 +635,9 @@ static void rx_handler_command(struct usb_ep *ep, struct usb_request *req) void (*func_cb)(struct usb_ep *ep,struct usb_request *req) =NULL ;int i; +if (req->status !=0 || req->length ==0 ) +return ; +for (i =0 ; i <ARRAY_SIZE (cmd_dispatch_info); i++) {if (!strcmp_l1 (cmd_dispatch_info[i].cmd, cmdbuf)) { func_cb = cmd_dispatch_info[i].cb;@@ -656,9 +659,7 @@ static void rx_handler_command(struct usb_ep *ep, struct usb_request *req) } }Quoted 11 lines
drivers/usb/gadget/f_fastboot.c static void rx_handler_command(struct usb_ep *ep, struct usb_request *req) · line 665
Quoted 9 lines
drivers/usb/gadget/f_fastboot.c +3
-5-if (req->status ==0 ) { - *cmdbuf ='\0' ; - req->actual =0 ; -usb_ep_queue (ep, req,0 ); - } + *cmdbuf ='\0' ; + req->actual =0 ; +usb_ep_queue (ep, req,0 ); }Any more comments to those patches? I mean 1/2 and 2/2.
Quoted 5 lines
I could test them on OMAP3 and OMAP5 if you'd like. Together with sunxi (that I developped this on), I think a fair share of platforms using musb-new would be covered. Testing on am33xx and omap4 would be nice, too.
More testing is always welcome. Please write proper Tested-by with target SoC/board name as a reply to this e-mail.
Then I will pick those patches to -dfu tree.
Thanks !
Signature · 5 lines
Best regards, Lukasz Majewski Samsung R&D Institute Poland (SRPOL) | Linux Platform Group
PATCH 1/2 usb: gadget: fastboot: Request status and length check in rx handler
Le mardi 14 juillet 2015 ? 13:32 +0200, Lukasz Majewski a ?crit :
Quoted 46 lines
Hi Paul,
Quoted 44 lines
Le mardi 14 juillet 2015 ? 12:22 +0200, Lukasz Majewski a ?crit :
Quoted 42 lines
Hi Paul,
Quoted 28 lines
This avoids handling requests that have an error status or no data. In particular, this avoids showing unnecessary error messages when the USB gadget gets disconnected (e.g. with fastboot continue) and the fastboot USB gadget driver sends an error back to the host (that has disconnected already).
Signed-off-by Paul Kocialkowski <[email protected]>--- drivers/usb/gadget/f_fastboot.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-)(quoted diff) +0
-0
b/drivers/usb/gadget/f_fastboot.cindex 206b6d1..b9a9099 100644drivers/usb/gadget/f_fastboot.c +3
-0@@ -635,6 +635,9 @@ static void rx_handler_command(struct usb_ep *ep, struct usb_request *req) void (*func_cb)(struct usb_ep *ep,struct usb_request *req) =NULL ;int i; +if (req->status !=0 || req->length ==0 ) +return ; +for (i =0 ; i <ARRAY_SIZE (cmd_dispatch_info); i++) {if (!strcmp_l1 (cmd_dispatch_info[i].cmd, cmdbuf)) { func_cb = cmd_dispatch_info[i].cb;@@ -656,9 +659,7 @@ static void rx_handler_command(struct usb_ep *ep, struct usb_request *req) } }drivers/usb/gadget/f_fastboot.c static void rx_handler_command(struct usb_ep · line 665
Quoted 10 lines
drivers/usb/gadget/f_fastboot.c +3
-5-if (req->status ==0 ) { - *cmdbuf ='\0' ; - req->actual =0 ; -usb_ep_queue (ep, req,0 ); - } + *cmdbuf ='\0' ; + req->actual =0 ; +usb_ep_queue (ep, req,0 ); }Any more comments to those patches? I mean 1/2 and 2/2.
Quoted 8 lines
Quoted 5 lines
I could test them on OMAP3 and OMAP5 if you'd like. Together with sunxi (that I developped this on), I think a fair share of platforms using musb-new would be covered. Testing on am33xx and omap4 would be nice, too.
More testing is always welcome. Please write proper Tested-by with target SoC/board name as a reply to this e-mail.
In the end, the OMAP5 uses another OTG controller, so it doesn't apply. However, I could test on the LG Optimus Black (P970), which is not yet upstream (I'll submit patches soon).
(sunxi) and the LG Optimus Black (P970) (omap3)
Quoted 1 line
Then I will pick those patches to -dfu tree.
Thanks, I'm looking forward to it, and to see it land it the main tree
as well, now that the merge window is open!
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150715/8f437a22/attachment.sig>
PATCH 1/2 usb: gadget: fastboot: Request status and length check in rx handler
In reply to Paul KocialkowskiHi Paul,
Quoted 28 lines
This avoids handling requests that have an error status or no data. In particular, this avoids showing unnecessary error messages when the USB gadget gets disconnected (e.g. with fastboot continue) and the fastboot USB gadget driver sends an error back to the host (that has disconnected already).
Signed-off-by Paul Kocialkowski <[email protected]>--- drivers/usb/gadget/f_fastboot.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-)(quoted diff) +0
-0
b/drivers/usb/gadget/f_fastboot.cindex 206b6d1..b9a9099 100644drivers/usb/gadget/f_fastboot.c +0
-0@@ -635,6 +635,9 @@ static void rx_handler_command(struct usb_ep *ep,struct
usb_request*req) void (*func_cb)(structusb_ep*ep, structusb_request*req) = NULL; int i;drivers/usb/gadget/f_fastboot.c +3
-0+if (req->status !=0 || req->length ==0 ) +return ; +for (i =0 ; i <ARRAY_SIZE (cmd_dispatch_info); i++) {if (!strcmp_l1 (cmd_dispatch_info[i].cmd, cmdbuf)) { func_cb = cmd_dispatch_info[i].cb;@@ -656,9 +659,7 @@ static void rx_handler_command(struct usb_ep *ep, struct usb_request *req) } }
drivers/usb/gadget/f_fastboot.c static void rx_handler_command(struct usb_ep *ep, · line 665
Quoted 10 lines
drivers/usb/gadget/f_fastboot.c +3
-5-if (req->status ==0 ) { - *cmdbuf ='\0' ; - req->actual =0 ; -usb_ep_queue (ep, req,0 ); - } + *cmdbuf ='\0' ; + req->actual =0 ; +usb_ep_queue (ep, req,0 ); }
Applied to u-boot-dfu.
Thanks for the patch!
Signature · 5 lines
Best regards, Lukasz Majewski Samsung R&D Institute Poland (SRPOL) | Linux Platform Group