[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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



On 01/06/2017 08:58 AM, Jan Beulich wrote:

On 05.01.17 at 23:39, <swapnil.paratey@xxxxxxx> wrote:
@@ -299,6 +299,15 @@ Both option `com1` and `com2` follow the same format.
    the bootloader or other earlier firmware has already set it up.
  * Optionally, the base baud rate (usually the highest baud rate the
    device can communicate at) can be specified.
+* `<reg-width>` is the access size, or width, for programming
+  the UART device registers.  Accepted values are 1 and 4 (bytes).
+  The UART device datasheet defines the register width to be used when
+  reading or writing the registers. This field is optional.
+  The default value is 1.
+* `<reg-shift>` is the number of bits to shift the register offset value
+  for programming the UART device registers. The UART device datasheet
+  defines the register shift needed to access the registers properly.
+  This field is optional. The default value is 0.
I was about to point out that the two defaults listed here aren't
really correct for all cases, but the situation is worse:

@@ -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);
+    }
Putting the new override settings here is not only undesirable
because the / separator so far really separates two similar
things (while you now make it also separate dissimilar ones),
but it also means one won't be able to override anything
coming out of pci_uart_config(). Therefore I think you need
to move this further down (and use , as the separator), in
turn requiring an adjustment to the doc change.

Jan

Just to understand this correctly, is this the expected syntax?
<baud>[/<base-baud>][,[DPS][,[<io-base>|pci|amt][,[<irq>][,[<port-bdf>]
[,[<bridge-bdf>][,[<reg_width>][,[<reg_shift>]]]]]]]]

Can I add these override settings (reg_width and reg_shift) just before
the sanity checks?

Should I have sanity checks for reg_width and reg_shift?
If so, should reg_width just check for values 1 and 4?
And should reg_shift have a max shift value as a sanity check?

Swapnil


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

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.