USB CDC/ACM gadget

[sebastienNEC]

Hello, It would be quite handy to have the raspberry pi appear as /dev/ttyACMx so as to send over USB e.g. the log messages of circle. This way, debugging would be possible without a serial/usb adapter. I'll start working on such an implementation, as usbcdcgadget(endpoint).h/cpp. Unless this is in the works already by someone else ?

[rsta2]

This would be a great new feature. Feel free to develop it, if you like.

[sebastienNEC]

I am facing the issue that linux issues class-specific control messages on EP0, which circle reacts to by STALLing the communication on line 142 of dwusbgadgetendpoint0.cpp

The control message is a SET_LINE_CODING one: bmRequestType = 0b100001 bRequest = 0h20 wValue = 0 wIndex = 0 wLength = 7

I don't actually need this information, so how can I just "swallow" for now this control message in dwusbgadgetendpoint0.cpp ?

Thank you !

[rsta2]

The following patch should ignore all other OUT requests:

diff --git a/lib/usb/gadget/dwusbgadgetendpoint0.cpp b/lib/usb/gadget/dwusbgadgetendpoint0.cpp
index 58cb6cf..570012e 100644
--- a/lib/usb/gadget/dwusbgadgetendpoint0.cpp
+++ b/lib/usb/gadget/dwusbgadgetendpoint0.cpp
@@ -139,8 +139,27 @@ void CDWUSBGadgetEndpoint0::OnControlMessage (void)
            break;

        default:
-           Stall (TRUE);
-           BeginTransfer (TransferSetupOut, m_OutBuffer, sizeof (TSetupData));
+           // Ignore all other OUT requests
+           if (pSetupData->wLength)
+           {
+               m_State = StateOutDataPhase;
+
+               // EP0 can transfer only up to 127 bytes at once. Therefore we split greater
+               // transfers into multiple transfers, with up to max. packet size each.
+               assert (pSetupData->wLength <= sizeof m_OutBuffer);
+               m_nBytesLeft = pSetupData->wLength;
+               m_pBufPtr = m_OutBuffer;
+
+               BeginTransfer (TransferDataOut, m_pBufPtr,
+                        m_nBytesLeft <= m_nMaxPacketSize
+                          ? m_nBytesLeft : m_nMaxPacketSize);
+           }
+           else
+           {
+               m_State = StateInStatusPhase;
+
+               BeginTransfer (TransferDataIn, nullptr, 0);
+           }
            break;
        }
    }

This is untested. I hope, this is sufficient for now. Later we'll need a method, so that the gadget class driver can receive class specific requests, but currently I'm busy with other tasks (RPi 5).

[sebastienNEC]

Thanks you this works ! I'll clean up the code and send the new usbcdcgadget* files. Good luck with the Raspi5 - they changed everything with the auxiliary chip for the low-speed interfaces...

[rsta2]

Good. I'm looking forward to your CDC gadget files. Thanks! Yes, it's a challenge (RPi 5).

[sebastienNEC]

Here are the files usbcdcgadget.zip

[sebastienNEC]

The API is different from the MIDI case. I made the Gadget class a CDevice, overriding the Write() method, so that it would be suitable for CLogger::SetNewTarget() And for the reading part, I am not overriding Read() but providing a callback mechanism with RegisterReceiveHandler() If that makes sense ?

One subtelty/hack in the code: Linux needs 3 endpoints to recognize a CDC/ACM device as such. The regular bulk In/Out endpoints, plus an interrupt Notif endpoint. Circle does not support (yet?) interrupt endpoints, but fortunately linux does not seem to use this endpoint, so we can get away with simply not instantiating it.

[sebastienNEC]

Usage e.g.:

CSerialDevice* pSerial = new CSerialDevice(CInterruptSystem::Get()); pSerial->Initialize(115200);

CLogger* pLogger = new CLogger(LogDebug, CTimer::Get()); pLogger->Initialize(pSerial);

void usbCDCReceiveHandler(void *pBuffer, unsigned nLength) { pSerial->Write(pBuffer, nLength); }

CUSBCDCGadget* pUSB = new CUSBCDCGadget(CInterruptSystem::Get()); pUSB->Initialize(); pUSB->RegisterReceiveHandler(usbCDCReceiveHandler); pUSB->UpdatePlugAndPlay(); pLogger->SetNewTarget(pUSB);

while (true) { pUSB->UpdatePlugAndPlay(); }

[rsta2]

Thank you for this valuable contribution! I will test it next weekend and give you feedback.

[rsta2]

Thanks again for this driver! I pushed your sources (with small modifications) to the branch serial-cdc-gadget in the Circle repo and added a test program in test/usb-serial-cdc-gadget/. Your driver works already well. I tested it successfully with Linux 6.6.1. Nevertheless I would like to have some modifications:

• To be compatible with CSerialDevice, I would prefer to have a Read() method, instead of the callback mechanism.

