> From: Matthijs Kooijman [mailto:matthijs@xxxxxxxx] > Sent: Friday, March 22, 2013 5:02 PM > > I think you misread the code. The two nested ifs can indeed be true at > > the same time. > You are right, I had placed the parenthesis wrong. I see what happens > there now, yes. > > > So I will leave out that part of your patch, unless you can point out > > a problem it actually caused? > There were actually two sides to this part of the patch. One was fixing > the bug I thought I saw, but the other was to remove the handling of > DWC2_HC_XFER_URB_DEQUEUE from dwc2_hc_chhltd_intr_dma, since that > function is never called with that halt_status anymore (and having dead > code makes it confusing). Armed with the correct knowledge about those > nested ifs, I've removed this handling in the below patch, which makes > the code even simpler. Hi Matthijs, I would prefer to keep that part of the code as-is for now. I haven't quite convinced myself that the function can never be called with halt_status = DWC2_HC_XFER_URB_DEQUEUE. So I have just sent Greg my modified version of your previous patch. > > @@ -1708,8 +1708,9 @@ static bool dwc2_halt_status_ok(struct dwc2_hsotg *hsotg, > > - 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); > Why change this function? AFAICS it's never called with qtd = NULL with > the patch? You're probably right, but I feel better adding a little defensive programming here, just in case. -- Paul -- 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
