From 4e36213a35397fa8b262529c4946025ab58af111 Mon Sep 17 00:00:00 2001 From: P33M Date: Sat, 13 Jul 2013 20:41:26 +0100 Subject: [PATCH 1/2] dwc_otg: mask correct interrupts after transaction error recovery The dwc_otg driver will unmask certain interrupts on a transaction that previously halted in the error state in order to reset the QTD error count. The various fine-grained interrupt handlers do not consider that other interrupts besides themselves were unmasked. By disabling the two other interrupts only ever enabled in DMA mode for this purpose, we can avoid unnecessary function calls in the IRQ handler. This will also prevent an unneccesary FIQ interrupt from being generated if the FIQ is enabled. --- drivers/usb/host/dwc_otg/dwc_otg_hcd_intr.c | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/drivers/usb/host/dwc_otg/dwc_otg_hcd_intr.c b/drivers/usb/host/dwc_otg/dwc_otg_hcd_intr.c index e8b4d350922d96..27b673f4d0bde4 100644 --- a/drivers/usb/host/dwc_otg/dwc_otg_hcd_intr.c +++ b/drivers/usb/host/dwc_otg/dwc_otg_hcd_intr.c @@ -1851,7 +1851,11 @@ static int32_t handle_hc_nak_intr(dwc_otg_hcd_t * hcd, * transfers in DMA mode for the sole purpose of * resetting the error count after a transaction error * occurs. The core will continue transferring data. + * Disable other interrupts unmasked for the same + * reason. */ + disable_hc_int(hc_regs, datatglerr); + disable_hc_int(hc_regs, ack); qtd->error_count = 0; goto handle_nak_done; } @@ -1963,6 +1967,15 @@ static int32_t handle_hc_ack_intr(dwc_otg_hcd_t * hcd, halt_channel(hcd, hc, qtd, DWC_OTG_HC_XFER_ACK); } } else { + /* + * An unmasked ACK on a non-split DMA transaction is + * for the sole purpose of resetting error counts. Disable other + * interrupts unmasked for the same reason. + */ + if(hcd->core_if->dma_enable) { + disable_hc_int(hc_regs, datatglerr); + disable_hc_int(hc_regs, nak); + } qtd->error_count = 0; if (hc->qh->ping_state) { @@ -2328,6 +2341,14 @@ static int32_t handle_hc_datatglerr_intr(dwc_otg_hcd_t * hcd, qtd->urb, qtd, DWC_OTG_HC_XFER_XACT_ERR); halt_channel(hcd, hc, qtd, DWC_OTG_HC_XFER_XACT_ERR); } else if (hc->ep_is_in) { + /* An unmasked data toggle error on a non-split DMA transaction is + * for the sole purpose of resetting error counts. Disable other + * interrupts unmasked for the same reason. + */ + if(hcd->core_if->dma_enable) { + disable_hc_int(hc_regs, ack); + disable_hc_int(hc_regs, nak); + } qtd->error_count = 0; } From c46c85b2489c90cbc438a81a33d20fd2b2af8880 Mon Sep 17 00:00:00 2001 From: P33M Date: Sat, 13 Jul 2013 21:48:41 +0100 Subject: [PATCH 2/2] dwc_otg: fiq: prevent FIQ thrash and incorrect state passing to IRQ In the case of a transaction to a device that had previously aborted due to an error, several interrupts are enabled to reset the error count when a device responds. This has the side-effect of making the FIQ thrash because the hardware will generate multiple instances of a NAK on an IN bulk/interrupt endpoint and multiple instances of ACK on an OUT bulk/interrupt endpoint. Make the FIQ mask and clear the associated interrupts. Additionally, on non-split transactions make sure that only unmasked interrupts are cleared. This caused a hard-to-trigger but serious race condition when you had the combination of an endpoint awaiting error recovery and a transaction completed on an endpoint - due to the sequencing and timing of interrupts generated by the dwc_otg core, it was possible to confuse the IRQ handler. --- drivers/usb/host/dwc_otg/dwc_otg_hcd_intr.c | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/drivers/usb/host/dwc_otg/dwc_otg_hcd_intr.c b/drivers/usb/host/dwc_otg/dwc_otg_hcd_intr.c index 27b673f4d0bde4..d6553634cd0cca 100644 --- a/drivers/usb/host/dwc_otg/dwc_otg_hcd_intr.c +++ b/drivers/usb/host/dwc_otg/dwc_otg_hcd_intr.c @@ -324,6 +324,27 @@ int fiq_hcintr_handle(int channel, hfnum_data_t hfnum) } } } + else + { + /* + * If we have any of NAK, ACK, Datatlgerr active on a + * non-split channel, the sole reason is to reset error + * counts for a previously broken transaction. The FIQ + * will thrash on NAK IN and ACK OUT in particular so + * handle it "once" and allow the IRQ to do the rest. + */ + hcint.d32 &= hcintmsk.d32; + if(hcint.b.nak) + { + hcintmsk.b.nak = 0; + FIQ_WRITE((dwc_regs_base + 0x500 + (channel * 0x20) + 0xc), hcintmsk.d32); + } + if (hcint.b.ack) + { + hcintmsk.b.ack = 0; + FIQ_WRITE((dwc_regs_base + 0x500 + (channel * 0x20) + 0xc), hcintmsk.d32); + } + } // Clear the interrupt, this will also clear the HAINT bit FIQ_WRITE((dwc_regs_base + 0x500 + (channel * 0x20) + 0x8), hcint.d32);