[PATCH v3 0/6] console: Implement flush() function

[PATCH v3 0/6] console: Implement flush() function

[Pali Rohár]

On certain places it is required to flush output print buffers to ensure
that text strings were sent to console or serial devices. For example when
printing message that U-Boot is going to boot kernel or when U-Boot is
going to change baudrate of terminal device.

Some console devices, like UART, have putc/puts functions which just put
characters into HW transmit queue and do not wait until all data are
transmitted. Doing some sensitive operations (like changing baudrate or
starting kernel which resets UART HW) cause that U-Boot messages are lost.

Therefore introduce a new flush() function, implement it for all serial
devices via pending(false) callback and use this new flush() function on
sensitive places after which output device may go into reset state.

This change fixes printing of U-Boot messages:
"## Starting application at ..."
"## Switch baudrate to ..."

Changes in v3:
* add macro STDIO_DEV_ASSIGN_FLUSH()
* fix commit messages
* remove changes from serial.c

Changes in v2:
* split one big patch into smaller 6 patches
* add config option to allow disabling this new function

Pali Rohár (6):
  sandbox: Add function os_flush()
  console: Implement flush() function
  serial: Implement flush callback
  serial: Implement serial_flush() function for console flush() fallback
  serial: Call flush() before changing baudrate
  boot: Call flush() before booting

 arch/sandbox/cpu/os.c          |  5 +++
 boot/bootm_os.c                |  1 +
 cmd/boot.c                     |  1 +
 cmd/elf.c                      |  2 ++
 cmd/load.c                     |  5 +++
 common/Kconfig                 |  6 ++++
 common/console.c               | 64 ++++++++++++++++++++++++++++++++++
 common/stdio.c                 |  8 +++++
 drivers/serial/serial-uclass.c | 31 ++++++++++++++++
 include/_exports.h             |  3 ++
 include/os.h                   |  8 +++++
 include/serial.h               |  5 +++
 include/stdio.h                | 15 ++++++++
 include/stdio_dev.h            |  7 ++++
 14 files changed, 161 insertions(+)

-- 
2.20.1

[PATCH v3 1/6] sandbox: Add function os_flush()

[Pali Rohár]

It flushes stdout.

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 arch/sandbox/cpu/os.c | 5 +++++
 include/os.h          | 8 ++++++++
 2 files changed, 13 insertions(+)

diff --git a/arch/sandbox/cpu/os.c b/arch/sandbox/cpu/os.c
index f937991139c9..01845e388d35 100644
--- a/arch/sandbox/cpu/os.c
+++ b/arch/sandbox/cpu/os.c
@@ -669,6 +669,11 @@ void os_puts(const char *str)
 		os_putc(*str++);
 }
 
+void os_flush(void)
+{
+	fflush(stdout);
+}
+
 int os_write_ram_buf(const char *fname)
 {
 	struct sandbox_state *state = state_get_current();
diff --git a/include/os.h b/include/os.h
index 148178787bc2..5b353ae9d94b 100644
--- a/include/os.h
+++ b/include/os.h
@@ -295,6 +295,14 @@ void os_putc(int ch);
  */
 void os_puts(const char *str);
 
+/**
+ * os_flush() - flush controlling OS terminal
+ *
+ * This bypasses the U-Boot console support and flushes directly the OS
+ * stdout file descriptor.
+ */
+void os_flush(void);
+
 /**
  * os_write_ram_buf() - write the sandbox RAM buffer to a existing file
  *
-- 
2.20.1

[PATCH v3 2/6] console: Implement flush() function

[Pali Rohár]

On certain places it is required to flush output print buffers to ensure
that text strings were sent to console or serial devices. For example when
printing message that U-Boot is going to boot kernel or when U-Boot is
going to change baudrate of terminal device.

Therefore introduce a new flush() and fflush() functions into console code.
These functions will call .flush callback of associated stdio_dev device.

As this function may increase U-Boot side, allow to compile U-Boot without
this function. For this purpose there is a new config CONSOLE_FLUSH_SUPPORT
which is enabled by default and can be disabled. It is a good idea to have
this option enabled for all boards which have enough space for it.

When option is disabled when U-Boot defines just empty static inline
function fflush() to avoid ifdefs in other code.

Signed-off-by: Pali Rohár <pali@kernel.org>
---
Changes in v3:
* Added macro STDIO_DEV_ASSIGN_FLUSH()
---
 common/Kconfig      |  6 +++++
 common/console.c    | 61 +++++++++++++++++++++++++++++++++++++++++++++
 include/_exports.h  |  3 +++
 include/stdio.h     | 15 +++++++++++
 include/stdio_dev.h |  7 ++++++
 5 files changed, 92 insertions(+)

diff --git a/common/Kconfig b/common/Kconfig
index e7914ca750a3..109741cc6c44 100644
--- a/common/Kconfig
+++ b/common/Kconfig
@@ -186,6 +186,12 @@ config PRE_CON_BUF_ADDR
 	  We should consider removing this option and allocating the memory
 	  in board_init_f_init_reserve() instead.
 
+config CONSOLE_FLUSH_SUPPORT
+	bool "Enable console flush support"
+	default y
+	help
+	  This enables compilation of flush() function for console flush support.
+
 config CONSOLE_MUX
 	bool "Enable console multiplexing"
 	default y if DM_VIDEO || VIDEO || LCD
diff --git a/common/console.c b/common/console.c
index e783f309bf06..0abfc224b53b 100644
--- a/common/console.c
+++ b/common/console.c
@@ -199,6 +199,7 @@ static int console_setfile(int file, struct stdio_dev * dev)
 		case stdout:
 			gd->jt->putc  = putc;
 			gd->jt->puts  = puts;
+			STDIO_DEV_ASSIGN_FLUSH(gd->jt, flush);
 			gd->jt->printf = printf;
 			break;
 		}
@@ -364,6 +365,19 @@ static void console_puts(int file, const char *s)
 	}
 }
 
+#ifdef CONFIG_CONSOLE_FLUSH_SUPPORT
+static void console_flush(int file)
+{
+	int i;
+	struct stdio_dev *dev;
+
+	for_each_console_dev(i, file, dev) {
+		if (dev->flush != NULL)
+			dev->flush(dev);
+	}
+}
+#endif
+
 #if CONFIG_IS_ENABLED(SYS_CONSOLE_IS_IN_ENV)
 static inline void console_doenv(int file, struct stdio_dev *dev)
 {
@@ -413,6 +427,14 @@ static inline void console_puts(int file, const char *s)
 	stdio_devices[file]->puts(stdio_devices[file], s);
 }
 
+#ifdef CONFIG_CONSOLE_FLUSH_SUPPORT
+static inline void console_flush(int file)
+{
+	if (stdio_devices[file]->flush)
+		stdio_devices[file]->flush(stdio_devices[file]);
+}
+#endif
+
 #if CONFIG_IS_ENABLED(SYS_CONSOLE_IS_IN_ENV)
 static inline void console_doenv(int file, struct stdio_dev *dev)
 {
@@ -526,6 +548,14 @@ void fputs(int file, const char *s)
 		console_puts(file, s);
 }
 
+#ifdef CONFIG_CONSOLE_FLUSH_SUPPORT
+void fflush(int file)
+{
+	if (file < MAX_FILES)
+		console_flush(file);
+}
+#endif
+
 int fprintf(int file, const char *fmt, ...)
 {
 	va_list args;
@@ -740,6 +770,37 @@ void puts(const char *s)
 	}
 }
 
+#ifdef CONFIG_CONSOLE_FLUSH_SUPPORT
+void flush(void)
+{
+	if (!gd)
+		return;
+
+	/* sandbox can send characters to stdout before it has a console */
+	if (IS_ENABLED(CONFIG_SANDBOX) && !(gd->flags & GD_FLG_SERIAL_READY)) {
+		os_flush();
+		return;
+	}
+
+	if (IS_ENABLED(CONFIG_DEBUG_UART) && !(gd->flags & GD_FLG_SERIAL_READY))
+		return;
+
+	if (IS_ENABLED(CONFIG_SILENT_CONSOLE) && (gd->flags & GD_FLG_SILENT))
+		return;
+
+	if (IS_ENABLED(CONFIG_DISABLE_CONSOLE) && (gd->flags & GD_FLG_DISABLE_CONSOLE))
+		return;
+
+	if (!gd->have_console)
+		return;
+
+	if (gd->flags & GD_FLG_DEVINIT) {
+		/* Send to the standard output */
+		fflush(stdout);
+	}
+}
+#endif
+
 #ifdef CONFIG_CONSOLE_RECORD
 int console_record_init(void)
 {
diff --git a/include/_exports.h b/include/_exports.h
index f6df8b610734..1af946fac327 100644
--- a/include/_exports.h
+++ b/include/_exports.h
@@ -12,6 +12,9 @@
 	EXPORT_FUNC(tstc, int, tstc, void)
 	EXPORT_FUNC(putc, void, putc, const char)
 	EXPORT_FUNC(puts, void, puts, const char *)
+#ifdef CONFIG_CONSOLE_FLUSH_SUPPORT
+	EXPORT_FUNC(flush, void, flush, void)
+#endif
 	EXPORT_FUNC(printf, int, printf, const char*, ...)
 #if (defined(CONFIG_X86) && !defined(CONFIG_X86_64)) || defined(CONFIG_PPC)
 	EXPORT_FUNC(irq_install_handler, void, install_hdlr,
diff --git a/include/stdio.h b/include/stdio.h
index 1939a48f0fb6..3241e2d493fa 100644
--- a/include/stdio.h
+++ b/include/stdio.h
@@ -15,6 +15,11 @@ int tstc(void);
 		defined(CONFIG_SPL_SERIAL))
 void putc(const char c);
 void puts(const char *s);
+#ifdef CONFIG_CONSOLE_FLUSH_SUPPORT
+void flush(void);
+#else
+static inline void flush(void) {}
+#endif
 int __printf(1, 2) printf(const char *fmt, ...);
 int vprintf(const char *fmt, va_list args);
 #else
@@ -26,6 +31,10 @@ static inline void puts(const char *s)
 {
 }
 
+static inline void flush(void)
+{
+}
+
 static inline int __printf(1, 2) printf(const char *fmt, ...)
 {
 	return 0;
@@ -48,11 +57,17 @@ static inline int vprintf(const char *fmt, va_list args)
 /* stderr */
 #define eputc(c)		fputc(stderr, c)
 #define eputs(s)		fputs(stderr, s)
+#define eflush()		fflush(stderr)
 #define eprintf(fmt, args...)	fprintf(stderr, fmt, ##args)
 
 int __printf(2, 3) fprintf(int file, const char *fmt, ...);
 void fputs(int file, const char *s);
 void fputc(int file, const char c);
+#ifdef CONFIG_CONSOLE_FLUSH_SUPPORT
+void fflush(int file);
+#else
+static inline void fflush(int file) {}
+#endif
 int ftstc(int file);
 int fgetc(int file);
 
diff --git a/include/stdio_dev.h b/include/stdio_dev.h
index 270fa2729fb2..3105928970db 100644
--- a/include/stdio_dev.h
+++ b/include/stdio_dev.h
@@ -37,6 +37,13 @@ struct stdio_dev {
 	void (*putc)(struct stdio_dev *dev, const char c);
 	/* To put a string (accelerator) */
 	void (*puts)(struct stdio_dev *dev, const char *s);
+#ifdef CONFIG_CONSOLE_FLUSH_SUPPORT
+	/* To flush output queue */
+	void (*flush)(struct stdio_dev *dev);
+#define STDIO_DEV_ASSIGN_FLUSH(dev, flush_func) ((dev)->flush = (flush_func))
+#else
+#define STDIO_DEV_ASSIGN_FLUSH(dev, flush_func)
+#endif
 
 /* INPUT functions */
 
-- 
2.20.1

[PATCH v3 3/6] serial: Implement flush callback

[Pali Rohár]

UART drivers have putc/puts functions which just put characters into HW
transmit queue and do not wait until all data are transmitted.

Implement flush callback via serial driver's pending(false) callback which
waits until HW transmit all characters from the queue.

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 drivers/serial/serial-uclass.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/drivers/serial/serial-uclass.c b/drivers/serial/serial-uclass.c
index 30650e37b0d7..be6502f3d24c 100644
--- a/drivers/serial/serial-uclass.c
+++ b/drivers/serial/serial-uclass.c
@@ -238,6 +238,18 @@ static void _serial_puts(struct udevice *dev, const char *str)
 	} while (*str);
 }
 
+#ifdef CONFIG_CONSOLE_FLUSH_SUPPORT
+static void _serial_flush(struct udevice *dev)
+{
+	struct dm_serial_ops *ops = serial_get_ops(dev);
+
+	if (!ops->pending)
+		return;
+	while (ops->pending(dev, false) > 0)
+		;
+}
+#endif
+
 static int __serial_getc(struct udevice *dev)
 {
 	struct dm_serial_ops *ops = serial_get_ops(dev);
@@ -398,6 +410,13 @@ static void serial_stub_puts(struct stdio_dev *sdev, const char *str)
 	_serial_puts(sdev->priv, str);
 }
 
+#ifdef CONFIG_CONSOLE_FLUSH_SUPPORT
+static void serial_stub_flush(struct stdio_dev *sdev)
+{
+	_serial_flush(sdev->priv);
+}
+#endif
+
 static int serial_stub_getc(struct stdio_dev *sdev)
 {
 	return _serial_getc(sdev->priv);
@@ -520,6 +539,7 @@ static int serial_post_probe(struct udevice *dev)
 	sdev.priv = dev;
 	sdev.putc = serial_stub_putc;
 	sdev.puts = serial_stub_puts;
+	STDIO_DEV_ASSIGN_FLUSH(&sdev, serial_stub_flush);
 	sdev.getc = serial_stub_getc;
 	sdev.tstc = serial_stub_tstc;
 
-- 
2.20.1

[PATCH v3 4/6] serial: Implement serial_flush() function for console flush() fallback

[Pali Rohár]

Like in all other console functions, implement also serial_flush() function
as a fallback int console flush() function.

Flush support is available only when config option CONSOLE_FLUSH_SUPPORT is
enabled. So when it is disabled then provides just empty static inline
function serial_flush().

Signed-off-by: Pali Rohár <pali@kernel.org>
---
 common/console.c               |  3 +++
 common/stdio.c                 |  8 ++++++++
 drivers/serial/serial-uclass.c | 10 ++++++++++
 include/serial.h               |  5 +++++
 4 files changed, 26 insertions(+)

diff --git a/common/console.c b/common/console.c
index 0abfc224b53b..e4dc1da44a35 100644
--- a/common/console.c
+++ b/common/console.c
@@ -797,6 +797,9 @@ void flush(void)
 	if (gd->flags & GD_FLG_DEVINIT) {
 		/* Send to the standard output */
 		fflush(stdout);
+	} else {
+		/* Send directly to the handler */
+		serial_flush();
 	}
 }
 #endif
diff --git a/common/stdio.c b/common/stdio.c
index 92161a0df87d..13083842cbd9 100644
--- a/common/stdio.c
+++ b/common/stdio.c
@@ -87,6 +87,13 @@ static void stdio_serial_puts(struct stdio_dev *dev, const char *s)
 	serial_puts(s);
 }
 
