Hi folks, I've been trying to get the dwc2 driver running on a Ralink RT3052 board, with success (I'm still cleaning up the platform bindings and I want to see if they work for the raspberry pi as well, but my latest patches are here if you're interested: https://github.com/matthijskooijman/linux-1/tree/ralink-next). However, when connecting a 3G modem, I'm running into timeouts, for which I've found the cause, but I'm unsure of the proper fix. Here's what happens: - The ttyUSBx device exported by the 3G modem and the option driver is opened. - The option driver queues 4 BULK IN urbs. - The ttyUSBx device is closed. - The option driver dequeues the urbs. - If an active urb is dequeued, the driver halts the corresponding host channel. - The channel halted interrupt handler (dwc2_hc_chhltd_intr_dma) releases the channel, making it available again for subsequent transfers. However, when the last urb is dequeued, the qtd list for the qh associated with the channel becomes empty, which triggers the following shortcut in dwc2_hc_n_intr: if (list_empty(&chan->qh->qtd_list)) { dev_dbg(hsotg->dev, "## no QTD queued for channel %d ##\n", chnum); dev_dbg(hsotg->dev, " hcint 0x%08x, hcintmsk 0x%08x, hcint&hcintmsk 0x%08x\n", chan->hcint, hcintmsk, hcint); chan->halt_status = DWC2_HC_XFER_NO_HALT_STATUS; disable_hc_int(hsotg, chnum, HCINTMSK_CHHLTD); chan->hcint = 0; return; } This shortcut does not release the channel, causing it to be occupied forever. If you do this a few times, all USB activity halts because the hardware runs out of channels. I suspect that this shortcut exists because, if the qtd_list is empty, there is no "current qtd" to pass onto all the interrupt handlers, so better not to run them with a NULL qtd. However, as we've seen, there is a legitimate case where the qtd_list is empty during a chhltd interrupt, where the above shortcut is wrong. Furthermore, if the above shortcut is indeed intended to apply to this particular case, then it does not do enough: Even when the qtd_list is not empty, if the active qtd/urb has just been dequeued, the next qtd in the list will not be the one that the halted interupt applies to (in this particular case, the qtd value isn't actually used, so it doesn't break, but it's still a bit of a half-hearted fix). If I disable the above piece of code, things work again as expected for me (qtd will be invalid, but dwc2_hc_chhltd_intr_dma also contains a shortcut to handle the DWC2_HC_XFER_URB_DEQUEUE halt_status without touch qtd). Alternatively, I've come up with the below patch, which also works for me. However, I'm unsure if there might be some case which is not properly handled with the below. If my analysis about the shortcut above is correct, perhaps it makes sense to remove the shortcut altogether (perhaps leave a WARN_ON just in case?). Note that the below patch also fixes a small bug where the dwc2_hcd_complete_xfer_ddma would never be called because the two nested ifs around it could never be true at the same time. I think it is correct now, but I don't have DDMA hardware to test. Gr. Matthijs commit c49ae5c418b561cf277f34d14e20eb49226f8dd7 Author: Matthijs Kooijman <matthijs@xxxxxxxx> Date: Fri Mar 22 13:05:22 2013 +0100 staging: dwc2: Always release host channel after dequeueing Previously, when an active urb was dequeued, its host channel would not always be released. There is some special handling for this in dwc2_hc_chhltd_intr_dma, but when it was the last urb/qtd in its qh, a safeguard in dwc2_hc_n_intr would short-circuit and prevent the regular interrupt handlers from running, without releasing the channel. This is easily triggered when using a 3G modem using the option driver. Opening and closing any ttyUSBx device will eat up a host channel that is forever unusable from that point on. Signed-off-by: Matthijs Kooijman <matthijs@xxxxxxxx> diff --git a/drivers/staging/dwc2/hcd_intr.c b/drivers/staging/dwc2/hcd_intr.c index 4b007ab..bc6b274 100644 --- a/drivers/staging/dwc2/hcd_intr.c +++ b/drivers/staging/dwc2/hcd_intr.c @@ -1761,9 +1761,7 @@ static void dwc2_hc_chhltd_intr_dma(struct dwc2_hsotg *hsotg, } } - if (chan->halt_status == DWC2_HC_XFER_URB_DEQUEUE || - (chan->halt_status == DWC2_HC_XFER_AHB_ERR && - hsotg->core_params->dma_desc_enable <= 0)) { + if (chan->halt_status == DWC2_HC_XFER_AHB_ERR) { if (hsotg->core_params->dma_desc_enable > 0) dwc2_hcd_complete_xfer_ddma(hsotg, chan, chnum, chan->halt_status); @@ -1937,7 +1935,26 @@ static void dwc2_hc_n_intr(struct dwc2_hsotg *hsotg, int chnum) chan->hcint = hcint; hcint &= hcintmsk; + /* If the channel was halted de to a dequeue, the qtd list might + * be empty or at least the first entry will not be the active + * qtd. In this case, take a shortcut and just release the + * channel. */ + if (chan->halt_status == DWC2_HC_XFER_URB_DEQUEUE) { + /* If the channel was halted, this should be the only + * interrupt unmasked */ + WARN_ON(hcint != HCINTMSK_CHHLTD); + if (hsotg->core_params->dma_desc_enable > 0) + dwc2_hcd_complete_xfer_ddma(hsotg, chan, chnum, + chan->halt_status); + else + dwc2_release_channel(hsotg, chan, NULL, + chan->halt_status); + return; + } + if (list_empty(&chan->qh->qtd_list)) { + /* TODO: Will this ever happen with the + * DWC2_HC_XFER_URB_DEQUEUE handling above? */ dev_dbg(hsotg->dev, "## no QTD queued for channel %d ##\n", chnum); dev_dbg(hsotg->dev, -- 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
