diff mbox series

[2/2] serial: introduce CONFIG_CONSOLE_FLUSH_ON_NEWLINE

Message ID 20230925110101.376878-3-rasmus.villemoes@prevas.dk
State Superseded
Delegated to: Tom Rini
Headers show
Series serial: introduce CONFIG_CONSOLE_FLUSH_ON_NEWLINE | expand

Commit Message

Rasmus Villemoes Sept. 25, 2023, 11:01 a.m. UTC
When debugging, one sometimes only gets partial output lines or
nothing at all from the last printf, because the uart has a largish
buffer, and the code after the printf() may cause the CPU to hang
before the uart IP has time to actually emit all the characters. That
can be very confusing, because one doesn't then know exactly where the
hang happens.

Introduce a config knob allowing one to wait for the uart fifo to
drain whenever a newline character is printed, roughly corresponding
to the effect of setvbuf(..., _IOLBF, ...) in ordinary C programs.

Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
---
 common/Kconfig                 | 11 +++++++++++
 drivers/serial/serial-uclass.c |  8 ++++++--
 2 files changed, 17 insertions(+), 2 deletions(-)

Comments

Michal Suchánek Sept. 25, 2023, 12:10 p.m. UTC | #1
Hello,

On Mon, Sep 25, 2023 at 01:01:01PM +0200, Rasmus Villemoes wrote:
> When debugging, one sometimes only gets partial output lines or
> nothing at all from the last printf, because the uart has a largish
> buffer, and the code after the printf() may cause the CPU to hang
> before the uart IP has time to actually emit all the characters. That
> can be very confusing, because one doesn't then know exactly where the
> hang happens.
> 
> Introduce a config knob allowing one to wait for the uart fifo to
> drain whenever a newline character is printed, roughly corresponding
> to the effect of setvbuf(..., _IOLBF, ...) in ordinary C programs.
> 
> Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
> ---
>  common/Kconfig                 | 11 +++++++++++
>  drivers/serial/serial-uclass.c |  8 ++++++--
>  2 files changed, 17 insertions(+), 2 deletions(-)
> 
> diff --git a/common/Kconfig b/common/Kconfig
> index cdb77a6a7d..49130f2a2b 100644
> --- a/common/Kconfig
> +++ b/common/Kconfig
> @@ -224,6 +224,17 @@ config CONSOLE_FLUSH_SUPPORT
>  	help
>  	  This enables compilation of flush() function for console flush support.
>  
> +config CONSOLE_FLUSH_ON_NEWLINE
> +	bool "Flush console buffer on every newline character"
> +	depends on CONSOLE_FLUSH_SUPPORT

Maybe selects would be more appropriate here if it can be arbitrarily
enabled on any console

> +	depends on DM_SERIAL
> +	help
> +	  This makes the serial core code flush the console device
> +	  whenever a newline (\n) character has been emitted. This can
> +	  be especially useful when "printf debugging", as otherwise
> +	  lots of output could still be in the UART's FIFO by the time
> +	  one hits the code which causes the CPU to hang or reset.
> +
>  config CONSOLE_MUX
>  	bool "Enable console multiplexing"
>  	default y if VIDEO || LCD
> diff --git a/drivers/serial/serial-uclass.c b/drivers/serial/serial-uclass.c
> index 2f75ff878c..b41e3ebe7f 100644
> --- a/drivers/serial/serial-uclass.c
> +++ b/drivers/serial/serial-uclass.c
> @@ -181,7 +181,6 @@ int serial_initialize(void)
>  	return serial_init();
>  }
>  
> -#ifdef CONFIG_CONSOLE_FLUSH_SUPPORT

This should not be removed. CONSOLE_FLUSH_ON_NEWLINE will only ever be
configured when CONFIG_CONSOLE_FLUSH_SUPPORT is also enabled so there is
no need to remove, either.

Thanks

Michal

