Page 1 of 1

UHCI using a second QH for STATUS

Posted: Mon Mar 20, 2023 11:09 pm
by sounds
My UHCI driver uses two QHs for SETUP request. If the first QH doesn't use all the TDs, the UHCI controller will see the incomplete TD and stop processing. Then the second QH has just the STATUS packet.

bochsrc:

Code: Select all

usb_uhci: enabled=1
usb_uhci: port1=disk:test.raw, option1="sect_size=512"
It doesn't seem to have worked:

Code: Select all

sending 80 06 00 02 00 00 00 01
td[0] 5608460 .nxt=5608474 sts=9800000 ep=  e0012d buf=5608340 qh0 = 5608460
td[1] 5608470 .nxt=5608484 sts=9800000 ep= 7e80169 buf=5608348
td[2] 5608480 .nxt=5608494 sts=9800000 ep= 7e00169 buf=5608388
td[3] 5608490 .nxt=56084a4 sts=9800000 ep= 7e80169 buf=56083c8
td[4] 56084a0 .nxt=56084b4 sts=9800000 ep= 7e00169 buf=5608408
td[5] 56084b0 .nxt=1       sts=9800000 ep=ffe801e1 buf=5608448 qh1 = 56084b0

...

td[0] sts=9000007
td[1] sts=900001f
td[2] sts=1400007 STALL
td[3] sts=9800000
td[4] sts=9800000
td[5] sts=1400007 STALL
The Bochs log says "STATUS: Packet Toggle indicator doesn't match Device Toggle indicator. 0 != 1"

I wasn't expecting Bit 0x400000 = STALL.

From the spec, I would've thought 0x10000000 = Short Packet Detect would be set for td[2]. And the second QH with td[5] I think would not have STALL set either.

Re: UHCI using a second QH for STATUS

Posted: Tue Mar 21, 2023 1:42 am
by Klakap
I do not see SPD byte set in your packets. also, maybe you forgot to set bit 7 in command register to enable 64 bytes transfers.

Re: UHCI using a second QH for STATUS

Posted: Tue Mar 21, 2023 2:55 am
by sounds
Klakap wrote:I do not see SPD byte set in your packets.
That was my question - Bochs is not returning the SPD bit in the sts field of the TD after the transfer. Can you help me figure out why?
Klakap wrote: also, maybe you forgot to set bit 7 in command register to enable 64 bytes transfers.
I do already have that bit set. It's helpful advice though, thanks.

Re: UHCI using a second QH for STATUS

Posted: Tue Mar 21, 2023 11:06 am
by Klakap
SPD bit is not set by controller, it is set by you. If you do not set SPD bit, UHCI will not move to horizontal transfer in queue, but it will try to execute next vertical packet, what probably causes stall in your case, because first IN packet already returned all from descriptor.

Re: UHCI using a second QH for STATUS

Posted: Tue Mar 21, 2023 11:29 am
by sounds
That makes sense, thanks! Bochs is still setting STALL though:

Code: Select all

sending 80 06 00 02 00 00 00 01
td[0] 2e08460 .nxt=2e08474 sts= 9800000 ep=  e0012d buf=2e08340 qh0 = 2e08460
td[1] 2e08470 .nxt=2e08484 sts=19800000 ep= 7e80169 buf=2e08348
td[2] 2e08480 .nxt=2e08494 sts=19800000 ep= 7e00169 buf=2e08388
td[3] 2e08490 .nxt=2e084a4 sts=19800000 ep= 7e80169 buf=2e083c8
td[4] 2e084a0 .nxt=2e084b4 sts=19800000 ep= 7e00169 buf=2e08408
td[5] 2e084b0 .nxt=1       sts= 9800000 ep=ffe801e1 buf=2e08448 qh1 = 2e084b0

...

td[0] sts= 9000007
td[1] sts=1900001f
td[2] sts=1400007 STALL
td[3] sts=19800000
td[4] sts=19800000
td[5] sts=1400007 STALL
And Bochs logs
00024318000 d [USBMSD] packet hexdump (8 bytes)
00024318000 d [USBMSD] 80 06 00 02 00 00 00 01
00024318000 d [USBMSD] USB_REQ_GET_DESCRIPTOR: Config
00024318000 d [USBMSD] packet hexdump (32 bytes)
00024318000 d [USBMSD] 09 02 20 00 01 01 00 C0-00 09 04 00 00 02 08 06
00024318000 d [USBMSD] 50 00 07 05 81 02 40 00-00 07 05 02 02 40 00 00
00024318000 e [USBMSD] STATUS: Packet Toggle indicator doesn't match Device Toggle indicator. 0 != 1

Re: UHCI using a second QH for STATUS

Posted: Tue Mar 21, 2023 7:39 pm
by BenLunt
It's not your fault, it's mine... :-) In my re-write of the UHCI emulation, I forgot to add that little part. Created the flag for it, but forgot to include the flag in the "next element" check.

I will create a pull request for the fix now.

Sorry about that,
Ben

Re: UHCI using a second QH for STATUS

Posted: Tue Mar 21, 2023 8:42 pm
by sounds
Hey, that's great!

I target Bochs as supported, so I was willing to "break" things to get it working, but it sounds like this isn't what Bochs is going to be doing either. Thank you for keeping the number of corner cases and exceptions down :lol: