David Brownell wrote:
We really want to use DMA mode 1 for all multi-packet transfers; that's one IRQ on DMA completion, instead of one per packet.
We're still using it for the single packet transfers with CPPI too (although there's no win from that, and this probably should be changed).
There is an important issue with such transfers, especially on the host side: when such transfers end with a full-size packet, we must defer musb_dma_completion() calls until the FIFO empties.
Sigh, again I'm seeing that you've spoiled my patch description... and that's after I have once requested you to stop doing such things! :-/
The size of the last packet totally doesn't matter here! We must react on the interrupt on TxPktRdy being cleared, not on the DMA completion interrupt, whatever the size of the last packet is. That's what the patch was about!
Else we report URB completions too soon, and may clobber data in the FIFO fifo when writing the next packet (losing data).
The Inventra DMA support uses DMA mode 1, but it ignores that issue. The CPPI DMA support uses mode 0, but doesn't handle its TXPKTRDY interrupts quite right either;
This text is just nonsense -- it's not the task of CPPI DMA support.
it can get stale "packet ready" interrupts, and report transfer completion too early using slightly different code paths,
The common musb_host_tx() path that ignores TxPktRdy being set is in no way different. Except that driver doesn't ever call musb_dma_completion() on Tx path -- which doesn't help it to achieve what it tries to...
also losing data.
So I'm solving it in a generic way -- by adding a sort of the "interrupt filter" into musb_host_tx(), catching these cases where a DMA completion IRQ doesn't suffice
It just *never* suffices. We must wait for TxPktRdy interrupt. Hopefully, they can concide.
and removing some needlessly controller-specific logic. When a TXDMA interrupt happens and DMA request mode 1 is active, that filter resets to mode 0
It tries to reset gracefully, first clearing the DMAReqEnab bit -- which should've been enough actually if I wasn't using mode 1 as a criterion. My description was pretty clear about that but you decided to just drop this.
and defers URB completion processing until TXPKTRDY,
TXPKTRDY what -- set, cleared?
unless the FIFO is already empty. Related filtering logic in Inventra and CPPI code gets removed.
Since it should be competely safe now to use the DMA request mode 1 for host side transfers with the CPPI DMA controller, set it in musb_h_tx_dma_start() ... now renamed (and shared).
It was always shared, hence the rename. Although now I'm not seeing why TUSB needs to use it as it happily programs TXCSR inside the DMA driver beforehand -- the usual MUSB driver's incosistency...
[ dbrownell@xxxxxxxxxxxxxxxxxxxxx: don't introduce more CamElCase; use more concise explanations ]
Where have you found the CamelCase, in the comments? I was just naming the bits the same as the programming guide does. If that somehow doesn't match your taste, it's basically your problem... :-/
In short, I have found your rewrite of my description not clearing up things but making them more unclear. Looks like I have not choice but again request you to remove your description changes and return mine. Feel free to ignore me, like you have done in the past...
WBR, Sergei -- 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
