Remodel the USB in Kconfig

[MrKevinWeiss]

Contribution description

The issues with current architecture

Generally there has been some confusion on how to manage KConfig with respect to the board selection of default STDIO backends, specifically for boards that require a USB based backend as there are possible stacks to use.

The <BOARD>.config way of selecting cannot handle conditional selects.

The issues is more with boards such as esp32s2-wemos-mini, currently some USB stack will be selected regardless of overridding the preferred STDIO.

Selecting a USB stack directly with STDIO_USB* creates some circular dependency issues with kconfig and is hard to manage.

We also have a mutually exclusive USB stacks, TINYUSB or USBUS which should probably be a choice.

Desired behaviour

1. Ideally we want a board to default to the most obvious STDIO implementation, for example, if I have nucleo, it uses a UART, for some ESPs, USB is the default way to communicate.

2. These backends could always be overridden though, for example, I may just connect directly to a UART and want my STDIO there, or maybe use a ble based STDIO.

3. The next condition would be specifically for boards with a USB based STDIO. Since we have a TINYUSB stack and a USBUS stack we would want to use the associated STDIO depending on the stack the application selects.

4. However, if nothing is selected by the application, than bring in a USB stack (board based preference) unless there is a specific non-USB based STDIO is selected. For these boards that have this requirement, we DO NOT want to bring in the USB stack if the STDIO is specifically overridden (important for kconfig).

Update kconfiglib package to RIOT-OS org managed one

There is a problem with the upstreamed Kconfiglib implementation and the maintainer is not responsive to the fix. The issue is to do with menuconfigs in choices and has been fixed with the RIOT-OS based fork. This PR requires this fix.

Changes to the USB stack

A new entry point is introduced USB_DEVICE which indicates wanting a USB device but not caring which stack is used. This allows making a choice between the TINYUSB and USBUS stack allowing mutual exclusivity.

Making the USB stack a choice means that a specific stack cannot be selected from non-board/non-cpu/non-application based symbols. Thus the REQUIRES_ design pattern is used for a module to indicate a specific stack should be selected. This is needed for the MODULE_TINYUSB_NETDEV in this case.

Changes to USB STDIO implementations

The MODULE_STDIO_CDC_ACM and MODULE_STDIO_TINYUSB_CDC_ACM are both depends on now, using a REQUIRES_USB_STDIO to select the dependencies.
This means we do not have to use select PACKAGE_TINYUSB if TEST_KCONFIG && !MODULE_USBUS in the board select.

Why not just select the USB from STDIO_USB

Issue with using select for STDIO choices is that we cannot check which stack we are using to default the STDIO to that, breaking desired behaviour 3.

The FORCE_USB_STDIO

Desired behaviour 4 means that we do not want to bring in the USB stack if we override, say, to the UART STDIO backend. Due to the limitations of Kconfig, this is my solution to prevent the USB from being brought in if there is an STDIO that doesn't need it. It is only for the esp32s2-wemos-mini board and would not be used in other places and would only need to be explicitly disabled for applications requiring different STDIO backend and no USB. It is not perfect but I think the best solution and fairly understandable...

Issues with Kconfig

When using a choice and having conditional defaults, for example:

choice IMPL
    default FOO if CHOOSE_FOO
    default BAR

there is a limitation of the level of the level of knowledge that can be expected from Kconfig, a limitation on circular dependencies, and a limitation that the dependencies only get resolved once.

For example, if BAR selects something that would eventually select CHOOSE_FOO, then the default should be FOO and which would no longer select BAR preventing the select CHOOSE_FOO... Messy stuff and we would want an error saying no no no.

What Kconfig cannot handle is something like:

choice IMPL
    bool "Implementation"
    default FOO if CHOOSE_FOO
    default BAR

config FOO
    bool "Foo"

config BAR
    bool "Bar"

endchoice

config CHOOSE_FOO
    bool

config SYMBOL
    bool
    select CHOOSE_FOO if !BAR

SYMBOL causes a circular dependency in Kconfig even though the only possible outcome for the choice selection would be static. If we select BAR then CHOOSE_FOO would not be selected and we stay with BAR. If we select FOO than CHOOSE_FOO will be selected which stays with FOO. Everything should be fine, but isn't because Kconfig does not resolve to that degree, it simply sees that there is a dependency of the IMPL choice outcome (ie. if !BAR) that is a condition for a dependency of the IMPL choice selection (ie. if CHOOSE_FOO).

