[PATCH 1/2] USB: musb-gadget: enable autoclear for OUT transfer in both DMA 0 and DMA 1

[PATCH 1/2] USB: musb-gadget: enable autoclear for OUT transfer in both DMA 0 and DMA 1

[tom.leiming]

From: Ming Lei <tom.leiming@gmail.com>

This patch fixes one bugs of OUT transfer in double buffer case:

-the current code only enable autoclear for dma mode 1, and not
for dma mode 0

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 and the following patch(fix dma length),
on my beagle B5, test#5(queued bulk out) may go beyond
18Mbyte/s(seems dma mode 0 is quicker in double buffer case)
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]

Also I have tested this patch can't make g_ether broken.

[1],
    -source of testusb : tools/usb/testusb.c under linux kernel;

Signed-off-by: Ming Lei <tom.leiming@gmail.com>
Cc: Anand Gadiyar <gadiyar@ti.com>
Cc: Mike Frysinger <vapier@gentoo.org>
Cc: Sergei Shtylyov <sshtylyov@ru.mvista.com>
---
 drivers/usb/musb/musb_gadget.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/usb/musb/musb_gadget.c b/drivers/usb/musb/musb_gadget.c
index e0bd1c1..c0dfec4 100644
--- a/drivers/usb/musb/musb_gadget.c
+++ b/drivers/usb/musb/musb_gadget.c
@@ -645,9 +645,9 @@ static void rxstate(struct musb *musb, struct musb_request *req)
 	 */
 
 				csr |= MUSB_RXCSR_DMAENAB;
-#ifdef USE_MODE1
 				if (!musb_ep->hb_mult)
 					csr |= MUSB_RXCSR_AUTOCLEAR;
