Re: [PATCH] ns16550: misc minor adjustments

[Ian Campbell <ian.campbell@citrix.com>]

On Thu, 2015-11-12 at 03:31 -0700, Jan Beulich wrote:
> First and foremost: fix documentation: The use of "clock_hz", when
> "base_baud" was meant, has taken me several hours (suspecting a more
> complicated problem with the PCIe card I've been trying to get
> working). At once correct the "gdb" option, which is more like
> "console", not like "com<N>".
> 
> Next, fix the types of ns_{read,write}_reg(): Especially the former
> having had a signed return type so far caused quite interesting effects
> when determining to baud rate if "auto" was specified. In that same
> code, also avoid dividing by zero when in fact the baud rate was not
> previously set up.
> 
> Further, accept I/O port based serial PCI cards with a port range wider
> than 8 bytes.
> 
> Finally, slightly rearrange struct ns16550 to reduce holes.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> --- a/docs/misc/xen-command-line.markdown
> +++ b/docs/misc/xen-command-line.markdown
> @@ -277,13 +277,13 @@ Flag to indicate whether to probe for a
>  ACPI indicating none to be there.
>  
>  ### com1,com2
> -> `= <baud>[/<clock_hz>][,[DPS][,[<io-base>|pci|amt][,[<irq>][,[<port-
> bdf>][,[<bridge-bdf>]]]]]]`
> +> `= <baud>[/<base_baud>][,[DPS][,[<io-base>|pci|amt][,[<irq>][,[<port-
> bdf>][,[<bridge-bdf>]]]]]]`
>  
>  Both option `com1` and `com2` follow the same format.
>  
>  * `<baud>` may be either an integer baud rate, or the string `auto` if
>    the bootloader or other earlier firmware has already set it up.
> -* Optionally, a clock speed measured in hz can be specified.
> +* Optionally, the base baud rate can be specified.

I do wonder if everyone trying to use this (which will include less-savy
users who are trying to produce a useful bug report by our request) will
know what this means?

On the other hand I'm not exactly sure how to better describe it other than
as the "basic baud rate, i.e. the one you get with divisor == 1", which is
no more likely to enlighten anyone who didn't already know that.

Is it true (enough) to say something like "typically the fastest baud rate
supported by the device"?

> @@ -530,7 +531,8 @@ static void ns16550_setup_preirq(struct
>          /* Baud rate already set: read it out from the divisor latch. */
>          divisor  = ns_read_reg(uart, UART_DLL);
>          divisor |= ns_read_reg(uart, UART_DLM) << 8;
> -        uart->baud = uart->clock_hz / (divisor << 4);
> +        if ( divisor )
> +            uart->baud = uart->clock_hz / (divisor << 4);

Will following code cope with uart->baud == BAUD_AUTO? Or should we pick a
static fallback rate (115200?) and set the divisor appropriately?

Ian.

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