> From: Matthijs Kooijman [mailto:matthijs@xxxxxxxx] > Sent: Friday, March 22, 2013 9:32 AM ... > 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. > 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?). Hi Matthijs, I added the above code because there were cases where the QTD list was empty and the driver would crash. So I think we want to use your alternate patch instead of removing the above code entirely. But obviously I missed an important case here, so thanks for catching this. > 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. I think you misread the code. The two nested ifs can indeed be true at the same time. So I will leave out that part of your patch, unless you can point out a problem it actually caused? I have taken your patch and reworked it a bit. Would you mind testing my patch below and confirm that it also fixes the problem for you? Also, can you let me know what device it was that triggered this problem for you? I would like to buy one so I can test this case myself. -- Paul diff --git a/drivers/staging/dwc2/hcd_intr.c b/drivers/staging/dwc2/hcd_intr.c index 4b007ab..8b68df8 100644 --- a/drivers/staging/dwc2/hcd_intr.c +++ b/drivers/staging/dwc2/hcd_intr.c @@ -708,7 +708,7 @@ static void dwc2_release_channel(struct dwc2_hsotg *hsotg, free_qtd = 1; break; case DWC2_HC_XFER_XACT_ERR: - if (qtd->error_count >= 3) { + if (qtd && qtd->error_count >= 3) { dev_vdbg(hsotg->dev, " Complete URB with transaction error\n"); free_qtd = 1; @@ -729,7 +729,7 @@ static void dwc2_release_channel(struct dwc2_hsotg *hsotg, case DWC2_HC_XFER_PERIODIC_INCOMPLETE: dev_vdbg(hsotg->dev, " Complete URB with I/O error\n"); free_qtd = 1; - if (qtd->urb) { + if (qtd && qtd->urb) { qtd->urb->status = -EIO; dwc2_host_complete(hsotg, qtd->urb->priv, qtd->urb, -EIO); @@ -1708,8 +1708,9 @@ static bool dwc2_halt_status_ok(struct dwc2_hsotg *hsotg, dev_dbg(hsotg->dev, "hcint 0x%08x, hcintmsk 0x%08x, hcsplt 0x%08x,\n", chan->hcint, hcintmsk, hcsplt); - dev_dbg(hsotg->dev, "qtd->complete_split %d\n", - qtd->complete_split); + if (qtd) + dev_dbg(hsotg->dev, "qtd->complete_split %d\n", + qtd->complete_split); dev_warn(hsotg->dev, "%s: no halt status, channel %d, ignoring interrupt\n", __func__, chnum); @@ -1937,7 +1938,31 @@ static void dwc2_hc_n_intr(struct dwc2_hsotg *hsotg, int chnum) chan->hcint = hcint; hcint &= hcintmsk; + /* + * If the channel was halted due 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