+#ifdef USE_MODE1
 				/* csr |= MUSB_RXCSR_DMAMODE; */
 
 				/* this special sequence (enabling and then
-- 
1.6.2.5

RE: [PATCH 1/2] USB: musb-gadget: enable autoclear for OUT transfer in both DMA 0 and DMA 1

[Gadiyar, Anand]

tom.leiming@gmail.com wrote:
> 
> From: Ming Lei <tom.leiming@gmail.com>
> 
> This patch fixes one bugs of OUT transfer in double buffer case:
> 
> -the current code only enable autoclear for dma mode 1, and not
> for dma mode 0
> 
> 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 and the following patch(fix dma length),
> on my beagle B5, test#5(queued bulk out) may go beyond
> 18Mbyte/s(seems dma mode 0 is quicker in double buffer case)
> 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]
> 
> Also I have tested this patch can't make g_ether broken.
> 
> [1],
>     -source of testusb : tools/usb/testusb.c under linux kernel;
> 
> Signed-off-by: Ming Lei <tom.leiming@gmail.com>
> Cc: Anand Gadiyar <gadiyar@ti.com>
> Cc: Mike Frysinger <vapier@gentoo.org>
> Cc: Sergei Shtylyov <sshtylyov@ru.mvista.com>

I'm not 100% sure why not having autoclear was causing a failure
with double-buffering enabled. I'd like this patch to be tested
more on OMAP as well as on Blackfin/Davinci.

I'll need time to study MUSB specs again to understand this,
so I'll skip acking this patch and let Felipe/Sergei/Mike/Ajay
take a look.

Thanks for taking the time to debug this - if this patch series
makes double buffering work reliably, that's a very good boost
to throughput.

- Anand

> ---
>  drivers/usb/musb/musb_gadget.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/usb/musb/musb_gadget.c b/drivers/usb/musb/musb_gadget.c
> index e0bd1c1..c0dfec4 100644
> --- a/drivers/usb/musb/musb_gadget.c
> +++ b/drivers/usb/musb/musb_gadget.c
> @@ -645,9 +645,9 @@ static void rxstate(struct musb *musb, struct musb_request *req)
>  	 */
>  
>  				csr |= MUSB_RXCSR_DMAENAB;
> -#ifdef USE_MODE1
>  				if (!musb_ep->hb_mult)
>  					csr |= MUSB_RXCSR_AUTOCLEAR;
> +#ifdef USE_MODE1
>  				/* csr |= MUSB_RXCSR_DMAMODE; */
>  
>  				/* this special sequence (enabling and then
> -- 
> 1.6.2.5
> 
> 

Re: [PATCH 1/2] USB: musb-gadget: enable autoclear for OUT transfer in both DMA 0 and DMA 1

[Ming Lei]

2010/8/27 Gadiyar, Anand <gadiyar@ti.com>:
> tom.leiming@gmail.com wrote:
>>
>> From: Ming Lei <tom.leiming@gmail.com>
>>
>> This patch fixes one bugs of OUT transfer in double buffer case:
>>
>> -the current code only enable autoclear for dma mode 1, and not
>> for dma mode 0
>>
>> 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 and the following patch(fix dma length),
>> on my beagle B5, test#5(queued bulk out) may go beyond
>> 18Mbyte/s(seems dma mode 0 is quicker in double buffer case)
>> 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]
>>
>> Also I have tested this patch can't make g_ether broken.
>>
>> [1],
>>     -source of testusb : tools/usb/testusb.c under linux kernel;
>>
>> Signed-off-by: Ming Lei <tom.leiming@gmail.com>
>> Cc: Anand Gadiyar <gadiyar@ti.com>
>> Cc: Mike Frysinger <vapier@gentoo.org>
>> Cc: Sergei Shtylyov <sshtylyov@ru.mvista.com>
>
> I'm not 100% sure why not having autoclear was causing a failure
> with double-buffering enabled. I'd like this patch to be tested
> more on OMAP as well as on Blackfin/Davinci.

We don't clear MUSB_RXCSR_RXPKTRDY explicitly when dma complete,
so if autoclear is not enabled, the dma read for packets from fifo will
not be acked, then any odd things may happen...

I have tested the patch for both single buffer and double buffer case,
test #5 and test #6 are passed, and result of test #5 in double buffer
may go beyond 18M/s(seems much quicker than dma mode 1).

Also, g_ether is OK with the patch.

>
> I'll need time to study MUSB specs again to understand this,
> so I'll skip acking this patch and let Felipe/Sergei/Mike/Ajay
> take a look.

Very good.

>
> Thanks for taking the time to debug this - if this patch series
> makes double buffering work reliably, that's a very good boost
> to throughput.

>
> - Anand
>
>> ---
>>  drivers/usb/musb/musb_gadget.c |    2 +-
>>  1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/drivers/usb/musb/musb_gadget.c b/drivers/usb/musb/musb_gadget.c
>> index e0bd1c1..c0dfec4 100644
>> --- a/drivers/usb/musb/musb_gadget.c
>> +++ b/drivers/usb/musb/musb_gadget.c
>> @@ -645,9 +645,9 @@ static void rxstate(struct musb *musb, struct musb_request *req)
>>        */
>>
>>                               csr |= MUSB_RXCSR_DMAENAB;
>> -#ifdef USE_MODE1
>>                               if (!musb_ep->hb_mult)
>>                                       csr |= MUSB_RXCSR_AUTOCLEAR;
>> +#ifdef USE_MODE1
>>                               /* csr |= MUSB_RXCSR_DMAMODE; */
>>
>>                               /* this special sequence (enabling and then
>> --
>> 1.6.2.5
>>
>>



-- 
Lei Ming
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH 1/2] USB: musb-gadget: enable autoclear for OUT transfer in both DMA 0 and DMA 1

[Gadiyar, Anand]

tom.leiming@gmail.com wrote:
> 
> From: Ming Lei <tom.leiming@gmail.com>
> 
> This patch fixes one bugs of OUT transfer in double buffer case:
> 
> -the current code only enable autoclear for dma mode 1, and not
> for dma mode 0
> 
> 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 and the following patch(fix dma length),
> on my beagle B5, test#5(queued bulk out) may go beyond
> 18Mbyte/s(seems dma mode 0 is quicker in double buffer case)
> 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]
> 
> Also I have tested this patch can't make g_ether broken.
> 
> [1],
>     -source of testusb : tools/usb/testusb.c under linux kernel;
> 
> Signed-off-by: Ming Lei <tom.leiming@gmail.com>
> Cc: Anand Gadiyar <gadiyar@ti.com>
> Cc: Mike Frysinger <vapier@gentoo.org>
> Cc: Sergei Shtylyov <sshtylyov@ru.mvista.com>
> ---
>  drivers/usb/musb/musb_gadget.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/usb/musb/musb_gadget.c b/drivers/usb/musb/musb_gadget.c
> index e0bd1c1..c0dfec4 100644
> --- a/drivers/usb/musb/musb_gadget.c
> +++ b/drivers/usb/musb/musb_gadget.c
> @@ -645,9 +645,9 @@ static void rxstate(struct musb *musb, struct musb_request *req)
>  	 */
>  
>  				csr |= MUSB_RXCSR_DMAENAB;
> -#ifdef USE_MODE1
>  				if (!musb_ep->hb_mult)
>  					csr |= MUSB_RXCSR_AUTOCLEAR;

