[U-Boot] [PATCH] USB-CDC: called handle_interrupts inside usb_eth_send
The patch removes an endless loop in the usb_eth_send if the tx_complete is not called before going in the loop. The driver interrupt routine is called allowing the driver to check if the TX is completed.
Signed-off-by: Stefano Babic sbabic@denx.de --- drivers/usb/gadget/ether.c | 10 +++++++++- 1 files changed, 9 insertions(+), 1 deletions(-)
diff --git a/drivers/usb/gadget/ether.c b/drivers/usb/gadget/ether.c index 1481d76..9eb43d7 100644 --- a/drivers/usb/gadget/ether.c +++ b/drivers/usb/gadget/ether.c @@ -1808,6 +1808,8 @@ static int usb_eth_send(struct eth_device* netdev, volatile void* packet, int le struct usb_request *req = NULL;
struct eth_dev *dev = &l_ethdev; + unsigned long ts; + unsigned long timeout = USB_CONNECT_TIMEOUT; dprintf("%s:...\n",__func__);
req = dev->tx_req; @@ -1837,9 +1839,15 @@ static int usb_eth_send(struct eth_device* netdev, volatile void* packet, int le
if (!retval) dprintf("%s: packet queued\n",__func__); + ts = get_timer(0); + packet_sent=0; while(!packet_sent) { - packet_sent=0; + if (get_timer(ts) > timeout) { + printf ("timeout sending packets to usb ethernet\n"); + return -1; + } + usb_gadget_handle_interrupts(); }
return 0;
Hi Stefano,
2010/8/11 Stefano Babic sbabic@denx.de:
The patch removes an endless loop in the usb_eth_send if the tx_complete is not called before going in the loop. The driver interrupt routine is called allowing the driver to check if the TX is completed.
Signed-off-by: Stefano Babic sbabic@denx.de
drivers/usb/gadget/ether.c | 10 +++++++++- 1 files changed, 9 insertions(+), 1 deletions(-)
diff --git a/drivers/usb/gadget/ether.c b/drivers/usb/gadget/ether.c index 1481d76..9eb43d7 100644 --- a/drivers/usb/gadget/ether.c +++ b/drivers/usb/gadget/ether.c
- ts = get_timer(0);
- packet_sent=0;
This line breaks the at91sam9261 code. Can you please fix this?
Rest of the patches do not seem to break anything and look okay to me. Thanks.
Kind regards,
Remy
Remy Bohmer wrote:
Hi Stefano,
Hi Remy,
packet_sent=0;This line breaks the at91sam9261 code. Can you please fix this?
This means to me that tx_complete() is not called for at91sam9261, as it supposed to be (or better, as I supposed..).
I see that packet_sent is reset only in the eth_init() function. Does it mean that usb_eth_send() does not wait for tx completion in the case of at91sam9261 ?
I supposed (and I manage in this way..) that packet_sent is used as flag to detect when a single packet is really transmitted, and in this way is used when I call usb_handle_interrupts(). The callback stored into the USB request is tx_complete() that is the one responsible to set the flag. So the simple mechanism is that usb_eth_send clear the flag and wait until tx_complete sets it. It seems now to me that on at91sam9261 usb_eth_send() waits only for the first packet, and then it does not wait anymore. How can we be sure that the packet is really transmitted ?
Should I protect the line with some AT91 CONFIG or am I missing something ?
Best regards, Stefano
Hi Stefano,
2010/8/12 Stefano Babic sbabic@denx.de:
Remy Bohmer wrote:
Hi Stefano,
Hi Remy,
- packet_sent=0;
This line breaks the at91sam9261 code. Can you please fix this?
This means to me that tx_complete() is not called for at91sam9261, as it supposed to be (or better, as I supposed..).
I see that packet_sent is reset only in the eth_init() function. Does it mean that usb_eth_send() does not wait for tx completion in the case of at91sam9261 ?
Looking at this I agree that this packet_sent flag should be reset on every usb_eth_send().
I supposed (and I manage in this way..) that packet_sent is used as flag to detect when a single packet is really transmitted, and in this way is used when I call usb_handle_interrupts(). The callback stored into the USB request is tx_complete() that is the one responsible to set the flag. So the simple mechanism is that usb_eth_send clear the flag and wait until tx_complete sets it. It seems now to me that on at91sam9261 usb_eth_send() waits only for the first packet, and then it does not wait anymore. How can we be sure that the packet is really transmitted ?
Should I protect the line with some AT91 CONFIG or am I missing something ?
No, it should be AT91 specific...
Looking at the flow on at91 we see that this happens: ether.c:usb_eth_send() gadget.h:usb_ep_queue() at91_udc.c:at91_ep_queue() at91_udc.c:write_fifo() at91_udc.c:done() req->req.complete() tx_complete() packet_send = 1
The packet is transmitted in the context of the usb_ep_queue() routine, not in the context of the interrupt handler. So, the timeout should incorporate usb_ep_queue also.
I think this is better: ts = get_timer(0); packet_sent=0; retval = usb_ep_queue (dev->in_ep, req, GFP_ATOMIC);
compared to: ts = get_timer(0); packet_sent=0; while(!packet_sent)
But I must mention that I have not yet tested it though...
Kind regards,
Remy
Remy Bohmer wrote:
Looking at this I agree that this packet_sent flag should be reset on every usb_eth_send().
Ok
Should I protect the line with some AT91 CONFIG or am I missing something ?
No, it should be AT91 specific...
Looking at the flow on at91 we see that this happens: ether.c:usb_eth_send() gadget.h:usb_ep_queue() at91_udc.c:at91_ep_queue() at91_udc.c:write_fifo() at91_udc.c:done() req->req.complete() tx_complete() packet_send = 1
Ok, understood.
The packet is transmitted in the context of the usb_ep_queue() routine, not in the context of the interrupt handler. So, the timeout should incorporate usb_ep_queue also.
I think this is better: ts = get_timer(0); packet_sent=0; retval = usb_ep_queue (dev->in_ep, req, GFP_ATOMIC);
compared to: ts = get_timer(0); packet_sent=0; while(!packet_sent)
Yes, agree, it should be enough to call usb_ep_queue after clearing the flag as you suggest.
But I must mention that I have not yet tested it though...
It does not matter. I will repost the whole patch series with the comments we get up now. After that we can do some more tests to check if I break something...
Best regards, Stefano
participants (2)
-
Remy Bohmer -
Stefano Babic