[U-Boot] [PATCH] serial: Enable checking UART is ready for console based on device tree

[Tien Fong Chee]

On Tue, 2016-07-26 at 15:20 +0200, Marek Vasut wrote:
> On 07/26/2016 12:55 PM, Tien Fong Chee wrote:
> > This patch would do checking on device tree to ensure the UART
> > exist
> > in the system and ready for console before setting have_console to
> > true.
> > This is required to avoid unexpected behavior such as hang during
> > UART
> > initialization.
>
> Can you elaborate on the issue in more depth ?
> This patch looks like some sort of workaround/hack.
>
In current init sequence flow from board_init_f, both serial_init and
console_init_f are executed blindly without checking the UART is
available at that moment or not. This happened in our SOCFPGA where
the UART is only available after init sequence. The UART peripheral is
programmed and being configured by FPGA imge during runtime. Hence,
serial_init is called without UART would causing the U-boot stuck in
polling some UART TX/RX registers forever.
> > Signed-off-by: Tien Fong Chee <tfchee at altera.com>
> > Cc: Dinh Nguyen <dinguyen at opensource.altera.com>
> > Cc: Dinh Nguyen <dinh.linux at gmail.com>
> > Cc: ChinLiang <clsee at altera.com>
> > Cc: Thomas Chou <thomas at wytron.com.tw>
> > Cc: Simon Glass <sjg at chromium.org>
> > Cc: Marek Vasut <marex at denx.de>
> > ---
> >  common/console.c                  |   11 ++++---
> >  common/spl/spl.c                  |    3 +-
> >  drivers/serial/serial.c           |   58
> > +++++++++++++++++++++++++++++++++++-
> >  include/asm-generic/global_data.h |    1 +
> >  include/serial.h                  |    1 +
> >  5 files changed, 66 insertions(+), 8 deletions(-)
> >
> > diff --git a/common/console.c b/common/console.c
> > index 12293f3..c37486c 100644
> > --- a/common/console.c
> > +++ b/common/console.c
> > @@ -690,14 +690,15 @@ int console_assign(int file, const char
> > *devname)
> >  /* Called before relocation - use serial functions */
> >  int console_init_f(void)
> >  {
> > -   gd->have_console = 1;
> > -
> > +   if(gd->uart_ready_for_console) {
> > +           gd->have_console = 1;
> >  #ifdef CONFIG_SILENT_CONSOLE
> > -   if (getenv("silent") != NULL)
> > -           gd->flags |= GD_FLG_SILENT;
> > +           if (getenv("silent") != NULL)
> > +                   gd->flags |= GD_FLG_SILENT;
> >  #endif
> >
> > -   print_pre_console_buffer(PRE_CONSOLE_FLUSHPOINT1_SERIAL);
> > +           print_pre_console_buffer(PRE_CONSOLE_FLUSHPOINT1_S
> > ERIAL);
> > +   }
> >
> >     return 0;
> >  }
> > diff --git a/common/spl/spl.c b/common/spl/spl.c
> > index b7ec333..c18827d 100644
> > --- a/common/spl/spl.c
> > +++ b/common/spl/spl.c
> > @@ -488,7 +488,8 @@ void preloader_console_init(void)
> >
> >     serial_init();          /* serial communications
> > setup */
> >
> > -   gd->have_console = 1;
> > +   if(gd->uart_ready_for_console)
> > +           gd->have_console = 1;
> >
> >     puts("\nU-Boot SPL " PLAIN_VERSION " (" U_BOOT_DATE " - "
> > \
> >                     U_BOOT_TIME ")\n");
> > diff --git a/drivers/serial/serial.c b/drivers/serial/serial.c
> > index f1bd15b..95690e0 100644
> > --- a/drivers/serial/serial.c
> > +++ b/drivers/serial/serial.c
> > @@ -35,6 +35,51 @@ static void serial_null(void)
> >  }
> >
> >  /**
> > + * uart_console_null() - This empty function would be used if UART
> > console checking
> > + *                       function is not compiled into U-boot.
> > + *
> > + * This routine returns true to indicate UART is ready for console
> > by default.
> > + * The driver is aliased to this empty function in case
> > + * the driver is not compiled into U-Boot.
> > + */
> > +static int uart_console_null(void)
> > +{
> > +   return 1;
> > +}
> > +
> > +/**
> > + * uart_console_func() - Forward declare of driver checking UART
> > is ready based
> > + *                          on device tree.
> > + * @name:  Name of driver which checking UART is ready based
> > on device tree .
> > + *
> > + * This macro expands onto forward declaration of a driver
> > checking
> > + * UART is ready based on device tree, which allows own assigned
> > function name.
> > + * The declaration is made weak and aliases to uart_console_null()
> > so in case
> > + * the driver is not compiled in, the function is still declared
> > and can
> > + * be used, but aliases to uart_console_null() and thus is
> > optimized away.
> > + */
> > +#define uart_console_func(name)
> > \
> > +   int name(const void *blob)
> >     \
> > +           __attribute__((weak, alias("uart_console_null")));
> > +
> > +uart_console_func(is_default_uart_console_true);
> > +
> > +/**
> > + * is_uart_console_true() - Common function to abstract the
> > specific UART
> > + *                     console checking function.
> > + *
> > + * This is common interface for specific UART console checking
> > function.
> > + */
> > +int is_uart_console_true(const void *blob)
> > +{
> > +   int ret = 0;
> > +
> > +   ret |= is_default_uart_console_true(blob);
> > +
> > +   return ret;
> > +}
> > +
> > +/**
> >   * on_baudrate() - Update the actual baudrate when the env var
> > changes
> >   *
> >   * This will check for a valid baudrate and only apply it if
> > valid.
> > @@ -415,8 +460,17 @@ static struct serial_device *get_current(void)
> >   */
> >  int serial_init(void)
> >  {
> > -   gd->flags |= GD_FLG_SERIAL_READY;
> > -   return get_current()->start();
> > +   if(is_uart_console_true(gd->fdt_blob))
> > +           gd->uart_ready_for_console = 1;
> > +   else
> > +           gd->uart_ready_for_console = 0;
> > +
> > +   if(gd->uart_ready_for_console) {
> > +           gd->flags |= GD_FLG_SERIAL_READY;
> > +           return get_current()->start();
> > +   }
> > +
> > +   return 0;
> >  }
> >
> >  /**
> > diff --git a/include/asm-generic/global_data.h b/include/asm
> > -generic/global_data.h
> > index a6d1d2a..1cb7ef7 100644
> > --- a/include/asm-generic/global_data.h
> > +++ b/include/asm-generic/global_data.h
> > @@ -44,6 +44,7 @@ typedef struct global_data {
> >  #ifdef CONFIG_BOARD_TYPES
> >     unsigned long board_type;
> >  #endif
> > +   unsigned long uart_ready_for_console; /* UART ready to
> > support console */
> >     unsigned long have_console;     /* serial_init() was
> > called */
> >  #ifdef CONFIG_PRE_CONSOLE_BUFFER
> >     unsigned long precon_buf_idx;   /* Pre-Console buffer
> > index */
> > diff --git a/include/serial.h b/include/serial.h
> > index 47332c5..a95bf70 100644
> > --- a/include/serial.h
> > +++ b/include/serial.h
> > @@ -50,6 +50,7 @@ extern void serial_initialize(void);
> >  extern void serial_stdio_init(void);
> >  extern int serial_assign(const char *name);
> >  extern void serial_reinit_all(void);
> > +extern int is_uart_console_true(const void *blob);
> >
> >  /* For usbtty */
> >  #ifdef CONFIG_USB_TTY
> >
>
>