[PATCH v7 0/3] add gpio support to exar

[PATCH v7 0/3] add gpio support to exar

[Sudip Mukherjee]

Exar XR17V352/354/358 chips have 16 multi-purpose inputs/outputs which
can be controlled using gpio interface.

v5 was sent in January, 2016 and after reviews it was suggested to
split the exar code out of 8250_pci and make its own driver.

For reference it is at https://patchwork.kernel.org/patch/8058311/

First split attempt was patch series v6.

regards
sudip

Sudip Mukherjee (3):
  gpio: exar: add gpio for exar cards
  serial: exar: split out the exar code from 8250_pci
  serial: 8250_pci: remove exar code

 drivers/gpio/Kconfig                |   7 +
 drivers/gpio/Makefile               |   1 +
 drivers/gpio/gpio-exar.c            | 238 +++++++++++++++++
 drivers/tty/serial/8250/8250_exar.c | 515 ++++++++++++++++++++++++++++++++++++
 drivers/tty/serial/8250/8250_pci.c  | 336 +----------------------
 drivers/tty/serial/8250/Kconfig     |   5 +
 drivers/tty/serial/8250/Makefile    |   1 +
 7 files changed, 770 insertions(+), 333 deletions(-)
 create mode 100644 drivers/gpio/gpio-exar.c
 create mode 100644 drivers/tty/serial/8250/8250_exar.c

-- 
1.9.1

[PATCH v7 1/3] gpio: exar: add gpio for exar cards

[Sudip Mukherjee]

Exar XR17V352/354/358 chips have 16 multi-purpose inputs/outputs which
can be controlled using gpio interface.

Add the gpio specific code.

Signed-off-by: Sudip Mukherjee <sudip.mukherjee@codethink.co.uk>
---
 drivers/gpio/Kconfig     |   7 ++
 drivers/gpio/Makefile    |   1 +
 drivers/gpio/gpio-exar.c | 238 +++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 246 insertions(+)
 create mode 100644 drivers/gpio/gpio-exar.c

diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index ed37e59..89033d6 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -185,6 +185,13 @@ config GPIO_ETRAXFS
 	help
 	  Say yes here to support the GPIO controller on Axis ETRAX FS SoCs.
 
+config GPIO_EXAR
+	tristate "Support for GPIO pins on XR17V352/354/358"
+	depends on SERIAL_8250_EXAR
+	help
+	  Selecting this option will enable handling of GPIO pins present
+	  on Exar XR17V352/354/358 chips.
+
 config GPIO_GE_FPGA
 	bool "GE FPGA based GPIO"
 	depends on GE_FPGA
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index d074c22..d4d44f8 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -44,6 +44,7 @@ obj-$(CONFIG_GPIO_DWAPB)	+= gpio-dwapb.o
 obj-$(CONFIG_GPIO_EM)		+= gpio-em.o
 obj-$(CONFIG_GPIO_EP93XX)	+= gpio-ep93xx.o
 obj-$(CONFIG_GPIO_ETRAXFS)	+= gpio-etraxfs.o
+obj-$(CONFIG_GPIO_EXAR)		+= gpio-exar.o
 obj-$(CONFIG_GPIO_F7188X)	+= gpio-f7188x.o
 obj-$(CONFIG_GPIO_GE_FPGA)	+= gpio-ge.o
 obj-$(CONFIG_GPIO_GPIO_MM)	+= gpio-gpio-mm.o
