Re: [U-Boot] [PATCH 8/9] Tegra30: Add common pinmux config in board_early_init_f

2012-09-18 Thread Simon Glass
Hi,

On Thu, Sep 13, 2012 at 3:37 PM, Stephen Warren swar...@wwwdotorg.org wrote:
 On 09/12/2012 04:10 PM, Tom Warren wrote:
 Signed-off-by: Tom Warren twar...@nvidia.com
 ---
  board/nvidia/common/board.c |   27 ++-
  1 files changed, 26 insertions(+), 1 deletions(-)

 Common code:-) :-) But ...

 diff --git a/board/nvidia/common/board.c b/board/nvidia/common/board.c

 +#ifdef CONFIG_TEGRA30
 +#include ../cardhu/pinmux-config-common.h
 +#endif

 Not all Tegra30 will be Cardhu...

 Given this is really board-specific, shouldn't the following be an empty
 weak definition:

 +/*
 + * Routine: pinmux_init
 + * Description: Do individual peripheral pinmux configs
 + */
 +static void pinmux_init(void)
 +{
 +#if defined(CONFIG_TEGRA30)
 + pinmux_config_table(tegra3_pinmux_common,
 + ARRAY_SIZE(tegra3_pinmux_common));
 +
 + pinmux_config_table(unused_pins_lowpower,
 + ARRAY_SIZE(unused_pins_lowpower));
 +#endif
 +}

 ... and the function be overridden in board files as needed?

 If we are moving to a model of a single function that sets up the entire
 pin mux at boot (which seems fine to me, and could eventually be driven
 by DT if it happened late enough), then it seems like we wouldn't need
 e.g. pin_mux_mmc() or pin_mux_usb() any more.

While the fdt may eventually remove this discussion, I don't think
forcing a one-time pinmux init is the best idea. Some peripherals will
not be needed on every boot (e.g. normally boot from eMMC unless USB
is available). Some peripherals may want to change their config based
on run-time settings (although this is unlikely I suppose,
particularly if we have the fdt).

Regards,
Simon

 ___
 U-Boot mailing list
 U-Boot@lists.denx.de
 http://lists.denx.de/mailman/listinfo/u-boot
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH 8/9] Tegra30: Add common pinmux config in board_early_init_f

2012-09-18 Thread Tom Warren
Simon,

On Tue, Sep 18, 2012 at 12:53 PM, Simon Glass s...@chromium.org wrote:
 Hi,

 On Thu, Sep 13, 2012 at 3:37 PM, Stephen Warren swar...@wwwdotorg.org wrote:
 On 09/12/2012 04:10 PM, Tom Warren wrote:
 Signed-off-by: Tom Warren twar...@nvidia.com
 ---
  board/nvidia/common/board.c |   27 ++-
  1 files changed, 26 insertions(+), 1 deletions(-)

 Common code:-) :-) But ...

 diff --git a/board/nvidia/common/board.c b/board/nvidia/common/board.c

 +#ifdef CONFIG_TEGRA30
 +#include ../cardhu/pinmux-config-common.h
 +#endif

 Not all Tegra30 will be Cardhu...

 Given this is really board-specific, shouldn't the following be an empty
 weak definition:

 +/*
 + * Routine: pinmux_init
 + * Description: Do individual peripheral pinmux configs
 + */
 +static void pinmux_init(void)
 +{
 +#if defined(CONFIG_TEGRA30)
 + pinmux_config_table(tegra3_pinmux_common,
 + ARRAY_SIZE(tegra3_pinmux_common));
 +
 + pinmux_config_table(unused_pins_lowpower,
 + ARRAY_SIZE(unused_pins_lowpower));
 +#endif
 +}

 ... and the function be overridden in board files as needed?

 If we are moving to a model of a single function that sets up the entire
 pin mux at boot (which seems fine to me, and could eventually be driven
 by DT if it happened late enough), then it seems like we wouldn't need
 e.g. pin_mux_mmc() or pin_mux_usb() any more.

 While the fdt may eventually remove this discussion, I don't think
 forcing a one-time pinmux init is the best idea. Some peripherals will
 not be needed on every boot (e.g. normally boot from eMMC unless USB
 is available). Some peripherals may want to change their config based
 on run-time settings (although this is unlikely I suppose,
 particularly if we have the fdt).

I've been basically adapting our internal T30 U-Boot code to fit
within the framework of what's upstream, including Allen's SPL reorg.
Pinmux on our T30 codebase (including the Chromium U-Boot code, I
believe) was done in a single monolithic file, as demonstrated in this
patch. The advantages are that you can construct the table to ensure
there are no conflicts, no small task with 4 options per mux and over
100 muxes. Even the HW power-on defaults have conflicts.  The only
exception in this patchset was the UART muxing, so we can start
putting out debug/status spew to the console as early as possible.

As far as I'm aware, an FDT pinmux for Tegra (which is on my plate,
but quite a bit behind T30) would be essentially the same deal - one
large list of mux settings per build/board.

Also, we've had bugs submitted by the kernel guys at times because
U-Boot didn't do _enough_ init. While I agree with the U-Boot
philosophy of doing just what's necessary to get a kernel loaded, I
don't consider pinmux init from a table as a time-consuming or
code-intensive operation, and see no harm in leaving it in. The driver
can always remap the muxes as it chooses at a later date, based on
what it discovers or knows from its config or the DT.

