USB: musb: bugfixes for multi-packet TXDMA support

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.

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.
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; it can get stale
"packet ready" interrupts, and report transfer completion too
early using slightly different code paths, 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 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 and defers URB completion processing until TXPKTRDY,
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).

[ dbrownell@users.sourceforge.net: don't introduce more
CamElCase; use more concise explanations ]

Signed-off-by: Sergei Shtylyov <sshtylyov@ru.mvista.com>
Cc: Felipe Balbi <felipe.balbi@nokia.com>
Signed-off-by: David Brownell <dbrownell@users.sourceforge.net>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>

diff --git a/drivers/usb/musb/cppi_dma.c b/drivers/usb/musb/cppi_dma.c
index 569ef0f..30e2489 100644 (file)
--- a/drivers/usb/musb/cppi_dma.c
+++ b/drivers/usb/musb/cppi_dma.c
@@ -1228,27 +1228,7 @@ void cppi_completion(struct musb *musb, u32 rx, u32 tx)
 
                                hw_ep = tx_ch->hw_ep;
 
-                               /* Peripheral role never repurposes the
-                                * endpoint, so immediate completion is
-                                * safe.  Host role waits for the fifo
-                                * to empty (TXPKTRDY irq) before going
-                                * to the next queued bulk transfer.
-                                */
-                               if (is_host_active(cppi->musb)) {
-#if 0
-                                       /* WORKAROUND because we may
-                                        * not always get TXKPTRDY ...
-                                        */
-                                       int     csr;
-
-                                       csr = musb_readw(hw_ep->regs,
-                                               MUSB_TXCSR);
-                                       if (csr & MUSB_TXCSR_TXPKTRDY)
-#endif
-                                               completed = false;
-                               }
-                               if (completed)
-                                       musb_dma_completion(musb, index + 1, 1);
+                               musb_dma_completion(musb, index + 1, 1);
 
                        } else {
                                /* Bigger transfer than we could fit in

diff --git a/drivers/usb/musb/musb_host.c b/drivers/usb/musb/musb_host.c
index 521fd83..518abfc 100644 (file)
--- a/drivers/usb/musb/musb_host.c
+++ b/drivers/usb/musb/musb_host.c
@@ -4,6 +4,7 @@
  * Copyright 2005 Mentor Graphics Corporation
  * Copyright (C) 2005-2006 by Texas Instruments
  * Copyright (C) 2006-2007 Nokia Corporation
+ * Copyright (C) 2008-2009 MontaVista Software, Inc. <source@mvista.com>
  *
  * This program is free software; you can redistribute it and/or
  * modify it under the terms of the GNU General Public License
@@ -168,13 +169,15 @@ static inline void musb_h_tx_start(struct musb_hw_ep *ep)
 
 }
 
-static inline void cppi_host_txdma_start(struct musb_hw_ep *ep)
+static inline void musb_h_tx_dma_start(struct musb_hw_ep *ep)
 {
        u16     txcsr;
 
        /* NOTE: no locks here; caller should lock and select EP */
        txcsr = musb_readw(ep->regs, MUSB_TXCSR);
        txcsr |= MUSB_TXCSR_DMAENAB | MUSB_TXCSR_H_WZC_BITS;
+       if (is_cppi_enabled())
+               txcsr |= MUSB_TXCSR_DMAMODE;
        musb_writew(ep->regs, MUSB_TXCSR, txcsr);
 }
 
@@ -279,7 +282,7 @@ start:
                if (!hw_ep->tx_channel)
                        musb_h_tx_start(hw_ep);
                else if (is_cppi_enabled() || tusb_dma_omap())
-                       cppi_host_txdma_start(hw_ep);
+                       musb_h_tx_dma_start(hw_ep);
        }
 }
 
@@ -1250,6 +1253,67 @@ void musb_host_tx(struct musb *musb, u8 epnum)
 
        }
 
