March 24, 2025

Black Pill boards - F411 USB -- tidy up the source code

I have already explored some of this with a look at the VCP class interdependencies. But this marks a turning point. I am now starting to do some rearranging and rewriting.

I added a file library/public.c to library. It holds every entry point I expect the outside world to access.

I added another file vcp/class.c to the vcp directory. This is only partially complete, but It has a variety of entry points named "class_usb_*" that are intended to be the only access to the VCP class. There are currently two parts of the current design that bypass this. There is a callback structure and a descriptor structure. I have an idea about eliminating the descriptor structure, and then it may be possible to also get rid of the callback structure by moving everything in class.c

Here are some goals in all of this:

Once I am in a directory like "library", I don't need all the filenames to have a common prefix like "usbd_". It is just visible clutter, and pointless if every file has the same prefix. Part of tidying is to eliminate visual clutter. Code can be grouped and organized. Why not have everything related to initializion in "init.c" and everything related to enumeration in "enum.c". Short meaningful filenames make life better.

Some dead wood

While investigating descriptors used during enumeration in VCP, I find them in two places:

The descriptor types are defined in usbd_def.h 

#define  USB_DESC_TYPE_DEVICE                              1
#define  USB_DESC_TYPE_CONFIGURATION                       2
#define  USB_DESC_TYPE_STRING                              3
#define  USB_DESC_TYPE_INTERFACE                           4
#define  USB_DESC_TYPE_ENDPOINT                            5
#define  USB_DESC_TYPE_DEVICE_QUALIFIER                    6
#define  USB_DESC_TYPE_OTHER_SPEED_CONFIGURATION           7
#define  USB_DESC_TYPE_BOS                                 0x0F
My debug output is like this. So type 2 gets the "generic" CDC descriptor that my attention is currently focused on. 
Get Descriptor: 1
desc: (8) 1201000200000040
Get Descriptor: 1
desc: (18) 120100020000004083044057000201020301
Get Descriptor: 6
Get Descriptor: 6
Get Descriptor: 6
Get Descriptor: 2
 ** getting CDC config descriptor **
desc: (9) 09024300020100C032
Get Descriptor: 2
 ** getting CDC config descriptor **
desc: (67) 09024300020100C032090400000102020100052400100105240100010424020205240600010705820308001009040100020A0000000705010200020007058102000200
Get Descriptor: 3
desc: (4) 04030904
Get Descriptor: 3
desc: (66) 4203530054004D003300320020005600690072007400750061006C00200043006F006D0050006F0072007400200069006E0020004600530020004D006F0064006500
Get Descriptor: 3
desc: (38) 2603410043004D0045002000620061007200200061006E00640020006700720069006C006C00
Get Descriptor: 3
desc: (26) 1A03300030003000300030003000300030003000350030004300
All the excitement is going on in USBD_GetDescriptor() in USBD_GetDescriptor() in library/usbd_req.c. Descriptors are obtained in either of two ways: 
pbuf = pdev->dev.usr_device->GetDeviceDescriptor(pdev->cfg.speed, &len);
pbuf = (uint8_t *)pdev->dev.class_cb->GetConfigDescriptor(pdev->cfg.speed, &len);
One way uses function pointers in the usr_device structure, the other uses function pointers in the class_cb structure. I want to unify all this and probably eliminate the use of functions altogether. Why not move the big switch/case in usbd_req.c into the class and have a function "class_get_descriptor(type)" that handles the lookup. I would put all the descriptors into "descriptor.c" or some such.

All of this is clean up as I prepare to do away with CDC/ACM entirely and replace it with a "tty" style class as per a CP2102. To do this, any mention of "cdc" or "acm" must be eradicated from library.

The work is well underway with vcp/desc.c

What might be nice to do someday is to move library to the top level, then make vcp and driver subdirectories.

Now let's get rid of the class callbacks

I am making changes that look like this: 
	// pdev->dev.class_cb->Setup (pdev, req);
    CLASS_Setup (pdev, req);
To be distinctive, I am using the prefix "CLASS_" for what used to be callbacks. My intent is to use all capital prefix names like this for callbacks I have erradicated.

I thought long and hard about whether I should get rid of the callbacks. The advantages outweigh the negatives. There is certainly no execution speed penalty, actually the code ought to be faster if anything. The big advantage is clarity. In this case we don't loose flexibility, we can replace vcp with some other class, as long as it provides the expected Klass_Setup() and such. And we don't need to check for null pointers as the current code does, we just supply stub routines where a NULL pointer is currently used.

I am more interested in code clarity than anything else -- and honestly that is always the right choice. A huge advantage is that ctags and cscope will work nicely with this scheme as the code is being studied. The callbacks are a wall and a nightmare to ctags.

Simplify some filenames

cd vcp
mv usbd_cdc_core.c cdc.c
mv usbd_cdc_vcp.c vcp.c
Why not? These long filenames are just visual clutter and a pain to type in without making mistakes. The include files in this directory are all now strictly used within the directory, so I can rename those as well.

Erradicate another callback scheme

I find this one hard to understand. It is within the "vcp" directory and controls calls from the CDC to the VCP layer. Indeed, the VCP layer is a specific way to set up CDC which is much more general, but this would only be useful if alternative (and useful) alternatives to VCP were available.

In the current context it yields only obfuscation, with absolutely no benefit, so let's get rid of it. There are layers of crazy name mapping. A structure named VCP_fops is set up and initialized with actual functions like VCP_Init(). But then this is mapped to APP_FOPS and that is used to make calls like the following, that I will change as shown: 

APP_FOPS.pIf_Init(pdev);
VCP_Init(pdev);
Who knows what pIf stands for, perhaps "Polish International Federation", but that is just a guess -- but it is going away.
Feedback? Questions? Drop me a line!

Tom's Computer Info / tom@mmto.org