Re: [PATCH 1/2] usb: musb: use DMA mode 1 whenever possible

2012-10-23 Thread ABRAHAM, KISHON VIJAY
Hi,

On Wed, Aug 8, 2012 at 11:28 AM, Rajaram R rajaram.officem...@gmail.com wrote:
 On Tue, Aug 7, 2012 at 6:39 PM, Roger Quadros rog...@ti.com wrote:
 Do not rely on any hints from gadget drivers and use DMA mode 1
 whenever we expect data of at least the endpoint's packet size and
 have not yet received a short packet.

 Could you please let us know what all combination this was tested ?
 What will happen if the request length is 513 ?


 The last packet if short is always transferred using DMA mode 0.

 This patch fixes USB throughput issues in mass storage mode for
 host to device transfers.

 Signed-off-by: Roger Quadros rog...@ti.com

This commit is causing regression while using the test gadget.

output of ./test.sh in usb host machine

./test.sh
./test.sh: 31: ./test.sh: declare: not found
TESTING:  control out in
Tue Oct 23 15:25:29 IST 2012
** Control test cases:
test 9: ch9 postconfig
/dev/bus/usb/001/020 test 9,   63.749319 secs
test 10: control queueing
/dev/bus/usb/001/020 test 10,   10.417282 secs
test 14: control writes
/dev/bus/usb/001/020 test 14,4.579272 secs

assuming sink-src configuration
** Host Write (OUT) test cases:
test 1: 5000 transfers, same size
stays here infinitely

Thanks
Kishon
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 1/2] usb: musb: use DMA mode 1 whenever possible

2012-08-07 Thread Roger Quadros
Do not rely on any hints from gadget drivers and use DMA mode 1
whenever we expect data of at least the endpoint's packet size and
have not yet received a short packet.

The last packet if short is always transferred using DMA mode 0.

This patch fixes USB throughput issues in mass storage mode for
host to device transfers.

Signed-off-by: Roger Quadros rog...@ti.com
---
 drivers/usb/musb/musb_gadget.c |   30 --
 1 files changed, 4 insertions(+), 26 deletions(-)

diff --git a/drivers/usb/musb/musb_gadget.c b/drivers/usb/musb/musb_gadget.c
index 8d2cce1..5c4392b 100644
--- a/drivers/usb/musb/musb_gadget.c
+++ b/drivers/usb/musb/musb_gadget.c
@@ -707,12 +707,11 @@ static void rxstate(struct musb *musb, struct 
musb_request *req)
fifo_count = musb_readw(epio, MUSB_RXCOUNT);
 
/*
-* Enable Mode 1 on RX transfers only when short_not_ok flag
-* is set. Currently short_not_ok flag is set only from
-* file_storage and f_mass_storage drivers
+*  use mode 1 only if we expect data of at least ep packet_sz
+*  and have not yet received a short packet
 */
-
-   if (request-short_not_ok  fifo_count == musb_ep-packet_sz)
+   if ((request-length - request-actual = musb_ep-packet_sz) 
+   (fifo_count = musb_ep-packet_sz))
use_mode_1 = 1;
else
use_mode_1 = 0;
@@ -727,27 +726,6 @@ static void rxstate(struct musb *musb, struct musb_request 
*req)
c = musb-dma_controller;
channel = musb_ep-dma;
 
-   /* We use DMA Req mode 0 in rx_csr, and DMA controller operates in
-* mode 0 only. So we do not get endpoint interrupts due to DMA
-* completion. We only get interrupts from DMA controller.
-*
-* We could operate in DMA mode 1 if we knew the size of the tranfer
-* in advance. For mass storage class, request-length = what the host
-* sends, so that'd work.  But for pretty much everything else,
-* request-length is routinely more than what the host sends. For
-* most these gadgets, end of is signified either by a short packet,
-* or filling the last byte of the buffer.  (Sending extra data in
-* that last pckate should trigger an overflow fault.)  But in mode 1,
-* we don't get DMA completion interrupt for short packets.
-*
-* Theoretically, we could enable DMAReq irq (MUSB_RXCSR_DMAMODE = 1),
-* to get endpoint interrupt on every DMA req, but that didn't seem
-* to work reliably.
-*
-* REVISIT an updated g_file_storage can set req-short_not_ok, which
-* then becomes usable as a runtime use mode 1 hint...
-*/
-
/* Experimental: Mode1 works with mass storage 
use cases */
if (use_mode_1) {
csr |= MUSB_RXCSR_AUTOCLEAR;
-- 
1.7.4.1

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


Re: [PATCH 1/2] usb: musb: use DMA mode 1 whenever possible

2012-08-07 Thread Sergei Shtylyov
Hello.

On 08/07/2012 05:09 PM, Roger Quadros wrote:

 Do not rely on any hints from gadget drivers and use DMA mode 1
 whenever we expect data of at least the endpoint's packet size and
 have not yet received a short packet.

 The last packet if short is always transferred using DMA mode 0.

 This patch fixes USB throughput issues in mass storage mode for
 host to device transfers.

 Signed-off-by: Roger Quadros rog...@ti.com
 ---
  drivers/usb/musb/musb_gadget.c |   30 --
  1 files changed, 4 insertions(+), 26 deletions(-)

 diff --git a/drivers/usb/musb/musb_gadget.c b/drivers/usb/musb/musb_gadget.c
 index 8d2cce1..5c4392b 100644
 --- a/drivers/usb/musb/musb_gadget.c
 +++ b/drivers/usb/musb/musb_gadget.c
 @@ -707,12 +707,11 @@ static void rxstate(struct musb *musb, struct 
 musb_request *req)
   fifo_count = musb_readw(epio, MUSB_RXCOUNT);
  
   /*
 -  * Enable Mode 1 on RX transfers only when short_not_ok flag
 -  * is set. Currently short_not_ok flag is set only from
 -  * file_storage and f_mass_storage drivers
 +  *  use mode 1 only if we expect data of at least ep packet_sz
 +  *  and have not yet received a short packet

   Why offset the comment text with 2 spaces?

*/
 -
 - if (request-short_not_ok  fifo_count == musb_ep-packet_sz)
 + if ((request-length - request-actual = musb_ep-packet_sz) 
 + (fifo_count = musb_ep-packet_sz))

   'fifo_count' shouldn't ever be  packet size (unless there's babble error??).

