[PATCH 1/2] musb: sanitize clearing TXCSR DMA bits (take 2)

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

 



The MUSB code either expects TXCSR.DMAReqEnab bit cleared in several places
and then proceeds with clearing the TXCSR.DMAMode bit (but does not bother
with actually clearing DMAReqEnab beforehand) or just clears both bits at
once -- while is the programming guide explicitly forbids to clear DMAMode
before or in the same cycle as DMAReEnab.  Fix this and while at it:

- in musb_gadget::txstate(), stop clearing the AutoSet and DMAReqMode bits for
  the CPPI case since they never get set anyway (the former bit is reserved on
  DaVinci) but do clear the DMAReqEnab bit on the DMA error path;

- in musb_host::musb_ep_program(), remove the duplicate DMA controller specific
  code code clearing the TXCSR previous state, add the code to clear TXCSR DMA
  bits on the Inventra DMA error path, to replace such code (executed late) on
  the PIO path;

- in musbhsdma::dma_channel_abort()/dma_controller_irq(), add/use the 'offset'
  variable to avoid MUSB_EP_OFFSET() invocations on every RXCSR/TXCSR access.

Signed-off-by: Sergei Shtylyov <sshtylyov@xxxxxxxxxxxxx>

---
The patch is against the recent Linus' kernel, atop of the previosly sent patch
series. Reworked the musb_gadget::nuke() change as suggested by Felipe Balbi...

 drivers/usb/musb/musb_gadget.c |   31 ++++++++----
 drivers/usb/musb/musb_host.c   |  100 ++++++++++++++++-------------------------
 drivers/usb/musb/musbhsdma.c   |   58 ++++++++++++++---------
 3 files changed, 98 insertions(+), 91 deletions(-)

