[PATCH v3 1/2] ns16550: specify uart param for ns_{read,write}_reg as const

[PATCH v3 1/2] ns16550: specify uart param for ns_{read,write}_reg as const

[Marek Marczykowski-Górecki]

They don't modify it, after all.

Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
---
New in v3. There was "ns16550: do not override fifo size if explicitly
set" here before, but it's already committed.
---
 xen/drivers/char/ns16550.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c
index 97b85b0225cc..20da8fd3b421 100644
--- a/xen/drivers/char/ns16550.c
+++ b/xen/drivers/char/ns16550.c
@@ -108,7 +108,7 @@ struct ns16550_config_param {
 
 static void ns16550_delayed_resume(void *data);
 
-static u8 ns_read_reg(struct ns16550 *uart, unsigned int reg)
+static u8 ns_read_reg(const struct ns16550 *uart, unsigned int reg)
 {
     void __iomem *addr = uart->remapped_io_base + (reg << uart->reg_shift);
 #ifdef CONFIG_HAS_IOPORTS
@@ -126,7 +126,7 @@ static u8 ns_read_reg(struct ns16550 *uart, unsigned int reg)
     }
 }
 
-static void ns_write_reg(struct ns16550 *uart, unsigned int reg, u8 c)
+static void ns_write_reg(const struct ns16550 *uart, unsigned int reg, u8 c)
 {
     void __iomem *addr = uart->remapped_io_base + (reg << uart->reg_shift);
 #ifdef CONFIG_HAS_IOPORTS
-- 
2.31.1

[PATCH v3 2/2] ns16550: add Exar PCIe UART cards support

[Marek Marczykowski-Górecki]

Besides standard UART setup, this device needs enabling
(vendor-specific) "Enhanced Control Bits" - otherwise disabling hardware
control flow (MCR[2]) is ignored. Add appropriate quirk to the
ns16550_setup_preirq(), similar to the handle_dw_usr_busy_quirk(). The
new function act on Exar 2-, 4-, and 8- port cards only. I have tested
the functionality on 2-port card but based on the Linux driver, the same
applies to other models too.

Additionally, Exar card supports fractional divisor (DLD[3:0] register,
at 0x02). This part is not supported here yet, and seems to not
be required for working 115200bps at the very least.

The specification for the 2-port card is available at:
https://www.maxlinear.com/product/interface/uarts/pcie-uarts/xr17v352

Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
---
Changes in v2:
 - use read-modify-write for enabling ECB
 - handle also 4- and 8-ports cards (but not everything from Exar)
 - formatting fixes
Changes in v3:
 - guard the quirk based on a PCI device ID only (via checking
   uart->param)
 - use const param
 - code style fixes

I've opted to move the #ifdef into ns16550_setup_preirq() itself and
move the whole enable_exar_enhanced_bits() into appropriate #ifdef too
(need to move it anyway, to use uart_param). But if you prefer the older
structure (#ifdef inside ns16550_setup_preirq()), I can do that too.
---
 xen/drivers/char/ns16550.c  | 83 ++++++++++++++++++++++++++++++++++++-
 xen/include/xen/8250-uart.h |  5 +++
 xen/include/xen/pci_ids.h   |  2 +
 3 files changed, 88 insertions(+), 2 deletions(-)

diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c
index 20da8fd3b421..b777c8711ee0 100644
--- a/xen/drivers/char/ns16550.c
+++ b/xen/drivers/char/ns16550.c
@@ -88,6 +88,9 @@ struct ns16550_config {
         param_pericom_2port,
         param_pericom_4port,
         param_pericom_8port,
+        param_exar_xr17v352,
+        param_exar_xr17v354,
+        param_exar_xr17v358,
     } param;
 };
 
@@ -104,6 +107,8 @@ struct ns16550_config_param {
     unsigned int uart_offset;
     unsigned int first_offset;
 };
+
+static void enable_exar_enhanced_bits(const struct ns16550 *uart);
 #endif
 
 static void ns16550_delayed_resume(void *data);
@@ -303,6 +308,11 @@ static void ns16550_setup_preirq(struct ns16550 *uart)
     /* Handle the DesignWare 8250 'busy-detect' quirk. */
     handle_dw_usr_busy_quirk(uart);
 
+#ifdef CONFIG_HAS_PCI
+    /* Enable Exar "Enhanced function bits" */
+    enable_exar_enhanced_bits(uart);
+#endif
+
     /* Line control and baud-rate generator. */
     ns_write_reg(uart, UART_LCR, lcr | UART_LCR_DLAB);
     if ( uart->baud != BAUD_AUTO )
@@ -781,7 +791,37 @@ static const struct ns16550_config_param __initconst uart_param[] = {
         .lsr_mask = UART_LSR_THRE,
         .bar0 = 1,
         .max_ports = 8,
-    }
+    },
+    [param_exar_xr17v352] = {
+        .base_baud = 7812500,
+        .uart_offset = 0x400,
+        .reg_width = 1,
+        .fifo_size = 256,
+        .lsr_mask = UART_LSR_THRE,
+        .bar0 = 1,
+        .mmio = 1,
+        .max_ports = 2,
+    },
+    [param_exar_xr17v354] = {
+        .base_baud = 7812500,
+        .uart_offset = 0x400,
+        .reg_width = 1,
+        .fifo_size = 256,
+        .lsr_mask = UART_LSR_THRE,
+        .bar0 = 1,
+        .mmio = 1,
+        .max_ports = 4,
+    },
+    [param_exar_xr17v358] = {
+        .base_baud = 7812500,
+        .uart_offset = 0x400,
+        .reg_width = 1,
+        .fifo_size = 256,
+        .lsr_mask = UART_LSR_THRE,
+        .bar0 = 1,
+        .mmio = 1,
+        .max_ports = 8,
+    },
 };
 
 static const struct ns16550_config __initconst uart_config[] =
