USB GetFeature/SetFeature implementation - problem with request length

1 post / 0 new
Author
Message
#1
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Hi everyone, first time poster here!

 

I am messing around with ASF4 running on an ATSAME54P20A trying to get a custom HID device going. I have the data reports working, report descriptors, etc. In addition I am trying to provide to the host a vendor-specific Feature report that is used to configure the device. The idea is the host can write/read there to configure it. 

The way I have implemented it is by adding a handler before fully initialising the stack:

static struct usbdc_handler usbdc_custom_handler_cb_h = {NULL, (FUNC_PTR)usbdc_custom_handler};
...
usbdc_init(..);
usbdc_register_handler(USBDC_HDL_REQ, &usbdc_custom_handler_cb_h);
hiddf_generic_init(...);	
usbdc_start(...);
usbdc_attach();

So far so good. The handler is called before the HID handler. Inside I can check the USB stage, request type, request, wValue, etc and catch those GetFeature/SetFeature commands on the Control pipe. I can properly decode them and reply to GetFeature. And don't mess with the rest of the packets (that's why the ERR_NOT_FOUND at the bottom)

int32_t usbdc_custom_handler(uint8_t ep, struct usb_req *req, enum usb_ctrl_stage stage) {
    if (stage == USB_SETUP_STAGE && req->bmRequestType == 0b10100001) {
		// A1 - GET_REPORT bRequestType value, wValue bytes holds the report type and the report ID
		if (req->bRequest == USB_REQ_HID_GET_REPORT) {
			if (req->wValueBytes.h == USB_REQ_WVALUE_H_REPORT_TYPE_FEATURE) {
			    // req->wValueBytes.l is the report ID
			    // do some processing of the GetFeature request
			    
			    // reply to the GetFeature request. buf, len contain the reply
			    usbdc_xfer(ep, buf, len, false);
			    return ERR_NONE;
			}
		}
    }
    
    if (stage == USB_DATA_STAGE && req->bmRequestType == 0b00100001) {
		if (req->bRequest == USB_REQ_HID_SET_REPORT) {
			if (req->wValueBytes.h == USB_REQ_WVALUE_H_REPORT_TYPE_FEATURE) {
			    // again - read report_id from the wValue
			    // process SetRequest as needed
			    usbdc_xfer(ep, NULL, 0, 0); // not sure if needed
			    return ERR_NONE;
			}
		}
    }
    
    return ERR_NOT_FOUND; // allow the USB stack handler to execute
}

Looks nice, works fine. Not cleaned up but that's not the idea. Now let's get to the main issue.

 

The way I test this is with a python script (using libhidapi wrapper) on Linux. I do a send_feature_report with 3 bytes packet. It is accepted and processed by the code above. Then my host sends a get_feature_report, requesting 26 bytes packet. The first one is a command, the next one is reading back the result of the command. The second packet is also accepted by the code above, processed and replied to nicely. However the host receives just 3 bytes, not 26!

 

I have investigated in depth what is going on. Wireshark-ing the packets shows the host is properly sending the requested data length in the payload of the get_feature_request packet (0xA1 0x01 0x04 0x03 0x00 0x00 0x1A 0x00). The call to usbdc_xfer to send the reply contains proper buffer and length. After a lot of debugging I have concluded the problem is actually quite complicated. I see in usb_d_ep_transfer (in file hal_usb_device.c around line 460) the stack reads the length of the packet from the request. However for some reason that returns the length of the PREVIOUS packet, not the one we are currently replying to. In fact diving deeper I have noticed that the request reading (hal_usb_device.c near line 174) calls _usb_d_dev_ep_read_req which produces wrong req buffer.

 

But things are even weirder. Because somehow only the request length (wLength, last 2 bytes) is affected. If I request with a different report ID I see that it is changed (wValue in the req bytes 2-4). So something somewhere somehow is messing the last (at least) 2 bytes of that req buffer and it contains the ones from the previous (SetFeature) request. In fact if I send a second GetFeature request from the host it gets replied with 26 bytes correctly.

 

Makes no sense. There is a some DMA involved and what not and I am not even entirely sure if what I see in the debugger is true. This could be some cache that decides to change the report length because I have written it to some value, but sounds unlikely. I was even thinking some clock configuration problem but again - doubtful. Even tried to send the reply from the main loop instead of the handler as that is called in an interrupt. No change...

 

The workaround I have implemented is to tap in my usbdc_custom_handler, cache the usb_req structure and when replying to a request - overwrite its wLength bytes. This way the code in the usb_d_ep_transfer function doesn't mess with the length (USB_GET_wLength(ept->xfer.req) returns what I need it to) and everything works OK.

 

Has anyone had any similar issues? I know I am using this thing in a bit of corner-case (and slightly outside the spec by writing and reading a report of different length) but the problem I am facing seems too strange and deep in the stack to be ignored.

 

Thanks to anyone with any thoughts on this. I will be happy to provide additional information if needed. I just don't like the way I have hacked it right now and I am keen on finding a better solution to this problem. Maybe I have gone completely the wrong way with this - it's my first time dabbing in USB stuff :)

Dimitar Kunchev