Clarification of what is meant by "serial output" in the Board porting "General recommendations"

[jfischer-no]

Introduction

There is ambiguity of "serial output" in the Bord porting "General recommendations".

Problem description

Bord porting General recommendations contains a list of items to follow, the first of which is [0] "Unless explicitly recommended otherwise by this section, leave peripherals and their drivers disabled by default.". One of the recommended exceptions is [1] "If available, configure and enable a serial output for the console using the zephyr,console chosen node in the devicetree.".
(The same rule applies to the shields.)

Although not explicitly stated, we can infer from [0] that subsystems must not be enabled by default. Mainly to avoid unnecessary bloat and conflicts in the samples and tests, and to keep the samples and tests configuration as generic as possible.

[1] seems to be a valid exception, since most of the samples and tests use serial peripheral to provide output/input to the user but it does not explicitly describe what type of "serial output" is allowed. We can see that most boards use a hardware UART controller for the console. A few boards use the virtual UART controller provided by the USB CDC ACM, and to make it work they enable the USB device subsystem (CONFIG_USB_DEVICE_STACK=y). The motivation is obviously to get samples like hello_world to work out of the box. But it leads to a conflict in the USB device support samples and other subsystems samples that use the USB device stack for their own purposes. This is not unique to USB support, we would have a similar problem if there would be a virtual UART controller provided by e.g. Bluetooth support.

Proposed change

The suggestion is to change [0] to "Leave all subsystems disabled by default. Unless explicitly recommended otherwise by this section, leave peripherals and their drivers disabled by default. "
and in [1] clearly describe what kind of hardware is meant by "serial output", e.g. hardware UART controller.

Kconfig options CONFIG_USB_DEVICE_STACK or CONFIG_USB_CDC_ACM should not be enabled by any board. Board should not chosen property like zephyr,console = &usb_cdc_acm_uart;. The approach of using CDC ACM UART as a console is completely generic, so the code is the same for every board. The solution to this (although not as complete as shield approach proposed in #40645) is already in the form of "CDC-ACM Console Snippet" in the tree and can be used. What is missing is a documentation/intruction (maybe as template) to use snippet where it is appropriate e.g. hello_world sample and where it is not e.g. samples/subsys/usb/console.

Dependencies

• ✅ CDC-ACM Console Snippet

Concerns

One could say that it can be annoying to have to specify, or even know about, e.g. -S cdc-acm-console ... to build options. I think it is acceptable if it is clearly pointed out in the board documentation, and if you work with Zephyr, you will have to learn pretty quickly that you need to enter additional arguments, and they will not get any shorter (e.g. EXTRA_DTC_OVERLAY_CONFIG).

Alternatives

chaos

[henrikbrixandersen]

@jfischer-no This seems like a good idea overall. Can you provide a PR containing the proposed changes in detail?

[carlescufi]

Architecture WG:

• @jfischer-no mentions that his proposal has two distinct parts:
  • Disable USB by default, even if the board has a USB virtual COM Port
  • Use snippets for this
• @bjarki-trackunit mentions that there is a similar case on a board that use an I2C bridge
• @nashif states that we should clarify the output to be a console in general, not UART specifically
• @nashif disagrees with the idea of not having a console without a default and having to use a snippet to get output. This affects users who want to try out hello_world as well as testing, which will now require to add -S <snippet name> to the west invocations
• @jfischer-no prefers the approach presented in this RFC, with subsystems being disabled by default (eg. USB) and asking the users to enable via snippet. This also makes sure actual USB samples work, which is not the case now.

[jfischer-no]

Many boards have similar code to configure the USB and CDC ACM UART that they want to use as a logging or shell backend. Also, many boards have incorrect or incomplete configuration.
This needs to be fixed and cleaned up. Ideally before we deprecate the old USB device stack.

Besides that the code needs to be moved to a common part, controversial context is whether to disable it by default and let the user choose where to configure CDC ACM UART as backend for logging and shell or keep the current situation that it is enabled by default but has conflict with any USB related sample or application.

#81228 adds common configuration for CDC ACM UART to boards/common and uses board variants (e.g. cdc_acm_serial)

#81220 allows snippets to have Kconfig files and "move" backend configuration to the snippet cdc-acm-console.

[carlescufi]

Architecture WG:

• Should serial output on USB-enabled boards be ON by default?
• Regardless of the answer above, how do we enable serial output in USB boards?
  • Boards variants: west build -b nrf52840dk/nrf52840/cdc_acm_serial <sample>
  • Snippets: west build -b nrf52840dk/nrf52840 -S cdc_acm_serial <sample>
• @nashif: objects to using board variants to enable the CDC ACM, considers this a misuse of variants (including additional variants like xip). Objection to removing the functionality from the base board, instead of having to specify a snippet to get the same functionality
• @erwango considers CDC ACM variant orthogonal with other ones (e.g. ns variants), will be difficult to combine variants. Could snippets be selected by default with a board?
• @nordicjm considers that those boards should continue to work out-of-the box as part of the default board configuration
• @jfischer-no: having CDC ACM in the default board configuration will not work with the new stack (will conflict with the configuration from other samples, particularly)

[nordicjm]

@jfischer-no: having CDC ACM in the default board configuration will not work with the new stack (will conflict with the configuration from other samples, particularly)

Clash how? Is that the same point I said whereby it won't work with a tiny subset of samples like hci_usb but they're by far in the minority of samples and majority have UART output or something else?