[PATCH v2 2/6] console: Implement flush() function
Simon Glass
sjg at chromium.org
Thu Aug 11 16:47:50 CEST 2022
Hi Pali,
On Thu, 11 Aug 2022 at 06:39, Pali Rohár <pali at 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 at kernel.org>
> ---
> common/Kconfig | 6 +++++
> common/console.c | 63 +++++++++++++++++++++++++++++++++++++++++++++
> include/_exports.h | 3 +++
> include/stdio.h | 15 +++++++++++
> include/stdio_dev.h | 4 +++
> 5 files changed, 91 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.
This needs at least two more lines, I think. Perhaps you can explain
what flush does and the fact that there is no timeout?
> +
> 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 dc071f1ed665..42415e34d16f 100644
> --- a/common/console.c
> +++ b/common/console.c
> @@ -198,6 +198,9 @@ static int console_setfile(int file, struct stdio_dev * dev)
> case stdout:
> gd->jt->putc = putc;
> gd->jt->puts = puts;
> +#ifdef CONFIG_CONSOLE_FLUSH_SUPPORT
How about CONFIG_IS_ENABLED() so we don't enable this in SPL too?
> + gd->jt->flush = flush;
> +#endif
> gd->jt->printf = printf;
> break;
> }
> @@ -363,6 +366,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)
> {
> @@ -412,6 +428,14 @@ static inline void console_puts(int file, const char *s)
> stdio_devices[file]->puts(stdio_devices[file], s);
> }
>
> +#ifdef CONFIG_CONSOLE_FLUSH_SUPPORT
I hope you can drop this #ifdef - see below.
> +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)
> {
> @@ -525,6 +549,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;
> @@ -731,6 +763,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..06278366ae88 100644
> --- a/include/stdio_dev.h
> +++ b/include/stdio_dev.h
> @@ -37,6 +37,10 @@ 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
I'd argue it isn't worth the #ifdef and we might as well have this
member here always. Then you can drop some #ifdefs from your code.
> + /* To flush output queue */
> + void (*flush)(struct stdio_dev *dev);
> +#endif
>
> /* INPUT functions */
>
> --
> 2.20.1
>
Regards,
SImon
More information about the U-Boot
mailing list