RE: [RFCv1 1/1] USB: EHCI: Do not return -EPIPE when hub is disconnected

["Erkka Talvitie" <erkka.talvitie@vincit.fi>]

> -----Original Message-----
> From: Erkka Talvitie <erkka.talvitie@vincit.fi>
> Sent: torstai 5. joulukuuta 2019 17.00
> To: 'Alan Stern' <stern@rowland.harvard.edu>
> Cc: gregkh@linuxfoundation.org; linux-usb@vger.kernel.org;
> claus.baumgartner@med.ge.com
> Subject: RE: [RFCv1 1/1] USB: EHCI: Do not return -EPIPE when hub is
> disconnected
> 
> 
> 
> > -----Original Message-----
> > From: Alan Stern <stern@rowland.harvard.edu>
> > Sent: torstai 5. joulukuuta 2019 16.38
> > To: Erkka Talvitie <erkka.talvitie@vincit.fi>
> > Cc: gregkh@linuxfoundation.org; linux-usb@vger.kernel.org;
> > claus.baumgartner@med.ge.com
> > 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