[PATCH 1/2] MUSB: fix unhandled gadget endpoint 0 IRQs

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

 



The EP0 code routinely ignores interrupt at end of the data phase because of
musb_g_ep0_giveback() resetting the state machine to "idle, waiting for SETUP"
phase prematurely.

The driver also prematurely leaves the status phase on receiving the SetupEnd
interrupt...

As there were still unhandled endpoint 0 interrupts happening from time to
time after fixing these issues, there turned to be yet another culprit: two
distinct gadget states collapsed into one.  The state that comes after STATUS
IN/OUT states is typically indiscernible from them since the corresponding
interrupts tend to happen within too little period of time (due to only a
zero-length status packet in between) and so they get coalesced; yet this
state is not the same as the next one which is associated with the reception
of a SETUP packet.

Adding this extra state seems to have fixed the rest of unhandled interrupts
that generic_interrupt() and davinci_interrupt() hid from user by faking their
result and only emitting a debug message -- so, stop doing that...

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

---
This patch are is atop of the Linus' tree plus 3 more patches posted earlier
that haven't made it into any tree yet:

http://marc.info/?l=linux-usb&m=123502258502676
http://marc.info/?l=linux-usb&m=123558766411239
http://marc.info/?l=linux-usb&m=123516211401715

 drivers/usb/musb/davinci.c         |    7 -----
 drivers/usb/musb/musb_core.c       |    8 ------
 drivers/usb/musb/musb_core.h       |    3 +-
 drivers/usb/musb/musb_gadget_ep0.c |   45 ++++++++++++++++++++++++++++++++-----
 4 files changed, 43 insertions(+), 20 deletions(-)

Index: linux-2.6/drivers/usb/musb/davinci.c
===================================================================
--- linux-2.6.orig/drivers/usb/musb/davinci.c
+++ linux-2.6/drivers/usb/musb/davinci.c
@@ -357,12 +357,7 @@ static irqreturn_t davinci_interrupt(int
 
 	spin_unlock_irqrestore(&musb->lock, flags);
 
-	/* REVISIT we sometimes get unhandled IRQs
-	 * (e.g. ep0).  not clear why...
-	 */
-	if (retval != IRQ_HANDLED)
-		DBG(5, "unhandled? %08x\n", tmp);
-	return IRQ_HANDLED;
+	return retval;
 }
 
 int musb_platform_set_mode(struct musb *musb, u8 mode)
Index: linux-2.6/drivers/usb/musb/musb_core.c
===================================================================
--- linux-2.6.orig/drivers/usb/musb/musb_core.c
+++ linux-2.6/drivers/usb/musb/musb_core.c
@@ -1481,13 +1481,7 @@ static irqreturn_t generic_interrupt(int
 
 	spin_unlock_irqrestore(&musb->lock, flags);
 
-	/* REVISIT we sometimes get spurious IRQs on g_ep0
-	 * not clear why...
-	 */
-	if (retval != IRQ_HANDLED)
-		DBG(5, "spurious?\n");
-
-	return IRQ_HANDLED;
+	return retval;
 }
 
 #else