Index: linux-2.6/drivers/usb/musb/musb_gadget.c
===================================================================
--- linux-2.6.orig/drivers/usb/musb/musb_gadget.c
+++ linux-2.6/drivers/usb/musb/musb_gadget.c
@@ -165,9 +165,15 @@ static void nuke(struct musb_ep *ep, con
 	if (is_dma_capable() && ep->dma) {
 		struct dma_controller	*c = ep->musb->dma_controller;
 		int value;
+
 		if (ep->is_in) {
+			/*
+			 * The programming guide says that we must not clear
+			 * the DMAReqMode bit before DMAReqEnab, so we only
+			 * clear it in the second write...
+			 */
 			musb_writew(epio, MUSB_TXCSR,
-					0 | MUSB_TXCSR_FLUSHFIFO);
+				    MUSB_TXCSR_DMAMODE | MUSB_TXCSR_FLUSHFIFO);
 			musb_writew(epio, MUSB_TXCSR,
 					0 | MUSB_TXCSR_FLUSHFIFO);
 		} else {
@@ -230,7 +236,7 @@ static inline int max_ep_writesize(struc
 		  |	IN token(s) are recd from Host.
 		  |		-> DMA interrupt on completion
 		  |		   calls TxAvail.
-		  |		      -> stop DMA, ~DmaEenab,
+		  |		      -> stop DMA, ~DMAEnab,
 		  |		      -> set TxPktRdy for last short pkt or zlp
 		  |		      -> Complete Request
 		  |		      -> Continue next request (call txstate)
@@ -315,9 +321,17 @@ static void txstate(struct musb *musb, s
 					request->dma, request_size);
 			if (use_dma) {
 				if (musb_ep->dma->desired_mode == 0) {
-					/* ASSERT: DMAENAB is clear */
+					/*
+					 * We must not clear the DMAReqMode bit
+					 * before the DMAReqEnab bit -- and the
+					 * latter doesn't always get cleared
+					 * before we get here...
+					 */
 					csr &= ~(MUSB_TXCSR_AUTOSET |
-							MUSB_TXCSR_DMAMODE);
+						 MUSB_TXCSR_DMAENAB);
+					musb_writew(epio, MUSB_TXCSR, csr |
+						    MUSB_TXCSR_P_WZC_BITS);
+					csr &= ~MUSB_TXCSR_DMAMODE;
 					csr |= (MUSB_TXCSR_DMAENAB |
 							MUSB_TXCSR_MODE);
 					/* against programming guide */
@@ -334,10 +348,7 @@ static void txstate(struct musb *musb, s
 
 #elif defined(CONFIG_USB_TI_CPPI_DMA)
 		/* program endpoint CSR first, then setup DMA */
-		csr &= ~(MUSB_TXCSR_AUTOSET
-				| MUSB_TXCSR_DMAMODE
-				| MUSB_TXCSR_P_UNDERRUN
-				| MUSB_TXCSR_TXPKTRDY);
+		csr &= ~(MUSB_TXCSR_P_UNDERRUN | MUSB_TXCSR_TXPKTRDY);
 		csr |= MUSB_TXCSR_MODE | MUSB_TXCSR_DMAENAB;
 		musb_writew(epio, MUSB_TXCSR,
 			(MUSB_TXCSR_P_WZC_BITS & ~MUSB_TXCSR_P_UNDERRUN)
@@ -364,8 +375,8 @@ static void txstate(struct musb *musb, s
 		if (!use_dma) {
 			c->channel_release(musb_ep->dma);
 			musb_ep->dma = NULL;
-			/* ASSERT: DMAENAB clear */
-			csr &= ~(MUSB_TXCSR_DMAMODE | MUSB_TXCSR_MODE);
+			csr &= ~MUSB_TXCSR_DMAENAB;
+			musb_writew(epio, MUSB_TXCSR, csr);
 			/* invariant: prequest->buf is non-null */
 		}
 #elif defined(CONFIG_USB_TUSB_OMAP_DMA)
Index: linux-2.6/drivers/usb/musb/musb_host.c
===================================================================
--- linux-2.6.orig/drivers/usb/musb/musb_host.c
+++ linux-2.6/drivers/usb/musb/musb_host.c
@@ -575,10 +575,17 @@ musb_rx_reinit(struct musb *musb, struct
 		csr = musb_readw(ep->regs, MUSB_TXCSR);
 		if (csr & MUSB_TXCSR_MODE) {
 			musb_h_tx_flush_fifo(ep);
+			csr = musb_readw(ep->regs, MUSB_TXCSR);
 			musb_writew(ep->regs, MUSB_TXCSR,
-					MUSB_TXCSR_FRCDATATOG);
+				    csr | MUSB_TXCSR_FRCDATATOG);
 		}
-		/* clear mode (and everything else) to enable Rx */
+
+		/*
+		 * Clear the Mode bit (and everything else) to enable Rx.
+		 * NOTE: we mustn't clear the DMAReqMode bit before DMAReqEnab.
+		 */
+		if (csr & MUSB_TXCSR_DMAMODE)
+			musb_writew(ep->regs, MUSB_TXCSR, MUSB_TXCSR_DMAMODE);
 		musb_writew(ep->regs, MUSB_TXCSR, 0);
 
 	/* scrub all previous state, clearing toggle */
@@ -675,29 +682,30 @@ static void musb_ep_program(struct musb 
 
 		/* general endpoint setup */
 		if (epnum) {
-			/* ASSERT:  TXCSR_DMAENAB was already cleared */
-
 			/* flush all old state, set default */
 			musb_h_tx_flush_fifo(hw_ep);
-			csr &= ~(MUSB_TXCSR_H_NAKTIMEOUT
-					| MUSB_TXCSR_DMAMODE
-					| MUSB_TXCSR_FRCDATATOG
-					| MUSB_TXCSR_H_RXSTALL
-					| MUSB_TXCSR_H_ERROR
-					| MUSB_TXCSR_TXPKTRDY
-					);
+
+			/*
+			 * We must not clear the DMAReqMode bit before or in
+			 * the same cycle with the DMAReqEnab bit, so we clear
+			 * the latter first...
+			 */
+			csr &= ~(MUSB_TXCSR_AUTOSET | MUSB_TXCSR_DMAENAB |
+				 MUSB_TXCSR_FRCDATATOG |
+				 MUSB_TXCSR_H_NAKTIMEOUT |
+				 MUSB_TXCSR_H_RXSTALL | MUSB_TXCSR_H_ERROR |
+				 MUSB_TXCSR_TXPKTRDY);
 			csr |= MUSB_TXCSR_MODE;
 
-			if (usb_gettoggle(urb->dev,
-					qh->epnum, 1))
+			if (usb_gettoggle(urb->dev, qh->epnum, 1))
 				csr |= MUSB_TXCSR_H_WR_DATATOGGLE
 					| MUSB_TXCSR_H_DATATOGGLE;
 			else
 				csr |= MUSB_TXCSR_CLRDATATOG;
 
-			/* twice in case of double packet buffering */
 			musb_writew(epio, MUSB_TXCSR, csr);
 			/* REVISIT may need to clear FLUSHFIFO ... */
+			csr &= ~MUSB_TXCSR_DMAMODE;
 			musb_writew(epio, MUSB_TXCSR, csr);
 			csr = musb_readw(epio, MUSB_TXCSR);
 		} else {
@@ -744,34 +752,19 @@ static void musb_ep_program(struct musb 
 
 #ifdef CONFIG_USB_INVENTRA_DMA
 		if (dma_channel) {
-
-			/* clear previous state */
-			csr = musb_readw(epio, MUSB_TXCSR);
-			csr &= ~(MUSB_TXCSR_AUTOSET
-				| MUSB_TXCSR_DMAMODE
-				| MUSB_TXCSR_DMAENAB);
-			csr |= MUSB_TXCSR_MODE;
-			musb_writew(epio, MUSB_TXCSR,
-				csr | MUSB_TXCSR_MODE);
-
 			qh->segsize = min(len, dma_channel->max_len);
-
 			if (qh->segsize <= packet_sz)
 				dma_channel->desired_mode = 0;
 			else
 				dma_channel->desired_mode = 1;
 
-
-			if (dma_channel->desired_mode == 0) {
-				csr &= ~(MUSB_TXCSR_AUTOSET
-					| MUSB_TXCSR_DMAMODE);
-				csr |= (MUSB_TXCSR_DMAENAB);
-					/* against programming guide */
-			} else
-				csr |= (MUSB_TXCSR_AUTOSET
-					| MUSB_TXCSR_DMAENAB
-					| MUSB_TXCSR_DMAMODE);
-
+			if (dma_channel->desired_mode == 0)
+				/* Against the programming guide */
+				csr |= MUSB_TXCSR_DMAENAB;
+			else
+				csr |= MUSB_TXCSR_AUTOSET |
+				       MUSB_TXCSR_DMAMODE |
+				       MUSB_TXCSR_DMAENAB;
 			musb_writew(epio, MUSB_TXCSR, csr);
 
 			dma_ok = dma_controller->channel_program(
@@ -788,6 +781,17 @@ static void musb_ep_program(struct musb 
 				else
 					hw_ep->rx_channel = NULL;
 				dma_channel = NULL;
+
+				/*
+				 * The programming guide says that we must only
+				 * clear the DMAReqEnab bit before DMAReqMode...
+				 */
+				csr = musb_readw(epio, MUSB_TXCSR);
+				csr &= ~(MUSB_TXCSR_DMAENAB |
+					 MUSB_TXCSR_AUTOSET);
+				musb_writew(epio, MUSB_TXCSR, csr);
+				csr &= ~MUSB_TXCSR_DMAMODE;
+				musb_writew(epio, MUSB_TXCSR, csr);
 			}
 		}
 #endif
@@ -795,18 +799,7 @@ static void musb_ep_program(struct musb 
 		/* candidate for DMA */
 		if ((is_cppi_enabled() || tusb_dma_omap()) && dma_channel) {
 
-			/* program endpoint CSRs first, then setup DMA.
-			 * assume CPPI setup succeeds.
-			 * defer enabling dma.
-			 */
-			csr = musb_readw(epio, MUSB_TXCSR);
-			csr &= ~(MUSB_TXCSR_AUTOSET
-					| MUSB_TXCSR_DMAMODE
-					| MUSB_TXCSR_DMAENAB);
-			csr |= MUSB_TXCSR_MODE;
-			musb_writew(epio, MUSB_TXCSR,
-				csr | MUSB_TXCSR_MODE);
-
+			/* Defer enabling DMA */
 			dma_channel->actual_len = 0L;
 			qh->segsize = len;
 
@@ -835,20 +828,9 @@ static void musb_ep_program(struct musb 
 		}
 
 		if (load_count) {
-			/* ASSERT:  TXCSR_DMAENAB was already cleared */
-
 			/* PIO to load FIFO */
 			qh->segsize = load_count;
 			musb_write_fifo(hw_ep, load_count, buf);
-			csr = musb_readw(epio, MUSB_TXCSR);
-			csr &= ~(MUSB_TXCSR_DMAENAB
-				| MUSB_TXCSR_DMAMODE
-				| MUSB_TXCSR_AUTOSET);
-			/* write CSR */
-			csr |= MUSB_TXCSR_MODE;
-
-			if (epnum)
-				musb_writew(epio, MUSB_TXCSR, csr);
 		}
 
 		/* re-enable interrupt */
Index: linux-2.6/drivers/usb/musb/musbhsdma.c
===================================================================
--- linux-2.6.orig/drivers/usb/musb/musbhsdma.c
+++ linux-2.6/drivers/usb/musb/musbhsdma.c
@@ -195,30 +195,32 @@ static int dma_channel_abort(struct dma_
 	void __iomem *mbase = musb_channel->controller->base;
 
 	u8 bchannel = musb_channel->idx;
+	int offset;
 	u16 csr;
 
 	if (channel->status == MUSB_DMA_STATUS_BUSY) {
 		if (musb_channel->transmit) {
+			offset = MUSB_EP_OFFSET(musb_channel->epnum,
+						MUSB_TXCSR);
 
-			csr = musb_readw(mbase,
-				MUSB_EP_OFFSET(musb_channel->epnum,
-						MUSB_TXCSR));
-			csr &= ~(MUSB_TXCSR_AUTOSET |
-				 MUSB_TXCSR_DMAENAB |
-				 MUSB_TXCSR_DMAMODE);
-			musb_writew(mbase,
-				MUSB_EP_OFFSET(musb_channel->epnum, MUSB_TXCSR),
-				csr);
+			/*
+			 * The programming guide says that we must only clear
+			 * the DMAReqEnab bit before the DMAReqMode bit...
+			 */
+			csr = musb_readw(mbase, offset);
+			csr &= ~(MUSB_TXCSR_AUTOSET | MUSB_TXCSR_DMAENAB);
+			musb_writew(mbase, offset, csr);
+			csr &= ~MUSB_TXCSR_DMAMODE;
+			musb_writew(mbase, offset, csr);
 		} else {
-			csr = musb_readw(mbase,
-				MUSB_EP_OFFSET(musb_channel->epnum,
-						MUSB_RXCSR));
+			offset = MUSB_EP_OFFSET(musb_channel->epnum,
+						MUSB_RXCSR);
+
+			csr = musb_readw(mbase, offset);
 			csr &= ~(MUSB_RXCSR_AUTOCLEAR |
 				 MUSB_RXCSR_DMAENAB |
 				 MUSB_RXCSR_DMAMODE);
-			musb_writew(mbase,
-				MUSB_EP_OFFSET(musb_channel->epnum, MUSB_RXCSR),
-				csr);
+			musb_writew(mbase, offset, csr);
 		}
 
 		musb_writew(mbase,
@@ -296,14 +298,26 @@ static irqreturn_t dma_controller_irq(in
 					&& ((channel->desired_mode == 0)
 					    || (channel->actual_len &
 					    (musb_channel->max_packet_sz - 1)))
-					 ) {
+				    ) {
+					u8  epnum  = musb_channel->epnum;
+					int offset = MUSB_EP_OFFSET(epnum,
+								    MUSB_TXCSR);
+					u16 txcsr;
+
+					/*
+					 * The programming guide says that we
+					 * must only clear the DMAReqEnab bit
+					 * before the DMAReqMode bit...
+					 */
+					musb_ep_select(mbase, epnum);
+					txcsr = musb_readw(mbase, offset);
+					txcsr &= ~(MUSB_TXCSR_DMAENAB |
+						   MUSB_TXCSR_AUTOSET);
+					musb_writew(mbase, offset, txcsr);
 					/* Send out the packet */
-					musb_ep_select(mbase,
-						musb_channel->epnum);
-					musb_writew(mbase, MUSB_EP_OFFSET(
-							musb_channel->epnum,
-							MUSB_TXCSR),
-						MUSB_TXCSR_TXPKTRDY);
+					txcsr &= ~MUSB_TXCSR_DMAMODE;
+					txcsr |=  MUSB_TXCSR_TXPKTRDY;
+					musb_writew(mbase, offset, txcsr);
 				}
 				musb_dma_completion(musb, musb_channel->epnum,
 						    musb_channel->transmit);

--
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