Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

boards: add common configuration for CDC ACM UART #81308

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions boards/Kconfig
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,20 @@ config QEMU_EXTRA_FLAGS
to setup devices, for example to allocate interface for Zephyr
GDBstub over serial with `-serial tcp:127.0.0.1:5678,server`

config BOARD_REQUIRES_SERIAL_BACKEND_CDC_ACM
bool
help
Indicates that a board has no other capabilities than to use the CDC
ACM UART as a backend for logging or shell.

config BOARD_SERIAL_BACKEND_CDC_ACM
bool "Board uses USB CDC ACM UART as serial backend"
Comment on lines +137 to +138
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the prompt / naming of this indicates to me that there potentially could be other serial backends.

From initial looks, then this seems to be something worth changing into a choice entry.

Something like:

choice SERIAL_BACKEND
       prompt "Serial backend

config SERIAL_BACKEND_CDC_ACM
    bool "<prompt>"
    ...


endchoice

Copy link
Collaborator Author

@jfischer-no jfischer-no Nov 18, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I cannot think of anything that would require a different backend, it would be against "General recommendations", and USB is currently the only assumed exception.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Segger RTT could be added in the same way later on.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While using CDC ACM as the default backend for the boards without debug adapter, but with USB device controller and bootloader makes sense, what would be the case for "Segger RTT"? Is Segger RTT a subsystem that needs some configuration and how would that look like?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We currently have this configuration available as a snippet (https://github.com/zephyrproject-rtos/zephyr/tree/main/snippets/rtt-console) which works fine for me. It was merely an example of another "serial" backend, that might be added like this in the future.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I consider USB CDC to be a serial output device, and from my prior job as a vendor that had things like USB dongles added to zephyr, the company opinion was that they too are serial output devices and should be enabled by default

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is completely wrong here, user capable to use build arguments should use -DSNIPPET="rtt_console" or -DSNIPPET="cdc-acm-console".

and what does -DCONFIG_SERIAL_BACKEND_FOO vs -DSNIPPET=<foo> selecting the backend have to do with the selecting being a choice or not ?

Even if this is configured through a snippet the Kconfig symbol should still be a choice.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and what does -DCONFIG_SERIAL_BACKEND_FOO vs -DSNIPPET=<foo> selecting the backend have to do with the selecting being a choice or not ?

-DSNIPPET="cdc-acm-console" selects logging serial backend, -DCONFIG_BOARD_SERIAL_BACKEND_CDC_ACM=y does not, it just enables USB device stack and applies few optional default settings. For this PR, the board has already chosen the backend using chosen property zephyr,console or zephyr,shell-uart, boards/common/usb/Kconfig.cdc_acm_serial.defconfig are dependencies for that choice.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if this setting is not intended for the user to select, then the prompt should be removed.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if this setting is not intended for the user to select, then the prompt should be removed.

The user should be able to disable it, because boards/common/usb/Kconfig.cdc_acm_serial.defconfig enables the USB device stack at boot time, and there would be a conflict with all USB samples.

depends on BOARD_REQUIRES_SERIAL_BACKEND_CDC_ACM
default y
help
USB stack and CDC ACM UART are configured and initialized at boot
time to be used as a backend for logging or shell.

# There might not be any board options, hence the optional source
osource "$(KCONFIG_BOARD_DIR)/Kconfig"
endmenu
Expand Down
6 changes: 6 additions & 0 deletions boards/adafruit/feather_nrf52840/Kconfig.defconfig
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,10 @@

if BOARD_ADAFRUIT_FEATHER_NRF52840

if BOARD_ADAFRUIT_FEATHER_NRF52840_NRF52840_UF2 || BOARD_ADAFRUIT_FEATHER_NRF52840_NRF52840_SENSE_UF2

source "boards/common/usb/Kconfig.cdc_acm_serial.defconfig"

endif

endif # BOARD_ADAFRUIT_FEATHER_NRF52840
Original file line number Diff line number Diff line change
Expand Up @@ -8,19 +8,12 @@
/dts-v1/;
#include "adafruit_feather_nrf52840_common.dtsi"
#include <nordic/nrf52840_partition_uf2_sdv6.dtsi>
#include <../boards/common/usb/cdc_acm_serial.dtsi>

/ {
model = "Adafruit Feather nRF52840 Sense";
compatible = "adafruit,feather-nrf52840-sense-uf2";

chosen {
zephyr,console = &cdc_acm_uart0;
zephyr,shell-uart = &cdc_acm_uart0;
zephyr,uart-mcumgr = &cdc_acm_uart0;
zephyr,bt-mon-uart = &cdc_acm_uart0;
zephyr,bt-c2h-uart = &cdc_acm_uart0;
};

leds {
led0: led_0 {
gpios = <&gpio1 9 0>;
Expand All @@ -34,9 +27,3 @@
reg = <0x44>;
};
};

zephyr_udc0: &usbd {
cdc_acm_uart0: cdc_acm_uart0 {
compatible = "zephyr,cdc-acm-uart";
};
};
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,5 @@ CONFIG_UART_CONSOLE=y
CONFIG_CLOCK_CONTROL_NRF_K32SRC_RC=y
CONFIG_CLOCK_CONTROL_NRF_K32SRC_500PPM=y

# Logger cannot use itself to log
CONFIG_USB_CDC_ACM_LOG_LEVEL_OFF=y

# Enable USB
CONFIG_USB_DEVICE_STACK=y

# Build UF2 by default, supported by the Adafruit nRF52 Bootloader
CONFIG_BUILD_OUTPUT_UF2=y
Original file line number Diff line number Diff line change
Expand Up @@ -7,28 +7,15 @@
/dts-v1/;
#include "adafruit_feather_nrf52840_common.dtsi"
#include <nordic/nrf52840_partition_uf2_sdv6.dtsi>
#include <../boards/common/usb/cdc_acm_serial.dtsi>

/ {
model = "Adafruit Feather nRF52840 Express";
compatible = "adafruit,feather-nrf52840-uf2";

chosen {
zephyr,console = &cdc_acm_uart0;
zephyr,shell-uart = &cdc_acm_uart0;
zephyr,uart-mcumgr = &cdc_acm_uart0;
zephyr,bt-mon-uart = &cdc_acm_uart0;
zephyr,bt-c2h-uart = &cdc_acm_uart0;
};

leds {
led0: led_0 {
gpios = <&gpio1 15 0>;
};
};
};

zephyr_udc0: &usbd {
cdc_acm_uart0: cdc_acm_uart0 {
compatible = "zephyr,cdc-acm-uart";
};
};
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,5 @@ CONFIG_SERIAL=y
CONFIG_CONSOLE=y
CONFIG_UART_CONSOLE=y

# Logger cannot use itself to log
CONFIG_USB_CDC_ACM_LOG_LEVEL_OFF=y

# Enable USB
CONFIG_USB_DEVICE_STACK=y

# Build UF2 by default, supported by the Adafruit nRF52 Bootloader
CONFIG_BUILD_OUTPUT_UF2=y
9 changes: 0 additions & 9 deletions boards/adafruit/itsybitsy/Kconfig

This file was deleted.

42 changes: 1 addition & 41 deletions boards/adafruit/itsybitsy/Kconfig.defconfig
Original file line number Diff line number Diff line change
Expand Up @@ -5,46 +5,6 @@

if BOARD_ADAFRUIT_ITSYBITSY

if BOARD_SERIAL_BACKEND_CDC_ACM

config USB_DEVICE_STACK
default y

config USB_CDC_ACM
default SERIAL

config UART_CONSOLE
default CONSOLE

config USB_DEVICE_INITIALIZE_AT_BOOT
default y if CONSOLE

config SHELL_BACKEND_SERIAL_CHECK_DTR
default SHELL
depends on UART_LINE_CTRL

config UART_LINE_CTRL
default SHELL

config USB_DEVICE_REMOTE_WAKEUP
default n

if LOG

# Logger cannot use itself to log
config USB_CDC_ACM_LOG_LEVEL
default 0

# Set USB log level to error only
config USB_DEVICE_LOG_LEVEL
default 1

# Wait 1500ms at startup for logging
config LOG_PROCESS_THREAD_STARTUP_DELAY_MS
default 1500

endif # LOG

endif # BOARD_SERIAL_BACKEND_CDC_ACM
source "boards/common/usb/Kconfig.cdc_acm_serial.defconfig"

endif # BOARD_ADAFRUIT_ITSYBITSY
11 changes: 2 additions & 9 deletions boards/adafruit/itsybitsy/adafruit_itsybitsy_nrf52840.dts
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,6 @@
compatible = "adafruit,itsybitsy-nrf52840";

chosen {
zephyr,console = &cdc_acm_uart0;
zephyr,shell-uart = &cdc_acm_uart0;
zephyr,uart-mcumgr = &cdc_acm_uart0;
zephyr,bt-mon-uart = &cdc_acm_uart0;
zephyr,bt-c2h-uart = &cdc_acm_uart0;
zephyr,ieee802154 = &ieee802154;
};

Expand Down Expand Up @@ -155,8 +150,6 @@
zephyr_udc0: &usbd {
compatible = "nordic,nrf-usbd";
status = "okay";

cdc_acm_uart0: cdc_acm_uart0 {
compatible = "zephyr,cdc-acm-uart";
};
};

#include <../boards/common/usb/cdc_acm_serial.dtsi>
30 changes: 3 additions & 27 deletions boards/arduino/opta/Kconfig.defconfig
Original file line number Diff line number Diff line change
Expand Up @@ -11,34 +11,10 @@ config NET_L2_ETHERNET

endif # NETWORKING

if USB_DEVICE_STACK
if BOARD_ARDUINO_OPTA_STM32H747XX_M7

config USB_DEVICE_PRODUCT
default "Arduino Opta"
source "boards/common/usb/Kconfig.cdc_acm_serial.defconfig"

config USB_DEVICE_PID
default 0x0164

config USB_DEVICE_VID
default 0x35d1

config USB_DEVICE_INITIALIZE_AT_BOOT
default y

if LOG

# Logger cannot use itself to log
choice USB_CDC_ACM_LOG_LEVEL_CHOICE
default USB_CDC_ACM_LOG_LEVEL_OFF
endchoice

# Set USB log level to error only
choice USB_DEVICE_LOG_LEVEL_CHOICE
default USB_DEVICE_LOG_LEVEL_ERR
endchoice

endif # LOG

endif # USB_DEVICE_STACK
endif # BOARD_ARDUINO_OPTA_STM32H747XX_M7

endif # BOARD_ARDUINO_OPTA
14 changes: 2 additions & 12 deletions boards/arduino/opta/arduino_opta_stm32h747xx_m7.dts
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,6 @@
compatible = "arduino,opta-m7";

chosen {
zephyr,console = &cdc_acm_uart0;
zephyr,shell-uart = &cdc_acm_uart0;
zephyr,cdc-acm-uart0 = &cdc_acm_uart0;
zephyr,sram = &sram0;
zephyr,flash = &flash0;
zephyr,code-partition = &slot0_partition;
Expand All @@ -29,12 +26,10 @@ zephyr_udc0: &usbotg_fs {
pinctrl-0 = <&usb_otg_fs_dm_pa11 &usb_otg_fs_dp_pa12>;
pinctrl-names = "default";
status = "okay";

cdc_acm_uart0: cdc_acm_uart0 {
compatible = "zephyr,cdc-acm-uart";
};
};

#include <../boards/common/usb/cdc_acm_serial.dtsi>

&clk_hse {
clock-frequency = <DT_FREQ_M(25)>;
hse-bypass;
Expand Down Expand Up @@ -126,8 +121,3 @@ zephyr_udc0: &usbotg_fs {
status = "okay";
};
};

/* Assign USB serial (ACM) to M7 to have a working console out of the box */
&cdc_acm_uart0 {
status = "okay";
};
3 changes: 0 additions & 3 deletions boards/arduino/opta/arduino_opta_stm32h747xx_m7_defconfig
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,3 @@ CONFIG_STM32H7_BOOT_M4_AT_INIT=n
CONFIG_SERIAL=y
CONFIG_CONSOLE=y
CONFIG_UART_CONSOLE=y

# Enable USB Stack (needed for the console to work)
CONFIG_USB_DEVICE_STACK=y
30 changes: 3 additions & 27 deletions boards/arduino/portenta_h7/Kconfig.defconfig
Original file line number Diff line number Diff line change
Expand Up @@ -10,34 +10,10 @@ config NET_L2_ETHERNET

endif # NETWORKING

if USB_DEVICE_STACK
if BOARD_ARDUINO_PORTENTA_H7_STM32H747XX_M7

config USB_DEVICE_PRODUCT
default "Arduino SA Portenta H7"
source "boards/common/usb/Kconfig.cdc_acm_serial.defconfig"

config USB_DEVICE_PID
default 0x035b

config USB_DEVICE_VID
default 0x2341

config USB_DEVICE_INITIALIZE_AT_BOOT
default y

if LOG

# Logger cannot use itself to log
choice USB_CDC_ACM_LOG_LEVEL_CHOICE
default USB_CDC_ACM_LOG_LEVEL_OFF
endchoice

# Set USB log level to error only
choice USB_DEVICE_LOG_LEVEL_CHOICE
default USB_DEVICE_LOG_LEVEL_ERR
endchoice

endif # LOG

endif # USB_DEVICE_STACK
endif # BOARD_ARDUINO_PORTENTA_H7_STM32H747XX_M7

endif # BOARD_ARDUINO_PORTENTA_H7
3 changes: 0 additions & 3 deletions boards/arduino/portenta_h7/arduino_portenta_h7-common.dtsi
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,4 @@ zephyr_udc0: &usbotg_hs {
<&rcc STM32_SRC_HSI48 USB_SEL(3)>;
num-bidir-endpoints = < 4 >;
status = "okay";
cdc_acm_uart0: cdc_acm_uart0 {
compatible = "zephyr,cdc-acm-uart";
};
};
Original file line number Diff line number Diff line change
Expand Up @@ -8,16 +8,14 @@
#include <st/h7/stm32h747Xi_m7.dtsi>
#include <st/h7/stm32h747xihx-pinctrl.dtsi>
#include "arduino_portenta_h7-common.dtsi"
#include <../boards/common/usb/cdc_acm_serial.dtsi>

/ {
model = "Arduino Portenta H7 board";
compatible = "arduino,portenta-h7";

/* HW resources are split between CM7 and CM4 */
chosen {
zephyr,console = &cdc_acm_uart0;
zephyr,shell-uart = &cdc_acm_uart0;
zephyr,cdc-acm-uart0 = &cdc_acm_uart0;
zephyr,sram = &sram0;
zephyr,flash = &flash0;
zephyr,code-partition = &slot0_partition;
Expand Down Expand Up @@ -98,10 +96,6 @@
status = "okay";
};

&cdc_acm_uart0 {
status = "okay";
};

&flash0 {
partitions {
compatible = "fixed-partitions";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,3 @@ CONFIG_UART_LINE_CTRL=y
# Enable regulator
CONFIG_REGULATOR=y
CONFIG_REGULATOR_FIXED=y

# Enable USB Stack
CONFIG_USB_DEVICE_STACK=y
25 changes: 1 addition & 24 deletions boards/atmarktechno/degu_evk/Kconfig.defconfig
Original file line number Diff line number Diff line change
Expand Up @@ -5,29 +5,6 @@

if BOARD_DEGU_EVK

if USB_DEVICE_STACK

config USB_DEVICE_PRODUCT
default "Degu Evaluation Kit"

config UART_INTERRUPT_DRIVEN
default y

config UART_LINE_CTRL
default y

endif # USB_DEVICE_STACK

if LOG

# Logger cannot use itself to log
config USB_CDC_ACM_LOG_LEVEL
default 0

# Set USB log level to error only
config USB_DEVICE_LOG_LEVEL
default 1

endif # LOG
source "boards/common/usb/Kconfig.cdc_acm_serial.defconfig"

endif # BOARD_DEGU_EVK
Loading
Loading