[PATCH 1/2] configs: sandbox: activate DEBUG_UART

Add CONFIG_DEBUG_UART=y for all sandbox defconfig as it is already done in sandbox_defconfig.
Signed-off-by: Patrick Delaunay patrick.delaunay@st.com ---
configs/sandbox64_defconfig | 1 + configs/sandbox_flattree_defconfig | 1 + configs/sandbox_spl_defconfig | 1 + 3 files changed, 3 insertions(+)
diff --git a/configs/sandbox64_defconfig b/configs/sandbox64_defconfig index dc993cd13a..3bcf472a47 100644 --- a/configs/sandbox64_defconfig +++ b/configs/sandbox64_defconfig @@ -5,6 +5,7 @@ CONFIG_PRE_CON_BUF_ADDR=0x100000 CONFIG_BOOTSTAGE_STASH_ADDR=0x0 CONFIG_DEFAULT_DEVICE_TREE="sandbox64" CONFIG_SANDBOX64=y +CONFIG_DEBUG_UART=y CONFIG_DISTRO_DEFAULTS=y CONFIG_FIT=y CONFIG_FIT_SIGNATURE=y diff --git a/configs/sandbox_flattree_defconfig b/configs/sandbox_flattree_defconfig index 1f593eba8f..4ac72aa5ce 100644 --- a/configs/sandbox_flattree_defconfig +++ b/configs/sandbox_flattree_defconfig @@ -3,6 +3,7 @@ CONFIG_NR_DRAM_BANKS=1 CONFIG_ENV_SIZE=0x2000 CONFIG_BOOTSTAGE_STASH_ADDR=0x0 CONFIG_DEFAULT_DEVICE_TREE="sandbox" +CONFIG_DEBUG_UART=y CONFIG_DISTRO_DEFAULTS=y CONFIG_FIT=y CONFIG_FIT_SIGNATURE=y diff --git a/configs/sandbox_spl_defconfig b/configs/sandbox_spl_defconfig index 1ac843e1bc..e3b435837b 100644 --- a/configs/sandbox_spl_defconfig +++ b/configs/sandbox_spl_defconfig @@ -9,6 +9,7 @@ CONFIG_SPL=y CONFIG_BOOTSTAGE_STASH_ADDR=0x0 CONFIG_DEFAULT_DEVICE_TREE="sandbox" CONFIG_SANDBOX_SPL=y +CONFIG_DEBUG_UART=y CONFIG_DISTRO_DEFAULTS=y CONFIG_FIT=y CONFIG_FIT_SIGNATURE=y

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(-)
diff --git a/common/console.c b/common/console.c index 3348436da6..11d3694159 100644 --- a/common/console.c +++ b/common/console.c @@ -13,7 +13,6 @@ #include <iomux.h> #include <malloc.h> #include <mapmem.h> -#include <os.h> #include <serial.h> #include <stdio_dev.h> #include <exports.h> @@ -517,13 +516,6 @@ static inline void print_pre_console_buffer(int flushpoint) {}
void putc(const char c) { -#ifdef CONFIG_SANDBOX - /* sandbox can send characters to stdout before it has a console */ - if (!gd || !(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 || !(gd->flags & GD_FLG_SERIAL_READY)) { @@ -565,13 +557,6 @@ void putc(const char c)
void puts(const char *s) { -#ifdef CONFIG_SANDBOX - /* sandbox can send characters to stdout before it has a console */ - if (!gd || !(gd->flags & GD_FLG_SERIAL_READY)) { - os_puts(s); - return; - } -#endif #ifdef CONFIG_DEBUG_UART if (!gd || !(gd->flags & GD_FLG_SERIAL_READY)) { while (*s) {

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

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
Regards, Patrick

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.
Regards, Simon

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_SANDBOX
by
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
Regards,
Patrick

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_SANDBOX
by
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

On Fri, 27 Nov 2020 at 03:49, Patrick Delaunay patrick.delaunay@st.com wrote:
Add CONFIG_DEBUG_UART=y for all sandbox defconfig as it is already done in sandbox_defconfig.
Signed-off-by: Patrick Delaunay patrick.delaunay@st.com
configs/sandbox64_defconfig | 1 + configs/sandbox_flattree_defconfig | 1 + configs/sandbox_spl_defconfig | 1 + 3 files changed, 3 insertions(+)
Reviewed-by: Simon Glass sjg@chromium.org

On Fri, 27 Nov 2020 at 03:49, Patrick Delaunay patrick.delaunay@st.com wrote:
Add CONFIG_DEBUG_UART=y for all sandbox defconfig as it is already done in sandbox_defconfig.
Signed-off-by: Patrick Delaunay patrick.delaunay@st.com
configs/sandbox64_defconfig | 1 + configs/sandbox_flattree_defconfig | 1 + configs/sandbox_spl_defconfig | 1 + 3 files changed, 3 insertions(+)
Reviewed-by: Simon Glass sjg@chromium.org
Applied to u-boot-dm/next, thanks!
participants (4)
-
Patrick DELAUNAY
-
Patrick DELAUNAY
-
Patrick Delaunay
-
Simon Glass