[Uboot-stm32] [PATCH 2/2] console: sandbox: remove unnecessary sandbox code

[Patrick DELAUNAY]

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 at st.com> wrote:
>> Hi Simon,
>>
>>> From: Simon Glass <sjg at chromium.org>
>>> Sent: lundi 30 novembre 2020 21:12
>>>
>>> Hi Patrick,
>>>
>>> On Fri, 27 Nov 2020 at 03:49, Patrick Delaunay <patrick.delaunay at 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 at 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