test/usb-serial-cdc-gadget and Windows / USB mass-storage gadget development

[messinm]

Was trying test\usb-serial-cdc-gadget to connect Raspi zero2 to Windows (11), and could not get it to connect properly. Code 10, device would not start.

I have a board running Yocto with gadget driver,and it connected to Windows with no problems, and so I ran packet captures to compare the two. The Yocto driver responds to GET DESCRIPTOR DEVICE with bDeviceClass =0x02 (CDC), while Circle responds with 0x0.

I changed CUSBCDCGadget::s_DeviceDescriptor so that Circle would respond with bDeviceClass =0x02, and that fixed the issue. Seems like this would work with other OS as well, but I haven't tested.

Should I create an issue for this?

[rsta2]

The serial USB CDC gadget has be developed by @sebastienNEC. Sebastien do you think, the described modification should be generally added for the device descriptor? I think, we can discuss this here.

[sebastienNEC]

Aha indeed, I tested your modification and linux (5.4.0) still works, while Windows (10) now works (it didn't use to).

This needs fixed in the develop branch indeed, sorry for overlooking that as I had only tested on linux !

[messinm]

No worries, it was fun to dig into low-level USB stuff (I don't have any expertise in USB). Having read a bit more, seems that bDeviceClass=0x0 indicates a composite device. Did you select this so that additional functions could be added to your driver?

I've been thinking it would be nice to add USB mass storage to the gadget USB, to allow easy access to the RPi Sd card. Might start experimenting with that, any suggestions would be welcome.

[sebastienNEC]

While I'm playing with USB: would it be reasonable to replace in dwusbgadget.cpp all CTimer::Get()->xxxxDelay() by their CScheduler::Get()->xxxSleep() equivalent ?

I tried that for the CDC gadget and it works, of course by having the USB initialization code in a CTask.

If so, care must be taken to fallback to CTimer delays if the scheduler was not instantiated. Noting that one cannot use a test on (CScheduler::Get() == nullptr) because the assert would fail.

[sebastienNEC]

To be honest, the bDeviceClass=0x0 was an oversight on my side, but indeed, a composite CDC/MSC device would be super useful.

From my experience on the STM32 though, windows/linux are pretty fussy about the exact descriptors of composite devices - they only recognize certain configuration. Do you have an existing composite device whose descriptor you could copy ?

Now mind you, this was a few years ago, the situation might have improved :)

[rsta2]

Thank you for contributing your MSD gadget driver! I will integrate it from your known repository into Circle.

[messinm]

You're welcome, and thanks for circle!

[rsta2]

The integrated USB MSD gadget driver is on the branch usb-msd-gadget with the test program in test/usb-msd-gadget. I did some small modifications and removed the memory device support in the test, because it is not necessary any more, with the SD card supported. You have to add the option logdev=ttyS1 to the file cmdline.txt on the SD card, to get the log output via the serial interface. I tested it on a RPi 3 A+ with a Linux 6.9.9 host so far.

Can you please have a look, if this is OK so far. If you have modifications, please send a PR for the branch usb-msd-gadget. Probably I will also have another look again. After completion this branch will be merged to the develop branch to be part of the next release.

[messinm]

It looks good to me, thanks.

[rsta2]

The USB MSD gadget driver and test has been merged to the develop branch now. The branch usb-msd-gadget has been removed. Thanks again for your work!

[rsta2]

I have made the bDeviceClass modification on the develop branch and added support for the system option NO_BUSY_WAIT to dwusbgadget.cpp. When this option is defined, the (longer) delays are replaced with sleep calls. You have to define this option, because this needs the scheduler in the system, which is not always available. NO_BUSY_WAIT was already used in other drivers (e.g. USB, EMMC).

@messinm Thanks for reporting this, and @sebastienNEC Thanks for testing the recommended change and for clarifying the situation.

[messinm]

You're welcome!