[U-Boot] [PATCH] usb: ci_udc: fix interaction with CONFIG_USB_ETH_CDC
From: Stephen Warren swarren@nvidia.com
ci_udc.c's usb_gadget_unregister_driver() doesn't call driver->unbind() unlike other USB gadget drivers. Fix it to do this.
Without this, when ether.c's CDC Ethernet device is torn down, eth_unbind() is never called, so dev->gadget is never set to NULL. For some reason, usb_eth_halt() is called both at the end of the first use of the Ethernet device, and prior to any subsequent use. Since dev->gadget is never cleared, all calls to usb_eth_halt() attempt to stop, disconnect, and clean up the device, resulting in double cleanup, which hangs U-Boot on my Tegra device at least.
ci_udc allocates its own singleton EP0 request object, and cleans it up during usb_gadget_unregister_driver(). This appears necessary when using the USB gadget framework in U-Boot, since that does not allocate/free the EP0 request. However, the CDC Ethernet driver *does* allocate and free its own EP0 requests. Consequently, we must protect ci_ep_free_request() against double-freeing the request.
Signed-off-by: Stephen Warren swarren@nvidia.com --- drivers/usb/gadget/ci_udc.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/gadget/ci_udc.c b/drivers/usb/gadget/ci_udc.c index b18bee43ad89..c3f6467b7db4 100644 --- a/drivers/usb/gadget/ci_udc.c +++ b/drivers/usb/gadget/ci_udc.c @@ -226,8 +226,11 @@ static void ci_ep_free_request(struct usb_ep *ep, struct usb_request *req) int num;
num = ci_ep->desc->bEndpointAddress & USB_ENDPOINT_NUMBER_MASK; - if (num == 0) + if (num == 0) { + if (!controller.ep0_req) + return; controller.ep0_req = 0; + }
if (ci_req->b_buf) free(ci_req->b_buf); @@ -909,6 +912,9 @@ int usb_gadget_unregister_driver(struct usb_gadget_driver *driver) { udc_disconnect();
+ driver->unbind(&controller.gadget); + controller.driver = NULL; + ci_ep_free_request(&controller.ep[0].ep, &controller.ep0_req->req); free(controller.items_mem); free(controller.epts);
Thanks Stephen,
On 06/23/2014 11:02 AM, Stephen Warren wrote:
From: Stephen Warren swarren@nvidia.com
ci_udc.c's usb_gadget_unregister_driver() doesn't call driver->unbind() unlike other USB gadget drivers. Fix it to do this.
Without this, when ether.c's CDC Ethernet device is torn down, eth_unbind() is never called, so dev->gadget is never set to NULL. For some reason, usb_eth_halt() is called both at the end of the first use of the Ethernet device, and prior to any subsequent use. Since dev->gadget is never cleared, all calls to usb_eth_halt() attempt to stop, disconnect, and clean up the device, resulting in double cleanup, which hangs U-Boot on my Tegra device at least.
ci_udc allocates its own singleton EP0 request object, and cleans it up during usb_gadget_unregister_driver(). This appears necessary when using the USB gadget framework in U-Boot, since that does not allocate/free the EP0 request. However, the CDC Ethernet driver *does* allocate and free its own EP0 requests. Consequently, we must protect ci_ep_free_request() against double-freeing the request.
Tested-by: Eric Nelson eric.nelson@boundarydevices.com
Tested on Nitrogen6x (i.MX6). On this platform, without the patch, I could execute ping exactly once.
With the patch, repeated pings work great!
Regards,
Eric
On Monday, June 23, 2014 at 08:02:48 PM, Stephen Warren wrote:
+CC Jorg, rest of email is intact. Jorg, does this patch fix your issue?
From: Stephen Warren swarren@nvidia.com
ci_udc.c's usb_gadget_unregister_driver() doesn't call driver->unbind() unlike other USB gadget drivers. Fix it to do this.
Without this, when ether.c's CDC Ethernet device is torn down, eth_unbind() is never called, so dev->gadget is never set to NULL. For some reason, usb_eth_halt() is called both at the end of the first use of the Ethernet device, and prior to any subsequent use. Since dev->gadget is never cleared, all calls to usb_eth_halt() attempt to stop, disconnect, and clean up the device, resulting in double cleanup, which hangs U-Boot on my Tegra device at least.
ci_udc allocates its own singleton EP0 request object, and cleans it up during usb_gadget_unregister_driver(). This appears necessary when using the USB gadget framework in U-Boot, since that does not allocate/free the EP0 request. However, the CDC Ethernet driver *does* allocate and free its own EP0 requests. Consequently, we must protect ci_ep_free_request() against double-freeing the request.
Signed-off-by: Stephen Warren swarren@nvidia.com
drivers/usb/gadget/ci_udc.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/gadget/ci_udc.c b/drivers/usb/gadget/ci_udc.c index b18bee43ad89..c3f6467b7db4 100644 --- a/drivers/usb/gadget/ci_udc.c +++ b/drivers/usb/gadget/ci_udc.c @@ -226,8 +226,11 @@ static void ci_ep_free_request(struct usb_ep *ep, struct usb_request *req) int num;
num = ci_ep->desc->bEndpointAddress & USB_ENDPOINT_NUMBER_MASK;
- if (num == 0)
if (num == 0) {
if (!controller.ep0_req)return;controller.ep0_req = 0;
}
if (ci_req->b_buf) free(ci_req->b_buf);
@@ -909,6 +912,9 @@ int usb_gadget_unregister_driver(struct usb_gadget_driver *driver) { udc_disconnect();
- driver->unbind(&controller.gadget);
- controller.driver = NULL;
- ci_ep_free_request(&controller.ep[0].ep, &controller.ep0_req->req); free(controller.items_mem); free(controller.epts);
Best regards, Marek Vasut
On 06/25/2014 07:51 AM, Marek Vasut wrote:
On Monday, June 23, 2014 at 08:02:48 PM, Stephen Warren wrote:
+CC Jorg, rest of email is intact. Jorg, does this patch fix your issue?
Jorg's issue was timeouts during transfers, whereas this patch addresses cleanup issues once the device isn't being used any more. I can't imagine how this patch could influence the issue Jorg is seeing.
From: Stephen Warren swarren@nvidia.com
ci_udc.c's usb_gadget_unregister_driver() doesn't call driver->unbind() unlike other USB gadget drivers. Fix it to do this.
Without this, when ether.c's CDC Ethernet device is torn down, eth_unbind() is never called, so dev->gadget is never set to NULL. For some reason, usb_eth_halt() is called both at the end of the first use of the Ethernet device, and prior to any subsequent use. Since dev->gadget is never cleared, all calls to usb_eth_halt() attempt to stop, disconnect, and clean up the device, resulting in double cleanup, which hangs U-Boot on my Tegra device at least.
ci_udc allocates its own singleton EP0 request object, and cleans it up during usb_gadget_unregister_driver(). This appears necessary when using the USB gadget framework in U-Boot, since that does not allocate/free the EP0 request. However, the CDC Ethernet driver *does* allocate and free its own EP0 requests. Consequently, we must protect ci_ep_free_request() against double-freeing the request.
I added the last series of patches beginning from 2014-06-10 for testing purposes. The patches from 2014-05-29 were already applied.
First series of patches:
Applying: usb: ci_udc: call udc_disconnect() from ci_pullup() Applying: usb: ci_udc: fix freeing of ep0 req Applying: usb: ci_udc: fix probe error cleanup Applying: usb: ci_udc: clean up all allocations in unregister
Calling tftp the first time after a reset runs fine, but calling it a second time ends always in a crash. I have to reset the device.
Next patch:
Applying: usb: ci_udc: terminate ep0 INs with a zlp when required
Did not help.
Next patch:
Applying: usb: ci_udc: fix interaction with CONFIG_USB_ETH_CDC
This helps! U-Boot does not crash anymore. But there is still a problem: I have to wait some seconds before I can run a second time tftp. This is the output from U-Boot:
=> run update_rootfs Updating rootfs ... using ci_udc, OUT ep- IN ep- STATUS ep- high speed config #1: 2 mA, Ethernet Gadget, using CDC Ethernet USB network up! Using usb_ether device [snip]
=> run update_rootfs Updating rootfs ... using ci_udc, OUT ep- IN ep- STATUS ep- high speed config #1: 2 mA, Ethernet Gadget, using CDC Ethernet ERROR: The remote end did not respond in time. at drivers/usb/gadget/ether.c:2388/usb_eth_init()
Wait some seconds ...
=> run update_rootfs Updating rootfs ... using ci_udc, OUT ep- IN ep- STATUS ep- high speed config #1: 2 mA, Ethernet Gadget, using CDC Ethernet USB network up! Using usb_ether device [snip]
Best regards Jörg Krause
On 06/25/2014 06:06 PM, Stephen Warren wrote:
On 06/25/2014 07:51 AM, Marek Vasut wrote:
On Monday, June 23, 2014 at 08:02:48 PM, Stephen Warren wrote:
+CC Jorg, rest of email is intact. Jorg, does this patch fix your issue?
Jorg's issue was timeouts during transfers, whereas this patch addresses cleanup issues once the device isn't being used any more. I can't imagine how this patch could influence the issue Jorg is seeing.
From: Stephen Warren swarren@nvidia.com
ci_udc.c's usb_gadget_unregister_driver() doesn't call driver->unbind() unlike other USB gadget drivers. Fix it to do this.
Without this, when ether.c's CDC Ethernet device is torn down, eth_unbind() is never called, so dev->gadget is never set to NULL. For some reason, usb_eth_halt() is called both at the end of the first use of the Ethernet device, and prior to any subsequent use. Since dev->gadget is never cleared, all calls to usb_eth_halt() attempt to stop, disconnect, and clean up the device, resulting in double cleanup, which hangs U-Boot on my Tegra device at least.
ci_udc allocates its own singleton EP0 request object, and cleans it up during usb_gadget_unregister_driver(). This appears necessary when using the USB gadget framework in U-Boot, since that does not allocate/free the EP0 request. However, the CDC Ethernet driver *does* allocate and free its own EP0 requests. Consequently, we must protect ci_ep_free_request() against double-freeing the request.
I have tested a little bit more.
It does NOT help waiting some seconds before running tftp again. Sometimes it just works running tfpd immediately after a previous tfpd. Sometimes waiting even a minute ends up in the described error.
Odd, very odd, sorry :-)
On 06/27/2014 11:37 PM, Jörg Krause wrote:
I added the last series of patches beginning from 2014-06-10 for testing purposes. The patches from 2014-05-29 were already applied.
First series of patches:
Applying: usb: ci_udc: call udc_disconnect() from ci_pullup() Applying: usb: ci_udc: fix freeing of ep0 req Applying: usb: ci_udc: fix probe error cleanup Applying: usb: ci_udc: clean up all allocations in unregister
Calling tftp the first time after a reset runs fine, but calling it a second time ends always in a crash. I have to reset the device.
Next patch:
Applying: usb: ci_udc: terminate ep0 INs with a zlp when required
Did not help.
Next patch:
Applying: usb: ci_udc: fix interaction with CONFIG_USB_ETH_CDC
This helps! U-Boot does not crash anymore. But there is still a problem: I have to wait some seconds before I can run a second time tftp. This is the output from U-Boot:
=> run update_rootfs Updating rootfs ... using ci_udc, OUT ep- IN ep- STATUS ep- high speed config #1: 2 mA, Ethernet Gadget, using CDC Ethernet USB network up! Using usb_ether device [snip]
=> run update_rootfs Updating rootfs ... using ci_udc, OUT ep- IN ep- STATUS ep- high speed config #1: 2 mA, Ethernet Gadget, using CDC Ethernet ERROR: The remote end did not respond in time. at drivers/usb/gadget/ether.c:2388/usb_eth_init()
Wait some seconds ...
=> run update_rootfs Updating rootfs ... using ci_udc, OUT ep- IN ep- STATUS ep- high speed config #1: 2 mA, Ethernet Gadget, using CDC Ethernet USB network up! Using usb_ether device [snip]
Best regards Jörg Krause
On 06/25/2014 06:06 PM, Stephen Warren wrote:
On 06/25/2014 07:51 AM, Marek Vasut wrote:
On Monday, June 23, 2014 at 08:02:48 PM, Stephen Warren wrote:
+CC Jorg, rest of email is intact. Jorg, does this patch fix your issue?
Jorg's issue was timeouts during transfers, whereas this patch addresses cleanup issues once the device isn't being used any more. I can't imagine how this patch could influence the issue Jorg is seeing.
From: Stephen Warren swarren@nvidia.com
ci_udc.c's usb_gadget_unregister_driver() doesn't call driver->unbind() unlike other USB gadget drivers. Fix it to do this.
Without this, when ether.c's CDC Ethernet device is torn down, eth_unbind() is never called, so dev->gadget is never set to NULL. For some reason, usb_eth_halt() is called both at the end of the first use of the Ethernet device, and prior to any subsequent use. Since dev->gadget is never cleared, all calls to usb_eth_halt() attempt to stop, disconnect, and clean up the device, resulting in double cleanup, which hangs U-Boot on my Tegra device at least.
ci_udc allocates its own singleton EP0 request object, and cleans it up during usb_gadget_unregister_driver(). This appears necessary when using the USB gadget framework in U-Boot, since that does not allocate/free the EP0 request. However, the CDC Ethernet driver *does* allocate and free its own EP0 requests. Consequently, we must protect ci_ep_free_request() against double-freeing the request.
U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
On 06/27/2014 03:52 PM, Jörg Krause wrote:
I have tested a little bit more.
It does NOT help waiting some seconds before running tftp again. Sometimes it just works running tfpd immediately after a previous tfpd. Sometimes waiting even a minute ends up in the described error.
Odd, very odd, sorry :-)
OK, that makes a little more sense. Intermittent issues are fine, but intermittent issues affected by time delays are odd!
Does the *first* invocation of "tftp" after the board is powered on always work perfectly?
On 06/27/2014 11:37 PM, Jörg Krause wrote:
I added the last series of patches beginning from 2014-06-10 for testing purposes. The patches from 2014-05-29 were already applied.
First series of patches:
Applying: usb: ci_udc: call udc_disconnect() from ci_pullup() Applying: usb: ci_udc: fix freeing of ep0 req Applying: usb: ci_udc: fix probe error cleanup Applying: usb: ci_udc: clean up all allocations in unregister
Calling tftp the first time after a reset runs fine, but calling it a second time ends always in a crash. I have to reset the device.
Next patch:
Applying: usb: ci_udc: terminate ep0 INs with a zlp when required
Did not help.
Next patch:
Applying: usb: ci_udc: fix interaction with CONFIG_USB_ETH_CDC
This helps! U-Boot does not crash anymore. But there is still a problem: I have to wait some seconds before I can run a second time tftp. This is the output from U-Boot:
=> run update_rootfs Updating rootfs ... using ci_udc, OUT ep- IN ep- STATUS ep- high speed config #1: 2 mA, Ethernet Gadget, using CDC Ethernet USB network up! Using usb_ether device [snip]
=> run update_rootfs Updating rootfs ... using ci_udc, OUT ep- IN ep- STATUS ep- high speed config #1: 2 mA, Ethernet Gadget, using CDC Ethernet ERROR: The remote end did not respond in time. at drivers/usb/gadget/ether.c:2388/usb_eth_init()
Wait some seconds ...
=> run update_rootfs Updating rootfs ... using ci_udc, OUT ep- IN ep- STATUS ep- high speed config #1: 2 mA, Ethernet Gadget, using CDC Ethernet USB network up! Using usb_ether device [snip]
Best regards Jörg Krause
On 06/25/2014 06:06 PM, Stephen Warren wrote:
On 06/25/2014 07:51 AM, Marek Vasut wrote:
On Monday, June 23, 2014 at 08:02:48 PM, Stephen Warren wrote:
+CC Jorg, rest of email is intact. Jorg, does this patch fix your issue?
Jorg's issue was timeouts during transfers, whereas this patch addresses cleanup issues once the device isn't being used any more. I can't imagine how this patch could influence the issue Jorg is seeing.
From: Stephen Warren swarren@nvidia.com
ci_udc.c's usb_gadget_unregister_driver() doesn't call driver->unbind() unlike other USB gadget drivers. Fix it to do this.
Without this, when ether.c's CDC Ethernet device is torn down, eth_unbind() is never called, so dev->gadget is never set to NULL. For some reason, usb_eth_halt() is called both at the end of the first use of the Ethernet device, and prior to any subsequent use. Since dev->gadget is never cleared, all calls to usb_eth_halt() attempt to stop, disconnect, and clean up the device, resulting in double cleanup, which hangs U-Boot on my Tegra device at least.
ci_udc allocates its own singleton EP0 request object, and cleans it up during usb_gadget_unregister_driver(). This appears necessary when using the USB gadget framework in U-Boot, since that does not allocate/free the EP0 request. However, the CDC Ethernet driver *does* allocate and free its own EP0 requests. Consequently, we must protect ci_ep_free_request() against double-freeing the request.
U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
On 06/27/2014 11:56 PM, Stephen Warren wrote:
On 06/27/2014 03:52 PM, Jörg Krause wrote:
I have tested a little bit more.
It does NOT help waiting some seconds before running tftp again. Sometimes it just works running tfpd immediately after a previous tfpd. Sometimes waiting even a minute ends up in the described error.
Odd, very odd, sorry :-)
OK, that makes a little more sense. Intermittent issues are fine, but intermittent issues affected by time delays are odd!
Does the *first* invocation of "tftp" after the board is powered on always work perfectly?
Yes, always! The next runs of tftp sometimes works perfectly, sometimes I have timeouts, but it runs to end, and sometimes it did not even start, throwing the described error. But there are no crashes while running tfpd anymore :-)
FYI: I applied a series of patches from Heiko Schocher: [PATCH v5 0/5] mtd, ubi, ubifs: resync with Linux-3.14 on top of the patches for ci_udc. Now the USB Ethernet driver is completely broken. It does not even run a first time.
On 06/27/2014 11:37 PM, Jörg Krause wrote: [snip]
On 06/27/2014 03:37 PM, Jörg Krause wrote:
I added the last series of patches beginning from 2014-06-10 for testing purposes. The patches from 2014-05-29 were already applied.
First series of patches:
Applying: usb: ci_udc: call udc_disconnect() from ci_pullup() Applying: usb: ci_udc: fix freeing of ep0 req Applying: usb: ci_udc: fix probe error cleanup Applying: usb: ci_udc: clean up all allocations in unregisterCalling tftp the first time after a reset runs fine,
I thought the issue you reported was that the *first* time you run the "tftp" command, it has issues such as timeouts? Did I misunderstand, or did that issue somehow go away?
but calling it a second time ends always in a crash. I have to reset the device.
Yes, issues running "tftp" (or perhaps similar commands too) failing the second time are expected with just those patches applied...
Next patch:
Applying: usb: ci_udc: terminate ep0 INs with a zlp when requiredDid not help.
Next patch:
Applying: usb: ci_udc: fix interaction with CONFIG_USB_ETH_CDCThis helps! U-Boot does not crash anymore.
Yes, that is certainly expected to solve issues with *multiple* invocations of "tftp" or similar commands and ethernet over ci_udc. The first hunk in that patch fixes something I probably introduced during one of my ci_udc patches (probably the one that you originally replied to "usb: ci_udc: allow multiple buffer allocs per ep"), but the second hunk fixes a problem that I /think/ has always been there. Did multiple invocations of "tftp" using ci_udc as the network device ever work for you before, without random crashes or memory leaks?
But there is still a problem: I have to wait some seconds before I can run a second time tftp. This is the output from U-Boot:
=> run update_rootfs Updating rootfs ... using ci_udc, OUT ep- IN ep- STATUS ep- high speed config #1: 2 mA, Ethernet Gadget, using CDC Ethernet USB network up! Using usb_ether device [snip] => run update_rootfs Updating rootfs ... using ci_udc, OUT ep- IN ep- STATUS ep- high speed config #1: 2 mA, Ethernet Gadget, using CDC Ethernet ERROR: The remote end did not respond in time. at drivers/usb/gadget/ether.c:2388/usb_eth_init()Wait some seconds ...
=> run update_rootfs Updating rootfs ... using ci_udc, OUT ep- IN ep- STATUS ep- high speed config #1: 2 mA, Ethernet Gadget, using CDC Ethernet USB network up! Using usb_ether device [snip]
Hmm. That's odd. I didn't notice that, but I'll try retesting sometime. What exactly does $update_rootfs contain? It might be useful to know some details of your network topology (e.g. is the TFTP server on the machine that the USB cable is plugged into or further away, and are the machine and network lightly loaded) and rough sizes of the files you're downloading.
On 06/27/2014 11:55 PM, Stephen Warren wrote:
On 06/27/2014 03:37 PM, Jörg Krause wrote:
I added the last series of patches beginning from 2014-06-10 for testing purposes. The patches from 2014-05-29 were already applied.
First series of patches:
Applying: usb: ci_udc: call udc_disconnect() from ci_pullup() Applying: usb: ci_udc: fix freeing of ep0 req Applying: usb: ci_udc: fix probe error cleanup Applying: usb: ci_udc: clean up all allocations in unregisterCalling tftp the first time after a reset runs fine,
I thought the issue you reported was that the *first* time you run the "tftp" command, it has issues such as timeouts? Did I misunderstand, or did that issue somehow go away?
That's right! This was the state before applying a series of patches after allow multiple buffer allocs per ep. Now, the first run of tftp runs without any errors.
[snip] Yes, that is certainly expected to solve issues with *multiple* invocations of "tftp" or similar commands and ethernet over ci_udc. The first hunk in that patch fixes something I probably introduced during one of my ci_udc patches (probably the one that you originally replied to "usb: ci_udc: allow multiple buffer allocs per ep"), but the second hunk fixes a problem that I /think/ has always been there. Did multiple invocations of "tftp" using ci_udc as the network device ever work for you before, without random crashes or memory leaks?
Yes, it did. If I can remember right it stopped working after a series of patches from 2014-04-24. But I am not sure.
But there is still a problem: I have to wait some seconds before I can run a second time tftp. This is the output from U-Boot:
=> run update_rootfs Updating rootfs ... using ci_udc, OUT ep- IN ep- STATUS ep- high speed config #1: 2 mA, Ethernet Gadget, using CDC Ethernet USB network up! Using usb_ether device [snip] => run update_rootfs Updating rootfs ... using ci_udc, OUT ep- IN ep- STATUS ep- high speed config #1: 2 mA, Ethernet Gadget, using CDC Ethernet ERROR: The remote end did not respond in time. at drivers/usb/gadget/ether.c:2388/usb_eth_init()Wait some seconds ...
=> run update_rootfs Updating rootfs ... using ci_udc, OUT ep- IN ep- STATUS ep- high speed config #1: 2 mA, Ethernet Gadget, using CDC Ethernet USB network up! Using usb_ether device [snip]Hmm. That's odd. I didn't notice that, but I'll try retesting sometime. What exactly does $update_rootfs contain? It might be useful to know some details of your network topology (e.g. is the TFTP server on the machine that the USB cable is plugged into or further away, and are the machine and network lightly loaded) and rough sizes of the files you're downloading.
This is what update_rootfs is doing:
"update_rootfs=echo Updating rootfs ...; " \ "if tftp ${rootfs_file}; then " \ "mtdparts default; " \ "nand erase.part rootfs; " \ "ubi part rootfs; " \ "ubi create rootfs; " \ "ubi write ${loadaddr} rootfs ${filesize}; " \ "fi; " \ "\0" \
Filesize of rootfs.ubifs is about 13 MB.
The tftp server (tftp-hpa 5.2-4) is running on my notebook (running Arch Linux), where the device is plugged via USB cable. Ethernet is not used, but wireless network, which is used "normal" I would say.
On 06/27/2014 05:16 PM, Jörg Krause wrote:
On 06/27/2014 11:55 PM, Stephen Warren wrote:
On 06/27/2014 03:37 PM, Jörg Krause wrote:
I added the last series of patches beginning from 2014-06-10 for testing purposes. The patches from 2014-05-29 were already applied.
First series of patches:
Applying: usb: ci_udc: call udc_disconnect() from ci_pullup() Applying: usb: ci_udc: fix freeing of ep0 req Applying: usb: ci_udc: fix probe error cleanup Applying: usb: ci_udc: clean up all allocations in unregisterCalling tftp the first time after a reset runs fine,
I thought the issue you reported was that the *first* time you run the "tftp" command, it has issues such as timeouts? Did I misunderstand, or did that issue somehow go away?
That's right! This was the state before applying a series of patches after allow multiple buffer allocs per ep. Now, the first run of tftp runs without any errors.
Just to make sure I understand, here's what you saw:
1) tftp works fine to start with. No timeouts even on repeated invocation.
2) You applied "allow multiple buffer allocs per ep"
3) Now, you see tftp timeouts.
4) You applied "a series of patches *after* allow multiple buffer allocs per ep"
5) Now, the first tftp command works fine, but repeated invocations fail (intermittently).
And in (4) above the patch you applied that solved the problem was "Applying: usb: ci_udc: fix interaction with CONFIG_USB_ETH_CDC"?
But there is still a problem: I have to wait some seconds before I can run a second time tftp. This is the output from U-Boot:
=> run update_rootfs Updating rootfs ... using ci_udc, OUT ep- IN ep- STATUS ep- high speed config #1: 2 mA, Ethernet Gadget, using CDC Ethernet USB network up! Using usb_ether device [snip] => run update_rootfs Updating rootfs ... using ci_udc, OUT ep- IN ep- STATUS ep- high speed config #1: 2 mA, Ethernet Gadget, using CDC Ethernet ERROR: The remote end did not respond in time. at drivers/usb/gadget/ether.c:2388/usb_eth_init()Wait some seconds ...
=> run update_rootfs Updating rootfs ... using ci_udc, OUT ep- IN ep- STATUS ep- high speed config #1: 2 mA, Ethernet Gadget, using CDC Ethernet USB network up! Using usb_ether device [snip]Hmm. That's odd. I didn't notice that, but I'll try retesting sometime. What exactly does $update_rootfs contain? It might be useful to know some details of your network topology (e.g. is the TFTP server on the machine that the USB cable is plugged into or further away, and are the machine and network lightly loaded) and rough sizes of the files you're downloading.
This is what update_rootfs is doing:
"update_rootfs=echo Updating rootfs ...; " \ "if tftp ${rootfs_file}; then " \ "mtdparts default; " \ "nand erase.part rootfs; " \ "ubi part rootfs; " \ "ubi create rootfs; " \ "ubi write ${loadaddr} rootfs ${filesize}; " \
I wonder if there's some kind of memory corruption caused by the mtdparts, nand, or ubi commands? I'm especially curious about this, since your other email mentioned that some mtd/ubi patches cause complete networking failures.
If you *just* run "tftp ${rootfs_file}" over and over, does that work? If so, perhaps try running more and more of the commands from $update_rootfs above until you find the one that causes problems.
"fi; " \ "\0" \Filesize of rootfs.ubifs is about 13 MB.
The tftp server (tftp-hpa 5.2-4) is running on my notebook (running Arch Linux), where the device is plugged via USB cable. Ethernet is not used, but wireless network, which is used "normal" I would say.
OK, that's basically the same setup I used for testing, network/USB-wise.
Unfortunately, I don't have and NAND or ubifs to test with.
On 06/28/2014 01:37 AM, Stephen Warren wrote:
On 06/27/2014 05:16 PM, Jörg Krause wrote:
On 06/27/2014 11:55 PM, Stephen Warren wrote:
On 06/27/2014 03:37 PM, Jörg Krause wrote:
I added the last series of patches beginning from 2014-06-10 for testing purposes. The patches from 2014-05-29 were already applied.
First series of patches:
Applying: usb: ci_udc: call udc_disconnect() from ci_pullup() Applying: usb: ci_udc: fix freeing of ep0 req Applying: usb: ci_udc: fix probe error cleanup Applying: usb: ci_udc: clean up all allocations in unregisterCalling tftp the first time after a reset runs fine,
I thought the issue you reported was that the *first* time you run the "tftp" command, it has issues such as timeouts? Did I misunderstand, or did that issue somehow go away?
That's right! This was the state before applying a series of patches after allow multiple buffer allocs per ep. Now, the first run of tftp runs without any errors.
Just to make sure I understand, here's what you saw:
- tftp works fine to start with. No timeouts even on repeated invocation.
True.
- You applied "allow multiple buffer allocs per ep"
I did a pull from the u-boot-imx branch. I am not sure which date it stop working.
- Now, you see tftp timeouts.
At the beginning I had random timeouts even running update_rootfs the first time after a reset.
- You applied "a series of patches *after* allow multiple buffer allocs
per ep"
Yes. I applied these patches:
[U-Boot,1/4] usb: ci_udc: detect queued requests on ep0 [U-Boot,2/4] usb: ci_udc: use a single descriptor for ep0 [U-Boot,3/4] usb: ci_udc: pre-allocate ep0 req [U-Boot,4/4] usb: ci_udc: complete ep0 direction handling
But the error still existed. I found out that setting #define CONFIG_SYS_CACHELINE_SIZE 32 in my config file helped. This is a quotation from my mail from 06/12/2014:
I checked this and I found that ARCH_DMA_MINALIGN is set to 64, which is not true for Freescale i.MX28. This processor has an ARM926EJ-S, which has an cache line size of 32. In arch/arm/include/asm/cache.h the macro ARCH_DMA_MINALIGN is defined as followed: #ifdef CONFIG_SYS_CACHELINE_SIZE #define ARCH_DMA_MINALIGN CONFIG_SYS_CACHELINE_SIZE #else #define ARCH_DMA_MINALIGN 64 #endif And in /arch/arm/cpu/arm926ejs/cache.c as followed: #ifndef CONFIG_SYS_CACHELINE_SIZE #define CONFIG_SYS_CACHELINE_SIZE 32 #endif arch/arm/include/asm/cache.h does not see the definition of CONFIG_SYS_CACHELINE_SIZE in /arch/arm/cpu/arm926ejs/cache.c, so it's a bad place to put it in there.
- Now, the first tftp command works fine, but repeated invocations fail
(intermittently).
Yes. The first tftp command almost always works fine. Sometimes I have a timeout in between, but it runs to the end. But the timeouts are really rare.
And in (4) above the patch you applied that solved the problem was "Applying: usb: ci_udc: fix interaction with CONFIG_USB_ETH_CDC"?
This is also a quotation from my previous mail:
I defined CONFIG_SYS_CACHELINE_SIZE to 32 in my config header file and it works under the following circumstances: I have to disable the macro CONFIG_USB_GADGET_DUALSPEED. But, and this is strange, it works only the first time of a tftp download after a reset of the board. If I try to use tftp a second time, I get the same timeout error as before.
So, in short:
=> reset => run update_rootfs [...] done => reset => run update_rootfs [...] doneworks and
=> reset => run update_rootfs [...] done => run update_rootfs [...] timeout sending packets to usb ethernetresults in a timeout. Strange.
Lastly, I changed CONFIG_SYS_CACHELINE_SIZE to 16 and this works for me in normal mode an in dual speed mode.
So it worked, but there was already the error with running ftpd a second time. I am not so sure about the setting of the CONFIG_SYS_CACHELINE_SIZE to 16, because I did not used it anymore after some test runs.
But there is still a problem: I have to wait some seconds before I can run a second time tftp. This is the output from U-Boot:
=> run update_rootfs Updating rootfs ... using ci_udc, OUT ep- IN ep- STATUS ep- high speed config #1: 2 mA, Ethernet Gadget, using CDC Ethernet USB network up! Using usb_ether device [snip] => run update_rootfs Updating rootfs ... using ci_udc, OUT ep- IN ep- STATUS ep- high speed config #1: 2 mA, Ethernet Gadget, using CDC Ethernet ERROR: The remote end did not respond in time. at drivers/usb/gadget/ether.c:2388/usb_eth_init()Wait some seconds ...
=> run update_rootfs Updating rootfs ... using ci_udc, OUT ep- IN ep- STATUS ep- high speed config #1: 2 mA, Ethernet Gadget, using CDC Ethernet USB network up! Using usb_ether device [snip]Hmm. That's odd. I didn't notice that, but I'll try retesting sometime. What exactly does $update_rootfs contain? It might be useful to know some details of your network topology (e.g. is the TFTP server on the machine that the USB cable is plugged into or further away, and are the machine and network lightly loaded) and rough sizes of the files you're downloading.
This is what update_rootfs is doing:
"update_rootfs=echo Updating rootfs ...; " \ "if tftp ${rootfs_file}; then " \ "mtdparts default; " \ "nand erase.part rootfs; " \ "ubi part rootfs; " \ "ubi create rootfs; " \ "ubi write ${loadaddr} rootfs ${filesize}; " \I wonder if there's some kind of memory corruption caused by the mtdparts, nand, or ubi commands? I'm especially curious about this, since your other email mentioned that some mtd/ubi patches cause complete networking failures.
If you *just* run "tftp ${rootfs_file}" over and over, does that work? If so, perhaps try running more and more of the commands from $update_rootfs above until you find the one that causes problems.
I will check this.
"fi; " \ "\0" \Filesize of rootfs.ubifs is about 13 MB.
The tftp server (tftp-hpa 5.2-4) is running on my notebook (running Arch Linux), where the device is plugged via USB cable. Ethernet is not used, but wireless network, which is used "normal" I would say.
OK, that's basically the same setup I used for testing, network/USB-wise.
Unfortunately, I don't have and NAND or ubifs to test with.
On 06/28/2014 01:37 AM, Stephen Warren wrote:
On 06/27/2014 05:16 PM, Jörg Krause wrote:
On 06/27/2014 11:55 PM, Stephen Warren wrote:
On 06/27/2014 03:37 PM, Jörg Krause wrote:
I added the last series of patches beginning from 2014-06-10 for testing purposes. The patches from 2014-05-29 were already applied.
First series of patches:
Applying: usb: ci_udc: call udc_disconnect() from ci_pullup() Applying: usb: ci_udc: fix freeing of ep0 req Applying: usb: ci_udc: fix probe error cleanup Applying: usb: ci_udc: clean up all allocations in unregisterCalling tftp the first time after a reset runs fine,
I thought the issue you reported was that the *first* time you run the "tftp" command, it has issues such as timeouts? Did I misunderstand, or did that issue somehow go away?
That's right! This was the state before applying a series of patches after allow multiple buffer allocs per ep. Now, the first run of tftp runs without any errors.
Just to make sure I understand, here's what you saw:
- tftp works fine to start with. No timeouts even on repeated invocation.
I went back in time and now I can be more precise. Everything worked fine until commit usb: ci_udc: allow multiple buffer allocs per ep which introduces timeouts in almost all tftp file downloads. Even the first run of tfpt can end in a timeout.
- You applied "allow multiple buffer allocs per ep"
Setting #define CONFIG_SYS_CACHELINE_SIZE 32 to my config file helped here. But still timeouts. First run almost always runs fine, only sometimes timeouts while receiving a packet, but always running to the end. Running tftp after this a second time and more fails with a ERROR: The remote end did not respond in time. at drivers/usb/gadget/ether.c:2388/usb_eth_init(), but sometimes it works.
Setting CONFIG_SYS_CACHELINE_SIZE 32 does not make it better (as I previously wrote it).
Removing CONFIG_USB_GADGET_DUALSPEED helps a little bit, but I am getting also errors after the second or third run.
Now, you see tftp timeouts.
You applied "a series of patches *after* allow multiple buffer allocs
per ep"
Applying: usb: ci_udc: parse QTD before over-writing it Applying: usb: ci_udc: detect queued requests on ep0 Applying: usb: ci_udc: use a single descriptor for ep0 Applying: usb: ci_udc: pre-allocate ep0 req Applying: usb: ci_udc: complete ep0 direction handling Applying: usb: ci_udc: call udc_disconnect() from ci_pullup() Applying: usb: ci_udc: fix freeing of ep0 req Applying: usb: ci_udc: fix probe error cleanup Applying: usb: ci_udc: clean up all allocations in unregister Applying: usb: ci_udc: terminate ep0 INs with a zlp when required Applying: usb: ci_udc: fix interaction with CONFIG_USB_ETH_CDC Applying: usb: ci_udc: fix typo in debug message
- Now, the first tftp command works fine, but repeated invocations fail
(intermittently).
Now tftp runs fine the first time (and sometimes more often) after a reboot, but after some tftp runs I am getting the ERROR: The remote end did not respond in time. at drivers/usb/gadget/ether.c:2388/usb_eth_init(), as mentioned above.
And in (4) above the patch you applied that solved the problem was "Applying: usb: ci_udc: fix interaction with CONFIG_USB_ETH_CDC"?
But there is still a problem: I have to wait some seconds before I can run a second time tftp. This is the output from U-Boot:
=> run update_rootfs Updating rootfs ... using ci_udc, OUT ep- IN ep- STATUS ep- high speed config #1: 2 mA, Ethernet Gadget, using CDC Ethernet USB network up! Using usb_ether device [snip] => run update_rootfs Updating rootfs ... using ci_udc, OUT ep- IN ep- STATUS ep- high speed config #1: 2 mA, Ethernet Gadget, using CDC Ethernet ERROR: The remote end did not respond in time. at drivers/usb/gadget/ether.c:2388/usb_eth_init()Wait some seconds ...
=> run update_rootfs Updating rootfs ... using ci_udc, OUT ep- IN ep- STATUS ep- high speed config #1: 2 mA, Ethernet Gadget, using CDC Ethernet USB network up! Using usb_ether device [snip]Hmm. That's odd. I didn't notice that, but I'll try retesting sometime. What exactly does $update_rootfs contain? It might be useful to know some details of your network topology (e.g. is the TFTP server on the machine that the USB cable is plugged into or further away, and are the machine and network lightly loaded) and rough sizes of the files you're downloading.
This is what update_rootfs is doing:
"update_rootfs=echo Updating rootfs ...; " \ "if tftp ${rootfs_file}; then " \ "mtdparts default; " \ "nand erase.part rootfs; " \ "ubi part rootfs; " \ "ubi create rootfs; " \ "ubi write ${loadaddr} rootfs ${filesize}; " \I wonder if there's some kind of memory corruption caused by the mtdparts, nand, or ubi commands? I'm especially curious about this, since your other email mentioned that some mtd/ubi patches cause complete networking failures.
If you *just* run "tftp ${rootfs_file}" over and over, does that work? If so, perhaps try running more and more of the commands from $update_rootfs above until you find the one that causes problems.
For all tests I only run tftp rootfs.ubifs. The mtd patches from Heiko Schocher make some changes to the gadget driver. Maybe that's the cause, that it fails together.
"fi; " \ "\0" \Filesize of rootfs.ubifs is about 13 MB.
The tftp server (tftp-hpa 5.2-4) is running on my notebook (running Arch Linux), where the device is plugged via USB cable. Ethernet is not used, but wireless network, which is used "normal" I would say.
OK, that's basically the same setup I used for testing, network/USB-wise.
Unfortunately, I don't have and NAND or ubifs to test with.
On 06/28/2014 09:59 PM, Marek Vasut wrote:
I would actually be really glad if you could test plain u-boot-usb/master and see if your USB problem is present there. Also, it would be really nice to isolate the sequence of commands which triggers the bug, so we have a proper reproducible testcase.
Anyway, this thread is not a place where I would like to discuss this. This should be discussed in the "original" thread, that is:
Subj: [PATCH] usb: ci_udc: fix interaction with CONFIG_USB_ETH_CDC
I cloned u-boot-usb/master and tested it. I am running "tftpd rootfs.ubifs" several times. Here is the log (I shortened the loading process bar):
HTLLCLLC
U-Boot 2014.07-rc3-g18e0313 (Jun 28 2014 - 22:17:52)
CPU: Freescale i.MX28 rev1.2 at 454 MHz BOOT: NAND, 3V3 DRAM: 64 MiB NAND: 128 MiB In: serial Out: serial Err: serial Net: usb_ether [PRIME] Hit any key to stop autoboot: 0 => tftp rootfs.ubifs using ci_udc, OUT ep- IN ep- STATUS ep- MAC 00:19:b8:00:00:02 HOST MAC 00:19:b8:00:00:01 high speed config #1: 2 mA, Ethernet Gadget, using CDC Ethernet USB network up! Using usb_ether device TFTP from server 10.0.0.1; our IP address is 10.0.0.2 Filename 'rootfs.ubifs'. Load address: 0x40008000 Loading: [snip] ##############################################################T ### [snip] 1.7 MiB/s done Bytes transferred = 13205504 (c98000 hex) => tftp rootfs.ubifs using ci_udc, OUT ep- IN ep- STATUS ep- MAC 00:19:b8:00:00:02 HOST MAC 00:19:b8:00:00:01 high speed config #1: 2 mA, Ethernet Gadget, using CDC Ethernet USB network up! Using usb_ether device TFTP from server 10.0.0.1; our IP address is 10.0.0.2 Filename 'rootfs.ubifs'. Load address: 0x40008000 Loading: ################################################################# [snip] 5.6 MiB/s done Bytes transferred = 13205504 (c98000 hex) => tftp rootfs.ubifs using ci_udc, OUT ep- IN ep- STATUS ep- MAC 00:19:b8:00:00:02 HOST MAC 00:19:b8:00:00:01 high speed config #1: 2 mA, Ethernet Gadget, using CDC Ethernet USB network up! Using usb_ether device TFTP from server 10.0.0.1; our IP address is 10.0.0.2 Filename 'rootfs.ubifs'. Load address: 0x40008000 Loading: ################################################################# [snip] 5.6 MiB/s done Bytes transferred = 13205504 (c98000 hex) => tftp rootfs.ubifs using ci_udc, OUT ep- IN ep- STATUS ep- MAC 00:19:b8:00:00:02 HOST MAC 00:19:b8:00:00:01 high speed config #1: 2 mA, Ethernet Gadget, using CDC Ethernet ERROR: The remote end did not respond in time. at drivers/usb/gadget/ether.c:2388/usb_eth_init() =>
The error always appears after calling tftp the third time! The first two runs work fine, but the third one always fails. I run this test about 20 times.
Best regards Jörg Krause
On Saturday, June 28, 2014 at 10:37:35 PM, Jörg Krause wrote:
On 06/28/2014 09:59 PM, Marek Vasut wrote:
I would actually be really glad if you could test plain u-boot-usb/master and see if your USB problem is present there. Also, it would be really nice to isolate the sequence of commands which triggers the bug, so we have a proper reproducible testcase.
Anyway, this thread is not a place where I would like to discuss this. This should be discussed in the "original" thread, that is:
Subj: [PATCH] usb: ci_udc: fix interaction with CONFIG_USB_ETH_CDC
I cloned u-boot-usb/master and tested it. I am running "tftpd rootfs.ubifs" several times. Here is the log (I shortened the loading process bar):
[...]
=> tftp rootfs.ubifs using ci_udc, OUT ep- IN ep- STATUS ep- MAC 00:19:b8:00:00:02 HOST MAC 00:19:b8:00:00:01 high speed config #1: 2 mA, Ethernet Gadget, using CDC Ethernet USB network up! Using usb_ether device TFTP from server 10.0.0.1; our IP address is 10.0.0.2 Filename 'rootfs.ubifs'. Load address: 0x40008000 Loading: [snip] ##############################################################T ### [snip] 1.7 MiB/s
Here it's 1.7MB/s ...
done Bytes transferred = 13205504 (c98000 hex) => tftp rootfs.ubifs using ci_udc, OUT ep- IN ep- STATUS ep- MAC 00:19:b8:00:00:02 HOST MAC 00:19:b8:00:00:01 high speed config #1: 2 mA, Ethernet Gadget, using CDC Ethernet USB network up! Using usb_ether device TFTP from server 10.0.0.1; our IP address is 10.0.0.2 Filename 'rootfs.ubifs'. Load address: 0x40008000 Loading: ################################################################# [snip] 5.6 MiB/s
... and here it's 5.6 MB/s ... that's weird.
[...]
The error always appears after calling tftp the third time! The first two runs work fine, but the third one always fails. I run this test about 20 times.
And finally, what is the contents of 'rootfs.ubifs' ?
Best regards,
On 06/28/2014 10:45 PM, Marek Vasut wrote:
On Saturday, June 28, 2014 at 10:37:35 PM, Jörg Krause wrote:
On 06/28/2014 09:59 PM, Marek Vasut wrote:
I would actually be really glad if you could test plain u-boot-usb/master and see if your USB problem is present there. Also, it would be really nice to isolate the sequence of commands which triggers the bug, so we have a proper reproducible testcase.
Anyway, this thread is not a place where I would like to discuss this. This should be discussed in the "original" thread, that is:
Subj: [PATCH] usb: ci_udc: fix interaction with CONFIG_USB_ETH_CDC
I cloned u-boot-usb/master and tested it. I am running "tftpd rootfs.ubifs" several times. Here is the log (I shortened the loading process bar):
[...]
=> tftp rootfs.ubifs using ci_udc, OUT ep- IN ep- STATUS ep- MAC 00:19:b8:00:00:02 HOST MAC 00:19:b8:00:00:01 high speed config #1: 2 mA, Ethernet Gadget, using CDC Ethernet USB network up! Using usb_ether device TFTP from server 10.0.0.1; our IP address is 10.0.0.2 Filename 'rootfs.ubifs'. Load address: 0x40008000 Loading: [snip] ##############################################################T ### [snip] 1.7 MiB/sHere it's 1.7MB/s ...
Sometimes I get a timeout "#T " while loading, which slows down the transfer rate. It is not always the case.
done Bytes transferred = 13205504 (c98000 hex) => tftp rootfs.ubifs using ci_udc, OUT ep- IN ep- STATUS ep- MAC 00:19:b8:00:00:02 HOST MAC 00:19:b8:00:00:01 high speed config #1: 2 mA, Ethernet Gadget, using CDC Ethernet USB network up! Using usb_ether device TFTP from server 10.0.0.1; our IP address is 10.0.0.2 Filename 'rootfs.ubifs'. Load address: 0x40008000 Loading: ################################################################# [snip] 5.6 MiB/s... and here it's 5.6 MB/s ... that's weird.
[...]
The error always appears after calling tftp the third time! The first two runs work fine, but the third one always fails. I run this test about 20 times.
And finally, what is the contents of 'rootfs.ubifs' ?
It's an rootf ubifs filesystem. I also tested other files. One small u-boot.sb file with a filesize of 400 KB and a 62 MB compressed ubuntu core file (ubuntu-core-14.04-core.ext4.gz). For all I get the same result.
Best regards,
On 06/28/2014 10:53 PM, Jörg Krause wrote:
[snip] _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
I did some tests this weekend on u-boot-usb/master branch.
If I run "env default -a" and then "saveenv" after a reset, I get the same error as running three time "tftp file" in a row. Log:
U-Boot 2014.07-rc3-g18e0313-dirty (Jun 29 2014 - 21:56:02)
CPU: Freescale i.MX28 rev1.2 at 454 MHz BOOT: NAND, 3V3 DRAM: 64 MiB NAND: 128 MiB In: serial Out: serial Err: serial Net: usb_ether [PRIME] Hit any key to stop autoboot: 0 => env default -a ## Resetting to default environment => saveenv Saving Environment to NAND... Erasing NAND... Erasing at 0x360000 -- 100% complete. Writing to NAND... OK => tftp rootfs.ubifs using ci_udc, OUT ep- IN ep- STATUS ep- MAC 00:19:b8:00:00:02 HOST MAC 00:19:b8:00:00:01 high speed config #1: 2 mA, Ethernet Gadget, using CDC Ethernet ERROR: The remote end did not respond in time. at drivers/usb/gadget/ether.c:2388/usb_eth_init()
"env default -a" removes stdin, stdout, stderr, and ver from the output of "printenv".
Looking at drivers/usb/gadget/ether.c:usb_eth_init I found the environment variable "cdc_connect_timeout". I played a little bit with the settings.
1) Using "setenv cdc_connect_timeout 1" from the command line: tftp runs more then three time in a row. Actually I can run tftp more than ten times in row and it produces no error. I tested the values 1, 3, and 15 for cdc_connect_timeout.
2) Setting #define CONFIG_EXTRA_ENV_SETTINGS "cdc_connect_timeout=1\0" \ in my config header file. This does not help and produces the error on the fourth run of tfpd. Tested with values 1 and 3 for timeout.
Very, very strange.
On Sunday, June 29, 2014 at 10:33:26 PM, Jörg Krause wrote:
On 06/28/2014 10:53 PM, Jörg Krause wrote:
[snip] _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
I did some tests this weekend on u-boot-usb/master branch.
If I run "env default -a" and then "saveenv" after a reset, I get the same error as running three time "tftp file" in a row. Log:
U-Boot 2014.07-rc3-g18e0313-dirty (Jun 29 2014 - 21:56:02) CPU: Freescale i.MX28 rev1.2 at 454 MHz BOOT: NAND, 3V3 DRAM: 64 MiB NAND: 128 MiB In: serial Out: serial Err: serial Net: usb_ether [PRIME] Hit any key to stop autoboot: 0 => env default -a ## Resetting to default environment => saveenv Saving Environment to NAND... Erasing NAND... Erasing at 0x360000 -- 100% complete. Writing to NAND... OK => tftp rootfs.ubifs using ci_udc, OUT ep- IN ep- STATUS ep- MAC 00:19:b8:00:00:02 HOST MAC 00:19:b8:00:00:01 high speed config #1: 2 mA, Ethernet Gadget, using CDC Ethernet ERROR: The remote end did not respond in time. at drivers/usb/gadget/ether.c:2388/usb_eth_init()"env default -a" removes stdin, stdout, stderr, and ver from the output of "printenv".
Looking at drivers/usb/gadget/ether.c:usb_eth_init I found the environment variable "cdc_connect_timeout". I played a little bit with the settings.
- Using "setenv cdc_connect_timeout 1" from the command line: tftp runs
more then three time in a row. Actually I can run tftp more than ten times in row and it produces no error. I tested the values 1, 3, and 15 for cdc_connect_timeout.
- Setting #define CONFIG_EXTRA_ENV_SETTINGS "cdc_connect_timeout=1\0" \
in my config header file. This does not help and produces the error on the fourth run of tfpd. Tested with values 1 and 3 for timeout.
I just tested the CDC ethernet on M28EVK with u-boot-usb/master and loading 64MiB file from a TFTP server running on a local machine. It seems that for some reason, in the udc_gadget_handle_interrupts() or somewhere there, it starts not getting interrupts. Can you try with this change:
diff --git a/drivers/usb/gadget/ci_udc.c b/drivers/usb/gadget/ci_udc.c index a6433e8..1af6d12 100644 --- a/drivers/usb/gadget/ci_udc.c +++ b/drivers/usb/gadget/ci_udc.c @@ -727,14 +727,8 @@ void udc_irq(void)
int usb_gadget_handle_interrupts(void) { - u32 value; - struct ci_udc *udc = (struct ci_udc *)controller.ctrl->hcor; - - value = readl(&udc->usbsts); - if (value) - udc_irq(); - - return value; + udc_irq(); + return 0; }
void udc_disconnect(void)
Best regards, Marek Vasut
On 06/30/2014 11:37 AM, Marek Vasut wrote:
On Sunday, June 29, 2014 at 10:33:26 PM, Jörg Krause wrote:
On 06/28/2014 10:53 PM, Jörg Krause wrote:
[snip] _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
I did some tests this weekend on u-boot-usb/master branch.
If I run "env default -a" and then "saveenv" after a reset, I get the same error as running three time "tftp file" in a row. Log:
U-Boot 2014.07-rc3-g18e0313-dirty (Jun 29 2014 - 21:56:02) CPU: Freescale i.MX28 rev1.2 at 454 MHz BOOT: NAND, 3V3 DRAM: 64 MiB NAND: 128 MiB In: serial Out: serial Err: serial Net: usb_ether [PRIME] Hit any key to stop autoboot: 0 => env default -a ## Resetting to default environment => saveenv Saving Environment to NAND... Erasing NAND... Erasing at 0x360000 -- 100% complete. Writing to NAND... OK => tftp rootfs.ubifs using ci_udc, OUT ep- IN ep- STATUS ep- MAC 00:19:b8:00:00:02 HOST MAC 00:19:b8:00:00:01 high speed config #1: 2 mA, Ethernet Gadget, using CDC Ethernet ERROR: The remote end did not respond in time. at drivers/usb/gadget/ether.c:2388/usb_eth_init()"env default -a" removes stdin, stdout, stderr, and ver from the output of "printenv".
Looking at drivers/usb/gadget/ether.c:usb_eth_init I found the environment variable "cdc_connect_timeout". I played a little bit with the settings.
- Using "setenv cdc_connect_timeout 1" from the command line: tftp runs
more then three time in a row. Actually I can run tftp more than ten times in row and it produces no error. I tested the values 1, 3, and 15 for cdc_connect_timeout.
- Setting #define CONFIG_EXTRA_ENV_SETTINGS "cdc_connect_timeout=1\0" \
in my config header file. This does not help and produces the error on the fourth run of tfpd. Tested with values 1 and 3 for timeout.
I just tested the CDC ethernet on M28EVK with u-boot-usb/master and loading 64MiB file from a TFTP server running on a local machine. It seems that for some reason, in the udc_gadget_handle_interrupts() or somewhere there, it starts not getting interrupts. Can you try with this change:
diff --git a/drivers/usb/gadget/ci_udc.c b/drivers/usb/gadget/ci_udc.c index a6433e8..1af6d12 100644 --- a/drivers/usb/gadget/ci_udc.c +++ b/drivers/usb/gadget/ci_udc.c @@ -727,14 +727,8 @@ void udc_irq(void)
int usb_gadget_handle_interrupts(void) {
u32 value;struct ci_udc *udc = (struct ci_udc *)controller.ctrl->hcor;value = readl(&udc->usbsts);if (value)udc_irq();return value;
udc_irq();return 0;}
void udc_disconnect(void)
Does not help, sorry.
Best regards, Marek Vasut
I run the test with a smaller file of around 18 KB and DEBUG messages enabled in ci_udc.c. I attached the output for the first run of tftp imx28-airlino.dtb and the fourth rund of tftp imx28-airlino.dtb, which fails with an error. Maybe this helps.
On 06/27/2014 07:34 PM, Jörg Krause wrote:
On 06/28/2014 01:37 AM, Stephen Warren wrote:
On 06/27/2014 05:16 PM, Jörg Krause wrote:
On 06/27/2014 11:55 PM, Stephen Warren wrote:
On 06/27/2014 03:37 PM, Jörg Krause wrote:
I added the last series of patches beginning from 2014-06-10 for testing purposes. The patches from 2014-05-29 were already applied.
First series of patches:
Applying: usb: ci_udc: call udc_disconnect() from ci_pullup() Applying: usb: ci_udc: fix freeing of ep0 req Applying: usb: ci_udc: fix probe error cleanup Applying: usb: ci_udc: clean up all allocations in unregisterCalling tftp the first time after a reset runs fine,
I thought the issue you reported was that the *first* time you run the "tftp" command, it has issues such as timeouts? Did I misunderstand, or did that issue somehow go away?
That's right! This was the state before applying a series of patches after allow multiple buffer allocs per ep. Now, the first run of tftp runs without any errors.
Just to make sure I understand, here's what you saw:
- tftp works fine to start with. No timeouts even on repeated
invocation.
I went back in time and now I can be more precise. Everything worked fine until commit usb: ci_udc: allow multiple buffer allocs per ep which introduces timeouts in almost all tftp file downloads. Even the first run of tfpt can end in a timeout.
- You applied "allow multiple buffer allocs per ep"
Setting #define CONFIG_SYS_CACHELINE_SIZE 32 to my config file helped here. But still timeouts. First run almost always runs fine, only sometimes timeouts while receiving a packet, but always running to the end. Running tftp after this a second time and more fails with a ERROR: The remote end did not respond in time. at drivers/usb/gadget/ether.c:2388/usb_eth_init(), but sometimes it works.
Setting CONFIG_SYS_CACHELINE_SIZE 32 does not make it better (as I previously wrote it).
Removing CONFIG_USB_GADGET_DUALSPEED helps a little bit, but I am getting also errors after the second or third run.
Sigh. There's been a lot of flip-flopping re: whether the cacheline size affects the issue or not, and whether the first TFTP download always works fine, or whether only the 3rd fails. This makes it very hard for me to understand the issue that you're seeing. For instance, if even the first TFTP download can fail (even if intermittently), then there's clearly a problem with basic driver operation. However, if only the 2nd or 3rd TFTP ever fails, then the problem is likely isolated to some part of the cleanup/shutdown code. Given that your reports of the problem keep flip-flopping about, it makes it hard to decide which part of the code to look at.
For now, I'm going to simply assume that any TFTP download (1st, 2nd, or 100th) can fail intermittently, and that cache line size is irrelevant.
I'll look at the problematic commit you mentioned (2813006fecda "usb: ci_udc: allow multiple buffer allocs per ep") and see if I can create a series of patches that do the same thing, and have you bisect that. If I can do that, I will ask you to test 100 times each: the commit before the bad commit, then each of the commits in my series, always resetting the board between each test and doing a single TFTP download. Then I'd like to see the raw results from those tests.
On 06/30/2014 10:02 AM, Stephen Warren wrote:
On 06/27/2014 07:34 PM, Jörg Krause wrote:
On 06/28/2014 01:37 AM, Stephen Warren wrote:
On 06/27/2014 05:16 PM, Jörg Krause wrote:
On 06/27/2014 11:55 PM, Stephen Warren wrote:
On 06/27/2014 03:37 PM, Jörg Krause wrote:
I added the last series of patches beginning from 2014-06-10 for testing purposes. The patches from 2014-05-29 were already applied.
First series of patches:
Applying: usb: ci_udc: call udc_disconnect() from ci_pullup() Applying: usb: ci_udc: fix freeing of ep0 req Applying: usb: ci_udc: fix probe error cleanup Applying: usb: ci_udc: clean up all allocations in unregisterCalling tftp the first time after a reset runs fine,
I thought the issue you reported was that the *first* time you run the "tftp" command, it has issues such as timeouts? Did I misunderstand, or did that issue somehow go away?
That's right! This was the state before applying a series of patches after allow multiple buffer allocs per ep. Now, the first run of tftp runs without any errors.
Just to make sure I understand, here's what you saw:
- tftp works fine to start with. No timeouts even on repeated
invocation.
I went back in time and now I can be more precise. Everything worked fine until commit usb: ci_udc: allow multiple buffer allocs per ep which introduces timeouts in almost all tftp file downloads. Even the first run of tfpt can end in a timeout.
I've pushed out some branches for you to test at: git://github.com/swarren/u-boot.git
I looked back to see where the problematic commit 2813006fecda "usb: ci_udc: allow multiple buffer allocs per ep" was first applied. I then started with that same commit, and split up the problematic commit into tiny pieces, and applied each piece in turn. The result is in branch ci-udc-debug-base. If you could please test every commit in that branch:
fabf0df9523a usb: ci_udc: remainder of "allow multiple buffer allocs" 373098f12a1d usb: ci_udc: dynamically alloc USB request objects 43fff2b11860 usb: ci_udc: dynamically alloc bounce buffer f21b9989bf83 usb: ci_udc: bounce only actual (rounded) packet len not buffer len for small pkts 09ba064f9a99 usb: ci_udc: debounce only actual packet len not buffer len 7e90ae0e4973 usb: ci_udc: use ci_req-> not ep-> a bit more 9663ff89c764 usb: ci_udc: pass ci_req not ep to bounce/debounce b9fdf9dd3f96 usb: ci_udc: move buffer fields from ep to req 82f35a839e31 usb: ci_udc: introduce struct ci_req c9396bc55069 usb: ci_udc: create "req" variable in bounce/unbounce 173d294b94cf Merge branch 'serial' of git://www.denx.de/git/u-boot-microblaze
... and tell me which commit causes the problem, that would be useful. Since the problem is intermittent, please make sure you test each commit many times (at least 10-20, preferably nearer 100). You can stop early if you see the problem, but if you don't, please test many times to make sure there is no problem.
You mentioned that your board support is not upstream. In order to test, you should probably do: For each commit I mention above, check it out, apply the minimal set of patches needed to support your board, and test.
Please don't apply any other patches while testing this series. Please only test TFTP download, and not ubifs writes, etc.
I've also pushed out branch ci-udc-debug-tegra-dfu-working which has a whole load of other commits I needed to test the split up patches. These were needed to add board support for the board I'm currently using, and to fix/enhance the core DFU code. Most of these commits are already applied in u-boot.git/master, it's just that I cherry-picked them on top of the old base commit so I would have something to test with.
BTW, I'm going on vacation from Jul 3rd-July 20th. I might answer some limited email during this time, but I certainly won't have access to any test HW.
On 06/30/2014 09:55 PM, Stephen Warren wrote:
On 06/30/2014 10:02 AM, Stephen Warren wrote:
On 06/27/2014 07:34 PM, Jörg Krause wrote:
On 06/28/2014 01:37 AM, Stephen Warren wrote:
On 06/27/2014 05:16 PM, Jörg Krause wrote:
On 06/27/2014 11:55 PM, Stephen Warren wrote:
On 06/27/2014 03:37 PM, Jörg Krause wrote: > I added the last series of patches beginning from 2014-06-10 for > testing > purposes. The patches from 2014-05-29 were already applied. > > First series of patches: > > Applying: usb: ci_udc: call udc_disconnect() from ci_pullup() > Applying: usb: ci_udc: fix freeing of ep0 req > Applying: usb: ci_udc: fix probe error cleanup > Applying: usb: ci_udc: clean up all allocations in unregister > > Calling tftp the first time after a reset runs fine, I thought the issue you reported was that the *first* time you run the "tftp" command, it has issues such as timeouts? Did I misunderstand, or did that issue somehow go away?
That's right! This was the state before applying a series of patches after allow multiple buffer allocs per ep. Now, the first run of tftp runs without any errors.
Just to make sure I understand, here's what you saw:
- tftp works fine to start with. No timeouts even on repeated
invocation.
I went back in time and now I can be more precise. Everything worked fine until commit usb: ci_udc: allow multiple buffer allocs per ep which introduces timeouts in almost all tftp file downloads. Even the first run of tfpt can end in a timeout.
I've pushed out some branches for you to test at: git://github.com/swarren/u-boot.git
I looked back to see where the problematic commit 2813006fecda "usb: ci_udc: allow multiple buffer allocs per ep" was first applied. I then started with that same commit, and split up the problematic commit into tiny pieces, and applied each piece in turn. The result is in branch ci-udc-debug-base. If you could please test every commit in that branch:
fabf0df9523a usb: ci_udc: remainder of "allow multiple buffer allocs" 373098f12a1d usb: ci_udc: dynamically alloc USB request objects 43fff2b11860 usb: ci_udc: dynamically alloc bounce buffer f21b9989bf83 usb: ci_udc: bounce only actual (rounded) packet len not buffer len for small pkts 09ba064f9a99 usb: ci_udc: debounce only actual packet len not buffer len 7e90ae0e4973 usb: ci_udc: use ci_req-> not ep-> a bit more 9663ff89c764 usb: ci_udc: pass ci_req not ep to bounce/debounce b9fdf9dd3f96 usb: ci_udc: move buffer fields from ep to req 82f35a839e31 usb: ci_udc: introduce struct ci_req c9396bc55069 usb: ci_udc: create "req" variable in bounce/unbounce 173d294b94cf Merge branch 'serial' of git://www.denx.de/git/u-boot-microblaze
... and tell me which commit causes the problem, that would be useful. Since the problem is intermittent, please make sure you test each commit many times (at least 10-20, preferably nearer 100). You can stop early if you see the problem, but if you don't, please test many times to make sure there is no problem.
Commit 373098f12a1d usb: ci_udc: dynamically alloc USB request objects cannot be build.
The error is introduced with the latest commit fabf0df9523a usb: ci_udc: remainder of "allow multiple buffer allocs". I attached the output message for this commit.
You mentioned that your board support is not upstream. In order to test, you should probably do: For each commit I mention above, check it out, apply the minimal set of patches needed to support your board, and test.
Done. Just added board support.
Please don't apply any other patches while testing this series. Please only test TFTP download, and not ubifs writes, etc.
This is my test script:
"test_usb=" \ "setenv i 64; " \ "while test ${i} -gt 0; do " \ "echo ${i}; " \ "tftp ${rootfs_file}; " \ "setexpr i ${i} - 1; " \ "done; " \ "setenv i; " \ "\0"
I've also pushed out branch ci-udc-debug-tegra-dfu-working which has a whole load of other commits I needed to test the split up patches. These were needed to add board support for the board I'm currently using, and to fix/enhance the core DFU code. Most of these commits are already applied in u-boot.git/master, it's just that I cherry-picked them on top of the old base commit so I would have something to test with.
BTW, I'm going on vacation from Jul 3rd-July 20th. I might answer some limited email during this time, but I certainly won't have access to any test HW.
On 06/30/2014 04:44 PM, Jörg Krause wrote:
On 06/30/2014 09:55 PM, Stephen Warren wrote:
On 06/30/2014 10:02 AM, Stephen Warren wrote:
On 06/27/2014 07:34 PM, Jörg Krause wrote:
On 06/28/2014 01:37 AM, Stephen Warren wrote:
On 06/27/2014 05:16 PM, Jörg Krause wrote:
On 06/27/2014 11:55 PM, Stephen Warren wrote: > On 06/27/2014 03:37 PM, Jörg Krause wrote: >> I added the last series of patches beginning from 2014-06-10 for >> testing >> purposes. The patches from 2014-05-29 were already applied. >> >> First series of patches: >> >> Applying: usb: ci_udc: call udc_disconnect() from ci_pullup() >> Applying: usb: ci_udc: fix freeing of ep0 req >> Applying: usb: ci_udc: fix probe error cleanup >> Applying: usb: ci_udc: clean up all allocations in unregister >> >> Calling tftp the first time after a reset runs fine, > I thought the issue you reported was that the *first* time you run the > "tftp" command, it has issues such as timeouts? Did I misunderstand, or > did that issue somehow go away? That's right! This was the state before applying a series of patches after allow multiple buffer allocs per ep. Now, the first run of tftp runs without any errors.
Just to make sure I understand, here's what you saw:
- tftp works fine to start with. No timeouts even on repeated
invocation.
I went back in time and now I can be more precise. Everything worked fine until commit usb: ci_udc: allow multiple buffer allocs per ep which introduces timeouts in almost all tftp file downloads. Even the first run of tfpt can end in a timeout.
I've pushed out some branches for you to test at: git://github.com/swarren/u-boot.git
I looked back to see where the problematic commit 2813006fecda "usb: ci_udc: allow multiple buffer allocs per ep" was first applied. I then started with that same commit, and split up the problematic commit into tiny pieces, and applied each piece in turn. The result is in branch ci-udc-debug-base. If you could please test every commit in that branch:
fabf0df9523a usb: ci_udc: remainder of "allow multiple buffer allocs" 373098f12a1d usb: ci_udc: dynamically alloc USB request objects 43fff2b11860 usb: ci_udc: dynamically alloc bounce buffer f21b9989bf83 usb: ci_udc: bounce only actual (rounded) packet len not buffer len for small pkts 09ba064f9a99 usb: ci_udc: debounce only actual packet len not buffer len 7e90ae0e4973 usb: ci_udc: use ci_req-> not ep-> a bit more 9663ff89c764 usb: ci_udc: pass ci_req not ep to bounce/debounce b9fdf9dd3f96 usb: ci_udc: move buffer fields from ep to req 82f35a839e31 usb: ci_udc: introduce struct ci_req c9396bc55069 usb: ci_udc: create "req" variable in bounce/unbounce 173d294b94cf Merge branch 'serial' of git://www.denx.de/git/u-boot-microblaze
... and tell me which commit causes the problem, that would be useful. Since the problem is intermittent, please make sure you test each commit many times (at least 10-20, preferably nearer 100). You can stop early if you see the problem, but if you don't, please test many times to make sure there is no problem.
Thanks very much for testing.
(when you remove the blank lines between the message you're replying to and your response, it's much harder to read. Perhaps this is because you're writing HTML email. Most people won't read that; please use plain text).
Commit 373098f12a1d usb: ci_udc: dynamically alloc USB request objects cannot be build.
Oh dear. Differentiating between that commit and the next one is perhaps the most important thing. Can you please apply the following patch to that commit and retest it:
diff --git a/drivers/usb/gadget/ci_udc.c b/drivers/usb/gadget/ci_udc.c index b00fc2613779..c8c64d755195 100644 --- a/drivers/usb/gadget/ci_udc.c +++ b/drivers/usb/gadget/ci_udc.c @@ -423,7 +423,7 @@ static void handle_ep_complete(struct ci_ep *ep) num, in ? "in" : "out", item->info, item->page0);
len = (item->info >> 16) & 0x7fff; - ci_ep->current_req = 0; + ep->current_req = 0; ci_req->req.actual = ci_req->req.length - len; ci_debounce(ci_req, in);
The error is introduced with the latest commit fabf0df9523a usb: ci_udc: remainder of "allow multiple buffer allocs". I attached the output message for this commit.
OK, at least we can rule out all the bounce buffer and cache handling changes.
On 07/01/2014 12:51 AM, Stephen Warren wrote:
On 06/30/2014 04:44 PM, Jörg Krause wrote:
On 06/30/2014 09:55 PM, Stephen Warren wrote:
On 06/30/2014 10:02 AM, Stephen Warren wrote:
On 06/27/2014 07:34 PM, Jörg Krause wrote:
On 06/28/2014 01:37 AM, Stephen Warren wrote:
On 06/27/2014 05:16 PM, Jörg Krause wrote: > On 06/27/2014 11:55 PM, Stephen Warren wrote: >> On 06/27/2014 03:37 PM, Jörg Krause wrote: >>> I added the last series of patches beginning from 2014-06-10 for >>> testing >>> purposes. The patches from 2014-05-29 were already applied. >>> >>> First series of patches: >>> >>> Applying: usb: ci_udc: call udc_disconnect() from ci_pullup() >>> Applying: usb: ci_udc: fix freeing of ep0 req >>> Applying: usb: ci_udc: fix probe error cleanup >>> Applying: usb: ci_udc: clean up all allocations in unregister >>> >>> Calling tftp the first time after a reset runs fine, >> I thought the issue you reported was that the *first* time you run the >> "tftp" command, it has issues such as timeouts? Did I misunderstand, or >> did that issue somehow go away? > That's right! This was the state before applying a series of patches > after allow multiple buffer allocs per ep. Now, the first run of tftp > runs without any errors. Just to make sure I understand, here's what you saw:
- tftp works fine to start with. No timeouts even on repeated
invocation.
I went back in time and now I can be more precise. Everything worked fine until commit usb: ci_udc: allow multiple buffer allocs per ep which introduces timeouts in almost all tftp file downloads. Even the first run of tfpt can end in a timeout.
I've pushed out some branches for you to test at: git://github.com/swarren/u-boot.git
I looked back to see where the problematic commit 2813006fecda "usb: ci_udc: allow multiple buffer allocs per ep" was first applied. I then started with that same commit, and split up the problematic commit into tiny pieces, and applied each piece in turn. The result is in branch ci-udc-debug-base. If you could please test every commit in that branch:
fabf0df9523a usb: ci_udc: remainder of "allow multiple buffer allocs" 373098f12a1d usb: ci_udc: dynamically alloc USB request objects 43fff2b11860 usb: ci_udc: dynamically alloc bounce buffer f21b9989bf83 usb: ci_udc: bounce only actual (rounded) packet len not buffer len for small pkts 09ba064f9a99 usb: ci_udc: debounce only actual packet len not buffer len 7e90ae0e4973 usb: ci_udc: use ci_req-> not ep-> a bit more 9663ff89c764 usb: ci_udc: pass ci_req not ep to bounce/debounce b9fdf9dd3f96 usb: ci_udc: move buffer fields from ep to req 82f35a839e31 usb: ci_udc: introduce struct ci_req c9396bc55069 usb: ci_udc: create "req" variable in bounce/unbounce 173d294b94cf Merge branch 'serial' of git://www.denx.de/git/u-boot-microblaze
... and tell me which commit causes the problem, that would be useful. Since the problem is intermittent, please make sure you test each commit many times (at least 10-20, preferably nearer 100). You can stop early if you see the problem, but if you don't, please test many times to make sure there is no problem.
Thanks very much for testing.
(when you remove the blank lines between the message you're replying to and your response, it's much harder to read. Perhaps this is because you're writing HTML email. Most people won't read that; please use plain text).
Done, I hope it is better now.
Commit 373098f12a1d usb: ci_udc: dynamically alloc USB request objects cannot be build.
Oh dear. Differentiating between that commit and the next one is perhaps the most important thing. Can you please apply the following patch to that commit and retest it:
diff --git a/drivers/usb/gadget/ci_udc.c b/drivers/usb/gadget/ci_udc.c index b00fc2613779..c8c64d755195 100644 --- a/drivers/usb/gadget/ci_udc.c +++ b/drivers/usb/gadget/ci_udc.c @@ -423,7 +423,7 @@ static void handle_ep_complete(struct ci_ep *ep) num, in ? "in" : "out", item->info, item->page0);
len = (item->info >> 16) & 0x7fff;
- ci_ep->current_req = 0;
- ep->current_req = 0; ci_req->req.actual = ci_req->req.length - len; ci_debounce(ci_req, in);
Applied and tested. This commit produces also an error, but it differs to the previous one. I attached a log.
The error is introduced with the latest commit fabf0df9523a usb: ci_udc: remainder of "allow multiple buffer allocs". I attached the output message for this commit.
OK, at least we can rule out all the bounce buffer and cache handling changes.
Just FYI: I added CONFIG_SYS_CACHE_SIZE 32 to my config header for all tests.
On Tuesday, July 01, 2014 at 01:17:54 AM, Jörg Krause wrote: [...]
OK, at least we can rule out all the bounce buffer and cache handling changes.
Just FYI: I added CONFIG_SYS_CACHE_SIZE 32 to my config header for all tests.
Feel free to join our slumber party at #u-boot IRC channel ... and btw. I just sent out a patch , so please test that one.
Best regards, Marek Vasut
On 06/30/2014 06:02 PM, Stephen Warren wrote:
On 06/27/2014 07:34 PM, Jörg Krause wrote:
On 06/28/2014 01:37 AM, Stephen Warren wrote:
On 06/27/2014 05:16 PM, Jörg Krause wrote:
On 06/27/2014 11:55 PM, Stephen Warren wrote:
On 06/27/2014 03:37 PM, Jörg Krause wrote:
I added the last series of patches beginning from 2014-06-10 for testing purposes. The patches from 2014-05-29 were already applied.
First series of patches:
Applying: usb: ci_udc: call udc_disconnect() from ci_pullup() Applying: usb: ci_udc: fix freeing of ep0 req Applying: usb: ci_udc: fix probe error cleanup Applying: usb: ci_udc: clean up all allocations in unregisterCalling tftp the first time after a reset runs fine,
I thought the issue you reported was that the *first* time you run the "tftp" command, it has issues such as timeouts? Did I misunderstand, or did that issue somehow go away?
That's right! This was the state before applying a series of patches after allow multiple buffer allocs per ep. Now, the first run of tftp runs without any errors.
Just to make sure I understand, here's what you saw:
- tftp works fine to start with. No timeouts even on repeated
invocation.
I went back in time and now I can be more precise. Everything worked fine until commit usb: ci_udc: allow multiple buffer allocs per ep which introduces timeouts in almost all tftp file downloads. Even the first run of tfpt can end in a timeout.
- You applied "allow multiple buffer allocs per ep"
Setting #define CONFIG_SYS_CACHELINE_SIZE 32 to my config file helped here. But still timeouts. First run almost always runs fine, only sometimes timeouts while receiving a packet, but always running to the end. Running tftp after this a second time and more fails with a ERROR: The remote end did not respond in time. at drivers/usb/gadget/ether.c:2388/usb_eth_init(), but sometimes it works.
Setting CONFIG_SYS_CACHELINE_SIZE 32 does not make it better (as I previously wrote it).
Sorry, this is a typo. It should be CONFIG_SYS_CACHELINE_SIZE 16 (not 32).
Removing CONFIG_USB_GADGET_DUALSPEED helps a little bit, but I am getting also errors after the second or third run.
Sigh. There's been a lot of flip-flopping re: whether the cacheline size affects the issue or not, and whether the first TFTP download always works fine, or whether only the 3rd fails. This makes it very hard for me to understand the issue that you're seeing. For instance, if even the first TFTP download can fail (even if intermittently), then there's clearly a problem with basic driver operation. However, if only the 2nd or 3rd TFTP ever fails, then the problem is likely isolated to some part of the cleanup/shutdown code. Given that your reports of the problem keep flip-flopping about, it makes it hard to decide which part of the code to look at.
I am very sorry if I confused you with my attempt to explain you what I am seeing. The "flip-flopping" comes from the different results and behaviour after applying a patch or a series of patches. And I also tried this and that and looked if it changes the behaviour. I must admit that this is not good testing practise.
For now, I'm going to simply assume that any TFTP download (1st, 2nd, or 100th) can fail intermittently, and that cache line size is irrelevant.
I'll look at the problematic commit you mentioned (2813006fecda "usb: ci_udc: allow multiple buffer allocs per ep") and see if I can create a series of patches that do the same thing, and have you bisect that. If I can do that, I will ask you to test 100 times each: the commit before the bad commit, then each of the commits in my series, always resetting the board between each test and doing a single TFTP download. Then I'd like to see the raw results from those tests.
I will do this and I hope it brings some clarifications.
On Monday, June 30, 2014 at 10:55:37 PM, Jörg Krause wrote: [...]
- You applied "allow multiple buffer allocs per ep"
Setting #define CONFIG_SYS_CACHELINE_SIZE 32 to my config file helped here. But still timeouts. First run almost always runs fine, only sometimes timeouts while receiving a packet, but always running to the end. Running tftp after this a second time and more fails with a ERROR: The remote end did not respond in time. at drivers/usb/gadget/ether.c:2388/usb_eth_init(), but sometimes it works.
Setting CONFIG_SYS_CACHELINE_SIZE 32 does not make it better (as I previously wrote it).
Sorry, this is a typo. It should be CONFIG_SYS_CACHELINE_SIZE 16 (not 32).
MX28 has 32b-long cachelines. Setting this to 16 is nonsense. [...]
Best regards, Marek Vasut
On 06/30/2014 11:15 PM, Marek Vasut wrote:
On Monday, June 30, 2014 at 10:55:37 PM, Jörg Krause wrote: [...]
- You applied "allow multiple buffer allocs per ep"
Setting #define CONFIG_SYS_CACHELINE_SIZE 32 to my config file helped here. But still timeouts. First run almost always runs fine, only sometimes timeouts while receiving a packet, but always running to the end. Running tftp after this a second time and more fails with a ERROR: The remote end did not respond in time. at drivers/usb/gadget/ether.c:2388/usb_eth_init(), but sometimes it works.
Setting CONFIG_SYS_CACHELINE_SIZE 32 does not make it better (as I previously wrote it).
Sorry, this is a typo. It should be CONFIG_SYS_CACHELINE_SIZE 16 (not 32).
MX28 has 32b-long cachelines. Setting this to 16 is nonsense. [...]
Yes it is. But if I do not set cacheline size to 32 in my config header file it will be 64, as I stated in a previous mail.
I checked this and I found that ARCH_DMA_MINALIGN is set to 64, which is not true for Freescale i.MX28. This processor has an ARM926EJ-S, which has an cache line size of 32. In arch/arm/include/asm/cache.h the macro ARCH_DMA_MINALIGN is defined as followed: #ifdef CONFIG_SYS_CACHELINE_SIZE #define ARCH_DMA_MINALIGN CONFIG_SYS_CACHELINE_SIZE #else #define ARCH_DMA_MINALIGN 64 #endif
And in /arch/arm/cpu/arm926ejs/cache.c as followed: #ifndef CONFIG_SYS_CACHELINE_SIZE #define CONFIG_SYS_CACHELINE_SIZE 32 #endif
arch/arm/include/asm/cache.h does not see the definition of CONFIG_SYS_CACHELINE_SIZE in /arch/arm/cpu/arm926ejs/cache.c, so it's a bad place to put it in there.
I was just curious of the impact on the behaviour of the USB ethernet gadget driver.
Best regards, Marek Vasut
On Monday, June 30, 2014 at 11:43:24 PM, Jörg Krause wrote:
On 06/30/2014 11:15 PM, Marek Vasut wrote:
On Monday, June 30, 2014 at 10:55:37 PM, Jörg Krause wrote: [...]
- You applied "allow multiple buffer allocs per ep"
Setting #define CONFIG_SYS_CACHELINE_SIZE 32 to my config file helped here. But still timeouts. First run almost always runs fine, only sometimes timeouts while receiving a packet, but always running to the end. Running tftp after this a second time and more fails with a ERROR: The remote end did not respond in time. at drivers/usb/gadget/ether.c:2388/usb_eth_init(), but sometimes it works.
Setting CONFIG_SYS_CACHELINE_SIZE 32 does not make it better (as I previously wrote it).
Sorry, this is a typo. It should be CONFIG_SYS_CACHELINE_SIZE 16 (not 32).
MX28 has 32b-long cachelines. Setting this to 16 is nonsense. [...]
Yes it is. But if I do not set cacheline size to 32 in my config header file it will be 64, as I stated in a previous mail.
I checked this and I found that ARCH_DMA_MINALIGN is set to 64, which is not true for Freescale i.MX28. This processor has an ARM926EJ-S, which has an cache line size of 32. In arch/arm/include/asm/cache.h the macro ARCH_DMA_MINALIGN is defined as followed: #ifdef CONFIG_SYS_CACHELINE_SIZE #define ARCH_DMA_MINALIGN CONFIG_SYS_CACHELINE_SIZE #else #define ARCH_DMA_MINALIGN 64 #endif And in /arch/arm/cpu/arm926ejs/cache.c as followed: #ifndef CONFIG_SYS_CACHELINE_SIZE #define CONFIG_SYS_CACHELINE_SIZE 32 #endif arch/arm/include/asm/cache.h does not see the definition of CONFIG_SYS_CACHELINE_SIZE in /arch/arm/cpu/arm926ejs/cache.c, so it's a bad place to put it in there.I was just curious of the impact on the behaviour of the USB ethernet gadget driver.
Well okay, cacheline length and DMA alignment needs are two different things. Larger than necessary ARCH_DMA_MINALIGN cannot hurt, though there should be a patch adding CONFIG_SYS_CACHELINE_SIZE 32 for MXS into include/configs/mxs.h it seems.
Best regards, Marek Vasut
On Monday, June 23, 2014 at 08:02:48 PM, Stephen Warren wrote:
From: Stephen Warren swarren@nvidia.com
ci_udc.c's usb_gadget_unregister_driver() doesn't call driver->unbind() unlike other USB gadget drivers. Fix it to do this.
Without this, when ether.c's CDC Ethernet device is torn down, eth_unbind() is never called, so dev->gadget is never set to NULL. For some reason, usb_eth_halt() is called both at the end of the first use of the Ethernet device, and prior to any subsequent use. Since dev->gadget is never cleared, all calls to usb_eth_halt() attempt to stop, disconnect, and clean up the device, resulting in double cleanup, which hangs U-Boot on my Tegra device at least.
ci_udc allocates its own singleton EP0 request object, and cleans it up during usb_gadget_unregister_driver(). This appears necessary when using the USB gadget framework in U-Boot, since that does not allocate/free the EP0 request. However, the CDC Ethernet driver *does* allocate and free its own EP0 requests. Consequently, we must protect ci_ep_free_request() against double-freeing the request.
Signed-off-by: Stephen Warren swarren@nvidia.com
Applied, thanks.
Best regards, Marek Vasut
participants (4)
-
Eric Nelson -
Jörg Krause -
Marek Vasut -
Stephen Warren