[PATCH v3] Enable serial output for Oxford Semiconductor PCIe cards and fix bugs.

[PATCH v3] Enable serial output for Oxford Semiconductor PCIe cards and fix bugs.

[Konrad Rzeszutek Wilk]

Since v1 [http://mid.gmane.org/1394040334-16278-1-git-send-email-konrad.wilk@oracle.com]:
  - Update it per Jan and Andrew comments
  - Tackle on two bug-fixes.
v2: [http://mid.gmane.org/1394218904-21535-1-git-send-email-konrad.wilk@oracle.com]:
 - Squash two patches in,
 - Add appropiate tags,
 - Rework bug-fixes per Jan's recommendations.

Hey,

This is the third posting of the changes to the ns16550 driver
to also support other chipsets - like the Oxford 16C950 one - popular on
the inexpensive Rosewill RC-300E.

There are also two bug-fixes that I discovered while testing my code.
One of them was found through code inspection while the other was
when I was testing my code. Aravind graciously had tested the patches -
so I affixed Tested-by on the patches that did not have much (or any)
code churn since v2 posting.

Of interest is:

[PATCH 3/6] serial: Support OXPCIe952 aka Oxford Semiconductor Ltd

  that enables the functionality on the PCIe card I have - while the rest
of the patches flesh out the code to support more of PCI and PCIe type cards.

 xen/arch/x86/oprofile/op_model_athlon.c |    4 +-
 xen/arch/x86/x86_64/mmconf-fam10h.c     |    1 +
 xen/arch/x86/x86_64/mmconfig-shared.c   |    1 +
 xen/arch/x86/x86_64/mmconfig.h          |    4 -
 xen/drivers/char/ns16550.c              |  255 ++++++++++++++++++++++++++++---
 xen/include/xen/pci_ids.h               |    9 +
 6 files changed, 249 insertions(+), 25 deletions(-)

Konrad Rzeszutek Wilk (6):
      serial: Skip over PCIe device which have no quirks (fix AMT regression).
      serial: Fix COM1 assumption if pci_uart_config did not find the AMT card.
      serial: Support OXPCIe952 aka Oxford Semiconductor Ltd Device c138 (1415:c138)
      serial: Seperate the PCI device ids and parameters (v1)
      pci: Use #defines for PCI vendors.
      serial: Expand the PCI serial quirks for OXPCIe200 and OXPCIe952 1 Native UART

[PATCH 1/6] serial: Skip over PCIe device which have no quirks (fix AMT regression).

[Konrad Rzeszutek Wilk]

The "ns16550: Add support for UART present in Broadcom TruManage
capable NetXtreme chips" implies that only devices that are have
an MMIO BAR and are in the quirks table should be processed.

Even the comment at the end says so:

 If we have an io_base, then we succeeded in the lookup

But the code was checking for the !io_base - which is to say if
the io_base was 0 then we would skip scanning. But io_base
always has a value - it is set by 'ns16550_init' to a default
value - so it would never hit the 'continue' path.

This means that if we have an communication device followed by
a serial AMT device we would pick the communication device instead
of the AMT device.

See:
00:16.0 Communication controller: Intel Corporation Cougar Point HECI Controller #1 (rev 04)
        Subsystem: Intel Corporation Device 2008
        Flags: bus master, fast devsel, latency 0, IRQ 11
        Memory at fb12a000 (64-bit, non-prefetchable) [size=16]
00:16.3 Serial controller: Intel Corporation Cougar Point KT Controller (rev 04) (prog-if 02 [16550])
        Subsystem: Intel Corporation Device 2008
        Flags: bus master, 66MHz, fast devsel, latency 0, IRQ 17
        I/O ports at f0e0 [size=8]
        Memory at fb129000 (32-bit, non-prefetchable) [size=4K]

pci 0000:00:16.0: [8086:1c3a] type 00 class 0x078000
pci 0000:00:16.3: [8086:1c3d] type 00 class 0x070002

And Xen picks 00:16.0 as its console when using 'com1=115200,8n1,amt'.

This patch fixes it and allows us to use AMT again by zeroing
out io_base to zero. If the scan did not work, the io_base is
set back to a default value (the 'pci_uart_config' does that
already at its end).

Tested-by: Aravind Gopalakrishnan <Aravind.Gopalakrishnan@amd.com>
CC: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
CC: Thomas Lendacky <Thomas.Lendacky@amd.com>
CC: Jan Beulich <jbeulich@suse.com>
CC: Keir Fraser <keir@xen.org>
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
---
 xen/drivers/char/ns16550.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c
index 53e49a1..2fded08 100644
--- a/xen/drivers/char/ns16550.c
+++ b/xen/drivers/char/ns16550.c
@@ -616,6 +616,7 @@ pci_uart_config (struct ns16550 *uart, int skip_amt, int bar_idx)
     unsigned int b, d, f, nextf, i;
     u16 vendor, device;
 
+    uart->io_base = 0;
     /* NB. Start at bus 1 to avoid AMT: a plug-in card cannot be on bus 0. */
     for ( b = skip_amt ? 1 : 0; b < 0x100; b++ )
     {
-- 
1.7.7.6

[PATCH 2/6] serial: Fix COM1 assumption if pci_uart_config did not find the AMT card.

[Konrad Rzeszutek Wilk]

The io_base by default is set to be 0x3f8 for COM1 and 0x2f8 for COM2
in __setup_xen. Then we call 'ns16550_init' which copies those in
the appropriate uart, which then calls 'ns16550_parse_port_config'
to deal with parameter parsing. If the 'amt' parameter has been
specified we further call 'pci_uart_config code' which scans the PCI bus.

If it does not find the AMT device it would overwrite the io_base with
0x3f8 regardless whether this is COM1 or COM2 - but only if 'amt'
parameter had been specified.

The overwrite is a way to set it back to the failsafe defaults -
except for COM2 it is bogus.

Note again - if an AMT card is found, this over-write will not happen.

This in theory (as I don't have a machine with two COM ports
readily available) means that if the user specified 'com2=9600,8n1,amt'
and the device did not have an AMT serial device, instead of using
0x2f8 for the io_base it ends up using 0x3f8 - and we don't get the
output on COM2. If the user had done 'com2=9600,8n1' we would never
get in this path so this bug would never manifest itself
(because we don't end up scanning for the AMT device).

We also unconditionally reset the IRQ value - so we would never get the
proper interrupt when falling back to the legacy 0x3f8 and 0x2f8 COM ports.
That is OK - as we would end up using the polling mode - while
not the best - it still would work.

Lastly the clock_hz is also set to the default one (UART_CLOCK_HZ,
which is the same for legacy COM1 and COM2 ports)- that is strictly
not a bug, but it is redundant and not needed.

This bug was introduced with the original AMT support and I cannot
recall why it was done that way - it is a bug.

Fix it by saving the original io_base before starting the
scan of the PCI bus. If we don't find an serial PCI device (because
we did not exit out of the loop using return) then
assign the original io_base value back.

Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
[v1: Also remove the irq override spotted by Jan]
[v2: Add more details to the commit description]
---
 xen/drivers/char/ns16550.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c
index 2fded08..d5fe689 100644
--- a/xen/drivers/char/ns16550.c
+++ b/xen/drivers/char/ns16550.c
@@ -612,10 +612,11 @@ static int __init
 pci_uart_config (struct ns16550 *uart, int skip_amt, int bar_idx)
 {
     uint32_t bar, bar_64 = 0, len, len_64;
-    u64 size, mask;
+    u64 size, mask, orig_base;
     unsigned int b, d, f, nextf, i;
     u16 vendor, device;
 
+    orig_base = uart->io_base;
     uart->io_base = 0;
     /* NB. Start at bus 1 to avoid AMT: a plug-in card cannot be on bus 0. */
     for ( b = skip_amt ? 1 : 0; b < 0x100; b++ )
@@ -747,9 +748,8 @@ pci_uart_config (struct ns16550 *uart, int skip_amt, int bar_idx)
     if ( !skip_amt )
         return -1;
 
-    uart->io_base = 0x3f8;
-    uart->irq = 0;
-    uart->clock_hz  = UART_CLOCK_HZ;
+    /* No AMT found, fallback to the defaults. */
+    uart->io_base = orig_base;
 
     return 0;
 }
-- 
1.7.7.6

[PATCH 3/6] serial: Support OXPCIe952 aka Oxford Semiconductor Ltd Device c138 (1415:c138)

[Konrad Rzeszutek Wilk]

Because they are PCIe and machine nowadys have those instead of
PCI, and they are inexpensive.

Tested with 1415:c138. Should also work on 0xc11f, 0xc11b models
of that chip.

Also on  OXPCIe200 1 Native UART 1415: 0xc40b, 0xc40f, 0xc41b,
0xc41f, 0xc42b, 0xc42f, 0xc43b, 0xc43f, 0xc44b, 0xc44f, 0xc45b
0xc45f, 0xc46b, 0xc46f, 0xc47b, 0xc47f, 0xc48b, 0xc48f, 0xc49b
0xc49f, 0xc4ab, 0xc4af, 0xc4bb, 0xc4bf, 0xc4cb, 0xc4cf

but since I don't have any of those cards this patch does not
enable it.

Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
[v1: Init for ARM and add offset to virt addr]
[v2: Remove the offset usage]
Tested-by: Aravind Gopalakrishnan <aravind.gopalakrishnan@amd.com>
---
 xen/drivers/char/ns16550.c |   20 ++++++++++++++++++++
 1 files changed, 20 insertions(+), 0 deletions(-)

diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c
index d5fe689..72da46d 100644
--- a/xen/drivers/char/ns16550.c
+++ b/xen/drivers/char/ns16550.c
@@ -89,6 +89,9 @@ struct ns16550_config_mmio {
     unsigned int fifo_size;
     u8 lsr_mask;
     unsigned int max_bars;
+    unsigned int base_baud;
+    unsigned int uart_offset;
+    unsigned int first_offset;
 };
 
 
@@ -111,6 +114,19 @@ static struct ns16550_config_mmio __initdata uart_config[] =
         .lsr_mask = (UART_LSR_THRE | UART_LSR_TEMT),
         .max_bars = 1,
     },
+    /* OXPCIe952 1 Native UART  */
+    {
+        .vendor_id = 0x1415,
+        .dev_id = 0xc138,
+        .base_baud = 4000000,
+        .uart_offset = 0x200,
+        .first_offset = 0x1000,
+        .reg_width = 1,
+        .reg_shift = 0,
+        .fifo_size = 16,
+        .lsr_mask = UART_LSR_THRE,
+        .max_bars = 1, /* It can do more, but we would need more custom code.*/
+    }
 };
 #endif
 
@@ -703,6 +719,10 @@ pci_uart_config (struct ns16550 *uart, int skip_amt, int bar_idx)
                         uart->lsr_mask = uart_config[i].lsr_mask;
                         uart->io_base = ((u64)bar_64 << 32) |
                                         (bar & PCI_BASE_ADDRESS_MEM_MASK);
+                        uart->io_base += uart_config[i].first_offset;
+                        uart->io_base += bar_idx * uart_config[i].uart_offset;
+                        if ( uart_config[i].base_baud )
+                            uart->clock_hz = uart_config[i].base_baud * 16;
                         /* Set device and MMIO region read only to Dom0 */
                         uart->enable_ro = 1;
                         break;
-- 
1.7.7.6

[PATCH 4/6] serial: Seperate the PCI device ids and parameters (v1)

[Konrad Rzeszutek Wilk]

This will allow us to re-use the parameters for multiple PCI
devices.

No functional change.

Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
[v1: s/nr/idx/ of the enum, use __initconst and const by Jan's review]
---
 xen/drivers/char/ns16550.c |   67 +++++++++++++++++++++++++++++---------------
 1 files changed, 44 insertions(+), 23 deletions(-)

diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c
index 72da46d..66d10f7 100644
--- a/xen/drivers/char/ns16550.c
+++ b/xen/drivers/char/ns16550.c
@@ -80,10 +80,14 @@ static struct ns16550 {
 #endif
 } ns16550_com[2] = { { 0 } };
 
-/* Defining uart config options for MMIO devices */
 struct ns16550_config_mmio {
     u16 vendor_id;
     u16 dev_id;
+    unsigned int param;
+};
+
+/* Defining uart config options for MMIO devices */
+struct ns16550_config_param {
     unsigned int reg_shift;
     unsigned int reg_width;
     unsigned int fifo_size;
@@ -96,28 +100,27 @@ struct ns16550_config_mmio {
 
 
 #ifdef HAS_PCI
+enum {
+    param_default = 0,
+    param_trumanage,
+    param_oxford,
+};
 /*
  * Create lookup tables for specific MMIO devices..
  * It is assumed that if the device found is MMIO,
  * then you have indexed it here. Else, the driver
  * does nothing.
  */
-static struct ns16550_config_mmio __initdata uart_config[] =
-{
-    /* Broadcom TruManage device */
-    {
-        .vendor_id = 0x14e4,
-        .dev_id = 0x160a,
+static const struct ns16550_config_param __initconst uart_param[] = {
+    [param_default] = { }, /* Ignored. */
+    [param_trumanage] = {
         .reg_shift = 2,
         .reg_width = 1,
         .fifo_size = 16,
         .lsr_mask = (UART_LSR_THRE | UART_LSR_TEMT),
         .max_bars = 1,
     },
-    /* OXPCIe952 1 Native UART  */
-    {
-        .vendor_id = 0x1415,
-        .dev_id = 0xc138,
+    [param_oxford] = {
         .base_baud = 4000000,
         .uart_offset = 0x200,
         .first_offset = 0x1000,
@@ -128,6 +131,21 @@ static struct ns16550_config_mmio __initdata uart_config[] =
         .max_bars = 1, /* It can do more, but we would need more custom code.*/
     }
 };
+static const struct ns16550_config_mmio __initconst uart_config[] =
+{
+    /* Broadcom TruManage device */
+    {
+        .vendor_id = 0x14e4,
+        .dev_id = 0x160a,
+        .param = param_trumanage,
+    },
+    /* OXPCIe952 1 Native UART  */
+    {
+        .vendor_id = 0x1415,
+        .dev_id = 0xc138,
+        .param = param_oxford,
+    }
+};
 #endif
 
 static void ns16550_delayed_resume(void *data);
@@ -692,37 +710,40 @@ pci_uart_config (struct ns16550 *uart, int skip_amt, int bar_idx)
 
                     size &= -size;
 
-                    /* Check for quirks in uart_config lookup table */
+                    /* Check for params in uart_config lookup table */
                     for ( i = 0; i < ARRAY_SIZE(uart_config); i++)
                     {
+                        unsigned int p;
+
                         if ( uart_config[i].vendor_id != vendor )
                             continue;
 
                         if ( uart_config[i].dev_id != device )
                             continue;
 
+                        p = uart_config[i].param;
                         /*
                          * Force length of mmio region to be at least
                          * 8 bytes times (1 << reg_shift)
                          */
-                        if ( size < (0x8 * (1 << uart_config[i].reg_shift)) )
+                        if ( size < (0x8 * (1 << uart_param[p].reg_shift)) )
                             continue;
 
-                        if ( bar_idx >= uart_config[i].max_bars )
+                        if ( bar_idx >= uart_param[p].max_bars )
                             continue;
 
-                        if ( uart_config[i].fifo_size )
-                            uart->fifo_size = uart_config[i].fifo_size;
+                        if ( uart_param[p].fifo_size )
+                            uart->fifo_size = uart_param[p].fifo_size;
 
-                        uart->reg_shift = uart_config[i].reg_shift;
-                        uart->reg_width = uart_config[i].reg_width;
-                        uart->lsr_mask = uart_config[i].lsr_mask;
+                        uart->reg_shift = uart_param[p].reg_shift;
+                        uart->reg_width = uart_param[p].reg_width;
+                        uart->lsr_mask = uart_param[p].lsr_mask;
                         uart->io_base = ((u64)bar_64 << 32) |
                                         (bar & PCI_BASE_ADDRESS_MEM_MASK);
-                        uart->io_base += uart_config[i].first_offset;
-                        uart->io_base += bar_idx * uart_config[i].uart_offset;
-                        if ( uart_config[i].base_baud )
-                            uart->clock_hz = uart_config[i].base_baud * 16;
+                        uart->io_base += uart_param[p].first_offset;
+                        uart->io_base += bar_idx * uart_param[p].uart_offset;
+                        if ( uart_param[p].base_baud )
+                            uart->clock_hz = uart_param[p].base_baud * 16;
                         /* Set device and MMIO region read only to Dom0 */
                         uart->enable_ro = 1;
                         break;
-- 
1.7.7.6

[PATCH 5/6] pci: Use #defines for PCI vendors.

[Konrad Rzeszutek Wilk]

Instead of having hard-coded values. We only do PCI vendors
as Jan requested and put all PCI device vendors in one
new file.

Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
 xen/arch/x86/oprofile/op_model_athlon.c |    4 ++--
 xen/arch/x86/x86_64/mmconf-fam10h.c     |    1 +
 xen/arch/x86/x86_64/mmconfig-shared.c   |    1 +
 xen/arch/x86/x86_64/mmconfig.h          |    4 ----
 xen/drivers/char/ns16550.c              |    5 +++--
 xen/include/xen/pci_ids.h               |    9 +++++++++
 6 files changed, 16 insertions(+), 8 deletions(-)
 create mode 100644 xen/include/xen/pci_ids.h

diff --git a/xen/arch/x86/oprofile/op_model_athlon.c b/xen/arch/x86/oprofile/op_model_athlon.c
index ad84768..204343d 100644
--- a/xen/arch/x86/oprofile/op_model_athlon.c
+++ b/xen/arch/x86/oprofile/op_model_athlon.c
@@ -20,7 +20,8 @@
 #include <asm/current.h>
 #include <asm/hvm/support.h>
 #include <xen/pci_regs.h>
- 
+#include <xen/pci_ids.h>
+
 #include "op_x86_model.h"
 #include "op_counter.h"
 
@@ -445,7 +446,6 @@ static inline void __init init_ibs_nmi_per_cpu(void *arg)
 	apic_write(reg, APIC_EILVT_MSG_NMI << 8);
 }
 
-#define PCI_VENDOR_ID_AMD               0x1022
 #define PCI_DEVICE_ID_AMD_10H_NB_MISC   0x1203
 #define IBSCTL                          0x1cc
 static int __init init_ibs_nmi(void)
diff --git a/xen/arch/x86/x86_64/mmconf-fam10h.c b/xen/arch/x86/x86_64/mmconf-fam10h.c
index 1373acb..65260f6 100644
--- a/xen/arch/x86/x86_64/mmconf-fam10h.c
+++ b/xen/arch/x86/x86_64/mmconf-fam10h.c
@@ -6,6 +6,7 @@
 #include <xen/acpi.h>
 #include <xen/pci.h>
 #include <xen/pci_regs.h>
+#include <xen/pci_ids.h>
 #include <xen/init.h>
 #include <xen/dmi.h>
 #include <asm/amd.h>
diff --git a/xen/arch/x86/x86_64/mmconfig-shared.c b/xen/arch/x86/x86_64/mmconfig-shared.c
index 7589b64..742bc18 100644
--- a/xen/arch/x86/x86_64/mmconfig-shared.c
+++ b/xen/arch/x86/x86_64/mmconfig-shared.c
@@ -19,6 +19,7 @@
 #include <xen/xmalloc.h>
 #include <xen/pci.h>
 #include <xen/pci_regs.h>
+#include <xen/pci_ids.h>
 #include <asm/e820.h>
 #include <asm/msr.h>
 #include <asm/msr-index.h>
diff --git a/xen/arch/x86/x86_64/mmconfig.h b/xen/arch/x86/x86_64/mmconfig.h
index 36e0448..c447e5a 100644
--- a/xen/arch/x86/x86_64/mmconfig.h
+++ b/xen/arch/x86/x86_64/mmconfig.h
@@ -17,7 +17,6 @@
  * Author: Allen Kay <allen.m.kay@intel.com> - adapted from linux
  */
 
-#define PCI_VENDOR_ID_INTEL        0x8086
 #define PCI_DEVICE_ID_INTEL_E7520_MCH    0x3590
 #define PCI_DEVICE_ID_INTEL_82945G_HB    0x2770
 
@@ -29,11 +28,8 @@
 #define PCI_PROBE_MASK        0x000f
 #define PCI_PROBE_NOEARLY    0x0010
 
-#define PCI_VENDOR_ID_AMD             0x1022
 #define PCI_CHECK_ENABLE_AMD_MMCONF     0x20000
 
-#define PCI_VENDOR_ID_NVIDIA       0x10de
-
 extern unsigned int pci_probe;
 
 /*
diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c
index 66d10f7..fd66238 100644
--- a/xen/drivers/char/ns16550.c
+++ b/xen/drivers/char/ns16550.c
@@ -19,6 +19,7 @@
 #ifdef HAS_PCI
 #include <xen/pci.h>
 #include <xen/pci_regs.h>
+#include <xen/pci_ids.h>
 #endif
 #include <xen/8250-uart.h>
 #include <xen/vmap.h>
@@ -135,13 +136,13 @@ static const struct ns16550_config_mmio __initconst uart_config[] =
 {
     /* Broadcom TruManage device */
     {
-        .vendor_id = 0x14e4,
+        .vendor_id = PCI_VENDOR_ID_BROADCOM,
         .dev_id = 0x160a,
         .param = param_trumanage,
     },
     /* OXPCIe952 1 Native UART  */
     {
-        .vendor_id = 0x1415,
+        .vendor_id = PCI_VENDOR_ID_OXSEMI,
         .dev_id = 0xc138,
         .param = param_oxford,
     }
diff --git a/xen/include/xen/pci_ids.h b/xen/include/xen/pci_ids.h
new file mode 100644
index 0000000..4e3aa54
--- /dev/null
+++ b/xen/include/xen/pci_ids.h
@@ -0,0 +1,9 @@
+#define PCI_VENDOR_ID_AMD                0x1022
+
+#define PCI_VENDOR_ID_BROADCOM           0x14e4
+
+#define PCI_VENDOR_ID_INTEL              0x8086
+
+#define PCI_VENDOR_ID_NVIDIA             0x10de
+
+#define PCI_VENDOR_ID_OXSEMI             0x1415
-- 
1.7.7.6

Re: [PATCH 5/6] pci: Use #defines for PCI vendors.

[Jan Beulich]

>>> On 10.03.14 at 20:35, Konrad Rzeszutek Wilk <konrad@kernel.org> wrote:
> --- /dev/null
> +++ b/xen/include/xen/pci_ids.h
> @@ -0,0 +1,9 @@
> +#define PCI_VENDOR_ID_AMD                0x1022
> +
> +#define PCI_VENDOR_ID_BROADCOM           0x14e4
> +
> +#define PCI_VENDOR_ID_INTEL              0x8086
> +
> +#define PCI_VENDOR_ID_NVIDIA             0x10de
> +
> +#define PCI_VENDOR_ID_OXSEMI             0x1415

If I'm going to be the one to commit this (and I don't forget), I'll
take the liberty of sorting these numerically, matching Linux and
making imo lookup easier, since especially considering company
acquires/mergers the name to look for may be ambiguous.

Jan

Re: [PATCH 5/6] pci: Use #defines for PCI vendors.

[Konrad Rzeszutek Wilk]

On Tue, Mar 11, 2014 at 08:35:38AM +0000, Jan Beulich wrote:
> >>> On 10.03.14 at 20:35, Konrad Rzeszutek Wilk <konrad@kernel.org> wrote:
> > --- /dev/null
> > +++ b/xen/include/xen/pci_ids.h
> > @@ -0,0 +1,9 @@
> > +#define PCI_VENDOR_ID_AMD                0x1022
> > +
> > +#define PCI_VENDOR_ID_BROADCOM           0x14e4
> > +
> > +#define PCI_VENDOR_ID_INTEL              0x8086
> > +
> > +#define PCI_VENDOR_ID_NVIDIA             0x10de
> > +
> > +#define PCI_VENDOR_ID_OXSEMI             0x1415
> 
> If I'm going to be the one to commit this (and I don't forget), I'll
> take the liberty of sorting these numerically, matching Linux and
> making imo lookup easier, since especially considering company
> acquires/mergers the name to look for may be ambiguous.

Of course. I think I am depending on you to merge them as I don't
have the commit access.

If you would like, I can pluck them in a git tree with this above
fixed and your Reviewed-by added and I can just send you a GIT PULL
request.

Thank you!
> 
> Jan
> 

Re: [PATCH 5/6] pci: Use #defines for PCI vendors.

[Jan Beulich]

>>> On 11.03.14 at 16:15, Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> wrote:
> On Tue, Mar 11, 2014 at 08:35:38AM +0000, Jan Beulich wrote:
>> >>> On 10.03.14 at 20:35, Konrad Rzeszutek Wilk <konrad@kernel.org> wrote:
>> > --- /dev/null
>> > +++ b/xen/include/xen/pci_ids.h
>> > @@ -0,0 +1,9 @@
>> > +#define PCI_VENDOR_ID_AMD                0x1022
>> > +
>> > +#define PCI_VENDOR_ID_BROADCOM           0x14e4
>> > +
>> > +#define PCI_VENDOR_ID_INTEL              0x8086
>> > +
>> > +#define PCI_VENDOR_ID_NVIDIA             0x10de
>> > +
>> > +#define PCI_VENDOR_ID_OXSEMI             0x1415
>> 
>> If I'm going to be the one to commit this (and I don't forget), I'll
>> take the liberty of sorting these numerically, matching Linux and
>> making imo lookup easier, since especially considering company
>> acquires/mergers the name to look for may be ambiguous.
> 
> Of course. I think I am depending on you to merge them as I don't
> have the commit access.
> 
> If you would like, I can pluck them in a git tree with this above
> fixed and your Reviewed-by added and I can just send you a GIT PULL
> request.

I don't think that would help much, not the least because we'll have
to wait for Keir's ack first anyway.

Jan

[PATCH 6/6] serial: Expand the PCI serial quirks for OXPCIe200 and OXPCIe952 1 Native UART

[Konrad Rzeszutek Wilk]

This covers all of the OXPCIe952 1 Native UART and
OXPCIe200 1 Native UART chipsets.

Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
 xen/drivers/char/ns16550.c |  174 ++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 174 insertions(+), 0 deletions(-)

diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c
index fd66238..429d786 100644
--- a/xen/drivers/char/ns16550.c
+++ b/xen/drivers/char/ns16550.c
@@ -143,8 +143,182 @@ static const struct ns16550_config_mmio __initconst uart_config[] =
     /* OXPCIe952 1 Native UART  */
     {
         .vendor_id = PCI_VENDOR_ID_OXSEMI,
+        .dev_id = 0xc11b,
+        .param = param_oxford,
+    },
+    /* OXPCIe952 1 Native UART  */
+    {
+        .vendor_id = PCI_VENDOR_ID_OXSEMI,
+        .dev_id = 0xc11f,
+        .param = param_oxford,
+    },
+    /* OXPCIe952 1 Native UART  */
+    {
+        .vendor_id = PCI_VENDOR_ID_OXSEMI,
         .dev_id = 0xc138,
         .param = param_oxford,
+    },
+    /* OXPCIe952 1 Native UART  */
+    {
+        .vendor_id = PCI_VENDOR_ID_OXSEMI,
+        .dev_id = 0xc13d,
+        .param = param_oxford,
+    },
+    /* OXPCIe952 1 Native UART  */
+    {
+        .vendor_id = PCI_VENDOR_ID_OXSEMI,
+        .dev_id = 0xc40b,
+        .param = param_oxford,
+    },
+    /* OXPCIe200 1 Native UART */
+    {
+        .vendor_id = PCI_VENDOR_ID_OXSEMI,
+        .dev_id = 0xc40f,
+        .param = param_oxford,
+    },
+    /* OXPCIe200 1 Native UART  */
+    {
+        .vendor_id = PCI_VENDOR_ID_OXSEMI,
+        .dev_id = 0xc41b,
+        .param = param_oxford,
+    },
+    /* OXPCIe200 1 Native UART  */
+    {
+        .vendor_id = PCI_VENDOR_ID_OXSEMI,
+        .dev_id = 0xc41f,
+        .param = param_oxford,
+    },
+    /* OXPCIe200 1 Native UART  */
+    {
+        .vendor_id = PCI_VENDOR_ID_OXSEMI,
+        .dev_id = 0xc42b,
+        .param = param_oxford,
+    },
+    /* OXPCIe200 1 Native UART  */
+    {
+        .vendor_id = PCI_VENDOR_ID_OXSEMI,
+        .dev_id = 0xc42f,
+        .param = param_oxford,
+    },
+    /* OXPCIe200 1 Native UART  */
+    {
+        .vendor_id = PCI_VENDOR_ID_OXSEMI,
+        .dev_id = 0xc43b,
+        .param = param_oxford,
+    },
+    /* OXPCIe200 1 Native UART  */
+    {
+        .vendor_id = PCI_VENDOR_ID_OXSEMI,
+        .dev_id = 0xc43f,
+        .param = param_oxford,
+    },
+    /* OXPCIe200 1 Native UART  */
+    {
+        .vendor_id = PCI_VENDOR_ID_OXSEMI,
+        .dev_id = 0xc44b,
+        .param = param_oxford,
+    },
+    /* OXPCIe200 1 Native UART  */
+    {
+        .vendor_id = PCI_VENDOR_ID_OXSEMI,
+        .dev_id = 0xc44f,
+        .param = param_oxford,
+    },
+    /* OXPCIe200 1 Native UART  */
+    {
+        .vendor_id = PCI_VENDOR_ID_OXSEMI,
+        .dev_id = 0xc45b,
+        .param = param_oxford,
+    },
+    /* OXPCIe200 1 Native UART  */
+    {
+        .vendor_id = PCI_VENDOR_ID_OXSEMI,
+        .dev_id = 0xc45f,
+        .param = param_oxford,
+    },
+    /* OXPCIe200 1 Native UART  */
+    {
+        .vendor_id = PCI_VENDOR_ID_OXSEMI,
+        .dev_id = 0xc46b,
+        .param = param_oxford,
+    },
+    /* OXPCIe200 1 Native UART  */
+    {
+        .vendor_id = PCI_VENDOR_ID_OXSEMI,
+        .dev_id = 0xc46f,
+        .param = param_oxford,
+    },
+    /* OXPCIe200 1 Native UART  */
+    {
+        .vendor_id = PCI_VENDOR_ID_OXSEMI,
+        .dev_id = 0xc47b,
+        .param = param_oxford,
+    },
+    /* OXPCIe200 1 Native UART  */
+    {
+        .vendor_id = PCI_VENDOR_ID_OXSEMI,
+        .dev_id = 0xc47f,
+        .param = param_oxford,
+    },
+    /* OXPCIe200 1 Native UART  */
+    {
+        .vendor_id = PCI_VENDOR_ID_OXSEMI,
+        .dev_id = 0xc48b,
+        .param = param_oxford,
+    },
+    /* OXPCIe200 1 Native UART  */
+    {
+        .vendor_id = PCI_VENDOR_ID_OXSEMI,
+        .dev_id = 0xc48f,
+        .param = param_oxford,
+    },
+    /* OXPCIe200 1 Native UART  */
+    {
+        .vendor_id = PCI_VENDOR_ID_OXSEMI,
+        .dev_id = 0xc49b,
+        .param = param_oxford,
+    },
+    /* OXPCIe200 1 Native UART  */
+    {
+        .vendor_id = PCI_VENDOR_ID_OXSEMI,
+        .dev_id = 0xc49f,
+        .param = param_oxford,
+    },
+    /* OXPCIe200 1 Native UART  */
+    {
+        .vendor_id = PCI_VENDOR_ID_OXSEMI,
+        .dev_id = 0xc4ab,
+        .param = param_oxford,
+    },
+    /* OXPCIe200 1 Native UART  */
+    {
+        .vendor_id = PCI_VENDOR_ID_OXSEMI,
+        .dev_id = 0xc4af,
+        .param = param_oxford,
+    },
+    /* OXPCIe200 1 Native UART  */
+    {
+        .vendor_id = PCI_VENDOR_ID_OXSEMI,
+        .dev_id = 0xc4bb,
+        .param = param_oxford,
+    },
+    /* OXPCIe200 1 Native UART  */
+    {
+        .vendor_id = PCI_VENDOR_ID_OXSEMI,
+        .dev_id = 0xc4bf,
+        .param = param_oxford,
+    },
+    /* OXPCIe200 1 Native UART  */
+    {
+        .vendor_id = PCI_VENDOR_ID_OXSEMI,
+        .dev_id = 0xc4cb,
+        .param = param_oxford,
+    },
+    /* OXPCIe200 1 Native UART  */
+    {
+        .vendor_id = PCI_VENDOR_ID_OXSEMI,
+        .dev_id = 0xc4cf,
+        .param = param_oxford,
     }
 };
 #endif
-- 
1.7.7.6

Re: [PATCH v3] Enable serial output for Oxford Semiconductor PCIe cards and fix bugs.

[Jan Beulich]

>>> On 10.03.14 at 20:35, Konrad Rzeszutek Wilk <konrad@kernel.org> wrote:
> This is the third posting of the changes to the ns16550 driver
> to also support other chipsets - like the Oxford 16C950 one - popular on
> the inexpensive Rosewill RC-300E.
> 
> There are also two bug-fixes that I discovered while testing my code.
> One of them was found through code inspection while the other was
> when I was testing my code. Aravind graciously had tested the patches -
> so I affixed Tested-by on the patches that did not have much (or any)
> code churn since v2 posting.
> 
> Of interest is:
> 
> [PATCH 3/6] serial: Support OXPCIe952 aka Oxford Semiconductor Ltd
> 
>   that enables the functionality on the PCIe card I have - while the rest
> of the patches flesh out the code to support more of PCI and PCIe type 
> cards.
> 
>  xen/arch/x86/oprofile/op_model_athlon.c |    4 +-
>  xen/arch/x86/x86_64/mmconf-fam10h.c     |    1 +
>  xen/arch/x86/x86_64/mmconfig-shared.c   |    1 +
>  xen/arch/x86/x86_64/mmconfig.h          |    4 -
>  xen/drivers/char/ns16550.c              |  255 ++++++++++++++++++++++++++++---
>  xen/include/xen/pci_ids.h               |    9 +
>  6 files changed, 249 insertions(+), 25 deletions(-)
> 
> Konrad Rzeszutek Wilk (6):
>       serial: Skip over PCIe device which have no quirks (fix AMT regression).
>       serial: Fix COM1 assumption if pci_uart_config did not find the AMT card.
>       serial: Support OXPCIe952 aka Oxford Semiconductor Ltd Device c138 (1415:c138)
>       serial: Seperate the PCI device ids and parameters (v1)
>       pci: Use #defines for PCI vendors.
>       serial: Expand the PCI serial quirks for OXPCIe200 and OXPCIe952 1 Native UART

Reviewed-by: Jan Beulich <jbeulich@suse.com>
(with a minor comment on patch 5 - see separate mail)