@@ -1007,7 +1047,25 @@ static const struct ns16550_config __initconst uart_config[] =
         .vendor_id = PCI_VENDOR_ID_PERICOM,
         .dev_id = 0x7958,
         .param = param_pericom_8port
-    }
+    },
+    /* Exar Corp. XR17V352 Dual PCIe UART */
+    {
+        .vendor_id = PCI_VENDOR_ID_EXAR,
+        .dev_id = 0x0352,
+        .param = param_exar_xr17v352
+    },
+    /* Exar Corp. XR17V354 Quad PCIe UART */
+    {
+        .vendor_id = PCI_VENDOR_ID_EXAR,
+        .dev_id = 0x0354,
+        .param = param_exar_xr17v354
+    },
+    /* Exar Corp. XR17V358 Octal PCIe UART */
+    {
+        .vendor_id = PCI_VENDOR_ID_EXAR,
+        .dev_id = 0x0358,
+        .param = param_exar_xr17v358
+    },
 };
 
 static int __init
@@ -1177,6 +1235,27 @@ pci_uart_config(struct ns16550 *uart, bool_t skip_amt, unsigned int idx)
     return 0;
 }
 
+static void enable_exar_enhanced_bits(const struct ns16550 *uart)
+{
+    uint8_t efr;
+
+    switch ( uart->param - uart_param )
+    {
+    case param_exar_xr17v352:
+    case param_exar_xr17v354:
+    case param_exar_xr17v358:
+        /*
+         * Exar XR17V35x cards ignore setting MCR[2] (hardware flow control)
+         * unless "Enhanced control bits" is enabled.
+         * The below checks for a 2, 4 or 8 port UART, following Linux driver.
+         */
+        efr = ns_read_reg(uart, UART_XR_EFR);
+        efr |= UART_EFR_ECB;
+        ns_write_reg(uart, UART_XR_EFR, efr);
+        break;
+    }
+}
+
 #endif /* CONFIG_HAS_PCI */
 
 /*
diff --git a/xen/include/xen/8250-uart.h b/xen/include/xen/8250-uart.h
index 5c3bac33221e..74e9d552a718 100644
--- a/xen/include/xen/8250-uart.h
+++ b/xen/include/xen/8250-uart.h
@@ -35,6 +35,8 @@
 #define UART_USR          0x1f    /* Status register (DW) */
 #define UART_DLL          0x00    /* divisor latch (ls) (DLAB=1) */
 #define UART_DLM          0x01    /* divisor latch (ms) (DLAB=1) */
