[PATCH v13 1/2] serial: exar: split out the exar code from 8250_pci

[PATCH v13 1/2] serial: exar: split out the exar code from 8250_pci

[Sudip Mukherjee]

From: Sudip Mukherjee <sudip.mukherjee@codethink.co.uk>

Add the serial driver for the Exar chips. And also register the
platform device for the GPIO provided by the Exar chips.

Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: Sudip Mukherjee <sudip.mukherjee@codethink.co.uk>
---

Andy,
I have added the if (!board) check, but I am not sure how board can be
NULL here. If probe executes that will mean there was a match of the
device id and so in that case board can not be NULL.

 drivers/tty/serial/8250/8250_exar.c | 396 ++++++++++++++++++++++++++++++++++++
 drivers/tty/serial/8250/Kconfig     |   4 +
 drivers/tty/serial/8250/Makefile    |   1 +
 3 files changed, 401 insertions(+)
 create mode 100644 drivers/tty/serial/8250/8250_exar.c

diff --git a/drivers/tty/serial/8250/8250_exar.c b/drivers/tty/serial/8250/8250_exar.c
new file mode 100644
index 0000000..ba1f359
--- /dev/null
+++ b/drivers/tty/serial/8250/8250_exar.c
@@ -0,0 +1,396 @@
+/*
+ *  Probe module for 8250/16550-type Exar chips PCI serial ports.
+ *
+ *  Based on drivers/tty/serial/8250/8250_pci.c,
+ *
+ *  Copyright (C) 2017 Sudip Mukherjee, All Rights Reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License.
+ */
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/pci.h>
+#include <linux/serial_core.h>
+#include <linux/serial_reg.h>
+#include <linux/slab.h>
+#include <linux/string.h>
+#include <linux/tty.h>
+#include <linux/8250_pci.h>
+
+#include <asm/byteorder.h>
+
+#include "8250.h"
+
+#define PCI_DEVICE_ID_COMMTECH_4224PCIE	0x0020
+#define PCI_DEVICE_ID_COMMTECH_4228PCIE	0x0021
+#define PCI_DEVICE_ID_COMMTECH_4222PCIE	0x0022
+#define PCI_DEVICE_ID_EXAR_XR17V4358	0x4358
+#define PCI_DEVICE_ID_EXAR_XR17V8358	0x8358
+
+#define UART_EXAR_MPIOINT_7_0	0x8f	/* MPIOINT[7:0] */
+#define UART_EXAR_MPIOLVL_7_0	0x90	/* MPIOLVL[7:0] */
+#define UART_EXAR_MPIO3T_7_0	0x91	/* MPIO3T[7:0] */
+#define UART_EXAR_MPIOINV_7_0	0x92	/* MPIOINV[7:0] */
+#define UART_EXAR_MPIOSEL_7_0	0x93	/* MPIOSEL[7:0] */
+#define UART_EXAR_MPIOOD_7_0	0x94	/* MPIOOD[7:0] */
+#define UART_EXAR_MPIOINT_15_8	0x95	/* MPIOINT[15:8] */
+#define UART_EXAR_MPIOLVL_15_8	0x96	/* MPIOLVL[15:8] */
+#define UART_EXAR_MPIO3T_15_8	0x97	/* MPIO3T[15:8] */
+#define UART_EXAR_MPIOINV_15_8	0x98	/* MPIOINV[15:8] */
+#define UART_EXAR_MPIOSEL_15_8	0x99	/* MPIOSEL[15:8] */
+#define UART_EXAR_MPIOOD_15_8	0x9a	/* MPIOOD[15:8] */
+
+struct exar8250;
+
+/**
+ * struct exar8250_board - board information
+ * @num_ports: number of serial ports
+ * @reg_shift: describes UART register mapping in PCI memory
+ */
+struct exar8250_board {
+	unsigned int num_ports;
+	unsigned int reg_shift;
+	bool has_slave;
+	int	(*setup)(struct exar8250 *, struct pci_dev *,
+			 struct uart_8250_port *, int);
+	void	(*exit)(struct pci_dev *pcidev);
+};
+
+struct exar8250 {
+	unsigned int		nr;
+	struct exar8250_board	*board;
+	int			line[0];
+};
+
+static int default_setup(struct exar8250 *priv, struct pci_dev *pcidev,
+			 int idx, unsigned int offset,
+			 struct uart_8250_port *port)
+{
+	const struct exar8250_board *board = priv->board;
+	unsigned int bar = 0;
+
+	port->port.iotype = UPIO_MEM;
+	port->port.mapbase = pci_resource_start(pcidev, bar) + offset;
+	port->port.membase = pcim_iomap_table(pcidev)[bar] + offset;
+	port->port.regshift = board->reg_shift;
+
+	return 0;
+}
+
+static int
+pci_connect_tech_setup(struct exar8250 *priv, struct pci_dev *pcidev,
+		       struct uart_8250_port *port, int idx)
+{
+	unsigned int offset = idx * 0x200;
+	unsigned int baud = 1843200;
+
+	port->port.uartclk = baud * 16;
+	return default_setup(priv, pcidev, idx, offset, port);
+}
+
+static int
+pci_xr17c154_setup(struct exar8250 *priv, struct pci_dev *pcidev,
+		   struct uart_8250_port *port, int idx)
+{
+	unsigned int offset = idx * 0x200;
+	unsigned int baud = 921600;
+
+	port->port.uartclk = baud * 16;
+	return default_setup(priv, pcidev, idx, offset, port);
+}
+
+static void setup_gpio(u8 __iomem *p)
+{
+	writeb(0x00, p + UART_EXAR_MPIOINT_7_0);
+	writeb(0x00, p + UART_EXAR_MPIOLVL_7_0);
+	writeb(0x00, p + UART_EXAR_MPIO3T_7_0);
+	writeb(0x00, p + UART_EXAR_MPIOINV_7_0);
+	writeb(0x00, p + UART_EXAR_MPIOSEL_7_0);
+	writeb(0x00, p + UART_EXAR_MPIOOD_7_0);
+	writeb(0x00, p + UART_EXAR_MPIOINT_15_8);
+	writeb(0x00, p + UART_EXAR_MPIOLVL_15_8);
+	writeb(0x00, p + UART_EXAR_MPIO3T_15_8);
+	writeb(0x00, p + UART_EXAR_MPIOINV_15_8);
+	writeb(0x00, p + UART_EXAR_MPIOSEL_15_8);
+	writeb(0x00, p + UART_EXAR_MPIOOD_15_8);
+}
+
+static void *
+xr17v35x_register_gpio(struct pci_dev *pcidev)
+{
+	struct platform_device *pdev;
+
+	pdev = platform_device_alloc("gpio_exar", PLATFORM_DEVID_AUTO);
+	if (!pdev)
+		return NULL;
+
+	platform_set_drvdata(pdev, pcidev);
+	if (platform_device_add(pdev) < 0) {
+		platform_device_put(pdev);
+		return NULL;
+	}
+
+	return pdev;
+}
+
+static int
+pci_xr17v35x_setup(struct exar8250 *priv, struct pci_dev *pcidev,
+		   struct uart_8250_port *port, int idx)
+{
+	const struct exar8250_board *board = priv->board;
+	unsigned int offset = idx * 0x400;
+	unsigned int baud = 7812500;
+	u8 __iomem *p;
+	int ret;
+
+	port->port.uartclk = baud * 16;
+	/*
+	 * Setup the uart clock for the devices on expansion slot to
+	 * half the clock speed of the main chip (which is 125MHz)
+	 */
+	if (board->has_slave && idx >= 8)
+		port->port.uartclk /= 2;
+
+	p = pci_ioremap_bar(pcidev, 0);
+	if (!p)
+		return -ENOMEM;
+
+	/* Setup Multipurpose Input/Output pins. */
+	if (idx == 0)
+		setup_gpio(p);
+
+	writeb(0x00, p + UART_EXAR_8XMODE);
+	writeb(UART_FCTR_EXAR_TRGD, p + UART_EXAR_FCTR);
+	writeb(128, p + UART_EXAR_TXTRG);
+	writeb(128, p + UART_EXAR_RXTRG);
+	iounmap(p);
+
+	ret = default_setup(priv, pcidev, idx, offset, port);
+	if (ret)
+		return ret;
+
+	if (idx == 0)
+		port->port.private_data =
+			xr17v35x_register_gpio(pcidev);
+
+	return 0;
+}
+
+static void pci_xr17v35x_exit(struct pci_dev *pcidev)
+{
+	struct exar8250 *priv = pci_get_drvdata(pcidev);
+	struct uart_8250_port *port = serial8250_get_port(priv->line[0]);
+	struct platform_device *pdev = port->port.private_data;
+
+	platform_device_unregister(pdev);
+	port->port.private_data = NULL;
+}
+
+static int
+exar_pci_probe(struct pci_dev *pcidev, const struct pci_device_id *ent)
+{
+	unsigned int nr_ports, i, bar = 0, maxnr;
+	struct exar8250_board *board;
+	struct uart_8250_port uart;
+	struct exar8250 *priv;
+	int rc;
+
+	board = (struct exar8250_board *)ent->driver_data;
+	if (!board)
+		return -EINVAL;
+
+	rc = pcim_enable_device(pcidev);
+	if (rc)
+		return rc;
+
+	maxnr = pci_resource_len(pcidev, bar) >> (board->reg_shift + 3);
+
+	nr_ports = board->num_ports ? board->num_ports : pcidev->device & 0x0f;
+
+	priv = devm_kzalloc(&pcidev->dev, sizeof(*priv) +
+			    sizeof(unsigned int) * nr_ports,
+			    GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	priv->board = board;
+
+	memset(&uart, 0, sizeof(uart));
+	uart.port.flags = UPF_SKIP_TEST | UPF_BOOT_AUTOCONF | UPF_SHARE_IRQ
+			  | UPF_EXAR_EFR;
+	uart.port.irq = pcidev->irq;
+	uart.port.dev = &pcidev->dev;
+
+	for (i = 0; i < nr_ports && i < maxnr; i++) {
+		rc = board->setup(priv, pcidev, &uart, i);
+		if (rc) {
+			dev_err(&pcidev->dev, "Failed to setup port %u\n", i);
+			break;
+		}
+
+		dev_dbg(&pcidev->dev, "Setup PCI port: port %lx, irq %d, type %d\n",
+			uart.port.iobase, uart.port.irq, uart.port.iotype);
+
+		priv->line[i] = serial8250_register_8250_port(&uart);
+		if (priv->line[i] < 0) {
+			dev_err(&pcidev->dev,
+				"Couldn't register serial port %lx, irq %d, type %d, error %d\n",
+				uart.port.iobase, uart.port.irq,
+				uart.port.iotype, priv->line[i]);
+			break;
+		}
+	}
+	priv->nr = i;
+	pci_set_drvdata(pcidev, priv);
+	return 0;
+}
+
+static void exar_pci_remove(struct pci_dev *pcidev)
+{
+	struct exar8250 *priv = pci_get_drvdata(pcidev);
+	unsigned int i;
+
+	for (i = 0; i < priv->nr; i++)
+		serial8250_unregister_port(priv->line[i]);
+
+	if (priv->board->exit)
+		priv->board->exit(pcidev);
+}
+
+static int __maybe_unused exar_suspend(struct device *dev)
+{
+	struct pci_dev *pcidev = to_pci_dev(dev);
+	struct exar8250 *priv = pci_get_drvdata(pcidev);
+	unsigned int i;
+
+	for (i = 0; i < priv->nr; i++)
+		if (priv->line[i] >= 0)
+			serial8250_suspend_port(priv->line[i]);
+
+	/* Ensure that every init quirk is properly torn down */
+	if (priv->board->exit)
+		priv->board->exit(pcidev);
+
+	return 0;
+}
+
+static int __maybe_unused exar_resume(struct device *dev)
+{
+	struct pci_dev *pcidev = to_pci_dev(dev);
+	struct exar8250 *priv = pci_get_drvdata(pcidev);
+	unsigned int i;
+
+	for (i = 0; i < priv->nr; i++)
+		if (priv->line[i] >= 0)
+			serial8250_resume_port(priv->line[i]);
+
+	return 0;
+}
+
+static SIMPLE_DEV_PM_OPS(exar_pci_pm, exar_suspend, exar_resume);
+
+static const struct exar8250_board pbn_connect = {
+	.setup		= pci_connect_tech_setup,
+};
+
+static const struct exar8250_board pbn_exar_ibm_saturn = {
+	.num_ports	= 1,
+	.setup		= pci_xr17c154_setup,
+};
+
+static const struct exar8250_board pbn_exar_XR17C15x = {
+	.setup		= pci_xr17c154_setup,
+};
+
+static const struct exar8250_board pbn_exar_XR17V35x = {
+	.setup		= pci_xr17v35x_setup,
+	.exit		= pci_xr17v35x_exit,
+};
+
+static const struct exar8250_board pbn_exar_XR17V4358 = {
+	.num_ports	= 12,
+	.has_slave	= true,
+	.setup		= pci_xr17v35x_setup,
+	.exit		= pci_xr17v35x_exit,
+};
+
+static const struct exar8250_board pbn_exar_XR17V8358 = {
+	.num_ports	= 16,
+	.has_slave	= true,
+	.setup		= pci_xr17v35x_setup,
+	.exit		= pci_xr17v35x_exit,
+};
+
+#define CONNECT_DEVICE(devid, sdevid, bd) {				\
+	PCI_DEVICE_SUB(							\
+		PCI_VENDOR_ID_EXAR,					\
+		PCI_DEVICE_ID_EXAR_##devid,				\
+		PCI_SUBVENDOR_ID_CONNECT_TECH,				\
+		PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_##sdevid), 0, 0,	\
+		(kernel_ulong_t)&bd					\
+	}
+
+#define EXAR_DEVICE(vend, devid, bd) {					\
+	PCI_VDEVICE(vend, PCI_DEVICE_ID_##devid), (kernel_ulong_t)&bd	\
+	}
+
+#define IBM_DEVICE(devid, sdevid, bd) {			\
+	PCI_DEVICE_SUB(					\
+		PCI_VENDOR_ID_EXAR,			\
+		PCI_DEVICE_ID_EXAR_##devid,		\
+		PCI_VENDOR_ID_IBM,			\
+		PCI_SUBDEVICE_ID_IBM_##sdevid), 0, 0,	\
+		(kernel_ulong_t)&bd			\
+	}
+
+static struct pci_device_id exar_pci_tbl[] = {
+	CONNECT_DEVICE(XR17C152, UART_2_232, pbn_connect),
+	CONNECT_DEVICE(XR17C154, UART_4_232, pbn_connect),
+	CONNECT_DEVICE(XR17C158, UART_8_232, pbn_connect),
+	CONNECT_DEVICE(XR17C152, UART_1_1, pbn_connect),
+	CONNECT_DEVICE(XR17C154, UART_2_2, pbn_connect),
+	CONNECT_DEVICE(XR17C158, UART_4_4, pbn_connect),
+	CONNECT_DEVICE(XR17C152, UART_2, pbn_connect),
+	CONNECT_DEVICE(XR17C154, UART_4, pbn_connect),
+	CONNECT_DEVICE(XR17C158, UART_8, pbn_connect),
+	CONNECT_DEVICE(XR17C152, UART_2_485, pbn_connect),
+	CONNECT_DEVICE(XR17C154, UART_4_485, pbn_connect),
+	CONNECT_DEVICE(XR17C158, UART_8_485, pbn_connect),
+
+	IBM_DEVICE(XR17C152, SATURN_SERIAL_ONE_PORT, pbn_exar_ibm_saturn),
+
+	/* Exar Corp. XR17C15[248] Dual/Quad/Octal UART */
+	EXAR_DEVICE(EXAR, EXAR_XR17C152, pbn_exar_XR17C15x),
+	EXAR_DEVICE(EXAR, EXAR_XR17C154, pbn_exar_XR17C15x),
+	EXAR_DEVICE(EXAR, EXAR_XR17C158, pbn_exar_XR17C15x),
+
+	/* Exar Corp. XR17V[48]35[248] Dual/Quad/Octal/Hexa PCIe UARTs */
+	EXAR_DEVICE(EXAR, EXAR_XR17V352, pbn_exar_XR17V35x),
+	EXAR_DEVICE(EXAR, EXAR_XR17V354, pbn_exar_XR17V35x),
+	EXAR_DEVICE(EXAR, EXAR_XR17V358, pbn_exar_XR17V35x),
+	EXAR_DEVICE(EXAR, EXAR_XR17V4358, pbn_exar_XR17V4358),
+	EXAR_DEVICE(EXAR, EXAR_XR17V8358, pbn_exar_XR17V8358),
+	EXAR_DEVICE(COMMTECH, COMMTECH_4222PCIE, pbn_exar_XR17V35x),
+	EXAR_DEVICE(COMMTECH, COMMTECH_4224PCIE, pbn_exar_XR17V35x),
+	EXAR_DEVICE(COMMTECH, COMMTECH_4228PCIE, pbn_exar_XR17V35x),
+	{ 0, }
+};
+MODULE_DEVICE_TABLE(pci, exar_pci_tbl);
+
+static struct pci_driver exar_pci_driver = {
+	.name		= "exar_serial",
+	.probe		= exar_pci_probe,
+	.remove		= exar_pci_remove,
+	.driver         = {
+		.pm     = &exar_pci_pm,
+	},
+	.id_table	= exar_pci_tbl,
+};
+module_pci_driver(exar_pci_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("Exar Serial Dricer");
+MODULE_AUTHOR("Sudip Mukherjee <sudip.mukherjee@codethink.co.uk>");
diff --git a/drivers/tty/serial/8250/Kconfig b/drivers/tty/serial/8250/Kconfig
index c0bf996..2573ded 100644
--- a/drivers/tty/serial/8250/Kconfig
+++ b/drivers/tty/serial/8250/Kconfig
@@ -127,6 +127,10 @@ config SERIAL_8250_PCI
 	  Note that serial ports on NetMos 9835 Multi-I/O cards are handled
 	  by the parport_serial driver, enabled with CONFIG_PARPORT_SERIAL.
 
+config SERIAL_8250_EXAR
+        tristate "8250/16550 PCI device support"
+        depends on SERIAL_8250_PCI
+
 config SERIAL_8250_HP300
 	tristate
 	depends on SERIAL_8250 && HP300
diff --git a/drivers/tty/serial/8250/Makefile b/drivers/tty/serial/8250/Makefile
index 850e721..2f30f9e 100644
--- a/drivers/tty/serial/8250/Makefile
+++ b/drivers/tty/serial/8250/Makefile
@@ -10,6 +10,7 @@ obj-$(CONFIG_SERIAL_8250)		+= 8250.o 8250_base.o
 8250_base-$(CONFIG_SERIAL_8250_FINTEK)	+= 8250_fintek.o
 obj-$(CONFIG_SERIAL_8250_GSC)		+= 8250_gsc.o
 obj-$(CONFIG_SERIAL_8250_PCI)		+= 8250_pci.o
+obj-$(CONFIG_SERIAL_8250_EXAR)		+= 8250_exar.o
 obj-$(CONFIG_SERIAL_8250_HP300)		+= 8250_hp300.o
 obj-$(CONFIG_SERIAL_8250_CS)		+= serial_cs.o
 obj-$(CONFIG_SERIAL_8250_ACORN)		+= 8250_acorn.o
-- 
1.9.1

[PATCH v13 2/2] serial: 8250_pci: remove exar code

[Sudip Mukherjee]

From: Sudip Mukherjee <sudip.mukherjee@codethink.co.uk>

Remove the Exar specific codes from 8250_pci and blacklist those chips
so that the new Exar serial driver binds to the devices.

Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Signed-off-by: Sudip Mukherjee <sudip.mukherjee@codethink.co.uk>
---
 drivers/tty/serial/8250/8250_pci.c | 336 +------------------------------------
 drivers/tty/serial/8250/Kconfig    |   1 +
 2 files changed, 4 insertions(+), 333 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_pci.c b/drivers/tty/serial/8250/8250_pci.c
index f7ee4e0..3eb638c 100644
--- a/drivers/tty/serial/8250/8250_pci.c
+++ b/drivers/tty/serial/8250/8250_pci.c
@@ -1610,9 +1610,6 @@ static int pci_eg20t_init(struct pci_dev *dev)
 #endif
 }
 
-#define PCI_DEVICE_ID_EXAR_XR17V4358	0x4358
-#define PCI_DEVICE_ID_EXAR_XR17V8358	0x8358
-
 #define UART_EXAR_MPIOINT_7_0	0x8f	/* MPIOINT[7:0] */
 #define UART_EXAR_MPIOLVL_7_0	0x90	/* MPIOLVL[7:0] */
 #define UART_EXAR_MPIO3T_7_0	0x91	/* MPIO3T[7:0] */
@@ -1625,71 +1622,6 @@ static int pci_eg20t_init(struct pci_dev *dev)
 #define UART_EXAR_MPIOINV_15_8	0x98	/* MPIOINV[15:8] */
 #define UART_EXAR_MPIOSEL_15_8	0x99	/* MPIOSEL[15:8] */
 #define UART_EXAR_MPIOOD_15_8	0x9a	/* MPIOOD[15:8] */
-
-static int
-pci_xr17c154_setup(struct serial_private *priv,
-		  const struct pciserial_board *board,
-		  struct uart_8250_port *port, int idx)
-{
-	port->port.flags |= UPF_EXAR_EFR;
-	return pci_default_setup(priv, board, port, idx);
-}
-
-static inline int
-xr17v35x_has_slave(struct serial_private *priv)
-{
-	const int dev_id = priv->dev->device;
-
-	return ((dev_id == PCI_DEVICE_ID_EXAR_XR17V4358) ||
-		(dev_id == PCI_DEVICE_ID_EXAR_XR17V8358));
-}
-
-static int
-pci_xr17v35x_setup(struct serial_private *priv,
-		  const struct pciserial_board *board,
-		  struct uart_8250_port *port, int idx)
-{
-	u8 __iomem *p;
-
-	p = pci_ioremap_bar(priv->dev, 0);
-	if (p == NULL)
-		return -ENOMEM;
-
-	port->port.flags |= UPF_EXAR_EFR;
-
-	/*
-	 * Setup the uart clock for the devices on expansion slot to
-	 * half the clock speed of the main chip (which is 125MHz)
-	 */
-	if (xr17v35x_has_slave(priv) && idx >= 8)
-		port->port.uartclk = (7812500 * 16 / 2);
-
-	/*
-	 * Setup Multipurpose Input/Output pins.
-	 */
-	if (idx == 0) {
-		writeb(0x00, p + UART_EXAR_MPIOINT_7_0);
-		writeb(0x00, p + UART_EXAR_MPIOLVL_7_0);
-		writeb(0x00, p + UART_EXAR_MPIO3T_7_0);
-		writeb(0x00, p + UART_EXAR_MPIOINV_7_0);
-		writeb(0x00, p + UART_EXAR_MPIOSEL_7_0);
-		writeb(0x00, p + UART_EXAR_MPIOOD_7_0);
-		writeb(0x00, p + UART_EXAR_MPIOINT_15_8);
-		writeb(0x00, p + UART_EXAR_MPIOLVL_15_8);
-		writeb(0x00, p + UART_EXAR_MPIO3T_15_8);
-		writeb(0x00, p + UART_EXAR_MPIOINV_15_8);
-		writeb(0x00, p + UART_EXAR_MPIOSEL_15_8);
-		writeb(0x00, p + UART_EXAR_MPIOOD_15_8);
-	}
-	writeb(0x00, p + UART_EXAR_8XMODE);
-	writeb(UART_FCTR_EXAR_TRGD, p + UART_EXAR_FCTR);
-	writeb(128, p + UART_EXAR_TXTRG);
-	writeb(128, p + UART_EXAR_RXTRG);
-	iounmap(p);
-
-	return pci_default_setup(priv, board, port, idx);
-}
-
 #define PCI_DEVICE_ID_COMMTECH_4222PCI335 0x0004
 #define PCI_DEVICE_ID_COMMTECH_4224PCI335 0x0002
 #define PCI_DEVICE_ID_COMMTECH_2324PCI335 0x000a
@@ -1814,9 +1746,6 @@ static int pci_eg20t_init(struct pci_dev *dev)
 #define PCI_VENDOR_ID_AGESTAR		0x5372
 #define PCI_DEVICE_ID_AGESTAR_9375	0x6872
 #define PCI_VENDOR_ID_ASIX		0x9710
-#define PCI_DEVICE_ID_COMMTECH_4224PCIE	0x0020
-#define PCI_DEVICE_ID_COMMTECH_4228PCIE	0x0021
-#define PCI_DEVICE_ID_COMMTECH_4222PCIE	0x0022
 #define PCI_DEVICE_ID_BROADCOM_TRUMANAGE 0x160a
 #define PCI_DEVICE_ID_AMCC_ADDIDATA_APCI7800 0x818e
 
@@ -2278,65 +2207,6 @@ static int pci_eg20t_init(struct pci_dev *dev)
 		.setup		= pci_timedia_setup,
 	},
 	/*
-	 * Exar cards
-	 */
-	{
-		.vendor = PCI_VENDOR_ID_EXAR,
-		.device = PCI_DEVICE_ID_EXAR_XR17C152,
-		.subvendor	= PCI_ANY_ID,
-		.subdevice	= PCI_ANY_ID,
-		.setup		= pci_xr17c154_setup,
-	},
-	{
-		.vendor = PCI_VENDOR_ID_EXAR,
-		.device = PCI_DEVICE_ID_EXAR_XR17C154,
-		.subvendor	= PCI_ANY_ID,
-		.subdevice	= PCI_ANY_ID,
-		.setup		= pci_xr17c154_setup,
-	},
-	{
-		.vendor = PCI_VENDOR_ID_EXAR,
-		.device = PCI_DEVICE_ID_EXAR_XR17C158,
-		.subvendor	= PCI_ANY_ID,
-		.subdevice	= PCI_ANY_ID,
-		.setup		= pci_xr17c154_setup,
-	},
-	{
-		.vendor = PCI_VENDOR_ID_EXAR,
-		.device = PCI_DEVICE_ID_EXAR_XR17V352,
-		.subvendor	= PCI_ANY_ID,
-		.subdevice	= PCI_ANY_ID,
-		.setup		= pci_xr17v35x_setup,
-	},
-	{
-		.vendor = PCI_VENDOR_ID_EXAR,
-		.device = PCI_DEVICE_ID_EXAR_XR17V354,
-		.subvendor	= PCI_ANY_ID,
-		.subdevice	= PCI_ANY_ID,
-		.setup		= pci_xr17v35x_setup,
-	},
-	{
-		.vendor = PCI_VENDOR_ID_EXAR,
-		.device = PCI_DEVICE_ID_EXAR_XR17V358,
-		.subvendor	= PCI_ANY_ID,
-		.subdevice	= PCI_ANY_ID,
-		.setup		= pci_xr17v35x_setup,
-	},
-	{
-		.vendor = PCI_VENDOR_ID_EXAR,
-		.device = PCI_DEVICE_ID_EXAR_XR17V4358,
-		.subvendor	= PCI_ANY_ID,
-		.subdevice	= PCI_ANY_ID,
-		.setup		= pci_xr17v35x_setup,
-	},
-	{
-		.vendor = PCI_VENDOR_ID_EXAR,
-		.device = PCI_DEVICE_ID_EXAR_XR17V8358,
-		.subvendor	= PCI_ANY_ID,
-		.subdevice	= PCI_ANY_ID,
-		.setup		= pci_xr17v35x_setup,
-	},
-	/*
 	 * Xircom cards
 	 */
 	{
@@ -2592,27 +2462,6 @@ static int pci_eg20t_init(struct pci_dev *dev)
 		.subdevice	= PCI_ANY_ID,
 		.setup		= pci_fastcom335_setup,
 	},
