Hi Patrick,
On Tue, 15 Dec 2020 at 07:35, Patrick DELAUNAY patrick.delaunay@foss.st.com wrote:
Hi Simon,
On 12/12/20 4:39 PM, Simon Glass wrote:
Hi Patrick,
On Wed, 2 Dec 2020 at 07:08, Patrick DELAUNAY patrick.delaunay@st.com wrote:
Hi Simon,
From: Simon Glass sjg@chromium.org Sent: lundi 30 novembre 2020 21:12
Hi Patrick,
On Fri, 27 Nov 2020 at 03:49, Patrick Delaunay patrick.delaunay@st.com wrote:
Remove the specific sandbox code in console.c, as the config CONFIG_DEBUG_UART is already supported in drivers/serial/sandbox.c and activated by default in all sandbox defconfig (CONFIG_DEBUG_UART_SANDBOX=y and CONFIG_DEBUG_UART=y).
This patch allows to test the console code under DEBUG_UART in sandbox and avoids to include the file <os.h> in this u-boot generic code.
Signed-off-by: Patrick Delaunay patrick.delaunay@st.com
common/console.c | 15 --------------- 1 file changed, 15 deletions(-)
Please see this commit as to why I put that code back, after removing it myself.
64e9b4f346f Revert "sandbox: Drop special case console code for sandbox"
Regards, Simon
Thanks to point it, I miss this old commit.
I don't understood the issue in the commit message:
Revert "sandbox: Drop special case console code for sandbox" While sandbox works OK without the special-case code, it does result in console output being stored in the pre-console buffer while sandbox starts up. If there is a crash or a problem then there is no indication of what is going on. For ease of debugging it seems better to revert this change also.the existing code (here for putc, but it is the same for puts) is:
#ifdef CONFIG_SANDBOX /* sandbox can send characters to stdout before it has a console */ if (!(gd->flags & GD_FLG_SERIAL_READY)) { os_putc(c); return; } #endif #ifdef CONFIG_DEBUG_UART /* if we don't have a console yet, use the debug UART */ if (!(gd->flags & GD_FLG_SERIAL_READY)) { printch(c); return; } #endif
For sandbox, when CONFIG_DEBUG_UART is activated printch => _printch => _debug_uart_putc => os_putc
For me these 2 code block are identical for sandbox when CONFIG_DEBUG_UART
And the issue described is also solved by CONFIG_DEBUG_UART=y (consle no use preconsole buffer when serial driver s not ready).
Your concern is when sandbox is compiled without CONFIG_DEBUG_UART ?
Because it is no more the case with my previous patch (I activate it in sandbox*defconfig)
but to avoid issue in futur (new sandbox*defconfig) it should be better to select (or imply) his feature for sandbox arch in Kconfig and not more activate it in sandbox*defconfig ?
PS: with this sandox code, I don't see how to test the pre console buffer in sandbox... I think that the pre console buffer is alway empty for sandbox
OK maybe things have changed. Previously I noticed that the banner did not output until later. I will take another look.
But I don't want to rely on the debug UART for sandbox to work.
Ok, it is not problem for me (avoid assumption on CONFIG_DEBUG_UART for sandbox)
you can consider that I abandon this change.
I will only replace
#ifdef CONFIG_SANDBOXby
if (IS_ACTIVATED(CONFIG_SANDBOX))in the other console the cleanup serie "console: remove #ifdef CONFIG when it is possible"
http://patchwork.ozlabs.org/project/uboot/list/?series=218309
OK good.
Just to close the loop, if I disable the debug UART, and put a printf() in a suitable function before console_init_f() - e.g.initf_bootstage()
Then print_pre_console_buffer() prints out the text, but it does not appear before that. I consider that confusing/annoying when using gdb with sandbox, which I why I reverted my original patch.
I am not a fan of special-case sandbox code, and there is probably a nicer way of doing it, but I do think having this special case is worth it.
Regards, Simon