| Submitter | Stefan Reinauer |
|---|---|
| Date | 2011-04-21 23:06:29 |
| Message ID | <20110421230628.GA25213@coreboot.org> |
| Download | mbox | patch |
| Permalink | /patch/2902/ |
| State | Accepted |
| Headers | show |
Comments
Stefan Reinauer wrote: > The UART divider should be calculated based on the base frequency > and baudrate, not hardcoded in addition to that. > > Signed-off-by: Stefan Reinauer <stefan.reinauer@coreboot.org> With some changes pointed out below it's: Acked-by: Peter Stuge <peter@stuge.se> > +++ src/mainboard/iwave/iWRainbowG6/Kconfig (working copy) > @@ -37,9 +37,7 @@ > int > default 10 > > -# WTF, is this 19200? > -config TTYS0_DIV > - int > - default 6 > +# This mainboard might have a higher clocked UART or might not be able to run > +# serial output at 115200 baud I don't think this board has a special UART, I think the setting was just chosen by the original developer as a way to make coreboot configure the desired baudrate. So just drop the setting and let the regular baudrate mechanism in Kconfig do the job. > +++ src/console/uart8250_console.c (working copy) > @@ -25,7 +25,13 @@ > { > static const unsigned char div[8] = { 1, 2, 3, 6, 12, 24, 48, 96 }; > int b_index = 0; > - unsigned int divisor = CONFIG_TTYS0_DIV; > + /* TODO the divisor calculation is hard coded to standard UARTs. Some > + * UARTs won't work with these values. This should be a property of the > + * UART used, worst case a Kconfig variable. For now live with hard > + * codes as the only devices that are different are the iWave > + * iRainbowG6 and the OXPCIe952 card (and the latter is memory mapped) Also change this wording since it's not really certain that iRainbowG6 actually is different. (Btw it would be nice to make the mainboard directory name all lowercase, like with every other board.) > +++ src/lib/uart8250.c (working copy) > @@ -88,6 +88,12 @@ > #if defined(__PRE_RAM__) || defined(__SMM__) > void uart_init(void) > { > + /* TODO the divisor calculation is hard coded to standard UARTs. Some > + * UARTs won't work with these values. This should be a property of the > + * UART used, worst case a Kconfig variable. For now live with hard > + * codes as the only devices that are different are the iWave > + * iRainbowG6 and the OXPCIe952 card (and the latter is memory mapped) Same with this comment. //Peter
On 4/21/11 4:43 PM, Peter Stuge wrote: > Stefan Reinauer wrote: >> The UART divider should be calculated based on the base frequency >> and baudrate, not hardcoded in addition to that. >> >> Signed-off-by: Stefan Reinauer<stefan.reinauer@coreboot.org> > With some changes pointed out below it's: > > Acked-by: Peter Stuge<peter@stuge.se> > Thanks. Comments improved and committed as r6538
Patch
The UART divider should be calculated based on the base frequency and baudrate, not hardcoded in addition to that. Signed-off-by: Stefan Reinauer <stefan.reinauer@coreboot.org> Index: src/include/uart8250.h =================================================================== --- src/include/uart8250.h (revision 6537) +++ src/include/uart8250.h (working copy) @@ -28,13 +28,9 @@ #ifndef CONFIG_TTYS0_BAUD #define CONFIG_TTYS0_BAUD 115200 #endif - -#ifndef CONFIG_TTYS0_DIV #if ((115200%CONFIG_TTYS0_BAUD) != 0) #error Bad ttys0 baud rate #endif -#define CONFIG_TTYS0_DIV (115200/CONFIG_TTYS0_BAUD) -#endif /* Line Control Settings */ #ifndef CONFIG_TTYS0_LCS Index: src/mainboard/iwave/iWRainbowG6/Kconfig =================================================================== --- src/mainboard/iwave/iWRainbowG6/Kconfig (revision 6537) +++ src/mainboard/iwave/iWRainbowG6/Kconfig (working copy) @@ -37,9 +37,7 @@ int default 10 -# WTF, is this 19200? -config TTYS0_DIV - int - default 6 +# This mainboard might have a higher clocked UART or might not be able to run +# serial output at 115200 baud endif Index: src/console/uart8250_console.c =================================================================== --- src/console/uart8250_console.c (revision 6537) +++ src/console/uart8250_console.c (working copy) @@ -25,7 +25,13 @@ { static const unsigned char div[8] = { 1, 2, 3, 6, 12, 24, 48, 96 }; int b_index = 0; - unsigned int divisor = CONFIG_TTYS0_DIV; + /* TODO the divisor calculation is hard coded to standard UARTs. Some + * UARTs won't work with these values. This should be a property of the + * UART used, worst case a Kconfig variable. For now live with hard + * codes as the only devices that are different are the iWave + * iRainbowG6 and the OXPCIe952 card (and the latter is memory mapped) + */ + unsigned int divisor = 115200 / CONFIG_TTYS0_BAUD; if (get_option(&b_index, "baud_rate") == 0) { divisor = div[b_index]; Index: src/lib/uart8250.c =================================================================== --- src/lib/uart8250.c (revision 6537) +++ src/lib/uart8250.c (working copy) @@ -88,6 +88,12 @@ #if defined(__PRE_RAM__) || defined(__SMM__) void uart_init(void) { + /* TODO the divisor calculation is hard coded to standard UARTs. Some + * UARTs won't work with these values. This should be a property of the + * UART used, worst case a Kconfig variable. For now live with hard + * codes as the only devices that are different are the iWave + * iRainbowG6 and the OXPCIe952 card (and the latter is memory mapped) + */ #if CONFIG_USE_OPTION_TABLE && !defined(__SMM__) static const unsigned char divisor[] = { 1, 2, 3, 6, 12, 24, 48, 96 }; unsigned ttys0_div, ttys0_index; @@ -97,7 +103,7 @@ uart8250_init(CONFIG_TTYS0_BASE, ttys0_div); #else - uart8250_init(CONFIG_TTYS0_BASE, CONFIG_TTYS0_DIV); + uart8250_init(CONFIG_TTYS0_BASE, (115200 / CONFIG_TTYS0_BAUD)); #endif } #endif