boards/*nrf52*: Enable tinyusb support for all nrf52-based boards

[mguetschow]

Contribution description

TinyUSB support is given for all nrf52-based boards, but only explicitly enabled for some selected ones. This PR enables support for all such boards.

I also noticed that usb_board_reset would currently only be enabled with the usbus stack, fixed that as well.

Testing procedure

make BOARD=feather-nrf52840-sense -C tests/pkg/tinyusb_cdc_acm_stdio flash term or any other nrf52-based board. Without this PR, only works for selected boards (e.g., nrf52840dk). Flash again to test the usb_board_reset feature.

Issues/PRs references

Hinted to by @dylad on Matrix

[dylad]

Did you tried usb_board_reset alongside with stdio_tinyusb_cdc_acm ? Does it work well ?

[riot-ci]

Murdock results

:heavy_check_mark: PASSED

e55c12de2608d867813df959d1e76bab089a60a8 fixup! boards/nrf52: enable tinyusb support if periph_usbdev is present

Success	Failures	Total	Runtime
10287	0	10287	14m:29s

Artifacts

• Documentation preview

[mguetschow]

FWIW, this seems to be a (small) subset of https://github.com/RIOT-OS/RIOT/pull/18998

[mguetschow]

Thanks CI :) good catch: apparently not all nRF52 boards have the necessary USBD peripheral, see https://docs.nordicsemi.com/bundle/struct_nrf52/page/struct/nrf52.html

[mguetschow]

With the last commit, I've moved FEATURES_PROVIDED += periph_usbdev and FEATURES_PROVIDED += tinyusb_dev to be CPU instead of board dependant. That should enable tinyusb for all nrf52-based boards that actually have the USBD peripheral.

Disclaimer: I'm not sure whether this is the right way to model this in RIOT, please have a closer look.

[mguetschow]

Hum :/

Modules should not check the content of FEATURES_PROVIDED/REQUIRED/OPTIONAL:
	cpu/nrf52/Makefile.features:17:ifneq (,$(filter periph_usbdev,$(FEATURES_PROVIDED)))

[dylad]

Maybe you could tried to check periph_usbdev against USEMODULE or FEATURES_USED instead ?

[mguetschow]

But that's not what I want: I want to express that tinyusb_device is supported iff periph_usbdev is on any nrf52 boards, not that the former is supported only if I actually want to use the latter.

[mguetschow]

Thanks, seems to work indeed, and CI doesn't complain anymore:

$ make BOARD=feather-nrf52840-sense -C tests/pkg/tinyusb_cdc_acm_stdio info-features-provided | grep tinyusb
tinyusb_device

[maribu]

I think moving the test to the different Makefile is not the right thing to do here, as the rule is specific to nRF5x.

The build system sanity check was created back them when FEATURES_REQUIRED and FEATURES_USED were added. Before, there was only FEATURES_PROVIDED and every user manually checked if all required features were provided, rather than declaring what features are required. The purpose of the check was to avoid merging PRs that have not moved to the declarative style yet.

That particular build system check has IMO done its purpose and could be retired now. I think the RIOT community has adopted the new style of declaring required features rather than checking them by hand. The last 10 times I saw that check trigger it were false positives and resulted in ugly work arounds to be created.

It is probably something to raise to a larger crowd, but I think keeping the code where it was before and dropping the now obsolete check would be better.

[crasbe]

Thank you @maribu for the "history lesson". With that in mind it is indeed the better solution to remove the check and not move the feature check to a global Makefile.

[maribu]

Probably even better: We could add a new variable:

FEATURE_IMPLIES += periph_usb:tinyusb_devive

(So adding pairs of features where the rhs is implicit provided if the lhs is provided. The list of such dependency could be declared also in CPU specific MAakefile.features.)

[crasbe]

This would be similar to the BOARD_ALIAS in #21242, right?

[mguetschow]

Probably even better: We could add a new variable:

FEATURE_IMPLIES += periph_usb:tinyusb_devive

(So adding pairs of features where the rhs is implicit provided if the lhs is provided. The list of such dependency could be declared also in CPU specific Makefile.features.)

Do you have other examples where that new mechanism would be used? Trying to understand the possible use-cases apart this concrete one.

[maribu]

Another example would be the netif feature. If a board declares it has the netif_openwsn or the netif_ethernet, it currently also has to provide the netif feature by hand. We could add FEATURES_IMPLIES += netif_openwsn:netif netif_ethernet:netif somewhere central, and boards would only need to specify the specific netif they have.

Other examples would be:

FEATURES_IMPLIES += arduio_shield_mega:arduino_shield_uno
FEATURES_IMPLIES += arch_arm7:arch_arm
FEATURES_IMPLIES += cpu_core_cortexm:arch_arm
FEATURES_IMPLIES += arch_esp_xtensa:arch_esp
FEATURES_IMPLIES += arch_esp8266:arch_esp_xtensa
FEATURES_IMPLIES += arch_esp32_xtensa:arch_esp_xtensa
FEATURES_IMPLIES += cpu_samd21:cpu_core_cortexm
FEATURES_IMPLIES += cpu_gd32v:arch_riscv
...