Skip to content

Conversation

@Martinhoff-maker
Copy link
Member

@Martinhoff-maker Martinhoff-maker commented May 15, 2025

The ns16550 UART driver has multiple variant Kconfig options.

To successfully run the uart_basic_api test, the correct variant must be selected.

This commit adds the missing Kconfig options to the siwx917_rb4338a and siwx917_rb4342a boards.

I don't find the fix really clean but the proper way would be to have a different compatible.

@Martinhoff-maker Martinhoff-maker marked this pull request as ready for review May 15, 2025 14:09
@github-actions github-actions bot added the platform: Silabs Silicon Labs label May 15, 2025
@github-actions github-actions bot requested review from asmellby and jhedberg May 15, 2025 14:10
asmellby
asmellby previously approved these changes May 15, 2025
Comment on lines 11 to 12
# Variant for the ns16550 driver
CONFIG_UART_NS16550_DW8250_DW_APB=y
Copy link
Member

Choose a reason for hiding this comment

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

Wouldn't Kconfig.defconfig be the right place? Putting it here means that you'll get conflicts/warnings if you want to make a build without any UART enabled (since the above option depends on UART_NS16550 being enabled?

Comment on lines 15 to 16
# Variant for the ns16550 driver
CONFIG_UART_NS16550_DW8250_DW_APB=y
Copy link
Member

Choose a reason for hiding this comment

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

Same here. IMO we should modify this with Kconfig.defconfig instead, i.e. something like:

configdefault UART_NS16550_DW8250_DW_APB
	default y

The above should retain the dependency on UART_NS16550, making it still possible to completely disable UART support for a build.

Copy link
Member Author

Choose a reason for hiding this comment

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

I have hesitate between those two options but only with config and not configdefault (always forgot it exists). With configdefault and inheritance of the dependancy, that's clearly the right things to do. Changed.

The ns16550 UART driver has multiple variant Kconfig options. To
successfully run the uart_basic_api test, the correct variant must be
selected. This commit adds the missing Kconfig option.

Signed-off-by: Martin Hoff <martin.hoff@silabs.com>
Add missing uart tag for siwx91x boards.

Signed-off-by: Martin Hoff <martin.hoff@silabs.com>
@Martinhoff-maker Martinhoff-maker dismissed stale reviews from asmellby and jerome-pouiller via da46758 May 15, 2025 15:53
@Martinhoff-maker Martinhoff-maker force-pushed the fix_usart_kconfig_siwx91x branch from 577ccdf to da46758 Compare May 15, 2025 15:53
@Martinhoff-maker
Copy link
Member Author

v2: move to Kconfig.defconfig

@Martinhoff-maker Martinhoff-maker changed the title boards: silabs: add missing kconfig for uart on siwx917 board soc: silabs: siwx91x: add missing kconfig for ns16550 driver variant May 15, 2025
@sonarqubecloud
Copy link

@kartben kartben merged commit 34aff5e into zephyrproject-rtos:main May 19, 2025
31 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

platform: Silabs Silicon Labs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants