April 10, 2025

Black Pill boards - F411 USB -- Kondo treatment, part 2

I didn't think this would work, but I made a test copy of the code, tried it, rebuilt, tested, and it works just fine. I used "sed" to make the following two substitutions throughout the source code: 
s/USB_OTG_CORE_HANDLE/HANDLE/g
s/USB_OTG_//g
The first was somewhat cautious, but now we see just the first of the following two lines: 
EnableCommonInt(HANDLE *pdev);

USB_OTG_EnableCommonInt(USB_OTG_CORE_HANDLE *pdev);
I find the first much easier on the eyes (and mind).

Repeat the process for DCD_

When this is done, we have a name collision with code in driver/driver.c, so I previx those routines with "DRV_" for now. Someday they should be static (or combined with the DCD routines). 
void         DRV_SetEPStatus (HANDLE *pdev , EP *ep , uint32_t Status);
uint32_t     DRV_GetEPStatus(HANDLE *pdev ,EP *ep);

Repeat the process for DCD_

Now we get an issue in library/usb_core.h where we have These two enums: 
typedef enum {
  USBD_OK   = 0,
  USBD_BUSY,
  USBD_FAIL,
} USBD_Status;

typedef enum {
  OK = 0,
  FAIL
} STS;
I would like to do something about the second enum, but that will do for now.
To deal with the first, I will try using this sequence of substitutions: 
s/USBD_OK/UU_OK/
s/USBD_BUSY/UU_BUSY/
s/USBD_FAIL/UU_FAIL/
s/USBD_Status/UU_Status/
s/USBD_//
This works! But now I have multiple definitions of "Init()". I rename USBD_Init() to BOGUS_Init() then I can do the substitutions, build and test. It works.

Clean up the two Status enums

How often does UU_BUSY now get used? As it turns out, it is only used (returned) by VCP_DataRx() in vcp/vcp.c The return value is tested in CLASS_DataOut() in vcp/cdc.c but the value is only tested agains UU_OK. In other words we could do away with BUSY and/or merge this with the STS enum and do some more unification/cleanup.

So I replace both with an enum "Status" with values OK, FAIL, and BUSY. As the code is now, BUSY is returned in one place and may as well be FAIL given how it is treated.

Many of the routines that return enum values like OK are declared like this: 

uint8_t
CORE_Resume ( HANDLE  *pdev )
{
  ...
  return OK;
}
I am surprised that C puts up with this. An enum is apparently sort of a weak "hint" to the compiler, not a strongly enforced type, which is sort of unfortunate.

There are quite a few routines that always return OK. These really ought to just be void -- this might actually yield more compact code.

Conclusion

The code looks entirely different without all of the prefix clutter. Perhaps this served someone somewhere some purpose at some time, but for my puposes, concise and clean code is the win.
Feedback? Questions? Drop me a line!

Tom's Computer Info / tom@mmto.org