This is a limitation of the Kconfig what what makes this problem so challenging, with Make we say "select some sort of USB backend if no other stdio is specifically requested" and it will.

An attempt at remodelling the dependencies of the USB stack in Kconfig.

Currently there are some issues, especially with the integration of TinyUSB package as a backend.
This will require a kconfiglib package fix though...

Testing procedure

TEST_KCONFIG=1 BOARD=reel make menuconfig -C examples/hello-world

Issues/PRs references

Requires ulfalizer/Kconfiglib#123 to be merged upstream or fork for RIOT
Relates maybe to #18998 and #19038

[MrKevinWeiss]

Note that we can try to add some FORCE_STDIO_CDC_ACM type symbol to get around the requirement of all the entry points.

I think with make you just need stdio_cdc_acm and it will bring in everything... With Kconfig, we would need to add the MODULE_USB_DEVICE, MODULE_USBUS_CDC_ACM and somehow override MODULE_STDIO_CDC_ACM and MODULE_USBUS.

The FORCE_STDIO_CDC_ACM would just be a helper that would select all of them. An issue would be where is it visible?

Also I wonder if MODULE_USB is better than MODULE_USB_DEVICE...

[gschorcht]

Also I wonder if MODULE_USB is better than MODULE_USB_DEVICE...

This is easy to answer, MODULE_USB_DEVICE is better since it declares that the USB device implementation is used. Once we have also USB host implementation, MODULE_USB_HOST would be used.

[MrKevinWeiss]

@leandrolanzieri Would it make sense to drop the KCONFIG_USEMODULE_* stuff? I think supporting the configuration values for both might be a bit unpleasant.

[gschorcht]

I took a look, the video seems promising. Two questions: ulfalizer/Kconfiglib#123 is closed. Does that mean that we would use the RIOT fork in future? Could you try to rebase the PR so that I could play a bit with current master to see were the problems are?

[MrKevinWeiss]

It seems like ulfalizer is not maintaining kconfiglib anymore so probably we should have and use our own fork... I hope there are not too many changing we will need to make but this is an obvious one.

I will rebase and give it a more serious try, maybe I can get something mergeable that will relieve the issues with the current implementation we have... Which I really agree is not so straight forward (for me at least).

[MrKevinWeiss]

Hmmm I am currently hitting a wall with the esp32s2-wemos-mini (and others) selecting usb (either usbus or tinyusb) if no other stdio is being selected.

This issue is that the STDIO implementation choice has a choice circular dependency (I think a limitation of Kconfig).
ugg I have to look into it. I think it is similar to the native RTC problem, an unreachable choice would have this circular dep but since it is unreachable it is not really one.

[MrKevinWeiss]

Sorry for the long response, I took me a while to figure it out.

I updated the PR description so maybe rereading it would be good.

I will reformat the commits to make it a bit easier to review and maybe not drop or touch the KCONFIG_USEMODULE stuff just so it can be a smaller change at a time.

Running the tests for a subset of boards and apps locally all worked. This was an especially challenging case but I think I am happy with it. I will update the design PR with a filtered Kconfig to show what is happening then make comment a gif of it.

[bors]

try

Build failed:

• Murdock

[MrKevinWeiss]

OK, I think everything is up-to-date: ping @aabadie @gschorcht

[MrKevinWeiss]

bors try

[aabadie]

Looks good to me.

ACK

[maribu]

Do we need a try run? Or since this is ACK'd, just go for bors merge?

[bors]

try

Build failed:

• Murdock

[MrKevinWeiss]

Now I will try to merge after the fix, always something...

[MrKevinWeiss]

bors merge

[bors]

🕐 Waiting for PR status (GitHub check) to be set, probably by CI. Bors will automatically try to run when all required PR statuses are set.

[aabadie]

bors merge

[bors]

Build succeeded!

The publicly hosted instance of bors-ng is deprecated and will go away soon.

If you want to self-host your own instance, instructions are here.
For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

• Murdock
• static-tests
• tools-build-success

[MrKevinWeiss]

Wooo, thanks everybody. Hopefully the stdio and usb stuff will be easier now!