Hi Tom,

Which codebase is this patch meant for?
I don't see the hb_mult in current mainline code?

Are there any patches which this one depends on?

Thanks,
Anand

> +#ifdef USE_MODE1
>  				/* csr |= MUSB_RXCSR_DMAMODE; */
>  
>  				/* this special sequence (enabling and then

Re: [PATCH 1/2] USB: musb-gadget: enable autoclear for OUT transfer in both DMA 0 and DMA 1

[Ming Lei]

2010/9/6 Gadiyar, Anand <gadiyar-l0cyMroinI0@public.gmane.org>:
> Hi Tom,
>
> Which codebase is this patch meant for?
> I don't see the hb_mult in current mainline code?
>
> Are there any patches which this one depends on?

This depends on the two iso for device mode patches:
         http://marc.info/?l=linux-usb&m=128076716001885&w=2
         http://marc.info/?l=linux-usb&m=128076716901924&w=2

I plan to resend the two patches above and the double buffer patches
later.

thanks,

-- 
Lei Ming
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] USB: musb-gadget: enable autoclear for OUT transfer in both DMA 0 and DMA 1

[Sergei Shtylyov]

Hello.

Ming Lei wrote:

>> Which codebase is this patch meant for?
>> I don't see the hb_mult in current mainline code?

>> Are there any patches which this one depends on?

> This depends on the two iso for device mode patches:
>          http://marc.info/?l=linux-usb&m=128076716001885&w=2
>          http://marc.info/?l=linux-usb&m=128076716901924&w=2

> I plan to resend the two patches above and the double buffer patches
> later.

    I recommend that you change the ordering of patches so that fixes come 
first, and new features last. I.e. this patch should precede the second one of 
those two above.

> thanks,

WBR, Sergei
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] USB: musb-gadget: enable autoclear for OUT transfer in both DMA 0 and DMA 1

[Ming Lei]

2010/9/7 Sergei Shtylyov <sshtylyov-Igf4POYTYCDQT0dZR+AlfA@public.gmane.org>:
> Hello.
>
> Ming Lei wrote:
>
>>> Which codebase is this patch meant for?
>>> I don't see the hb_mult in current mainline code?
>
>>> Are there any patches which this one depends on?
>
>> This depends on the two iso for device mode patches:
>>         http://marc.info/?l=linux-usb&m=128076716001885&w=2
>>         http://marc.info/?l=linux-usb&m=128076716901924&w=2
>
>> I plan to resend the two patches above and the double buffer patches
>> later.
>
>   I recommend that you change the ordering of patches so that fixes come
> first, and new features last. I.e. this patch should precede the second one
> of those two above.

Good point, I will resend all the fix patches first, then the
isochronous support
patch.

thanks,

-- 
Lei Ming
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html