Re: [Xen-devel] [PATCH] 16550: Add command line parsing adjustments

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 05/01/17 19:47, Swapnil Paratey wrote:
> Add parsing options for reg_width and reg_shift in bootup command line
> parameters. This adds flexibility in setting register values
> for MMIO UART devices.
>
> Increase length of opt_com1 and opt_com2 buffer to accommodate more
> command line parameters.
>
> eg. com1=115200,8n1,0x3f8,4 (legacy IO)
> eg. com1=115200/3000000/4/2,8n1,0xfedc9000,4 (MMIO adjustments)
>
> Reviewed-by: Suravee Suthikulpanit <suravee.suthikulpanit@xxxxxxx>
> Signed-off-by: Swapnil Paratey <swapnil.paratey@xxxxxxx>
> ---
>  docs/misc/xen-command-line.markdown |  2 +-
>  xen/drivers/char/ns16550.c          | 20 +++++++++++++++++---
>  2 files changed, 18 insertions(+), 4 deletions(-)
>
> diff --git a/docs/misc/xen-command-line.markdown 
> b/docs/misc/xen-command-line.markdown
> index 0138978..3297646 100644
> --- a/docs/misc/xen-command-line.markdown
> +++ b/docs/misc/xen-command-line.markdown
> @@ -291,7 +291,7 @@ Flag to indicate whether to probe for a CMOS Real Time 
> Clock irrespective of
>  ACPI indicating none to be there.
>  
>  ### com1,com2
> -> `= 
> <baud>[/<base-baud>][,[DPS][,[<io-base>|pci|amt][,[<irq>][,[<port-bdf>][,[<bridge-bdf>]]]]]]`
> +> `= 
> <baud>[/[<base-baud>][/[<reg-width>][/[<reg-shift>]]]][,DPS[,<io-base>[,<irq>[,<port-bdf>[,<bridge-bdf>]]]]]`
>  
>  Both option `com1` and `com2` follow the same format.
>  

Please also add two bullet points to the list just out of context below
this.

> diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c
> index 1da103a..e076b29 100644
> --- a/xen/drivers/char/ns16550.c
> +++ b/xen/drivers/char/ns16550.c
> @@ -33,14 +33,14 @@
>  
>  /*
>   * Configure serial port with a string:
> - *   
> <baud>[/<base_baud>][,DPS[,<io-base>[,<irq>[,<port-bdf>[,<bridge-bdf>]]]]].
> + *   
> <baud>[/[<base-baud>][/[<reg-width>][/[<reg-shift>]]]][,DPS[,<io-base>[,<irq>[,<port-bdf>[,<bridge-bdf>]]]]].
>   * The tail of the string can be omitted if platform defaults are sufficient.
>   * If the baud rate is pre-configured, perhaps by a bootloader, then 'auto'
>   * 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[50] = "";
> +static char __initdata opt_com2[50] = "";

I'd up these to 64, to be a nice power of two.  Its initdata, so size
(on this scale) isn't a problem.

Otherwise, looks good.

~Andrew

>  string_param("com1", opt_com1);
>  string_param("com2", opt_com2);
>  
> @@ -1118,6 +1118,20 @@ static void __init ns16550_parse_port_config(
>          uart->clock_hz = simple_strtoul(conf, &conf, 0) << 4;
>      }
>  
> +    if ( *conf == '/' )
> +    {
> +        conf++;
> +        if ( *conf != '/' && *conf != ',' )
> +            uart->reg_width = simple_strtol(conf, &conf, 0);
> +    }
> +
> +    if ( *conf == '/' )
> +    {
> +        conf++;
> +        if ( *conf != '/' && *conf != ',' )
> +            uart->reg_shift = simple_strtol(conf, &conf, 0);
> +    }
> +
>      if ( *conf == ',' && *++conf != ',' )
>      {
>          uart->data_bits = simple_strtoul(conf, &conf, 10);


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel