Hello.
Felipe Balbi wrote:
From: Arnaud Mandy <ext-arnaud.2.mandy@xxxxxxxxx>
g_zero was broken, because it started a TX which never
completed, then nuked the EP, generating a spurious
TX IRQ. This confused the previous driver.
If it confuses the *current* driver, you should fix at *before*
revamping it, not after.
Signed-off-by: Arnaud Mandy <ext-arnaud.2.mandy@xxxxxxxxx>
Signed-off-by: Felipe Balbi <felipe.balbi@xxxxxxxxx>
---
drivers/usb/musb/musb_core.c | 20 ++++++++++++++------
drivers/usb/musb/musb_gadget.c | 40 ++++++++++++++++++++++++++++++++++++++++
2 files changed, 54 insertions(+), 6 deletions(-)
diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
index 19d6185..893d7a5 100644
--- a/drivers/usb/musb/musb_core.c
+++ b/drivers/usb/musb/musb_core.c
@@ -1519,7 +1519,7 @@ static irqreturn_t generic_interrupt(int irq, void *__hci)
irqreturn_t musb_interrupt(struct musb *musb)
{
irqreturn_t retval = IRQ_NONE;
- u8 devctl, power;
+ u8 devctl, power, int_usb;
int ep_num;
u32 reg;
@@ -1527,26 +1527,32 @@ irqreturn_t musb_interrupt(struct musb *musb)
power = musb_readb(musb->mregs, MUSB_POWER);
DBG(4, "** IRQ %s usb%04x tx%04x rx%04x\n",
- (devctl & MUSB_DEVCTL_HM) ? "host" : "peripheral",
- musb->int_usb, musb->int_tx, musb->int_rx);
+ (devctl & MUSB_DEVCTL_HM) ? "host" : "peripheral",
+ musb->int_usb, musb->int_tx, musb->int_rx);
Why change this? It was perefectly indented already...
if (is_otg_enabled(musb) || is_peripheral_enabled(musb))
if (!musb->gadget_driver) {
DBG(5, "No gadget driver loaded\n");
+ musb->int_usb = 0;
+ musb->int_tx = 0;
+ musb->int_rx = 0;
return IRQ_HANDLED;
}
/* the core can interrupt us for multiple reasons; docs have
* a generic interrupt flowchart to follow
*/
- if (musb->int_usb & STAGE0_MASK)
- retval |= musb_stage0_irq(musb, musb->int_usb,
- devctl, power);
+ int_usb = musb->int_usb;
+ musb->int_usb = 0;
+ int_usb &= ~MUSB_INTR_SOF;
Why?
+ if (int_usb)
+ retval |= musb_stage0_irq(musb, int_usb, devctl, power);
/* "stage 1" is handling endpoint irqs */
/* handle endpoint 0 first */
if (musb->int_tx & 1) {
+ musb->int_tx &= ~1;
if (devctl & MUSB_DEVCTL_HM)
retval |= musb_h_ep0_irq(musb);
else
@@ -1555,6 +1561,7 @@ irqreturn_t musb_interrupt(struct musb *musb)
/* RX on endpoints 1-15 */
reg = musb->int_rx >> 1;
+ musb->int_rx = 0;
ep_num = 1;
while (reg) {
if (reg & 1) {
@@ -1576,6 +1583,7 @@ irqreturn_t musb_interrupt(struct musb *musb)
/* TX on endpoints 1-15 */
reg = musb->int_tx >> 1;
+ musb->int_rx = 0;
Not musb->int_tx?
diff --git a/drivers/usb/musb/musb_gadget.c b/drivers/usb/musb/musb_gadget.c
index a12cc55..1afd01f 100644
--- a/drivers/usb/musb/musb_gadget.c
+++ b/drivers/usb/musb/musb_gadget.c
@@ -333,6 +333,9 @@ static void nuke(struct musb_ep *ep, const int status)
stop_dma(musb, ep, req);
if (ep->is_in) {
+ u16 csr;
+
+ csr = musb_readw(epio, MUSB_TXCSR);
/*
* The programming guide says that we must not clear
* the DMAMODE bit before DMAENAB, so we only
@@ -342,6 +345,19 @@ static void nuke(struct musb_ep *ep, const int status)
MUSB_TXCSR_DMAMODE | MUSB_TXCSR_FLUSHFIFO);
musb_writew(epio, MUSB_TXCSR,
0 | MUSB_TXCSR_FLUSHFIFO);
+
+ if (csr & MUSB_TXCSR_TXPKTRDY) {
+ /* If TxPktRdy was set, an extra IRQ was just
+ * generated. This IRQ will confuse things if
+ * a we don't handle it before a new TX request
"a" is out of place here.
@@ -464,6 +486,7 @@ void musb_g_tx(struct musb *musb, u8 epnum)
struct musb_ep *musb_ep = &musb->endpoints[epnum].ep_in;
void __iomem *epio = musb->endpoints[epnum].regs;
struct dma_channel *dma;
+ int count;
musb_ep_select(mbase, epnum);
request = next_request(musb_ep);
@@ -508,6 +531,23 @@ void musb_g_tx(struct musb *musb, u8 epnum)
DBG(2, "underrun on ep%d, req %p\n", epnum, request);
}
+
+ /* The interrupt is generated when this bit gets cleared,
+ * if we fall here while TXPKTRDY is still set, then that's
+ * a really messed up case. One such case seems to be due to
+ * the HW -- sometimes the IRQ is generated early.
Arnaud, I wonder on what kind of hardware you encountered this? Were you
using DMA?
+ */
+ count = 0;
+ while (csr & MUSB_TXCSR_TXPKTRDY) {
+ count++;
+ if (count == 1000) {
+ DBG(1, "TX IRQ while TxPktRdy still set "
+ "(CSR %04x)\n", csr);
+ return;
+ }
+ csr = musb_readw(epio, MUSB_TXCSR);
+ }
+
if (dma != NULL && dma_channel_status(dma) == MUSB_DMA_STATUS_BUSY) {
/* SHOULD NOT HAPPEN ... has with cppi though, after
* changing SENDSTALL (and other cases); harmless?
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
