|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [Xen-devel] [PATCH v7] 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>
Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
---
Changed since v6:
* Changed '1' to bool true in bridge_bdf case statement for uart->ps_bdf_enable
* Removed break for default case in switch block in parse_namevalue_pairs()
Changed since v5:
* Extended changelist for PATCH v4 and v5
* Removed const for name[12] in serial_param_var
* Changed "ext_value" pointer to "value" double pointer in get_token()
* Removed parenthesis in clock_hz case statement
Changed since v4:
* Changed max_serial_params to num_serial_params in enum e_serial_param_type
* Changed *sp_name in struct serial_param_var to a 12 byte buffer
* Removed sp_ prefix in sp_name in struct serial_param_var
* Removed if ( name_val_pos ) check before while() loop in parse_positional()
* Added blank lines for non-fall-through case blocks in switch statement
inside parse_namevalue_pairs
* Changed comparison of a bool with "true" for dev_set in io_base case.
* Changed cmdline to com_console_options in ns16550_parse_port_config()
* Changed io_base setting after pci device specification to give an error
message instead of PARSE_ERR_RET
* Removed config_parsed goto statements outside of error cleanup
* Removed the full-stop (.) in sanity checks for reg_width at the end of
ns16550_parse_port_config
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.
* Changed wrongly used bus:device:function to bus:device.function in
xen-command-line.markdown
* Using '-' instead of '_' for name-value pairs
eg. reg-width instead of reg_width, stop-bits instead of stop_bits
* Used __initconst for serial_param_var struct
* Changed get_token_value function name to get_token
* Values for name-value pairs in get_token are extracted only when a match
is found in the table for valid name-value pairs
* Removed intermediate return variable for get_token function and used it
directly in the switch statement for setting UART configs in
parse_namevalue_pairs
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 | 244 +++++++++++++++++++++++++++++++++---
3 files changed, 267 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..f393bb6 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);
+enum 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. */
+ num_serial_params
+};
+
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,30 @@ static struct ns16550 {
#endif
} ns16550_com[2] = { { 0 } };
+struct serial_param_var {
+ char name[12];
+ enum serial_param_type type;
+};
+
+/*
+ * Enum struct keeping a table of all accepted parameter names for parsing
+ * com_console_options 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 +1124,73 @@ 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.
+ */
+static enum __init serial_param_type get_token(char *token, char **value)
+{
+ const char *param_name;
+ unsigned int i;
+
+ param_name = strsep(&token, "=");
+ if ( param_name == NULL )
+ return num_serial_params;
+
+ /* Linear search for the parameter. */
+ for ( i = 0; i < ARRAY_SIZE(sp_vars); i++ )
+ {
+ if ( strcmp(sp_vars[i].name, param_name) == 0 )
+ {
+ *value = token;
+ return sp_vars[i].type;
+ }
+ }
+
+ return num_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. */
+ while ( name_val_pos > *str )
{
- /* Some platforms may automatically probe the UART configuartion. */
- if ( uart->baud != 0 )
- goto config_parsed;
- return;
+ /* Working backwards from the '=' sign. */
+ name_val_pos--;
+ 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 +1220,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 +1245,141 @@ 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 = NULL;
+ bool dev_set = false;
+
+ if ( (str == NULL) || (*str == '\0') )
+ return true;
+
+ do
+ {
+ /* When no tokens are found, start will be NULL */
+ token = strsep(&start, ",");
+
+ switch ( get_token(token, ¶m_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 = true;
+ 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 )
+ {
+ 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);
+
+ }
+ } while ( start != NULL );
+
+ return true;
+}
+
+static void __init ns16550_parse_port_config(
+ struct ns16550 *uart, const char *conf)
+{
+ char com_console_options[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(com_console_options, conf, ARRAY_SIZE(com_console_options));
+ str = com_console_options;
+
+ /* 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 +1387,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 |