On 9/13/21 4:24 PM, Tom Rini wrote:
In order to move CONFIG_SYS_BAUDRATE_TABLE to Kconfig, we need to rework the logic a bit. Rename the users of CONFIG_SYS_BAUDRATE_TABLE to SYS_BAUDRATE_TABLE. Introduce a series of CONFIG_BAUDRATE_TABLE_... that include some number of baud rates. These match all existing users. The help for each entry specifies what the exact table is, for a given option. Define what SYS_BAUDRATE_TABLE will be in include/serial.h now.
Signed-off-by: Tom Rini trini@konsulko.com
diff --git a/include/serial.h b/include/serial.h index 6d1e62c6770c..150644c4c3d4 100644 --- a/include/serial.h +++ b/include/serial.h @@ -3,6 +3,42 @@
#include <post.h>
+#if defined(CONFIG_BAUDRATE_TABLE_300_TO_38400_115200) +#define SYS_BAUDRATE_TABLE { 300, 600, 1200, 2400, 4800, 9600, 19200, \
38400, 115200 }+#elif defined(CONFIG_BAUDRATE_TABLE_300_TO_115200) +#define SYS_BAUDRATE_TABLE { 300, 600, 1200, 2400, 4800, 9600, 19200, \
38400, 57600, 115200 }+#elif defined(CONFIG_BAUDRATE_TABLE_300_TO_230400) +#define SYS_BAUDRATE_TABLE { 300, 600, 1200, 2400, 4800, 9600, 19200, \
38400, 57600, 115200, 230400 }+#elif defined(CONFIG_BAUDRATE_TABLE_300_TO_6000000) +#define SYS_BAUDRATE_TABLE { 300, 600, 1200, 1800, 2400, 4800, 9600, \
19200, 38400, 57600, 115200, 230400, \460800, 500000, 576000, 921600, 1000000, \1152000, 1500000, 2000000, 2500000, \3000000, 3500000, 4000000, 4500000, \5000000, 5500000, 6000000 }+#elif defined(CONFIG_BAUDRATE_TABLE_4800_TO_115200) +#define SYS_BAUDRATE_TABLE { 4800, 9600, 19200, 38400, 57600, 115200 } +#elif defined(CONFIG_BAUDRATE_TABLE_9600_TO_115200) +#define SYS_BAUDRATE_TABLE { 9600, 19200, 38400, 57600, 115200 } +#elif defined(CONFIG_BAUDRATE_TABLE_9600_TO_230400) +#define SYS_BAUDRATE_TABLE { 9600, 19200, 38400, 57600, 115200, 230400 } +#elif defined(CONFIG_BAUDRATE_TABLE_9600_TO_460800) +#define SYS_BAUDRATE_TABLE { 9600, 19200, 38400, 57600, 115200, 230400, 460800 } +#elif defined(CONFIG_BAUDRATE_TABLE_9600_TO_921600) +#define SYS_BAUDRATE_TABLE { 9600, 19200, 38400, 57600, 115200, 230400, \
460800, 921600 }+#elif defined(CONFIG_BAUDRATE_TABLE_9600_TO_230400_500000_1500000) +#define SYS_BAUDRATE_TABLE { 9600, 19200, 38400, 57600, 115200, 230400, \
500000, 1500000 }+#elif defined(CONFIG_BAUDRATE_TABLE_38400_115200_ONLY) +#define SYS_BAUDRATE_TABLE { 38400, 115200 } +#elif defined(CONFIG_BAUDRATE_TABLE_115200_ONLY) +#define SYS_BAUDRATE_TABLE { 115200 } +#endif
- struct serial_device { /* enough bytes to match alignment of following func pointer */ char name[16];
This opens the gates to #ifdefing the heck out of serial.h. What happens to my board that goes from 300 to 2000000? * We need a new Kconfig and new ifdef What happens to my other board that goes from 300 to 2500000? * We need a new Kconfig and new ifdef The pattern doesn't look promising.
I actually think this change can make the situation worse. We trade having an antiquated and inconvenient SYS_BAUDRATE_TABLE for one Kconfig per each possible baudrate combination. How does this make sense?
I've seen situations were SPL boots with 2Mbaud and executes succesfully, u-boot starts up with 2Mbaud just fine. few lines later, u-boot, downswitches to 115200 because CONFIG_SYS_BAUDRATE_TABLE says so.
Suggestion I: Can we have a MIN/MAX value for baudrates, and have the code work from there ?
Suggestion II: Define the Kconfig SYS_BAUDRATE_TABLE table to a C array, like 'default "{ 300, 420, 690}" ' and forego the #ifdefs in serial.h
Suggestion III: Get rid of the logic that says "baudrate must be one of these predefined values" and let the serial driver return -ENOBUENO or -EINVAL if the hardware really can't do that baudrate. Most UARTs nowadays can do a wide range of values, and the baudrate table doesn't model that very well. Combine this with a CONFIG_MAX_BAUDRATE so that boards with shitty RS232 converters can set a safe upper limit -- and make sure CONFIG_BAUDRATE also enforces this.
There's a lot of unrealized potential here.
Alex