Patchwork [U-Boot,5/5] tegra: Implement board_pre_console_panic() for Seaboard

login
register
mail settings
Submitter Simon Glass
Date March 19, 2012, 8:27 p.m.
Message ID <1332188824-5447-5-git-send-email-sjg@chromium.org>
Download mbox | patch
Permalink /patch/147618/
State New, archived
Headers show

Comments

Simon Glass - March 19, 2012, 8:27 p.m.
We enable this feature on all UARTs for Seaboard. This ensures that a
message is printed if CONFIG_OF_CONTROL is in use and a value device tree
is not available.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
 board/nvidia/seaboard/seaboard.c |   18 ++++++++++++++++++
 1 files changed, 18 insertions(+), 0 deletions(-)
Stephen Warren - March 19, 2012, 9:18 p.m.
On 03/19/2012 02:27 PM, Simon Glass wrote:
> We enable this feature on all UARTs for Seaboard. This ensures that a
> message is printed if CONFIG_OF_CONTROL is in use and a value device tree
> is not available.

Why not just enabled this on UARTD, since that's what Seaboard uses?

I guess some derivatives do use UARTB too, which makes things quite
painful. Perhaps at least limit this to UARTB + UARTD, and not all the
others?
Simon Glass - March 19, 2012, 10:59 p.m.
Hi Stephen,

On Mon, Mar 19, 2012 at 2:18 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
> On 03/19/2012 02:27 PM, Simon Glass wrote:
>> We enable this feature on all UARTs for Seaboard. This ensures that a
>> message is printed if CONFIG_OF_CONTROL is in use and a value device tree
>> is not available.
>
> Why not just enabled this on UARTD, since that's what Seaboard uses?
>
> I guess some derivatives do use UARTB too, which makes things quite
> painful. Perhaps at least limit this to UARTB + UARTD, and not all the
> others?

At the moment we can use Seaboard as a generic Tegra2 board, so we
want the widest possible select of UARTs. I think there is one board
that uses A?

Really I would prefer that we explicitly create a generic Tegra2
board, once the fdt stuff is bedded in.

Regards,
Simon
Stephen Warren - March 20, 2012, 1:22 a.m.
On 03/19/2012 04:59 PM, Simon Glass wrote:
> Hi Stephen,
> 
> On Mon, Mar 19, 2012 at 2:18 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
>> On 03/19/2012 02:27 PM, Simon Glass wrote:
>>> We enable this feature on all UARTs for Seaboard. This ensures that a
>>> message is printed if CONFIG_OF_CONTROL is in use and a value device tree
>>> is not available.
>>
>> Why not just enabled this on UARTD, since that's what Seaboard uses?
>>
>> I guess some derivatives do use UARTB too, which makes things quite
>> painful. Perhaps at least limit this to UARTB + UARTD, and not all the
>> others?
> 
> At the moment we can use Seaboard as a generic Tegra2 board, so we
> want the widest possible select of UARTs. I think there is one board
> that uses A?
> 
> Really I would prefer that we explicitly create a generic Tegra2
> board, once the fdt stuff is bedded in.

Well, one of Wolfgang's main objections was blasting the panic message
through all possible UARTs, which might send junk to something other
than a debug UART (e.g. machinery and life support systems were
mentioned). This change doesn't seem to solve that. For low-level debug
like this, shouldn't we just route it to one single UART that the config
file selects?

We can certainly think about refactoring things into a unified board
file, but that seems like something unrelated to do later...
Simon Glass - March 20, 2012, 1:31 a.m.
Hi Stephen,

On Mon, Mar 19, 2012 at 6:22 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
> On 03/19/2012 04:59 PM, Simon Glass wrote:
>> Hi Stephen,
>>
>> On Mon, Mar 19, 2012 at 2:18 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
>>> On 03/19/2012 02:27 PM, Simon Glass wrote:
>>>> We enable this feature on all UARTs for Seaboard. This ensures that a
>>>> message is printed if CONFIG_OF_CONTROL is in use and a value device tree
>>>> is not available.
>>>
>>> Why not just enabled this on UARTD, since that's what Seaboard uses?
>>>
>>> I guess some derivatives do use UARTB too, which makes things quite
>>> painful. Perhaps at least limit this to UARTB + UARTD, and not all the
>>> others?
>>
>> At the moment we can use Seaboard as a generic Tegra2 board, so we
>> want the widest possible select of UARTs. I think there is one board
>> that uses A?
>>
>> Really I would prefer that we explicitly create a generic Tegra2
>> board, once the fdt stuff is bedded in.
>
> Well, one of Wolfgang's main objections was blasting the panic message
> through all possible UARTs, which might send junk to something other
> than a debug UART (e.g. machinery and life support systems were
> mentioned). This change doesn't seem to solve that. For low-level debug
> like this, shouldn't we just route it to one single UART that the config
> file selects?

The objection was that we did it blindly without knowing what ports
were safe to use. Now it is under board control. In the case of a
board where we want the pre-console panic function but only want it on
UARTB we can do that by creating a board file and a config.

The CONFIG cannot select which UART to use, because we only have one
config for all the Seaboard variants, and some use different UARTs.
Only the device tree can tell you which is the console UART. There is
a bit of a conflict here, but keep in mind we are trying to have a
single U-Boot binary - anything that relies on a CONFIG will break
that.

>
> We can certainly think about refactoring things into a unified board
> file, but that seems like something unrelated to do later...

Yes it is. But we use Seaboard as our base board for all the Tegra2
board variants. Some use UARTA, C and one uses D. UART D is a pain
because it is shared with SPI.

So my preference is to leave it as it is, but if you just want it to
be D, then we can go with that for now. At least now it is only a
single line change. Let me know.

Regards,
Simon
Stephen Warren - March 20, 2012, 1:46 a.m.
On 03/19/2012 07:31 PM, Simon Glass wrote:
> Hi Stephen,
> 
> On Mon, Mar 19, 2012 at 6:22 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
>> On 03/19/2012 04:59 PM, Simon Glass wrote:
>>> Hi Stephen,
>>>
>>> On Mon, Mar 19, 2012 at 2:18 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
>>>> On 03/19/2012 02:27 PM, Simon Glass wrote:
>>>>> We enable this feature on all UARTs for Seaboard. This ensures that a
>>>>> message is printed if CONFIG_OF_CONTROL is in use and a value device tree
>>>>> is not available.
>>>>
>>>> Why not just enabled this on UARTD, since that's what Seaboard uses?
>>>>
>>>> I guess some derivatives do use UARTB too, which makes things quite
>>>> painful. Perhaps at least limit this to UARTB + UARTD, and not all the
>>>> others?
>>>
>>> At the moment we can use Seaboard as a generic Tegra2 board, so we
>>> want the widest possible select of UARTs. I think there is one board
>>> that uses A?
>>>
>>> Really I would prefer that we explicitly create a generic Tegra2
>>> board, once the fdt stuff is bedded in.
>>
>> Well, one of Wolfgang's main objections was blasting the panic message
>> through all possible UARTs, which might send junk to something other
>> than a debug UART (e.g. machinery and life support systems were
>> mentioned). This change doesn't seem to solve that. For low-level debug
>> like this, shouldn't we just route it to one single UART that the config
>> file selects?
> 
> The objection was that we did it blindly without knowing what ports
> were safe to use. Now it is under board control. In the case of a
> board where we want the pre-console panic function but only want it on
> UARTB we can do that by creating a board file and a config.
> 
> The CONFIG cannot select which UART to use, because we only have one
> config for all the Seaboard variants, and some use different UARTs.
> Only the device tree can tell you which is the console UART. There is
> a bit of a conflict here, but keep in mind we are trying to have a
> single U-Boot binary - anything that relies on a CONFIG will break
> that.

I don't believe there's any specific requirement or decision to have a
single U-Boot binary. Who has decided that and where is the discussion?

Having a single set of sources seems like quite a large step for a
bootloader, and perhaps can be achieved with DT. Building a binary for
each specific debug UART you need (and potentially for many other
things) seems entirely reasonable.

>> We can certainly think about refactoring things into a unified board
>> file, but that seems like something unrelated to do later...
> 
> Yes it is. But we use Seaboard as our base board for all the Tegra2
> board variants. Some use UARTA, C and one uses D. UART D is a pain
> because it is shared with SPI.
> 
> So my preference is to leave it as it is, but if you just want it to
> be D, then we can go with that for now. At least now it is only a
> single line change. Let me know.

That seems safest for now, especially considering that only baseline
Seaboard is actually supported in mainline U-Boot.
Simon Glass - March 20, 2012, 1:58 a.m.
Hi Stephen,

