Hello,
on Saturday 02 January 2021 at 09:34, Marcus Glocker wrote:
> I was not following up the initial conversation on this thread, so I
> probably missed a lot of what was already discussed.
A lot of the conversation was outside of mailing lists, and I'm not sure
exactly why. I summed up my understanding, but my knowledge very limit I
might have misunderstood or misexplained some things.
> 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 :-)
Here are the explanations I got from patrick@, on 2019-11-05 (in case
things changed since then), I hope I'm not committing a vile act by
cut-and-pasting a private mail into a public ML:
Interrupt transfers (IN and OUT) use a specific method to find out if
packets have been lost. This can be done by toggling the PID between
DATA0 <-> DATA1.
When we open/close uhid(4) we open/close the pipe. On non-xHCI we
can save the endpoint toggle state and keep it. With xHCI it looks
like we can only reset the toggle state... to 0.
This is because there are only add/drop and reset endpoint commands.
"Add" and "drop" reset the toggle state, while with "reset" endpoint
we have the possibility to keep it. open/close does add/drop, so we
always lose it, but that's life.
If there is a way to save the toggle state between add/drop, then it
must have gotten lost somewhere, because it all reads like it does
not exist.
Are there other ways to "set" the toggle? Yes, there is. We can
reset it on _both_ ends by clearing an endpoint stall. Yes, even
though we are not actually stalled, a CLEAR_FEATURE on the endpoint
always resets the toggle state, and clearing endpoint stall is perfect
for this.
Note that we have this issue on both the IN and OUT pipe. On the OUT
pipe we did always send it with DATA0, even though the device expected
DATA1. On the IN pipe (after drop/add) we expected DATA0, but the
device sent DATA1.
> 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);
> }
I have just tried it, and it makes the problem worse instead of solving
it.
To recap, I updated my CURRENT, by downloading a fresh bsd.rd, booting
on it and choosing (U)pgrade. I ended up with:
/bsd:
OpenBSD 6.8-current (GENERIC.MP) #254: Fri Jan 1 02:37:00 MST 2021
I also ran `pkg_add -u`, but I didn't track which packages versions I
used to have, hopefully nothing much has changed.
With the "vanilla" CURRENT, I reproduced the "works once" problem, i.e.
the first `scanimage -L` after booting works fine, and but all
subsequent uses of this command time out.
When patching the kernel, I use the github mirror, which got me commit
372bf492fc0b8782893148b4f05722d209d72ed7
I recompiled and installed the new kernel with
# cd /sys/arch/amd64/compile/GENERIC.MP
# make cleandir
# make config
# make -j2
# make install
With your patch, even the first `scanimage -L` times out.
With my usual patch, all `scanimage -L` work. I haven't tried actual
scanning yet, but usually it works too when the multiple `scanimage -L`
is OK.
Hoping this helps,
Natasha