• Currently there is a problem with self-powered gadgets. The application does not notice, when the device is unplugged and cannot react on it. This can only be handled, when the gadget class is not directly derived from CDevice, but a separate API device class (e.g. CUSBCDCSerialDevice) is used, which has a device name (e.g. "ttyACM0") and which is deleted in CUSBCDCGadget::OnSuspend(). The application can register a device removed handler for this device and can react, if an unplugging occurs.

• Ideally this new API device should support the method SetOptions() (as CSerialDevice is doing) with the serial option SERIAL_OPTION_ONLCR (translate NL to NL+CR on output). This should be the default setting too, because it is also in CSerialDevice.

Some more notes:

• I modified the USB device ID in the device descriptor to USB_GADGET_DEVICE_ID_BASE + 1, because USB_GADGET_DEVICE_ID_BASE is already used for the USB MIDI gadget driver. Using the same vendor/device ID combination for different gadgets might cause problems (e.g. with Windows hosts), when the host remembers a previously attached gadget.

• Currently I have not the time to implement interrupt endpoints in the USB gadget library. We will see, if problems will occur, when the gadget driver is used with other operating systems like Windows.

• Another task is adding the propagation of received class-specific requests on EP0 to the gadget class driver, to be able to handle it. Currently the patch is active, which simply ignores such requests. This should be also a task for later.

Please let me know, if you want to implement the modifications above, or if I should get active on by myself here!

[sebastienNEC]

Hello Rene, thank you for these comments, they make sense. I think it's better if you implement them yourself since you are more familiar with the rest of the USB stack. In particular the integration with CUSBSerialCDCDevice, which is at the moment specific to the USB Host use-case (e.g. it calls GetHost()). I wonder also whether some modifications are necesarry with CKernelOptions::GetLogDevice(). CLogger will get created before the USB device is fully initialized, so some caching should probably happen ?

[rsta2]

Hi Sebastien, I will implement it, but because of other tasks it will take some time. The API device should be CUSBSerialDevice (not CUSBSerialCDCDevice, my mistake), but it still needs some work to separate the logic from the existing host class driver from the interface code. I have to think about the log caching. There is already a backlog in CLogger. Perhaps there should be an additional boolean option for CLogger::SetNewTarget(), which simply sends the backlog contents to the new target, when it is set. Thanks again!

[sebastienNEC]

Thank you, and there is no rush at all for these modifications, of course !

[rsta2]

After a long while I have now applied the the suggested modifications and merged the USB CDC/ACM gadget support to the develop branch. It will be part of the next release. The branch serial-cdc-gadget has been removed. This gadget can be tested with the test/usb-serial-cdc-gadget for example.

[rsta2]

The USB CDC/ACM gadget support has been released with Circle 46.

[sebastienNEC]

Hello Rene, In usbcdcgadget.cpp line 98: is there any reason why you set the bInterval of the notif interrupt descriptor to 11 instead of 255 like in my original code ? This seems to crash circle (at least when plugged into my linux) with: 00:00:10.25 dwusbgadgetendpoint.cpp(270): assertion failed: nXferSize <= m_nTransferLength Thank you !

[rsta2]

Hi Sebastien, yes, there was a reason for this modification. With the original value 255 I get the following message from Linux 6.7.7 (2nd line):

[ 1368.378210] usb 1-4: new high-speed USB device number 9 using xhci_hcd
[ 1368.520037] usb 1-4: config 1 interface 0 altsetting 0 endpoint 0x81 has an invalid bInterval 255, changing to 11
[ 1368.521564] usb 1-4: New USB device found, idVendor=cafe, idProduct=8002, bcdDevice= 1.00
[ 1368.521579] usb 1-4: New USB device strings: Mfr=1, Product=2, SerialNumber=0
[ 1368.521584] usb 1-4: Product: CDC Gadget
[ 1368.521588] usb 1-4: Manufacturer: Circle
[ 1368.688660] cdc_acm 1-4:1.0: ttyACM0: USB ACM device
[ 1368.688764] usbcore: registered new interface driver cdc_acm
[ 1368.688778] cdc_acm: USB Abstract Control Model driver for USB modems and ISDN adapters

Looking into the USB 2.0 spec. this value can have a maximum of 16 for high-speed interrupt endpoints. I changed the value to 11, because my Linux does it anyway, and I had no problems with that.

So what to do? Unfortunately I have currently no idea, why this assertion fails, and I cannot reproduce this here. Perhaps we should reset the value to 255, because it works for both of us. Remember that interrupt endpoints are currently not really supported by Circle. This can be a problem.

[sebastienNEC]

Ooops Rene, I must apologize: I have retested this on my end and 11 works fine actually. I am also getting the same linux warning as you, so 11 is probably a good value indeed. Sorry !

[rsta2]

No problem, it's good, that it is working now. Thanks for re-testing.