Page MenuHomeFreeBSD

Check XHCI endpoint state before issuing XHCI endpoint commands
ClosedPublic

Authored by hselasky on Aug 14 2020, 10:07 AM.

Diff Detail

Repository
rS FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

bz added a subscriber: bz.
bz added inline comments.
sys/dev/usb/controller/xhci.c
3826 ↗(On Diff #75817)

I assume it's fairly save to assume epno >= 1 here?

This revision is now accepted and ready to land.Aug 14 2020, 10:13 AM
sys/dev/usb/controller/xhci.c
3866 ↗(On Diff #75817)

@bz : A value of zero is checked here.

pstef added inline comments.
sys/dev/usb/controller/xhci.c
3826 ↗(On Diff #75817)

Since it's checked somewhere else, maybe it'd be a good idea to put here an assertion?

This revision now requires review to proceed.Aug 14 2020, 10:47 AM
This revision is now accepted and ready to land.Aug 14 2020, 2:27 PM

For what it's worth this doesn't fix the issues I'm seeing with a Corigine xHCI controller.
I wonder if it's worth adding explicit cases for ERROR (do nothing) and DISABLED (do nothing).

@kp: Feel free to comander this patch.

While not the fix for my problem I do believe this patch is correct, and aligns us better to the xHCI Endpoint State Diagram (xhci v1.1, 4.8.3 Endpoint Context State).

I believe my problem is that the state transitions for EP0 are different. Other OSs (Linux, NetBSD) simply skip the entire stop/set tr dequeue /... configure dance for EP0. This patch tries to do that for us: https://pastebin.com/hudthnUZ and does fix the issues I see with Corigine xHCI.