Skip to contentSkip to conversation

Revert "Add board_pre_console_putc to deal with early console output"

PATCH New Simon Glass 3 participants 10 messages Mar 20, 2012 – Mar 23, 2012

Subsystems (4)
lore
Share & tools
lore
#0 lore raw

PATCH v2 1/5 Revert "Add board_pre_console_putc to deal with early console output"

This reverts commit 295d3942b806552503243f5cfb36aec6f1b5a9bf.

It turns that this really doesn't work very nicely. Instead we should have a pre-console panic function so that we know that further execution is impossible and we don't need to worry about trampling on UARTs, etc.

---

 README           |   17 -----------------
 common/console.c |   10 +---------
 include/common.h |    7 -------
 3 files changed, 1 insertions(+), 33 deletions(-)
README +0 -17
index 7adf7c7..84757a5 100644
@@ -638,23 +638,6 @@ The following options need to be configured:
 		'Sane' compilers will generate smaller code if
 		CONFIG_PRE_CON_BUF_SZ is a power of 2
 
-- Pre-console putc():
-		Prior to the console being initialised, console output is
-		normally silently discarded. This can be annoying if a
-		panic() happens in this time.
-
-		If the CONFIG_PRE_CONSOLE_PUTC option is defined, then
-		U-Boot will call board_pre_console_putc() for each output
-		character in this case, This function should try to output
-		the character if possible, perhaps on all available UARTs
-		(it will need to do this directly, since the console code
-		is not functional yet). Note that if the panic happens
-		early enough, then it is possible that board_init_f()
-		(or even arch_cpu_init() on ARM) has not been called yet.
-		You should init all clocks, GPIOs, etc. that are needed
-		to get the character out. Baud rates will need to default
-		to something sensible.
-
 - Safe printf() functions
 		Define CONFIG_SYS_VSNPRINTF to compile in safe versions of
 		the printf() functions. These are defined in
common/console.c +1 -9
index 1d9fd7f..1177f7d 100644
@@ -329,19 +329,14 @@ int tstc(void)
 	return serial_tstc();
 }
 
-#if defined(CONFIG_PRE_CONSOLE_BUFFER) || defined(CONFIG_PRE_CONSOLE_PUTC)
+#ifdef CONFIG_PRE_CONSOLE_BUFFER
 #define CIRC_BUF_IDX(idx) ((idx) % (unsigned long)CONFIG_PRE_CON_BUF_SZ)
 
 static void pre_console_putc(const char c)
 {
-#ifdef CONFIG_PRE_CONSOLE_BUFFER
 	char *buffer = (char *)CONFIG_PRE_CON_BUF_ADDR;
 
 	buffer[CIRC_BUF_IDX(gd->precon_buf_idx++)] = c;
-#endif
-#ifdef CONFIG_PRE_CONSOLE_PUTC
-	board_pre_console_putc(c);
-#endif
 }
 
 static void pre_console_puts(const char *s)
@@ -352,7 +347,6 @@ static void pre_console_puts(const char *s)
 
 static void print_pre_console_buffer(void)
 {
-#ifdef CONFIG_PRE_CONSOLE_BUFFER
 	unsigned long i = 0;
 	char *buffer = (char *)CONFIG_PRE_CON_BUF_ADDR;
 
@@ -361,9 +355,7 @@ static void print_pre_console_buffer(void)
 
 	while (i < gd->precon_buf_idx)
 		putc(buffer[CIRC_BUF_IDX(i++)]);
-#endif
 }
-
 #else
 static inline void pre_console_putc(const char c) {}
 static inline void pre_console_puts(const char *s) {}
include/common.h +0 -7
index 59e0b00..b588410 100644
@@ -285,13 +285,6 @@ extern ulong monitor_flash_len;
 int mac_read_from_eeprom(void);
 extern u8 _binary_dt_dtb_start[];	/* embedded device tree blob */
 
-/*
- * Called when console output is requested before the console is available.
- * The board should do its best to get the character out to the user any way
- * it can.
- */
-void board_pre_console_putc(int ch);
-
 /* common/flash.c */
 void flash_perror (int);
 
Signature · 1 line
1.7.7.3
#1 lore raw

PATCH v2 2/5 Add board_panic_no_console() to deal with early critical errors

This patch adds support for console output in the event of a panic() before the console is inited. The main purpose of this is to deal with a very early panic() which would otherwise cause a silent hang.

A new board_pre_console_panic() function is added to the board API. If provided by the board it will be called in the event of a panic before the console is ready. This function should turn on all available UARTs and output the string (plus a newline) if it possibly can.

---

 README           |   34 ++++++++++++++++++++++++++++++++++
 include/common.h |    8 ++++++++
 lib/vsprintf.c   |   25 +++++++++++++++++++++++--
 3 files changed, 65 insertions(+), 2 deletions(-)
README +34 -0
index 84757a5..38ce52a 100644
@@ -638,6 +638,40 @@ The following options need to be configured:
 		'Sane' compilers will generate smaller code if
 		CONFIG_PRE_CON_BUF_SZ is a power of 2
 
+- Pre-console panic():
+		Prior to the console being initialised, console output is
+		normally silently discarded. This can be annoying if a
+		panic() happens in this time. One reason for this might be
+		that you have CONFIG_OF_CONTROL defined but have not
+		provided a valid device tree for U-Boot. In general U-Boot's
+		console code cannot resolve this problem, since the console
+		has not been set up, and it may not be known which UART is
+		the console anyway (for example if this information is in
+		the device tree).
+
+		The solution is to define a function called
+		board_pre_console_panic() for your board, which alerts the
+		user however it can. Hopefuly this will involve displaying
+		a message on available UARTs, or perhaps at least flashing
+		an LED. The function should be very careful to only output
+		to UARTs that are known to be unused for peripheral
+		interfaces, and change GPIOs that will have no ill effect
+		on the board. That said, it is fine to do something
+		destructive that will prevent the board from continuing to
+		boot, since it isn't going to...
+
+		The function will need to output serial data directly,
+		since the console code is not functional yet. Note that if
+		the panic happens early enough, then it is possible that
+		board_init_f() (or even arch_cpu_init() on ARM) has not
+		been called yet. You should init all clocks, GPIOs, etc.
+		that are needed to get the string out. Baud rates will need
+		to default to something sensible.
+
+		If your function returns, then the normal panic processing
+		will occur (it will either hang or reset depending on
+		CONFIG_PANIC_HANG).
+
 - Safe printf() functions
 		Define CONFIG_SYS_VSNPRINTF to compile in safe versions of
 		the printf() functions. These are defined in
include/common.h +8 -0
index b588410..9db3a6a 100644
@@ -285,6 +285,14 @@ extern ulong monitor_flash_len;
 int mac_read_from_eeprom(void);
 extern u8 _binary_dt_dtb_start[];	/* embedded device tree blob */
 
+/*
+ * Called during a panic() when no console is available. The board should do
+ * its best to get a message to the user any way it can. This function should
+ * return if it can, in which case the system will either hang or reset.
+ * See panic().
+ */
+void board_pre_console_panic(const char *str);
+
 /* common/flash.c */
 void flash_perror (int);
 
lib/vsprintf.c +23 -2
index e38a4b7..a478ff5ab 100644
@@ -33,6 +33,9 @@ const char hex_asc[] = "0123456789abcdef";
 #define hex_asc_lo(x)   hex_asc[((x) & 0x0f)]
 #define hex_asc_hi(x)   hex_asc[((x) & 0xf0) >> 4]
 
+DECLARE_GLOBAL_DATA_PTR;
+
+
 static inline char *pack_hex_byte(char *buf, u8 byte)
 {
 	*buf++ = hex_asc_hi(byte);
@@ -777,13 +780,31 @@ int sprintf(char * buf, const char *fmt, ...)
 	return i;
 }
 
