Re: [PATCH v4] usb: musb: Enable DMA mode1 RX for USB-Mass-Storage

2011-07-28 Thread Felipe Balbi
Hi,

On Tue, Jul 19, 2011 at 10:11:58PM -0700, Vikram Pandita wrote:
 From: Anand Gadiyar gadi...@ti.com
 
 This patch enables the DMA mode1 RX support.
 This feature is enabled based on the short_not_ok flag passed from
 gadget drivers.
 
 This will result in a thruput performance gain of around
 40% for USB mass-storage/mtp use cases.
 
 Signed-off-by: Anand Gadiyar gadi...@ti.com
 Signed-off-by: Moiz Sonasath m-sonas...@ti.com
 Signed-off-by: Vikram Pandita vikram.pand...@ti.com
 Tested-by: Vikram Pandita vikram.pand...@ti.com

applied this one, but I changed commit log and code comment a little
bit. Here's updated commit:

commit e9c281b174f188adb7950ea8f6a55ca07be69914
Author: Anand Gadiyar gadi...@ti.com
Date:   Tue Jul 19 22:11:58 2011 -0700

usb: musb: Enable DMA mode1 RX for transfers without short packets

This patch enables DMA mode1 for the RX patch when we know
there won't be any short packets. We check that by looking
into the short_no_ok flag, if it's true we enable mode1, otherwise
we use mode0 to transfer the data.

This will result in a throughput performance gain of around
40% for USB mass-storage/mtp use cases.

[ ba...@ti.com : updated commit log and code comments slightly ]

Signed-off-by: Anand Gadiyar gadi...@ti.com
Signed-off-by: Moiz Sonasath m-sonas...@ti.com
Signed-off-by: Vikram Pandita vikram.pand...@ti.com
Tested-by: Vikram Pandita vikram.pand...@ti.com
Signed-off-by: Felipe Balbi ba...@ti.com

diff --git a/drivers/usb/musb/musb_gadget.c b/drivers/usb/musb/musb_gadget.c
index b67a062..d314f58 100644
--- a/drivers/usb/musb/musb_gadget.c
+++ b/drivers/usb/musb/musb_gadget.c
@@ -634,6 +634,7 @@ static void rxstate(struct musb *musb, struct musb_request 
*req)
u16 len;
u16 csr = musb_readw(epio, MUSB_RXCSR);
struct musb_hw_ep   *hw_ep = musb-endpoints[epnum];
+   u8  use_mode_1;
 
if (hw_ep-is_shared_fifo)
musb_ep = hw_ep-ep_in;
@@ -683,6 +684,18 @@ static void rxstate(struct musb *musb, struct musb_request 
*req)
 