-	{
-		.vendor = PCI_VENDOR_ID_COMMTECH,
-		.device = PCI_DEVICE_ID_COMMTECH_4222PCIE,
-		.subvendor	= PCI_ANY_ID,
-		.subdevice	= PCI_ANY_ID,
-		.setup		= pci_xr17v35x_setup,
-	},
-	{
-		.vendor = PCI_VENDOR_ID_COMMTECH,
-		.device = PCI_DEVICE_ID_COMMTECH_4224PCIE,
-		.subvendor	= PCI_ANY_ID,
-		.subdevice	= PCI_ANY_ID,
-		.setup		= pci_xr17v35x_setup,
-	},
-	{
-		.vendor = PCI_VENDOR_ID_COMMTECH,
-		.device = PCI_DEVICE_ID_COMMTECH_4228PCIE,
-		.subvendor	= PCI_ANY_ID,
-		.subdevice	= PCI_ANY_ID,
-		.setup		= pci_xr17v35x_setup,
-	},
 	/*
 	 * Broadcom TruManage (NetXtreme)
 	 */
@@ -2825,15 +2674,6 @@ enum pci_board_num_t {
 	pbn_computone_6,
 	pbn_computone_8,
 	pbn_sbsxrsio,
-	pbn_exar_XR17C152,
-	pbn_exar_XR17C154,
-	pbn_exar_XR17C158,
-	pbn_exar_XR17V352,
-	pbn_exar_XR17V354,
-	pbn_exar_XR17V358,
-	pbn_exar_XR17V4358,
-	pbn_exar_XR17V8358,
-	pbn_exar_ibm_saturn,
 	pbn_pasemi_1682M,
 	pbn_ni8430_2,
 	pbn_ni8430_4,
@@ -3474,76 +3314,6 @@ enum pci_board_num_t {
 		.reg_shift	= 4,
 	},
 	/*
-	 * Exar Corp. XR17C15[248] Dual/Quad/Octal UART
-	 *  Only basic 16550A support.
-	 *  XR17C15[24] are not tested, but they should work.
-	 */
-	[pbn_exar_XR17C152] = {
-		.flags		= FL_BASE0,
-		.num_ports	= 2,
-		.base_baud	= 921600,
-		.uart_offset	= 0x200,
-	},
-	[pbn_exar_XR17C154] = {
-		.flags		= FL_BASE0,
-		.num_ports	= 4,
-		.base_baud	= 921600,
-		.uart_offset	= 0x200,
-	},
-	[pbn_exar_XR17C158] = {
-		.flags		= FL_BASE0,
-		.num_ports	= 8,
-		.base_baud	= 921600,
-		.uart_offset	= 0x200,
-	},
-	[pbn_exar_XR17V352] = {
-		.flags		= FL_BASE0,
-		.num_ports	= 2,
-		.base_baud	= 7812500,
-		.uart_offset	= 0x400,
-		.reg_shift	= 0,
-		.first_offset	= 0,
-	},
-	[pbn_exar_XR17V354] = {
-		.flags		= FL_BASE0,
-		.num_ports	= 4,
-		.base_baud	= 7812500,
-		.uart_offset	= 0x400,
-		.reg_shift	= 0,
-		.first_offset	= 0,
-	},
-	[pbn_exar_XR17V358] = {
-		.flags		= FL_BASE0,
-		.num_ports	= 8,
-		.base_baud	= 7812500,
-		.uart_offset	= 0x400,
-		.reg_shift	= 0,
-		.first_offset	= 0,
-	},
-	[pbn_exar_XR17V4358] = {
-		.flags		= FL_BASE0,
-		.num_ports	= 12,
-		.base_baud	= 7812500,
-		.uart_offset	= 0x400,
-		.reg_shift	= 0,
-		.first_offset	= 0,
-	},
-	[pbn_exar_XR17V8358] = {
-		.flags		= FL_BASE0,
-		.num_ports	= 16,
-		.base_baud	= 7812500,
-		.uart_offset	= 0x400,
-		.reg_shift	= 0,
-		.first_offset	= 0,
-	},
-	[pbn_exar_ibm_saturn] = {
-		.flags		= FL_BASE0,
-		.num_ports	= 1,
-		.base_baud	= 921600,
-		.uart_offset	= 0x200,
-	},
-
-	/*
 	 * PA Semi PWRficient PA6T-1682M on-chip UART
 	 */
 	[pbn_pasemi_1682M] = {
@@ -3739,6 +3509,9 @@ enum pci_board_num_t {
 	{ PCI_VDEVICE(INTEL, 0x228c), },
 	{ PCI_VDEVICE(INTEL, 0x9ce3), },
 	{ PCI_VDEVICE(INTEL, 0x9ce4), },
+
+	/* Exar devices */
+	{ PCI_VDEVICE(EXAR, PCI_ANY_ID), },
 };
 
 /*
@@ -4164,58 +3937,6 @@ static SIMPLE_DEV_PM_OPS(pciserial_pm_ops, pciserial_suspend_one,
 		PCI_VENDOR_ID_AFAVLAB,
 		PCI_SUBDEVICE_ID_AFAVLAB_P061, 0, 0,
 		pbn_b0_4_1152000 },
-	{	PCI_VENDOR_ID_EXAR, PCI_DEVICE_ID_EXAR_XR17C152,
-		PCI_SUBVENDOR_ID_CONNECT_TECH,
-		PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_2_232, 0, 0,
-		pbn_b0_2_1843200_200 },
-	{	PCI_VENDOR_ID_EXAR, PCI_DEVICE_ID_EXAR_XR17C154,
-		PCI_SUBVENDOR_ID_CONNECT_TECH,
-		PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_4_232, 0, 0,
-		pbn_b0_4_1843200_200 },
-	{	PCI_VENDOR_ID_EXAR, PCI_DEVICE_ID_EXAR_XR17C158,
-		PCI_SUBVENDOR_ID_CONNECT_TECH,
-		PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_8_232, 0, 0,
-		pbn_b0_8_1843200_200 },
-	{	PCI_VENDOR_ID_EXAR, PCI_DEVICE_ID_EXAR_XR17C152,
-		PCI_SUBVENDOR_ID_CONNECT_TECH,
-		PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_1_1, 0, 0,
-		pbn_b0_2_1843200_200 },
-	{	PCI_VENDOR_ID_EXAR, PCI_DEVICE_ID_EXAR_XR17C154,
-		PCI_SUBVENDOR_ID_CONNECT_TECH,
-		PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_2_2, 0, 0,
-		pbn_b0_4_1843200_200 },
-	{	PCI_VENDOR_ID_EXAR, PCI_DEVICE_ID_EXAR_XR17C158,
-		PCI_SUBVENDOR_ID_CONNECT_TECH,
-		PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_4_4, 0, 0,
-		pbn_b0_8_1843200_200 },
-	{	PCI_VENDOR_ID_EXAR, PCI_DEVICE_ID_EXAR_XR17C152,
-		PCI_SUBVENDOR_ID_CONNECT_TECH,
-		PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_2, 0, 0,
-		pbn_b0_2_1843200_200 },
-	{	PCI_VENDOR_ID_EXAR, PCI_DEVICE_ID_EXAR_XR17C154,
-		PCI_SUBVENDOR_ID_CONNECT_TECH,
-		PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_4, 0, 0,
-		pbn_b0_4_1843200_200 },
-	{	PCI_VENDOR_ID_EXAR, PCI_DEVICE_ID_EXAR_XR17C158,
-		PCI_SUBVENDOR_ID_CONNECT_TECH,
-		PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_8, 0, 0,
-		pbn_b0_8_1843200_200 },
-	{	PCI_VENDOR_ID_EXAR, PCI_DEVICE_ID_EXAR_XR17C152,
-		PCI_SUBVENDOR_ID_CONNECT_TECH,
-		PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_2_485, 0, 0,
-		pbn_b0_2_1843200_200 },
-	{	PCI_VENDOR_ID_EXAR, PCI_DEVICE_ID_EXAR_XR17C154,
-		PCI_SUBVENDOR_ID_CONNECT_TECH,
-		PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_4_485, 0, 0,
-		pbn_b0_4_1843200_200 },
-	{	PCI_VENDOR_ID_EXAR, PCI_DEVICE_ID_EXAR_XR17C158,
-		PCI_SUBVENDOR_ID_CONNECT_TECH,
-		PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_8_485, 0, 0,
-		pbn_b0_8_1843200_200 },
-	{	PCI_VENDOR_ID_EXAR, PCI_DEVICE_ID_EXAR_XR17C152,
-		PCI_VENDOR_ID_IBM, PCI_SUBDEVICE_ID_IBM_SATURN_SERIAL_ONE_PORT,
-		0, 0, pbn_exar_ibm_saturn },
-
 	{	PCI_VENDOR_ID_SEALEVEL, PCI_DEVICE_ID_SEALEVEL_U530,
 		PCI_ANY_ID, PCI_ANY_ID, 0, 0,
 		pbn_b2_bt_1_115200 },
@@ -4943,45 +4664,6 @@ static SIMPLE_DEV_PM_OPS(pciserial_pm_ops, pciserial_suspend_one,
 	{	PCI_VENDOR_ID_DCI, PCI_DEVICE_ID_DCI_PCCOM8,
 		PCI_ANY_ID, PCI_ANY_ID, 0, 0,
 		pbn_b3_8_115200 },
-
-	/*
-	 * Exar Corp. XR17C15[248] Dual/Quad/Octal UART
-	 */
-	{	PCI_VENDOR_ID_EXAR, PCI_DEVICE_ID_EXAR_XR17C152,
-		PCI_ANY_ID, PCI_ANY_ID,
-		0,
-		0, pbn_exar_XR17C152 },
-	{	PCI_VENDOR_ID_EXAR, PCI_DEVICE_ID_EXAR_XR17C154,
-		PCI_ANY_ID, PCI_ANY_ID,
-		0,
-		0, pbn_exar_XR17C154 },
-	{	PCI_VENDOR_ID_EXAR, PCI_DEVICE_ID_EXAR_XR17C158,
-		PCI_ANY_ID, PCI_ANY_ID,
-		0,
-		0, pbn_exar_XR17C158 },
-	/*
-	 * Exar Corp. XR17V[48]35[248] Dual/Quad/Octal/Hexa PCIe UARTs
-	 */
-	{	PCI_VENDOR_ID_EXAR, PCI_DEVICE_ID_EXAR_XR17V352,
-		PCI_ANY_ID, PCI_ANY_ID,
-		0,
-		0, pbn_exar_XR17V352 },
-	{	PCI_VENDOR_ID_EXAR, PCI_DEVICE_ID_EXAR_XR17V354,
-		PCI_ANY_ID, PCI_ANY_ID,
-		0,
-		0, pbn_exar_XR17V354 },
-	{	PCI_VENDOR_ID_EXAR, PCI_DEVICE_ID_EXAR_XR17V358,
-		PCI_ANY_ID, PCI_ANY_ID,
-		0,
-		0, pbn_exar_XR17V358 },
-	{	PCI_VENDOR_ID_EXAR, PCI_DEVICE_ID_EXAR_XR17V4358,
-		PCI_ANY_ID, PCI_ANY_ID,
-		0,
-		0, pbn_exar_XR17V4358 },
-	{	PCI_VENDOR_ID_EXAR, PCI_DEVICE_ID_EXAR_XR17V8358,
-		PCI_ANY_ID, PCI_ANY_ID,
-		0,
-		0, pbn_exar_XR17V8358 },
 	/*
 	 * Pericom PI7C9X795[1248] Uno/Dual/Quad/Octal UART
 	 */
@@ -5576,18 +5258,6 @@ static SIMPLE_DEV_PM_OPS(pciserial_pm_ops, pciserial_suspend_one,
 		PCI_ANY_ID, PCI_ANY_ID,
 		0,
 		0, pbn_b0_8_1152000_200 },
-	{	PCI_VENDOR_ID_COMMTECH, PCI_DEVICE_ID_COMMTECH_4222PCIE,
-		PCI_ANY_ID, PCI_ANY_ID,
-		0,
-		0, pbn_exar_XR17V352 },
-	{	PCI_VENDOR_ID_COMMTECH, PCI_DEVICE_ID_COMMTECH_4224PCIE,
-		PCI_ANY_ID, PCI_ANY_ID,
-		0,
-		0, pbn_exar_XR17V354 },
-	{	PCI_VENDOR_ID_COMMTECH, PCI_DEVICE_ID_COMMTECH_4228PCIE,
-		PCI_ANY_ID, PCI_ANY_ID,
-		0,
-		0, pbn_exar_XR17V358 },
 
 	/* Fintek PCI serial cards */
 	{ PCI_DEVICE(0x1c29, 0x1104), .driver_data = pbn_fintek_4 },
diff --git a/drivers/tty/serial/8250/Kconfig b/drivers/tty/serial/8250/Kconfig
index 2573ded..a65fb81 100644
--- a/drivers/tty/serial/8250/Kconfig
+++ b/drivers/tty/serial/8250/Kconfig
@@ -130,6 +130,7 @@ config SERIAL_8250_PCI
 config SERIAL_8250_EXAR
         tristate "8250/16550 PCI device support"
         depends on SERIAL_8250_PCI
+	default SERIAL_8250
 
 config SERIAL_8250_HP300
 	tristate
-- 
1.9.1

Re: [PATCH v13 1/2] serial: exar: split out the exar code from 8250_pci

[Jan Kiszka]

On 2017-01-30 23:28, Sudip Mukherjee wrote:
> From: Sudip Mukherjee <sudip.mukherjee@codethink.co.uk>
> 
> Add the serial driver for the Exar chips. And also register the
> platform device for the GPIO provided by the Exar chips.

"Also" means you are doing two things in one patch - was this already
discussed and accepted in previous review rounds? If so, ignore my
comment, but I would have asked for two patches, one that just
translates the existing code and another that adds this new feature.

> 
> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> Signed-off-by: Sudip Mukherjee <sudip.mukherjee@codethink.co.uk>
> ---
> 
> Andy,
> I have added the if (!board) check, but I am not sure how board can be
> NULL here. If probe executes that will mean there was a match of the
> device id and so in that case board can not be NULL.
> 
>  drivers/tty/serial/8250/8250_exar.c | 396 ++++++++++++++++++++++++++++++++++++
>  drivers/tty/serial/8250/Kconfig     |   4 +
>  drivers/tty/serial/8250/Makefile    |   1 +
>  3 files changed, 401 insertions(+)
>  create mode 100644 drivers/tty/serial/8250/8250_exar.c
> 
> diff --git a/drivers/tty/serial/8250/8250_exar.c b/drivers/tty/serial/8250/8250_exar.c
> new file mode 100644
> index 0000000..ba1f359
> --- /dev/null
> +++ b/drivers/tty/serial/8250/8250_exar.c
> @@ -0,0 +1,396 @@
> +/*
> + *  Probe module for 8250/16550-type Exar chips PCI serial ports.
> + *
> + *  Based on drivers/tty/serial/8250/8250_pci.c,
> + *
> + *  Copyright (C) 2017 Sudip Mukherjee, All Rights Reserved.

It's legally cleaner to carry over the copyright notice from the
original file, unless you rewrote everything (unlikely on first glance).
You may still add yours to the list for the significant contributions.

BTW, are you personally the copyright holder or your employer Codethink?
Depends on your contractual situation, but the former is less common.

Jan

-- 
Siemens AG, Corporate Technology, CT RDA ITP SES-DE
Corporate Competence Center Embedded Linux

Re: [PATCH v13 1/2] serial: exar: split out the exar code from 8250_pci

[Andy Shevchenko]

On Fri, Feb 3, 2017 at 4:02 PM, Jan Kiszka <jan.kiszka@siemens.com> wrote:
> On 2017-01-30 23:28, Sudip Mukherjee wrote:
>> From: Sudip Mukherjee <sudip.mukherjee@codethink.co.uk>
>>
>> Add the serial driver for the Exar chips. And also register the
>> platform device for the GPIO provided by the Exar chips.
>
> "Also" means you are doing two things in one patch - was this already
> discussed and accepted in previous review rounds? If so, ignore my
> comment, but I would have asked for two patches, one that just
> translates the existing code and another that adds this new feature.

Since it's already in Greg's tty-next, no point to fix anymore this
particular part.
However, you are right that few lines of code might be split to a
separate change.

>> +/*
>> + *  Probe module for 8250/16550-type Exar chips PCI serial ports.
>> + *
>> + *  Based on drivers/tty/serial/8250/8250_pci.c,
>> + *
>> + *  Copyright (C) 2017 Sudip Mukherjee, All Rights Reserved.
>
> It's legally cleaner to carry over the copyright notice from the
> original file, unless you rewrote everything (unlikely on first glance).
> You may still add yours to the list for the significant contributions.
>
> BTW, are you personally the copyright holder or your employer Codethink?
> Depends on your contractual situation, but the former is less common.

This is good comment and I think it needs to be addressed (as a
separate change due to above).

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v13 1/2] serial: exar: split out the exar code from 8250_pci

[Sudip Mukherjee]

On Friday 03 February 2017 02:02 PM, Jan Kiszka wrote:
> On 2017-01-30 23:28, Sudip Mukherjee wrote:
>> From: Sudip Mukherjee <sudip.mukherjee@codethink.co.uk>
>>
>> Add the serial driver for the Exar chips. And also register the
>> platform device for the GPIO provided by the Exar chips.
>
> "Also" means you are doing two things in one patch - was this already
> discussed and accepted in previous review rounds? If so, ignore my
> comment, but I would have asked for two patches, one that just
> translates the existing code and another that adds this new feature.
>

Like Andy replied, this is already in tty-next.

>>
>> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
>> Signed-off-by: Sudip Mukherjee <sudip.mukherjee@codethink.co.uk>
>> ---
>>
>> Andy,
>> I have added the if (!board) check, but I am not sure how board can be
>> NULL here. If probe executes that will mean there was a match of the
>> device id and so in that case board can not be NULL.
>>
>>   drivers/tty/serial/8250/8250_exar.c | 396 ++++++++++++++++++++++++++++++++++++
>>   drivers/tty/serial/8250/Kconfig     |   4 +
>>   drivers/tty/serial/8250/Makefile    |   1 +
>>   3 files changed, 401 insertions(+)
>>   create mode 100644 drivers/tty/serial/8250/8250_exar.c
>>
>> diff --git a/drivers/tty/serial/8250/8250_exar.c b/drivers/tty/serial/8250/8250_exar.c
>> new file mode 100644
>> index 0000000..ba1f359
>> --- /dev/null
>> +++ b/drivers/tty/serial/8250/8250_exar.c
>> @@ -0,0 +1,396 @@
>> +/*
>> + *  Probe module for 8250/16550-type Exar chips PCI serial ports.
>> + *
>> + *  Based on drivers/tty/serial/8250/8250_pci.c,
>> + *
>> + *  Copyright (C) 2017 Sudip Mukherjee, All Rights Reserved.
>
> It's legally cleaner to carry over the copyright notice from the
> original file, unless you rewrote everything (unlikely on first glance).
> You may still add yours to the list for the significant contributions.

Should i send a separate patch to modify those? Andy?

>
> BTW, are you personally the copyright holder or your employer Codethink?
> Depends on your contractual situation, but the former is less common.

Well, Codethink has nothing to do with this patch. This was a voluntary 
work started before I joined Codethink, but then I joined Codethink and 
found very little time to finish this. So finally now its done.

https://lists.kernelnewbies.org/pipermail/kernelnewbies/2015-November/015372.html


Regards
Sudip

Re: [PATCH v13 1/2] serial: exar: split out the exar code from 8250_pci

[Andy Shevchenko]

On Fri, Feb 3, 2017 at 11:31 PM, Sudip Mukherjee
<sudipm.mukherjee@gmail.com> wrote:
> On Friday 03 February 2017 02:02 PM, Jan Kiszka wrote:
>> On 2017-01-30 23:28, Sudip Mukherjee wrote:


>>> @@ -0,0 +1,396 @@
>>> +/*
>>> + *  Probe module for 8250/16550-type Exar chips PCI serial ports.
>>> + *
>>> + *  Based on drivers/tty/serial/8250/8250_pci.c,
>>> + *
>>> + *  Copyright (C) 2017 Sudip Mukherjee, All Rights Reserved.
>>
>>
>> It's legally cleaner to carry over the copyright notice from the
>> original file, unless you rewrote everything (unlikely on first glance).
>> You may still add yours to the list for the significant contributions.
>
> Should i send a separate patch to modify those? Andy?

Please, do.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v13 1/2] serial: exar: split out the exar code from 8250_pci

[Jan Kiszka]

On 2017-02-03 22:31, Sudip Mukherjee wrote:
> On Friday 03 February 2017 02:02 PM, Jan Kiszka wrote:
>> BTW, are you personally the copyright holder or your employer Codethink?
>> Depends on your contractual situation, but the former is less common.
> 
> Well, Codethink has nothing to do with this patch. This was a voluntary
> work started before I joined Codethink, but then I joined Codethink and
> found very little time to finish this. So finally now its done.
> 
> https://lists.kernelnewbies.org/pipermail/kernelnewbies/2015-November/015372.html
> 

Hmm, why using your corporate email address then? This suggests a
different copyright situation.

Funnily, I just received this question internally: How can you tell
apart if someone sends a personal contribution via his/her employer
account from someone contributing on behalf of a company, thus with that
company holding the rights? I argued that no one would do the former to
prevent wrong accounting, but you just proved a counterexample. :)

Regards,
Jan

-- 
Siemens AG, Corporate Technology, CT RDA ITP SES-DE
Corporate Competence Center Embedded Linux

Re: [PATCH v13 1/2] serial: exar: split out the exar code from 8250_pci

[Greg Kroah-Hartman]

On Mon, Feb 06, 2017 at 02:49:07PM +0100, Jan Kiszka wrote:
> On 2017-02-03 22:31, Sudip Mukherjee wrote:
> > On Friday 03 February 2017 02:02 PM, Jan Kiszka wrote:
> >> BTW, are you personally the copyright holder or your employer Codethink?
> >> Depends on your contractual situation, but the former is less common.
> > 
> > Well, Codethink has nothing to do with this patch. This was a voluntary
> > work started before I joined Codethink, but then I joined Codethink and
> > found very little time to finish this. So finally now its done.
> > 
> > https://lists.kernelnewbies.org/pipermail/kernelnewbies/2015-November/015372.html
> > 
> 
> Hmm, why using your corporate email address then? This suggests a
> different copyright situation.
> 
> Funnily, I just received this question internally: How can you tell
> apart if someone sends a personal contribution via his/her employer
> account from someone contributing on behalf of a company, thus with that
> company holding the rights? I argued that no one would do the former to
> prevent wrong accounting, but you just proved a counterexample. :)

There are numerous companies that do this, some create whole shell
orginizations in order to "hide" their kernel contributions for various
"interesting" reasons.

Fun stuff.  I suggest having your internal people talk to your lawyers,
they should know all about this (and if not, have those lawyers talk to
the LF lawyers...)

But that's not the issue here, we know Sudip :)

thanks,

greg k-h

Re: [PATCH v13 1/2] serial: exar: split out the exar code from 8250_pci

[Jan Kiszka]

On 2017-02-06 15:06, Greg Kroah-Hartman wrote:
> On Mon, Feb 06, 2017 at 02:49:07PM +0100, Jan Kiszka wrote:
>> On 2017-02-03 22:31, Sudip Mukherjee wrote:
>>> On Friday 03 February 2017 02:02 PM, Jan Kiszka wrote:
>>>> BTW, are you personally the copyright holder or your employer Codethink?
>>>> Depends on your contractual situation, but the former is less common.
>>>
>>> Well, Codethink has nothing to do with this patch. This was a voluntary
>>> work started before I joined Codethink, but then I joined Codethink and
>>> found very little time to finish this. So finally now its done.
>>>
>>> https://lists.kernelnewbies.org/pipermail/kernelnewbies/2015-November/015372.html
>>>
>>
>> Hmm, why using your corporate email address then? This suggests a
>> different copyright situation.
>>
>> Funnily, I just received this question internally: How can you tell
>> apart if someone sends a personal contribution via his/her employer
>> account from someone contributing on behalf of a company, thus with that
>> company holding the rights? I argued that no one would do the former to
>> prevent wrong accounting, but you just proved a counterexample. :)
> 
> There are numerous companies that do this, some create whole shell
> orginizations in order to "hide" their kernel contributions for various
> "interesting" reasons.

I was not talking about companies but individuals: If they use their
company address for something written in their spare time (and their
contract allow to keep ownership of that), they needless suggest their
company holds the copyright that way around. If they stick with a
private address, it remains more clearly in their hand.

What you mentioned is a different story and can indeed be interesting
for the companies when they realize they would like for prove their code
ownership to some legal authority for whatever reason.

Anyway, off-topic now.

Jan

> 
> Fun stuff.  I suggest having your internal people talk to your lawyers,
> they should know all about this (and if not, have those lawyers talk to
> the LF lawyers...)
> 
> But that's not the issue here, we know Sudip :)
> 
> thanks,
> 
> greg k-h
> 

-- 
Siemens AG, Corporate Technology, CT RDA ITP SES-DE
Corporate Competence Center Embedded Linux

Re: [PATCH v13 1/2] serial: exar: split out the exar code from 8250_pci

[Jan Kiszka]

On 2017-01-30 23:28, Sudip Mukherjee wrote:
> From: Sudip Mukherjee <sudip.mukherjee@codethink.co.uk>
> 
> Add the serial driver for the Exar chips. And also register the
> platform device for the GPIO provided by the Exar chips.
> 

And another question: you left pci_fastcom335_setup and related things
untouched - did that code come later, or is it left in 8250_pci.c for a
reason?

Jan

-- 
Siemens AG, Corporate Technology, CT RDA ITP SES-DE
Corporate Competence Center Embedded Linux

Re: [PATCH v13 1/2] serial: exar: split out the exar code from 8250_pci

[Jan Kiszka]

On 2017-01-30 23:28, Sudip Mukherjee wrote:
> From: Sudip Mukherjee <sudip.mukherjee@codethink.co.uk>
> 
> Add the serial driver for the Exar chips. And also register the
> platform device for the GPIO provided by the Exar chips.
> 
> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> Signed-off-by: Sudip Mukherjee <sudip.mukherjee@codethink.co.uk>

...

> +static int default_setup(struct exar8250 *priv, struct pci_dev *pcidev,
> +			 int idx, unsigned int offset,
> +			 struct uart_8250_port *port)
> +{
> +	const struct exar8250_board *board = priv->board;
> +	unsigned int bar = 0;
> +
> +	port->port.iotype = UPIO_MEM;
> +	port->port.mapbase = pci_resource_start(pcidev, bar) + offset;
> +	port->port.membase = pcim_iomap_table(pcidev)[bar] + offset;

This always gives you 0 for membase because you missed to call
pcim_iomap for bar 0.

Sorry to pick on this piece-wise, but I just ran into this bug now while
porting patches over.

> +	port->port.regshift = board->reg_shift;
> +
> +	return 0;
> +}
> +
> +static int
> +pci_connect_tech_setup(struct exar8250 *priv, struct pci_dev *pcidev,
> +		       struct uart_8250_port *port, int idx)
> +{
> +	unsigned int offset = idx * 0x200;
> +	unsigned int baud = 1843200;
> +
> +	port->port.uartclk = baud * 16;
> +	return default_setup(priv, pcidev, idx, offset, port);
> +}
> +
> +static int
> +pci_xr17c154_setup(struct exar8250 *priv, struct pci_dev *pcidev,
> +		   struct uart_8250_port *port, int idx)
> +{
> +	unsigned int offset = idx * 0x200;
> +	unsigned int baud = 921600;
> +
> +	port->port.uartclk = baud * 16;
> +	return default_setup(priv, pcidev, idx, offset, port);
> +}
> +
> +static void setup_gpio(u8 __iomem *p)
> +{
> +	writeb(0x00, p + UART_EXAR_MPIOINT_7_0);
> +	writeb(0x00, p + UART_EXAR_MPIOLVL_7_0);
> +	writeb(0x00, p + UART_EXAR_MPIO3T_7_0);
> +	writeb(0x00, p + UART_EXAR_MPIOINV_7_0);
> +	writeb(0x00, p + UART_EXAR_MPIOSEL_7_0);
> +	writeb(0x00, p + UART_EXAR_MPIOOD_7_0);
> +	writeb(0x00, p + UART_EXAR_MPIOINT_15_8);
> +	writeb(0x00, p + UART_EXAR_MPIOLVL_15_8);
> +	writeb(0x00, p + UART_EXAR_MPIO3T_15_8);
> +	writeb(0x00, p + UART_EXAR_MPIOINV_15_8);
> +	writeb(0x00, p + UART_EXAR_MPIOSEL_15_8);
> +	writeb(0x00, p + UART_EXAR_MPIOOD_15_8);
> +}
> +
> +static void *
> +xr17v35x_register_gpio(struct pci_dev *pcidev)
> +{
> +	struct platform_device *pdev;
> +
> +	pdev = platform_device_alloc("gpio_exar", PLATFORM_DEVID_AUTO);
> +	if (!pdev)
> +		return NULL;
> +
> +	platform_set_drvdata(pdev, pcidev);
> +	if (platform_device_add(pdev) < 0) {
> +		platform_device_put(pdev);
> +		return NULL;
> +	}
> +
> +	return pdev;
> +}
> +
> +static int
> +pci_xr17v35x_setup(struct exar8250 *priv, struct pci_dev *pcidev,
> +		   struct uart_8250_port *port, int idx)
> +{
> +	const struct exar8250_board *board = priv->board;
> +	unsigned int offset = idx * 0x400;
> +	unsigned int baud = 7812500;
> +	u8 __iomem *p;
> +	int ret;
> +
> +	port->port.uartclk = baud * 16;
> +	/*
> +	 * Setup the uart clock for the devices on expansion slot to
> +	 * half the clock speed of the main chip (which is 125MHz)
> +	 */
> +	if (board->has_slave && idx >= 8)
> +		port->port.uartclk /= 2;
> +
> +	p = pci_ioremap_bar(pcidev, 0);

If we move the default_setup before this, we can use pcim_iomap_table()
and avoid this temporary mapping completely.

> +	if (!p)
> +		return -ENOMEM;
> +
> +	/* Setup Multipurpose Input/Output pins. */
> +	if (idx == 0)
> +		setup_gpio(p);
> +
> +	writeb(0x00, p + UART_EXAR_8XMODE);
> +	writeb(UART_FCTR_EXAR_TRGD, p + UART_EXAR_FCTR);
> +	writeb(128, p + UART_EXAR_TXTRG);
> +	writeb(128, p + UART_EXAR_RXTRG);
> +	iounmap(p);
> +
> +	ret = default_setup(priv, pcidev, idx, offset, port);
> +	if (ret)
> +		return ret;
> +
> +	if (idx == 0)
> +		port->port.private_data =
> +			xr17v35x_register_gpio(pcidev);
> +
> +	return 0;
> +}

I suppose I should still send patches on top, right?

Jan

-- 
Siemens AG, Corporate Technology, CT RDA ITP SES-DE
Corporate Competence Center Embedded Linux

Re: [PATCH v13 1/2] serial: exar: split out the exar code from 8250_pci

[Jan Kiszka]

On 2017-02-06 20:37, Jan Kiszka wrote:
> On 2017-01-30 23:28, Sudip Mukherjee wrote:
>> From: Sudip Mukherjee <sudip.mukherjee@codethink.co.uk>
>>
>> Add the serial driver for the Exar chips. And also register the
>> platform device for the GPIO provided by the Exar chips.
>>
>> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
>> Signed-off-by: Sudip Mukherjee <sudip.mukherjee@codethink.co.uk>
> 
> ...
> 
>> +static int default_setup(struct exar8250 *priv, struct pci_dev *pcidev,
>> +			 int idx, unsigned int offset,
>> +			 struct uart_8250_port *port)
>> +{
>> +	const struct exar8250_board *board = priv->board;
>> +	unsigned int bar = 0;
>> +
>> +	port->port.iotype = UPIO_MEM;
>> +	port->port.mapbase = pci_resource_start(pcidev, bar) + offset;
>> +	port->port.membase = pcim_iomap_table(pcidev)[bar] + offset;
> 
> This always gives you 0 for membase because you missed to call
> pcim_iomap for bar 0.
> 
> Sorry to pick on this piece-wise, but I just ran into this bug now while
> porting patches over.
> 
>> +	port->port.regshift = board->reg_shift;
>> +
>> +	return 0;
>> +}
>> +
>> +static int
>> +pci_connect_tech_setup(struct exar8250 *priv, struct pci_dev *pcidev,
>> +		       struct uart_8250_port *port, int idx)
>> +{
>> +	unsigned int offset = idx * 0x200;
>> +	unsigned int baud = 1843200;
>> +
>> +	port->port.uartclk = baud * 16;
>> +	return default_setup(priv, pcidev, idx, offset, port);
>> +}
>> +
>> +static int
>> +pci_xr17c154_setup(struct exar8250 *priv, struct pci_dev *pcidev,
>> +		   struct uart_8250_port *port, int idx)
>> +{
>> +	unsigned int offset = idx * 0x200;
>> +	unsigned int baud = 921600;
>> +
>> +	port->port.uartclk = baud * 16;
>> +	return default_setup(priv, pcidev, idx, offset, port);
>> +}
>> +
>> +static void setup_gpio(u8 __iomem *p)
>> +{
>> +	writeb(0x00, p + UART_EXAR_MPIOINT_7_0);
>> +	writeb(0x00, p + UART_EXAR_MPIOLVL_7_0);
>> +	writeb(0x00, p + UART_EXAR_MPIO3T_7_0);
>> +	writeb(0x00, p + UART_EXAR_MPIOINV_7_0);
>> +	writeb(0x00, p + UART_EXAR_MPIOSEL_7_0);
>> +	writeb(0x00, p + UART_EXAR_MPIOOD_7_0);
>> +	writeb(0x00, p + UART_EXAR_MPIOINT_15_8);
>> +	writeb(0x00, p + UART_EXAR_MPIOLVL_15_8);
>> +	writeb(0x00, p + UART_EXAR_MPIO3T_15_8);
>> +	writeb(0x00, p + UART_EXAR_MPIOINV_15_8);
>> +	writeb(0x00, p + UART_EXAR_MPIOSEL_15_8);
>> +	writeb(0x00, p + UART_EXAR_MPIOOD_15_8);
>> +}
>> +
>> +static void *
>> +xr17v35x_register_gpio(struct pci_dev *pcidev)
>> +{
>> +	struct platform_device *pdev;
>> +
>> +	pdev = platform_device_alloc("gpio_exar", PLATFORM_DEVID_AUTO);
>> +	if (!pdev)
>> +		return NULL;
>> +
>> +	platform_set_drvdata(pdev, pcidev);
>> +	if (platform_device_add(pdev) < 0) {
>> +		platform_device_put(pdev);
>> +		return NULL;
>> +	}
>> +
>> +	return pdev;
>> +}
>> +
>> +static int
>> +pci_xr17v35x_setup(struct exar8250 *priv, struct pci_dev *pcidev,
>> +		   struct uart_8250_port *port, int idx)
>> +{
>> +	const struct exar8250_board *board = priv->board;
>> +	unsigned int offset = idx * 0x400;
>> +	unsigned int baud = 7812500;
>> +	u8 __iomem *p;
>> +	int ret;
>> +
>> +	port->port.uartclk = baud * 16;
>> +	/*
>> +	 * Setup the uart clock for the devices on expansion slot to
>> +	 * half the clock speed of the main chip (which is 125MHz)
>> +	 */
>> +	if (board->has_slave && idx >= 8)
>> +		port->port.uartclk /= 2;
>> +
>> +	p = pci_ioremap_bar(pcidev, 0);
> 
> If we move the default_setup before this, we can use pcim_iomap_table()
> and avoid this temporary mapping completely.

Actually, we should use port->port.membase here: The original code that
came from 8250_pci was broken already by always programming port 0,
irrespective of idx. Using membase, we will pick the right address for
the target port.

Jan

> 
>> +	if (!p)
>> +		return -ENOMEM;
>> +
>> +	/* Setup Multipurpose Input/Output pins. */
>> +	if (idx == 0)
>> +		setup_gpio(p);
>> +
>> +	writeb(0x00, p + UART_EXAR_8XMODE);
>> +	writeb(UART_FCTR_EXAR_TRGD, p + UART_EXAR_FCTR);
>> +	writeb(128, p + UART_EXAR_TXTRG);
>> +	writeb(128, p + UART_EXAR_RXTRG);
>> +	iounmap(p);
>> +
>> +	ret = default_setup(priv, pcidev, idx, offset, port);
>> +	if (ret)
>> +		return ret;
>> +
>> +	if (idx == 0)
>> +		port->port.private_data =
>> +			xr17v35x_register_gpio(pcidev);
>> +
>> +	return 0;
>> +}
> 
> I suppose I should still send patches on top, right?
> 
> Jan
> 

-- 
Siemens AG, Corporate Technology, CT RDA ITP SES-DE
Corporate Competence Center Embedded Linux

Re: [PATCH v13 1/2] serial: exar: split out the exar code from 8250_pci

[Sudip Mukherjee]

On Monday 06 February 2017 02:45 PM, Jan Kiszka wrote:
> On 2017-01-30 23:28, Sudip Mukherjee wrote:
>> From: Sudip Mukherjee <sudip.mukherjee@codethink.co.uk>
>>
>> Add the serial driver for the Exar chips. And also register the
>> platform device for the GPIO provided by the Exar chips.
>>
>
> And another question: you left pci_fastcom335_setup and related things
> untouched - did that code come later, or is it left in 8250_pci.c for a
> reason?

That was discussed.
Those are separate chips from different vendor and this patchset was 
specifically for Exar chips. So i suggested I will do it via separate patch.

Regards
Sudip

Re: [PATCH v13 1/2] serial: exar: split out the exar code from 8250_pci

[Sudip Mukherjee]

On Monday 06 February 2017 02:06 PM, Greg Kroah-Hartman wrote:
> On Mon, Feb 06, 2017 at 02:49:07PM +0100, Jan Kiszka wrote:
>> On 2017-02-03 22:31, Sudip Mukherjee wrote:
>>> On Friday 03 February 2017 02:02 PM, Jan Kiszka wrote:
>>>> BTW, are you personally the copyright holder or your employer Codethink?
>>>> Depends on your contractual situation, but the former is less common.
>>>
>>> Well, Codethink has nothing to do with this patch. This was a voluntary
>>> work started before I joined Codethink, but then I joined Codethink and
>>> found very little time to finish this. So finally now its done.
>>>
>>> https://lists.kernelnewbies.org/pipermail/kernelnewbies/2015-November/015372.html
>>>
>>
>> Hmm, why using your corporate email address then? This suggests a
>> different copyright situation.
>>
>> Funnily, I just received this question internally: How can you tell
>> apart if someone sends a personal contribution via his/her employer
>> account from someone contributing on behalf of a company, thus with that
>> company holding the rights? I argued that no one would do the former to
>> prevent wrong accounting, but you just proved a counterexample. :)
>
> There are numerous companies that do this, some create whole shell
> orginizations in order to "hide" their kernel contributions for various
> "interesting" reasons.
>
> Fun stuff.  I suggest having your internal people talk to your lawyers,
> they should know all about this (and if not, have those lawyers talk to
> the LF lawyers...)
>
> But that's not the issue here, we know Sudip :)

:)

Regards
Sudip

Re: [PATCH v13 1/2] serial: exar: split out the exar code from 8250_pci

[Sudip Mukherjee]

On Monday 06 February 2017 01:49 PM, Jan Kiszka wrote:
> On 2017-02-03 22:31, Sudip Mukherjee wrote:
>> On Friday 03 February 2017 02:02 PM, Jan Kiszka wrote:
>>> BTW, are you personally the copyright holder or your employer Codethink?
>>> Depends on your contractual situation, but the former is less common.
>>
>> Well, Codethink has nothing to do with this patch. This was a voluntary
>> work started before I joined Codethink, but then I joined Codethink and
>> found very little time to finish this. So finally now its done.
>>
>> https://lists.kernelnewbies.org/pipermail/kernelnewbies/2015-November/015372.html
>>
>
> Hmm, why using your corporate email address then? This suggests a
> different copyright situation.
>
> Funnily, I just received this question internally: How can you tell
> apart if someone sends a personal contribution via his/her employer
> account from someone contributing on behalf of a company, thus with that
> company holding the rights? I argued that no one would do the former to
> prevent wrong accounting, but you just proved a counterexample. :)

well, I have been doing it this way from the very first day I started 
contributing.


Regards
Sudip

Re: [PATCH v13 1/2] serial: exar: split out the exar code from 8250_pci

[Jan Kiszka]

On 2017-02-06 23:04, Sudip Mukherjee wrote:
> On Monday 06 February 2017 02:45 PM, Jan Kiszka wrote:
>> On 2017-01-30 23:28, Sudip Mukherjee wrote:
>>> From: Sudip Mukherjee <sudip.mukherjee@codethink.co.uk>
>>>
>>> Add the serial driver for the Exar chips. And also register the
>>> platform device for the GPIO provided by the Exar chips.
>>>
>>
>> And another question: you left pci_fastcom335_setup and related things
>> untouched - did that code come later, or is it left in 8250_pci.c for a
>> reason?
> 
> That was discussed.
> Those are separate chips from different vendor and this patchset was
> specifically for Exar chips. So i suggested I will do it via separate
> patch.

If they are from different vendors, why are they addressing Exar
registers? Seems more like they are just rebranded and should indeed be
moved as well.

Jan