From a5073b52833e4df8e16c93dc4cbb7e0c558c74a2 Mon Sep 17 00:00:00 2001 From: Sergei Shtylyov Date: Fri, 27 Mar 2009 12:52:43 -0700 Subject: [PATCH] musb_gadget: fix unhandled endpoint 0 IRQs The gadget EP0 code routinely ignores an 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 (missing) state that comes after STATUS IN/OUT states was 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 got 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 the unhandled interrupts that generic_interrupt() and davinci_interrupt() hid by faking their result and only emitting a debug message -- so, stop doing that. Signed-off-by: Sergei Shtylyov Signed-off-by: David Brownell Signed-off-by: Greg Kroah-Hartman --- 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(-) diff --git a/drivers/usb/musb/davinci.c b/drivers/usb/musb/davinci.c index 10d11ab113ab..898b52fcff5d 100644 --- a/drivers/usb/musb/davinci.c +++ b/drivers/usb/musb/davinci.c @@ -372,12 +372,7 @@ static irqreturn_t davinci_interrupt(int irq, void *__hci) 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) diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c index 4000cf6d1e81..324459b619f7 100644 --- a/drivers/usb/musb/musb_core.c +++ b/drivers/usb/musb/musb_core.c @@ -1481,13 +1481,7 @@ static irqreturn_t generic_interrupt(int irq, void *__hci) 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 diff --git a/drivers/usb/musb/musb_core.h b/drivers/usb/musb/musb_core.h index efb39b5e55b5..c2a776ee1947 100644 --- a/drivers/usb/musb/musb_core.h +++ b/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) */ diff --git a/drivers/usb/musb/musb_gadget_ep0.c b/drivers/usb/musb/musb_gadget_ep0.c index 3f5e30ddfa27..40ed50ecedff 100644 --- a/drivers/usb/musb/musb_gadget_ep0.c +++ b/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. * * 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) 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 *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 *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 *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 *e, int value) 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: