Wrong device speed detected
marcosox opened this issue · 11 comments
I have a low-speed USB 1.1 mouse, that when connected directly to the host declares in the EP descriptor a bInterval value of 10 (getting a polling rate of 8ms).
When passing through USBProxy the mouse gets identified by the host as a High Speed device (bcdUSB = 0x200), and the same bInterval value in the EP descriptor is interpreted differently (according to the standard), bringing the polling rate to a slow 64ms.
Since the mouse is correctly enumerated on the device side of the beaglebone (at 1ms interval), the packets start to accumulate in the USBProxy queues and the mouse performances host-side are poor.
My question is: where should I look to resolve this issue? it doesn't seem that the bInterval value is hardcoded somewhere, so why and how does the program changes the device speed when presenting it to the host side?
Nice catch!
From http://www.beyondlogic.org/usbnutshell/usb5.shtml#EndpointDescriptors
bInterval is used to specify the polling interval of certain transfers. The units are expressed in frames, thus this equates to either 1ms for low/full speed devices and 125us for high speed devices.
So when we force the device to be a high speed device, we alter the interval, although I would expect the effect to be in the opposite direction? i.e. the host thinks it's a faster device so polls more often than the BBB polls the device.
yes but the same bInterval value is interpreted differently if the device is high speed. from the standard:
A full-speed endpoint can specify a desired period from 1 ms to 255 ms.
Low-speed endpoints are limited to specifying only 10 ms to 255 ms.
High-speed endpoints can specify a desired period (2^(bInterval-1))x125 μ s, where bInterval is in the range 1 to (including) 16.
Since the bInterval declared by my mouse is 10, in full speed is 10ms, in high speed it's (2^9) * 125us = 64ms.
The issue is, USBProxy presents the device to the host as a High speed device, when it is actually a low-speed one.
Ah I see, the meaning of bInterval changed, not just the scale.
Yes, I agree that the problem is that we're changing the meaning of that because we're changing the speed of the device. I seem to remember that we represent everything as a high speed device because of strangeness in gadgetfs, but I may be misremembering that.
The place to fix this is probably in src/Plugins/Hosts/HostProxy_GadgetFS.cpp in the function generate_descriptor(Device* device)). This is where we produce two configuration descriptors whether or not the device is high speed. We should either stop doing that or correct the value of bInterval in the high speed descriptor.
I put together a crude conversion routine that calculates the bInterval value when passing from low/full speed to high speed. it works with my mouse, didn't test with anything else.
This goes between lines 91 and 92 in src/Plugins/Hosts/HostProxy_GadgetFS.cpp:
if(device->get_descriptor()->bcdUSB != 0x200){
char* pointer = (char*) buf;
pointer += buf->bLength; //move to end of cfg desc
for(unsigned int k=0;k<buf->bNumInterfaces;k++){
//move to end of intf desc
pointer += cfg->get_interface(k)->get_descriptor()->bLength;
if(cfg->get_interface(k)->has_HID()){
pointer+= cfg->get_interface(k)->get_HID_descriptor_length();
}
for(int gen_idx = 0; gen_idx < cfg->get_interface(k)->get_generic_descriptor_count();gen_idx++){
pointer+= cfg->get_interface(k)->get_generic_descriptor(gen_idx)->bLength;
}
for(int ep_idx = 0; ep_idx < cfg->get_interface(k)->get_descriptor()->bNumEndpoints;ep_idx++){
usb_endpoint_descriptor* epd = (usb_endpoint_descriptor*) pointer;
// conversion is: newValue = log2(8*oldValue)+1
int newValue = (log10(8*(epd->bInterval))/log10(2)) + 1;
fprintf(stderr,"old bInterval: %02X\ncalculated new bInterval: %02X\n",epd->bInterval,newValue);
memset(&epd->bInterval,newValue,1);
pointer+= epd->bLength;
}
}
}
This looks like a reasonable work around to me. Do you want to submit it as a pull request? Or would you like me to merge it as is?
If you can merge it as is it would save me a lot of work. You may have to add the missing get_HID_descriptor_length() method in interface.cpp
This fails to build with a couple of other issues too:
/home/dominicgs/CS/USB/USBProxy/src/Plugins/Hosts/HostProxy_GadgetFS.cpp: In member function ‘int HostProxy_GadgetFS::generate_descriptor(Device*)’:
/home/dominicgs/CS/USB/USBProxy/src/Plugins/Hosts/HostProxy_GadgetFS.cpp:101:31: error: ‘class Interface’ has no member named ‘has_HID’
if(cfg->get_interface(k)->has_HID()){
^
/home/dominicgs/CS/USB/USBProxy/src/Plugins/Hosts/HostProxy_GadgetFS.cpp:102:42: error: ‘class Interface’ has no member named ‘get_HID_descriptor_length’
pointer+= cfg->get_interface(k)->get_HID_descriptor_length();
^
/home/dominicgs/CS/USB/USBProxy/src/Plugins/Hosts/HostProxy_GadgetFS.cpp:113:49: error: ‘log10’ was not declared in this scope
int newValue = (log10(8*(epd->bInterval))/log10(2)) + 1;
^
Plugins/Hosts/CMakeFiles/HostProxy_GadgetFS.dir/build.make:62: recipe for target 'Plugins/Hosts/CMakeFiles/HostProxy_GadgetFS.dir/HostProxy_GadgetFS.cpp.o' failed
Sorry, i added also the corresponding helper functions in the Interface class:
const bool Interface::has_HID(){
return (hid_descriptor!=NULL);
}
size_t Interface::get_HID_descriptor_length(){
return hid_descriptor->get_full_descriptor_length();
}
you also have to include math.h in the HostProxy_GadgetFS class to use log10().
Added. Thanks for the contribution!
@dominicgs I made a mistake in the check for the device speed. A device can declare bcdUSB=0x200 but still be a low/full speed device. In this case the issue appears again.
Instead of doing this:
if(device->get_descriptor()->bcdUSB != 0x200)
it should do this:
if(!device->is_high_speed())
thanks for the patience. If I have other contributions I will fork the repo and submit pull requests, saving you from the hassle.
No problem, thanks for letting me know. I've pushed the fix.