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

[Tom Warren]

Simon,

On Tue, Sep 18, 2012 at 12:53 PM, Simon Glass <sjg at chromium.org> wrote:
> Hi,
>
> On Thu, Sep 13, 2012 at 3:37 PM, Stephen Warren <swarren at wwwdotorg.org> wrote:
>> On 09/12/2012 04:10 PM, Tom Warren wrote:
>>> Signed-off-by: Tom Warren <twarren at 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 at lists.denx.de
>> http://lists.denx.de/mailman/listinfo/u-boot