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