[U-Boot] [PATCH RFC] at91sam9/at91cap: move common initialisation to cpu

[Remy Bohmer <linux@bohmer.net>]

Hello Stelian and Jean-Christophe,

> I agree with the general idea of moving the serial and spi from board
> specific files to the cpu specific files.

I agree partly on this, but I do not like with the way this patch is
doing it, and this is the reason why:
PIN multiplexing is board specific, not only SoC specific. pins can be
multiplexed in different ways per board per SoC.
In this patch all this 'knowledge' is moved to the generic SoC code,
where the assumption is made that all boards use the same pin layout.
This is _not_ true; in the generic code is, for example(!!), assumed
that all 9261 SoC derived boards only configure NPCS0, while a board
can utilise all NPCS pins and can have multiple dataflashes.
It may be true that all boards, currently existing in U-boot, use a
similar pin layout per SoC, but that does not mean that that is a
valid assumption for all future boards. The framework should be that
flexible that boards that are currently not in mainline can be added
in the future, without reverting patches like this, or creating
standalone copies from this framework. We should make it easier for
others to integrate their custom boards, instead of making it
harder...
The current code makes it very easy to create custom boards, this
patch makes that harder!

So, in short: pin layout must be specified by the board specific
files, not by the generic SoC code, although the generic SoC code can
provide (all) the possible options to choose from...

Kind Regards,

Remy



>
> Note however that serial and spi are not the only initialisations that
> can be moved: the very same thing can be done with all the other device
> initialisations. Some devices will need to specify which GPIOs are used
> when it's board specific, and this can be passed as arguments to the cpu
> specfic function. One can look at the Linux kernel files which nicely
> separates the cpu specific GPIOs (in arch/arm/mach-at91/XXX_devices.c)
> and the board specific GPIOs (in arch/arm/mach-at91/board-YYY.c). Or one
> can also RTFM of course.
>
> I also don't like this:
>
>> --- a/board/atmel/at91cap9adk/at91cap9adk.c
>> +++ b/board/atmel/at91cap9adk/at91cap9adk.c
>
>>  #ifdef CONFIG_USART0
>> -     at91_set_A_periph(AT91_PIN_PA22, 1);            /* TXD0 */
>> -     at91_set_A_periph(AT91_PIN_PA23, 0);            /* RXD0 */
>> -     at91_sys_write(AT91_PMC_PCER, 1 << AT91CAP9_ID_US0);
>> +     at91_serial_hw_init(0);
>>  #endif
>
> Why don't just do at91_serial_hw_init() once, and test the CONFIG_USART*
> inside the cpu specific at91_serial_hw_init function() ?
>
> This also eliminates that unneeded switch case in that function.
>
> Stelian.
> --
> Stelian Pop <stelian@popies.net>
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
>