Re: [PATCH 2/2] USB: musb-gadget: fix OUT transfer in double buffer case
Thanks for your review.
2010/8/27 Gadiyar, Anand :
> tom.leim...@gmail.com wrote:
>> From: Ming Lei
>>
>> This patch fixes two bugs of OUT transfer in double buffer case:
>>
>> -USE_MODE1 should be enabled except for ANOMALY_05000456 case, or
>> else may cause infinite hang and data error bug in double buffer
>>
>
> Double buffering should still work with or without mode1 DMA.
> I'm not sure using Mode1 DMA is the fix for enabling double buffering.
I see why enabling mode 1 may fix the TX transfer in double buffer case.
We only enable auto clear for dma mode 1 case and for dma mode 0,
so it should be a bug.
>
>> -DMA length should not go beyond the availabe space of request buffer
>>
>> Without this patch, test #5 of usbtest can't be passed if we configure musb
>> as
>> g_zero and use fifo mode 3 to enable double buffer mode.
>>
>> With this patch, on my beagle B5, test#5(queued bulk out) may go beyond
>> 14Mbyte/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 5 -s 32768 -g 8 [1]
>>
>> [1],
>> -source of testusb : tools/usb/testusb.c under linux kernel;
>>
>> Signed-off-by: Ming Lei
>> Cc: Anand Gadiyar
>> Cc: Mike Frysinger
>> Cc: Sergei Shtylyov
>
> The DMA length part is a good fix. However, I'm not so sure I like the
> combining of the MODE1 RX change here.
I will submit two patches to fix the issue, one is to add 'auto clear'
for both dma 0 and dma 1, another fix the DMA length. I will submit the
two patches later for your review.
>
> With current code, only g_zero and g_file_storage work well with
> Mode1 DMA RX. g_ether used to break (have not tested recently with mode1).
> Did you test g_ether with your patch?
Yes, I test the g_ether just now and find it is broken with the previous patch.
>
> We know that at least on OMAPs, MODE1 DMA RX works reliably with patches
> Felipe submitted last December. These have not been resubmitted and we're
> waiting on that.
>
> - Anand
>
>
>>
>> ---
>> drivers/usb/musb/musb_dma.h | 2 ++
>> drivers/usb/musb/musb_gadget.c | 5 +++--
>> 2 files changed, 5 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/usb/musb/musb_dma.h b/drivers/usb/musb/musb_dma.h
>> index 916065b..dff91ce 100644
>> --- a/drivers/usb/musb/musb_dma.h
>> +++ b/drivers/usb/musb/musb_dma.h
>> @@ -89,6 +89,8 @@ struct musb_hw_ep;
>> # if !ANOMALY_05000456
>> # define USE_MODE1
>> # endif
>> +#else
>> +# define USE_MODE1
>> #endif
>>
>> /*
>> diff --git a/drivers/usb/musb/musb_gadget.c b/drivers/usb/musb/musb_gadget.c
>> index e0bd1c1..f1c44b9 100644
>> --- a/drivers/usb/musb/musb_gadget.c
>> +++ b/drivers/usb/musb/musb_gadget.c
>> @@ -662,10 +662,11 @@ static void rxstate(struct musb *musb,
>> struct musb_request *req)
>> if (request->actual < request->length) {
>> int transfer_size = 0;
>> #ifdef USE_MODE1
>> - transfer_size = min(request->length,
>> + transfer_size = min(request->length -
>> request->actual,
>> channel->max_len);
>> #else
>> - transfer_size = len;
>> + transfer_size = min(request->length -
>> request->actual,
>> + len);
>> #endif
>> if (transfer_size <=
>> musb_ep->packet_sz)
>> musb_ep->dma->desired_mode = 0;
>> --
>> 1.6.2.5
>>
>>
--
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
RE: [PATCH 2/2] USB: musb-gadget: fix OUT transfer in double buffer case
tom.leim...@gmail.com wrote:
> From: Ming Lei
>
> This patch fixes two bugs of OUT transfer in double buffer case:
>
> -USE_MODE1 should be enabled except for ANOMALY_05000456 case, or
> else may cause infinite hang and data error bug in double buffer
>
Double buffering should still work with or without mode1 DMA.
I'm not sure using Mode1 DMA is the fix for enabling double buffering.
> -DMA length should not go beyond the availabe space of request buffer
>
> Without this patch, test #5 of usbtest can't be passed if we configure musb as
> g_zero and use fifo mode 3 to enable double buffer mode.
>
> With this patch, on my beagle B5, test#5(queued bulk out) may go beyond
> 14Mbyte/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 5 -s 32768 -g 8 [1]
>
> [1],
> -source of testusb : tools/usb/testusb.c under linux kernel;
>
> Signed-off-by: Ming Lei
> Cc: Anand Gadiyar
> Cc: Mike Frysinger
> Cc: Sergei Shtylyov
The DMA length part is a good fix. However, I'm not so sure I like the
combining of the MODE1 RX change here.
With current code, only g_zero and g_file_storage work well with
Mode1 DMA RX. g_ether used to break (have not tested recently with mode1).
Did you test g_ether with your patch?
We know that at least on OMAPs, MODE1 DMA RX works reliably with patches
Felipe submitted last December. These have not been resubmitted and we're
waiting on that.
- Anand
>
> ---
> drivers/usb/musb/musb_dma.h|2 ++
> drivers/usb/musb/musb_gadget.c |5 +++--
> 2 files changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/usb/musb/musb_dma.h b/drivers/usb/musb/musb_dma.h
> index 916065b..dff91ce 100644
> --- a/drivers/usb/musb/musb_dma.h
> +++ b/drivers/usb/musb/musb_dma.h
> @@ -89,6 +89,8 @@ struct musb_hw_ep;
> # if !ANOMALY_05000456
> # define USE_MODE1
> # endif
> +#else
> +# define USE_MODE1
> #endif
>
> /*
> diff --git a/drivers/usb/musb/musb_gadget.c b/drivers/usb/musb/musb_gadget.c
> index e0bd1c1..f1c44b9 100644
> --- a/drivers/usb/musb/musb_gadget.c
> +++ b/drivers/usb/musb/musb_gadget.c
> @@ -662,10 +662,11 @@ static void rxstate(struct musb *musb,
> struct musb_request *req)
> if (request->actual < request->length) {
> int transfer_size = 0;
> #ifdef USE_MODE1
> - transfer_size = min(request->length,
> + transfer_size = min(request->length -
> request->actual,
> channel->max_len);
> #else
> - transfer_size = len;
> + transfer_size = min(request->length -
> request->actual,
> + len);
> #endif
> if (transfer_size <= musb_ep->packet_sz)
> musb_ep->dma->desired_mode = 0;
> --
> 1.6.2.5
>
> --
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
[PATCH 2/2] USB: musb-gadget: fix OUT transfer in double buffer case
From: Ming Lei
This patch fixes two bugs of OUT transfer in double buffer case:
-USE_MODE1 should be enabled except for ANOMALY_05000456 case, or
else may cause infinite hang and data error bug in double buffer
-DMA length should not go beyond the availabe space of request buffer
Without this patch, test #5 of usbtest can't be passed if we configure musb as
g_zero and use fifo mode 3 to enable double buffer mode.
With this patch, on my beagle B5, test#5(queued bulk out) may go beyond
14Mbyte/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 5 -s 32768 -g 8 [1]
[1],
-source of testusb : tools/usb/testusb.c under linux kernel;
Signed-off-by: Ming Lei
Cc: Anand Gadiyar
Cc: Mike Frysinger
Cc: Sergei Shtylyov
---
drivers/usb/musb/musb_dma.h|2 ++
drivers/usb/musb/musb_gadget.c |5 +++--
2 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/musb/musb_dma.h b/drivers/usb/musb/musb_dma.h
index 916065b..dff91ce 100644
--- a/drivers/usb/musb/musb_dma.h
+++ b/drivers/usb/musb/musb_dma.h
@@ -89,6 +89,8 @@ struct musb_hw_ep;
# if !ANOMALY_05000456
# define USE_MODE1
# endif
+#else
+# define USE_MODE1
#endif
/*
diff --git a/drivers/usb/musb/musb_gadget.c b/drivers/usb/musb/musb_gadget.c
index e0bd1c1..f1c44b9 100644
--- a/drivers/usb/musb/musb_gadget.c
+++ b/drivers/usb/musb/musb_gadget.c
@@ -662,10 +662,11 @@ static void rxstate(struct musb *musb, struct
musb_request *req)
if (request->actual < request->length) {
int transfer_size = 0;
#ifdef USE_MODE1
- transfer_size = min(request->length,
+ transfer_size = min(request->length -
request->actual,
channel->max_len);
#else
- transfer_size = len;
+ transfer_size = min(request->length -
request->actual,
+ len);
#endif
if (transfer_size <= musb_ep->packet_sz)
musb_ep->dma->desired_mode = 0;
--
1.6.2.5
--
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
