Re: [PATCH v2] usb: fix mass storage gadgets to work with Synopsys UDC

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sun, Jun 3, 2012 at 2:30 AM, Rajaram R <rajaram.officemail@xxxxxxxxx> wrote:
> On Thu, May 31, 2012 at 3:24 PM, Felipe Balbi <balbi@xxxxxx> wrote:
>> On Thu, May 31, 2012 at 03:09:26PM +0530, Rajaram R wrote:
>>> On Thu, May 31, 2012 at 1:41 PM, Felipe Balbi <balbi@xxxxxx> wrote:
>>> > Hi,
>>> >
>>> > (please don't top-post, ever)
>>>
>>> hmm i missed that... thanks for pointing ;-)
>>>
>>> >
>>> > On Thu, May 31, 2012 at 12:06:21PM +0530, Rajaram R wrote:
>>> >>  Hi Felipe/Paul
>>> >>
>>> >>  Any reason for removing short_not_ok ? I believe it still holds true
>>> >>  even after this patch. I will push a patch for the same if there is no
>>> >>  objection.
>>> >
>>> > It doesn't hold true at all. We _want_ a short packet in order to end
>>> > the transfer. Let me illustrate:
>>> >
>>> > We want to receive 31-byte CBw, but our UDC can't handle short packets
>>> > on OUT direction, so we start a transfer for wMaxPacketSize. If we said
>>> > short_not_ok, the driver wouldn't like when we received 31-byte and
>>> > ended the transfer.
>>>
>>> IMO any tweak specific to a controller should be in the platform
>>> files.. In case of musb also the controller has some limitation and
>>> short packets are handled appropriately within the musb driver.
>>
>> I have explained why it wouldn't be a good choice to do that for dwc3.
>> USB3 transfers are quite high bandwidth (5Gbps), if we needed to use a
>> bounce buffer for every non-aligned transfer, we would have to do quite
>> a lot of memcpy() and that would degrade performance quite a lot and
>> cause quite some load on the CPU.
>
> In such a case shouldnt we do this change only for SuperSpeed
> scenarios ?

Does this solution sounds OK ? If yes, lets have this change for
Superspeed ? Please let me know

>As Alan tracked earlier when this patch was pushed
>
>       http://marc.info/?l=linux-usb&m=130096570404235&w=2
>       http://marc.info/?l=linux-usb&m=130164710522223&w=2
>       http://marc.info/?l=linux-usb&m=130279510629643&w=2
>
> Our setup is getting affected by this change !
>>
>> That was the reason why we chose to change g_mass_storage instead :-s
>>
>>> In case of DWC, if there is a limitation you can always ignore this
>>> field. This field is being used by musb driver for managing DMA
>>> transfers. Removing it will affect musb drivers. Please clarify
>>
>> yeah, musb is quite buggy :-p patches fixing that are very welcome
>> actually.
>
> Since you also maintain musb driver could you please clarify none of
> the musb driver is affected by this change??
>
> Is no other musb user finding this change affecting their setup ??
>
>>
>> To be fair, the whole EP handling in MUSB is quite messed up and should
>> change. Together with DMA setup, endpoint initialization, FIFO
>> allocation (and re-allocation if necessary after SetInterface) and so
>> on...
>>
>> Just look at rxstate() and txstate(), those two functions are too big
>> and they know too much about the underlying DMA engine. I'm sure many
>> many bugs are hidden on those two functions. If you would like to help
>> out, just splitting txstate() and rxstate() into dma and non-dma
>> functions would already help you see issues. After that, go back to the
>> MUSB programming guide and unify DMA programming (it can be done) so
>
> If that was that straight forward it would been there by now :-)
>
>> that all those ifdefs are dropped and checks for which DMA engine are
>> dropped.
>>
>> --
>> balbi
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux