Patchwork [U-Boot,v2,1/5] Revert "Add board_pre_console_putc to deal with early console output"

login
register
mail settings
Submitter Simon Glass
Date March 20, 2012, 4:59 a.m.
Message ID <1332219558-9143-1-git-send-email-sjg@chromium.org>
Download mbox | patch
Permalink /patch/147740/
State New, archived
Headers show

Comments

Simon Glass - March 20, 2012, 4:59 a.m.
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.

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

 README           |   17 -----------------
 common/console.c |   10 +---------
 include/common.h |    7 -------
 3 files changed, 1 insertions(+), 33 deletions(-)
Stephen Warren - March 20, 2012, 7:03 p.m.
On 03/19/2012 10:59 PM, Simon Glass 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.

The series:

Tested-by: Stephen Warren <swarren@wwwdotorg.org>

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.
Simon Glass - March 21, 2012, 2:19 a.m.
Hi Wolfgang,

On Tue, Mar 20, 2012 at 12:03 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
> On 03/19/2012 10:59 PM, Simon Glass 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.
>
> The series:
>
> Tested-by: Stephen Warren <swarren@wwwdotorg.org>

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

>
> 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
Wolfgang Denk - March 21, 2012, 9:13 a.m.
Dear Simon Glass,

In message <1332219558-9143-1-git-send-email-sjg@chromium.org> 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.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>

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
Simon Glass - March 21, 2012, 4:48 p.m.
Hi Wolfgang,

On Wed, Mar 21, 2012 at 2:13 AM, Wolfgang Denk <wd@denx.de> wrote:
> Dear Simon Glass,
>
> In message <1332219558-9143-1-git-send-email-sjg@chromium.org> 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.
>>
>> Signed-off-by: Simon Glass <sjg@chromium.org>
>
> 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

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).

>
> Patches ignored.

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

Regards,
Simon

>
> 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@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
Wolfgang Denk - March 23, 2012, 8:13 p.m.
Dear Simon,

In message <1332219558-9143-1-git-send-email-sjg@chromium.org> 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.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
> 
>  README           |   17 -----------------
>  common/console.c |   10 +---------
>  include/common.h |    7 -------
>  3 files changed, 1 insertions(+), 33 deletions(-)

Applied, thanks.

Best regards,

Wolfgang Denk

Patch

diff --git a/README b/README
index 7adf7c7..84757a5 100644
--- a/README
+++ b/README
@@ -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
diff --git a/common/console.c b/common/console.c
index 1d9fd7f..1177f7d 100644
--- a/common/console.c
+++ b/common/console.c
@@ -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) {}
diff --git a/include/common.h b/include/common.h
index 59e0b00..b588410 100644
--- a/include/common.h
+++ b/include/common.h
@@ -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);