Thanks,

Tom

 Regards,
 Simon

 ___
 U-Boot mailing list
 U-Boot@lists.denx.de
 http://lists.denx.de/mailman/listinfo/u-boot
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH 8/9] Tegra30: Add common pinmux config in board_early_init_f

2012-09-18 Thread Stephen Warren
On 09/18/2012 03:32 PM, Tom Warren wrote:
...
 As far as I'm aware, an FDT pinmux for Tegra (which is on my plate,
 but quite a bit behind T30) would be essentially the same deal - one
 large list of mux settings per build/board.

Initializing pinmux from DT can work either way, depending on how the DT
author wrote the DT file.

The pinmux DT node can contain a pinmux configuration which is applied
as soon as the pinmux driver loads. This configuration can contain as
little as you want (even nothing) all the way through to containing the
entire board's static pinmux configuration.

For portions of the pinmux settings which the pinmux driver's own DT
node doesn't configure (if any, based on the above), the relevant
individual driver DT node can configure the pinmux as required, and that
configuration would be applied when the relevant driver loads and parses
its DT node.

In practice, so far, all the kernel board files for Tegra almost
exclusively use static muxing in the pinmux controller's own DT node.
However, there are a couple small dynamic cases (e.g.
Seaboard/Springbank's pinctrl-based I2C mux for example).
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


Re: [U-Boot] [PATCH 8/9] Tegra30: Add common pinmux config in board_early_init_f

2012-09-13 Thread Stephen Warren
On 09/12/2012 04:10 PM, Tom Warren wrote:
 Signed-off-by: Tom Warren twar...@nvidia.com
 ---
  board/nvidia/common/board.c |   27 ++-
  1 files changed, 26 insertions(+), 1 deletions(-)

Common code:-) :-) But ...

 diff --git a/board/nvidia/common/board.c b/board/nvidia/common/board.c

 +#ifdef CONFIG_TEGRA30
 +#include ../cardhu/pinmux-config-common.h
 +#endif

Not all Tegra30 will be Cardhu...

Given this is really board-specific, shouldn't the following be an empty
weak definition:

 +/*
 + * Routine: pinmux_init
 + * Description: Do individual peripheral pinmux configs
 + */
 +static void pinmux_init(void)
 +{
 +#if defined(CONFIG_TEGRA30)
 + pinmux_config_table(tegra3_pinmux_common,
 + ARRAY_SIZE(tegra3_pinmux_common));
 +
 + pinmux_config_table(unused_pins_lowpower,
 + ARRAY_SIZE(unused_pins_lowpower));
 +#endif
 +}

... and the function be overridden in board files as needed?

If we are moving to a model of a single function that sets up the entire
pin mux at boot (which seems fine to me, and could eventually be driven
by DT if it happened late enough), then it seems like we wouldn't need
e.g. pin_mux_mmc() or pin_mux_usb() any more.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


[U-Boot] [PATCH 8/9] Tegra30: Add common pinmux config in board_early_init_f

2012-09-12 Thread Tom Warren
Signed-off-by: Tom Warren twar...@nvidia.com
---
 board/nvidia/common/board.c |   27 ++-
 1 files changed, 26 insertions(+), 1 deletions(-)

diff --git a/board/nvidia/common/board.c b/board/nvidia/common/board.c
index afe832a..4a86c30 100644
--- a/board/nvidia/common/board.c
+++ b/board/nvidia/common/board.c
@@ -1,5 +1,5 @@
 /*
- *  (C) Copyright 2010,2011
+ *  (C) Copyright 2010-2012
  *  NVIDIA Corporation www.nvidia.com
  *
  * See file CREDITS for list of people who contributed to this
@@ -25,7 +25,11 @@
 #include ns16550.h
 #include linux/compiler.h
 #include asm/io.h
+#if defined(CONFIG_TEGRA20)
 #include asm/arch/tegra20.h
+#else  /* Tegra30 */
+#include asm/arch/tegra30.h
+#endif
 #include asm/arch/sys_proto.h
 
 #include asm/arch/board.h
@@ -87,6 +91,25 @@ static void power_det_init(void)
 #endif
 }
 
+#ifdef CONFIG_TEGRA30
+#include ../cardhu/pinmux-config-common.h
+#endif
+
+/*
+ * Routine: pinmux_init
+ * Description: Do individual peripheral pinmux configs
+ */
+static void pinmux_init(void)
+{
+#if defined(CONFIG_TEGRA30)
+   pinmux_config_table(tegra3_pinmux_common,
+   ARRAY_SIZE(tegra3_pinmux_common));
+
+   pinmux_config_table(unused_pins_lowpower,
+   ARRAY_SIZE(unused_pins_lowpower));
+#endif
+}
+
 /*
  * Routine: board_init
  * Description: Early hardware init.
@@ -152,6 +175,8 @@ void gpio_early_init(void) __attribute__((weak, 
alias(__gpio_early_init)));
 
 int board_early_init_f(void)
 {
+   pinmux_init();
+
board_init_uart_f();
 
/* Initialize periph GPIOs */
-- 
1.7.0.4

___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot