> -----Original Message----- > From: Erkka Talvitie <erkka.talvitie@xxxxxxxxx> > Sent: torstai 5. joulukuuta 2019 17.00 > To: 'Alan Stern' <stern@xxxxxxxxxxxxxxxxxxx> > Cc: gregkh@xxxxxxxxxxxxxxxxxxx; linux-usb@xxxxxxxxxxxxxxx; > claus.baumgartner@xxxxxxxxxx > Subject: RE: [RFCv1 1/1] USB: EHCI: Do not return -EPIPE when hub is > disconnected > > > > > -----Original Message----- > > From: Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> > > Sent: torstai 5. joulukuuta 2019 16.38 > > To: Erkka Talvitie <erkka.talvitie@xxxxxxxxx> > > Cc: gregkh@xxxxxxxxxxxxxxxxxxx; linux-usb@xxxxxxxxxxxxxxx; > > claus.baumgartner@xxxxxxxxxx > > Subject: RE: [RFCv1 1/1] USB: EHCI: Do not return -EPIPE when hub is > > disconnected > > > > On Thu, 5 Dec 2019, Erkka Talvitie wrote: > > > > > I tested this change and the issue did not reproduce. > > > > > > However when I was writing the comments for the patch I started to > > > think what happens in this following scenario: > > > > > > The PID Code is IN. > > > > > > 1. First there will be XACT, the CERR is decremented, let's say from > > > 3 to 2 and the host controller executes a retry. > > > 2. On this next try there will happen the condition mentioned in the > > > Table > > > 4-13 of the EHCI specification so that the MMF is set and the queue > > > is halted (because it is IN). > > > 3. To my understanding now the execution enters to this first stall > > > check if, as CERR is > 0 and CERR < EHCI_TUNE_CERR. > > > 4. The -EPIPE (stall) is returned when actually the queue was halted > > > due to MMF - not stall - and the -EPROTO should be returned. > > > > > > Is my logic correct or am I missing something? > > > > The same thought had occurred to me. > > > > > If you agree with me then I would like to present you a bit more > > > bold (in a sense of amount of refactoring) RFC. In high level this > > > another RFC separates 1. error check and 2. stall check. For me this > > > approach is a bit easier to understand from the code. Or then please > > > propose another solution. > > > > I was going to suggest: Just check for MMF and PID == IN before > > checking > for > > STALL. Everything else can remain the way it is. > > Ok, just to double check: > > I take the existing driver code (I will not apply the earlier RFC on top of > that) and apply one more check before the original stall check (that is): > } else if (QTD_CERR(token)) { > > The check that I will add is checking MMF bit && PID == IN, and this check > comes right after the babble check, right? > > Good, seems like a simple change. Yet I still prefer to test the change. > Unfortunately that goes to the next week as we have a national holiday > tomorrow. > I will get back to you most likely on Monday. I tested this change and it removes the error messages from the output. > > > > > Alan Stern > > > Erkka Talvitie Erkka Talvitie