diff --git a/drivers/gpio/gpio-exar.c b/drivers/gpio/gpio-exar.c
new file mode 100644
index 0000000..b7d1363
--- /dev/null
+++ b/drivers/gpio/gpio-exar.c
@@ -0,0 +1,238 @@
+/*
+ * GPIO driver for Exar XR17V35X chip
+ *
+ * Copyright (C) 2015 Sudip Mukherjee <sudipm.mukherjee@gmail.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+#include <linux/device.h>
+#include <linux/gpio.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/pci.h>
+#include <linux/platform_device.h>
+
+#define EXAR_OFFSET_MPIOLVL_LO 0x90
+#define EXAR_OFFSET_MPIOSEL_LO 0x93
+#define EXAR_OFFSET_MPIOLVL_HI 0x96
+#define EXAR_OFFSET_MPIOSEL_HI 0x99
+
+#define DRIVER_NAME "gpio_exar"
+
+static LIST_HEAD(exar_list);
+static DEFINE_MUTEX(exar_list_mtx);
+DEFINE_IDA(ida_index);
+
+struct exar_gpio_chip {
+	struct gpio_chip gpio_chip;
+	struct pci_dev *pcidev;
+	struct mutex lock;
+	struct list_head list;
+	int index;
+	void __iomem *regs;
+	char name[16];
+};
+
+#define to_exar_chip(n) container_of(n, struct exar_gpio_chip, gpio_chip)
+
+static inline unsigned int read_exar_reg(struct exar_gpio_chip *chip,
+					 int offset)
+{
+	dev_dbg(chip->gpio_chip.parent, "regs=%p offset=%x\n",
+		chip->regs, offset);
+
+	return readb(chip->regs + offset);
+}
+
+static inline void write_exar_reg(struct exar_gpio_chip *chip, int offset,
+				  int value)
+{
+	dev_dbg(chip->gpio_chip.parent,
+		"regs=%p value=%x offset=%x\n", chip->regs, value,
+		offset);
+	writeb(value, chip->regs + offset);
+}
+
+static void exar_update(struct gpio_chip *chip, unsigned int reg, int val,
+			unsigned int offset)
+{
+	struct exar_gpio_chip *exar_gpio = to_exar_chip(chip);
+	int temp;
+
+	mutex_lock(&exar_gpio->lock);
+	temp = read_exar_reg(exar_gpio, reg);
+	temp &= ~(1 << offset);
+	temp |= val << offset;
+	write_exar_reg(exar_gpio, reg, temp);
+	mutex_unlock(&exar_gpio->lock);
+}
+
+static int exar_set_direction(struct gpio_chip *chip, int direction,
+			      unsigned int offset)
+{
+	if (offset < 8)
+		exar_update(chip, EXAR_OFFSET_MPIOSEL_LO, direction,
+			    offset);
+	else
+		exar_update(chip, EXAR_OFFSET_MPIOSEL_HI, direction,
+			    offset - 8);
+	return 0;
+}
+
+static int exar_direction_output(struct gpio_chip *chip, unsigned int offset,
+				 int value)
+{
+	return exar_set_direction(chip, 0, offset);
+}
+
+static int exar_direction_input(struct gpio_chip *chip, unsigned int offset)
+{
+	return exar_set_direction(chip, 1, offset);
+}
+
+static int exar_get(struct gpio_chip *chip, unsigned int reg)
+{
+	struct exar_gpio_chip *exar_gpio = to_exar_chip(chip);
+	int value;
+
+	mutex_lock(&exar_gpio->lock);
+	value = read_exar_reg(exar_gpio, reg);
+	mutex_unlock(&exar_gpio->lock);
+
+	return value;
+}
+
+static int exar_get_direction(struct gpio_chip *chip, unsigned int offset)
+{
+	int val;
+
+	if (offset < 8)
+		val = exar_get(chip, EXAR_OFFSET_MPIOSEL_LO) >> offset;
+	else
+		val = exar_get(chip, EXAR_OFFSET_MPIOSEL_HI) >>
+			       (offset - 8);
+
+	return val & 0x01;
+}
+
+static int exar_get_value(struct gpio_chip *chip, unsigned int offset)
+{
+	int val;
+
+	if (offset < 8)
+		val = exar_get(chip, EXAR_OFFSET_MPIOLVL_LO) >> offset;
+	else
+		val = exar_get(chip, EXAR_OFFSET_MPIOLVL_HI) >>
+			       (offset - 8);
+	return val & 0x01;
+}
+
+static void exar_set_value(struct gpio_chip *chip, unsigned int offset,
+			   int value)
+{
+	if (offset < 8)
+		exar_update(chip, EXAR_OFFSET_MPIOLVL_LO, value,
+			    offset);
+	else
+		exar_update(chip, EXAR_OFFSET_MPIOLVL_HI, value,
+			    offset - 8);
+}
+
+static int gpio_exar_probe(struct platform_device *pdev)
+{
+	struct pci_dev *dev = platform_get_drvdata(pdev);
+	struct exar_gpio_chip *exar_gpio;
+	void __iomem *p;
+	int index = 1;
+	int ret;
+
+	if (dev->vendor != PCI_VENDOR_ID_EXAR)
+		return -ENODEV;
+
+	p = pci_ioremap_bar(dev, 0);
+	if (!p)
+		return -ENOMEM;
+
+	exar_gpio = devm_kzalloc(&dev->dev, sizeof(*exar_gpio), GFP_KERNEL);
+	if (!exar_gpio)
+		return -ENOMEM;
+
+	mutex_init(&exar_gpio->lock);
+	INIT_LIST_HEAD(&exar_gpio->list);
+
+	index = ida_simple_get(&ida_index, 0, 0, GFP_KERNEL);
+	mutex_lock(&exar_list_mtx);
+
+	sprintf(exar_gpio->name, "exar_gpio%d", index);
+	exar_gpio->gpio_chip.label = exar_gpio->name;
+	exar_gpio->gpio_chip.parent = &dev->dev;
+	exar_gpio->gpio_chip.direction_output = exar_direction_output;
+	exar_gpio->gpio_chip.direction_input = exar_direction_input;
+	exar_gpio->gpio_chip.get_direction = exar_get_direction;
+	exar_gpio->gpio_chip.get = exar_get_value;
+	exar_gpio->gpio_chip.set = exar_set_value;
+	exar_gpio->gpio_chip.base = -1;
+	exar_gpio->gpio_chip.ngpio = 16;
+	exar_gpio->regs = p;
+	exar_gpio->index = index;
+	exar_gpio->pcidev = dev;
+
+	ret = gpiochip_add(&exar_gpio->gpio_chip);
+	if (ret)
+		goto err_destroy;
+
+	list_add_tail(&exar_gpio->list, &exar_list);
+	mutex_unlock(&exar_list_mtx);
+
+	return 0;
+
+err_destroy:
+	mutex_unlock(&exar_list_mtx);
+	mutex_destroy(&exar_gpio->lock);
+	return ret;
+}
+
+static int gpio_exar_remove(struct platform_device *pdev)
+{
+	struct exar_gpio_chip *exar_gpio, *exar_temp;
+	struct pci_dev *pcidev;
+	int index;
+
+	pcidev = platform_get_drvdata(pdev);
+
+	mutex_lock(&exar_list_mtx);
+	list_for_each_entry_safe(exar_gpio, exar_temp, &exar_list, list) {
+		if (exar_gpio->pcidev == pcidev) {
+			list_del(&exar_gpio->list);
+			break;
+		}
+	}
+	mutex_unlock(&exar_list_mtx);
+
+	index = exar_gpio->index;
+	gpiochip_remove(&exar_gpio->gpio_chip);
+	mutex_destroy(&exar_gpio->lock);
+	ida_simple_remove(&ida_index, index);
+
+	return 0;
+}
+
+static struct platform_driver gpio_exar_driver = {
+	.probe	= gpio_exar_probe,
+	.remove	= gpio_exar_remove,
+	.driver	= {
+		.name = DRIVER_NAME,
+	},
+};
+
+module_platform_driver(gpio_exar_driver);
+
+MODULE_ALIAS("platform:" DRIVER_NAME);
+MODULE_DESCRIPTION("Exar GPIO driver");
+MODULE_AUTHOR("Sudip Mukherjee <sudipm.mukherjee@gmail.com>");
+MODULE_LICENSE("GPL");
-- 
1.9.1

Re: [PATCH v7 1/3] gpio: exar: add gpio for exar cards

[Linus Walleij]

On Sun, Jan 8, 2017 at 12:57 AM, Sudip Mukherjee
<sudipm.mukherjee@gmail.com> wrote:

> Exar XR17V352/354/358 chips have 16 multi-purpose inputs/outputs which
> can be controlled using gpio interface.
>
> Add the gpio specific code.
>
> Signed-off-by: Sudip Mukherjee <sudip.mukherjee@codethink.co.uk>

Will I be able to merge this independently to the GPIO trees
once we are done with review? (Looks like so...)

> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt

Is this really useful?

> +#include <linux/device.h>
> +#include <linux/gpio.h>

No use:
#include <linux/gpio/driver.h>
ONLY

> +static LIST_HEAD(exar_list);
> +static DEFINE_MUTEX(exar_list_mtx);
> +DEFINE_IDA(ida_index);

What is this? A local list? I can understand the IDA index but in
general, follow the state container pattern instead:
Documentation/driver-model/design-patterns.txt

> +#define to_exar_chip(n) container_of(n, struct exar_gpio_chip, gpio_chip)

Don't do this, use
gpiochip_get_data()

> +static inline unsigned int read_exar_reg(struct exar_gpio_chip *chip,
> +                                        int offset)
> +{
> +       dev_dbg(chip->gpio_chip.parent, "regs=%p offset=%x\n",
> +               chip->regs, offset);
> +
> +       return readb(chip->regs + offset);
> +}
> +
> +static inline void write_exar_reg(struct exar_gpio_chip *chip, int offset,
> +                                 int value)
> +{
> +       dev_dbg(chip->gpio_chip.parent,
> +               "regs=%p value=%x offset=%x\n", chip->regs, value,
> +               offset);
> +       writeb(value, chip->regs + offset);
> +}


I don't see why these need their own accessor functions, just inline
the readb()/writeb() calls.

> +static void exar_update(struct gpio_chip *chip, unsigned int reg, int val,
> +                       unsigned int offset)
> +{
> +       struct exar_gpio_chip *exar_gpio = to_exar_chip(chip);

So just:

struct exar_gpio_chip *exar_gpio = gpiochip_get_data(chip);

> +       int temp;
> +
> +       mutex_lock(&exar_gpio->lock);
> +       temp = read_exar_reg(exar_gpio, reg);
> +       temp &= ~(1 << offset);
> +       temp |= val << offset;

Use:
#include <linux/bitops.h>

temp &= BIT(offset);
if (val)
  temp |= BIT(offset);

> +static int exar_get(struct gpio_chip *chip, unsigned int reg)
> +{
> +       struct exar_gpio_chip *exar_gpio = to_exar_chip(chip);
> +       int value;
> +
> +       mutex_lock(&exar_gpio->lock);
> +       value = read_exar_reg(exar_gpio, reg);
> +       mutex_unlock(&exar_gpio->lock);
> +
> +       return value;

Please use:

return !!value;

To clamp to bool.

> +static int exar_get_direction(struct gpio_chip *chip, unsigned int offset)
> +{
> +       int val;
> +
> +       if (offset < 8)
> +               val = exar_get(chip, EXAR_OFFSET_MPIOSEL_LO) >> offset;
> +       else
> +               val = exar_get(chip, EXAR_OFFSET_MPIOSEL_HI) >>
> +                              (offset - 8);
> +
> +       return val & 0x01;

Just

return !!val;

or something.

> +static int exar_get_value(struct gpio_chip *chip, unsigned int offset)
> +{
> +       int val;
> +
> +       if (offset < 8)
> +               val = exar_get(chip, EXAR_OFFSET_MPIOLVL_LO) >> offset;
> +       else
> +               val = exar_get(chip, EXAR_OFFSET_MPIOLVL_HI) >>
> +                              (offset - 8);
> +       return val & 0x01;
> +}

Dito.

> +static int gpio_exar_probe(struct platform_device *pdev)
> +{
> +       struct pci_dev *dev = platform_get_drvdata(pdev);
> +       struct exar_gpio_chip *exar_gpio;
> +       void __iomem *p;
> +       int index = 1;
> +       int ret;
> +
> +       if (dev->vendor != PCI_VENDOR_ID_EXAR)
> +               return -ENODEV;
> +
> +       p = pci_ioremap_bar(dev, 0);
> +       if (!p)
> +               return -ENOMEM;
> +
> +       exar_gpio = devm_kzalloc(&dev->dev, sizeof(*exar_gpio), GFP_KERNEL);
> +       if (!exar_gpio)
> +               return -ENOMEM;
> +
> +       mutex_init(&exar_gpio->lock);
> +       INIT_LIST_HEAD(&exar_gpio->list);
> +
> +       index = ida_simple_get(&ida_index, 0, 0, GFP_KERNEL);
> +       mutex_lock(&exar_list_mtx);

This looks overkill.

> +       sprintf(exar_gpio->name, "exar_gpio%d", index);
> +       exar_gpio->gpio_chip.label = exar_gpio->name;
> +       exar_gpio->gpio_chip.parent = &dev->dev;
> +       exar_gpio->gpio_chip.direction_output = exar_direction_output;
> +       exar_gpio->gpio_chip.direction_input = exar_direction_input;
> +       exar_gpio->gpio_chip.get_direction = exar_get_direction;
> +       exar_gpio->gpio_chip.get = exar_get_value;
> +       exar_gpio->gpio_chip.set = exar_set_value;
> +       exar_gpio->gpio_chip.base = -1;
> +       exar_gpio->gpio_chip.ngpio = 16;
> +       exar_gpio->regs = p;
> +       exar_gpio->index = index;
> +       exar_gpio->pcidev = dev;
> +
> +       ret = gpiochip_add(&exar_gpio->gpio_chip);

Use devm_gpiochip_add_data(dev, &exar_gpio->gpio_chip, exar_gpio)

So you can later use gpiochip_get_data()

> +static int gpio_exar_remove(struct platform_device *pdev)
> +{
> +       struct exar_gpio_chip *exar_gpio, *exar_temp;
> +       struct pci_dev *pcidev;
> +       int index;
> +
> +       pcidev = platform_get_drvdata(pdev);
> +
> +       mutex_lock(&exar_list_mtx);
> +       list_for_each_entry_safe(exar_gpio, exar_temp, &exar_list, list) {
> +               if (exar_gpio->pcidev == pcidev) {
> +                       list_del(&exar_gpio->list);
> +                       break;
> +               }
> +       }
> +       mutex_unlock(&exar_list_mtx);

Looks convoluted.

Set the platform_drvdata to exar_gpio instead and just free it.

Yours,
Linus Wallleij

Re: [PATCH v7 1/3] gpio: exar: add gpio for exar cards

[Sudip Mukherjee]

On Monday 09 January 2017 10:35 AM, Linus Walleij wrote:
> On Sun, Jan 8, 2017 at 12:57 AM, Sudip Mukherjee
> <sudipm.mukherjee@gmail.com> wrote:
>
>> Exar XR17V352/354/358 chips have 16 multi-purpose inputs/outputs which
>> can be controlled using gpio interface.
>>
>> Add the gpio specific code.
>>
>> Signed-off-by: Sudip Mukherjee <sudip.mukherjee@codethink.co.uk>
>
> Will I be able to merge this independently to the GPIO trees
> once we are done with review? (Looks like so...)

Yes, there should not be any dependency on the tty.

>
>> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>
> Is this really useful?

no, initially I used pr_*, but then that was converted to dev_*.

>
>> +#include <linux/device.h>
>> +#include <linux/gpio.h>
>
> No use:
> #include <linux/gpio/driver.h>
> ONLY
>
>> +static LIST_HEAD(exar_list);
>> +static DEFINE_MUTEX(exar_list_mtx);
>> +DEFINE_IDA(ida_index);
>
> What is this? A local list? I can understand the IDA index but in
> general, follow the state container pattern instead:
> Documentation/driver-model/design-patterns.txt

The local list is not doing anything now, after I have moved to using 
ida. But I will need the ida_index here to have the device number incase 
of multiple devices.

regards
sudip

[PATCH v7 2/3] serial: exar: split out the exar code from 8250_pci

[Sudip Mukherjee]

Add the serial driver for the exar chips. And also register the
platform device for the exar gpio.

Signed-off-by: Sudip Mukherjee <sudip.mukherjee@codethink.co.uk>
---

Hi Andy,
Headers, if arranged in alphabetical order, will give a build warning.
And thanks for revewing that v6. I think those were the worst patch I
have ever posted.

 drivers/tty/serial/8250/8250_exar.c | 515 ++++++++++++++++++++++++++++++++++++
 drivers/tty/serial/8250/Kconfig     |   5 +
 drivers/tty/serial/8250/Makefile    |   1 +
 3 files changed, 521 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..abfd803
--- /dev/null
+++ b/drivers/tty/serial/8250/8250_exar.c
@@ -0,0 +1,515 @@
+/*
+ *  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.
+ */
+
+#undef DEBUG
+#include <asm/byteorder.h>
+#include <linux/pci.h>
+#include <linux/8250_pci.h>
+#include <linux/bitops.h>
+#include <linux/delay.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/serial_core.h>
+#include <linux/serial_reg.h>
+#include <linux/slab.h>
+#include <linux/string.h>
+#include <linux/tty.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] */
+
+#define PCI_NUM_BAR_RESOURCES	6
+
+struct exar8250;
+
+struct exar8250_board {
+	unsigned int num_ports;
+	unsigned int base_baud;
+	unsigned int uart_offset; /* the space between channels */
+	/*
+	 * reg_shift:  describes how the UART registers are mapped
+	 * to PCI memory by the card.
+	 */
+	unsigned int reg_shift;
+	unsigned int first_offset;
+	int	(*setup)(struct exar8250 *,
+			 const struct exar8250_board *,
+			 struct uart_8250_port *, int);
+	void	(*exit)(struct pci_dev *dev);
+};
+
+struct exar8250 {
+	struct pci_dev		*dev;
+	unsigned int		nr;
+	struct exar8250_board	*board;
+	int			line[0];
+};
+
+static int default_setup(struct exar8250 *priv,
+			     const struct exar8250_board *board,
+			     struct uart_8250_port *port, int idx)
+{
+	unsigned int bar = 0, offset = board->first_offset, maxnr;
+	struct pci_dev *dev = priv->dev;
+
+	offset += idx * board->uart_offset;
+
+	maxnr = (pci_resource_len(dev, bar) - board->first_offset) >>
+		(board->reg_shift + 3);
+
+	if (idx >= maxnr)
+		return 1;
+
+	if (!pcim_iomap(dev, bar, 0) && !pcim_iomap_table(dev))
+		return -ENOMEM;
+
+	port->port.iotype = UPIO_MEM;
+	port->port.iobase = 0;
+	port->port.mapbase = pci_resource_start(dev, bar) + offset;
+	port->port.membase = pcim_iomap_table(dev)[bar] + offset;
+	port->port.regshift = board->reg_shift;
+
+	return 0;
+}
+
+static int
+pci_xr17c154_setup(struct exar8250 *priv,
+		   const struct exar8250_board *board,
+		  struct uart_8250_port *port, int idx)
+{
+	port->port.flags |= UPF_EXAR_EFR;
+	return default_setup(priv, board, port, idx);
+}
+
+static inline int
+xr17v35x_has_slave(struct exar8250 *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 exar8250 *priv,
+		   const struct exar8250_board *board,
+		  struct uart_8250_port *port, int idx)
+{
+	u8 __iomem *p;
+	int ret;
+
+	p = pci_ioremap_bar(priv->dev, 0);
+	if (!p)
+		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);
+
+	ret = default_setup(priv, board, port, idx);
+	if (ret)
+		return ret;
+
+	if (idx == 0) {
+		struct platform_device *device;
+
+		device = platform_device_alloc("gpio_exar",
+					       PLATFORM_DEVID_AUTO);
+		if (!device)
+			return -ENOMEM;
+
+		platform_set_drvdata(device, priv->dev);
+		if (platform_device_add(device) < 0) {
+			platform_device_put(device);
+			return -ENODEV;
+		}
+
+		port->port.private_data = device;
+	}
+
+	return 0;
+}
+
+static void pci_xr17v35x_exit(struct pci_dev *dev)
+{
+	struct exar8250 *priv = pci_get_drvdata(dev);
+	struct uart_8250_port *port = serial8250_get_port(priv->line[0]);
+	struct platform_device *pdev = port->port.private_data;
+
+	if (pdev) {
+		platform_device_unregister(pdev);
+		port->port.private_data = NULL;
+	}
+}
+
+static int
+exar_pci_probe(struct pci_dev *dev, const struct pci_device_id *ent)
+{
+	struct exar8250_board *board;
+	struct uart_8250_port uart;
+	struct exar8250 *priv;
+	int nr_ports, i;
+	int rc;
+
+	board = (struct exar8250_board *)ent->driver_data;
+
+	rc = pcim_enable_device(dev);
+	pci_save_state(dev);
+	if (rc)
+		return rc;
+
+	nr_ports = board->num_ports;
+
+	priv = kzalloc(sizeof(*priv) + sizeof(unsigned int) * nr_ports,
+		       GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	priv->dev = dev;
+	priv->board = board;
+
+	memset(&uart, 0, sizeof(uart));
+	uart.port.flags = UPF_SKIP_TEST | UPF_BOOT_AUTOCONF | UPF_SHARE_IRQ;
+	uart.port.uartclk = board->base_baud * 16;
+	uart.port.irq = dev->irq;
+	uart.port.dev = &dev->dev;
+
+	for (i = 0; i < nr_ports; i++) {
+		if (board->setup(priv, board, &uart, i))
+			break;
+
+		dev_dbg(&dev->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(&dev->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(dev, priv);
+	return 0;
+}
+
+static void exar_pci_remove(struct pci_dev *dev)
+{
+	int i;
+	struct exar8250 *priv = pci_get_drvdata(dev);
+
+	for (i = 0; i < priv->nr; i++)
+		serial8250_unregister_port(priv->line[i]);
+
+	if (priv->board->exit)
+		priv->board->exit(priv->dev);
+
+	kfree(priv);
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int exar_suspend(struct device *dev)
+{
+	int i;
+	struct pci_dev *pdev = to_pci_dev(dev);
+	struct exar8250 *priv = pci_get_drvdata(pdev);
+
+
+	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(priv->dev);
+
+	return 0;
+}
+
+static int exar_resume(struct device *dev)
+{
+	int i;
+	int err;
+	struct pci_dev *pdev = to_pci_dev(dev);
+	struct exar8250 *priv = pci_get_drvdata(pdev);
+
+	if (priv) {
+		/*
+		 * The device may have been disabled.  Re-enable it.
+		 */
+		err = pci_enable_device(pdev);
+		/* FIXME: We cannot simply error out here */
+		if (err)
+			dev_err(dev, "Unable to re-enable ports, trying to continue.\n");
+
+		for (i = 0; i < priv->nr; i++)
+			if (priv->line[i] >= 0)
+				serial8250_resume_port(priv->line[i]);
+	}
+	return 0;
+}
+#endif
+
+static SIMPLE_DEV_PM_OPS(exar_pci_pm, exar_suspend, exar_resume);
+
+static const struct exar8250_board pbn_b0_2_1843200_200 = {
+	.num_ports	= 2,
+	.base_baud	= 1843200,
+	.uart_offset	= 0x200,
+	.setup		= pci_xr17c154_setup
+};
+
+static const struct exar8250_board pbn_b0_4_1843200_200 = {
+	.num_ports	= 4,
+	.base_baud	= 1843200,
+	.uart_offset	= 0x200,
+	.setup		= pci_xr17c154_setup
+};
+
+static const struct exar8250_board pbn_b0_8_1843200_200 = {
+	.num_ports	= 8,
+	.base_baud	= 1843200,
+	.uart_offset	= 0x200,
+	.setup		= pci_xr17c154_setup,
+};
+
+static const struct exar8250_board pbn_exar_ibm_saturn = {
+	.num_ports	= 1,
+	.base_baud	= 921600,
+	.uart_offset	= 0x200,
+	.setup		= pci_xr17c154_setup,
+};
+
+static const struct exar8250_board pbn_exar_XR17C152 = {
+	.num_ports	= 2,
+	.base_baud	= 921600,
+	.uart_offset	= 0x200,
+	.setup		= pci_xr17c154_setup,
+};
+
+static const struct exar8250_board pbn_exar_XR17C154 = {
+	.num_ports	= 4,
+	.base_baud	= 921600,
+	.uart_offset	= 0x200,
+	.setup		= pci_xr17c154_setup,
+};
+
+static const struct exar8250_board pbn_exar_XR17C158 = {
+	.num_ports	= 8,
+	.base_baud	= 921600,
+	.uart_offset	= 0x200,
+	.setup		= pci_xr17c154_setup,
+};
+
+static const struct exar8250_board pbn_exar_XR17V352 = {
+	.num_ports	= 2,
+	.base_baud	= 7812500,
+	.uart_offset	= 0x400,
+	.setup		= pci_xr17v35x_setup,
+	.exit		= pci_xr17v35x_exit,
+};
+
+static const struct exar8250_board pbn_exar_XR17V354 = {
+	.num_ports	= 4,
+	.base_baud	= 7812500,
+	.uart_offset	= 0x400,
+	.setup		= pci_xr17v35x_setup,
+	.exit		= pci_xr17v35x_exit,
+};
+
+static const struct exar8250_board pbn_exar_XR17V358 = {
+	.num_ports	= 8,
+	.base_baud	= 7812500,
+	.uart_offset	= 0x400,
+	.setup		= pci_xr17v35x_setup,
+	.exit		= pci_xr17v35x_exit,
+};
+
+static const struct exar8250_board pbn_exar_XR17V4358 = {
+	.num_ports	= 12,
+	.base_baud	= 7812500,
+	.uart_offset	= 0x400,
+	.setup		= pci_xr17v35x_setup,
+	.exit		= pci_xr17v35x_exit,
+};
+
+static const struct exar8250_board pbn_exar_XR17V8358 = {
+	.num_ports	= 16,
+	.base_baud	= 7812500,
+	.uart_offset	= 0x400,
+	.setup		= pci_xr17v35x_setup,
+	.exit		= pci_xr17v35x_exit,
+};
+
+static struct pci_device_id exar_pci_tbl[] = {
+	{	PCI_DEVICE_SUB(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,
+		(kernel_ulong_t)&pbn_b0_2_1843200_200 },
+	{	PCI_DEVICE_SUB(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,
+		(kernel_ulong_t)&pbn_b0_4_1843200_200 },
+	{	PCI_DEVICE_SUB(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,
+		(kernel_ulong_t)&pbn_b0_8_1843200_200 },
+	{	PCI_DEVICE_SUB(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,
+		(kernel_ulong_t)&pbn_b0_2_1843200_200 },
+	{	PCI_DEVICE_SUB(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,
+		(kernel_ulong_t)&pbn_b0_4_1843200_200 },
+	{	PCI_DEVICE_SUB(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,
+		(kernel_ulong_t)&pbn_b0_8_1843200_200 },
+	{	PCI_DEVICE_SUB(PCI_VENDOR_ID_EXAR,
+		PCI_DEVICE_ID_EXAR_XR17C152,
+		PCI_SUBVENDOR_ID_CONNECT_TECH,
+		PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_2), 0, 0,
+		(kernel_ulong_t)&pbn_b0_2_1843200_200 },
+	{	PCI_DEVICE_SUB(PCI_VENDOR_ID_EXAR,
+		PCI_DEVICE_ID_EXAR_XR17C154,
+		PCI_SUBVENDOR_ID_CONNECT_TECH,
+		PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_4), 0, 0,
+		(kernel_ulong_t)&pbn_b0_4_1843200_200 },
+	{	PCI_DEVICE_SUB(PCI_VENDOR_ID_EXAR,
+		PCI_DEVICE_ID_EXAR_XR17C158,
+		PCI_SUBVENDOR_ID_CONNECT_TECH,
+		PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_8), 0, 0,
+		(kernel_ulong_t)&pbn_b0_8_1843200_200 },
+	{	PCI_DEVICE_SUB(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,
+		(kernel_ulong_t)&pbn_b0_2_1843200_200 },
+	{	PCI_DEVICE_SUB(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,
+		(kernel_ulong_t)&pbn_b0_4_1843200_200 },
+	{	PCI_DEVICE_SUB(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,
+		(kernel_ulong_t)&pbn_b0_8_1843200_200 },
+	{	PCI_DEVICE_SUB(PCI_VENDOR_ID_EXAR,
+		PCI_DEVICE_ID_EXAR_XR17C152,
+		PCI_VENDOR_ID_IBM,
+		PCI_SUBDEVICE_ID_IBM_SATURN_SERIAL_ONE_PORT), 0, 0,
+		(kernel_ulong_t)&pbn_exar_ibm_saturn },
+	/*
+	 * Exar Corp. XR17C15[248] Dual/Quad/Octal UART
+	 */
+	{	PCI_VDEVICE(EXAR, PCI_DEVICE_ID_EXAR_XR17C152),
+		(kernel_ulong_t)&pbn_exar_XR17C152 },
+	{	PCI_VDEVICE(EXAR, PCI_DEVICE_ID_EXAR_XR17C154),
+		(kernel_ulong_t)&pbn_exar_XR17C154 },
+	{	PCI_VDEVICE(EXAR, PCI_DEVICE_ID_EXAR_XR17C158),
+		(kernel_ulong_t)&pbn_exar_XR17C158 },
+	/*
+	 * Exar Corp. XR17V[48]35[248] Dual/Quad/Octal/Hexa PCIe UARTs
+	 */
+	{	PCI_VDEVICE(EXAR, PCI_DEVICE_ID_EXAR_XR17V352),
+		(kernel_ulong_t)&pbn_exar_XR17V352 },
+	{	PCI_VDEVICE(EXAR, PCI_DEVICE_ID_EXAR_XR17V354),
+		(kernel_ulong_t)&pbn_exar_XR17V354 },
+	{	PCI_VDEVICE(EXAR, PCI_DEVICE_ID_EXAR_XR17V358),
+		(kernel_ulong_t)&pbn_exar_XR17V358 },
+	{	PCI_VDEVICE(EXAR, PCI_DEVICE_ID_EXAR_XR17V4358),
+		(kernel_ulong_t)&pbn_exar_XR17V4358 },
+	{	PCI_VDEVICE(EXAR, PCI_DEVICE_ID_EXAR_XR17V8358),
+		(kernel_ulong_t)&pbn_exar_XR17V8358 },
+	{	PCI_VDEVICE(COMMTECH, PCI_DEVICE_ID_COMMTECH_4222PCIE),
+		(kernel_ulong_t)&pbn_exar_XR17V352 },
+	{	PCI_VDEVICE(COMMTECH, PCI_DEVICE_ID_COMMTECH_4224PCIE),
+		(kernel_ulong_t)&pbn_exar_XR17V354 },
+	{	PCI_VDEVICE(COMMTECH, PCI_DEVICE_ID_COMMTECH_4228PCIE),
+		(kernel_ulong_t)&pbn_exar_XR17V358 },
+	{ 0, }
+};
+
+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_DEVICE_TABLE(pci, exar_pci_tbl);
diff --git a/drivers/tty/serial/8250/Kconfig b/drivers/tty/serial/8250/Kconfig
index 8998347..9f8bd0a 100644
--- a/drivers/tty/serial/8250/Kconfig
+++ b/drivers/tty/serial/8250/Kconfig
@@ -127,6 +127,11 @@ 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
+	default SERIAL_8250
+
 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 276c6fb..b771d37 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

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

[Andy Shevchenko]

On Sun, Jan 8, 2017 at 1:57 AM, Sudip Mukherjee
<sudipm.mukherjee@gmail.com> wrote:
> Add the serial driver for the exar chips. And also register the
> platform device for the exar gpio.

Did you ignore some comments?

IIRC I recommended to use proper vendor name like Exar (or how is it spelled?).

> Headers, if arranged in alphabetical order, will give a build warning.

I think I know how to make it better.

> And thanks for revewing that v6. I think those were the worst patch I
> have ever posted.

You may do even more better. See below.

> +#undef DEBUG

> +#include <asm/byteorder.h>

(1)

> +#include <linux/pci.h>

Squeez this to the rest

> +#include <linux/8250_pci.h>

(2)

> +#include <linux/bitops.h>
> +#include <linux/delay.h>
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/serial_core.h>
> +#include <linux/serial_reg.h>
> +#include <linux/slab.h>
> +#include <linux/string.h>
> +#include <linux/tty.h>

You perhaps need something like this here:

+ empty line
(1) +#include <asm/byteorder.h>

> +

(2) +#include <linux/8250_pci.h>

> +#include "8250.h"

> +
> +#define PCI_NUM_BAR_RESOURCES  6
> +
> +struct exar8250;


> +
> +struct exar8250_board {
> +       unsigned int num_ports;
> +       unsigned int base_baud;
> +       unsigned int uart_offset; /* the space between channels */
> +       /*
> +        * reg_shift:  describes how the UART registers are mapped
> +        * to PCI memory by the card.
> +        */
> +       unsigned int reg_shift;
> +       unsigned int first_offset;
> +       int     (*setup)(struct exar8250 *,
> +                        const struct exar8250_board *,
> +                        struct uart_8250_port *, int);
> +       void    (*exit)(struct pci_dev *dev);
> +};
> +
> +struct exar8250 {

> +       struct pci_dev          *dev;

You perhaps don't need to save parent device, thus

struct device *dev;

here and users will do

struct pci_dev *pci_dev = to_pci_dev(dev);

when needed.

> +       unsigned int            nr;
> +       struct exar8250_board   *board;
> +       int                     line[0];
> +};
> +
> +static int default_setup(struct exar8250 *priv,
> +                            const struct exar8250_board *board,
> +                            struct uart_8250_port *port, int idx)
> +{
> +       unsigned int bar = 0, offset = board->first_offset, maxnr;
> +       struct pci_dev *dev = priv->dev;
> +
> +       offset += idx * board->uart_offset;
> +

> +       maxnr = (pci_resource_len(dev, bar) - board->first_offset) >>
> +               (board->reg_shift + 3);

Can be calculated once?

> +
> +       if (idx >= maxnr)
> +               return 1;
> +

> +       if (!pcim_iomap(dev, bar, 0) && !pcim_iomap_table(dev))
> +               return -ENOMEM;

Do you need to check this per port? In probe you would know this.

> +
> +       port->port.iotype = UPIO_MEM;
> +       port->port.iobase = 0;
> +       port->port.mapbase = pci_resource_start(dev, bar) + offset;
> +       port->port.membase = pcim_iomap_table(dev)[bar] + offset;
> +       port->port.regshift = board->reg_shift;
> +
> +       return 0;
> +}
> +
> +static int
> +pci_xr17c154_setup(struct exar8250 *priv,
> +                  const struct exar8250_board *board,
> +                 struct uart_8250_port *port, int idx)
> +{
> +       port->port.flags |= UPF_EXAR_EFR;
> +       return default_setup(priv, board, port, idx);
> +}
> +

> +static inline int
> +xr17v35x_has_slave(struct exar8250 *priv)
> +{
> +       const int dev_id = priv->dev->device;
> +
> +       return ((dev_id == PCI_DEVICE_ID_EXAR_XR17V4358) ||
> +               (dev_id == PCI_DEVICE_ID_EXAR_XR17V8358));
> +}

Can be easily converted to
unsigned int flags;
in your custom struct, and be assigned constantly based on ID.

> +
> +static int
> +pci_xr17v35x_setup(struct exar8250 *priv,
> +                  const struct exar8250_board *board,
> +                 struct uart_8250_port *port, int idx)
> +{
> +       u8 __iomem *p;
> +       int ret;
> +
> +       p = pci_ioremap_bar(priv->dev, 0);
> +       if (!p)
> +               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);
> +       }

I would move it to some helper function.

> +       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, board, port, idx);
> +       if (ret)
> +               return ret;
> +
> +       if (idx == 0) {

> +               struct platform_device *device;

Be consistent.
*pdev;

> +
> +               device = platform_device_alloc("gpio_exar",
> +                                              PLATFORM_DEVID_AUTO);
> +               if (!device)
> +                       return -ENOMEM;
> +
> +               platform_set_drvdata(device, priv->dev);
> +               if (platform_device_add(device) < 0) {
> +                       platform_device_put(device);
> +                       return -ENODEV;
> +               }
> +
> +               port->port.private_data = device;

Either this to move to another helper.

xr17v35x_register_gpio();

> +       }
> +
> +       return 0;
> +}
> +
> +static void pci_xr17v35x_exit(struct pci_dev *dev)
> +{
> +       struct exar8250 *priv = pci_get_drvdata(dev);
> +       struct uart_8250_port *port = serial8250_get_port(priv->line[0]);
> +       struct platform_device *pdev = port->port.private_data;
> +
> +       if (pdev) {
> +               platform_device_unregister(pdev);
> +               port->port.private_data = NULL;
> +       }
> +}
> +
> +static int
> +exar_pci_probe(struct pci_dev *dev, const struct pci_device_id *ent)
> +{
> +       struct exar8250_board *board;
> +       struct uart_8250_port uart;
> +       struct exar8250 *priv;
> +       int nr_ports, i;
> +       int rc;
> +
> +       board = (struct exar8250_board *)ent->driver_data;
> +
> +       rc = pcim_enable_device(dev);

> +       pci_save_state(dev);

What for?

> +       if (rc)
> +               return rc;
> +
> +       nr_ports = board->num_ports;
> +
> +       priv = kzalloc(sizeof(*priv) + sizeof(unsigned int) * nr_ports,
> +                      GFP_KERNEL);

devm_kzalloc();

> +       if (!priv)
> +               return -ENOMEM;
> +
> +       priv->dev = dev;
> +       priv->board = board;
> +
> +       memset(&uart, 0, sizeof(uart));
> +       uart.port.flags = UPF_SKIP_TEST | UPF_BOOT_AUTOCONF | UPF_SHARE_IRQ;
> +       uart.port.uartclk = board->base_baud * 16;
> +       uart.port.irq = dev->irq;
> +       uart.port.dev = &dev->dev;
> +
> +       for (i = 0; i < nr_ports; i++) {
> +               if (board->setup(priv, board, &uart, i))
> +                       break;
> +
> +               dev_dbg(&dev->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(&dev->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(dev, priv);
> +       return 0;
> +}
> +
> +static void exar_pci_remove(struct pci_dev *dev)
> +{
> +       int i;
> +       struct exar8250 *priv = pci_get_drvdata(dev);
> +
> +       for (i = 0; i < priv->nr; i++)
> +               serial8250_unregister_port(priv->line[i]);
> +
> +       if (priv->board->exit)
> +               priv->board->exit(priv->dev);
> +
> +       kfree(priv);
> +}
> +
> +#ifdef CONFIG_PM_SLEEP
> +static int exar_suspend(struct device *dev)
> +{

> +       int i;

Move it below...

> +       struct pci_dev *pdev = to_pci_dev(dev);
> +       struct exar8250 *priv = pci_get_drvdata(pdev);

...here
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(priv->dev);
> +
> +       return 0;
> +}
> +
> +static int exar_resume(struct device *dev)
> +{
> +       int i;
> +       int err;
> +       struct pci_dev *pdev = to_pci_dev(dev);
> +       struct exar8250 *priv = pci_get_drvdata(pdev);

Ditto.

> +
> +       if (priv) {

> +               /*
> +                * The device may have been disabled.  Re-enable it.
> +                */
> +               err = pci_enable_device(pdev);
> +               /* FIXME: We cannot simply error out here */
> +               if (err)
> +                       dev_err(dev, "Unable to re-enable ports, trying to continue.\n");

Do you really need this?

> +
> +               for (i = 0; i < priv->nr; i++)
> +                       if (priv->line[i] >= 0)
> +                               serial8250_resume_port(priv->line[i]);
> +       }
> +       return 0;
> +}
> +#endif
> +
> +static SIMPLE_DEV_PM_OPS(exar_pci_pm, exar_suspend, exar_resume);
> +
> +static const struct exar8250_board pbn_b0_2_1843200_200 = {
> +       .num_ports      = 2,
> +       .base_baud      = 1843200,
> +       .uart_offset    = 0x200,
> +       .setup          = pci_xr17c154_setup
> +};
> +
> +static const struct exar8250_board pbn_b0_4_1843200_200 = {
> +       .num_ports      = 4,
> +       .base_baud      = 1843200,
> +       .uart_offset    = 0x200,
> +       .setup          = pci_xr17c154_setup
> +};
> +
> +static const struct exar8250_board pbn_b0_8_1843200_200 = {
> +       .num_ports      = 8,
> +       .base_baud      = 1843200,
> +       .uart_offset    = 0x200,
> +       .setup          = pci_xr17c154_setup,
> +};
> +
> +static const struct exar8250_board pbn_exar_ibm_saturn = {
> +       .num_ports      = 1,
> +       .base_baud      = 921600,
> +       .uart_offset    = 0x200,
> +       .setup          = pci_xr17c154_setup,
> +};
> +
> +static const struct exar8250_board pbn_exar_XR17C152 = {
> +       .num_ports      = 2,
> +       .base_baud      = 921600,
> +       .uart_offset    = 0x200,
> +       .setup          = pci_xr17c154_setup,
> +};
> +
> +static const struct exar8250_board pbn_exar_XR17C154 = {
> +       .num_ports      = 4,
> +       .base_baud      = 921600,
> +       .uart_offset    = 0x200,
> +       .setup          = pci_xr17c154_setup,
> +};
> +
> +static const struct exar8250_board pbn_exar_XR17C158 = {
> +       .num_ports      = 8,
> +       .base_baud      = 921600,
> +       .uart_offset    = 0x200,
> +       .setup          = pci_xr17c154_setup,
> +};
> +
> +static const struct exar8250_board pbn_exar_XR17V352 = {
> +       .num_ports      = 2,
> +       .base_baud      = 7812500,
> +       .uart_offset    = 0x400,
> +       .setup          = pci_xr17v35x_setup,
> +       .exit           = pci_xr17v35x_exit,
> +};
> +
> +static const struct exar8250_board pbn_exar_XR17V354 = {
> +       .num_ports      = 4,
> +       .base_baud      = 7812500,
> +       .uart_offset    = 0x400,
> +       .setup          = pci_xr17v35x_setup,
> +       .exit           = pci_xr17v35x_exit,
> +};
> +
> +static const struct exar8250_board pbn_exar_XR17V358 = {
> +       .num_ports      = 8,
> +       .base_baud      = 7812500,
> +       .uart_offset    = 0x400,
> +       .setup          = pci_xr17v35x_setup,
> +       .exit           = pci_xr17v35x_exit,
> +};
> +
> +static const struct exar8250_board pbn_exar_XR17V4358 = {
> +       .num_ports      = 12,
> +       .base_baud      = 7812500,
> +       .uart_offset    = 0x400,
> +       .setup          = pci_xr17v35x_setup,
> +       .exit           = pci_xr17v35x_exit,
> +};
> +
> +static const struct exar8250_board pbn_exar_XR17V8358 = {
> +       .num_ports      = 16,
> +       .base_baud      = 7812500,
> +       .uart_offset    = 0x400,
> +       .setup          = pci_xr17v35x_setup,
> +       .exit           = pci_xr17v35x_exit,
> +};
> +
> +static struct pci_device_id exar_pci_tbl[] = {
> +       {       PCI_DEVICE_SUB(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,
> +               (kernel_ulong_t)&pbn_b0_2_1843200_200 },

You ignored my comment regarding to make a macro(s).

Moreover, some of data, like number of ports, can be easily calculated
from device ID.

Try smarter, you may reduce amount of lines here at least twice! And
don't ignore my comments.

> +       {       PCI_DEVICE_SUB(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,
> +               (kernel_ulong_t)&pbn_b0_4_1843200_200 },
> +       {       PCI_DEVICE_SUB(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,
> +               (kernel_ulong_t)&pbn_b0_8_1843200_200 },
> +       {       PCI_DEVICE_SUB(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,
> +               (kernel_ulong_t)&pbn_b0_2_1843200_200 },
> +       {       PCI_DEVICE_SUB(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,
> +               (kernel_ulong_t)&pbn_b0_4_1843200_200 },
> +       {       PCI_DEVICE_SUB(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,
> +               (kernel_ulong_t)&pbn_b0_8_1843200_200 },
> +       {       PCI_DEVICE_SUB(PCI_VENDOR_ID_EXAR,
> +               PCI_DEVICE_ID_EXAR_XR17C152,
> +               PCI_SUBVENDOR_ID_CONNECT_TECH,
> +               PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_2), 0, 0,
> +               (kernel_ulong_t)&pbn_b0_2_1843200_200 },
> +       {       PCI_DEVICE_SUB(PCI_VENDOR_ID_EXAR,
> +               PCI_DEVICE_ID_EXAR_XR17C154,
> +               PCI_SUBVENDOR_ID_CONNECT_TECH,
> +               PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_4), 0, 0,
> +               (kernel_ulong_t)&pbn_b0_4_1843200_200 },
> +       {       PCI_DEVICE_SUB(PCI_VENDOR_ID_EXAR,
> +               PCI_DEVICE_ID_EXAR_XR17C158,
> +               PCI_SUBVENDOR_ID_CONNECT_TECH,
> +               PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_8), 0, 0,
> +               (kernel_ulong_t)&pbn_b0_8_1843200_200 },
> +       {       PCI_DEVICE_SUB(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,
> +               (kernel_ulong_t)&pbn_b0_2_1843200_200 },
> +       {       PCI_DEVICE_SUB(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,
> +               (kernel_ulong_t)&pbn_b0_4_1843200_200 },
> +       {       PCI_DEVICE_SUB(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,
> +               (kernel_ulong_t)&pbn_b0_8_1843200_200 },
> +       {       PCI_DEVICE_SUB(PCI_VENDOR_ID_EXAR,
> +               PCI_DEVICE_ID_EXAR_XR17C152,
> +               PCI_VENDOR_ID_IBM,
> +               PCI_SUBDEVICE_ID_IBM_SATURN_SERIAL_ONE_PORT), 0, 0,
> +               (kernel_ulong_t)&pbn_exar_ibm_saturn },
> +       /*
> +        * Exar Corp. XR17C15[248] Dual/Quad/Octal UART
> +        */
> +       {       PCI_VDEVICE(EXAR, PCI_DEVICE_ID_EXAR_XR17C152),
> +               (kernel_ulong_t)&pbn_exar_XR17C152 },
> +       {       PCI_VDEVICE(EXAR, PCI_DEVICE_ID_EXAR_XR17C154),
> +               (kernel_ulong_t)&pbn_exar_XR17C154 },
> +       {       PCI_VDEVICE(EXAR, PCI_DEVICE_ID_EXAR_XR17C158),
> +               (kernel_ulong_t)&pbn_exar_XR17C158 },
> +       /*
> +        * Exar Corp. XR17V[48]35[248] Dual/Quad/Octal/Hexa PCIe UARTs
> +        */
> +       {       PCI_VDEVICE(EXAR, PCI_DEVICE_ID_EXAR_XR17V352),
> +               (kernel_ulong_t)&pbn_exar_XR17V352 },
> +       {       PCI_VDEVICE(EXAR, PCI_DEVICE_ID_EXAR_XR17V354),
> +               (kernel_ulong_t)&pbn_exar_XR17V354 },
> +       {       PCI_VDEVICE(EXAR, PCI_DEVICE_ID_EXAR_XR17V358),
> +               (kernel_ulong_t)&pbn_exar_XR17V358 },
> +       {       PCI_VDEVICE(EXAR, PCI_DEVICE_ID_EXAR_XR17V4358),
> +               (kernel_ulong_t)&pbn_exar_XR17V4358 },
> +       {       PCI_VDEVICE(EXAR, PCI_DEVICE_ID_EXAR_XR17V8358),
> +               (kernel_ulong_t)&pbn_exar_XR17V8358 },
> +       {       PCI_VDEVICE(COMMTECH, PCI_DEVICE_ID_COMMTECH_4222PCIE),
> +               (kernel_ulong_t)&pbn_exar_XR17V352 },
> +       {       PCI_VDEVICE(COMMTECH, PCI_DEVICE_ID_COMMTECH_4224PCIE),
> +               (kernel_ulong_t)&pbn_exar_XR17V354 },
> +       {       PCI_VDEVICE(COMMTECH, PCI_DEVICE_ID_COMMTECH_4228PCIE),
> +               (kernel_ulong_t)&pbn_exar_XR17V358 },
> +       { 0, }
> +};
> +
> +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_DEVICE_TABLE(pci, exar_pci_tbl);
> diff --git a/drivers/tty/serial/8250/Kconfig b/drivers/tty/serial/8250/Kconfig
> index 8998347..9f8bd0a 100644
> --- a/drivers/tty/serial/8250/Kconfig
> +++ b/drivers/tty/serial/8250/Kconfig
> @@ -127,6 +127,11 @@ 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
> +       default SERIAL_8250
> +
>  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 276c6fb..b771d37 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
>



-- 
With Best Regards,
Andy Shevchenko

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

[Sudip Mukherjee]

On Sunday 08 January 2017 01:02 AM, Andy Shevchenko wrote:
> On Sun, Jan 8, 2017 at 1:57 AM, Sudip Mukherjee
> <sudipm.mukherjee@gmail.com> wrote:
>> Add the serial driver for the exar chips. And also register the
>> platform device for the exar gpio.
>
> Did you ignore some comments?
>
> IIRC I recommended to use proper vendor name like Exar (or how is it spelled?).

oops, sorry. I missed that.

>
>> Headers, if arranged in alphabetical order, will give a build warning.
>
> I think I know how to make it better.
>
>> And thanks for revewing that v6. I think those were the worst patch I
>> have ever posted.
>
> You may do even more better. See below.
>
>> +#undef DEBUG
>
>> +#include <asm/byteorder.h>
>
> (1)
>
>> +#include <linux/pci.h>
>
> Squeez this to the rest
>
>> +#include <linux/8250_pci.h>
>
> (2)
>
>> +#include <linux/bitops.h>
>> +#include <linux/delay.h>
>> +#include <linux/io.h>
>> +#include <linux/kernel.h>
>> +#include <linux/module.h>
>> +#include <linux/serial_core.h>
>> +#include <linux/serial_reg.h>
>> +#include <linux/slab.h>
>> +#include <linux/string.h>
>> +#include <linux/tty.h>
>
> You perhaps need something like this here:
>
> + empty line
> (1) +#include <asm/byteorder.h>
>
>> +
>
> (2) +#include <linux/8250_pci.h>
>
>> +#include "8250.h"

not sure if I have understood this header thing properly. But let me 
play with it and see,

>
>> +
>> +#define PCI_NUM_BAR_RESOURCES  6
<snip>
>> +static struct pci_device_id exar_pci_tbl[] = {
>> +       {       PCI_DEVICE_SUB(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,
>> +               (kernel_ulong_t)&pbn_b0_2_1843200_200 },
>
> You ignored my comment regarding to make a macro(s).

I used PCI_DEVICE_SUB() and PCI_VDEVICE(), but yes, custom macro might 
be better here. I was trying to have one custom macro, but with two 
different macros it should be ok.

>
> Moreover, some of data, like number of ports, can be easily calculated
> from device ID.

yes, but since the baudrate is different i will need to have different 
board id for it.
Like: 'PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_UART_4_232' has a device id 
'PCI_DEVICE_ID_EXAR_XR17C154' is having a baudrate of 1843200 but the 
other devices with the same deviceid will have a baudrate of 921600.

unless, in the probe I compare the subvendor with 
PCI_SUBVENDOR_ID_CONNECT_TECH and modify the baud. Let me try.

Thanks for reviewing.

Regards
Sudip

[PATCH v7 3/3] serial: 8250_pci: remove exar code

[Sudip Mukherjee]

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

Signed-off-by: Sudip Mukherjee <sudip.mukherjee@codethink.co.uk>
---
 drivers/tty/serial/8250/8250_pci.c | 336 +------------------------------------
 1 file changed, 3 insertions(+), 333 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_pci.c b/drivers/tty/serial/8250/8250_pci.c
index b98c157..8a5b740 100644
--- a/drivers/tty/serial/8250/8250_pci.c
+++ b/drivers/tty/serial/8250/8250_pci.c
@@ -1580,9 +1580,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] */
@@ -1595,71 +1592,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
@@ -1784,9 +1716,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
 
@@ -2238,65 +2167,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
 	 */
 	{
@@ -2552,27 +2422,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)
 	 */
@@ -2785,15 +2634,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,
@@ -3434,76 +3274,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] = {
@@ -3699,6 +3469,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), },
 };
 
 /*
@@ -4119,58 +3892,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 },
@@ -4898,45 +4619,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
 	 */
@@ -5531,18 +5213,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 },
-- 
1.9.1