>  static void _serial_flush(struct udevice *dev)
>  {
>  	struct dm_serial_ops *ops = serial_get_ops(dev);
> @@ -191,7 +190,6 @@ static void _serial_flush(struct udevice *dev)
>  	while (ops->pending(dev, false) > 0)
>  		;
>  }
> -#endif
>  
>  static void _serial_putc(struct udevice *dev, char ch)
>  {
> @@ -204,6 +202,9 @@ static void _serial_putc(struct udevice *dev, char ch)
>  	do {
>  		err = ops->putc(dev, ch);
>  	} while (err == -EAGAIN);
> +
> +	if (IS_ENABLED(CONSOLE_FLUSH_ON_NEWLINE) && ch == '\n')
> +		_serial_flush(dev);
>  }
>  
>  static int __serial_puts(struct udevice *dev, const char *str, size_t len)
> @@ -242,6 +243,9 @@ static void _serial_puts(struct udevice *dev, const char *str)
>  		if (*newline && __serial_puts(dev, "\r\n", 2))
>  			return;
>  
> +		if (IS_ENABLED(CONSOLE_FLUSH_ON_NEWLINE) && *newline)
> +			_serial_flush(dev);
> +
>  		str += len + !!*newline;
>  	} while (*str);
>  }
> -- 
> 2.37.2
>
diff mbox series

Patch

diff --git a/common/Kconfig b/common/Kconfig
index cdb77a6a7d..49130f2a2b 100644
--- a/common/Kconfig
+++ b/common/Kconfig
@@ -224,6 +224,17 @@  config CONSOLE_FLUSH_SUPPORT
 	help
 	  This enables compilation of flush() function for console flush support.
 
+config CONSOLE_FLUSH_ON_NEWLINE
+	bool "Flush console buffer on every newline character"
+	depends on CONSOLE_FLUSH_SUPPORT
+	depends on DM_SERIAL
+	help
+	  This makes the serial core code flush the console device
+	  whenever a newline (\n) character has been emitted. This can
+	  be especially useful when "printf debugging", as otherwise
+	  lots of output could still be in the UART's FIFO by the time
+	  one hits the code which causes the CPU to hang or reset.
+
 config CONSOLE_MUX
 	bool "Enable console multiplexing"
 	default y if VIDEO || LCD
diff --git a/drivers/serial/serial-uclass.c b/drivers/serial/serial-uclass.c
index 2f75ff878c..b41e3ebe7f 100644
--- a/drivers/serial/serial-uclass.c
+++ b/drivers/serial/serial-uclass.c
@@ -181,7 +181,6 @@  int serial_initialize(void)
 	return serial_init();
 }
 
-#ifdef CONFIG_CONSOLE_FLUSH_SUPPORT
 static void _serial_flush(struct udevice *dev)
 {
 	struct dm_serial_ops *ops = serial_get_ops(dev);
@@ -191,7 +190,6 @@  static void _serial_flush(struct udevice *dev)
 	while (ops->pending(dev, false) > 0)
 		;
 }
-#endif
 
 static void _serial_putc(struct udevice *dev, char ch)
 {
@@ -204,6 +202,9 @@  static void _serial_putc(struct udevice *dev, char ch)
 	do {
 		err = ops->putc(dev, ch);
 	} while (err == -EAGAIN);
+
+	if (IS_ENABLED(CONSOLE_FLUSH_ON_NEWLINE) && ch == '\n')
+		_serial_flush(dev);
 }
 
 static int __serial_puts(struct udevice *dev, const char *str, size_t len)
@@ -242,6 +243,9 @@  static void _serial_puts(struct udevice *dev, const char *str)
 		if (*newline && __serial_puts(dev, "\r\n", 2))
 			return;
 
+		if (IS_ENABLED(CONSOLE_FLUSH_ON_NEWLINE) && *newline)
+			_serial_flush(dev);
+
 		str += len + !!*newline;
 	} while (*str);
 }