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))