if (csr  MUSB_RXCSR_RXPKTRDY) {
len = 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
+*/
+
+   if (request-short_not_ok  len == musb_ep-packet_sz)
+   use_mode_1 = 1;
+   else
+   use_mode_1 = 0;
+
if (request-actual  request-length) {
 #ifdef CONFIG_USB_INVENTRA_DMA
if (is_buffer_mapped(req)) {
@@ -714,37 +727,41 @@ static void rxstate(struct musb *musb, struct 
musb_request *req)
 * then becomes usable as a runtime use mode 1 hint...
 */
 
-   csr |= MUSB_RXCSR_DMAENAB;
-#ifdef USE_MODE1
-   csr |= MUSB_RXCSR_AUTOCLEAR;
-   /* csr |= MUSB_RXCSR_DMAMODE; */
-
-   /* this special sequence (enabling and then
-* disabling MUSB_RXCSR_DMAMODE) is required
-* to get DMAReq to activate
-*/
-   musb_writew(epio, MUSB_RXCSR,
-   csr | MUSB_RXCSR_DMAMODE);
-#else
-   if (!musb_ep-hb_mult 
-   musb_ep-hw_ep-rx_double_buffered)
+   /* Experimental: Mode1 works with mass storage 
use cases */
+   if (use_mode_1) {
csr |= MUSB_RXCSR_AUTOCLEAR;
-#endif
-   musb_writew(epio, MUSB_RXCSR, csr);
+   musb_writew(epio, MUSB_RXCSR, csr);
+   csr |= MUSB_RXCSR_DMAENAB;
+   musb_writew(epio, MUSB_RXCSR, csr);
+
+   /*
+* this special sequence (enabling and 
then
+* disabling MUSB_RXCSR_DMAMODE) is 
required
+* to get DMAReq to activate
+*/
+   musb_writew(epio, MUSB_RXCSR,
+   csr | MUSB_RXCSR_DMAMODE);
+   musb_writew(epio, MUSB_RXCSR, csr);
+
+   } else {
+   if (!musb_ep-hb_mult 
+   
musb_ep-hw_ep-rx_double_buffered)
+   csr 

Re: [PATCH v4] usb: musb: Enable DMA mode1 RX for USB-Mass-Storage

2011-07-28 Thread Pandita, Vikram
On Thu, Jul 28, 2011 at 9:45 PM, Felipe Balbi ba...@ti.com wrote:
 Hi,

 On Tue, Jul 19, 2011 at 10:11:58PM -0700, Vikram Pandita wrote:
 From: Anand Gadiyar gadi...@ti.com

 This patch enables the DMA mode1 RX support.
 This feature is enabled based on the short_not_ok flag passed from
 gadget drivers.

 This will result in a thruput performance gain of around
 40% for USB mass-storage/mtp use cases.

 Signed-off-by: Anand Gadiyar gadi...@ti.com
 Signed-off-by: Moiz Sonasath m-sonas...@ti.com
 Signed-off-by: Vikram Pandita vikram.pand...@ti.com
 Tested-by: Vikram Pandita vikram.pand...@ti.com

 applied this one, but I changed commit log and code comment a little
 bit. Here's updated commit:

 commit e9c281b174f188adb7950ea8f6a55ca07be69914
 Author: Anand Gadiyar gadi...@ti.com
 Date:   Tue Jul 19 22:11:58 2011 -0700

    usb: musb: Enable DMA mode1 RX for transfers without short packets

    This patch enables DMA mode1 for the RX patch when we know
    there won't be any short packets. We check that by looking
    into the short_no_ok flag, if it's true we enable mode1, otherwise
    we use mode0 to transfer the data.

    This will result in a throughput performance gain of around
    40% for USB mass-storage/mtp use cases.

    [ ba...@ti.com : updated commit log and code comments slightly ]

Ack


    Signed-off-by: Anand Gadiyar gadi...@ti.com
    Signed-off-by: Moiz Sonasath m-sonas...@ti.com
    Signed-off-by: Vikram Pandita vikram.pand...@ti.com
    Tested-by: Vikram Pandita vikram.pand...@ti.com
    Signed-off-by: Felipe Balbi ba...@ti.com

 diff --git a/drivers/usb/musb/musb_gadget.c b/drivers/usb/musb/musb_gadget.c
 index b67a062..d314f58 100644
 --- a/drivers/usb/musb/musb_gadget.c
 +++ b/drivers/usb/musb/musb_gadget.c
 @@ -634,6 +634,7 @@ static void rxstate(struct musb *musb, struct 
 musb_request *req)
        u16                     len;
        u16                     csr = musb_readw(epio, MUSB_RXCSR);
        struct musb_hw_ep       *hw_ep = musb-endpoints[epnum];
 +       u8                      use_mode_1;

        if (hw_ep-is_shared_fifo)
                musb_ep = hw_ep-ep_in;
 @@ -683,6 +684,18 @@ static void rxstate(struct musb *musb, struct 
 musb_request *req)

        if (csr  MUSB_RXCSR_RXPKTRDY) {
                len = 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
 +                */
 +
 +               if (request-short_not_ok  len == musb_ep-packet_sz)
 +                       use_mode_1 = 1;
 +               else
 +                       use_mode_1 = 0;
 +
                if (request-actual  request-length) {
  #ifdef CONFIG_USB_INVENTRA_DMA
                        if (is_buffer_mapped(req)) {
 @@ -714,37 +727,41 @@ static void rxstate(struct musb *musb, struct 
 musb_request *req)
         * then becomes usable as a runtime use mode 1 hint...
         */

 -                               csr |= MUSB_RXCSR_DMAENAB;
 -#ifdef USE_MODE1
 -                               csr |= MUSB_RXCSR_AUTOCLEAR;
 -                               /* csr |= MUSB_RXCSR_DMAMODE; */
 -
 -                               /* this special sequence (enabling and then
 -                                * disabling MUSB_RXCSR_DMAMODE) is required
 -                                * to get DMAReq to activate
 -                                */
 -                               musb_writew(epio, MUSB_RXCSR,
 -                                       csr | MUSB_RXCSR_DMAMODE);
 -#else
 -                               if (!musb_ep-hb_mult 
 -                                       musb_ep-hw_ep-rx_double_buffered)
 +                               /* Experimental: Mode1 works with mass 
 storage use cases */
 +                               if (use_mode_1) {
                                        csr |= MUSB_RXCSR_AUTOCLEAR;
 -#endif
 -                               musb_writew(epio, MUSB_RXCSR, csr);
 +                                       musb_writew(epio, MUSB_RXCSR, csr);
 +                                       csr |= MUSB_RXCSR_DMAENAB;
 +                                       musb_writew(epio, MUSB_RXCSR, csr);
 +
 +                                       /*
 +                                        * this special sequence (enabling 
 and then
 +                                        * disabling MUSB_RXCSR_DMAMODE) is 
 required
 +                                        * to get DMAReq to activate
 +                                        */
 +                                       musb_writew(epio, MUSB_RXCSR,
 +                                               csr | MUSB_RXCSR_DMAMODE);
 +                                       musb_writew(epio, MUSB_RXCSR, csr);
 +
 +                               } else {
 +                                       if (!musb_ep-hb_mult 
 +                  

Re: [PATCH v4] usb: musb: Enable DMA mode1 RX for USB-Mass-Storage

2011-07-27 Thread Felipe Balbi
Hi,

On Wed, Jul 27, 2011 at 09:20:02AM +0530, Jassi Brar wrote:
 On Tue, Jul 26, 2011 at 8:36 PM, Felipe Balbi ba...@ti.com wrote:
  Hi,
 
  On Wed, Jul 20, 2011 at 09:48:53AM -0700, Pandita, Vikram wrote:
   This *today happens to be only UMS* is my exact point here.
   Can you guarantee no other function driver will ever expect only
   full packet xfers and treat short as errors ?
  
 
  We are trying to test if short_not_ok may not be needed at all. But
  all gadgets need to be tested on MUSB for that.
  We will need wider help from MUSB maintainer/author(anand g) to
  determine if removing short_not_ok is fine on MUSB for _all_ gadgets.
 
  To be safe we only enable for UMS use case today that is definitely
  working fine.
 
  Time for Maintainer/author to pitch in !!
 
  I'm thinking on allowing this patch to go in if nobody has really strong
  arguments against it. The real fix, though, would need a bigger re-write
  of the endpoint IRQ handling and that would need more time to write and
  stabilize. Together with the huge amount of HW issues MUSB has, it's
  quite a task (been there, done that).
 
 Please note that I never objected to the _code_.
 
 I just think if the _comments_ could be made UMS agnostic... for ex if it 
 works
 for other protocols just fine(quite possible) in future the reader
 might wonder what
 is UMS specific about the code snippet.

for sure, makes sense to me. Vikram, do you want to update the comment
to remove UMS mentions ? Most likely UASP will have the same deal, btw.

-- 
balbi


signature.asc
Description: Digital signature


Re: [PATCH v4] usb: musb: Enable DMA mode1 RX for USB-Mass-Storage

2011-07-26 Thread Felipe Balbi
Hi,

On Wed, Jul 20, 2011 at 09:48:53AM -0700, Pandita, Vikram wrote:
  This *today happens to be only UMS* is my exact point here.
  Can you guarantee no other function driver will ever expect only
  full packet xfers and treat short as errors ?
 
 
 We are trying to test if short_not_ok may not be needed at all. But
 all gadgets need to be tested on MUSB for that.
 We will need wider help from MUSB maintainer/author(anand g) to
 determine if removing short_not_ok is fine on MUSB for _all_ gadgets.
 
 To be safe we only enable for UMS use case today that is definitely
 working fine.
 
 Time for Maintainer/author to pitch in !!

I'm thinking on allowing this patch to go in if nobody has really strong
arguments against it. The real fix, though, would need a bigger re-write
of the endpoint IRQ handling and that would need more time to write and
stabilize. Together with the huge amount of HW issues MUSB has, it's
quite a task (been there, done that).

-- 
balbi


signature.asc
Description: Digital signature


Re: [PATCH v4] usb: musb: Enable DMA mode1 RX for USB-Mass-Storage

2011-07-26 Thread Jassi Brar
On Tue, Jul 26, 2011 at 8:36 PM, Felipe Balbi ba...@ti.com wrote:
 Hi,

 On Wed, Jul 20, 2011 at 09:48:53AM -0700, Pandita, Vikram wrote:
  This *today happens to be only UMS* is my exact point here.
  Can you guarantee no other function driver will ever expect only
  full packet xfers and treat short as errors ?
 

 We are trying to test if short_not_ok may not be needed at all. But
 all gadgets need to be tested on MUSB for that.
 We will need wider help from MUSB maintainer/author(anand g) to
 determine if removing short_not_ok is fine on MUSB for _all_ gadgets.

 To be safe we only enable for UMS use case today that is definitely
 working fine.

 Time for Maintainer/author to pitch in !!

 I'm thinking on allowing this patch to go in if nobody has really strong
 arguments against it. The real fix, though, would need a bigger re-write
 of the endpoint IRQ handling and that would need more time to write and
 stabilize. Together with the huge amount of HW issues MUSB has, it's
 quite a task (been there, done that).

Please note that I never objected to the _code_.

I just think if the _comments_ could be made UMS agnostic... for ex if it works
for other protocols just fine(quite possible) in future the reader
might wonder what
is UMS specific about the code snippet.

Please feel free to go ahead and apply the patch.

thnx
--
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 v4] usb: musb: Enable DMA mode1 RX for USB-Mass-Storage

2011-07-20 Thread Pandita, Vikram
On Tue, Jul 19, 2011 at 10:53 PM, Jassi Brar jassisinghb...@gmail.com wrote:

 On Wed, Jul 20, 2011 at 11:15 AM, Pandita, Vikram vikram.pand...@ti.com 
 wrote:
  On Tue, Jul 19, 2011 at 10:34 PM, Jassi Brar jassisinghb...@gmail.com 
  wrote:
 
  On Wed, Jul 20, 2011 at 10:41 AM, Vikram Pandita vikram.pand...@ti.com 
  wrote:
   From: Anand Gadiyar gadi...@ti.com
  
   This patch enables the DMA mode1 RX support.
   This feature is enabled based on the short_not_ok flag passed from
   gadget drivers.
  
   This will result in a thruput performance gain of around
   40% for USB mass-storage/mtp use cases.
  
   Signed-off-by: Anand Gadiyar gadi...@ti.com
   Signed-off-by: Moiz Sonasath m-sonas...@ti.com
   Signed-off-by: Vikram Pandita vikram.pand...@ti.com
   Tested-by: Vikram Pandita vikram.pand...@ti.com
   ---
   v1 - initial push
   v2 - fixed whitespace issues as per Sergei 
   Shtylyovsshtyl...@mvista.com comments
   v3 - restor authorship to Anand Gadiyar gadi...@ti.com
   v4 - adding my signed-off as per Kevin Hilman khil...@ti.com
  
    drivers/usb/musb/musb_gadget.c |   68 
   ---
    1 files changed, 42 insertions(+), 26 deletions(-)
 
   @@ -683,6 +684,18 @@ static void rxstate(struct musb *musb, struct 
   musb_request *req)
  
          if (csr  MUSB_RXCSR_RXPKTRDY) {
                  len = musb_readw(epio, MUSB_RXCOUNT);
   +
   +               /*
   +                * Enable Mode 1 for RX transfers only for mass-storage
   +                * use-case, based on short_not_ok flag which is set only
   +                * from file_storage and f_mass_storage drivers
   +                */
   +
   +               if (request-short_not_ok  len == musb_ep-packet_sz)
   +                       use_mode_1 = 1;
   +               else
   +                       use_mode_1 = 0;
   +
  There is nothing UMS class specific in this patch...
  (request-short_not_ok  len == musb_ep-packet_sz) may not be the
  signature of, and only of, Mass Storage Functions. So maybe removing
  the UMS mention from
  comment and change-log is a good idea.
 
  Have you grepped the code in drivers/usb/gadget/*.*
  only UMS sets this flag today and hence the use of this flag.
 
  As i understand, on UMS, CSW/data/CBW  - the data part size is a known
  size and to be safe that mode=1 dma is not stuck up,
  the mode is enabled only for the gadget driver that sets short_not_ok
  flag - and that today happens to be only UMS.

 This *today happens to be only UMS* is my exact point here.
 Can you guarantee no other function driver will ever expect only
 full packet xfers and treat short as errors ?


We are trying to test if short_not_ok may not be needed at all. But
all gadgets need to be tested on MUSB for that.
We will need wider help from MUSB maintainer/author(anand g) to
determine if removing short_not_ok is fine on MUSB for _all_ gadgets.

To be safe we only enable for UMS use case today that is definitely
working fine.

Time for Maintainer/author to pitch in !!


  You might want to add is-ep-type-bulk-out check to the condition
  though, so that it doesn't affect
  cases that you didn't verify.
 
  TX path (IN host), already uses the mode=1 DMA and hence the comment
  is not valid.
  This patch just also enables mode=1 on RX path.
 Well, then no need for the ep-type check.

where did u see ep-type check? i can see only packet size check. Did i miss?
--
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 v4] usb: musb: Enable DMA mode1 RX for USB-Mass-Storage

2011-07-19 Thread Vikram Pandita
From: Anand Gadiyar gadi...@ti.com

This patch enables the DMA mode1 RX support.
This feature is enabled based on the short_not_ok flag passed from
gadget drivers.

This will result in a thruput performance gain of around
40% for USB mass-storage/mtp use cases.

Signed-off-by: Anand Gadiyar gadi...@ti.com
Signed-off-by: Moiz Sonasath m-sonas...@ti.com
Signed-off-by: Vikram Pandita vikram.pand...@ti.com
Tested-by: Vikram Pandita vikram.pand...@ti.com
---
v1 - initial push
v2 - fixed whitespace issues as per Sergei Shtylyovsshtyl...@mvista.com 
comments
v3 - restor authorship to Anand Gadiyar gadi...@ti.com
v4 - adding my signed-off as per Kevin Hilman khil...@ti.com

 drivers/usb/musb/musb_gadget.c |   68 ---
 1 files changed, 42 insertions(+), 26 deletions(-)

diff --git a/drivers/usb/musb/musb_gadget.c b/drivers/usb/musb/musb_gadget.c
index 6aeb363..4a1432e 100644
--- a/drivers/usb/musb/musb_gadget.c
+++ b/drivers/usb/musb/musb_gadget.c
@@ -634,6 +634,7 @@ static void rxstate(struct musb *musb, struct musb_request 
*req)
u16 len;
u16 csr = musb_readw(epio, MUSB_RXCSR);
struct musb_hw_ep   *hw_ep = musb-endpoints[epnum];
+   u8  use_mode_1;
 
if (hw_ep-is_shared_fifo)
musb_ep = hw_ep-ep_in;
@@ -683,6 +684,18 @@ static void rxstate(struct musb *musb, struct musb_request 
*req)
 
if (csr  MUSB_RXCSR_RXPKTRDY) {
len = musb_readw(epio, MUSB_RXCOUNT);
+
+   /*
+* Enable Mode 1 for RX transfers only for mass-storage
+* use-case, based on short_not_ok flag which is set only
+* from file_storage and f_mass_storage drivers
+*/
+
+   if (request-short_not_ok  len == musb_ep-packet_sz)
+   use_mode_1 = 1;
+   else
+   use_mode_1 = 0;
+
if (request-actual  request-length) {
 #ifdef CONFIG_USB_INVENTRA_DMA
if (is_buffer_mapped(req)) {
@@ -714,37 +727,40 @@ static void rxstate(struct musb *musb, struct 
musb_request *req)
 * then becomes usable as a runtime use mode 1 hint...
 */
 
-   csr |= MUSB_RXCSR_DMAENAB;
-#ifdef USE_MODE1
-   csr |= MUSB_RXCSR_AUTOCLEAR;
-   /* csr |= MUSB_RXCSR_DMAMODE; */
-
-   /* this special sequence (enabling and then
-* disabling MUSB_RXCSR_DMAMODE) is required
-* to get DMAReq to activate
-*/
-   musb_writew(epio, MUSB_RXCSR,
-   csr | MUSB_RXCSR_DMAMODE);
-#else
-   if (!musb_ep-hb_mult 
-   musb_ep-hw_ep-rx_double_buffered)
+   /* Experimental: Mode1 works with mass storage 
use cases */
+   if (use_mode_1) {
csr |= MUSB_RXCSR_AUTOCLEAR;
-#endif
-   musb_writew(epio, MUSB_RXCSR, csr);
+   musb_writew(epio, MUSB_RXCSR, csr);
+   csr |= MUSB_RXCSR_DMAENAB;
+   musb_writew(epio, MUSB_RXCSR, csr);
+
+   /* this special sequence (enabling and 
then
+   * disabling MUSB_RXCSR_DMAMODE) is 
required
+   * to get DMAReq to activate
+   */
+   musb_writew(epio, MUSB_RXCSR,
+   csr | MUSB_RXCSR_DMAMODE);
+   musb_writew(epio, MUSB_RXCSR, csr);
+
+   } else {
+   if (!musb_ep-hb_mult 
+   
musb_ep-hw_ep-rx_double_buffered)
+   csr |= MUSB_RXCSR_AUTOCLEAR;
+   csr |= MUSB_RXCSR_DMAENAB;
+   musb_writew(epio, MUSB_RXCSR, csr);
+   }
 
if (request-actual  request-length) {
int transfer_size = 0;
-#ifdef USE_MODE1
-   transfer_size = min(request-length - 
request-actual,
-   channel-max_len);
-#else
-   transfer_size = min(request-length - 
request-actual,
-   (unsigned)len);
-#endif
-   if 

Re: [PATCH v4] usb: musb: Enable DMA mode1 RX for USB-Mass-Storage

2011-07-19 Thread Jassi Brar
On Wed, Jul 20, 2011 at 10:41 AM, Vikram Pandita vikram.pand...@ti.com wrote:
 From: Anand Gadiyar gadi...@ti.com

 This patch enables the DMA mode1 RX support.
 This feature is enabled based on the short_not_ok flag passed from
 gadget drivers.

 This will result in a thruput performance gain of around
 40% for USB mass-storage/mtp use cases.

 Signed-off-by: Anand Gadiyar gadi...@ti.com
 Signed-off-by: Moiz Sonasath m-sonas...@ti.com
 Signed-off-by: Vikram Pandita vikram.pand...@ti.com
 Tested-by: Vikram Pandita vikram.pand...@ti.com
 ---
 v1 - initial push
 v2 - fixed whitespace issues as per Sergei Shtylyovsshtyl...@mvista.com 
 comments
 v3 - restor authorship to Anand Gadiyar gadi...@ti.com
 v4 - adding my signed-off as per Kevin Hilman khil...@ti.com

  drivers/usb/musb/musb_gadget.c |   68 ---
  1 files changed, 42 insertions(+), 26 deletions(-)

 @@ -683,6 +684,18 @@ static void rxstate(struct musb *musb, struct 
 musb_request *req)

        if (csr  MUSB_RXCSR_RXPKTRDY) {
                len = musb_readw(epio, MUSB_RXCOUNT);
 +
 +               /*
 +                * Enable Mode 1 for RX transfers only for mass-storage
 +                * use-case, based on short_not_ok flag which is set only
 +                * from file_storage and f_mass_storage drivers
 +                */
 +
 +               if (request-short_not_ok  len == musb_ep-packet_sz)
 +                       use_mode_1 = 1;
 +               else
 +                       use_mode_1 = 0;
 +
There is nothing UMS class specific in this patch...
(request-short_not_ok  len == musb_ep-packet_sz) may not be the
signature of, and only of, Mass Storage Functions. So maybe removing
the UMS mention from
comment and change-log is a good idea.
You might want to add is-ep-type-bulk-out check to the condition
though, so that it doesn't affect
cases that you didn't verify.
--
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 v4] usb: musb: Enable DMA mode1 RX for USB-Mass-Storage

2011-07-19 Thread Pandita, Vikram
On Tue, Jul 19, 2011 at 10:34 PM, Jassi Brar jassisinghb...@gmail.com wrote:

 On Wed, Jul 20, 2011 at 10:41 AM, Vikram Pandita vikram.pand...@ti.com 
 wrote:
  From: Anand Gadiyar gadi...@ti.com
 
  This patch enables the DMA mode1 RX support.
  This feature is enabled based on the short_not_ok flag passed from
  gadget drivers.
 
  This will result in a thruput performance gain of around
  40% for USB mass-storage/mtp use cases.
 
  Signed-off-by: Anand Gadiyar gadi...@ti.com
  Signed-off-by: Moiz Sonasath m-sonas...@ti.com
  Signed-off-by: Vikram Pandita vikram.pand...@ti.com
  Tested-by: Vikram Pandita vikram.pand...@ti.com
  ---
  v1 - initial push
  v2 - fixed whitespace issues as per Sergei Shtylyovsshtyl...@mvista.com 
  comments
  v3 - restor authorship to Anand Gadiyar gadi...@ti.com
  v4 - adding my signed-off as per Kevin Hilman khil...@ti.com
 
   drivers/usb/musb/musb_gadget.c |   68 
  ---
   1 files changed, 42 insertions(+), 26 deletions(-)

  @@ -683,6 +684,18 @@ static void rxstate(struct musb *musb, struct 
  musb_request *req)
 
         if (csr  MUSB_RXCSR_RXPKTRDY) {
                 len = musb_readw(epio, MUSB_RXCOUNT);
  +
  +               /*
  +                * Enable Mode 1 for RX transfers only for mass-storage
  +                * use-case, based on short_not_ok flag which is set only
  +                * from file_storage and f_mass_storage drivers
  +                */
  +
  +               if (request-short_not_ok  len == musb_ep-packet_sz)
  +                       use_mode_1 = 1;
  +               else
  +                       use_mode_1 = 0;
  +
 There is nothing UMS class specific in this patch...
 (request-short_not_ok  len == musb_ep-packet_sz) may not be the
 signature of, and only of, Mass Storage Functions. So maybe removing
 the UMS mention from
 comment and change-log is a good idea.

Have you grepped the code in drivers/usb/gadget/*.*
only UMS sets this flag today and hence the use of this flag.

As i understand, on UMS, CSW/data/CBW  - the data part size is a known
size and to be safe that mode=1 dma is not stuck up,
the mode is enabled only for the gadget driver that sets short_not_ok
flag - and that today happens to be only UMS.

 You might want to add is-ep-type-bulk-out check to the condition
 though, so that it doesn't affect
 cases that you didn't verify.

TX path (IN host), already uses the mode=1 DMA and hence the comment
is not valid.
This patch just also enables mode=1 on RX path.
--
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 v4] usb: musb: Enable DMA mode1 RX for USB-Mass-Storage

2011-07-19 Thread Jassi Brar
On Wed, Jul 20, 2011 at 11:15 AM, Pandita, Vikram vikram.pand...@ti.com wrote:
 On Tue, Jul 19, 2011 at 10:34 PM, Jassi Brar jassisinghb...@gmail.com wrote:

 On Wed, Jul 20, 2011 at 10:41 AM, Vikram Pandita vikram.pand...@ti.com 
 wrote:
  From: Anand Gadiyar gadi...@ti.com
 
  This patch enables the DMA mode1 RX support.
  This feature is enabled based on the short_not_ok flag passed from
  gadget drivers.
 
  This will result in a thruput performance gain of around
  40% for USB mass-storage/mtp use cases.
 
  Signed-off-by: Anand Gadiyar gadi...@ti.com
  Signed-off-by: Moiz Sonasath m-sonas...@ti.com
  Signed-off-by: Vikram Pandita vikram.pand...@ti.com
  Tested-by: Vikram Pandita vikram.pand...@ti.com
  ---
  v1 - initial push
  v2 - fixed whitespace issues as per Sergei Shtylyovsshtyl...@mvista.com 
  comments
  v3 - restor authorship to Anand Gadiyar gadi...@ti.com
  v4 - adding my signed-off as per Kevin Hilman khil...@ti.com
 
   drivers/usb/musb/musb_gadget.c |   68 
  ---
   1 files changed, 42 insertions(+), 26 deletions(-)

  @@ -683,6 +684,18 @@ static void rxstate(struct musb *musb, struct 
  musb_request *req)
 
         if (csr  MUSB_RXCSR_RXPKTRDY) {
                 len = musb_readw(epio, MUSB_RXCOUNT);
  +
  +               /*
  +                * Enable Mode 1 for RX transfers only for mass-storage
  +                * use-case, based on short_not_ok flag which is set only
  +                * from file_storage and f_mass_storage drivers
  +                */
  +
  +               if (request-short_not_ok  len == musb_ep-packet_sz)
  +                       use_mode_1 = 1;
  +               else
  +                       use_mode_1 = 0;
  +
 There is nothing UMS class specific in this patch...
 (request-short_not_ok  len == musb_ep-packet_sz) may not be the
 signature of, and only of, Mass Storage Functions. So maybe removing
 the UMS mention from
 comment and change-log is a good idea.

 Have you grepped the code in drivers/usb/gadget/*.*
 only UMS sets this flag today and hence the use of this flag.

 As i understand, on UMS, CSW/data/CBW  - the data part size is a known
 size and to be safe that mode=1 dma is not stuck up,
 the mode is enabled only for the gadget driver that sets short_not_ok
 flag - and that today happens to be only UMS.

This *today happens to be only UMS* is my exact point here.
Can you guarantee no other function driver will ever expect only
full packet xfers and treat short as errors ?


 You might want to add is-ep-type-bulk-out check to the condition
 though, so that it doesn't affect
 cases that you didn't verify.

 TX path (IN host), already uses the mode=1 DMA and hence the comment
 is not valid.
 This patch just also enables mode=1 on RX path.
Well, then no need for the ep-type check.
--
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