USB Driver: HID Set Report / how to read data from ep0?

[flabbergast]

So, I've played a bit with ChibiOS/HAL USB Driver for a while, and got a couple of examples working, e.g. raw HID pipes (you can see the code here, but it is still very raw, doesn't comply with formatting rules, has some test code left there, licenses aren't fixed, etc...)

However now I'm stuck on one thing I don't know how to do, so I'd like to ask for your help. I'm trying to do a USB HID Keyboard firmware (and it more-less works now), but the keyboard standard has one request I can't seem to implement correctly, namely SET REPORT. This is used by the host to send to the keyboard the status of LEDs (Caps Lock LED, ...).

The way it's supposed to work is that a SETUP packet arrives over the control endpoint, with the SET_REPORT request. It has wLength set to 1 (indicating that next, one byte should come via an OUT packet over the control endpoint); this byte is a bit-field of led statuses. What I don't know how is how to read this byte properly.

Here's the code specific which I have right now that tries to do this (which doesn't work, i.e. the byte isn't read): on github.

Thanks!

[flabbergast]

I should also say that I tried to sniff USB traffic with wireshark, and curiously the way it shows the traffic is going is that under the SETUP packet (SET_REPORT) it shows the usual 64 bytes and the extra byte that should follow by OUT is shown in the same raw packet (i.e. it has 65 bytes there, although it should only have 64), and the following OUT packet (64 bytes) seems to have no data in it.

screenshots here: SETUP packet following OUT packet

[Giovanni]

Hi,

Well, this is hard to verify, I don't have an USB analyzer. Is your FIFO 64 bytes long?

Giovanni

[flabbergast]

The maxPacketSize is set to 64 in the descriptor. However wireshark apparently joins the packets that are part of one transaction (you can see that in the "length" column in the wireshark screenshots), it says 65 for that particular setup transaction.

BTW, the 8 SETUP bytes are actually bytes 0x28 to 0x2F from the first 64 bytes; the 65-th byte is the one I'm trying to read, that's the SET_REPORT value (it's 0x00 in the screenshot). I don't know enough about the low-level drivers to understand how come the driver knows which 8 bytes out of the 64 are the SETUP packet. I also don't know how to convince them to give me the 65-th byte - apparently reading it as I tried in the code I linked above does not yield the desired result.

I have some experience with some 8-bit ATMEL chips, and in the usb drivers I've used there, getting the extra byte is really just reading the next byte from the control endpoint - they all have a way of doing that.

Also - I don't have an usb analyzer as well (these things are mighty expensive), it's just wireshark software, it can sniff usb packets on a regular linux.

If you want to generate and observe SET_REPORT transactions in wireshark, just run the wireshark on linux, have it monitor the USB transactions and press caps lock on an usb keyboard.

[flabbergast]

OK, so I did some digging and debugging, and I think the current code should do what I'm trying to do.

It's because the logic in _usb_ep0setup callback (in os/hal/src/usb.c) is written well, so that if the request is HOST2DEV, and the "user" callback (i.e. the code I linked above) sets up usbp->ep0n and usbp->ep0next properly (which it does by calling usbSetupTransfer), then it proceeds to receive the next data from ep0 (lines 706-713 in os/hal/src/usb.c).

However, I'm running this on STM32F072 (DISCOVERY board), and the code runs into an unhandled exception, which according to the debugger stems from
os/hal/ports/STM32/LLD/USBv1/usb_lld.c:136
which has a comment before it, saying

/* Note, this line relies on the Cortex-M3/M4 ability to perform
unaligned word accesses.*/

So I guess that's the problem, I'm running it on Cortex-M0.

I suppose without rewriting the above, I can change the variable I'm trying to read into word, and then use just the first byte (since the second one will be filled with random stuff with the usb_ldd.c code). Does the compiler automatically align word variables?

[Giovanni]

Yes, that is one of the problems that remain to be addressed with the USB driver.

Giovanni

[flabbergast]

Next status report:

So I've changed the variable into which I want to read the byte into uint16_t, and the code works, the SET_REPORTs are read properly. (But that I think that snippet with -M3/-M4 is potentially dangerous even on -M3, -M4, when one wants to read an odd number of bytes, and the buffer into which the data is read has exactly that odd length.)

The only problem I have now is that it doesn't work properly when I have two keyboard interfaces enabled (which is common for newer keyboards that have "NKRO", i.e. there's one "usual" keyboard interface and one other which uses a different (can encode more keys) report structure).
The result of having 2 keyboard interfaces is that the board receives two SET_REPORT SETUP packets (one for each interface) within very short time (I've seen <200us), so (I think) by the time the board asks to receive the extra byte (from the _usb_ep0setup callback, lines 708-713) the endpoint reports that it's busy and the receive is not started.

Here's the backtrace, breakpoint in usbStartReceiveI triggered exactly when the endpoint reports it's busy. Triggers precisely on SET_REPORT packets from the host, when I have 2 keyboard interfaces.

Code: Select all

(gdb) backtrace
#0  usbStartReceiveI (usbp=0x20000b80 <USBD1>, ep=0 '\000') at ../../chibios/os/hal/src/usb.c:512
#1  0x080026b6 in _usb_ep0setup (usbp=0x20000b80 <USBD1>, ep=0 '\000') at ../../chibios/os/hal/src/usb.c:712
#2  0x080013de in VectorBC () at ../../chibios/os/hal/ports/STM32/LLD/USBv1/usb_lld.c:376
#3  <signal handler called>
#4  0x55555554 in ?? ()

I do not know how to resolve this. Could you please give some suggestions? Maybe stall ep0 in ep0setup callback while it runs, so that requests like this could be read in proper order?

As an aside, in the code in os/hal/ports/STM32/LLD you have some defines that use "ENDOPOINT" instead of "ENDPOINT" - is this intentional?

[Giovanni]

Buffers must have a size that is a multiple of 4, this is a known issue.

About everything else, I don't know, the status is reported by HW.

Are debug options enabled in chconf.h? especially assertions and state checker.

Giovanni

[flabbergast]

Ah, thanks about the buffer size, I didn't know. Maybe I should make that variable even uint32_t, to be on the safe side.

And yes, I have the debug options enabled in chconf.h.

About the other stuff (not processing two SETUP packets which come very quickly after each other): I understand that the busy status is reported by HW - however I think that the second SETUP packet should not be received/accepted by the HW until the first one is processed completely (with the acknowledging IN or OUT packet), i.e. until the _usb_ep0setup callback finishes.
Precisely because if not, if the first SETUP packet is HOST2DEV with non-zero wLength, the wLength amount of data that is following up on the SETUP packet are not readable, because HW is apparently already dealing with the second SETUP packet. (BTW this confuses the whole stack so that the extra data from the second SETUP packet are not read correctly.)

I don't know how to achieve that the HW would wait until _usb_ep0setup is finished before it receives again.

(I have a working (but completely different software stack) AVR8 firmware that does process the two SETUP SET_REPORT packets in quick succession correctly.)

I didn't try to read the STM32 datasheets/manuals, so I don't really know what's going on on the low-level side.

[Giovanni]

Hi,

_usb_ep0setup triggers a state machine, the callback ep0endcb is called when the operation is finished over EP0.

Giovanni