From mboxrd@z Thu Jan 1 00:00:00 1970 From: Wolfgang Denk Date: Thu, 05 Nov 2009 21:08:41 +0100 Subject: [U-Boot] [PATCH] use "boot selecr" jumper on NGW100 to select USART In-Reply-To: References: Message-ID: <20091105200841.C1AFD3F6EC@gemini.denx.de> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Dear Thomas Sprinkmeier, In message you wrote: > Without the "boot select" jumper U-Boot will use the USART selected > using the CONFIG_USART1, CONFIG_USART2, directive. Please fix the typo in the Subject: > If CONFIG_ALT_USART1 (or ..2, ..3, ..0) is defined then, > with the jumper in place, that USART is used instead. > > Signed-off-by: Thomas Sprinkmeier > --- > board/atmel/atngw100/atngw100.c | 40 ++++++++++++++++++++++++++++ > drivers/serial/atmel_usart.c | 54 ++++++++++++++++++++++++++++++++++++++- > drivers/serial/atmel_usart.h | 9 +++--- > include/configs/atngw100.h | 1 + > 4 files changed, 98 insertions(+), 6 deletions(-) > > diff --git a/board/atmel/atngw100/atngw100.c b/board/atmel/atngw100/atngw100.c > index 004d8da..a2ffdb4 100644 > --- a/board/atmel/atngw100/atngw100.c > +++ b/board/atmel/atngw100/atngw100.c > @@ -53,7 +53,47 @@ int board_early_init_f(void) > hmatrix_slave_write(EBI, SFR, HMATRIX_BIT(EBI_SDRAM_ENABLE)); > > portmux_enable_ebi(16, 23, 0, PORTMUX_DRIVE_HIGH); > + > +#if defined(CONFIG_ALT_USART0) || defined(CONFIG_ALT_USART1) || \ > + defined(CONFIG_ALT_USART2) || defined(CONFIG_ALT_USART3) > +#define USART_USE_ALT (!gpio_get_value(GPIO_PIN_PB(30))) Your patch is white space corrupted. All "original" indentation was by TABs, but in your patch it's all spaces. > + /* PB30, the 'boot' jumper with external pull-up resistor */ > + /* PORTMUX_DIR_INPUT == 0 as high-impedance input is the default state. > + In other words, I don't think this actually does anything... */ Incorrect multiline comment style. Please fix globally. > + if (USART_USE_ALT) { > +#if defined(CONFIG_ALT_USART0) > + portmux_enable_usart0(PORTMUX_DRIVE_MIN); > +#elif defined(CONFIG_ALT_USART1) > + portmux_enable_usart1(PORTMUX_DRIVE_MIN); > +#elif defined(CONFIG_ALT_USART2) > + portmux_enable_usart2(PORTMUX_DRIVE_MIN); > +#elif defined(CONFIG_ALT_USART3) > + portmux_enable_usart3(PORTMUX_DRIVE_MIN); > +#endif Incorrect indentation. Please fix globally. > + } else { > +#endif > +#if defined(CONFIG_USART0) > + portmux_enable_usart0(PORTMUX_DRIVE_MIN); > +#elif defined(CONFIG_USART1) > portmux_enable_usart1(PORTMUX_DRIVE_MIN); > +#elif defined(CONFIG_USART2) > + portmux_enable_usart2(PORTMUX_DRIVE_MIN); > +#elif defined(CONFIG_USART3) > + portmux_enable_usart3(PORTMUX_DRIVE_MIN); > +#else If only one CONFIG_USARTx is allowed, you should think opf a way to get rid of these long, repeated #if...#elif...#elif constructs. > > -/* Register access macros */ > -#define usart3_readl(reg) \ > - readl((void *)USART_BASE + USART3_##reg) > -#define usart3_writel(reg,value) \ > - writel((value), (void *)USART_BASE + USART3_##reg) > +int atmel_serial_read(int reg); > +void atmel_serial_write(int reg, int value); > +#define usart3_readl(reg) atmel_serial_read(USART3_##reg) > +#define usart3_writel(reg, value) atmel_serial_write(USART3_##reg, value) Instead of making code more AT91 specific, we should try the opposite: make it _less_ so. Best regards, Wolfgang Denk -- DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de I really hate this damned machine It never does quite what I want I wish that they would sell it. But only what I tell it.