[PATCH] common/console.c: prevent pre-console buffer contents from being added to itself

[Rasmus Villemoes]

Ping.

The previous patch has already been applied [cff29636933a,
common/console.c: use CONFIG_VAL() with PRE_CON_BUF_* variables], and I
realize this one might be more "controversial" in how it makes
gd->precon_buf_idx serve as a "flag" when negative, but I'd really like
to make progress towards making pre-console-buffering usable in SPL
(since I'm debugging yet another issue where I need to know what
happened very early).

On 03/05/2022 15.13, Rasmus Villemoes wrote:
> I do not have any non-serial output devices, so a
> print_pre_console_buffer(PRE_CONSOLE_FLUSHPOINT2_EVERYTHING_BUT_SERIAL)
> does nothing for me.
> 
> However, I was manually inspected the pre-console buffer using md.b,
> and I noticed that the early part of it was repeated. The reason is
> that the first call of print_pre_console_buffer(), from
> console_init_f(), ends up invoking puts() with the contents of the
> buffer at that point, and puts() at that point ends up in the else
> branch of
> 
> 	if (gd->flags & GD_FLG_DEVINIT) {
> 		/* Send to the standard output */
> 		fputs(stdout, s);
> 	} else {
> 		/* Send directly to the handler */
> 		pre_console_puts(s);
> 		serial_puts(s);
> 	}
> 
> so indeed the contents is added again.
> 
> That can be somewhat confusing (both when reading the buffer manually,
> but also if it did actually come out on some device). So disable all
> use of the pre-console buffer while print_pre_console_buffer() is
> emitting it.
> 
> Signed-off-by: Rasmus Villemoes <rasmus.villemoes at prevas.dk>
> ---
>  common/console.c                  | 10 +++++++++-
>  include/asm-generic/global_data.h | 12 ++++++++----
>  2 files changed, 17 insertions(+), 5 deletions(-)
> 
> diff --git a/common/console.c b/common/console.c
> index dc071f1ed6..c5a72d9a2a 100644
> --- a/common/console.c
> +++ b/common/console.c
> @@ -599,6 +599,9 @@ static void pre_console_putc(const char c)
>  {
>  	char *buffer;
>  
> +	if (gd->precon_buf_idx < 0)
> +		return;
> +
>  	buffer = map_sysmem(CONFIG_VAL(PRE_CON_BUF_ADDR), CONFIG_VAL(PRE_CON_BUF_SZ));
>  
>  	buffer[CIRC_BUF_IDX(gd->precon_buf_idx++)] = c;
> @@ -608,13 +611,16 @@ static void pre_console_putc(const char c)
>  
>  static void pre_console_puts(const char *s)
>  {
> +	if (gd->precon_buf_idx < 0)
> +		return;
> +
>  	while (*s)
>  		pre_console_putc(*s++);
>  }
>  
>  static void print_pre_console_buffer(int flushpoint)
>  {
> -	unsigned long in = 0, out = 0;
> +	long in = 0, out = 0;
>  	char buf_out[CONFIG_VAL(PRE_CON_BUF_SZ) + 1];
>  	char *buf_in;
>  
> @@ -631,6 +637,7 @@ static void print_pre_console_buffer(int flushpoint)
>  
>  	buf_out[out] = 0;
>  
> +	gd->precon_buf_idx = -1;
>  	switch (flushpoint) {
>  	case PRE_CONSOLE_FLUSHPOINT1_SERIAL:
>  		puts(buf_out);
> @@ -639,6 +646,7 @@ static void print_pre_console_buffer(int flushpoint)
>  		console_puts_select(stdout, false, buf_out);
>  		break;
>  	}
> +	gd->precon_buf_idx = in;
>  }
>  #else
>  static inline void pre_console_putc(const char c) {}
> diff --git a/include/asm-generic/global_data.h b/include/asm-generic/global_data.h
> index 805a6fd679..2a747d91e1 100644
> --- a/include/asm-generic/global_data.h
> +++ b/include/asm-generic/global_data.h
> @@ -115,10 +115,14 @@ struct global_data {
>  	/**
>  	 * @precon_buf_idx: pre-console buffer index
>  	 *
> -	 * @precon_buf_idx indicates the current position of the buffer used to
> -	 * collect output before the console becomes available
> -	 */
> -	unsigned long precon_buf_idx;
> +	 * @precon_buf_idx indicates the current position of the
> +	 * buffer used to collect output before the console becomes
> +	 * available. When negative, the pre-console buffer is
> +	 * temporarily disabled (used when the pre-console buffer is
> +	 * being written out, to prevent adding its contents to
> +	 * itself).
> +	 */
> +	long precon_buf_idx;
>  #endif
>  	/**
>  	 * @env_addr: address of environment structure