Re: [U-Boot] [PATCH RFC] at91sam9/at91cap: move common initialisation to cpu
On 20:38 Fri 07 Nov , Jean-Christophe PLAGNIOL-VILLARD wrote: > Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD > --- The idea is to reduce common code initialisation which is actually in board. as done on afeb9260 and Ronetix eval board for 9260 & 9263 Any help to improve and continue this work is welcome Best Regards, J. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH RFC] at91sam9/at91cap: move common initialisation to cpu
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 <[EMAIL PROTECTED]> > > ___ > 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 RFC] at91sam9/at91cap: move common initialisation to cpu
Le jeudi 13 novembre 2008 à 23:32 +0100, Jean-Christophe PLAGNIOL-VILLARD a écrit : > > 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. > I've just put serial & spi for the RFC Ok then. [...] > > 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. > The idea is too allow multiple serial at the same time. > > I've plan to add the multi serial support to the at91 serial drivers I'm not sure exactly what you mean by "multi serial support" but isn't this orthogonal to what I'm saying (putting the #ifdef CONFIG_USART0 inside the cpu specific file instead of board specific file) ? Stelian. -- Stelian Pop <[EMAIL PROTECTED]> ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH RFC] at91sam9/at91cap: move common initialisation to cpu
On 22:28 Thu 13 Nov , Stelian Pop wrote: > Le vendredi 07 novembre 2008 à 20:38 +0100, Jean-Christophe > PLAGNIOL-VILLARD a écrit : > > > Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <[EMAIL PROTECTED]> > > --- > > The idea is to reduce common code initialisation which is actually in board. > > as done on afeb9260 and Ronetix eval board for 9260 & 9263 > > [...] > > I agree with the general idea of moving the serial and spi from board > specific files to the cpu specific files. > > 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. I've just put serial & spi for the RFC > 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). This the idea > > 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. The idea is too allow multiple serial at the same time. I've plan to add the multi serial support to the at91 serial drivers Best Regards, J. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH RFC] at91sam9/at91cap: move common initialisation to cpu
Le vendredi 07 novembre 2008 à 20:38 +0100, Jean-Christophe PLAGNIOL-VILLARD a écrit : > Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <[EMAIL PROTECTED]> > --- > The idea is to reduce common code initialisation which is actually in board. > as done on afeb9260 and Ronetix eval board for 9260 & 9263 [...] I agree with the general idea of moving the serial and spi from board specific files to the cpu specific files. 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 <[EMAIL PROTECTED]> ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH RFC] at91sam9/at91cap: move common initialisation to cpu
Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <[EMAIL PROTECTED]>
---
The idea is to reduce common code initialisation which is actually in board.
as done on afeb9260 and Ronetix eval board for 9260 & 9263
Best Regards,
J.
board/afeb9260/afeb9260.c | 32 ++--
board/atmel/at91cap9adk/at91cap9adk.c | 33 ++---
board/atmel/at91sam9260ek/at91sam9260ek.c | 34 ++---
board/atmel/at91sam9261ek/at91sam9261ek.c | 33 ++---
board/atmel/at91sam9263ek/at91sam9263ek.c | 33 ++---
board/atmel/at91sam9rlek/at91sam9rlek.c | 33 ++---
cpu/arm926ejs/at91/Makefile | 19 +++-
cpu/arm926ejs/at91/at91cap9_serial.c | 73 +
cpu/arm926ejs/at91/at91cap9_spi.c | 41
cpu/arm926ejs/at91/at91sam9260_serial.c | 73 +
cpu/arm926ejs/at91/at91sam9260_spi.c | 42
cpu/arm926ejs/at91/at91sam9261_serial.c | 73 +
cpu/arm926ejs/at91/at91sam9261_spi.c | 41
cpu/arm926ejs/at91/at91sam9263_serial.c | 73 +
cpu/arm926ejs/at91/at91sam9263_spi.c | 41
cpu/arm926ejs/at91/at91sam9rl_serial.c| 73 +
cpu/arm926ejs/at91/at91sam9rl_spi.c | 41
include/asm-arm/arch-at91/at91_common.h | 31
18 files changed, 656 insertions(+), 163 deletions(-)
create mode 100644 cpu/arm926ejs/at91/at91cap9_serial.c
create mode 100644 cpu/arm926ejs/at91/at91cap9_spi.c
create mode 100644 cpu/arm926ejs/at91/at91sam9260_serial.c
create mode 100644 cpu/arm926ejs/at91/at91sam9260_spi.c
create mode 100644 cpu/arm926ejs/at91/at91sam9261_serial.c
create mode 100644 cpu/arm926ejs/at91/at91sam9261_spi.c
create mode 100644 cpu/arm926ejs/at91/at91sam9263_serial.c
create mode 100644 cpu/arm926ejs/at91/at91sam9263_spi.c
create mode 100644 cpu/arm926ejs/at91/at91sam9rl_serial.c
create mode 100644 cpu/arm926ejs/at91/at91sam9rl_spi.c
create mode 100644 include/asm-arm/arch-at91/at91_common.h
diff --git a/board/afeb9260/afeb9260.c b/board/afeb9260/afeb9260.c
index 32445ab..e1d52ad 100644
--- a/board/afeb9260/afeb9260.c
+++ b/board/afeb9260/afeb9260.c
@@ -27,6 +27,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -47,27 +48,19 @@ DECLARE_GLOBAL_DATA_PTR;
static void afeb9260_serial_hw_init(void)
{
#ifdef CONFIG_USART0
- at91_set_A_periph(AT91_PIN_PB4, 1); /* TXD0 */
- at91_set_A_periph(AT91_PIN_PB5, 0); /* RXD0 */
- at91_sys_write(AT91_PMC_PCER, 1 << AT91SAM9260_ID_US0);
+ at91_serial_hw_init(0);
#endif
#ifdef CONFIG_USART1
- at91_set_A_periph(AT91_PIN_PB6, 1); /* TXD1 */
- at91_set_A_periph(AT91_PIN_PB7, 0); /* RXD1 */
- at91_sys_write(AT91_PMC_PCER, 1 << AT91SAM9260_ID_US1);
+ at91_serial_hw_init(1);
#endif
#ifdef CONFIG_USART2
- at91_set_A_periph(AT91_PIN_PB8, 1); /* TXD2 */
- at91_set_A_periph(AT91_PIN_PB9, 0); /* RXD2 */
- at91_sys_write(AT91_PMC_PCER, 1 << AT91SAM9260_ID_US2);
+ at91_serial_hw_init(2);
#endif
#ifdef CONFIG_USART3 /* DBGU */
- at91_set_A_periph(AT91_PIN_PB14, 0);/* DRXD */
- at91_set_A_periph(AT91_PIN_PB15, 1);/* DTXD */
- at91_sys_write(AT91_PMC_PCER, 1 << AT91_ID_SYS);
+ at91_serial_hw_init(3);
#endif
}
@@ -104,19 +97,6 @@ static void afeb9260_nand_hw_init(void)
at91_set_gpio_output(AT91_PIN_PC14, 1);
}
-static void afeb9260_spi_hw_init(void)
-{
- at91_set_A_periph(AT91_PIN_PA3, 0); /* SPI0_NPCS0 */
- at91_set_B_periph(AT91_PIN_PC11, 0);/* SPI0_NPCS1 */
-
- at91_set_A_periph(AT91_PIN_PA0, 0); /* SPI0_MISO */
- at91_set_A_periph(AT91_PIN_PA1, 0); /* SPI0_MOSI */
- at91_set_A_periph(AT91_PIN_PA2, 0); /* SPI0_SPCK */
-
- /* Enable clock */
- at91_sys_write(AT91_PMC_PCER, 1 << AT91SAM9260_ID_SPI0);
-}
-
#ifdef CONFIG_MACB
static void afeb9260_macb_hw_init(void)
{
@@ -205,7 +185,7 @@ int board_init(void)
#ifdef CONFIG_CMD_NAND
afeb9260_nand_hw_init();
#endif
- afeb9260_spi_hw_init();
+ at91_spi_hw_init();
#ifdef CONFIG_MACB
afeb9260_macb_hw_init();
#endif
diff --git a/board/atmel/at91cap9adk/at91cap9adk.c
b/board/atmel/at91cap9adk/at91cap9adk.c
index f7d68b7..2d2cfe5 100644
--- a/board/atmel/at91cap9adk/at91cap9adk.c
+++ b/board/atmel/at91cap9adk/at91cap9adk.c
@@ -26,6 +26,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -50,27 +51,19 @@ DECLARE_GLOBAL_DATA_PTR;
static void at91cap9_serial_hw_init(void)
{
#ifdef CONFIG_USART0
- at91_set_A_periph(AT91_PIN_PA22, 1);/* TXD0 */
- at91_set_A_periph(AT91_PIN_PA23, 0);/* RXD0 */
- at91_s