+       if (is_dma_capable() && dma && !status) {
+               /*
+                * DMA has completed.  But if we're using DMA mode 1 (multi
+                * packet DMA), we need a terminal TXPKTRDY interrupt before
+                * we can consider this transfer completed, lest we trash
+                * its last packet when writing the next URB's data.  So we
+                * switch back to mode 0 to get that interrupt; we'll come
+                * back here once it happens.
+                */
+               if (tx_csr & MUSB_TXCSR_DMAMODE) {
+                       /*
+                        * We shouldn't clear DMAMODE with DMAENAB set; so
+                        * clear them in a safe order.  That should be OK
+                        * once TXPKTRDY has been set (and I've never seen
+                        * it being 0 at this moment -- DMA interrupt latency
+                        * is significant) but if it hasn't been then we have
+                        * no choice but to stop being polite and ignore the
+                        * programmer's guide... :-)
+                        *
+                        * Note that we must write TXCSR with TXPKTRDY cleared
+                        * in order not to re-trigger the packet send (this bit
+                        * can't be cleared by CPU), and there's another caveat:
+                        * TXPKTRDY may be set shortly and then cleared in the
+                        * double-buffered FIFO mode, so we do an extra TXCSR
+                        * read for debouncing...
+                        */
+                       tx_csr &= musb_readw(epio, MUSB_TXCSR);
+                       if (tx_csr & MUSB_TXCSR_TXPKTRDY) {
+                               tx_csr &= ~(MUSB_TXCSR_DMAENAB |
+                                           MUSB_TXCSR_TXPKTRDY);
+                               musb_writew(epio, MUSB_TXCSR,
+                                           tx_csr | MUSB_TXCSR_H_WZC_BITS);
+                       }
+                       tx_csr &= ~(MUSB_TXCSR_DMAMODE |
+                                   MUSB_TXCSR_TXPKTRDY);
+                       musb_writew(epio, MUSB_TXCSR,
+                                   tx_csr | MUSB_TXCSR_H_WZC_BITS);
+
+                       /*
+                        * There is no guarantee that we'll get an interrupt
+                        * after clearing DMAMODE as we might have done this
+                        * too late (after TXPKTRDY was cleared by controller).
+                        * Re-read TXCSR as we have spoiled its previous value.
+                        */
+                       tx_csr = musb_readw(epio, MUSB_TXCSR);
+               }
+
+               /*
+                * We may get here from a DMA completion or TXPKTRDY interrupt.
+                * In any case, we must check the FIFO status here and bail out
+                * only if the FIFO still has data -- that should prevent the
+                * "missed" TXPKTRDY interrupts and deal with double-buffered
+                * FIFO mode too...
+                */
+               if (tx_csr & (MUSB_TXCSR_FIFONOTEMPTY | MUSB_TXCSR_TXPKTRDY)) {
+                       DBG(2, "DMA complete but packet still in FIFO, "
+                           "CSR %04x\n", tx_csr);
+                       return;
+               }
+       }
+
        /* REVISIT this looks wrong... */
        if (!status || dma || usb_pipeisoc(pipe)) {
                if (dma)

diff --git a/drivers/usb/musb/musbhsdma.c b/drivers/usb/musb/musbhsdma.c
index 8662e9e..de0e242 100644 (file)
--- a/drivers/usb/musb/musbhsdma.c
+++ b/drivers/usb/musb/musbhsdma.c
@@ -304,12 +304,9 @@ static irqreturn_t dma_controller_irq(int irq, void *private_data)
                                                        musb_channel->epnum,
                                                        MUSB_TXCSR),
                                                MUSB_TXCSR_TXPKTRDY);
-                               } else {
-                                       musb_dma_completion(
-                                               musb,
-                                               musb_channel->epnum,
-                                               musb_channel->transmit);
                                }
+                               musb_dma_completion(musb, musb_channel->epnum,
+                                                   musb_channel->transmit);
                        }
                }
        }