[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [Xen-devel] [PATCH v4] ns16550: Add support for UART parameters to be specifed with name-value pairs
Add name=value parsing options for com1 and com2 to add flexibility in setting register values for MMIO UART devices. Maintain backward compatibility with previous positional parameter specfications. eg. com1=115200,8n1,0x3f8,4 eg. com1=115200,8n1,0x3f8,4,reg_width=4,reg_shift=2 eg. com1=baud=115200,parity=n,reg_width=4,reg_shift=2,irq=4 Signed-off-by: Swapnil Paratey <swapnil.paratey@xxxxxxx> --- Changed since v3: * Changed subject/title of the patch Previous name: ns16550-Add-command-line-parsing-adjustments * Increased length of opt_com1 and opt_com2 buffers to 128 bytes. * Implementation changes from [PATCH v3] code review Changed since v2: * Added name=value specification for com1 and com2 command line parameter input for UART devices Syntax: "com1=(positional parameters),(name-value parameters)" * Maintained previous positional specification for UART parameters * All parameters should be comma-separated Changed since v1: * Changed opt_com1 and opt_com2 array size to 64 (power of 2). * Added descriptions for reg_width and reg_shift in docs/misc/xen-command-line.markdown * Changed subject to ns16550 from 16550 for better tracking. --- docs/misc/xen-command-line.markdown | 38 ++++++ xen/common/kernel.c | 2 +- xen/drivers/char/ns16550.c | 236 +++++++++++++++++++++++++++++++++--- 3 files changed, 259 insertions(+), 17 deletions(-) diff --git a/docs/misc/xen-command-line.markdown b/docs/misc/xen-command-line.markdown index 450b222..53052b2 100644 --- a/docs/misc/xen-command-line.markdown +++ b/docs/misc/xen-command-line.markdown @@ -324,6 +324,44 @@ Both option `com1` and `com2` follow the same format. A typical setup for most situations might be `com1=115200,8n1` +In addition to the above positional specification for UART parameters, +name=value pair specfications are also supported. This is used to add +flexibility for UART devices which require additional UART parameter +configurations. + +The comma separation still delineates positional parameters. Hence, +unless the parameter is explicitly specified with name=value option, it +will be considered a positional parameter. + +The syntax consists of +com1=(comma-separated positional parameters),(comma separated name-value pairs) + +The accepted name keywords for name=value pairs are + * `baud` - accepts integer baud rate (eg. 115200) or `auto` + * `bridge`- Similar to bridge-bdf in positional parameters. + Used to determine the PCI bridge to access the UART device. + Notation is xx:xx.xx <bus>:<device>.<function> + * `clock-hz`- accepts large integers to setup UART clock frequencies. + Do note - these values are multiplied by 16. + * `data-bits` - integer between 5 and 8 + * `dev` - accepted values are `pci` OR `amt`. If this option + is used to specify if the serial device is pci-based. The io_base + cannot be specified when `dev=pci` or `dev=amt` is used. + * `io-base` - accepts integer which specified IO base port for UART registers + * `irq` - IRQ number to use + * `parity` - accepted values are same as positional parameters + * `port` - Used to specify which port the PCI serial device is located on + Notation is xx:xx.xx <bus>:<device>.<function> + * `reg-shift` - register shifts required to set UART registers + * `reg-width` - register width required to set UART registers + (only accepts 1 and 4) + * `stop-bits` - only accepts 1 or 2 for the number of stop bits + +The following are examples of correct specifications: +`com1=115200,8n1,0x3f8,4` +`com1=115200,8n1,0x3f8,4,reg_width=4,reg_shift=2` +`com1=baud=115200,parity=n,stop_bits=1,io_base=0x3f8,reg_width=4` + ### conring\_size > `= <size>` diff --git a/xen/common/kernel.c b/xen/common/kernel.c index 8461871..e1ebb0b 100644 --- a/xen/common/kernel.c +++ b/xen/common/kernel.c @@ -49,7 +49,7 @@ static void __init assign_integer_param( static void __init _cmdline_parse(const char *cmdline) { - char opt[100], *optval, *optkey, *q; + char opt[128], *optval, *optkey, *q; const char *p = cmdline; const struct kernel_param *param; int bool_assert; diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c index e4de3b4..0d48dcf 100644 --- a/xen/drivers/char/ns16550.c +++ b/xen/drivers/char/ns16550.c @@ -38,11 +38,28 @@ * can be specified in place of a numeric baud rate. Polled mode is specified * by requesting irq 0. */ -static char __initdata opt_com1[30] = ""; -static char __initdata opt_com2[30] = ""; +static char __initdata opt_com1[128] = ""; +static char __initdata opt_com2[128] = ""; string_param("com1", opt_com1); string_param("com2", opt_com2); +typedef enum e_serial_param_type { + baud, + bridge_bdf, + clock_hz, + data_bits, + device, + io_base, + irq, + parity, + port_bdf, + reg_shift, + reg_width, + stop_bits, + /* list all parameters before this line */ + max_serial_params +} serial_param_type; + static struct ns16550 { int baud, clock_hz, data_bits, parity, stop_bits, fifo_size, irq; u64 io_base; /* I/O port or memory-mapped I/O address. */ @@ -77,6 +94,31 @@ static struct ns16550 { #endif } ns16550_com[2] = { { 0 } }; +struct serial_param_var +{ + const char *sp_name; + serial_param_type sp_type; +}; + +/* + * Enum struct keeping a table of all accepted parameter names + * for parsing cmdline for serial port com1 and com2 + */ +static const struct serial_param_var __initconst sp_vars[] = { + {"baud", baud}, + {"bridge", bridge_bdf}, + {"clock-hz", clock_hz}, + {"data-bits", data_bits}, + {"dev", device}, + {"io-base", io_base}, + {"irq", irq}, + {"parity", parity}, + {"port", port_bdf}, + {"reg-shift", reg_shift}, + {"reg-width", reg_width}, + {"stop-bits", stop_bits}, +}; + #ifdef CONFIG_HAS_PCI struct ns16550_config { u16 vendor_id; @@ -1083,26 +1125,76 @@ pci_uart_config(struct ns16550 *uart, bool_t skip_amt, unsigned int idx) } #endif +/* used to parse name value pairs and return which value it is + * along with pointer for the extracted value - ext_value */ +static __init serial_param_type get_token(char *token, char *ext_value) +{ + const char *param_name; + unsigned int i; + + param_name = strsep(&token, "="); + if ( param_name == NULL ) + return max_serial_params; + + /* linear search for the parameter */ + for ( i = 0; i < ARRAY_SIZE(sp_vars); i++ ) + { + if ( strcmp(sp_vars[i].sp_name, param_name) == 0 ) + { + /* token has the param value after the equal to sign + * ext_value is a 16-byte buffer - param_value[16] + */ + strlcpy(ext_value, token, 16); + return sp_vars[i].sp_type; + } + } + + return max_serial_params; +} + #define PARSE_ERR(_f, _a...) \ do { \ printk( "ERROR: " _f "\n" , ## _a ); \ return; \ } while ( 0 ) -static void __init ns16550_parse_port_config( - struct ns16550 *uart, const char *conf) +#define PARSE_ERR_RET(_f, _a...) \ + do { \ + printk( "ERROR: " _f "\n" , ## _a ); \ + return false; \ + } while ( 0 ) + + +static bool __init parse_positional(struct ns16550 *uart, char **str) { int baud; + const char *conf; + char *name_val_pos; - /* No user-specified configuration? */ - if ( (conf == NULL) || (*conf == '\0') ) + conf = *str; + name_val_pos = strchr(conf, '='); + + /* finding the end of the positional parameters */ + if ( name_val_pos ) { - /* Some platforms may automatically probe the UART configuartion. */ - if ( uart->baud != 0 ) - goto config_parsed; - return; + while (name_val_pos > *str) + { + name_val_pos--; /* working backwards from the = sign */ + if ( *name_val_pos == ',' ) + { + *name_val_pos = '\0'; + name_val_pos++; + break; + } + } } + *str = name_val_pos; + /* when there are no positional parameters, we return from the function */ + if (conf == *str) + return true; + + /* parse positional parameters here */ if ( strncmp(conf, "auto", 4) == 0 ) { uart->baud = BAUD_AUTO; @@ -1132,13 +1224,13 @@ static void __init ns16550_parse_port_config( if ( strncmp(conf, "pci", 3) == 0 ) { if ( pci_uart_config(uart, 1/* skip AMT */, uart - ns16550_com) ) - return; + return true; conf += 3; } else if ( strncmp(conf, "amt", 3) == 0 ) { if ( pci_uart_config(uart, 0, uart - ns16550_com) ) - return; + return true; conf += 3; } else @@ -1157,19 +1249,129 @@ static void __init ns16550_parse_port_config( conf = parse_pci(conf, NULL, &uart->ps_bdf[0], &uart->ps_bdf[1], &uart->ps_bdf[2]); if ( !conf ) - PARSE_ERR("Bad port PCI coordinates"); - uart->ps_bdf_enable = 1; + PARSE_ERR_RET("Bad port PCI coordinates"); + uart->ps_bdf_enable = true; } if ( *conf == ',' && *++conf != ',' ) { if ( !parse_pci(conf, NULL, &uart->pb_bdf[0], &uart->pb_bdf[1], &uart->pb_bdf[2]) ) - PARSE_ERR("Bad bridge PCI coordinates"); - uart->pb_bdf_enable = 1; + PARSE_ERR_RET("Bad bridge PCI coordinates"); + uart->pb_bdf_enable = true; } #endif + return true; +} + +static bool __init parse_namevalue_pairs(char *str, struct ns16550 *uart) +{ + char *token, *start = str; + char param_value[16]; + bool dev_set = false; + + if ( (str == NULL) || (*str == '\0') ) + return true; + + /* strsep gives NULL when there are no tokens found */ + do + { + /* when no tokens are found, start will be NULL */ + token = strsep(&start, ","); + + switch( get_token(token, param_value) ) { + case baud: + uart->baud = simple_strtoul(param_value, NULL, 0); + break; + case bridge_bdf: + if ( !parse_pci(param_value, NULL, &uart->ps_bdf[0], + &uart->ps_bdf[1], &uart->ps_bdf[2])) + PARSE_ERR_RET("Bad port PCI coordinates\n"); + uart->ps_bdf_enable = 1; + break; + case clock_hz: + uart->clock_hz = (simple_strtoul(param_value, NULL, 0) << 4); + break; + case device: + if ( strncmp(param_value, "pci", 3) == 0 ) + { + pci_uart_config(uart, 1/* skip AMT */, uart - ns16550_com); + dev_set = true; + } + else if ( strncmp(param_value, "amt", 3) == 0 ) + { + pci_uart_config(uart, 0, uart - ns16550_com); + dev_set = true; + } + break; + case io_base: + if ( dev_set == true ) + { + printk(XENLOG_WARNING + "Can't use io_base with dev=pci or dev=amt options\n"); + break; + } + uart->io_base = simple_strtoul(param_value, NULL, 0); + break; + case irq: + uart->irq = simple_strtoul(param_value, NULL, 0); + break; + case data_bits: + uart->data_bits = simple_strtoul(param_value, NULL, 0); + break; + case parity: + uart->parity = parse_parity_char(*param_value); + break; + case port_bdf: + if ( !parse_pci(param_value, NULL, &uart->pb_bdf[0], + &uart->pb_bdf[1], &uart->pb_bdf[2])) + PARSE_ERR_RET("Bad port PCI coordinates\n"); + uart->pb_bdf_enable = true; + break; + case stop_bits: + uart->stop_bits = simple_strtoul(param_value, NULL, 0); + break; + case reg_shift: + uart->reg_shift = simple_strtoul(param_value, NULL, 0); + break; + case reg_width: + uart->reg_width = simple_strtoul(param_value, NULL, 0); + break; + default: + PARSE_ERR_RET("Invalid parameter: %s\n", token); + break; + } + } while ( start != NULL ); + + return true; +} + +static void __init ns16550_parse_port_config( + struct ns16550 *uart, const char *conf) +{ + char cmdline[128]; + char *str; + + /* No user-specified configuration? */ + if ( (conf == NULL) || (*conf == '\0') ) + { + /* Some platforms may automatically probe the UART configuartion. */ + if ( uart->baud != 0 ) + goto config_parsed; + return; + } + + strlcpy(cmdline, conf, ARRAY_SIZE(cmdline)); + str = cmdline; + + /* parse positional parameters and get pointer for name-value pairs */ + if ( !parse_positional(uart, &str) ) + return; + + if ( !parse_namevalue_pairs(str, uart) ) + return; + config_parsed: /* Sanity checks. */ if ( (uart->baud != BAUD_AUTO) && @@ -1177,6 +1379,8 @@ static void __init ns16550_parse_port_config( PARSE_ERR("Baud rate %d outside supported range.", uart->baud); if ( (uart->data_bits < 5) || (uart->data_bits > 8) ) PARSE_ERR("%d data bits are unsupported.", uart->data_bits); + if ( (uart->reg_width != 1) && (uart->reg_width != 4) ) + PARSE_ERR("Accepted values of reg_width are 1 and 4 only"); if ( (uart->stop_bits < 1) || (uart->stop_bits > 2) ) PARSE_ERR("%d stop bits are unsupported.", uart->stop_bits); if ( uart->io_base == 0 ) -- 2.7.4 _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx https://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |