[U-Boot] [PATCH] USB-CDC: called handle_interrupts inside usb_eth_send

[Remy Bohmer]

Hi Stefano,

2010/8/12 Stefano Babic <sbabic at 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