USB device may not enumerate on Windows

[mciantyre]

mciantyre/teensy4-rs#126 indicates that a Teensy 4 running the USB serial example (previously maintained here, now at imxrt-rs/imxrt-hal#130) did not enumerate on a Windows host (unknown version). The same device will enumerate on a Raspberry Pi (Linux). macOS is also known to work.

Testing definitely used 0.2 of imxrt-usbd. Though I can't confirm, I believe all testing incorporated 0.2.9 of usb-device.

One workaround: patch usb-device by changing the bcdUSB version from 0x10 to 0x00. My understanding is that this prevents the host from querying for BOS descriptors. I haven't dug deeper into what else this affects.

I don't have a Windows host, so I'm not equipped to test this. Nevertheless, there's a few things I'm curious about:

• Did the previous release (0.1) imxrt-usbd ever work with Windows? If you've previously tested any of the examples in this repo on a Windows host, I'd love to hear from you. I'm curious if this is a regression introduced in this driver.
• Is there any 0.2 release of usb-device that does not demonstrate this issue? We might be able to simplify the workaround by stating "pin usb-device to 0.2.x in your dependencies." And we could gather feedback for the upstream project.

[Finomnis]

I can confirm that changing bcdUSB from 0x10 to 0x00 fixes the problem. Oddly, this only happened on one of my two Windows machines. The other one worked fine, no problem.

Is this a problem in this crate or in the usb-device upstream? Probably here?

[mciantyre]

Follow-on troubleshooting suggested that mciantyre/teensy4-rs#126 is due to a misconfigured imxrt-log crate. When operating as a high-speed USB device, the logging crate should require a 512 max packet size (MPS) for bulk endpoints.

If increasing the MPS is the right fix, we should take a similar approach in the imxrt-hal USB examples (imxrt-rs/imxrt-hal#130). I wrote down some thoughts while migrating these examples. A patch in usbd-serial that lets us specify the SerialPort MPS might do the trick.

[Finomnis]

I sadly have to report that

export IMXRT_LOG_USB_BULK_MPS=512
cargo clean
cargo run --release

did not fix the issue. Changing bcdUSB from 0x10 to 0x00 did fix it.

Note that I do not use the usb serial port crate. I'm attempting to port the Teensy Rebootor code to allow for self-rebooting during programming. Hence I write a VENDOR_DEFINED custom HID device.

        let class = HIDClass::new(bus_alloc, crate::hid_descriptor::Rebootor::desc(), 10);
        let device = UsbDeviceBuilder::new(bus_alloc, UsbVidPid(0x16C0, 0x0477))
            .product("Self-Rebootor")
            .manufacturer("PJRC")
            .self_powered(true)
            .build();

and

use usbd_hid::descriptor::generator_prelude::*;

#[gen_hid_descriptor(
    (collection = APPLICATION, usage_page = VENDOR_DEFINED_START, usage = 0x0100) = {
        (usage = 0x02,) = {
            output_buffer=output;
        };
    }
)]
pub struct Rebootor {
    output_buffer: [u8; 6],
}

and finally

pub fn poll(&mut self) {
        self.device.poll(&mut [&mut self.class]);

        if self.device.state() == UsbDeviceState::Configured {
            if !self.configured {
                self.device.bus().configure();
            }
            self.configured = true;
        } else {
            self.configured = false;
        }

        if self.configured {
            let mut buf = [0u8; 6];

            let result = self.class.pull_raw_output(&mut buf);
            match result {
                Ok(info) => {
                    let buf = &buf[..info];
                    if buf == b"reboot" {
                        reboot::do_reboot();
                    }
                }
                Err(usb_device::UsbError::WouldBlock) => (),
                Err(_) => {}
            }
        }
    }

[Finomnis]

Ok I just found out that what did help was to modify

        let class = HIDClass::new(bus_alloc, crate::hid_descriptor::Rebootor::desc(), 10);
        let device = UsbDeviceBuilder::new(bus_alloc, UsbVidPid(0x16C0, 0x0477))
            .product("Self-Rebootor")
            .manufacturer("PJRC")
            .self_powered(true)
            .build();

to

        let class = HIDClass::new(bus_alloc, crate::hid_descriptor::Rebootor::desc(), 10);
        let device = UsbDeviceBuilder::new(bus_alloc, UsbVidPid(0x16C0, 0x0477))
            .product("Self-Rebootor")
            .manufacturer("PJRC")
            .self_powered(true)
            .max_packet_size_0(64)
            .build();

I guess it is the equivalent to the IMXRT_LOG_USB_BULK_MPS. I wonder why this is necessary though.

[mciantyre]

Good find. I believe a 64 byte MPS for EP0 is required for a high-speed USB device. Similar to the bulk endpoint MPS, I'm not sure why Windows doesn't enforce this until the device signals USB2.1 support.

Keep in mind that this USB bus defaults to high-speed mode. The usb-device ecosystem may not be ready to support high-speed devices. If you encounter other issues, it might help to reduce the speed.

---

If you're looking for collaborators, there's a discussion here about resetting a Teensy 4 over USB.

[Finomnis]

Related: rust-embedded-community/usb-device#116

Might it be possible to update usb-device to 0.3.x, to allow those config options?

Note that this will require a major semver bump, and indirectly then also at imxrt-log and teensy4-bsp.
Although personally I'm not a fan of the fact that teensy4-bsp depends on imxrt-log, especially that it just magically enables its features usbd, log and defmt, which I don't need for my usecase. In my opinion that's pretty hefty dependency bloat.

[mciantyre]

#25 starts the update to 0.3. I have early signal that the TestClass test suite is still passing, but I haven't made all the other logging changes.

A 0.3 update also affects imxrt-hal, since it re-exports -usbd. I'm thinking about breaking that dependency so that HAL users can bring-their-own -usbd.

I've grown to agree with your BSP opinions. We can coordinate the teensy4-bsp simplifications in mciantyre/teensy4-rs#150.