+#ifdef CONFIG_CONSOLE_FLUSH_SUPPORT
+static void stdio_serial_flush(struct stdio_dev *dev)
+{
+	serial_flush();
+}
+#endif
+
 static int stdio_serial_getc(struct stdio_dev *dev)
 {
 	return serial_getc();
@@ -112,6 +119,7 @@ static void drv_system_init (void)
 	dev.flags = DEV_FLAGS_OUTPUT | DEV_FLAGS_INPUT;
 	dev.putc = stdio_serial_putc;
 	dev.puts = stdio_serial_puts;
+	STDIO_DEV_ASSIGN_FLUSH(&dev, stdio_serial_flush);
 	dev.getc = stdio_serial_getc;
 	dev.tstc = stdio_serial_tstc;
 	stdio_register (&dev);
diff --git a/drivers/serial/serial-uclass.c b/drivers/serial/serial-uclass.c
index be6502f3d24c..f028da0900cd 100644
--- a/drivers/serial/serial-uclass.c
+++ b/drivers/serial/serial-uclass.c
@@ -327,6 +327,16 @@ void serial_puts(const char *str)
 		_serial_puts(gd->cur_serial_dev, str);
 }
 
+#ifdef CONFIG_CONSOLE_FLUSH_SUPPORT
+void serial_flush(void)
+{
+	if (!gd->cur_serial_dev)
+		return;
+
+	_serial_flush(gd->cur_serial_dev);
+}
+#endif
+
 int serial_getc(void)
 {
 	if (!gd->cur_serial_dev)
diff --git a/include/serial.h b/include/serial.h
index 8c2e7adbc321..f9009d4046e3 100644
--- a/include/serial.h
+++ b/include/serial.h
@@ -362,6 +362,11 @@ void serial_setbrg(void);
 void serial_putc(const char ch);
 void serial_putc_raw(const char ch);
 void serial_puts(const char *str);
+#ifdef CONFIG_CONSOLE_FLUSH_SUPPORT
+void serial_flush(void);
+#else
+static inline void serial_flush(void) {}
+#endif
 int serial_getc(void);
 int serial_tstc(void);
 
-- 
2.20.1

[PATCH v3 5/6] serial: Call flush() before changing baudrate

[Pali Rohár]

Changing baudrate is a sensitive operation. To ensure that U-Boot messages
printed before changing baudrate are not lost, call new U-Boot console
flush() function.

Signed-off-by: Pali Rohár <pali@kernel.org>
---
Changes in v3:
* Remove support from serial.c
* Fix commit message
---
 cmd/load.c                     | 5 +++++
 drivers/serial/serial-uclass.c | 1 +
 2 files changed, 6 insertions(+)

diff --git a/cmd/load.c b/cmd/load.c
index e44ae0d56b75..5c4f34781d45 100644
--- a/cmd/load.c
+++ b/cmd/load.c
@@ -83,6 +83,7 @@ static int do_load_serial(struct cmd_tbl *cmdtp, int flag, int argc,
 		printf("## Switch baudrate to %d bps and press ENTER ...\n",
 			load_baudrate);
 		udelay(50000);
+		flush();
 		gd->baudrate = load_baudrate;
 		serial_setbrg();
 		udelay(50000);
@@ -126,6 +127,7 @@ static int do_load_serial(struct cmd_tbl *cmdtp, int flag, int argc,
 		printf("## Switch baudrate to %d bps and press ESC ...\n",
 			current_baudrate);
 		udelay(50000);
+		flush();
 		gd->baudrate = current_baudrate;
 		serial_setbrg();
 		udelay(50000);
@@ -317,6 +319,7 @@ int do_save_serial(struct cmd_tbl *cmdtp, int flag, int argc,
 		printf("## Switch baudrate to %d bps and press ESC ...\n",
 			(int)current_baudrate);
 		udelay(50000);
+		flush();
 		gd->baudrate = current_baudrate;
 		serial_setbrg();
 		udelay(50000);
@@ -471,6 +474,7 @@ static int do_load_serial_bin(struct cmd_tbl *cmdtp, int flag, int argc,
 		printf("## Switch baudrate to %d bps and press ENTER ...\n",
 			load_baudrate);
 		udelay(50000);
+		flush();
 		gd->baudrate = load_baudrate;
 		serial_setbrg();
 		udelay(50000);
@@ -533,6 +537,7 @@ static int do_load_serial_bin(struct cmd_tbl *cmdtp, int flag, int argc,
 		printf("## Switch baudrate to %d bps and press ESC ...\n",
 			current_baudrate);
 		udelay(50000);
+		flush();
 		gd->baudrate = current_baudrate;
 		serial_setbrg();
 		udelay(50000);
diff --git a/drivers/serial/serial-uclass.c b/drivers/serial/serial-uclass.c
index f028da0900cd..04b753c229ab 100644
--- a/drivers/serial/serial-uclass.c
+++ b/drivers/serial/serial-uclass.c
@@ -476,6 +476,7 @@ static int on_baudrate(const char *name, const char *value, enum env_op op,
 			printf("## Switch baudrate to %d bps and press ENTER ...\n",
 			       baudrate);
 			udelay(50000);
+			flush();
 		}
 
 		gd->baudrate = baudrate;
-- 
2.20.1

[PATCH v3 6/6] boot: Call flush() before booting

[Pali Rohár]

In a lot of cases kernel resets UART HW. To ensure that U-Boot messages
printed before booting the kernel are not lost, call new U-Boot console
flush() function.

Signed-off-by: Pali Rohár <pali@kernel.org>
---
Changes in v3:
* Fix commit message
---
 boot/bootm_os.c | 1 +
 cmd/boot.c      | 1 +
 cmd/elf.c       | 2 ++
 3 files changed, 4 insertions(+)

diff --git a/boot/bootm_os.c b/boot/bootm_os.c
index f31820cd07ef..079224ce585b 100644
--- a/boot/bootm_os.c
+++ b/boot/bootm_os.c
@@ -303,6 +303,7 @@ static void do_bootvx_fdt(bootm_headers_t *images)
 #else
 	printf("## Starting vxWorks at 0x%08lx\n", (ulong)images->ep);
 #endif
+	flush();
 
 	boot_jump_vxworks(images);
 
diff --git a/cmd/boot.c b/cmd/boot.c
index be67a5980de3..14839c1cedcc 100644
--- a/cmd/boot.c
+++ b/cmd/boot.c
@@ -32,6 +32,7 @@ static int do_go(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
 	addr = hextoul(argv[1], NULL);
 
 	printf ("## Starting application at 0x%08lX ...\n", addr);
+	flush();
 
 	/*
 	 * pass address parameter as argv[0] (aka command name),
diff --git a/cmd/elf.c b/cmd/elf.c
index ce40d3f72a7c..b7b9f506a526 100644
--- a/cmd/elf.c
+++ b/cmd/elf.c
@@ -72,6 +72,7 @@ int do_bootelf(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
 		return rcode;
 
 	printf("## Starting application at 0x%08lx ...\n", addr);
+	flush();
 
 	/*
 	 * pass address parameter as argv[0] (aka command name),
@@ -274,6 +275,7 @@ int do_bootvx(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
 		puts("## Not an ELF image, assuming binary\n");
 
 	printf("## Starting vxWorks at 0x%08lx ...\n", addr);
+	flush();
 
 	dcache_disable();
 #if defined(CONFIG_ARM64) && defined(CONFIG_ARMV8_PSCI)
-- 
2.20.1

Re: [PATCH v3 3/6] serial: Implement flush callback

[Michael Nazzareno Trimarchi]

Hi

Il lun 5 set 2022, 11:32 Pali Rohár <pali@kernel.org> ha scritto:

> UART drivers have putc/puts functions which just put characters into HW
> transmit queue and do not wait until all data are transmitted.
>
> Implement flush callback via serial driver's pending(false) callback which
> waits until HW transmit all characters from the queue.
>

This is a drain if you want to wait. Is not the right terminology?

Michael


> Signed-off-by: Pali Rohár <pali@kernel.org>
> ---
>  drivers/serial/serial-uclass.c | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
>
> diff --git a/drivers/serial/serial-uclass.c
> b/drivers/serial/serial-uclass.c
> index 30650e37b0d7..be6502f3d24c 100644
> --- a/drivers/serial/serial-uclass.c
> +++ b/drivers/serial/serial-uclass.c
> @@ -238,6 +238,18 @@ static void _serial_puts(struct udevice *dev, const
> char *str)
>         } while (*str);
>  }
>
> +#ifdef CONFIG_CONSOLE_FLUSH_SUPPORT
> +static void _serial_flush(struct udevice *dev)
> +{
> +       struct dm_serial_ops *ops = serial_get_ops(dev);
> +
> +       if (!ops->pending)
> +               return;
> +       while (ops->pending(dev, false) > 0)
> +               ;
> +}
> +#endif
> +
>  static int __serial_getc(struct udevice *dev)
>  {
>         struct dm_serial_ops *ops = serial_get_ops(dev);
> @@ -398,6 +410,13 @@ static void serial_stub_puts(struct stdio_dev *sdev,
> const char *str)
>         _serial_puts(sdev->priv, str);
>  }
>
> +#ifdef CONFIG_CONSOLE_FLUSH_SUPPORT
> +static void serial_stub_flush(struct stdio_dev *sdev)
> +{
> +       _serial_flush(sdev->priv);
> +}
> +#endif
> +
>  static int serial_stub_getc(struct stdio_dev *sdev)
>  {
>         return _serial_getc(sdev->priv);
> @@ -520,6 +539,7 @@ static int serial_post_probe(struct udevice *dev)
>         sdev.priv = dev;
>         sdev.putc = serial_stub_putc;
>         sdev.puts = serial_stub_puts;
> +       STDIO_DEV_ASSIGN_FLUSH(&sdev, serial_stub_flush);
>         sdev.getc = serial_stub_getc;
>         sdev.tstc = serial_stub_tstc;
>
> --
> 2.20.1
>
>

Re: [PATCH v3 3/6] serial: Implement flush callback

[Pali Rohár]

On Monday 05 September 2022 19:24:17 Michael Nazzareno Trimarchi wrote:
> Hi
> 
> Il lun 5 set 2022, 11:32 Pali Rohár <pali@kernel.org> ha scritto:
> 
> > UART drivers have putc/puts functions which just put characters into HW
> > transmit queue and do not wait until all data are transmitted.
> >
> > Implement flush callback via serial driver's pending(false) callback which
> > waits until HW transmit all characters from the queue.
> >
> 
> This is a drain if you want to wait. Is not the right terminology?

Yes, for tty devices it is drain. But for C stdio.h it is (f)flush.
Hence I used more generic name flush() as it is not only for serial
devices, but for any U-Boot stdio device. E.g. sandbox os use real
fflush function.

But feel free to choose any other name of function. I do not care a much
about it.

> Michael
> 
> 
> > Signed-off-by: Pali Rohár <pali@kernel.org>
> > ---
> >  drivers/serial/serial-uclass.c | 20 ++++++++++++++++++++
> >  1 file changed, 20 insertions(+)
> >
> > diff --git a/drivers/serial/serial-uclass.c
> > b/drivers/serial/serial-uclass.c
> > index 30650e37b0d7..be6502f3d24c 100644
> > --- a/drivers/serial/serial-uclass.c
> > +++ b/drivers/serial/serial-uclass.c
> > @@ -238,6 +238,18 @@ static void _serial_puts(struct udevice *dev, const
> > char *str)
> >         } while (*str);
> >  }
> >
> > +#ifdef CONFIG_CONSOLE_FLUSH_SUPPORT
> > +static void _serial_flush(struct udevice *dev)
> > +{
> > +       struct dm_serial_ops *ops = serial_get_ops(dev);
> > +
> > +       if (!ops->pending)
> > +               return;
> > +       while (ops->pending(dev, false) > 0)
> > +               ;
> > +}
> > +#endif
> > +
> >  static int __serial_getc(struct udevice *dev)
> >  {
> >         struct dm_serial_ops *ops = serial_get_ops(dev);
> > @@ -398,6 +410,13 @@ static void serial_stub_puts(struct stdio_dev *sdev,
> > const char *str)
> >         _serial_puts(sdev->priv, str);
> >  }
> >
> > +#ifdef CONFIG_CONSOLE_FLUSH_SUPPORT
> > +static void serial_stub_flush(struct stdio_dev *sdev)
> > +{
> > +       _serial_flush(sdev->priv);
> > +}
> > +#endif
> > +
> >  static int serial_stub_getc(struct stdio_dev *sdev)
> >  {
> >         return _serial_getc(sdev->priv);
> > @@ -520,6 +539,7 @@ static int serial_post_probe(struct udevice *dev)
> >         sdev.priv = dev;
> >         sdev.putc = serial_stub_putc;
> >         sdev.puts = serial_stub_puts;
> > +       STDIO_DEV_ASSIGN_FLUSH(&sdev, serial_stub_flush);
> >         sdev.getc = serial_stub_getc;
> >         sdev.tstc = serial_stub_tstc;
> >
> > --
> > 2.20.1
> >
> >

Re: [PATCH v3 1/6] sandbox: Add function os_flush()

[Simon Glass]

On Mon, 5 Sept 2022 at 03:31, Pali Rohár <pali@kernel.org> wrote:
>
> It flushes stdout.
>
> Signed-off-by: Pali Rohár <pali@kernel.org>
> ---
>  arch/sandbox/cpu/os.c | 5 +++++
>  include/os.h          | 8 ++++++++
>  2 files changed, 13 insertions(+)

Reviewed-by: Simon Glass <sjg@chromium.org>

BTW this should report an error.

Re: [PATCH v3 4/6] serial: Implement serial_flush() function for console flush() fallback

[Simon Glass]

On Mon, 5 Sept 2022 at 03:31, Pali Rohár <pali@kernel.org> wrote:
>
> Like in all other console functions, implement also serial_flush() function
> as a fallback int console flush() function.
>
> Flush support is available only when config option CONSOLE_FLUSH_SUPPORT is
> enabled. So when it is disabled then provides just empty static inline
> function serial_flush().
>
> Signed-off-by: Pali Rohár <pali@kernel.org>
> ---
>  common/console.c               |  3 +++
>  common/stdio.c                 |  8 ++++++++
>  drivers/serial/serial-uclass.c | 10 ++++++++++
>  include/serial.h               |  5 +++++
>  4 files changed, 26 insertions(+)

Reviewed-by: Simon Glass <sjg@chromium.org>

Re: [PATCH v3 5/6] serial: Call flush() before changing baudrate

[Simon Glass]

On Mon, 5 Sept 2022 at 03:31, Pali Rohár <pali@kernel.org> wrote:
>
> Changing baudrate is a sensitive operation. To ensure that U-Boot messages
> printed before changing baudrate are not lost, call new U-Boot console
> flush() function.
>
> Signed-off-by: Pali Rohár <pali@kernel.org>
> ---
> Changes in v3:
> * Remove support from serial.c
> * Fix commit message
> ---
>  cmd/load.c                     | 5 +++++
>  drivers/serial/serial-uclass.c | 1 +
>  2 files changed, 6 insertions(+)
>

Reviewed-by: Simon Glass <sjg@chromium.org>

Re: [PATCH v3 6/6] boot: Call flush() before booting

[Simon Glass]

On Mon, 5 Sept 2022 at 03:31, Pali Rohár <pali@kernel.org> wrote:
>
> In a lot of cases kernel resets UART HW. To ensure that U-Boot messages
> printed before booting the kernel are not lost, call new U-Boot console
> flush() function.
>
> Signed-off-by: Pali Rohár <pali@kernel.org>
> ---
> Changes in v3:
> * Fix commit message
> ---
>  boot/bootm_os.c | 1 +
>  cmd/boot.c      | 1 +
>  cmd/elf.c       | 2 ++
>  3 files changed, 4 insertions(+)
>

Reviewed-by: Simon Glass <sjg@chromium.org>

although I'm not sure why the load.c change is not in this patch too?

Re: [PATCH v3 2/6] console: Implement flush() function

[Simon Glass]

Hi Pali,

On Mon, 5 Sept 2022 at 03:31, Pali Rohár <pali@kernel.org> wrote:
>
> On certain places it is required to flush output print buffers to ensure
> that text strings were sent to console or serial devices. For example when
> printing message that U-Boot is going to boot kernel or when U-Boot is
> going to change baudrate of terminal device.
>
> Therefore introduce a new flush() and fflush() functions into console code.
> These functions will call .flush callback of associated stdio_dev device.
>
> As this function may increase U-Boot side, allow to compile U-Boot without
> this function. For this purpose there is a new config CONSOLE_FLUSH_SUPPORT
> which is enabled by default and can be disabled. It is a good idea to have
> this option enabled for all boards which have enough space for it.
>
> When option is disabled when U-Boot defines just empty static inline
> function fflush() to avoid ifdefs in other code.
>
> Signed-off-by: Pali Rohár <pali@kernel.org>
> ---
> Changes in v3:
> * Added macro STDIO_DEV_ASSIGN_FLUSH()
> ---
>  common/Kconfig      |  6 +++++
>  common/console.c    | 61 +++++++++++++++++++++++++++++++++++++++++++++
>  include/_exports.h  |  3 +++
>  include/stdio.h     | 15 +++++++++++
>  include/stdio_dev.h |  7 ++++++
>  5 files changed, 92 insertions(+)

Reviewed-by: Simon Glass <sjg@chromium.org>

Re: [PATCH v3 3/6] serial: Implement flush callback

[Simon Glass]

On Mon, 5 Sept 2022 at 03:31, Pali Rohár <pali@kernel.org> wrote:
>
> UART drivers have putc/puts functions which just put characters into HW
> transmit queue and do not wait until all data are transmitted.
>
> Implement flush callback via serial driver's pending(false) callback which
> waits until HW transmit all characters from the queue.
>
> Signed-off-by: Pali Rohár <pali@kernel.org>
> ---
>  drivers/serial/serial-uclass.c | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)

Reviewed-by: Simon Glass <sjg@chromium.org>

Re: [PATCH v3 6/6] boot: Call flush() before booting

[Pali Rohár]

On Wednesday 07 September 2022 15:10:54 Simon Glass wrote:
> although I'm not sure why the load.c change is not in this patch too?

Because I split boot and serial/load changes into separate patches as
they are different logical things.

Re: [PATCH v3 4/6] serial: Implement serial_flush() function for console flush() fallback

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 593 bytes --]

On Mon, Sep 05, 2022 at 11:31:19AM +0200, Pali Rohár wrote:

> Like in all other console functions, implement also serial_flush() function
> as a fallback int console flush() function.
> 
> Flush support is available only when config option CONSOLE_FLUSH_SUPPORT is
> enabled. So when it is disabled then provides just empty static inline
> function serial_flush().
> 
> Signed-off-by: Pali Rohár <pali@kernel.org>
> Reviewed-by: Simon Glass <sjg@chromium.org>

This breaks building on a number of platforms such as ds109 probably due
to DM_SERIAL not being enabled.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH v3 4/6] serial: Implement serial_flush() function for console flush() fallback

[Pali Rohár]

On Tuesday 20 September 2022 17:40:39 Tom Rini wrote:
> On Mon, Sep 05, 2022 at 11:31:19AM +0200, Pali Rohár wrote:
> 
> > Like in all other console functions, implement also serial_flush() function
> > as a fallback int console flush() function.
> > 
> > Flush support is available only when config option CONSOLE_FLUSH_SUPPORT is
> > enabled. So when it is disabled then provides just empty static inline
> > function serial_flush().
> > 
> > Signed-off-by: Pali Rohár <pali@kernel.org>
> > Reviewed-by: Simon Glass <sjg@chromium.org>
> 
> This breaks building on a number of platforms such as ds109 probably due
> to DM_SERIAL not being enabled.

ds109 is failing on error:

  LD      u-boot
arm-linux-gnueabihf-ld.bfd: common/console.o: in function `flush':
/tmp/u-boot/common/console.c:802: undefined reference to `serial_flush'
arm-linux-gnueabihf-ld.bfd: common/stdio.o: in function `stdio_serial_flush':
/tmp/u-boot/common/stdio.c:93: undefined reference to `serial_flush'
make: *** [Makefile:1782: u-boot] Error 1

And serial_flush() is implemented only in serial-uclass.c (which is
DM_SERIAL code).

So could you try to add this additional guard (into this 4/6 patch)?

diff --git a/include/serial.h b/include/serial.h
index f9009d4046e3..fe01bcfadb9b 100644
--- a/include/serial.h
+++ b/include/serial.h
@@ -362,7 +362,7 @@ void serial_setbrg(void);
 void serial_putc(const char ch);
 void serial_putc_raw(const char ch);
 void serial_puts(const char *str);
-#ifdef CONFIG_CONSOLE_FLUSH_SUPPORT
+#if defined(CONFIG_CONSOLE_FLUSH_SUPPORT) && CONFIG_IS_ENABLED(DM_SERIAL)
 void serial_flush(void);
 #else
 static inline void serial_flush(void) {}

ds109 with this change compiles fine on my computer.

Re: [PATCH v3 4/6] serial: Implement serial_flush() function for console flush() fallback

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 2016 bytes --]

On Wed, Sep 21, 2022 at 12:18:57AM +0200, Pali Rohár wrote:
> On Tuesday 20 September 2022 17:40:39 Tom Rini wrote:
> > On Mon, Sep 05, 2022 at 11:31:19AM +0200, Pali Rohár wrote:
> > 
> > > Like in all other console functions, implement also serial_flush() function
> > > as a fallback int console flush() function.
> > > 
> > > Flush support is available only when config option CONSOLE_FLUSH_SUPPORT is
> > > enabled. So when it is disabled then provides just empty static inline
> > > function serial_flush().
> > > 
> > > Signed-off-by: Pali Rohár <pali@kernel.org>
> > > Reviewed-by: Simon Glass <sjg@chromium.org>
> > 
> > This breaks building on a number of platforms such as ds109 probably due
> > to DM_SERIAL not being enabled.
> 
> ds109 is failing on error:
> 
>   LD      u-boot
> arm-linux-gnueabihf-ld.bfd: common/console.o: in function `flush':
> /tmp/u-boot/common/console.c:802: undefined reference to `serial_flush'
> arm-linux-gnueabihf-ld.bfd: common/stdio.o: in function `stdio_serial_flush':
> /tmp/u-boot/common/stdio.c:93: undefined reference to `serial_flush'
> make: *** [Makefile:1782: u-boot] Error 1
> 
> And serial_flush() is implemented only in serial-uclass.c (which is
> DM_SERIAL code).
> 
> So could you try to add this additional guard (into this 4/6 patch)?
> 
> diff --git a/include/serial.h b/include/serial.h
> index f9009d4046e3..fe01bcfadb9b 100644
> --- a/include/serial.h
> +++ b/include/serial.h
> @@ -362,7 +362,7 @@ void serial_setbrg(void);
>  void serial_putc(const char ch);
>  void serial_putc_raw(const char ch);
>  void serial_puts(const char *str);
> -#ifdef CONFIG_CONSOLE_FLUSH_SUPPORT
> +#if defined(CONFIG_CONSOLE_FLUSH_SUPPORT) && CONFIG_IS_ENABLED(DM_SERIAL)
>  void serial_flush(void);
>  #else
>  static inline void serial_flush(void) {}
> 
> ds109 with this change compiles fine on my computer.

It should, but that means that CONSOLE_FLUSH_SUPPORT itself should be
depending on DM_SERIAL.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH v3 4/6] serial: Implement serial_flush() function for console flush() fallback

[Pali Rohár]

On Tuesday 20 September 2022 18:29:02 Tom Rini wrote:
> On Wed, Sep 21, 2022 at 12:18:57AM +0200, Pali Rohár wrote:
> > On Tuesday 20 September 2022 17:40:39 Tom Rini wrote:
> > > On Mon, Sep 05, 2022 at 11:31:19AM +0200, Pali Rohár wrote:
> > > 
> > > > Like in all other console functions, implement also serial_flush() function
> > > > as a fallback int console flush() function.
> > > > 
> > > > Flush support is available only when config option CONSOLE_FLUSH_SUPPORT is
> > > > enabled. So when it is disabled then provides just empty static inline
> > > > function serial_flush().
> > > > 
> > > > Signed-off-by: Pali Rohár <pali@kernel.org>
> > > > Reviewed-by: Simon Glass <sjg@chromium.org>
> > > 
> > > This breaks building on a number of platforms such as ds109 probably due
> > > to DM_SERIAL not being enabled.
> > 
> > ds109 is failing on error:
> > 
> >   LD      u-boot
> > arm-linux-gnueabihf-ld.bfd: common/console.o: in function `flush':
> > /tmp/u-boot/common/console.c:802: undefined reference to `serial_flush'
> > arm-linux-gnueabihf-ld.bfd: common/stdio.o: in function `stdio_serial_flush':
> > /tmp/u-boot/common/stdio.c:93: undefined reference to `serial_flush'
> > make: *** [Makefile:1782: u-boot] Error 1
> > 
> > And serial_flush() is implemented only in serial-uclass.c (which is
> > DM_SERIAL code).
> > 
> > So could you try to add this additional guard (into this 4/6 patch)?
> > 
> > diff --git a/include/serial.h b/include/serial.h
> > index f9009d4046e3..fe01bcfadb9b 100644
> > --- a/include/serial.h
> > +++ b/include/serial.h
> > @@ -362,7 +362,7 @@ void serial_setbrg(void);
> >  void serial_putc(const char ch);
> >  void serial_putc_raw(const char ch);
> >  void serial_puts(const char *str);
> > -#ifdef CONFIG_CONSOLE_FLUSH_SUPPORT
> > +#if defined(CONFIG_CONSOLE_FLUSH_SUPPORT) && CONFIG_IS_ENABLED(DM_SERIAL)
> >  void serial_flush(void);
> >  #else
> >  static inline void serial_flush(void) {}
> > 
> > ds109 with this change compiles fine on my computer.
> 
> It should, but that means that CONSOLE_FLUSH_SUPPORT itself should be
> depending on DM_SERIAL.

No. Because CONSOLE_FLUSH_SUPPORT has nothing with serial. You can have
e.g. LCD/VGA or USB tty output console device on platform without serial
console (where would be whole serial subsystem disabled) and still able
to use new flush support.

Re: [PATCH v3 4/6] serial: Implement serial_flush() function for console flush() fallback

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 2700 bytes --]

On Wed, Sep 21, 2022 at 12:32:33AM +0200, Pali Rohár wrote:
> On Tuesday 20 September 2022 18:29:02 Tom Rini wrote:
> > On Wed, Sep 21, 2022 at 12:18:57AM +0200, Pali Rohár wrote:
> > > On Tuesday 20 September 2022 17:40:39 Tom Rini wrote:
> > > > On Mon, Sep 05, 2022 at 11:31:19AM +0200, Pali Rohár wrote:
> > > > 
> > > > > Like in all other console functions, implement also serial_flush() function
> > > > > as a fallback int console flush() function.
> > > > > 
> > > > > Flush support is available only when config option CONSOLE_FLUSH_SUPPORT is
> > > > > enabled. So when it is disabled then provides just empty static inline
> > > > > function serial_flush().
> > > > > 
> > > > > Signed-off-by: Pali Rohár <pali@kernel.org>
> > > > > Reviewed-by: Simon Glass <sjg@chromium.org>
> > > > 
> > > > This breaks building on a number of platforms such as ds109 probably due
> > > > to DM_SERIAL not being enabled.
> > > 
> > > ds109 is failing on error:
> > > 
> > >   LD      u-boot
> > > arm-linux-gnueabihf-ld.bfd: common/console.o: in function `flush':
> > > /tmp/u-boot/common/console.c:802: undefined reference to `serial_flush'
> > > arm-linux-gnueabihf-ld.bfd: common/stdio.o: in function `stdio_serial_flush':
> > > /tmp/u-boot/common/stdio.c:93: undefined reference to `serial_flush'
> > > make: *** [Makefile:1782: u-boot] Error 1
> > > 
> > > And serial_flush() is implemented only in serial-uclass.c (which is
> > > DM_SERIAL code).
> > > 
> > > So could you try to add this additional guard (into this 4/6 patch)?
> > > 
> > > diff --git a/include/serial.h b/include/serial.h
> > > index f9009d4046e3..fe01bcfadb9b 100644
> > > --- a/include/serial.h
> > > +++ b/include/serial.h
> > > @@ -362,7 +362,7 @@ void serial_setbrg(void);
> > >  void serial_putc(const char ch);
> > >  void serial_putc_raw(const char ch);
> > >  void serial_puts(const char *str);
> > > -#ifdef CONFIG_CONSOLE_FLUSH_SUPPORT
> > > +#if defined(CONFIG_CONSOLE_FLUSH_SUPPORT) && CONFIG_IS_ENABLED(DM_SERIAL)
> > >  void serial_flush(void);
> > >  #else
> > >  static inline void serial_flush(void) {}
> > > 
> > > ds109 with this change compiles fine on my computer.
> > 
> > It should, but that means that CONSOLE_FLUSH_SUPPORT itself should be
> > depending on DM_SERIAL.
> 
> No. Because CONSOLE_FLUSH_SUPPORT has nothing with serial. You can have
> e.g. LCD/VGA or USB tty output console device on platform without serial
> console (where would be whole serial subsystem disabled) and still able
> to use new flush support.

Ah, hum. I'll apply the above and look at the before/after for everyone
before I suggest anything further.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH v3 0/6] console: Implement flush() function

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 2012 bytes --]

On Mon, Sep 05, 2022 at 11:31:15AM +0200, Pali Rohár wrote:

> On certain places it is required to flush output print buffers to ensure
> that text strings were sent to console or serial devices. For example when
> printing message that U-Boot is going to boot kernel or when U-Boot is
> going to change baudrate of terminal device.
> 
> Some console devices, like UART, have putc/puts functions which just put
> characters into HW transmit queue and do not wait until all data are
> transmitted. Doing some sensitive operations (like changing baudrate or
> starting kernel which resets UART HW) cause that U-Boot messages are lost.
> 
> Therefore introduce a new flush() function, implement it for all serial
> devices via pending(false) callback and use this new flush() function on
> sensitive places after which output device may go into reset state.
> 
> This change fixes printing of U-Boot messages:
> "## Starting application at ..."
> "## Switch baudrate to ..."
> 
> Changes in v3:
> * add macro STDIO_DEV_ASSIGN_FLUSH()
> * fix commit messages
> * remove changes from serial.c
> 
> Changes in v2:
> * split one big patch into smaller 6 patches
> * add config option to allow disabling this new function
> 
> Pali Rohár (6):
>   sandbox: Add function os_flush()
>   console: Implement flush() function
>   serial: Implement flush callback
>   serial: Implement serial_flush() function for console flush() fallback
>   serial: Call flush() before changing baudrate
>   boot: Call flush() before booting

Including the change you suggested to 4/6 to fix that build issue,
there's at least one more large issue that prevents CI from getting too
far:
https://source.denx.de/u-boot/u-boot/-/pipelines/13534
and they all have a failure similar to:
https://source.denx.de/u-boot/u-boot/-/jobs/500794#L51

Please see
https://u-boot.readthedocs.io/en/latest/develop/ci_testing.html for how
to run CI on the world yourself before submitting v4. Thanks!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH v3 0/6] console: Implement flush() function

[Pali Rohár]

On Wednesday 21 September 2022 09:49:24 Tom Rini wrote:
> On Mon, Sep 05, 2022 at 11:31:15AM +0200, Pali Rohár wrote:
> 
> > On certain places it is required to flush output print buffers to ensure
> > that text strings were sent to console or serial devices. For example when
> > printing message that U-Boot is going to boot kernel or when U-Boot is
> > going to change baudrate of terminal device.
> > 
> > Some console devices, like UART, have putc/puts functions which just put
> > characters into HW transmit queue and do not wait until all data are
> > transmitted. Doing some sensitive operations (like changing baudrate or
> > starting kernel which resets UART HW) cause that U-Boot messages are lost.
> > 
> > Therefore introduce a new flush() function, implement it for all serial
> > devices via pending(false) callback and use this new flush() function on
> > sensitive places after which output device may go into reset state.
> > 
> > This change fixes printing of U-Boot messages:
> > "## Starting application at ..."
> > "## Switch baudrate to ..."
> > 
> > Changes in v3:
> > * add macro STDIO_DEV_ASSIGN_FLUSH()
> > * fix commit messages
> > * remove changes from serial.c
> > 
> > Changes in v2:
> > * split one big patch into smaller 6 patches
> > * add config option to allow disabling this new function
> > 
> > Pali Rohár (6):
> >   sandbox: Add function os_flush()
> >   console: Implement flush() function
> >   serial: Implement flush callback
> >   serial: Implement serial_flush() function for console flush() fallback
> >   serial: Call flush() before changing baudrate
> >   boot: Call flush() before booting
> 
> Including the change you suggested to 4/6 to fix that build issue,
> there's at least one more large issue that prevents CI from getting too
> far:
> https://source.denx.de/u-boot/u-boot/-/pipelines/13534
> and they all have a failure similar to:
> https://source.denx.de/u-boot/u-boot/-/jobs/500794#L51

It looks like that some efi stuff overloads u-boot functions, in this
case newly added flush() function.

Any idea how to handle this issue?

The only option which I see how to address it is to revert those changes
in source files which always calls flush() function and replace them by
my first attempt which use guard #ifdef to ensure that flush() call is
completely eliminated at preprocessor stage when efi is enabled.

> Please see
> https://u-boot.readthedocs.io/en/latest/develop/ci_testing.html for how
> to run CI on the world yourself before submitting v4. Thanks!

Re: [PATCH v3 0/6] console: Implement flush() function

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 2631 bytes --]

On Wed, Sep 21, 2022 at 03:54:13PM +0200, Pali Rohár wrote:
> On Wednesday 21 September 2022 09:49:24 Tom Rini wrote:
> > On Mon, Sep 05, 2022 at 11:31:15AM +0200, Pali Rohár wrote:
> > 
> > > On certain places it is required to flush output print buffers to ensure
> > > that text strings were sent to console or serial devices. For example when
> > > printing message that U-Boot is going to boot kernel or when U-Boot is
> > > going to change baudrate of terminal device.
> > > 
> > > Some console devices, like UART, have putc/puts functions which just put
> > > characters into HW transmit queue and do not wait until all data are
> > > transmitted. Doing some sensitive operations (like changing baudrate or
> > > starting kernel which resets UART HW) cause that U-Boot messages are lost.
> > > 
> > > Therefore introduce a new flush() function, implement it for all serial
> > > devices via pending(false) callback and use this new flush() function on
> > > sensitive places after which output device may go into reset state.
> > > 
> > > This change fixes printing of U-Boot messages:
> > > "## Starting application at ..."
> > > "## Switch baudrate to ..."
> > > 
> > > Changes in v3:
> > > * add macro STDIO_DEV_ASSIGN_FLUSH()
> > > * fix commit messages
> > > * remove changes from serial.c
> > > 
> > > Changes in v2:
> > > * split one big patch into smaller 6 patches
> > > * add config option to allow disabling this new function
> > > 
> > > Pali Rohár (6):
> > >   sandbox: Add function os_flush()
> > >   console: Implement flush() function
> > >   serial: Implement flush callback
> > >   serial: Implement serial_flush() function for console flush() fallback
> > >   serial: Call flush() before changing baudrate
> > >   boot: Call flush() before booting
> > 
> > Including the change you suggested to 4/6 to fix that build issue,
> > there's at least one more large issue that prevents CI from getting too
> > far:
> > https://source.denx.de/u-boot/u-boot/-/pipelines/13534
> > and they all have a failure similar to:
> > https://source.denx.de/u-boot/u-boot/-/jobs/500794#L51
> 
> It looks like that some efi stuff overloads u-boot functions, in this
> case newly added flush() function.
> 
> Any idea how to handle this issue?
> 
> The only option which I see how to address it is to revert those changes
> in source files which always calls flush() function and replace them by
> my first attempt which use guard #ifdef to ensure that flush() call is
> completely eliminated at preprocessor stage when efi is enabled.

Adding in Heinrich.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH v3 0/6] console: Implement flush() function

[Simon Glass]

Hi,

On Wed, 21 Sept 2022 at 15:56, Tom Rini <trini@konsulko.com> wrote:
>
> On Wed, Sep 21, 2022 at 03:54:13PM +0200, Pali Rohár wrote:
> > On Wednesday 21 September 2022 09:49:24 Tom Rini wrote:
> > > On Mon, Sep 05, 2022 at 11:31:15AM +0200, Pali Rohár wrote:
> > >
> > > > On certain places it is required to flush output print buffers to ensure
> > > > that text strings were sent to console or serial devices. For example when
> > > > printing message that U-Boot is going to boot kernel or when U-Boot is
> > > > going to change baudrate of terminal device.
> > > >
> > > > Some console devices, like UART, have putc/puts functions which just put
> > > > characters into HW transmit queue and do not wait until all data are
> > > > transmitted. Doing some sensitive operations (like changing baudrate or
> > > > starting kernel which resets UART HW) cause that U-Boot messages are lost.
> > > >
> > > > Therefore introduce a new flush() function, implement it for all serial
> > > > devices via pending(false) callback and use this new flush() function on
> > > > sensitive places after which output device may go into reset state.
> > > >
> > > > This change fixes printing of U-Boot messages:
> > > > "## Starting application at ..."
> > > > "## Switch baudrate to ..."
> > > >
> > > > Changes in v3:
> > > > * add macro STDIO_DEV_ASSIGN_FLUSH()
> > > > * fix commit messages
> > > > * remove changes from serial.c
> > > >
> > > > Changes in v2:
> > > > * split one big patch into smaller 6 patches
> > > > * add config option to allow disabling this new function
> > > >
> > > > Pali Rohár (6):
> > > >   sandbox: Add function os_flush()
> > > >   console: Implement flush() function
> > > >   serial: Implement flush callback
> > > >   serial: Implement serial_flush() function for console flush() fallback
> > > >   serial: Call flush() before changing baudrate
> > > >   boot: Call flush() before booting
> > >
> > > Including the change you suggested to 4/6 to fix that build issue,
> > > there's at least one more large issue that prevents CI from getting too
> > > far:
> > > https://source.denx.de/u-boot/u-boot/-/pipelines/13534
> > > and they all have a failure similar to:
> > > https://source.denx.de/u-boot/u-boot/-/jobs/500794#L51
> >
> > It looks like that some efi stuff overloads u-boot functions, in this
> > case newly added flush() function.
> >
> > Any idea how to handle this issue?

I think you should rename the EFI flush() function to something like
efi_flush().

> >
> > The only option which I see how to address it is to revert those changes
> > in source files which always calls flush() function and replace them by
> > my first attempt which use guard #ifdef to ensure that flush() call is
> > completely eliminated at preprocessor stage when efi is enabled.
>
> Adding in Heinrich.
>
> --
> Tom

Regards,
Simon

Re: [PATCH v3 0/6] console: Implement flush() function

[Heinrich Schuchardt]

On 9/22/22 13:27, Simon Glass wrote:
> Hi,
>
> On Wed, 21 Sept 2022 at 15:56, Tom Rini <trini@konsulko.com> wrote:
>>
>> On Wed, Sep 21, 2022 at 03:54:13PM +0200, Pali Rohár wrote:
>>> On Wednesday 21 September 2022 09:49:24 Tom Rini wrote:
>>>> On Mon, Sep 05, 2022 at 11:31:15AM +0200, Pali Rohár wrote:
>>>>
>>>>> On certain places it is required to flush output print buffers to ensure
>>>>> that text strings were sent to console or serial devices. For example when
>>>>> printing message that U-Boot is going to boot kernel or when U-Boot is
>>>>> going to change baudrate of terminal device.
>>>>>
>>>>> Some console devices, like UART, have putc/puts functions which just put
>>>>> characters into HW transmit queue and do not wait until all data are
>>>>> transmitted. Doing some sensitive operations (like changing baudrate or
>>>>> starting kernel which resets UART HW) cause that U-Boot messages are lost.
>>>>>
>>>>> Therefore introduce a new flush() function, implement it for all serial
>>>>> devices via pending(false) callback and use this new flush() function on
>>>>> sensitive places after which output device may go into reset state.
>>>>>
>>>>> This change fixes printing of U-Boot messages:
>>>>> "## Starting application at ..."
>>>>> "## Switch baudrate to ..."
>>>>>
>>>>> Changes in v3:
>>>>> * add macro STDIO_DEV_ASSIGN_FLUSH()
>>>>> * fix commit messages
>>>>> * remove changes from serial.c
>>>>>
>>>>> Changes in v2:
>>>>> * split one big patch into smaller 6 patches
>>>>> * add config option to allow disabling this new function
>>>>>
>>>>> Pali Rohár (6):
>>>>>    sandbox: Add function os_flush()
>>>>>    console: Implement flush() function
>>>>>    serial: Implement flush callback
>>>>>    serial: Implement serial_flush() function for console flush() fallback
>>>>>    serial: Call flush() before changing baudrate
>>>>>    boot: Call flush() before booting
>>>>
>>>> Including the change you suggested to 4/6 to fix that build issue,
>>>> there's at least one more large issue that prevents CI from getting too
>>>> far:
>>>> https://source.denx.de/u-boot/u-boot/-/pipelines/13534
>>>> and they all have a failure similar to:
>>>> https://source.denx.de/u-boot/u-boot/-/jobs/500794#L51
>>>
>>> It looks like that some efi stuff overloads u-boot functions, in this
>>> case newly added flush() function.
>>>
>>> Any idea how to handle this issue?
>
> I think you should rename the EFI flush() function to something like
> efi_flush().

lib/efi_selftest/efi_selftest_loadimage.c has a static function flush()
which will override any global flush() function in the context of the
unit test. The unit test does not access the console so it is not really
problematic. But I have no objections to renaming it to efi_st_flush().

Using flush() as name for a global function that only flushes the
console seems a bit generic. Why not use flush_console() as name to
clearly indicate what the function does?

Best regards

Heinrich

>
>>>
>>> The only option which I see how to address it is to revert those changes
>>> in source files which always calls flush() function and replace them by
>>> my first attempt which use guard #ifdef to ensure that flush() call is
>>> completely eliminated at preprocessor stage when efi is enabled.
>>
>> Adding in Heinrich.
>>
>> --
>> Tom
>
> Regards,
> Simon

Re: [PATCH v3 0/6] console: Implement flush() function

[Pali Rohár]

On Thursday 22 September 2022 13:27:51 Simon Glass wrote:
> Hi,
> 
> On Wed, 21 Sept 2022 at 15:56, Tom Rini <trini@konsulko.com> wrote:
> >
> > On Wed, Sep 21, 2022 at 03:54:13PM +0200, Pali Rohár wrote:
> > > On Wednesday 21 September 2022 09:49:24 Tom Rini wrote:
> > > > On Mon, Sep 05, 2022 at 11:31:15AM +0200, Pali Rohár wrote:
> > > >
> > > > > On certain places it is required to flush output print buffers to ensure
> > > > > that text strings were sent to console or serial devices. For example when
> > > > > printing message that U-Boot is going to boot kernel or when U-Boot is
> > > > > going to change baudrate of terminal device.
> > > > >
> > > > > Some console devices, like UART, have putc/puts functions which just put
> > > > > characters into HW transmit queue and do not wait until all data are
> > > > > transmitted. Doing some sensitive operations (like changing baudrate or
> > > > > starting kernel which resets UART HW) cause that U-Boot messages are lost.
> > > > >
> > > > > Therefore introduce a new flush() function, implement it for all serial
> > > > > devices via pending(false) callback and use this new flush() function on
> > > > > sensitive places after which output device may go into reset state.
> > > > >
> > > > > This change fixes printing of U-Boot messages:
> > > > > "## Starting application at ..."
> > > > > "## Switch baudrate to ..."
> > > > >
> > > > > Changes in v3:
> > > > > * add macro STDIO_DEV_ASSIGN_FLUSH()
> > > > > * fix commit messages
> > > > > * remove changes from serial.c
> > > > >
> > > > > Changes in v2:
> > > > > * split one big patch into smaller 6 patches
> > > > > * add config option to allow disabling this new function
> > > > >
> > > > > Pali Rohár (6):
> > > > >   sandbox: Add function os_flush()
> > > > >   console: Implement flush() function
> > > > >   serial: Implement flush callback
> > > > >   serial: Implement serial_flush() function for console flush() fallback
> > > > >   serial: Call flush() before changing baudrate
> > > > >   boot: Call flush() before booting
> > > >
> > > > Including the change you suggested to 4/6 to fix that build issue,
> > > > there's at least one more large issue that prevents CI from getting too
> > > > far:
> > > > https://source.denx.de/u-boot/u-boot/-/pipelines/13534
> > > > and they all have a failure similar to:
> > > > https://source.denx.de/u-boot/u-boot/-/jobs/500794#L51
> > >
> > > It looks like that some efi stuff overloads u-boot functions, in this
> > > case newly added flush() function.
> > >
> > > Any idea how to handle this issue?
> 
> I think you should rename the EFI flush() function to something like
> efi_flush().

Ok! I renamed EFI flush() function to efi_flush().

I put all patches into github pull request which started CI test:
https://github.com/u-boot/u-boot/pull/241

> > >
> > > The only option which I see how to address it is to revert those changes
> > > in source files which always calls flush() function and replace them by
> > > my first attempt which use guard #ifdef to ensure that flush() call is
> > > completely eliminated at preprocessor stage when efi is enabled.
> >
> > Adding in Heinrich.
> >
> > --
> > Tom
> 
> Regards,
> Simon

Re: [PATCH v3 0/6] console: Implement flush() function

[Heinrich Schuchardt]

On 9/22/22 15:14, Pali Rohár wrote:
> On Thursday 22 September 2022 13:27:51 Simon Glass wrote:
>> Hi,
>>
>> On Wed, 21 Sept 2022 at 15:56, Tom Rini <trini@konsulko.com> wrote:
>>>
>>> On Wed, Sep 21, 2022 at 03:54:13PM +0200, Pali Rohár wrote:
>>>> On Wednesday 21 September 2022 09:49:24 Tom Rini wrote:
>>>>> On Mon, Sep 05, 2022 at 11:31:15AM +0200, Pali Rohár wrote:
>>>>>
>>>>>> On certain places it is required to flush output print buffers to ensure
>>>>>> that text strings were sent to console or serial devices. For example when
>>>>>> printing message that U-Boot is going to boot kernel or when U-Boot is
>>>>>> going to change baudrate of terminal device.
>>>>>>
>>>>>> Some console devices, like UART, have putc/puts functions which just put
>>>>>> characters into HW transmit queue and do not wait until all data are
>>>>>> transmitted. Doing some sensitive operations (like changing baudrate or
>>>>>> starting kernel which resets UART HW) cause that U-Boot messages are lost.
>>>>>>
>>>>>> Therefore introduce a new flush() function, implement it for all serial
>>>>>> devices via pending(false) callback and use this new flush() function on
>>>>>> sensitive places after which output device may go into reset state.
>>>>>>
>>>>>> This change fixes printing of U-Boot messages:
>>>>>> "## Starting application at ..."
>>>>>> "## Switch baudrate to ..."
>>>>>>
>>>>>> Changes in v3:
>>>>>> * add macro STDIO_DEV_ASSIGN_FLUSH()
>>>>>> * fix commit messages
>>>>>> * remove changes from serial.c
>>>>>>
>>>>>> Changes in v2:
>>>>>> * split one big patch into smaller 6 patches
>>>>>> * add config option to allow disabling this new function
>>>>>>
>>>>>> Pali Rohár (6):
>>>>>>    sandbox: Add function os_flush()
>>>>>>    console: Implement flush() function
>>>>>>    serial: Implement flush callback
>>>>>>    serial: Implement serial_flush() function for console flush() fallback
>>>>>>    serial: Call flush() before changing baudrate
>>>>>>    boot: Call flush() before booting
>>>>>
>>>>> Including the change you suggested to 4/6 to fix that build issue,
>>>>> there's at least one more large issue that prevents CI from getting too
>>>>> far:
>>>>> https://source.denx.de/u-boot/u-boot/-/pipelines/13534
>>>>> and they all have a failure similar to:
>>>>> https://source.denx.de/u-boot/u-boot/-/jobs/500794#L51
>>>>
>>>> It looks like that some efi stuff overloads u-boot functions, in this
>>>> case newly added flush() function.
>>>>
>>>> Any idea how to handle this issue?
>>
>> I think you should rename the EFI flush() function to something like
>> efi_flush().
>
> Ok! I renamed EFI flush() function to efi_flush().
>
> I put all patches into github pull request which started CI test:
> https://github.com/u-boot/u-boot/pull/241

When merging, please, use
[PATCH 1/1] efi_selftest: prefix test functions with efi_st_
https://lists.denx.de/pipermail/u-boot/2022-September/495334.html

Best regards

Heinrich

>
>>>>
>>>> The only option which I see how to address it is to revert those changes
>>>> in source files which always calls flush() function and replace them by
>>>> my first attempt which use guard #ifdef to ensure that flush() call is
>>>> completely eliminated at preprocessor stage when efi is enabled.
>>>
>>> Adding in Heinrich.
>>>
>>> --
>>> Tom
>>
>> Regards,
>> Simon

Re: [PATCH v3 0/6] console: Implement flush() function

[Pali Rohár]

On Thursday 22 September 2022 17:06:31 Heinrich Schuchardt wrote:
> On 9/22/22 15:14, Pali Rohár wrote:
> > On Thursday 22 September 2022 13:27:51 Simon Glass wrote:
> > > Hi,
> > > 
> > > On Wed, 21 Sept 2022 at 15:56, Tom Rini <trini@konsulko.com> wrote:
> > > > 
> > > > On Wed, Sep 21, 2022 at 03:54:13PM +0200, Pali Rohár wrote:
> > > > > On Wednesday 21 September 2022 09:49:24 Tom Rini wrote:
> > > > > > On Mon, Sep 05, 2022 at 11:31:15AM +0200, Pali Rohár wrote:
> > > > > > 
> > > > > > > On certain places it is required to flush output print buffers to ensure
> > > > > > > that text strings were sent to console or serial devices. For example when
> > > > > > > printing message that U-Boot is going to boot kernel or when U-Boot is
> > > > > > > going to change baudrate of terminal device.
> > > > > > > 
> > > > > > > Some console devices, like UART, have putc/puts functions which just put
> > > > > > > characters into HW transmit queue and do not wait until all data are
> > > > > > > transmitted. Doing some sensitive operations (like changing baudrate or
> > > > > > > starting kernel which resets UART HW) cause that U-Boot messages are lost.
> > > > > > > 
> > > > > > > Therefore introduce a new flush() function, implement it for all serial
> > > > > > > devices via pending(false) callback and use this new flush() function on
> > > > > > > sensitive places after which output device may go into reset state.
> > > > > > > 
> > > > > > > This change fixes printing of U-Boot messages:
> > > > > > > "## Starting application at ..."
> > > > > > > "## Switch baudrate to ..."
> > > > > > > 
> > > > > > > Changes in v3:
> > > > > > > * add macro STDIO_DEV_ASSIGN_FLUSH()
> > > > > > > * fix commit messages
> > > > > > > * remove changes from serial.c
> > > > > > > 
> > > > > > > Changes in v2:
> > > > > > > * split one big patch into smaller 6 patches
> > > > > > > * add config option to allow disabling this new function
> > > > > > > 
> > > > > > > Pali Rohár (6):
> > > > > > >    sandbox: Add function os_flush()
> > > > > > >    console: Implement flush() function
> > > > > > >    serial: Implement flush callback
> > > > > > >    serial: Implement serial_flush() function for console flush() fallback
> > > > > > >    serial: Call flush() before changing baudrate
> > > > > > >    boot: Call flush() before booting
> > > > > > 
> > > > > > Including the change you suggested to 4/6 to fix that build issue,
> > > > > > there's at least one more large issue that prevents CI from getting too
> > > > > > far:
> > > > > > https://source.denx.de/u-boot/u-boot/-/pipelines/13534
> > > > > > and they all have a failure similar to:
> > > > > > https://source.denx.de/u-boot/u-boot/-/jobs/500794#L51
> > > > > 
> > > > > It looks like that some efi stuff overloads u-boot functions, in this
> > > > > case newly added flush() function.
> > > > > 
> > > > > Any idea how to handle this issue?
> > > 
> > > I think you should rename the EFI flush() function to something like
> > > efi_flush().
> > 
> > Ok! I renamed EFI flush() function to efi_flush().
> > 
> > I put all patches into github pull request which started CI test:
> > https://github.com/u-boot/u-boot/pull/241
> 
> When merging, please, use
> [PATCH 1/1] efi_selftest: prefix test functions with efi_st_
> https://lists.denx.de/pipermail/u-boot/2022-September/495334.html

Hello! I updated https://github.com/u-boot/u-boot/pull/241 pull request
for CI test. I added there above efi_selftest patch and after that CI
test passed.

So Tom, is this enough for you? Or do you need something more?

> Best regards
> 
> Heinrich
> 
> > 
> > > > > 
> > > > > The only option which I see how to address it is to revert those changes
> > > > > in source files which always calls flush() function and replace them by
> > > > > my first attempt which use guard #ifdef to ensure that flush() call is
> > > > > completely eliminated at preprocessor stage when efi is enabled.
> > > > 
> > > > Adding in Heinrich.
> > > > 
> > > > --
> > > > Tom
> > > 
> > > Regards,
> > > Simon
> 

Re: [PATCH v3 0/6] console: Implement flush() function

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 3936 bytes --]

On Fri, Sep 23, 2022 at 05:45:07PM +0200, Pali Rohár wrote:
> On Thursday 22 September 2022 17:06:31 Heinrich Schuchardt wrote:
> > On 9/22/22 15:14, Pali Rohár wrote:
> > > On Thursday 22 September 2022 13:27:51 Simon Glass wrote:
> > > > Hi,
> > > > 
> > > > On Wed, 21 Sept 2022 at 15:56, Tom Rini <trini@konsulko.com> wrote:
> > > > > 
> > > > > On Wed, Sep 21, 2022 at 03:54:13PM +0200, Pali Rohár wrote:
> > > > > > On Wednesday 21 September 2022 09:49:24 Tom Rini wrote:
> > > > > > > On Mon, Sep 05, 2022 at 11:31:15AM +0200, Pali Rohár wrote:
> > > > > > > 
> > > > > > > > On certain places it is required to flush output print buffers to ensure
> > > > > > > > that text strings were sent to console or serial devices. For example when
> > > > > > > > printing message that U-Boot is going to boot kernel or when U-Boot is
> > > > > > > > going to change baudrate of terminal device.
> > > > > > > > 
> > > > > > > > Some console devices, like UART, have putc/puts functions which just put
> > > > > > > > characters into HW transmit queue and do not wait until all data are
> > > > > > > > transmitted. Doing some sensitive operations (like changing baudrate or
> > > > > > > > starting kernel which resets UART HW) cause that U-Boot messages are lost.
> > > > > > > > 
> > > > > > > > Therefore introduce a new flush() function, implement it for all serial
> > > > > > > > devices via pending(false) callback and use this new flush() function on
> > > > > > > > sensitive places after which output device may go into reset state.
> > > > > > > > 
> > > > > > > > This change fixes printing of U-Boot messages:
> > > > > > > > "## Starting application at ..."
> > > > > > > > "## Switch baudrate to ..."
> > > > > > > > 
> > > > > > > > Changes in v3:
> > > > > > > > * add macro STDIO_DEV_ASSIGN_FLUSH()
> > > > > > > > * fix commit messages
> > > > > > > > * remove changes from serial.c
> > > > > > > > 
> > > > > > > > Changes in v2:
> > > > > > > > * split one big patch into smaller 6 patches
> > > > > > > > * add config option to allow disabling this new function
> > > > > > > > 
> > > > > > > > Pali Rohár (6):
> > > > > > > >    sandbox: Add function os_flush()
> > > > > > > >    console: Implement flush() function
> > > > > > > >    serial: Implement flush callback
> > > > > > > >    serial: Implement serial_flush() function for console flush() fallback
> > > > > > > >    serial: Call flush() before changing baudrate
> > > > > > > >    boot: Call flush() before booting
> > > > > > > 
> > > > > > > Including the change you suggested to 4/6 to fix that build issue,
> > > > > > > there's at least one more large issue that prevents CI from getting too
> > > > > > > far:
> > > > > > > https://source.denx.de/u-boot/u-boot/-/pipelines/13534
> > > > > > > and they all have a failure similar to:
> > > > > > > https://source.denx.de/u-boot/u-boot/-/jobs/500794#L51
> > > > > > 
> > > > > > It looks like that some efi stuff overloads u-boot functions, in this
> > > > > > case newly added flush() function.
> > > > > > 
> > > > > > Any idea how to handle this issue?
> > > > 
> > > > I think you should rename the EFI flush() function to something like
> > > > efi_flush().
> > > 
> > > Ok! I renamed EFI flush() function to efi_flush().
> > > 
> > > I put all patches into github pull request which started CI test:
> > > https://github.com/u-boot/u-boot/pull/241
> > 
> > When merging, please, use
> > [PATCH 1/1] efi_selftest: prefix test functions with efi_st_
> > https://lists.denx.de/pipermail/u-boot/2022-September/495334.html
> 
> Hello! I updated https://github.com/u-boot/u-boot/pull/241 pull request
> for CI test. I added there above efi_selftest patch and after that CI
> test passed.
> 
> So Tom, is this enough for you? Or do you need something more?

Were you also going to rename flush as suggested?

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH v3 0/6] console: Implement flush() function

[Pali Rohár]

On Friday 23 September 2022 11:57:30 Tom Rini wrote:
> On Fri, Sep 23, 2022 at 05:45:07PM +0200, Pali Rohár wrote:
> > On Thursday 22 September 2022 17:06:31 Heinrich Schuchardt wrote:
> > > On 9/22/22 15:14, Pali Rohár wrote:
> > > > On Thursday 22 September 2022 13:27:51 Simon Glass wrote:
> > > > > Hi,
> > > > > 
> > > > > On Wed, 21 Sept 2022 at 15:56, Tom Rini <trini@konsulko.com> wrote:
> > > > > > 
> > > > > > On Wed, Sep 21, 2022 at 03:54:13PM +0200, Pali Rohár wrote:
> > > > > > > On Wednesday 21 September 2022 09:49:24 Tom Rini wrote:
> > > > > > > > On Mon, Sep 05, 2022 at 11:31:15AM +0200, Pali Rohár wrote:
> > > > > > > > 
> > > > > > > > > On certain places it is required to flush output print buffers to ensure
> > > > > > > > > that text strings were sent to console or serial devices. For example when
> > > > > > > > > printing message that U-Boot is going to boot kernel or when U-Boot is
> > > > > > > > > going to change baudrate of terminal device.
> > > > > > > > > 
> > > > > > > > > Some console devices, like UART, have putc/puts functions which just put
> > > > > > > > > characters into HW transmit queue and do not wait until all data are
> > > > > > > > > transmitted. Doing some sensitive operations (like changing baudrate or
> > > > > > > > > starting kernel which resets UART HW) cause that U-Boot messages are lost.
> > > > > > > > > 
> > > > > > > > > Therefore introduce a new flush() function, implement it for all serial
> > > > > > > > > devices via pending(false) callback and use this new flush() function on
> > > > > > > > > sensitive places after which output device may go into reset state.
> > > > > > > > > 
> > > > > > > > > This change fixes printing of U-Boot messages:
> > > > > > > > > "## Starting application at ..."
> > > > > > > > > "## Switch baudrate to ..."
> > > > > > > > > 
> > > > > > > > > Changes in v3:
> > > > > > > > > * add macro STDIO_DEV_ASSIGN_FLUSH()
> > > > > > > > > * fix commit messages
> > > > > > > > > * remove changes from serial.c
> > > > > > > > > 
> > > > > > > > > Changes in v2:
> > > > > > > > > * split one big patch into smaller 6 patches
> > > > > > > > > * add config option to allow disabling this new function
> > > > > > > > > 
> > > > > > > > > Pali Rohár (6):
> > > > > > > > >    sandbox: Add function os_flush()
> > > > > > > > >    console: Implement flush() function
> > > > > > > > >    serial: Implement flush callback
> > > > > > > > >    serial: Implement serial_flush() function for console flush() fallback
> > > > > > > > >    serial: Call flush() before changing baudrate
> > > > > > > > >    boot: Call flush() before booting
> > > > > > > > 
> > > > > > > > Including the change you suggested to 4/6 to fix that build issue,
> > > > > > > > there's at least one more large issue that prevents CI from getting too
> > > > > > > > far:
> > > > > > > > https://source.denx.de/u-boot/u-boot/-/pipelines/13534
> > > > > > > > and they all have a failure similar to:
> > > > > > > > https://source.denx.de/u-boot/u-boot/-/jobs/500794#L51
> > > > > > > 
> > > > > > > It looks like that some efi stuff overloads u-boot functions, in this
> > > > > > > case newly added flush() function.
> > > > > > > 
> > > > > > > Any idea how to handle this issue?
> > > > > 
> > > > > I think you should rename the EFI flush() function to something like
> > > > > efi_flush().
> > > > 
> > > > Ok! I renamed EFI flush() function to efi_flush().
> > > > 
> > > > I put all patches into github pull request which started CI test:
> > > > https://github.com/u-boot/u-boot/pull/241
> > > 
> > > When merging, please, use
> > > [PATCH 1/1] efi_selftest: prefix test functions with efi_st_
> > > https://lists.denx.de/pipermail/u-boot/2022-September/495334.html
> > 
> > Hello! I updated https://github.com/u-boot/u-boot/pull/241 pull request
> > for CI test. I added there above efi_selftest patch and after that CI
> > test passed.
> > 
> > So Tom, is this enough for you? Or do you need something more?
> 
> Were you also going to rename flush as suggested?

I have not done any renaming (yet). I pushed on CI test above efi_st fix
and this patch series (with 4/6 build fix issue). Nothing more.

About renaming, I'm not sure what if flush_console() is better name as
this is generic API and it can do e.g. flushing LCD output. Just like
generic puts() or getc(). I just do not know if generic name is better
or worse...

Re: [PATCH v3 0/6] console: Implement flush() function

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 4862 bytes --]

On Fri, Sep 23, 2022 at 06:07:03PM +0200, Pali Rohár wrote:
> On Friday 23 September 2022 11:57:30 Tom Rini wrote:
> > On Fri, Sep 23, 2022 at 05:45:07PM +0200, Pali Rohár wrote:
> > > On Thursday 22 September 2022 17:06:31 Heinrich Schuchardt wrote:
> > > > On 9/22/22 15:14, Pali Rohár wrote:
> > > > > On Thursday 22 September 2022 13:27:51 Simon Glass wrote:
> > > > > > Hi,
> > > > > > 
> > > > > > On Wed, 21 Sept 2022 at 15:56, Tom Rini <trini@konsulko.com> wrote:
> > > > > > > 
> > > > > > > On Wed, Sep 21, 2022 at 03:54:13PM +0200, Pali Rohár wrote:
> > > > > > > > On Wednesday 21 September 2022 09:49:24 Tom Rini wrote:
> > > > > > > > > On Mon, Sep 05, 2022 at 11:31:15AM +0200, Pali Rohár wrote:
> > > > > > > > > 
> > > > > > > > > > On certain places it is required to flush output print buffers to ensure
> > > > > > > > > > that text strings were sent to console or serial devices. For example when
> > > > > > > > > > printing message that U-Boot is going to boot kernel or when U-Boot is
> > > > > > > > > > going to change baudrate of terminal device.
> > > > > > > > > > 
> > > > > > > > > > Some console devices, like UART, have putc/puts functions which just put
> > > > > > > > > > characters into HW transmit queue and do not wait until all data are
> > > > > > > > > > transmitted. Doing some sensitive operations (like changing baudrate or
> > > > > > > > > > starting kernel which resets UART HW) cause that U-Boot messages are lost.
> > > > > > > > > > 
> > > > > > > > > > Therefore introduce a new flush() function, implement it for all serial
> > > > > > > > > > devices via pending(false) callback and use this new flush() function on
> > > > > > > > > > sensitive places after which output device may go into reset state.
> > > > > > > > > > 
> > > > > > > > > > This change fixes printing of U-Boot messages:
> > > > > > > > > > "## Starting application at ..."
> > > > > > > > > > "## Switch baudrate to ..."
> > > > > > > > > > 
> > > > > > > > > > Changes in v3:
> > > > > > > > > > * add macro STDIO_DEV_ASSIGN_FLUSH()
> > > > > > > > > > * fix commit messages
> > > > > > > > > > * remove changes from serial.c
> > > > > > > > > > 
> > > > > > > > > > Changes in v2:
> > > > > > > > > > * split one big patch into smaller 6 patches
> > > > > > > > > > * add config option to allow disabling this new function
> > > > > > > > > > 
> > > > > > > > > > Pali Rohár (6):
> > > > > > > > > >    sandbox: Add function os_flush()
> > > > > > > > > >    console: Implement flush() function
> > > > > > > > > >    serial: Implement flush callback
> > > > > > > > > >    serial: Implement serial_flush() function for console flush() fallback
> > > > > > > > > >    serial: Call flush() before changing baudrate
> > > > > > > > > >    boot: Call flush() before booting
> > > > > > > > > 
> > > > > > > > > Including the change you suggested to 4/6 to fix that build issue,
> > > > > > > > > there's at least one more large issue that prevents CI from getting too
> > > > > > > > > far:
> > > > > > > > > https://source.denx.de/u-boot/u-boot/-/pipelines/13534
> > > > > > > > > and they all have a failure similar to:
> > > > > > > > > https://source.denx.de/u-boot/u-boot/-/jobs/500794#L51
> > > > > > > > 
> > > > > > > > It looks like that some efi stuff overloads u-boot functions, in this
> > > > > > > > case newly added flush() function.
> > > > > > > > 
> > > > > > > > Any idea how to handle this issue?
> > > > > > 
> > > > > > I think you should rename the EFI flush() function to something like
> > > > > > efi_flush().
> > > > > 
> > > > > Ok! I renamed EFI flush() function to efi_flush().
> > > > > 
> > > > > I put all patches into github pull request which started CI test:
> > > > > https://github.com/u-boot/u-boot/pull/241
> > > > 
> > > > When merging, please, use
> > > > [PATCH 1/1] efi_selftest: prefix test functions with efi_st_
> > > > https://lists.denx.de/pipermail/u-boot/2022-September/495334.html
> > > 
> > > Hello! I updated https://github.com/u-boot/u-boot/pull/241 pull request
> > > for CI test. I added there above efi_selftest patch and after that CI
> > > test passed.
> > > 
> > > So Tom, is this enough for you? Or do you need something more?
> > 
> > Were you also going to rename flush as suggested?
> 
> I have not done any renaming (yet). I pushed on CI test above efi_st fix
> and this patch series (with 4/6 build fix issue). Nothing more.
> 
> About renaming, I'm not sure what if flush_console() is better name as
> this is generic API and it can do e.g. flushing LCD output. Just like
> generic puts() or getc(). I just do not know if generic name is better
> or worse...

I'm not sure either, lets wait a little bit longer to see if anyone else
chimes in with a strong opinion?

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH v3 0/6] console: Implement flush() function

[Simon Glass]

Hi,

On Fri, 23 Sept 2022 at 10:19, Tom Rini <trini@konsulko.com> wrote:
>
> On Fri, Sep 23, 2022 at 06:07:03PM +0200, Pali Rohár wrote:
> > On Friday 23 September 2022 11:57:30 Tom Rini wrote:
> > > On Fri, Sep 23, 2022 at 05:45:07PM +0200, Pali Rohár wrote:
> > > > On Thursday 22 September 2022 17:06:31 Heinrich Schuchardt wrote:
> > > > > On 9/22/22 15:14, Pali Rohár wrote:
> > > > > > On Thursday 22 September 2022 13:27:51 Simon Glass wrote:
> > > > > > > Hi,
> > > > > > >
> > > > > > > On Wed, 21 Sept 2022 at 15:56, Tom Rini <trini@konsulko.com> wrote:
> > > > > > > >
> > > > > > > > On Wed, Sep 21, 2022 at 03:54:13PM +0200, Pali Rohár wrote:
> > > > > > > > > On Wednesday 21 September 2022 09:49:24 Tom Rini wrote:
> > > > > > > > > > On Mon, Sep 05, 2022 at 11:31:15AM +0200, Pali Rohár wrote:
> > > > > > > > > >
> > > > > > > > > > > On certain places it is required to flush output print buffers to ensure
> > > > > > > > > > > that text strings were sent to console or serial devices. For example when
> > > > > > > > > > > printing message that U-Boot is going to boot kernel or when U-Boot is
> > > > > > > > > > > going to change baudrate of terminal device.
> > > > > > > > > > >
> > > > > > > > > > > Some console devices, like UART, have putc/puts functions which just put
> > > > > > > > > > > characters into HW transmit queue and do not wait until all data are
> > > > > > > > > > > transmitted. Doing some sensitive operations (like changing baudrate or
> > > > > > > > > > > starting kernel which resets UART HW) cause that U-Boot messages are lost.
> > > > > > > > > > >
> > > > > > > > > > > Therefore introduce a new flush() function, implement it for all serial
> > > > > > > > > > > devices via pending(false) callback and use this new flush() function on
> > > > > > > > > > > sensitive places after which output device may go into reset state.
> > > > > > > > > > >
> > > > > > > > > > > This change fixes printing of U-Boot messages:
> > > > > > > > > > > "## Starting application at ..."
> > > > > > > > > > > "## Switch baudrate to ..."
> > > > > > > > > > >
> > > > > > > > > > > Changes in v3:
> > > > > > > > > > > * add macro STDIO_DEV_ASSIGN_FLUSH()
> > > > > > > > > > > * fix commit messages
> > > > > > > > > > > * remove changes from serial.c
> > > > > > > > > > >
> > > > > > > > > > > Changes in v2:
> > > > > > > > > > > * split one big patch into smaller 6 patches
> > > > > > > > > > > * add config option to allow disabling this new function
> > > > > > > > > > >
> > > > > > > > > > > Pali Rohár (6):
> > > > > > > > > > >    sandbox: Add function os_flush()
> > > > > > > > > > >    console: Implement flush() function
> > > > > > > > > > >    serial: Implement flush callback
> > > > > > > > > > >    serial: Implement serial_flush() function for console flush() fallback
> > > > > > > > > > >    serial: Call flush() before changing baudrate
> > > > > > > > > > >    boot: Call flush() before booting
> > > > > > > > > >
> > > > > > > > > > Including the change you suggested to 4/6 to fix that build issue,
> > > > > > > > > > there's at least one more large issue that prevents CI from getting too
> > > > > > > > > > far:
> > > > > > > > > > https://source.denx.de/u-boot/u-boot/-/pipelines/13534
> > > > > > > > > > and they all have a failure similar to:
> > > > > > > > > > https://source.denx.de/u-boot/u-boot/-/jobs/500794#L51
> > > > > > > > >
> > > > > > > > > It looks like that some efi stuff overloads u-boot functions, in this
> > > > > > > > > case newly added flush() function.
> > > > > > > > >
> > > > > > > > > Any idea how to handle this issue?
> > > > > > >
> > > > > > > I think you should rename the EFI flush() function to something like
> > > > > > > efi_flush().
> > > > > >
> > > > > > Ok! I renamed EFI flush() function to efi_flush().
> > > > > >
> > > > > > I put all patches into github pull request which started CI test:
> > > > > > https://github.com/u-boot/u-boot/pull/241
> > > > >
> > > > > When merging, please, use
> > > > > [PATCH 1/1] efi_selftest: prefix test functions with efi_st_
> > > > > https://lists.denx.de/pipermail/u-boot/2022-September/495334.html
> > > >
> > > > Hello! I updated https://github.com/u-boot/u-boot/pull/241 pull request
> > > > for CI test. I added there above efi_selftest patch and after that CI
> > > > test passed.
> > > >
> > > > So Tom, is this enough for you? Or do you need something more?
> > >
> > > Were you also going to rename flush as suggested?
> >
> > I have not done any renaming (yet). I pushed on CI test above efi_st fix
> > and this patch series (with 4/6 build fix issue). Nothing more.
> >
> > About renaming, I'm not sure what if flush_console() is better name as
> > this is generic API and it can do e.g. flushing LCD output. Just like
> > generic puts() or getc(). I just do not know if generic name is better
> > or worse...
>
> I'm not sure either, lets wait a little bit longer to see if anyone else
> chimes in with a strong opinion?

I feel that flush() is probably the right name for now, since this is
a generic function as the rest of those functions don't have prefixes
or suffixes.

Regards,
Simon

Re: [PATCH v3 1/6] sandbox: Add function os_flush()

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 283 bytes --]

On Mon, Sep 05, 2022 at 11:31:16AM +0200, Pali Rohár wrote:

> It flushes stdout.
> 
> Signed-off-by: Pali Rohár <pali@kernel.org>
> Reviewed-by: Simon Glass <sjg@chromium.org>

For the series, and with 4/6 fixed-up as suggested, applied to
u-boot/next, thanks!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]