+/* Provide a default function for when no console is available */
+static void __board_pre_console_panic(const char *msg)
+{
+	/* Just return since we can't access the console */
+}
+
+void board_pre_console_panic(const char *msg) __attribute__((weak,
+					alias("__board_pre_console_panic")));
+
 void panic(const char *fmt, ...)
 {
+	char str[CONFIG_SYS_PBSIZE];
 	va_list	args;
+
 	va_start(args, fmt);
-	vprintf(fmt, args);
-	putc('\n');
+	vsnprintf(str, sizeof(str), fmt, args);
 	va_end(args);
+
+	if (gd->have_console) {
+		puts(str);
+		putc('\n');
+	} else {
+		board_pre_console_panic(str);
+	}
+
 #if defined (CONFIG_PANIC_HANG)
 	hang();
 #else
Signature · 1 line
1.7.7.3
#2 lore raw

PATCH v2 3/5 tegra: Export the UART setup function for use by boards

In reply to Simon Glass

Allow boards to call the tegra_setup_uarts() function so that they can set up UARTs on demand. The UART selection enum is moved into the board.h header file so that boards can use this for pre-console panic.

---

 arch/arm/cpu/armv7/tegra2/board.c        |   26 +++++++-------------------
 arch/arm/include/asm/arch-tegra2/board.h |   17 +++++++++++++++++
 2 files changed, 24 insertions(+), 19 deletions(-)
arch/arm/cpu/armv7/tegra2/board.c +7 -19
index 77a627d..c9a7520 100644
@@ -24,6 +24,7 @@
 #include <common.h>
 #include <asm/io.h>
 #include "ap20.h"
+#include <asm/arch/board.h>
 #include <asm/arch/clock.h>
 #include <asm/arch/funcmux.h>
 #include <asm/arch/sys_proto.h>
@@ -32,14 +33,6 @@
 
 DECLARE_GLOBAL_DATA_PTR;
 
-enum {
-	/* UARTs which we can enable */
-	UARTA	= 1 << 0,
-	UARTB	= 1 << 1,
-	UARTD	= 1 << 3,
-	UART_COUNT = 4,
-};
-
 /*
  * Boot ROM initializes the odmdata in APBDEV_PMC_SCRATCH20_0,
  * so we are using this value to identify memory size.
@@ -101,12 +94,7 @@ int arch_cpu_init(void)
 }
 #endif
 
-/**
- * Set up the specified uarts
- *
- * @param uarts_ids	Mask containing UARTs to init (UARTx)
- */
-static void setup_uarts(int uart_ids)
+void tegra_setup_uarts(int uart_ids)
 {
 	static enum periph_id id_for_uart[] = {
 		PERIPH_ID_UART1,
@@ -116,7 +104,7 @@ static void setup_uarts(int uart_ids)
 	};
 	size_t i;
 
-	for (i = 0; i < UART_COUNT; i++) {
+	for (i = 0; i < TEGRA_UART_COUNT; i++) {
 		if (uart_ids & (1 << i)) {
 			enum periph_id id = id_for_uart[i];
 
@@ -131,15 +119,15 @@ void board_init_uart_f(void)
 	int uart_ids = 0;	/* bit mask of which UART ids to enable */
 
 #ifdef CONFIG_TEGRA2_ENABLE_UARTA
-	uart_ids |= UARTA;
+	uart_ids |= TEGRA_UARTA;
 #endif
 #ifdef CONFIG_TEGRA2_ENABLE_UARTB
-	uart_ids |= UARTB;
+	uart_ids |= TEGRA_UARTB;
 #endif
 #ifdef CONFIG_TEGRA2_ENABLE_UARTD
-	uart_ids |= UARTD;
+	uart_ids |= TEGRA_UARTD;
 #endif
-	setup_uarts(uart_ids);
+	tegra_setup_uarts(uart_ids);
 }
 
 #ifndef CONFIG_SYS_DCACHE_OFF
arch/arm/include/asm/arch-tegra2/board.h +17 -0
index a90d36c..fb88517 100644
@@ -24,6 +24,23 @@
 #ifndef _TEGRA_BOARD_H_
 #define _TEGRA_BOARD_H_
 
+enum {
+	/* UARTs which we can enable */
+	TEGRA_UARTA	= 1 << 0,
+	TEGRA_UARTB	= 1 << 1,
+	TEGRA_UARTD	= 1 << 3,
+	TEGRA_UART_ALL	= 0xf,
+
+	TEGRA_UART_COUNT = 4,
+};
+
+/**
+ * Set up the specified UARTs (pinmux and clocks)
+ *
+ * @param uarts_ids	Mask containing UARTs to init (see TEGRA_UARTx)
+ */
+void tegra_setup_uarts(int uart_ids);
+
 /* Setup UARTs for the board according to the selected config */
 void board_init_uart_f(void);
 
Signature · 1 line
1.7.7.3
#3 lore raw

PATCH v2 4/5 tegra: Provide tegra_pre_console_panic() for early panics

In reply to Simon Glass

This function is made available to board which want to display a message when a panic() occurs before the console is set up. This would otherwise result in a silent hang or reboot.

Boards should call tegra_pre_console_panic() and pass the UARTs which are available and safe for a message, as well as the selected clock and serial multiplier values. Defaults are available as CONFIG_DEFAULT_NS16550_CLK and CONFIG_DEFAULT_NS16550_MULT.

---

 arch/arm/cpu/armv7/tegra2/board.c        |   48 ++++++++++++++++++++++++++++++
 arch/arm/include/asm/arch-tegra2/board.h |   13 ++++++++
 include/configs/tegra2-common.h          |    4 ++
 3 files changed, 65 insertions(+), 0 deletions(-)
arch/arm/cpu/armv7/tegra2/board.c +48 -0
index c9a7520..54963d0 100644
@@ -22,6 +22,7 @@
  */
 
 #include <common.h>
+#include <ns16550.h>
 #include <asm/io.h>
 #include "ap20.h"
 #include <asm/arch/board.h>
@@ -137,3 +138,50 @@ void enable_caches(void)
 	dcache_enable();
 }
 #endif
+
+/*
+ * Possible UART locations: we ignore UARTC at 0x70006200 and UARTE at
+ * 0x70006400, since we don't have code to init them
+ */
+static const u32 uart_reg_addr[TEGRA_UART_COUNT] = {
+	NV_PA_APB_UARTA_BASE,
+	NV_PA_APB_UARTB_BASE,
+	0,
+	NV_PA_APB_UARTD_BASE,
+};
+
+void tegra_pre_console_panic(int uart_ids, unsigned clock_freq,
+			     unsigned multiplier, const char *str)
+{
+	int baudrate, divisor;
+	int i;
+
+	/* Enable all permitted UARTs */
+	tegra_setup_uarts(uart_ids);
+
+	/*
+	 * Now send the string out all the selected  UARTs. We don't try all
+	 * possible configurations, but this could be added if required.
+	 */
+	baudrate = CONFIG_BAUDRATE;
+	divisor = (clock_freq + (baudrate * (multiplier / 2))) /
+			(multiplier * baudrate);
+
+	for (i = 0; i < TEGRA_UART_COUNT; i++) {
+		if (uart_ids & (1 << i)) {
+			NS16550_t regs = (NS16550_t)uart_reg_addr[i];
+			const char *s;
+
+			if (!regs)
+				continue;
+			NS16550_init(regs, divisor);
+			for (s = str; *s; s++) {
+				NS16550_putc(regs, *s);
+				if (*s == '\n')
+					NS16550_putc(regs, '\r');
+			}
+			NS16550_putc(regs, '\n');
+			NS16550_putc(regs, '\r');
+		}
+	}
+}
arch/arm/include/asm/arch-tegra2/board.h +13 -0
index fb88517..fd2489f 100644
@@ -41,6 +41,19 @@ enum {
  */
 void tegra_setup_uarts(int uart_ids);
 
+/**
+ * Display a panic message on selected UARTs.
+ *
+ * This is called when we have no console yet but have hit a panic(). It
+ * is normally called from board_pre_console_panic(), which passes in the
+ * UARTs that we are permitted to output to.
+ *
+ * We display a message on each UART in the hope that one will reach the
+ * user.
+ */
+void tegra_pre_console_panic(int uart_ids, unsigned clock_freq,
+			     unsigned multiplier, const char *str);
+
 /* Setup UARTs for the board according to the selected config */
 void board_init_uart_f(void);
 
include/configs/tegra2-common.h +4 -0
index 837f859..14d7602 100644
@@ -84,6 +84,10 @@
 #define CONFIG_SYS_BAUDRATE_TABLE	{4800, 9600, 19200, 38400, 57600,\
 					115200}
 
+/* Default serial clock and multiplier */
+#define CONFIG_DEFAULT_NS16550_CLK	V_NS16550_CLK
+#define CONFIG_DEFAULT_NS16550_MULT	16
+
 /*
  * This parameter affects a TXFILLTUNING field that controls how much data is
  * sent to the latency fifo before it is sent to the wire. Without this
Signature · 1 line
1.7.7.3
#4 lore raw

PATCH v2 5/5 tegra: Implement board_pre_console_panic() for Seaboard

In reply to Simon Glass

We enable this feature on UARTD for Seaboard. This ensures that a message is printed if CONFIG_OF_CONTROL is in use and a valid device tree is not available (provided the Seaboard variant uses UARTD for console).

---
Changes in v2:
- Correctly use only selected UARTs in tegra_pre_console_panic()
- Only use UARTD for Seaboard

 board/nvidia/seaboard/seaboard.c |   18 ++++++++++++++++++
 1 files changed, 18 insertions(+), 0 deletions(-)
board/nvidia/seaboard/seaboard.c +18 -0
index 94efb1e..1862162 100644
@@ -24,6 +24,7 @@
 #include <common.h>
 #include <asm/io.h>
 #include <asm/arch/tegra2.h>
+#include <asm/arch/board.h>
 #include <asm/arch/clock.h>
 #include <asm/arch/funcmux.h>
 #include <asm/arch/pinmux.h>
@@ -96,3 +97,20 @@ void pin_mux_usb(void)
 	/* For USB's GPIO PD0. For now, since we have no pinmux in fdt */
 	pinmux_tristate_disable(PINGRP_SLXK);
 }
+
+/*
+ * This is called when we have no console. About the only reason that this
+ * happen is if we don't have a valid fdt. So we don't know what kind of
+ * Tegra board we are. We blindly try to print a message every which way we
+ * know.
+ */
+void board_pre_console_panic(const char *str)
+{
+	/* Seaboard has a UART switch on PI3 */
+#ifdef CONFIG_SPI_UART_SWITCH
+	gpio_direction_output(GPIO_PI3, 0);
+#endif
+
+	tegra_pre_console_panic(TEGRA_UARTD, CONFIG_DEFAULT_NS16550_CLK,
+				CONFIG_DEFAULT_NS16550_MULT, str);
+}
Signature · 1 line
1.7.7.3
#5 lore raw

PATCH v2 1/5 Revert "Add board_pre_console_putc to deal with early console output"

In reply to Simon Glass
Quoted 5 lines · Simon Glass

This reverts commit 295d3942b806552503243f5cfb36aec6f1b5a9bf.

It turns that this really doesn't work very nicely. Instead we should have a pre-console panic function so that we know that further execution is impossible and we don't need to worry about trampling on UARTs, etc.

The series:

For reference, I took u-boot-tegra/master, removed the top 4 commits that were the previous putc changes, then applied the 5 commits in this series.

Tom (assuming everyone else is OK with this series), when you apply this series, can you put it /before/ the USB/device-tree patches in your branch, so that there are no points in git history where the USB/DT code is active, but this code is not. In other words, apply these 5 commits, then use "git rebase -i" to re-order the patches so this series is before all the USB/DT changes. Thanks.

#6 lore raw

PATCH v2 1/5 Revert "Add board_pre_console_putc to deal with early console output"

Hi Wolfgang,

Quoted 10 lines · Stephen Warren
Quoted 5 lines · Simon Glass

This reverts commit 295d3942b806552503243f5cfb36aec6f1b5a9bf.

It turns that this really doesn't work very nicely. Instead we should have a pre-console panic function so that we know that further execution is impossible and we don't need to worry about trampling on UARTs, etc.

The series:

If you have concerns with this new approach please sing out.

Quoted 11 lines

For reference, I took u-boot-tegra/master, removed the top 4 commits that were the previous putc changes, then applied the 5 commits in this series.

Tom (assuming everyone else is OK with this series), when you apply this series, can you put it /before/ the USB/device-tree patches in your branch, so that there are no points in git history where the USB/DT code is active, but this code is not. In other words, apply these 5 commits, then use "git rebase -i" to re-order the patches so this series is before all the USB/DT changes. Thanks.

Regards,
Simon

#7 lore raw

PATCH v2 1/5 Revert "Add board_pre_console_putc to deal with early console output"

In reply to Simon Glass

Dear Simon Glass,

Quoted 7 lines · In message <[email protected]> you wrote:

This reverts commit 295d3942b806552503243f5cfb36aec6f1b5a9bf.

It turns that this really doesn't work very nicely. Instead we should have a pre-console panic function so that we know that further execution is impossible and we don't need to worry about trampling on UARTs, etc.

You failed to provide thw mandatory change log - it is impossible to know what you changend in this V2 of the patches compared to the previous verison.

I assume that no changes were made and all review comments still apply (at least the ones I made about design and concept do).

Please stick to http://www.denx.de/wiki/U-Boot/Patches and especially to
http://www.denx.de/wiki/view/U-Boot/Patches#Sending_updated_patch_versions

Patches ignored.

Best regards,

Wolfgang Denk

Signature · 6 lines
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
"Do we define evil as the absence of goodness? It seems only  logical
that shit happens--we discover this by the process of elimination."
                                                        -- Larry Wall
#8 lore raw

PATCH v2 1/5 Revert "Add board_pre_console_putc to deal with early console output"

Hi Wolfgang,

Quoted 15 lines · Wolfgang Denk

Dear Simon Glass,

Quoted 7 lines · In message <[email protected]> you wrote:

This reverts commit 295d3942b806552503243f5cfb36aec6f1b5a9bf.

It turns that this really doesn't work very nicely. Instead we should have a pre-console panic function so that we know that further execution is impossible and we don't need to worry about trampling on UARTs, etc.

You failed to provide thw mandatory change log - it is impossible to know what you changend in this V2 of the patches compared to the previous verison.

Quoted 6 lines

I assume that no changes were made and all review comments still apply (at least the ones I made about design and concept do).

Please stick to http://www.denx.de/wiki/U-Boot/Patches and especially to
http://www.denx.de/wiki/view/U-Boot/Patches#Sending_updated_patch_versions

There was no cover letter with this, which might have helped.

However your assumption is correct - the only changes to the series were in the final patch (where there is a change log) and the addition of the new 'tegra_pre_console_panic' patch (which should have had 'new patch' in the change log).

Quoted 2 lines

Patches ignored.

Fair enough, you provided the comments I was looking for anyway,

Regards,
Simon

Quoted 12 lines

Best regards,

Wolfgang Denk

--
DENX Software Engineering GmbH, ? ? MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de "Do we define evil as the absence of goodness? It seems only ?logical that shit happens--we discover this by the process of elimination." ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?-- Larry Wall

#9 lore raw

PATCH v2 1/5 Revert "Add board_pre_console_putc to deal with early console output"

In reply to Simon Glass

Dear Simon,

Quoted 6 lines · In message <[email protected]> you wrote:

This reverts commit 295d3942b806552503243f5cfb36aec6f1b5a9bf.

It turns that this really doesn't work very nicely. Instead we should have a pre-console panic function so that we know that further execution is impossible and we don't need to worry about trampling on UARTs, etc.

Quoted 7 lines

---

 README           |   17 -----------------
 common/console.c |   10 +---------
 include/common.h |    7 -------
 3 files changed, 1 insertions(+), 33 deletions(-)

Applied, thanks.

Best regards,

Wolfgang Denk

Signature · 4 lines
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
If all you have is a hammer, everything looks like a nail.