On Wed, Jul 20, 2011 at 11:15 AM, Pandita, Vikram <[email protected]> wrote:
> On Tue, Jul 19, 2011 at 10:34 PM, Jassi Brar <[email protected]> wrote:
>>
>> On Wed, Jul 20, 2011 at 10:41 AM, Vikram Pandita <[email protected]> 
>> wrote:
>> > From: Anand Gadiyar <[email protected]>
>> >
>> > 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 <[email protected]>
>> > Signed-off-by: Moiz Sonasath <[email protected]>
>> > Signed-off-by: Vikram Pandita <[email protected]>
>> > Tested-by: Vikram Pandita <[email protected]>
>> > ---
>> > v1 - initial push
>> > v2 - fixed whitespace issues as per Sergei Shtylyov<[email protected]> 
>> > comments
>> > v3 - restor authorship to Anand Gadiyar <[email protected]>
>> > v4 - adding my signed-off as per Kevin Hilman <[email protected]>
>> >
>> >  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 [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to