[U-Boot] [PATCH] serial: Enable checking UART is ready for console based on device tree
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.
Signed-off-by: Tien Fong Chee tfchee@altera.com Cc: Dinh Nguyen dinguyen@opensource.altera.com Cc: Dinh Nguyen dinh.linux@gmail.com Cc: ChinLiang clsee@altera.com Cc: Thomas Chou thomas@wytron.com.tw Cc: Simon Glass sjg@chromium.org Cc: Marek Vasut marex@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_SERIAL); + }
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
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.
Signed-off-by: Tien Fong Chee tfchee@altera.com Cc: Dinh Nguyen dinguyen@opensource.altera.com Cc: Dinh Nguyen dinh.linux@gmail.com Cc: ChinLiang clsee@altera.com Cc: Thomas Chou thomas@wytron.com.tw Cc: Simon Glass sjg@chromium.org Cc: Marek Vasut marex@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_SERIAL);}
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
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@altera.com Cc: Dinh Nguyen dinguyen@opensource.altera.com Cc: Dinh Nguyen dinh.linux@gmail.com Cc: ChinLiang clsee@altera.com Cc: Thomas Chou thomas@wytron.com.tw Cc: Simon Glass sjg@chromium.org Cc: Marek Vasut marex@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_SERIAL);
}
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 communicationssetup */
- 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
Hi,
On 26 July 2016 at 04:55, Tien Fong Chee tfchee@altera.com 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.
Signed-off-by: Tien Fong Chee tfchee@altera.com Cc: Dinh Nguyen dinguyen@opensource.altera.com Cc: Dinh Nguyen dinh.linux@gmail.com Cc: ChinLiang clsee@altera.com Cc: Thomas Chou thomas@wytron.com.tw Cc: Simon Glass sjg@chromium.org Cc: Marek Vasut marex@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(-)
Another way to deal with this would be to have the driver return -EBUSY (or perhaps -EAGAIN) and set this flag in serial_find_console_or_panic(). After all, the unavailability of serial is a driver-specific thing, right?
How / when does the FPGA get programmed on your system?
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_SERIAL);} 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;elsegd->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
1.7.7.4
Regards, Simon
On 08/01/2016 03:01 AM, Simon Glass wrote:
Hi,
On 26 July 2016 at 04:55, Tien Fong Chee tfchee@altera.com 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.
Signed-off-by: Tien Fong Chee tfchee@altera.com Cc: Dinh Nguyen dinguyen@opensource.altera.com Cc: Dinh Nguyen dinh.linux@gmail.com Cc: ChinLiang clsee@altera.com Cc: Thomas Chou thomas@wytron.com.tw Cc: Simon Glass sjg@chromium.org Cc: Marek Vasut marex@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(-)
Another way to deal with this would be to have the driver return -EBUSY (or perhaps -EAGAIN) and set this flag in serial_find_console_or_panic(). After all, the unavailability of serial is a driver-specific thing, right?
I think the -EAGAIN makes sense -- use default null console and probe the UART when it's ready.
How / when does the FPGA get programmed on your system?
On CV/AV, the FPGA is loaded via the fpga command .
participants (3)
-
Marek Vasut -
Simon Glass -
Tien Fong Chee