[PATCH 0/4] Rockchip: Allow to silent TPL/SPL debug console
Currently it is not possible to completely silent the debug console in i.e. PX30, what might be required in some embedded devices Disabling DEBUG_UART results in errors, due to missing symbols. In particular, rockchip sdram driver performs calls to debug_uart functions (i.e. printascii), defined directly in serial driver. This patch series fixes the dependencies in Kconfig and provides dummy implementation of debug_uart functions that are linked in case serial driver is not used.
Lukasz Czechowski (4): debug_uart: Replace debug functions with dummies if CONFIG_DEBUG_UART is not set ram: rockchip: Kconfig: Fix dependency of RAM_ROCKCHIP_DEBUG rockchip: Kconfig: PX30: Weaken dependency TPL/SPL serial arm: rockchip: PX30: Fix hard dependency to DEBUG_UART_BOARD_INIT
arch/arm/mach-rockchip/Kconfig | 5 ++--- drivers/ram/rockchip/Kconfig | 2 ++ include/debug_uart.h | 16 ++++++++++++++++ 3 files changed, 20 insertions(+), 3 deletions(-)
In case DEBUG UART is not used, define dummy macros replacing the actual function implementations that will not be available. This allows to compile code and avoid linker errors. Because the DEBUG_UART_FUNCS macro should not be used if DEBUG UART is not available, redefine it to generate compilation warning.
Signed-off-by: Lukasz Czechowski lukasz.czechowski@thaumatec.com --- include/debug_uart.h | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)
diff --git a/include/debug_uart.h b/include/debug_uart.h index 714b369e6fe..dc0f1aa4c98 100644 --- a/include/debug_uart.h +++ b/include/debug_uart.h @@ -128,6 +128,8 @@ void printdec(unsigned int value); (1 << CONFIG_DEBUG_UART_SHIFT), \ CONFIG_DEBUG_UART_SHIFT)
+#ifdef CONFIG_DEBUG_UART + /* * Now define some functions - this should be inserted into the serial driver */ @@ -197,4 +199,18 @@ void printdec(unsigned int value); _DEBUG_UART_ANNOUNCE \ } \
+#else + +#define DEBUG_UART_FUNCS \ + #warning "DEBUG_UART not defined!" + +#define printch(ch) do{}while(0); +#define printascii(str) do{}while(0); +#define printhex2(value) do{}while(0); +#define printhex4(value) do{}while(0); +#define printhex8(value) do{}while(0); +#define printdec(value) do{}while(0); + +#endif + #endif
On Tue, 27 Aug 2024 at 08:21, Lukasz Czechowski lukasz.czechowski@thaumatec.com wrote:
In case DEBUG UART is not used, define dummy macros replacing the actual function implementations that will not be available. This allows to compile code and avoid linker errors. Because the DEBUG_UART_FUNCS macro should not be used if DEBUG UART is not available, redefine it to generate compilation warning.
Signed-off-by: Lukasz Czechowski lukasz.czechowski@thaumatec.com
include/debug_uart.h | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)
Reviewed-by: Simon Glass sjg@chromium.org
diff --git a/include/debug_uart.h b/include/debug_uart.h index 714b369e6fe..dc0f1aa4c98 100644 --- a/include/debug_uart.h +++ b/include/debug_uart.h @@ -128,6 +128,8 @@ void printdec(unsigned int value); (1 << CONFIG_DEBUG_UART_SHIFT), \ CONFIG_DEBUG_UART_SHIFT)
+#ifdef CONFIG_DEBUG_UART
/*
- Now define some functions - this should be inserted into the serial driver
*/ @@ -197,4 +199,18 @@ void printdec(unsigned int value); _DEBUG_UART_ANNOUNCE \ } \
+#else
+#define DEBUG_UART_FUNCS \
#warning "DEBUG_UART not defined!"+#define printch(ch) do{}while(0); +#define printascii(str) do{}while(0); +#define printhex2(value) do{}while(0); +#define printhex4(value) do{}while(0); +#define printhex8(value) do{}while(0); +#define printdec(value) do{}while(0);
+#endif
#endif
2.43.0
The RAM_ROCKCHIP_DEBUG can be used only if DEBUG_UART is available, otherwise it won't have any effect. Add negative dependency to TPL_SILENT_CONSOLE.
Signed-off-by: Lukasz Czechowski lukasz.czechowski@thaumatec.com --- drivers/ram/rockchip/Kconfig | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/drivers/ram/rockchip/Kconfig b/drivers/ram/rockchip/Kconfig index 67c63ecba04..4a6a6328676 100644 --- a/drivers/ram/rockchip/Kconfig +++ b/drivers/ram/rockchip/Kconfig @@ -15,6 +15,8 @@ if RAM_ROCKCHIP
config RAM_ROCKCHIP_DEBUG bool "Rockchip ram drivers debugging" + depends on DEBUG_UART + depends on !TPL_SILENT_CONSOLE default y help This enables debugging ram driver API's for the platforms
Hi Lukasz,
On 8/27/24 4:20 PM, Lukasz Czechowski wrote:
The RAM_ROCKCHIP_DEBUG can be used only if DEBUG_UART is available, otherwise it won't have any effect. Add negative dependency to TPL_SILENT_CONSOLE.
Signed-off-by: Lukasz Czechowski lukasz.czechowski@thaumatec.com
drivers/ram/rockchip/Kconfig | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/drivers/ram/rockchip/Kconfig b/drivers/ram/rockchip/Kconfig index 67c63ecba04..4a6a6328676 100644 --- a/drivers/ram/rockchip/Kconfig +++ b/drivers/ram/rockchip/Kconfig @@ -15,6 +15,8 @@ if RAM_ROCKCHIP
config RAM_ROCKCHIP_DEBUG bool "Rockchip ram drivers debugging"
- depends on DEBUG_UART
- depends on !TPL_SILENT_CONSOLE
I am wondering if we should also do this for TPL-less systems? i.e. the ones for which the dram is configured in SPL.
Something like:
depends on !SPL_SILENT_CONSOLE if SPL_RAM depends on !TPL_SILENT_CONSOLE if TPL_RAM
maybe?
Cheers, Quentin
Hi Quentin,
Fair point. Unfortunately the syntax "depends on <symbol> if <expr>" seem to be not supported in Kconfig I can do something like
depends on !TPL_SILENT_CONSOLE && TPL_RAM || !SPL_SILENT_CONSOLE && SPL_RAM
instead. What do you think?
Best regards, Łukasz
czw., 29 sie 2024 o 12:19 Quentin Schulz quentin.schulz@cherry.de napisał(a):
Hi Lukasz,
On 8/27/24 4:20 PM, Lukasz Czechowski wrote:
The RAM_ROCKCHIP_DEBUG can be used only if DEBUG_UART is available, otherwise it won't have any effect. Add negative dependency to TPL_SILENT_CONSOLE.
Signed-off-by: Lukasz Czechowski lukasz.czechowski@thaumatec.com
drivers/ram/rockchip/Kconfig | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/drivers/ram/rockchip/Kconfig b/drivers/ram/rockchip/Kconfig index 67c63ecba04..4a6a6328676 100644 --- a/drivers/ram/rockchip/Kconfig +++ b/drivers/ram/rockchip/Kconfig @@ -15,6 +15,8 @@ if RAM_ROCKCHIP
config RAM_ROCKCHIP_DEBUG bool "Rockchip ram drivers debugging"
depends on DEBUG_UARTdepends on !TPL_SILENT_CONSOLEI am wondering if we should also do this for TPL-less systems? i.e. the ones for which the dram is configured in SPL.
Something like:
depends on !SPL_SILENT_CONSOLE if SPL_RAM depends on !TPL_SILENT_CONSOLE if TPL_RAM
maybe?
Cheers, Quentin
Hi Lukasz,
On 9/2/24 4:57 PM, Łukasz Czechowski wrote:
Hi Quentin,
Fair point. Unfortunately the syntax "depends on <symbol> if <expr>" seem to be not supported in Kconfig
Shoot, it's the ONLY Kconfig attribute that doesn't support if :)
I can do something like
depends on !TPL_SILENT_CONSOLE && TPL_RAM || !SPL_SILENT_CONSOLE && SPL_RAM
instead. What do you think?
Mmmm no, that wouldn't work I think.
E.g. this would allow RAM_ROCKCHIP_DEBUG to be enabled in TPL even if TPL_SILENT_CONSOLE is enabled, if SPL_RAM=y and SPL_SILENT_CONSOLE=n.
I think we have two options: 1) depends on (!SPL_SILENT_CONSOLE && SPL_RAM) || !SPL_RAM depends on (!TPL_SILENT_CONSOLE && TPL_RAM) || !TPL_RAM
This means RAM_ROCKCHIP_DEBUG would be selectable for RAM=y, TPL_RAM=n, SPL_RAM=n. I don't think anyone would want TPL_RAM AND SPL_RAM set at the same time, so having SPL_SILENT_CONSOLE set but not TPL_SILENT_CONSOLE wouldn't be an issue here.
2) Create an SPL and TPL symbol for RAM_ROCKCHIP_DEBUG, i.e. SPL_RAM_ROCKCHIP_DEBUG and TPL_RAM_ROCKCHIP_DEBUG.
This is a bit more involved in terms of changes but is closer to how U-Boot works when SPL/TPL is involved. We basically would need to change IS_ENABLED(CONFIG_RAM_ROCKCHIP_DEBUG) into CONFIG_IS_ENABLED(RAM_ROCKCHIP_DEBUG) and #ifdef CONFIG_RAM_ROCKCHIP_DEBUG into #if CONFIG_IS_ENABLED(RAM_ROCKCHIP_DEBUG).
I assume we would want those new symbols to default to y too. If that's the case, then we need to manually disable those symbols in defconfigs that disable RAM_ROCMCHIP_DEBUG right now: configs/anbernic-rgxx3-rk3566_defconfig configs/neu2-io-rv1126_defconfig configs/roc-pc-mezzanine-rk3399_defconfig configs/roc-pc-rk3399_defconfig configs/rock-pi-n10-rk3399pro_defconfig configs/rock-pi-n8-rk3288_defconfig configs/sonoff-ihost-rv1126_defconfig
Cheers, Quentin
Allow to disable serial console in TPL and SPL. Weak dependency to SPL_SERIAL and TPL_SERIAL is also used in other Rockchip boards.
Signed-off-by: Lukasz Czechowski lukasz.czechowski@thaumatec.com --- arch/arm/mach-rockchip/Kconfig | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm/mach-rockchip/Kconfig b/arch/arm/mach-rockchip/Kconfig index fc1b638ff01..47a6275d45f 100644 --- a/arch/arm/mach-rockchip/Kconfig +++ b/arch/arm/mach-rockchip/Kconfig @@ -11,8 +11,8 @@ config ROCKCHIP_PX30 select TPL_TINY_FRAMEWORK if TPL select TPL_NEEDS_SEPARATE_STACK if TPL imply SPL_SEPARATE_BSS - select SPL_SERIAL - select TPL_SERIAL + imply SPL_SERIAL + imply TPL_SERIAL select DEBUG_UART_BOARD_INIT imply ROCKCHIP_COMMON_BOARD imply SPL_ROCKCHIP_COMMON_BOARD
Hi Lukasz,
On 8/27/24 4:20 PM, Lukasz Czechowski wrote:
Allow to disable serial console in TPL and SPL. Weak dependency to SPL_SERIAL and TPL_SERIAL is also used in other Rockchip boards.
Signed-off-by: Lukasz Czechowski <lukasz.czechowski@thaumatec.com
FWIW, most commits in that file for px30 have the following prefix for the commit title:
rockchip: px30:
so following the same pattern would make sense.
The change is fine though, so:
Reviewed-by: Quentin Schulz quentin.schulz@cherry.de
Thanks! Quentin
Because DEBUG_UART_BOARD_INIT depends on DEBUG_UART, hard dependency to DEBUG_UART_BOARD_INIT in ROCKCHIP_PX30 can cause warnings if DEBUG_UART is disabled. The DEBUG_UART_BOARD_INIT is already implied by ARCH_ROCKCHIP entry. Remove hard dependency from ROCKCHIP_PX30, so that it will be consistent with other rockchip boards.
Signed-off-by: Lukasz Czechowski lukasz.czechowski@thaumatec.com --- arch/arm/mach-rockchip/Kconfig | 1 - 1 file changed, 1 deletion(-)
diff --git a/arch/arm/mach-rockchip/Kconfig b/arch/arm/mach-rockchip/Kconfig index 47a6275d45f..c9aaf6d3476 100644 --- a/arch/arm/mach-rockchip/Kconfig +++ b/arch/arm/mach-rockchip/Kconfig @@ -13,7 +13,6 @@ config ROCKCHIP_PX30 imply SPL_SEPARATE_BSS imply SPL_SERIAL imply TPL_SERIAL - select DEBUG_UART_BOARD_INIT imply ROCKCHIP_COMMON_BOARD imply SPL_ROCKCHIP_COMMON_BOARD imply ARMV8_CRYPTO
Hi Lukasz,
On 8/27/24 4:20 PM, Lukasz Czechowski wrote:
Because DEBUG_UART_BOARD_INIT depends on DEBUG_UART, hard dependency to DEBUG_UART_BOARD_INIT in ROCKCHIP_PX30 can cause warnings if DEBUG_UART is disabled. The DEBUG_UART_BOARD_INIT is already implied by ARCH_ROCKCHIP entry. Remove hard dependency from ROCKCHIP_PX30, so that it will be consistent with other rockchip boards.
Signed-off-by: Lukasz Czechowski lukasz.czechowski@thaumatec.com
Same remark for the commit title as for patch 3/4
Change is fine, so
Reviewed-by: Quentin Schulz quentin.schulz@cherry.de
Thanks! Quentin
participants (4)
-
Lukasz Czechowski -
Quentin Schulz -
Simon Glass -
Łukasz Czechowski