WBR, Sergei

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


Re: [PATCH 1/2] usb: musb: use DMA mode 1 whenever possible

2012-08-07 Thread Rajaram R
On Tue, Aug 7, 2012 at 6:39 PM, Roger Quadros rog...@ti.com wrote:
 Do not rely on any hints from gadget drivers and use DMA mode 1
 whenever we expect data of at least the endpoint's packet size and
 have not yet received a short packet.

Could you please let us know what all combination this was tested ?
What will happen if the request length is 513 ?


 The last packet if short is always transferred using DMA mode 0.

 This patch fixes USB throughput issues in mass storage mode for
 host to device transfers.

 Signed-off-by: Roger Quadros rog...@ti.com
 ---
  drivers/usb/musb/musb_gadget.c |   30 --
  1 files changed, 4 insertions(+), 26 deletions(-)

 diff --git a/drivers/usb/musb/musb_gadget.c b/drivers/usb/musb/musb_gadget.c
 index 8d2cce1..5c4392b 100644
 --- a/drivers/usb/musb/musb_gadget.c
 +++ b/drivers/usb/musb/musb_gadget.c
 @@ -707,12 +707,11 @@ static void rxstate(struct musb *musb, struct 
 musb_request *req)
 fifo_count = musb_readw(epio, MUSB_RXCOUNT);

 /*
 -* Enable Mode 1 on RX transfers only when short_not_ok flag
 -* is set. Currently short_not_ok flag is set only from
 -* file_storage and f_mass_storage drivers
 +*  use mode 1 only if we expect data of at least ep packet_sz
 +*  and have not yet received a short packet
  */
 -
 -   if (request-short_not_ok  fifo_count == musb_ep-packet_sz)
 +   if ((request-length - request-actual = musb_ep-packet_sz) 
 
 +   (fifo_count = musb_ep-packet_sz))
 use_mode_1 = 1;
 else
 use_mode_1 = 0;
 @@ -727,27 +726,6 @@ static void rxstate(struct musb *musb, struct 
 musb_request *req)
 c = musb-dma_controller;
 channel = musb_ep-dma;

 -   /* We use DMA Req mode 0 in rx_csr, and DMA controller operates in
 -* mode 0 only. So we do not get endpoint interrupts due to DMA
 -* completion. We only get interrupts from DMA controller.
 -*
 -* We could operate in DMA mode 1 if we knew the size of the tranfer
 -* in advance. For mass storage class, request-length = what the host
 -* sends, so that'd work.  But for pretty much everything else,
 -* request-length is routinely more than what the host sends. For
 -* most these gadgets, end of is signified either by a short packet,
 -* or filling the last byte of the buffer.  (Sending extra data in
 -* that last pckate should trigger an overflow fault.)  But in mode 1,
 -* we don't get DMA completion interrupt for short packets.
 -*
 -* Theoretically, we could enable DMAReq irq (MUSB_RXCSR_DMAMODE = 1),
 -* to get endpoint interrupt on every DMA req, but that didn't seem
 -* to work reliably.
 -*
 -* REVISIT an updated g_file_storage can set req-short_not_ok, which
 -* then becomes usable as a runtime use mode 1 hint...
 -*/
 -
 /* Experimental: Mode1 works with mass 
 storage use cases */
 if (use_mode_1) {
 csr |= MUSB_RXCSR_AUTOCLEAR;
 --
 1.7.4.1

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