Index: linux-2.6/drivers/usb/musb/musb_core.h
===================================================================
--- linux-2.6.orig/drivers/usb/musb/musb_core.h
+++ linux-2.6/drivers/usb/musb/musb_core.h
@@ -171,7 +171,8 @@ enum musb_h_ep0_state {
 
 /* peripheral side ep0 states */
 enum musb_g_ep0_state {
-	MUSB_EP0_STAGE_SETUP,		/* idle, waiting for setup */
+	MUSB_EP0_STAGE_IDLE,		/* idle, waiting for SETUP */
+	MUSB_EP0_STAGE_SETUP,		/* received SETUP */
 	MUSB_EP0_STAGE_TX,		/* IN data */
 	MUSB_EP0_STAGE_RX,		/* OUT data */
 	MUSB_EP0_STAGE_STATUSIN,	/* (after OUT data) */
Index: linux-2.6/drivers/usb/musb/musb_gadget_ep0.c
===================================================================
--- linux-2.6.orig/drivers/usb/musb/musb_gadget_ep0.c
+++ linux-2.6/drivers/usb/musb/musb_gadget_ep0.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@xxxxxxxxxx>
  *
  * This program is free software; you can redistribute it and/or
  * modify it under the terms of the GNU General Public License
@@ -58,7 +59,8 @@
 static char *decode_ep0stage(u8 stage)
 {
 	switch (stage) {
-	case MUSB_EP0_STAGE_SETUP:	return "idle";
+	case MUSB_EP0_STAGE_IDLE:	return "idle";
+	case MUSB_EP0_STAGE_SETUP:	return "setup";
 	case MUSB_EP0_STAGE_TX:		return "in";
 	case MUSB_EP0_STAGE_RX:		return "out";
 	case MUSB_EP0_STAGE_ACKWAIT:	return "wait";
@@ -628,7 +630,7 @@ irqreturn_t musb_g_ep0_irq(struct musb *
 		musb_writew(regs, MUSB_CSR0,
 				csr & ~MUSB_CSR0_P_SENTSTALL);
 		retval = IRQ_HANDLED;
-		musb->ep0_state = MUSB_EP0_STAGE_SETUP;
+		musb->ep0_state = MUSB_EP0_STAGE_IDLE;
 		csr = musb_readw(regs, MUSB_CSR0);
 	}
 
@@ -636,7 +638,18 @@ irqreturn_t musb_g_ep0_irq(struct musb *
 	if (csr & MUSB_CSR0_P_SETUPEND) {
 		musb_writew(regs, MUSB_CSR0, MUSB_CSR0_P_SVDSETUPEND);
 		retval = IRQ_HANDLED;
-		musb->ep0_state = MUSB_EP0_STAGE_SETUP;
+		/* Transition into the early status phase */
+		switch (musb->ep0_state) {
+		case MUSB_EP0_STAGE_TX:
+			musb->ep0_state = MUSB_EP0_STAGE_STATUSOUT;
+			break;
+		case MUSB_EP0_STAGE_RX:
+			musb->ep0_state = MUSB_EP0_STAGE_STATUSIN;
+			break;
+		default:
+			ERR("SetupEnd came in a wrong ep0stage %s",
+			    decode_ep0stage(musb->ep0_state));
+		}
 		csr = musb_readw(regs, MUSB_CSR0);
 		/* NOTE:  request may need completion */
 	}
@@ -697,11 +710,31 @@ irqreturn_t musb_g_ep0_irq(struct musb *
 			if (req)
 				musb_g_ep0_giveback(musb, req);
 		}
+
+		/*
+		 * In case when several interrupts can get coalesced,
+		 * check to see if we've already received a SETUP packet...
+		 */
+		if (csr & MUSB_CSR0_RXPKTRDY)
+			goto setup;
+
+		retval = IRQ_HANDLED;
+		musb->ep0_state = MUSB_EP0_STAGE_IDLE;
+		break;
+
+	case MUSB_EP0_STAGE_IDLE:
+		/*
+		 * This state is typically (but not always) indiscernible
+		 * from the status states since the corresponding interrupts
+		 * tend to happen within too little period of time (with only
+		 * a zero-length packet in between) and so get coalesced...
+		 */
 		retval = IRQ_HANDLED;
 		musb->ep0_state = MUSB_EP0_STAGE_SETUP;
 		/* FALLTHROUGH */
 
 	case MUSB_EP0_STAGE_SETUP:
+setup:
 		if (csr & MUSB_CSR0_RXPKTRDY) {
 			struct usb_ctrlrequest	setup;
 			int			handled = 0;
@@ -783,7 +816,7 @@ irqreturn_t musb_g_ep0_irq(struct musb *
 stall:
 				DBG(3, "stall (%d)\n", handled);
 				musb->ackpend |= MUSB_CSR0_P_SENDSTALL;
-				musb->ep0_state = MUSB_EP0_STAGE_SETUP;
+				musb->ep0_state = MUSB_EP0_STAGE_IDLE;
 finish:
 				musb_writew(regs, MUSB_CSR0,
 						musb->ackpend);
@@ -803,7 +836,7 @@ finish:
 		/* "can't happen" */
 		WARN_ON(1);
 		musb_writew(regs, MUSB_CSR0, MUSB_CSR0_P_SENDSTALL);
-		musb->ep0_state = MUSB_EP0_STAGE_SETUP;
+		musb->ep0_state = MUSB_EP0_STAGE_IDLE;
 		break;
 	}
 
@@ -959,7 +992,7 @@ static int musb_g_ep0_halt(struct usb_ep
 
 		csr |= MUSB_CSR0_P_SENDSTALL;
 		musb_writew(regs, MUSB_CSR0, csr);
-		musb->ep0_state = MUSB_EP0_STAGE_SETUP;
+		musb->ep0_state = MUSB_EP0_STAGE_IDLE;
 		musb->ackpend = 0;
 		break;
 	default:

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