USB: musb-gadget: fix OUT transfer in double buffer case
Subsystems (2)
PATCH 2/2 USB: musb-gadget: fix OUT transfer in double buffer case
From: Ming Lei <[email protected]>
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;
--- drivers/usb/musb/musb_dma.h | 2 ++ drivers/usb/musb/musb_gadget.c | 5 +++-- 2 files changed, 5 insertions(+), 2 deletions(-)
drivers/usb/musb/musb_dma.h +2 -0
@@ -89,6 +89,8 @@ struct musb_hw_ep;# if !ANOMALY_05000456 # define USE_MODE1 # endif +#else +# define USE_MODE1 #endif /*
drivers/usb/musb/musb_gadget.c +3 -2
@@ -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 ;
Signature · 1 line
1.6.2.5
Quoted 7 lines · [email protected] wrote:
From: Ming Lei <[email protected]>
This patch fixes two bugs of OUT transfer in double buffer case:
-USE_MODE1 should be enabled except for
ANOMALY_05000456case, 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.
Quoted 10 lines
-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_zeroand 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_zeroand fifo mode 3 is taken, follows the test command:#./testusb -D DEV_NAME -c 1024 -t 5 -s 32768 -g 8 [1]
Quoted 7 lines
[1],
-source of testusb : tools/usb/testusb.c under linux kernel;Signed-off-by Ming Lei <[email protected]>Cc Anand Gadiyar <[email protected]>Cc Mike Frysinger <[email protected]>Cc Sergei Shtylyov <[email protected]>
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
Quoted 42 lines
--- drivers/usb/musb/musb_dma.h | 2 ++ drivers/usb/musb/musb_gadget.c | 5 +++-- 2 files changed, 5 insertions(+), 2 deletions(-)drivers/usb/musb/musb_dma.h +2
-0@@ -89,6 +89,8 @@ struct musb_hw_ep;# if !ANOMALY_05000456 # define USE_MODE1 # endif +#else +# define USE_MODE1 #endif /* drivers/usb/musb/musb_gadget.c +3
-2@@ -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;Signature · 1 line
1.6.2.5
A message that is not in the archive (sent off-list or not yet received): 5A47E75E594F054BAF48C5E4FC4B92AB0328C1CB4B-/tLxBxkBPtCIQmiDNMet8wC/[email protected]
Thanks for your review.
2010/8/27 Gadiyar, Anand <[email protected]>:
Quoted 11 lines
Quoted 7 lines · [email protected] wrote:
From: Ming Lei <[email protected]>
This patch fixes two bugs of OUT transfer in double buffer case:
-USE_MODE1 should be enabled except for
ANOMALY_05000456case, or else may cause infinite hang and data error bug in double bufferDouble 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.
Quoted 14 lines
Quoted 14 lines
-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_zeroand 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_zeroand 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.cunder linux kernel;
Quoted 7 lines
Quoted 4 lines
Signed-off-by Ming Lei <[email protected]>Cc Anand Gadiyar <[email protected]>Cc Mike Frysinger <[email protected]>Cc Sergei Shtylyov <[email protected]>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.
Quoted 4 lines
With current code, only
g_zeroandg_file_storagework well with Mode1 DMA RX.g_etherused to break (have not tested recently with mode1). Did you testg_etherwith your patch?
Yes, I test the g_ether just now and find it is broken with the previous patch.
Quoted 50 lines
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
Quoted 42 lines
---
drivers/usb/musb/musb_dma.h | 2 ++
drivers/usb/musb/musb_gadget.c | 5 +++--2 files changed, 5 insertions(+), 2 deletions(-)drivers/usb/musb/musb_dma.h +2
-0@@ -89,6 +89,8 @@ struct musb_hw_ep; # if !ANOMALY_05000456 # define USE_MODE1 # endif +#else +# define USE_MODE1 #endif /*drivers/usb/musb/musb_gadget.c +3
-2@@ -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
Signature · 5 lines
Lei Ming -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html