On Mon, Mar 19, 2012 at 6:46 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
> On 03/19/2012 07:31 PM, Simon Glass wrote:
>> Hi Stephen,
>>
>> On Mon, Mar 19, 2012 at 6:22 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
>>> On 03/19/2012 04:59 PM, Simon Glass wrote:
>>>> Hi Stephen,
>>>>
>>>> On Mon, Mar 19, 2012 at 2:18 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
>>>>> On 03/19/2012 02:27 PM, Simon Glass wrote:
>>>>>> We enable this feature on all UARTs for Seaboard. This ensures that a
>>>>>> message is printed if CONFIG_OF_CONTROL is in use and a value device tree
>>>>>> is not available.
>>>>>
>>>>> Why not just enabled this on UARTD, since that's what Seaboard uses?
>>>>>
>>>>> I guess some derivatives do use UARTB too, which makes things quite
>>>>> painful. Perhaps at least limit this to UARTB + UARTD, and not all the
>>>>> others?
>>>>
>>>> At the moment we can use Seaboard as a generic Tegra2 board, so we
>>>> want the widest possible select of UARTs. I think there is one board
>>>> that uses A?
>>>>
>>>> Really I would prefer that we explicitly create a generic Tegra2
>>>> board, once the fdt stuff is bedded in.
>>>
>>> Well, one of Wolfgang's main objections was blasting the panic message
>>> through all possible UARTs, which might send junk to something other
>>> than a debug UART (e.g. machinery and life support systems were
>>> mentioned). This change doesn't seem to solve that. For low-level debug
>>> like this, shouldn't we just route it to one single UART that the config
>>> file selects?
>>
>> The objection was that we did it blindly without knowing what ports
>> were safe to use. Now it is under board control. In the case of a
>> board where we want the pre-console panic function but only want it on
>> UARTB we can do that by creating a board file and a config.
>>
>> The CONFIG cannot select which UART to use, because we only have one
>> config for all the Seaboard variants, and some use different UARTs.
>> Only the device tree can tell you which is the console UART. There is
>> a bit of a conflict here, but keep in mind we are trying to have a
>> single U-Boot binary - anything that relies on a CONFIG will break
>> that.
>
> I don't believe there's any specific requirement or decision to have a
> single U-Boot binary. Who has decided that and where is the discussion?

That is the whole point of the device tree effort :-)

>
> Having a single set of sources seems like quite a large step for a
> bootloader, and perhaps can be achieved with DT. Building a binary for
> each specific debug UART you need (and potentially for many other
> things) seems entirely reasonable.

That's up to you of course. Our intent is to have a single binary for
each SOC, with the device tree providing all required config.

>
>>> We can certainly think about refactoring things into a unified board
>>> file, but that seems like something unrelated to do later...
>>
>> Yes it is. But we use Seaboard as our base board for all the Tegra2
>> board variants. Some use UARTA, C and one uses D. UART D is a pain
>> because it is shared with SPI.
>>
>> So my preference is to leave it as it is, but if you just want it to
>> be D, then we can go with that for now. At least now it is only a
>> single line change. Let me know.
>
> That seems safest for now, especially considering that only baseline
> Seaboard is actually supported in mainline U-Boot.

You would be surprised. With a device tree I can boot mainline
'seaboard' U-Boot on a few things...

I will update the patch and will see if Wolfgang is comfortable with
this new panic mechanism also.

Regards,
Simon
Wolfgang Denk - March 21, 2012, 9:08 a.m.
Dear Simon Glass,

In message <1332188824-5447-5-git-send-email-sjg@chromium.org> you wrote:
> We enable this feature on all UARTs for Seaboard. This ensures that a
> message is printed if CONFIG_OF_CONTROL is in use and a value device tree
> is not available.

As explained before, I consider this concept broken.  either you know
which port(s) can be used as console (then use these, and this here is
not needed), or you don't know this (then don't mess with these
ports).

Best regards,

Wolfgang Denk

Patch

diff --git a/board/nvidia/seaboard/seaboard.c b/board/nvidia/seaboard/seaboard.c
index 94efb1e..3f69dfc 100644
--- a/board/nvidia/seaboard/seaboard.c
+++ b/board/nvidia/seaboard/seaboard.c
@@ -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_UART_ALL, CONFIG_DEFAULT_NS16550_CLK,
+				CONFIG_DEFAULT_NS16550_MULT, str);
+}