On Fri, Jan 01, 2021 at 07:15:53PM +0000, Natasha Kerensikova wrote:
> Hello,
>
> I'm only catching up with the thread now, but thanks a lot for including
> me, I am indeed interested in how this topic is evolve.
>
> FWIW, I have been running with a CURRENT and this patch since April, on
> my main computer (which I also use for remote working, so it's quite
> extensively used). But because I'm a bit lazy and need this computer to
> work, I don't update my CURRENT as often as I should.
>
> I have scanned about a hundred pages, and used common USB devices
> (keyboard, mouse, KVM masquerading as a hub, flash drive, PTP camera)
> without any issue whatsoever (except for an sudden shutdown when the
> power strip got loose from the wall socket, we clearly need a patch to
> handle such cases).
>
> > > Sorry, I was not clear; I meant to say that 'scanimage -L' isn't
> > > recognizing my EPSON ET-2750 device at all. But in the meantime I
> > > could get my hands on a CANON CanoScan LiDE 400, which gets
> > > recognized, and it looks like I see similar issues like you do.
> > > Based on that I can do some further investigations.
> >
> > It looks like with xhci(4) for some reason the bulk endpoints are
> > stalling after some operations, which isn't happening with ehci(4). I
> > currently can't see why this happens only with xhci(4). That's why
> > on your second attempt you see the timeouts happening, because the bulk
> > endpoints still are in a stalled state.
>
> Did you check my summary in
> https://marc.info/?l=openbsd-tech&m=158729024415324&w=2 ?
>
> If you have not already ruled it out, it seems worth considering:
> XHCI-specific packet loss detection feature, which uses alternating PID
> between DATA0 and DATA1, and only works when the OS and the device agree
> on the current PID for the next packet.
>
> When they get out of sync, e.g. because the OS kept the DATA1 state
> after the device reset its on state to DATA0, the OS will send a DATA1
> packet which will be ignored by the device as if a previous DATA0 was
> lost so that both a retransmitted. Maybe the OS retries, still with
> DATA1, until a timeout occurs.
>
> AFAIU, usbd_clear_endpoint_stall() is used for the side effect of both
> resetting the OS-side state to DATA0 (which I guess is done by
> usbd_clear_endpoint_toggle()) and the device-side state through the
> associated request UR_CLEAR_FAEATURE/UF_ENDPOINT_HALT.
>
> > Using usbd_clear_endpoint_stall() at the pipe closing routine simply
> > resets all the endpoints, which is why on the next attempt the
> > transfers are working fine again. That's probably why this was called
> > a workaround, because we should understand why the endpoints are
> > getting stalled in a first line with xhci(4).
>
> I'm not sure what exactly is going on in your setup, but on mine the
> stall came from the previous use which left the device and the OS
> out-of-sync. Hence USB scanner working only once, because they are in
> sync when both power up and get out-of-sync after the first use.
>
> I don't remember whether I tracked out whether OpenBSD-side state is
> reset when it shouldn't (e.g. because it's not stored after closing the
> device) or not reset when it should (e.g. because it's stored despite
> the device resetting its own state). But if I did I have completely
> forgotten the conclusion.
>
>
> Hoping this helps,
> Natasha
Thanks for your feedback.
I was not following up the initial conversation on this thread, so I
probably missed a lot of what was already discussed.
I'm also not very familiar with any xhci specific packet loss feature
mechanism. But if there was already an proposal from patrick@ to come
over this by doing an EP clearing on device open, I've just tested the
following diff, and it makes my scanner reliably fail now with xhci :-)
I saw that you already had a similar diff tested unsuccessfully.
Never the less, can you give this one another spin please on a freshly
updated -current kernel, and see if it makes any difference for you
as well?
Index: dev/usb/ugen.c
===================================================================
RCS file: /cvs/src/sys/dev/usb/ugen.c,v
retrieving revision 1.109
diff -u -p -u -p -r1.109 ugen.c
--- dev/usb/ugen.c 25 Dec 2020 12:59:52 -0000 1.109
+++ dev/usb/ugen.c 2 Jan 2021 08:22:42 -0000
@@ -389,6 +389,10 @@ ugenopen(dev_t dev, int flag, int mode,
}
}
sc->sc_is_open[endpt] = 1;
+
+ if (sce->pipeh)
+ usbd_clear_endpoint_stall(sce->pipeh);
+
return (0);
}
