Hello Sandrine,
On 19.05.21 12:27, Sandrine Bailleux wrote:
Hello Heiko,
Thanks for your patch!
TF-A project uses Gerrit as the review system for patches, would you mind posting your patch there instead? More details about the patch submission process can be found here:
https://trustedfirmware-a.readthedocs.io/en/latest/process/contributing.html
Oh, thanks for the hint ... my first imx-atf patch, sorry.
Regarding the patch itself, I agree with you that this feature would be useful to other platforms as well. I wonder whether we need a new build option for this, though. We already got LOG_LEVEL, which if set to 0,
My use case is a yocto build and I differ between a development and release build. And I simply pass the build option ... same as for IMX_UART port number...
will effectively silent any console output. However, it will still embed the console driver code within the firmware, if I am not mistaken. So perhaps we should change that, and make LOG_LEVEL=0 compile the console driver code out, in addition to what it already does today.
How does that sound to you?
Seems a valid approach to me. Code if not used at all, should be dropped. But I am an imx-atf code newbie and do not want to break any existing code ...
So, i just changed patch now to:
diff --git a/plat/imx/imx8m/imx8mp/imx8mp_bl31_setup.c b/plat/imx/imx8m/imx8mp/imx8mp_bl31_setup.c index 22fbd5e4b..b8f85b9b4 100644 --- a/plat/imx/imx8m/imx8mp/imx8mp_bl31_setup.c +++ b/plat/imx/imx8m/imx8mp/imx8mp_bl31_setup.c @@ -95,7 +95,9 @@ static void bl31_tzc380_setup(void) void bl31_early_platform_setup2(u_register_t arg0, u_register_t arg1, u_register_t arg2, u_register_t arg3) { +#if (LOG_LEVEL > LOG_LEVEL_NONE) static console_t console; +#endif unsigned int i;
/* Enable CSU NS access permission */ @@ -109,10 +111,12 @@ void bl31_early_platform_setup2(u_register_t arg0, u_register_t arg1,
imx8m_caam_init();
+#if (LOG_LEVEL > LOG_LEVEL_NONE) console_imx_uart_register(IMX_BOOT_UART_BASE, IMX_BOOT_UART_CLK_IN_HZ, IMX_CONSOLE_BAUDRATE, &console); /* This console is only used for boot stage */ console_set_scope(&console, CONSOLE_FLAG_BOOT); +#endif
/* * tell BL3-1 where the non-secure software image is located
without need for adding in Makefile new build option.
I tried to add this now to other imx8m* based platforms and find in
./plat/imx/imx8m/imx8mq/imx8mq_bl31_setup.c
135 #if DEBUG_CONSOLE 136 static console_t console; 137 138 console_imx_uart_register(IMX_BOOT_UART_BASE, IMX_BOOT_UART_CLK_IN_HZ, 139 IMX_CONSOLE_BAUDRATE, &console); 140 #endif
and adding the build option in "./plat/imx/imx8qx/platform.mk"
Added Igor to cc as he is the author of this part of code.
So, should I adapt this platform to LOG_LEVEL approach too? But this will may break board builds depending on DEBUG_CONSOLE...
bye, Heiko
Regards, Sandrine
On 5/18/21 4:01 PM, Heiko Schocher via TF-A wrote:
when bootloaders and kernels console output is disabled, (called silent boot) no output on console is recommended.
Add commandline option IMX_BOOT_SILENT to disable console registration at all.
Default value is 0, so patch does not change current functionality.
Signed-off-by: Heiko Schocher hs@denx.de
I just added this to imx8mp, but may this option is useful on other platforms too?
plat/imx/imx8m/imx8mp/imx8mp_bl31_setup.c | 4 ++++ plat/imx/imx8m/imx8mp/platform.mk | 3 +++ 2 files changed, 7 insertions(+)
diff --git a/plat/imx/imx8m/imx8mp/imx8mp_bl31_setup.c b/plat/imx/imx8m/imx8mp/imx8mp_bl31_setup.c index 22fbd5e4b..d75143270 100644 --- a/plat/imx/imx8m/imx8mp/imx8mp_bl31_setup.c +++ b/plat/imx/imx8m/imx8mp/imx8mp_bl31_setup.c @@ -95,7 +95,9 @@ static void bl31_tzc380_setup(void) void bl31_early_platform_setup2(u_register_t arg0, u_register_t arg1, u_register_t arg2, u_register_t arg3) { +#if (IMX_BOOT_SILENT == 0) static console_t console; +#endif unsigned int i; /* Enable CSU NS access permission */ @@ -109,10 +111,12 @@ void bl31_early_platform_setup2(u_register_t arg0, u_register_t arg1, imx8m_caam_init(); +#if (IMX_BOOT_SILENT == 0) console_imx_uart_register(IMX_BOOT_UART_BASE, IMX_BOOT_UART_CLK_IN_HZ, IMX_CONSOLE_BAUDRATE, &console); /* This console is only used for boot stage */ console_set_scope(&console, CONSOLE_FLAG_BOOT); +#endif /* * tell BL3-1 where the non-secure software image is located diff --git a/plat/imx/imx8m/imx8mp/platform.mk b/plat/imx/imx8m/imx8mp/platform.mk index 1d11e3df4..471c96e70 100644 --- a/plat/imx/imx8m/imx8mp/platform.mk +++ b/plat/imx/imx8m/imx8mp/platform.mk @@ -54,3 +54,6 @@ $(eval $(call add_define,BL32_SIZE)) IMX_BOOT_UART_BASE ?= 0x30890000 $(eval $(call add_define,IMX_BOOT_UART_BASE))
+IMX_BOOT_SILENT ?= 0 +$(eval $(call add_define,IMX_BOOT_SILENT))