On Sat, 2016-02-20 at 12:00 +0100, Mathieu OTHACEHE wrote:

Add support for :

- CP-102E: 2 ports RS232 PCIE card
- CP-102EL: 2 ports RS232 PCIE card
- CP-132EL: 2 ports RS422/485 PCIE card
- CP-114EL: 4 ports RS232/422/485 PCIE card
- CP-104EL-A: 4 ports RS232 PCIE card
- CP-168EL-A: 8 ports RS232 PCIE card
- CP-118EL-A: 8 ports RS232/422/485 PCIE card
- CP-118E-A: 8 ports RS422/485 PCIE card
- CP-138E-A: 8 ports RS422/485 PCIE card
- CP-134EL-A: 4 ports RS422/485 PCIE card
- CP-116E-A (A): 8 ports RS232/422/485 PCIE card
- CP-116E-A (B): 8 ports RS232/422/485 PCIE card

This patch is based on information extracted from
vendor mxupcie driver available on MOXA website.

I was able to test it on a CP-168EL-A on PC.

Looks much better!
Though few comments below.

+static int moxa8250_probe(struct pci_dev *pdev, const struct
pci_device_id *id)
+{
+	struct uart_8250_port uart;
+	struct moxa8250_board *brd;
+	void __iomem *ioaddr;
+	int nr_ports, ret, i, offset;

i looks like unsigned and maybe split per logical lines? I don't think
nr_ports correlates to ret somehow, same for the rest.

unsigned int i, nr_ports;
... offset;
int ret;

?

+
+	brd = &moxa8250_boards[id->driver_data];
+	nr_ports = brd->num_ports;
+
+	ret = pcim_enable_device(pdev);
+	if (ret)
+		return ret;
+
+	brd = devm_kzalloc(&pdev->dev, sizeof(struct moxa8250_board)
+
+			   sizeof(unsigned int) * nr_ports,
GFP_KERNEL);
+	if (!brd)
+		return -ENOMEM;
+
+	memset(&uart, 0, sizeof(struct uart_8250_port));
+
+	uart.port.dev = &pdev->dev;
+	uart.port.irq = pdev->irq;
+	uart.port.private_data = brd;
+	uart.port.uartclk = MOXA_BASE_BAUD * 16;
+	uart.port.flags = UPF_SKIP_TEST | UPF_BOOT_AUTOCONF |
UPF_SHARE_IRQ;
+
+	ioaddr = pcim_iomap(pdev, 1, 0);

And if we got NULL here?

1 is magic bar number, perhaps define it.

+
+	for (i = 0; i < nr_ports; i++) {
+
+		/*
+		 * MOXA Smartio MUE boards with 4 ports have
+		 * a different offset for port #3
+		 */
+		if (nr_ports == 4 && i == 3)
+			offset = 7 * MOXA_UART_OFFSET;
+		else
+			offset = i * MOXA_UART_OFFSET;
+
+		uart.port.iotype = UPIO_MEM;
+		uart.port.iobase = 0;
+		uart.port.mapbase = pci_resource_start(pdev, 1) +
offset;
+		uart.port.membase = ioaddr + offset;
+		uart.port.regshift = 0;
+
+		dev_dbg(&pdev->dev, "Setup PCI port: port %lx, irq
%d, type %d\n",
+			uart.port.iobase, uart.port.irq,
uart.port.iotype);
+
+		brd->line[i] = serial8250_register_8250_port(&uart);
+		if (brd->line[i] < 0) {
+			dev_err(&pdev->dev,
+				"Couldn't register serial port %lx,
irq %d, type %d, error %d\n",
+				uart.port.iobase, uart.port.irq,
+				uart.port.iotype, brd->line[i]);
+			break;
+		}
+	}
+
+	pci_set_drvdata(pdev, brd);
+	return 0;
+}
+
+static void moxa8250_remove(struct pci_dev *pdev)
+{
+	struct moxa8250_board *brd = pci_get_drvdata(pdev);
+	int i;

unsigned

+
+	for (i = 0; i < brd->num_ports; i++)
+		serial8250_unregister_port(brd->line[i]);
+}
+
+static const struct pci_device_id pci_ids[] = {
+	{PCI_DEVICE(PCI_VENDOR_ID_MOXA, PCI_DEVICE_ID_MOXA_CP102E),

...and PCI_VDEVICE will look even shorter!

+	 .driver_data = moxa8250_2p},

Maybe a macro which will also cast to kernel_ulong_t ?

Something like

#define MOXA_DEVICE(id,data) { PCI_VDEVICE(MOXA, id), .driver_data =
(kernel_ulong_t)(data) }

static const struct pci_device_id pci_ids[] = {
 
MOXA_DEVICE(PCI_DEVICE_ID_MOXA_CP102E, moxa8250_2p),
  ...
};

+	{PCI_DEVICE(PCI_VENDOR_ID_MOXA, PCI_DEVICE_ID_MOXA_CP102EL),
+	 .driver_data = moxa8250_2p},
+	{PCI_DEVICE(PCI_VENDOR_ID_MOXA,
PCI_DEVICE_ID_MOXA_CP104EL_A),
+	 .driver_data = moxa8250_4p},
+	{PCI_DEVICE(PCI_VENDOR_ID_MOXA, PCI_DEVICE_ID_MOXA_CP114EL),
+	 .driver_data = moxa8250_4p},
+	{PCI_DEVICE(PCI_VENDOR_ID_MOXA,
PCI_DEVICE_ID_MOXA_CP116E_A_A),
+	 .driver_data = moxa8250_8p},
+	{PCI_DEVICE(PCI_VENDOR_ID_MOXA,
PCI_DEVICE_ID_MOXA_CP116E_A_B),
+	 .driver_data = moxa8250_8p},
+	{PCI_DEVICE(PCI_VENDOR_ID_MOXA,
PCI_DEVICE_ID_MOXA_CP118EL_A),
+	 .driver_data = moxa8250_8p},
+	{PCI_DEVICE(PCI_VENDOR_ID_MOXA,
PCI_DEVICE_ID_MOXA_CP118E_A_I),
+	 .driver_data = moxa8250_8p},
+	{PCI_DEVICE(PCI_VENDOR_ID_MOXA, PCI_DEVICE_ID_MOXA_CP132EL),
+	 .driver_data = moxa8250_2p},
+	{PCI_DEVICE(PCI_VENDOR_ID_MOXA,
PCI_DEVICE_ID_MOXA_CP134EL_A),
+	 .driver_data = moxa8250_4p},
+	{PCI_DEVICE(PCI_VENDOR_ID_MOXA,
PCI_DEVICE_ID_MOXA_CP138E_A),
+	 .driver_data = moxa8250_8p},
+	{PCI_DEVICE(PCI_VENDOR_ID_MOXA,
PCI_DEVICE_ID_MOXA_CP168EL_A),
+	 .driver_data = moxa8250_8p},

I think your device provides a PCI serial class, thus you have to
blacklist them in 8250_pci.c.

+	{0}
+};

+};

Redundant empty line.

+MODULE_DEVICE_TABLE(pci, pci_ids);

-- 
Andy Shevchenko [off-list ref]
Intel Finland Oy