[U-Boot] use "boot select" jumper on NGW100 to select USART

Thomas Sprinkmeier thomas.sprinkmeier at gmail.com
Tue Nov 10 07:49:35 CET 2009


Dear Wolfgang Denk,

>> +     USART_JUMPER_CONFIG;
>> +     USART_ENABLE;
>
> Function / Macro when used as funxctions should always look as such,
> i. e. have parens with them.

So the patch for atmel_usart.c would look like this:

 	hmatrix_slave_write(EBI, SFR, HMATRIX_BIT(EBI_SDRAM_ENABLE));

 	portmux_enable_ebi(16, 23, 0, PORTMUX_DRIVE_HIGH);
-	portmux_enable_usart1(PORTMUX_DRIVE_MIN);
+
+	USART_JUMPER_CONFIG();
+	atmel_usart_enable();

and atmel_alt_usart.h would change to:

+#elif
+/* No alternate USART selected */
+#define USART_USE_ALT() (0)
+#define USART_PIN_CONFIG()
+#endif
+
+/* How to read/initialise the boot-select jumper */
+#ifndef USART_USE_ALT
+#define USART_USE_ALT() (!gpio_get_value(GPIO_PIN_PB(30)))
+#define USART_JUMPER_CONFIG() portmux_select_gpio(PORTMUX_PORT_B,	\
+						(1 << 30),										\
+						PORTMUX_DIR_INPUT)
+#endif

or should USART_USE_ALT and USART_JUMPER_CONFIG become static inline
functions (as below)?

I was trying to make as little impact as possible, somehow a macro
seemed 'better' than a function.

>> diff --git a/include/atmel_alt_usart.h b/include/atmel_alt_usart.h
>> new file mode 100644
>> index 0000000..1ed2fb8
> ...
>> +/* Enable the appropriate USART */
>> +#define USART_ENABLE {                                                       \
>> +  switch (USART_ID)                                                  \
>> +    {                                                                        \
>> +    case 0:  portmux_enable_usart0(PORTMUX_DRIVE_MIN); break;                \
>> +    case 1:  portmux_enable_usart1(PORTMUX_DRIVE_MIN); break;                \
>> +    case 2:  portmux_enable_usart2(PORTMUX_DRIVE_MIN); break;                \
>> +    default: portmux_enable_usart3(PORTMUX_DRIVE_MIN); break;                \
>> +    }                                                                        \
>> +  } while(0)
>
> NAK.  When you use a function-style macro it should look like one. But
> why is this a macro at all? Please make it a (static) inline function
> instead.



+/* Enable the appropriate USART */
+static inline void atmel_usart_enable()
+{
+	switch (USART_ID) {
+	case 0:
+		portmux_enable_usart0(PORTMUX_DRIVE_MIN);
+		break;
+	case 1:
+		portmux_enable_usart1(PORTMUX_DRIVE_MIN);
+		break;
+	case 2:
+		portmux_enable_usart2(PORTMUX_DRIVE_MIN);
+		break;
+	default:
+		portmux_enable_usart3(PORTMUX_DRIVE_MIN);
+	}
+}
+
+#endif /* __ATMEL_ALT_USART_H__ */

> Also please note that the Coding Style requires indentation by TABs.

fixed.

Thank you for your patience,

Thomas


More information about the U-Boot mailing list