* [U-Boot] [PATCH v2 2/4] Enable printf() console if pre-console putc() is available
2012-03-09 20:32 [U-Boot] [PATCH v2 1/4] ns16550: Add function to drain serial output Simon Glass
@ 2012-03-09 20:32 ` Simon Glass
2012-03-09 20:32 ` [U-Boot] [PATCH v2 3/4] tegra: Implement pre-console putc() for fdt warning Simon Glass
` (5 subsequent siblings)
6 siblings, 0 replies; 22+ messages in thread
From: Simon Glass @ 2012-03-09 20:32 UTC (permalink / raw)
To: u-boot
At present we skip printf() if there is no hope of it making it to the
console. But this check ignores CONFIG_PRE_CONSOLE_PUTC at present.
Add a check for that also.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
Changes in v2:
- Allow printf() output to make it to pre-console putc()
common/console.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/common/console.c b/common/console.c
index 1d9fd7f..38d7094 100644
--- a/common/console.c
+++ b/common/console.c
@@ -447,7 +447,7 @@ int vprintf(const char *fmt, va_list args)
uint i;
char printbuffer[CONFIG_SYS_PBSIZE];
-#ifndef CONFIG_PRE_CONSOLE_BUFFER
+#if !defined(CONFIG_PRE_CONSOLE_BUFFER) && !defined(CONFIG_PRE_CONSOLE_PUTC)
if (!gd->have_console)
return 0;
#endif
--
1.7.7.3
^ permalink raw reply related [flat|nested] 22+ messages in thread* [U-Boot] [PATCH v2 3/4] tegra: Implement pre-console putc() for fdt warning
2012-03-09 20:32 [U-Boot] [PATCH v2 1/4] ns16550: Add function to drain serial output Simon Glass
2012-03-09 20:32 ` [U-Boot] [PATCH v2 2/4] Enable printf() console if pre-console putc() is available Simon Glass
@ 2012-03-09 20:32 ` Simon Glass
2012-03-10 8:16 ` Wolfgang Denk
2012-03-09 20:32 ` [U-Boot] [PATCH v2 4/4] tegra: Enable pre-console putc() for Tegra boards Simon Glass
` (4 subsequent siblings)
6 siblings, 1 reply; 22+ messages in thread
From: Simon Glass @ 2012-03-09 20:32 UTC (permalink / raw)
To: u-boot
When there is not device tree file available to U-Boot, we panic.
Implement board_pre_console_putc() so that this panic will be displayed
on the serial console.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
Changes in v2:
- Tidy up casting in board_pre_console_putc()
- Use 'const' for uart_reg_addr
arch/arm/cpu/armv7/tegra2/board.c | 61 +++++++++++++++++++++++++++++++++++++
1 files changed, 61 insertions(+), 0 deletions(-)
diff --git a/arch/arm/cpu/armv7/tegra2/board.c b/arch/arm/cpu/armv7/tegra2/board.c
index 77a627d..0517d72 100644
--- a/arch/arm/cpu/armv7/tegra2/board.c
+++ b/arch/arm/cpu/armv7/tegra2/board.c
@@ -23,9 +23,11 @@
#include <common.h>
#include <asm/io.h>
+#include <ns16550.h>
#include "ap20.h"
#include <asm/arch/clock.h>
#include <asm/arch/funcmux.h>
+#include <asm/arch/gpio.h>
#include <asm/arch/sys_proto.h>
#include <asm/arch/tegra2.h>
#include <asm/arch/pmc.h>
@@ -37,6 +39,7 @@ enum {
UARTA = 1 << 0,
UARTB = 1 << 1,
UARTD = 1 << 3,
+ UART_ALL = 0xf,
UART_COUNT = 4,
};
@@ -149,3 +152,61 @@ void enable_caches(void)
dcache_enable();
}
#endif
+
+
+/*
+ * Possible UART locations: we ignore UARTC@0x70006200 and UARTE at
+ * 0x70006400, since we don't have code to init them
+ */
+static const u32 uart_reg_addr[] = {
+ NV_PA_APB_UARTA_BASE,
+ NV_PA_APB_UARTB_BASE,
+ NV_PA_APB_UARTD_BASE,
+ 0
+};
+
+#ifdef CONFIG_PRE_CONSOLE_PUTC
+/*
+ * 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_putc(int ch)
+{
+ int uart_ids = UART_ALL; /* turn it all on! */
+ const u32 *uart_addr;
+ int clock_freq, multiplier, baudrate, divisor;
+
+ /* Try to enable all possible UARTs */
+ setup_uarts(uart_ids);
+
+ /*
+ * Seaboard has a UART switch on PI3. We might be a Seaboard,
+ * so flip it!
+ */
+#ifdef CONFIG_SPI_UART_SWITCH
+ gpio_direction_output(GPIO_PI3, 0);
+#endif
+
+ /*
+ * Now send the string out all the Tegra UARTs. We don't try all
+ * possible configurations, but this could be added if required.
+ */
+ clock_freq = CONFIG_DEFAULT_NS16550_CLK;
+ multiplier = CONFIG_DEFAULT_NS16550_MULT;
+ baudrate = CONFIG_BAUDRATE;
+ divisor = (clock_freq + (baudrate * (multiplier / 2))) /
+ (multiplier * baudrate);
+
+ for (uart_addr = uart_reg_addr; *uart_addr; uart_addr++) {
+ NS16550_t regs = (NS16550_t)*uart_addr;
+
+ NS16550_init(regs, divisor);
+ NS16550_putc(regs, ch);
+ if (ch == '\n')
+ NS16550_putc(regs, '\r');
+ NS16550_drain(regs);
+ }
+}
+#endif
--
1.7.7.3
^ permalink raw reply related [flat|nested] 22+ messages in thread* [U-Boot] [PATCH v2 3/4] tegra: Implement pre-console putc() for fdt warning
2012-03-09 20:32 ` [U-Boot] [PATCH v2 3/4] tegra: Implement pre-console putc() for fdt warning Simon Glass
@ 2012-03-10 8:16 ` Wolfgang Denk
2012-03-10 19:22 ` Simon Glass
0 siblings, 1 reply; 22+ messages in thread
From: Wolfgang Denk @ 2012-03-10 8:16 UTC (permalink / raw)
To: u-boot
Dear Simon Glass,
In message <1331325178-14634-3-git-send-email-sjg@chromium.org> you wrote:
> When there is not device tree file available to U-Boot, we panic.
> Implement board_pre_console_putc() so that this panic will be displayed
> on the serial console.
...
> +void board_pre_console_putc(int ch)
> +{
...
> + for (uart_addr = uart_reg_addr; *uart_addr; uart_addr++) {
> + NS16550_t regs = (NS16550_t)*uart_addr;
> +
> + NS16550_init(regs, divisor);
> + NS16550_putc(regs, ch);
> + if (ch == '\n')
> + NS16550_putc(regs, '\r');
> + NS16550_drain(regs);
Why is this needed for every output character?
Actually, why is it needed at all?
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
All a hacker needs is a tight PUSHJ, a loose pair of UUOs, and a warm
place to shift.
^ permalink raw reply [flat|nested] 22+ messages in thread* [U-Boot] [PATCH v2 3/4] tegra: Implement pre-console putc() for fdt warning
2012-03-10 8:16 ` Wolfgang Denk
@ 2012-03-10 19:22 ` Simon Glass
2012-03-10 20:08 ` Wolfgang Denk
0 siblings, 1 reply; 22+ messages in thread
From: Simon Glass @ 2012-03-10 19:22 UTC (permalink / raw)
To: u-boot
Hi Wolfgang,
On Sat, Mar 10, 2012 at 12:16 AM, Wolfgang Denk <wd@denx.de> wrote:
> Dear Simon Glass,
>
> In message <1331325178-14634-3-git-send-email-sjg@chromium.org> you wrote:
>> When there is not device tree file available to U-Boot, we panic.
>> Implement board_pre_console_putc() so that this panic will be displayed
>> on the serial console.
> ...
>
>> +void board_pre_console_putc(int ch)
>> +{
> ...
>> + ? ? for (uart_addr = uart_reg_addr; *uart_addr; uart_addr++) {
>> + ? ? ? ? ? ? NS16550_t regs = (NS16550_t)*uart_addr;
>> +
>> + ? ? ? ? ? ? NS16550_init(regs, divisor);
>> + ? ? ? ? ? ? NS16550_putc(regs, ch);
>> + ? ? ? ? ? ? if (ch == '\n')
>> + ? ? ? ? ? ? ? ? ? ? NS16550_putc(regs, '\r');
>> + ? ? ? ? ? ? NS16550_drain(regs);
>
> Why is this needed for every output character?
>
> Actually, why is it needed at all?
Of course in this case the init could be done for each UART at the
start of the function rather than in the loop, by looping through the
UARTs twice.
More generally, I will explain both of these patches in this email.
The genesis of this was a requirement to print some sort of message
when things go horribly wrong in early init. This could be in SPL code
when we get an error loading U-Boot, or in board_init_f() when using
CONFIG_OF_CONTROL and we find there is no device tree.
At present in the case of a missing fdt the board is just dead, there
is no message and no indication of what the user should do to sort it
out. The normal console code will not get set up, since U-Boot does
not know which UART is the console. There is a panic() in
fdtdec_check_fdt() to make sure that we don't blindly continue and do
strange things in this case. But since there is no console, the
panic() displays nothing.
My original idea was board_panic_no_console() which boards or SOCs
could implement to print a message on all available UARTs or display,
flash lights, etc. to indicate that something went badly wrong.
The initial patch for board_panic_no_console() was here:
http://lists.denx.de/pipermail/u-boot/2011-August/099619.html
After requests on the list for general purpose pre-console output
function (the purpose of which I didn't necessary see) I changed this
to a putc() mechanism. This means that we need to set up the UARTs
each time it is called. We can't really add a flag to global data
since this might be called before that is even set up. There might be
a better way, but I'm not sure what it is.
For SPL I would like to be able use this same mechanism to call
panic() when something goes wrong, and use the same mechanism to get a
message to the user. Again, this avoids a bricked unit with no failure
indication.
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 at denx.de
> All a hacker needs is a tight PUSHJ, a loose pair of UUOs, and a warm
> place to shift.
^ permalink raw reply [flat|nested] 22+ messages in thread* [U-Boot] [PATCH v2 3/4] tegra: Implement pre-console putc() for fdt warning
2012-03-10 19:22 ` Simon Glass
@ 2012-03-10 20:08 ` Wolfgang Denk
2012-03-10 21:25 ` Simon Glass
0 siblings, 1 reply; 22+ messages in thread
From: Wolfgang Denk @ 2012-03-10 20:08 UTC (permalink / raw)
To: u-boot
Dear Simon Glass,
In message <CAPnjgZ1=KK9UNy2fyk9WJ-gcyYE5JgwqO_cT8igXjD-OFP_h6w@mail.gmail.com> you wrote:
>
> >> +void board_pre_console_putc(int ch)
> >> +{
> > ...
> >> + for (uart_addr = uart_reg_addr; *uart_addr; uart_addr++) {
> >> + NS16550_t regs = (NS16550_t)*uart_addr;
> >> +
> >> + NS16550_init(regs, divisor);
> >> + NS16550_putc(regs, ch);
> >> + if (ch == '\n')
> >> + NS16550_putc(regs, '\r');
> >> + NS16550_drain(regs);
> >
> > Why is this needed for every output character?
> >
> > Actually, why is it needed at all?
>
> Of course in this case the init could be done for each UART at the
> start of the function rather than in the loop, by looping through the
> UARTs twice.
Both the init() and the drain() should never be used in a character
output loop.
> After requests on the list for general purpose pre-console output
> function (the purpose of which I didn't necessary see) I changed this
> to a putc() mechanism. This means that we need to set up the UARTs
> each time it is called. We can't really add a flag to global data
> since this might be called before that is even set up. There might be
> a better way, but I'm not sure what it is.
>
> For SPL I would like to be able use this same mechanism to call
> panic() when something goes wrong, and use the same mechanism to get a
> message to the user. Again, this avoids a bricked unit with no failure
> indication.
I understand your intention, but I disagree with the design, and with
the implementation.
I think outputting data to all "potential" console ports is a really
bad thing, as you cannot know how your users are using the hardware.
They may have attached hardware to the UARTs, and the data you send to
the port causes a mis-function of the device. I guess you don't add
to your documentation a warning like: select one port as console, and
leave allother ports unused, because we may send random date to all
ports any time?
If you do not know which UART port to use, then the only consequence
can be not to use any UART prt at all. Use a LED with blink codes or
whatever your hardwar ehas, but do not mess with random ports.
I also cannot understand why you think you must init() and drain() the
UART for each character printed - ok, the latter is probably a
consequence of re-initializing the port for each character.
Eventually this will be not needed once you get rid of the re-init.
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
"Consistency requires you to be as ignorant today as you were a year
ago." - Bernard Berenson
^ permalink raw reply [flat|nested] 22+ messages in thread* [U-Boot] [PATCH v2 3/4] tegra: Implement pre-console putc() for fdt warning
2012-03-10 20:08 ` Wolfgang Denk
@ 2012-03-10 21:25 ` Simon Glass
2012-03-10 22:49 ` Wolfgang Denk
0 siblings, 1 reply; 22+ messages in thread
From: Simon Glass @ 2012-03-10 21:25 UTC (permalink / raw)
To: u-boot
+Graeme
Hi Wolfgang,
On Sat, Mar 10, 2012 at 12:08 PM, Wolfgang Denk <wd@denx.de> wrote:
> Dear Simon Glass,
>
> In message <CAPnjgZ1=KK9UNy2fyk9WJ-gcyYE5JgwqO_cT8igXjD-OFP_h6w@mail.gmail.com> you wrote:
>>
>> >> +void board_pre_console_putc(int ch)
>> >> +{
>> > ...
>> >> + ? ? for (uart_addr = uart_reg_addr; *uart_addr; uart_addr++) {
>> >> + ? ? ? ? ? ? NS16550_t regs = (NS16550_t)*uart_addr;
>> >> +
>> >> + ? ? ? ? ? ? NS16550_init(regs, divisor);
>> >> + ? ? ? ? ? ? NS16550_putc(regs, ch);
>> >> + ? ? ? ? ? ? if (ch == '\n')
>> >> + ? ? ? ? ? ? ? ? ? ? NS16550_putc(regs, '\r');
>> >> + ? ? ? ? ? ? NS16550_drain(regs);
>> >
>> > Why is this needed for every output character?
>> >
>> > Actually, why is it needed at all?
>>
>> Of course in this case the init could be done for each UART at the
>> start of the function rather than in the loop, by looping through the
>> UARTs twice.
>
> Both the init() and the drain() should never be used in a character
> output loop.
Yes I agree, but I can't move them out with the API as it is. If we
move back to a puts() type function then I could do this.
>
>> After requests on the list for general purpose pre-console output
>> function (the purpose of which I didn't necessary see) I changed this
>> to a putc() mechanism. This means that we need to set up the UARTs
>> each time it is called. We can't really add a flag to global data
>> since this might be called before that is even set up. There might be
>> a better way, but I'm not sure what it is.
>>
>> For SPL I would like to be able use this same mechanism to call
>> panic() when something goes wrong, and use the same mechanism to get a
>> message to the user. Again, this avoids a bricked unit with no failure
>> indication.
>
> I understand your intention, but I disagree with the design, and with
> the implementation.
It's not great, but let's work out something that is better.
>
> I think outputting data to all "potential" console ports is a really
> bad thing, as you cannot know how your users are using the hardware.
> They may have attached hardware to the UARTs, and the data you send to
> the port causes a mis-function of the device. ?I guess you don't add
> to your documentation a warning like: select one port as console, and
> leave allother ports unused, because we may send random date to all
> ports any time?
Only on a fatal error. Unfortunately this idea of 'fatal error' was
lost in the conversion from board_panic_no_console() to
board_pre_console_putc(). I would be keen to move back to that
original plan, so that the idea of a fatal error is captured. In a
fatal error situation there is no prospect of the board working and
the only hope is that we can alert the user.
>
> If you do not know which UART port to use, then the only consequence
> can be not to use any UART prt at all. ?Use a LED with blink codes or
> whatever your hardwar ehas, but do not mess with random ports.
I agres with the sentiment and this is a very ugly corner case, but in
practice people want to know what happened, not just be presented with
a brick.
>
> I also cannot understand why you think you must init() and drain() the
> UART for each character printed - ok, the latter is probably a
> consequence of re-initializing the port for each character.
> Eventually this will be not needed once you get rid of the re-init.
OK so how about moving to a puts()-type interface? Then I can remove this.
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 at denx.de
> "Consistency requires you to be as ignorant today as you were a ?year
> ago." ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?- Bernard Berenson
^ permalink raw reply [flat|nested] 22+ messages in thread* [U-Boot] [PATCH v2 3/4] tegra: Implement pre-console putc() for fdt warning
2012-03-10 21:25 ` Simon Glass
@ 2012-03-10 22:49 ` Wolfgang Denk
2012-03-11 0:48 ` Simon Glass
0 siblings, 1 reply; 22+ messages in thread
From: Wolfgang Denk @ 2012-03-10 22:49 UTC (permalink / raw)
To: u-boot
Dear Simon,
In message <CAPnjgZ1GzD3KJeaA1LLdYPQsoOLWJPxmX=FHFeUthkdPrUYt2g@mail.gmail.com> you wrote:
>
> > I think outputting data to all "potential" console ports is a really
> > bad thing, as you cannot know how your users are using the hardware.
> > They may have attached hardware to the UARTs, and the data you send to
> > the port causes a mis-function of the device. =A0I guess you don't add
> > to your documentation a warning like: select one port as console, and
> > leave allother ports unused, because we may send random date to all
> > ports any time?
>
> Only on a fatal error. Unfortunately this idea of 'fatal error' was
> lost in the conversion from board_panic_no_console() to
This "fatal error" might become a fatal accident in some cases if you
send random data to some port that controls some kind of machinery or
whatever. Never ever do that!
> board_pre_console_putc(). I would be keen to move back to that
> original plan, so that the idea of a fatal error is captured. In a
> fatal error situation there is no prospect of the board working and
> the only hope is that we can alert the user.
Agreed. But you must not do this by performing random actions on
random (potentially otherwise used [by the end user]) interfaces.
> > If you do not know which UART port to use, then the only consequence
> > can be not to use any UART prt at all. Use a LED with blink codes or
> > whatever your hardwar ehas, but do not mess with random ports.
>
> I agres with the sentiment and this is a very ugly corner case, but in
> practice people want to know what happened, not just be presented with
> a brick.
Yes, I understand this. But please try to find another way to signal
failure. Do not perform random actions on a device - you could as
well always output all data on all UART ports because with this
codeyou cannot use the other ports for any serious purpose anyway.
> > I also cannot understand why you think you must init() and drain() the
> > UART for each character printed - ok, the latter is probably a
> > consequence of re-initializing the port for each character.
> > Eventually this will be not needed once you get rid of the re-init.
>
> OK so how about moving to a puts()-type interface? Then I can remove this.
Sorry, but I don't have enough context - pts() interface where or for
what?
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
Each honest calling, each walk of life, has its own elite, its own
aristocracy based on excellence of performance. - James Bryant Conant
^ permalink raw reply [flat|nested] 22+ messages in thread
* [U-Boot] [PATCH v2 3/4] tegra: Implement pre-console putc() for fdt warning
2012-03-10 22:49 ` Wolfgang Denk
@ 2012-03-11 0:48 ` Simon Glass
0 siblings, 0 replies; 22+ messages in thread
From: Simon Glass @ 2012-03-11 0:48 UTC (permalink / raw)
To: u-boot
Hi Wolfgang,
On Sat, Mar 10, 2012 at 2:49 PM, Wolfgang Denk <wd@denx.de> wrote:
> Dear Simon,
>
> In message <CAPnjgZ1GzD3KJeaA1LLdYPQsoOLWJPxmX=FHFeUthkdPrUYt2g@mail.gmail.com> you wrote:
>>
>> > I think outputting data to all "potential" console ports is a really
>> > bad thing, as you cannot know how your users are using the hardware.
>> > They may have attached hardware to the UARTs, and the data you send to
>> > the port causes a mis-function of the device. =A0I guess you don't add
>> > to your documentation a warning like: select one port as console, and
>> > leave allother ports unused, because we may send random date to all
>> > ports any time?
>>
>> Only on a fatal error. Unfortunately this idea of 'fatal error' was
>> lost in the conversion from board_panic_no_console() to
>
> This "fatal error" might become a fatal accident in some cases if you
> send random data to some port that controls some kind of machinery or
> whatever. ?Never ever do that!
Yes I see that. Scary.
>
>> board_pre_console_putc(). I would be keen to move back to that
>> original plan, so that the idea of a fatal error is captured. In a
>> fatal error situation there is no prospect of the board working and
>> the only hope is that we can alert the user.
>
> Agreed. ?But you must not do this by performing random actions on
> random (potentially otherwise used [by the end user]) interfaces.
OK I will come up with a patch along these lines. See below for thoughts.
>
>> > If you do not know which UART port to use, then the only consequence
>> > can be not to use any UART prt at all. ?Use a LED with blink codes or
>> > whatever your hardwar ehas, but do not mess with random ports.
>>
>> I agres with the sentiment and this is a very ugly corner case, but in
>> practice people want to know what happened, not just be presented with
>> a brick.
>
> Yes, I understand this. ?But please try to find another way to signal
> failure. ?Do not perform random actions on a device - you could as
> well always output all data on all UART ports because with this
> codeyou cannot use the other ports for any serious purpose anyway.
OK so I think the first step is to move this function out of generic
Tegra code and into board-specific code.
Presumably the board owner will know what is safe for that board. In
many cases there is only one possible console UART. If there is an LED
available then the board could flash that, but it isn't hugely useful.
We already know the board is a brick - turning on a light to tell us
that it is a brick doesn't help much. So I do want to provide a way to
get a message out on boards where it is safe to do so.
In the case of the Tegra board there are four possible UARTs. I wonder
whether we could do something like provide a Tegra panic function
which is told which UARTs it can use.
Then the board can decide how to implement a board_pre_console_panic()
function, which could be a weak function, perhaps by just hanging,
perhaps by toggling a few GPIOs to flash LEDs, or perhaps by calling
this Tegra panic function asking for a message on UARTs it knows are
safe to use.
This would alleviate the concern that we might turn off someone's live
support system by mistake.
>
>> > I also cannot understand why you think you must init() and drain() the
>> > UART for each character printed - ok, the latter is probably a
>> > consequence of re-initializing the port for each character.
>> > Eventually this will be not needed once you get rid of the re-init.
>>
>> OK so how about moving to a puts()-type interface? Then I can remove this.
>
> Sorry, but I don't have enough context - pts() interface where or for
> what?
I just meant that this would be much easier if we had a panic function
instead of this putc() stuff. We can (for example) flash lights
indefinitely and never return.
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 at denx.de
> Each honest calling, each walk of life, has its own ?elite, ?its ?own
> aristocracy based on excellence of performance. - James Bryant Conant
^ permalink raw reply [flat|nested] 22+ messages in thread
* [U-Boot] [PATCH v2 4/4] tegra: Enable pre-console putc() for Tegra boards
2012-03-09 20:32 [U-Boot] [PATCH v2 1/4] ns16550: Add function to drain serial output Simon Glass
2012-03-09 20:32 ` [U-Boot] [PATCH v2 2/4] Enable printf() console if pre-console putc() is available Simon Glass
2012-03-09 20:32 ` [U-Boot] [PATCH v2 3/4] tegra: Implement pre-console putc() for fdt warning Simon Glass
@ 2012-03-09 20:32 ` Simon Glass
2012-03-09 20:59 ` [U-Boot] [PATCH v2 1/4] ns16550: Add function to drain serial output Tom Warren
` (3 subsequent siblings)
6 siblings, 0 replies; 22+ messages in thread
From: Simon Glass @ 2012-03-09 20:32 UTC (permalink / raw)
To: u-boot
This is used to display panic messages before the console is active.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
include/configs/tegra2-common.h | 7 +++++++
1 files changed, 7 insertions(+), 0 deletions(-)
diff --git a/include/configs/tegra2-common.h b/include/configs/tegra2-common.h
index 837f859..368f3eb 100644
--- a/include/configs/tegra2-common.h
+++ b/include/configs/tegra2-common.h
@@ -68,11 +68,18 @@
*/
#define V_NS16550_CLK 216000000 /* 216MHz (pllp_out0) */
+/* Default serial clock and multiplier */
+#define CONFIG_DEFAULT_NS16550_CLK V_NS16550_CLK
+#define CONFIG_DEFAULT_NS16550_MULT 16
+
#define CONFIG_SYS_NS16550
#define CONFIG_SYS_NS16550_SERIAL
#define CONFIG_SYS_NS16550_REG_SIZE (-4)
#define CONFIG_SYS_NS16550_CLK V_NS16550_CLK
+/* We use this for a warning message when no device tree is present */
+#define CONFIG_PRE_CONSOLE_PUTC
+
/*
* select serial console configuration
*/
--
1.7.7.3
^ permalink raw reply related [flat|nested] 22+ messages in thread* [U-Boot] [PATCH v2 1/4] ns16550: Add function to drain serial output
2012-03-09 20:32 [U-Boot] [PATCH v2 1/4] ns16550: Add function to drain serial output Simon Glass
` (2 preceding siblings ...)
2012-03-09 20:32 ` [U-Boot] [PATCH v2 4/4] tegra: Enable pre-console putc() for Tegra boards Simon Glass
@ 2012-03-09 20:59 ` Tom Warren
2012-03-10 8:20 ` Wolfgang Denk
2012-03-09 21:00 ` Stephen Warren
` (2 subsequent siblings)
6 siblings, 1 reply; 22+ messages in thread
From: Tom Warren @ 2012-03-09 20:59 UTC (permalink / raw)
To: u-boot
Simon,
> -----Original Message-----
> From: Simon Glass [mailto:sjg at chromium.org]
> Sent: Friday, March 09, 2012 1:33 PM
> To: U-Boot Mailing List
> Cc: Tom Warren; Stephen Warren; Simon Glass
> Subject: [PATCH v2 1/4] ns16550: Add function to drain serial output
This patch series (v2, 1-4) works for me on my Seaboard - I get a message about 'No valid fdt found', etc.
So:
Tested-by: Tom Warren <twarren@nvidia.com>
Acked-by: Tom Warren <twarren@nvidia.com>
If no one objects, I'll apply this series, and Stephen's 'Makefile: fdt: Make the final build result be u-boot.bin' patch (which also works OK, BTW) to my previous pull request and resend to Albert.
Tom
--
nvpublic
>
> Sometimes we want to be sure that the output FIFO has fully drained (e.g.
> because we are about to reset or the port or reset the board). Add a
> function for this.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
> Changes in v2:
> - Add function to drain the ns16550's FIFO
>
> drivers/serial/ns16550.c | 11 +++++++++++
> include/ns16550.h | 3 +++
> 2 files changed, 14 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/serial/ns16550.c b/drivers/serial/ns16550.c index
> 0c23955..a082112 100644
> --- a/drivers/serial/ns16550.c
> +++ b/drivers/serial/ns16550.c
> @@ -6,6 +6,7 @@
>
> #include <config.h>
> #include <ns16550.h>
> +#include <common.h>
> #include <watchdog.h>
> #include <linux/types.h>
> #include <asm/io.h>
> @@ -115,4 +116,14 @@ int NS16550_tstc(NS16550_t com_port)
> return (serial_in(&com_port->lsr) & UART_LSR_DR) != 0; }
>
> +/* Wait for the UART's output buffer and holding register to drain */
> +void NS16550_drain(NS16550_t regs) {
> + u32 mask = UART_LSR_TEMT | UART_LSR_THRE;
> +
> + /* Wait for the UART to finish sending */
> + while ((serial_in(®s->lsr) & mask) != mask)
> + udelay(100);
> +}
> +
> #endif /* CONFIG_NS16550_MIN_FUNCTIONS */ diff --git a/include/ns16550.h
> b/include/ns16550.h index e9d2eda..16b3828 100644
> --- a/include/ns16550.h
> +++ b/include/ns16550.h
> @@ -166,4 +166,7 @@ void NS16550_init(NS16550_t com_port, int baud_divisor);
> void NS16550_putc(NS16550_t com_port, char c); char NS16550_getc(NS16550_t
> com_port); int NS16550_tstc(NS16550_t com_port);
> +
> +/* Wait for the UART's output buffer and holding register to drain */
> +void NS16550_drain(NS16550_t regs);
> void NS16550_reinit(NS16550_t com_port, int baud_divisor);
> --
> 1.7.7.3
^ permalink raw reply [flat|nested] 22+ messages in thread* [U-Boot] [PATCH v2 1/4] ns16550: Add function to drain serial output
2012-03-09 20:59 ` [U-Boot] [PATCH v2 1/4] ns16550: Add function to drain serial output Tom Warren
@ 2012-03-10 8:20 ` Wolfgang Denk
0 siblings, 0 replies; 22+ messages in thread
From: Wolfgang Denk @ 2012-03-10 8:20 UTC (permalink / raw)
To: u-boot
Dear Tom Warren,
In message <5FBF8E85CA34454794F0F7ECBA79798F37971BC0CD@HQMAIL04.nvidia.com> you wrote:
>
> If no one objects, I'll apply this series, and Stephen's 'Makefile: fdt: Make the final build result be u-boot.bin' patch (which also works OK, BTW) to my previous pull request and resend to Albert.
I do object. This affects common code.
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
"On two occasions I have been asked [by members of Parliament!],
'Pray, Mr. Babbage, if you put into the machine wrong figures, will
the right answers come out?' I am not able rightly to apprehend the
kind of confusion of ideas that could provoke such a question."
- Charles Babbage
^ permalink raw reply [flat|nested] 22+ messages in thread
* [U-Boot] [PATCH v2 1/4] ns16550: Add function to drain serial output
2012-03-09 20:32 [U-Boot] [PATCH v2 1/4] ns16550: Add function to drain serial output Simon Glass
` (3 preceding siblings ...)
2012-03-09 20:59 ` [U-Boot] [PATCH v2 1/4] ns16550: Add function to drain serial output Tom Warren
@ 2012-03-09 21:00 ` Stephen Warren
2012-03-09 21:08 ` Simon Glass
2012-03-10 8:19 ` Wolfgang Denk
2012-03-11 2:06 ` Mike Frysinger
6 siblings, 1 reply; 22+ messages in thread
From: Stephen Warren @ 2012-03-09 21:00 UTC (permalink / raw)
To: u-boot
On 03/09/2012 01:32 PM, Simon Glass wrote:
> Sometimes we want to be sure that the output FIFO has fully drained (e.g.
> because we are about to reset or the port or reset the board). Add a
> function for this.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
The series,
Tested-by: Stephen Warren <swarren@wwwdotorg.org>
Tom,
I'm OK with you adding these patches to u-boot-tegra/master and sending
a pull request for that. Just please put these patches before any that
actually enable CONFIG_OF_CONTROL for any boards, so there's never a
window of broken commits.
I think it'd be nice at this stage to also drop the commit that turns
Ventana into a DT board, since re-using the Seaboard .dts file for
Ventana isn't correct. However, IIRC that commit was added to WAR some
build issue with Simon's patch series? If that's still a problem, then
I'm OK with converting Ventana if we have to, but we'll need to be very
careful that tegra-seaboard.dts doesn't add stuff that'll fail on, or
damage, Ventana.
^ permalink raw reply [flat|nested] 22+ messages in thread* [U-Boot] [PATCH v2 1/4] ns16550: Add function to drain serial output
2012-03-09 21:00 ` Stephen Warren
@ 2012-03-09 21:08 ` Simon Glass
2012-03-09 21:23 ` Tom Warren
0 siblings, 1 reply; 22+ messages in thread
From: Simon Glass @ 2012-03-09 21:08 UTC (permalink / raw)
To: u-boot
Hi,
On Fri, Mar 9, 2012 at 1:00 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
> On 03/09/2012 01:32 PM, Simon Glass wrote:
>> Sometimes we want to be sure that the output FIFO has fully drained (e.g.
>> because we are about to reset or the port or reset the board). Add a
>> function for this.
>>
>> Signed-off-by: Simon Glass <sjg@chromium.org>
>
> The series,
>
> Tested-by: Stephen Warren <swarren@wwwdotorg.org>
>
> Tom,
>
> I'm OK with you adding these patches to u-boot-tegra/master and sending
> a pull request for that. Just please put these patches before any that
> actually enable CONFIG_OF_CONTROL for any boards, so there's never a
> window of broken commits.
>
> I think it'd be nice at this stage to also drop the commit that turns
> Ventana into a DT board, since re-using the Seaboard .dts file for
> Ventana isn't correct. However, IIRC that commit was added to WAR some
> build issue with Simon's patch series? If that's still a problem, then
> I'm OK with converting Ventana if we have to, but we'll need to be very
> careful that tegra-seaboard.dts doesn't add stuff that'll fail on, or
> damage, Ventana.
A few points:
1. Yes you can drop the Ventana commit and I agree it is a good idea.
We should have a real Ventana .dts file I think. Ventana should built
ok without a device tree now. The problem was in my clock.c patch.
2. I have held off responding to Stephen's patch on the ML to see what
other say. My view is that it is controversial since it changes the
so-far accepted meaning of u-boot.bin and the behaviour of the U-Boot
Makefile. Plus it is not really necessary as a means of informing the
user since we put the pre-console putc() for exactly this problem. So
I would rather leave Stephen's patch out at until people have time to
decide that I am wrong about it. We already have CONFIG_OF_EMBED to
build the fdt into u-boot.bin. Grant Likely had big reservations about
this feature - let's not bring it in by stealth.
Regards,
Simon
^ permalink raw reply [flat|nested] 22+ messages in thread
* [U-Boot] [PATCH v2 1/4] ns16550: Add function to drain serial output
2012-03-09 21:08 ` Simon Glass
@ 2012-03-09 21:23 ` Tom Warren
2012-03-09 21:29 ` Simon Glass
0 siblings, 1 reply; 22+ messages in thread
From: Tom Warren @ 2012-03-09 21:23 UTC (permalink / raw)
To: u-boot
Simon/Stephen,
> -----Original Message-----
> From: sjg at google.com [mailto:sjg at google.com] On Behalf Of Simon Glass
> Sent: Friday, March 09, 2012 2:09 PM
> To: Stephen Warren
> Cc: U-Boot Mailing List; Tom Warren
> Subject: Re: [PATCH v2 1/4] ns16550: Add function to drain serial output
>
> Hi,
>
> On Fri, Mar 9, 2012 at 1:00 PM, Stephen Warren <swarren@wwwdotorg.org>
> wrote:
> > On 03/09/2012 01:32 PM, Simon Glass wrote:
> >> Sometimes we want to be sure that the output FIFO has fully drained (e.g.
> >> because we are about to reset or the port or reset the board). Add a
> >> function for this.
> >>
> >> Signed-off-by: Simon Glass <sjg@chromium.org>
> >
> > The series,
> >
> > Tested-by: Stephen Warren <swarren@wwwdotorg.org>
> >
> > Tom,
> >
> > I'm OK with you adding these patches to u-boot-tegra/master and
> > sending a pull request for that. Just please put these patches before
> > any that actually enable CONFIG_OF_CONTROL for any boards, so there's
> > never a window of broken commits.
> >
> > I think it'd be nice at this stage to also drop the commit that turns
> > Ventana into a DT board, since re-using the Seaboard .dts file for
> > Ventana isn't correct. However, IIRC that commit was added to WAR some
> > build issue with Simon's patch series? If that's still a problem, then
> > I'm OK with converting Ventana if we have to, but we'll need to be
> > very careful that tegra-seaboard.dts doesn't add stuff that'll fail
> > on, or damage, Ventana.
>
> A few points:
>
> 1. Yes you can drop the Ventana commit and I agree it is a good idea.
> We should have a real Ventana .dts file I think. Ventana should built ok
> without a device tree now. The problem was in my clock.c patch.
>
> 2. I have held off responding to Stephen's patch on the ML to see what other
> say. My view is that it is controversial since it changes the so-far
> accepted meaning of u-boot.bin and the behaviour of the U-Boot Makefile.
> Plus it is not really necessary as a means of informing the user since we
> put the pre-console putc() for exactly this problem. So I would rather leave
> Stephen's patch out at until people have time to decide that I am wrong
> about it. We already have CONFIG_OF_EMBED to build the fdt into u-boot.bin.
> Grant Likely had big reservations about this feature - let's not bring it in
> by stealth.
>
I see now why it's so hard to find custodians for the U-Boot repos. :/
I've done as Stephen requested and re-ordered the commits so his Makefile fix comes before the config file changes to add CONFIG_OF_CONTROL to Seaboard and Ventana.
I've left the Ventana change that uses the Seaboard .dts file in, as I think it can easily be edited to use a Ventana-specific dts file just as soon as one of you finds one and submits a patch for it. For now, the Seaboard .dts just works.
I've pushed my latest changes to u-boot-tegra/master, and I'm going to let it percolate over the weekend. I'd have liked to get a pull request in to Albert today, before the ARM HEAD moves again, but I'm not going sweat to it since there doesn't seem to be a consensus on how to proceed here. IMO, I see no problem with putting these patches in as they stand, since I've tested them to work OK on Seaboard and Ventana, other (non-DT) Tegra2 boards build fine, and anything here that needs to change can always be edited with a future patch. But I'm going to head out early (it's my birthday today), and pick this up again next week.
Tom
--
nvpublic
> Regards,
> Simon
^ permalink raw reply [flat|nested] 22+ messages in thread
* [U-Boot] [PATCH v2 1/4] ns16550: Add function to drain serial output
2012-03-09 21:23 ` Tom Warren
@ 2012-03-09 21:29 ` Simon Glass
2012-03-09 21:40 ` Stephen Warren
0 siblings, 1 reply; 22+ messages in thread
From: Simon Glass @ 2012-03-09 21:29 UTC (permalink / raw)
To: u-boot
Hi Stephen,
On Fri, Mar 9, 2012 at 1:23 PM, Tom Warren <TWarren@nvidia.com> wrote:
> Simon/Stephen,
>
>> -----Original Message-----
>> From: sjg at google.com [mailto:sjg at google.com] On Behalf Of Simon Glass
>> Sent: Friday, March 09, 2012 2:09 PM
>> To: Stephen Warren
>> Cc: U-Boot Mailing List; Tom Warren
>> Subject: Re: [PATCH v2 1/4] ns16550: Add function to drain serial output
>>
>> Hi,
>>
>> On Fri, Mar 9, 2012 at 1:00 PM, Stephen Warren <swarren@wwwdotorg.org>
>> wrote:
>> > On 03/09/2012 01:32 PM, Simon Glass wrote:
>> >> Sometimes we want to be sure that the output FIFO has fully drained (e.g.
>> >> because we are about to reset or the port or reset the board). Add a
>> >> function for this.
>> >>
>> >> Signed-off-by: Simon Glass <sjg@chromium.org>
>> >
>> > The series,
>> >
>> > Tested-by: Stephen Warren <swarren@wwwdotorg.org>
>> >
>> > Tom,
>> >
>> > I'm OK with you adding these patches to u-boot-tegra/master and
>> > sending a pull request for that. Just please put these patches before
>> > any that actually enable CONFIG_OF_CONTROL for any boards, so there's
>> > never a window of broken commits.
>> >
>> > I think it'd be nice at this stage to also drop the commit that turns
>> > Ventana into a DT board, since re-using the Seaboard .dts file for
>> > Ventana isn't correct. However, IIRC that commit was added to WAR some
>> > build issue with Simon's patch series? If that's still a problem, then
>> > I'm OK with converting Ventana if we have to, but we'll need to be
>> > very careful that tegra-seaboard.dts doesn't add stuff that'll fail
>> > on, or damage, Ventana.
>>
>> A few points:
>>
>> 1. Yes you can drop the Ventana commit and I agree it is a good idea.
>> We should have a real Ventana .dts file I think. Ventana should built ok
>> without a device tree now. The problem was in my clock.c patch.
>>
>> 2. I have held off responding to Stephen's patch on the ML to see what other
>> say. My view is that it is controversial since it changes the so-far
>> accepted meaning of u-boot.bin and the behaviour of the U-Boot Makefile.
>> Plus it is not really necessary as a means of informing the user since we
>> put the pre-console putc() for exactly this problem. So I would rather leave
>> Stephen's patch out at until people have time to decide that I am wrong
>> about it. We already have CONFIG_OF_EMBED to build the fdt into u-boot.bin.
>> Grant Likely had big reservations about this feature - let's not bring it in
>> by stealth.
>>
>
> I see now why it's so hard to find custodians for the U-Boot repos. :/
>
> I've done as Stephen requested and re-ordered the commits so his Makefile fix comes before the config file changes to add CONFIG_OF_CONTROL to Seaboard and Ventana.
>
> I've left the Ventana change that uses the Seaboard .dts file in, as I think it can easily be edited to use a Ventana-specific dts file just as soon as one of you finds one and submits a patch for it. For now, the Seaboard .dts just works.
>
> I've pushed my latest changes to u-boot-tegra/master, and I'm going to let it percolate over the weekend. I'd have liked to get a pull request in to Albert today, before the ARM HEAD moves again, but I'm not going sweat to it since there doesn't seem to be a consensus on how to proceed here. IMO, I see no problem with putting these patches in as they stand, since I've tested them to work OK on Seaboard and Ventana, other (non-DT) Tegra2 boards build fine, and anything here that needs to change can always be edited with a future patch. But I'm going to head out early (it's my birthday today), and pick this up again next week.
Happy Birthday Tom!
Stephen, what you do you about leaving out your Makefile patch for
now? It is (I think) the only controversial part of this, and is not
needed to make all this work...
Regards,
Simon
>
> Tom
>
> --
> nvpublic
>> Regards,
>> Simon
^ permalink raw reply [flat|nested] 22+ messages in thread
* [U-Boot] [PATCH v2 1/4] ns16550: Add function to drain serial output
2012-03-09 21:29 ` Simon Glass
@ 2012-03-09 21:40 ` Stephen Warren
0 siblings, 0 replies; 22+ messages in thread
From: Stephen Warren @ 2012-03-09 21:40 UTC (permalink / raw)
To: u-boot
On 03/09/2012 02:29 PM, Simon Glass wrote:
> On Fri, Mar 9, 2012 at 1:23 PM, Tom Warren <TWarren@nvidia.com> wrote:
>> sjg at google.com wrote at Friday, March 09, 2012 2:09 PM:
...
>>> 2. I have held off responding to Stephen's patch on the ML to see what other
>>> say. My view is that it is controversial since it changes the so-far
>>> accepted meaning of u-boot.bin and the behaviour of the U-Boot Makefile.
>>> Plus it is not really necessary as a means of informing the user since we
>>> put the pre-console putc() for exactly this problem. So I would rather leave
>>> Stephen's patch out at until people have time to decide that I am wrong
>>> about it. We already have CONFIG_OF_EMBED to build the fdt into u-boot.bin.
>>> Grant Likely had big reservations about this feature - let's not bring it in
>>> by stealth.
I don't quite see how this is bringing the feature in by stealth; the
exact same set of files (one with and without appended DTB already in
place) is still available, just under filenames that are likely to cause
less surprise to the user.
...
> Stephen, what you do you about leaving out your Makefile patch for
> now? It is (I think) the only controversial part of this, and is not
> needed to make all this work...
Yes, I'm happy with that. The issue I had was the surprising result of
U-Boot hanging without giving any clues why, and that's addressed by the
messages that are now printed with your latest patches.
While I still like the patch to rename the files in the top-level
Makefile, I do agree that including it in a pull request without more
widespread discussion and agreement is not a good idea.
BTW, happy birthday Tom.
^ permalink raw reply [flat|nested] 22+ messages in thread
* [U-Boot] [PATCH v2 1/4] ns16550: Add function to drain serial output
2012-03-09 20:32 [U-Boot] [PATCH v2 1/4] ns16550: Add function to drain serial output Simon Glass
` (4 preceding siblings ...)
2012-03-09 21:00 ` Stephen Warren
@ 2012-03-10 8:19 ` Wolfgang Denk
2012-03-10 19:27 ` Simon Glass
2012-03-11 2:06 ` Mike Frysinger
6 siblings, 1 reply; 22+ messages in thread
From: Wolfgang Denk @ 2012-03-10 8:19 UTC (permalink / raw)
To: u-boot
Dear Simon Glass,
In message <1331325178-14634-1-git-send-email-sjg@chromium.org> you wrote:
> Sometimes we want to be sure that the output FIFO has fully drained (e.g.
> because we are about to reset or the port or reset the board). Add a
Seems there is one unintentional "or".
> function for this.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
> Changes in v2:
> - Add function to drain the ns16550's FIFO
>
> drivers/serial/ns16550.c | 11 +++++++++++
> include/ns16550.h | 3 +++
> 2 files changed, 14 insertions(+), 0 deletions(-)
I dislike this. This is mostly dead code, enabled for eveybody where
it just increases the memory footprint for no use.
Also I do not understand why it would be needed at all. We did not
have such a requirement for any system before, so I feel you must be
doing something wrong, or at least very exotic.
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
Extreme feminine beauty is always disturbing.
-- Spock, "The Cloud Minders", stardate 5818.4
^ permalink raw reply [flat|nested] 22+ messages in thread* [U-Boot] [PATCH v2 1/4] ns16550: Add function to drain serial output
2012-03-10 8:19 ` Wolfgang Denk
@ 2012-03-10 19:27 ` Simon Glass
2012-03-10 20:24 ` Wolfgang Denk
0 siblings, 1 reply; 22+ messages in thread
From: Simon Glass @ 2012-03-10 19:27 UTC (permalink / raw)
To: u-boot
Hi Wolfgang,
On Sat, Mar 10, 2012 at 12:19 AM, Wolfgang Denk <wd@denx.de> wrote:
> Dear Simon Glass,
>
> In message <1331325178-14634-1-git-send-email-sjg@chromium.org> you wrote:
>> Sometimes we want to be sure that the output FIFO has fully drained (e.g.
>> because we are about to reset or the port or reset the board). Add a
>
> Seems there is one unintentional "or".
Yes, will fix.
>
>> function for this.
>>
>> Signed-off-by: Simon Glass <sjg@chromium.org>
>> ---
>> Changes in v2:
>> - Add function to drain the ns16550's FIFO
>>
>> ?drivers/serial/ns16550.c | ? 11 +++++++++++
>> ?include/ns16550.h ? ? ? ?| ? ?3 +++
>> ?2 files changed, 14 insertions(+), 0 deletions(-)
>
> I dislike this. ?This is mostly dead code, enabled for eveybody where
> it just increases the memory footprint for no use.
>
> Also I do not understand why it would be needed at all. ?We did not
> have such a requirement for any system before, so I feel you must be
> doing something wrong, or at least very exotic.
I put a fully explanation in the previous patch. Let me just add here
that I see a 100ms delay in the reset code to allow serial output to
flush, so there is at least in practice a need for this sort of thing.
For most archs the extra code will be removed by the linker.
You may also recall a discussion about a platform where the SPI flash
and console uart were muxed, and we had to flush the console uart
before switching to SPI. I feel that draining the FIFO could/should be
a useful function.
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 at denx.de
> Extreme feminine beauty is always disturbing.
> ? ? ? ?-- Spock, "The Cloud Minders", stardate 5818.4
^ permalink raw reply [flat|nested] 22+ messages in thread
* [U-Boot] [PATCH v2 1/4] ns16550: Add function to drain serial output
2012-03-10 19:27 ` Simon Glass
@ 2012-03-10 20:24 ` Wolfgang Denk
2012-03-11 0:52 ` Simon Glass
0 siblings, 1 reply; 22+ messages in thread
From: Wolfgang Denk @ 2012-03-10 20:24 UTC (permalink / raw)
To: u-boot
Dear Simon Glass,
In message <CAPnjgZ3wprPDNUbe0fGbsequXJgQ_bwzkDEu6FeuR-8GG1ndQQ@mail.gmail.com> you wrote:
>
> > Also I do not understand why it would be needed at all. =A0We did not
> > have such a requirement for any system before, so I feel you must be
> > doing something wrong, or at least very exotic.
>
> I put a fully explanation in the previous patch. Let me just add here
Explanations in some other patch are not available when someone reads
the commit message ...
> that I see a 100ms delay in the reset code to allow serial output to
> flush, so there is at least in practice a need for this sort of thing.
Note that such a delay is hardware independent. You implement a
special solution that works only for a minority of boards.
> For most archs the extra code will be removed by the linker.
Ok, granted.
> You may also recall a discussion about a platform where the SPI flash
> and console uart were muxed, and we had to flush the console uart
> before switching to SPI. I feel that draining the FIFO could/should be
> a useful function.
You quote an example of highly specific hardware, which I would not
hesitate to call a broken design. I do not really fancy adding code
to common files for such exceptional situations.
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
Hiring experienced unix people is like a built-in filter against
idiots. Hiring experienced NT people provides no such guarantee.
-- Miguel Cruz in WgL96.349$CC.122704 at typhoon2.ba-dsg.net
^ permalink raw reply [flat|nested] 22+ messages in thread* [U-Boot] [PATCH v2 1/4] ns16550: Add function to drain serial output
2012-03-10 20:24 ` Wolfgang Denk
@ 2012-03-11 0:52 ` Simon Glass
0 siblings, 0 replies; 22+ messages in thread
From: Simon Glass @ 2012-03-11 0:52 UTC (permalink / raw)
To: u-boot
Hi Wolfgang,
On Sat, Mar 10, 2012 at 12:24 PM, Wolfgang Denk <wd@denx.de> wrote:
> Dear Simon Glass,
>
> In message <CAPnjgZ3wprPDNUbe0fGbsequXJgQ_bwzkDEu6FeuR-8GG1ndQQ@mail.gmail.com> you wrote:
>>
>> > Also I do not understand why it would be needed at all. =A0We did not
>> > have such a requirement for any system before, so I feel you must be
>> > doing something wrong, or at least very exotic.
>>
>> I put a fully explanation in the previous patch. Let me just add here
>
> Explanations in some other patch are not available when someone reads
> the commit message ...
I did put a motivation in this patch at least:
Sometimes we want to be sure that the output FIFO has fully drained (e.g.
because we are about to reset or the port or reset the board). Add a
function for this.
>
>> that I see a 100ms delay in the reset code to allow serial output to
>> flush, so there is at least in practice a need for this sort of thing.
>
> Note that such a delay is hardware independent. ?You implement a
> special solution that works only for a minority of boards.
True, and we could just have the board panic function implement a
delay, particularly if we move to the panic idea and away from putc().
>
>> For most archs the extra code will be removed by the linker.
>
> Ok, granted.
>
>> You may also recall a discussion about a platform where the SPI flash
>> and console uart were muxed, and we had to flush the console uart
>> before switching to SPI. I feel that draining the FIFO could/should be
>> a useful function.
>
> You quote an example of highly specific hardware, which I would not
> hesitate to call a broken design. ?I do not really fancy adding code
> to common files for such exceptional situations.
It is certainly broken, yes.
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 at denx.de
> Hiring experienced unix people is ?like ?a ?built-in ?filter ?against
> idiots. Hiring experienced NT people provides no such guarantee.
> ? ? ? ? ? ?-- Miguel Cruz in WgL96.349$CC.122704 at typhoon2.ba-dsg.net
^ permalink raw reply [flat|nested] 22+ messages in thread
* [U-Boot] [PATCH v2 1/4] ns16550: Add function to drain serial output
2012-03-09 20:32 [U-Boot] [PATCH v2 1/4] ns16550: Add function to drain serial output Simon Glass
` (5 preceding siblings ...)
2012-03-10 8:19 ` Wolfgang Denk
@ 2012-03-11 2:06 ` Mike Frysinger
6 siblings, 0 replies; 22+ messages in thread
From: Mike Frysinger @ 2012-03-11 2:06 UTC (permalink / raw)
To: u-boot
On Friday 09 March 2012 15:32:55 Simon Glass wrote:
> Sometimes we want to be sure that the output FIFO has fully drained (e.g.
> because we are about to reset or the port or reset the board).
imo, we shouldn't be draining fifos before a reset in hardware that doesn't
matter -- it's just a waste of time. if it were a CONFIG knob for people, i
wouldn't complain too much, but i still wouldn't see the point ... you'd lose
like maybe 2 or 3 chars at the end of a msg ? not that big of a deal.
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20120310/7323d669/attachment.pgp>
^ permalink raw reply [flat|nested] 22+ messages in thread