2010/8/28 Mike Frysinger <vap...@gentoo.org>:
> On Friday, August 27, 2010 07:37:20 tom.leim...@gmail.com wrote:
>> From: Ming Lei <tom.leim...@gmail.com>
>>
>> This patch fixes one infinite hang of bulk IN transfer in double buffer
>> case, the hang can be observed easily by test #6 of usbtest if musb is
>> configured as g_zero and fifo mode 3 is taken to enable double fifo.
>>
>> In fact, the patch only removes the check for non-empty fifo before
>> loading data from new request into fifo since the check is not correct:
>>
>> -in double buffer case, fifo may accommodate more than one packet,
>> even though it has contained one packet already and is non-empty
>>
>> -since last DMA is completed before calling musb_g_tx, it is sure
>> that fifo may accommodate at least one packet
>>
>> Without applying the patch, new requst enqueued from .complte may not
>> have a chance to be loaded into fifo, then will never be completed and
>> cause infinite hangs.
>>
>> With the patch, on my beagle B5, test#6(queued bulk in) can be passed and
>> test result may go beyond 33Mbyte/s if musb is configured as g_zero and
>> fifo mode 3 is taken, follows the test command:
>>
>> #testusb -D DEV_NAME -c 1024 -t 6 -s 32768 -g 8 [1]
>>
>> [1],
>> -source of testusb : tools/usb/testusb.c under linux kernel;
>>
>> Signed-off-by: Ming Lei <tom.leim...@gmail.com>
>> Cc: Anand Gadiyar <gadi...@ti.com>
>> Cc: Mike Frysinger <vap...@gentoo.org>
>> Cc: Sergei Shtylyov <sshtyl...@ru.mvista.com>
>>
>> ---
>> drivers/usb/musb/musb_gadget.c | 12 ------------
>> 1 files changed, 0 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/usb/musb/musb_gadget.c
>> b/drivers/usb/musb/musb_gadget.c index 550b397..e0bd1c1 100644
>> --- a/drivers/usb/musb/musb_gadget.c
>> +++ b/drivers/usb/musb/musb_gadget.c
>> @@ -506,18 +506,6 @@ void musb_g_tx(struct musb *musb, u8 epnum)
>> /* ... or if not, then complete it. */
>> musb_g_giveback(musb_ep, request, 0);
>>
>> - /*
>> - * Kickstart next transfer if appropriate;
>> - * the packet that just completed might not
>> - * be transmitted for hours or days.
>> - * REVISIT for double buffering...
>> - * FIXME revisit for stalls too...
>> - */
>> - musb_ep_select(mbase, epnum);
>> - csr = musb_readw(epio, MUSB_TXCSR);
>> - if (csr & MUSB_TXCSR_FIFONOTEMPTY)
>> - return;
>> -
>> request = musb_ep->desc ? next_request(musb_ep) : NULL;
>> if (!request) {
>> DBG(4, "%s idle now\n",
>
> maybe this change will make a difference ?
No, only your change can not prevent the infinite hangs for OUT transfer,
see the explanation in my commit log. Even the change may cause infinite
hangs again, once return if request->actual is less than request->length,
musb will stop loading data from requst->buf into fifo, so hangs...
> From 13fa6c9cec387893e2b22db6c055028f96fea5b8 Mon Sep 17 00:00:00 2001
> From: Cliff Cai <cliff....@analog.com>
> Date: Fri, 3 Jul 2009 06:32:02 +0000
> Subject: [PATCH] USB: musb: only complete the request when all data has been
> transfered [local/log]
>
> bug[#5269],bug[#5268],bug[#5264]only complete the request when all data has
> been transfered
Oh, another local change...
>
> git-svn-id: svn://localhost/svn/linux-kernel/tr...@6915 526b6c2d-f592-4532-
> a319-5dd88ccb003d
> ---
> drivers/usb/musb/musb_gadget.c | 2 ++
> 1 files changed, 2 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/usb/musb/musb_gadget.c b/drivers/usb/musb/musb_gadget.c
> index 6fca870..f019843 100644
> --- a/drivers/usb/musb/musb_gadget.c
> +++ b/drivers/usb/musb/musb_gadget.c
> @@ -500,6 +500,8 @@ void musb_g_tx(struct musb *musb, u8 epnum)
> | MUSB_TXCSR_TXPKTRDY);
> request->zero = 0;
> }
> + if (request->actual < request->length)
> + return;
As said above, this change above should be wrong, follows the fix which
may complete request only if data is transfered over for the request.
diff --git a/drivers/usb/musb/musb_gadget.c b/drivers/usb/musb/musb_gadget.c
index e898228..1cf6d28 100644
--- a/drivers/usb/musb/musb_gadget.c
+++ b/drivers/usb/musb/musb_gadget.c
@@ -503,14 +503,14 @@ void musb_g_tx(struct musb *musb, u8 epnum)
request->zero = 0;
}
- /* ... or if not, then complete it. */
- musb_g_giveback(musb_ep, request, 0);
-
- request = musb_ep->desc ? next_request(musb_ep) : NULL;
- if (!request) {
- DBG(4, "%s idle now\n",
- musb_ep->end_point.name);
- return;
+ if (request->actual == request->length) {
+ musb_g_giveback(musb_ep, request, 0);
+ request = musb_ep->desc ? next_request(musb_ep)
: NULL;
+ if (!request) {
+ DBG(4, "%s idle now\n",
+ musb_ep->end_point.name);
+ return;
+ }
}
}
--
Lei Ming
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