+#define UART_XR_EFR       0x09    /* Enhanced function register (Exar) */
+#define UART_XR_DVID      0x8d    /* Device identification */
 
 /* Interrupt Enable Register */
 #define UART_IER_ERDAI    0x01    /* rx data recv'd       */
@@ -121,6 +123,9 @@
 /* Frequency of external clock source. This definition assumes PC platform. */
 #define UART_CLOCK_HZ     1843200
 
+/* Bits in Exar specific UART_XR_EFR register */
+#define UART_EFR_ECB      0x10
+
 /* Resume retry settings */
 #define RESUME_DELAY      MILLISECS(10)
 #define RESUME_RETRIES    100
diff --git a/xen/include/xen/pci_ids.h b/xen/include/xen/pci_ids.h
index 7788ba9d2f34..e798477a7e23 100644
--- a/xen/include/xen/pci_ids.h
+++ b/xen/include/xen/pci_ids.h
@@ -4,6 +4,8 @@
 
 #define PCI_VENDOR_ID_PERICOM            0x12d8
 
+#define PCI_VENDOR_ID_EXAR               0x13a8
+
 #define PCI_VENDOR_ID_OXSEMI             0x1415
 
 #define PCI_VENDOR_ID_BROADCOM           0x14e4
-- 
2.31.1

Re: [PATCH v3 1/2] ns16550: specify uart param for ns_{read,write}_reg as const

[Jan Beulich]

On 19.08.2021 21:06, Marek Marczykowski-Górecki wrote:
> They don't modify it, after all.
> 
> Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>

Reviewed-by: Jan Beulich <jbeulich@suse.com>

Thanks for taking this extra step.

Jan

Re: [PATCH v3 2/2] ns16550: add Exar PCIe UART cards support

[Jan Beulich]

On 19.08.2021 21:06, Marek Marczykowski-Górecki wrote:
> Besides standard UART setup, this device needs enabling
> (vendor-specific) "Enhanced Control Bits" - otherwise disabling hardware
> control flow (MCR[2]) is ignored. Add appropriate quirk to the
> ns16550_setup_preirq(), similar to the handle_dw_usr_busy_quirk(). The
> new function act on Exar 2-, 4-, and 8- port cards only. I have tested
> the functionality on 2-port card but based on the Linux driver, the same
> applies to other models too.
> 
> Additionally, Exar card supports fractional divisor (DLD[3:0] register,
> at 0x02). This part is not supported here yet, and seems to not
> be required for working 115200bps at the very least.
> 
> The specification for the 2-port card is available at:
> https://www.maxlinear.com/product/interface/uarts/pcie-uarts/xr17v352
> 
> Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>

Reviewed-by: Jan Beulich <jbeulich@suse.com>
with one line dropped (see bottom of mail).

> I've opted to move the #ifdef into ns16550_setup_preirq() itself and
> move the whole enable_exar_enhanced_bits() into appropriate #ifdef too
> (need to move it anyway, to use uart_param). But if you prefer the older
> structure (#ifdef inside ns16550_setup_preirq()), I can do that too.

That's okay for now, I think. When we gain more quirks we may want to
re-arrange this some. The only thing I don't like (but which is
unavoidable afaict) is ...

> @@ -104,6 +107,8 @@ struct ns16550_config_param {
>      unsigned int uart_offset;
>      unsigned int first_offset;
>  };
> +
> +static void enable_exar_enhanced_bits(const struct ns16550 *uart);
>  #endif

... this added forward declaration.

> --- a/xen/include/xen/8250-uart.h
> +++ b/xen/include/xen/8250-uart.h
> @@ -35,6 +35,8 @@
>  #define UART_USR          0x1f    /* Status register (DW) */
>  #define UART_DLL          0x00    /* divisor latch (ls) (DLAB=1) */
>  #define UART_DLM          0x01    /* divisor latch (ms) (DLAB=1) */
> +#define UART_XR_EFR       0x09    /* Enhanced function register (Exar) */
> +#define UART_XR_DVID      0x8d    /* Device identification */

The latter line can now be dropped again, as the symbol has no use
anymore. I'll try to remember to take care of this while committing.

Jan