linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v10 1/3] gpio: exar: add gpio for exar cards
@ 2017-01-19 22:23 Sudip Mukherjee
  2017-01-19 22:23 ` [PATCH v10 2/3] serial: exar: split out the exar code from 8250_pci Sudip Mukherjee
                   ` (3 more replies)
  0 siblings, 4 replies; 10+ messages in thread
From: Sudip Mukherjee @ 2017-01-19 22:23 UTC (permalink / raw)
  To: Linus Walleij, Alexandre Courbot, Greg Kroah-Hartman, Jiri Slaby,
	Andy Shevchenko, gnomes
  Cc: linux-kernel, linux-serial, linux-gpio

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

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 | 200 +++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 208 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..05c8946
--- /dev/null
+++ b/drivers/gpio/gpio-exar.c
@@ -0,0 +1,200 @@
+/*
+ * GPIO driver for Exar XR17V35X chip
+ *
+ * Copyright (C) 2015 Sudip Mukherjee <sudip.mukherjee@codethink.co.uk>
+ *
+ * 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.
+ */
+#include <linux/bitops.h>
+#include <linux/device.h>
+#include <linux/gpio/driver.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 DEFINE_IDA(ida_index);
+
+struct exar_gpio_chip {
+	struct gpio_chip gpio_chip;
+	struct mutex lock;
+	int index;
+	void __iomem *regs;
+	char name[20];
+};
+
+static void exar_update(struct gpio_chip *chip, unsigned int reg, int val,
+			unsigned int offset)
+{
+	struct exar_gpio_chip *exar_gpio = gpiochip_get_data(chip);
+	int temp;
+
+	mutex_lock(&exar_gpio->lock);
+	temp = readb(exar_gpio->regs + reg);
+	temp &= ~BIT(offset);
+	if (val)
+		temp |= BIT(offset);
+	writeb(temp, exar_gpio->regs + reg);
+	mutex_unlock(&exar_gpio->lock);
+}
+
+static int exar_set_direction(struct gpio_chip *chip, int direction,
+			      unsigned int offset)
+{
+	unsigned int bank = offset / 8;
+	unsigned int addr;
+
+	addr = bank ? EXAR_OFFSET_MPIOSEL_HI : EXAR_OFFSET_MPIOSEL_LO;
+	exar_update(chip, addr, 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 = gpiochip_get_data(chip);
+	int value;
+
+	mutex_lock(&exar_gpio->lock);
+	value = readb(exar_gpio->regs + reg);
+	mutex_unlock(&exar_gpio->lock);
+
+	return !!value;
+}
+
+static int exar_get_direction(struct gpio_chip *chip, unsigned int offset)
+{
+	unsigned int bank = offset / 8;
+	unsigned int addr;
+	int val;
+
+	addr = bank ? EXAR_OFFSET_MPIOSEL_HI : EXAR_OFFSET_MPIOSEL_LO;
+	val = exar_get(chip, addr) >> (offset % 8);
+
+	return !!val;
+}
+
+static int exar_get_value(struct gpio_chip *chip, unsigned int offset)
+{
+	unsigned int bank = offset / 8;
+	unsigned int addr;
+	int val;
+
+	addr = bank ? EXAR_OFFSET_MPIOLVL_LO : EXAR_OFFSET_MPIOLVL_HI;
+	val = exar_get(chip, addr) >> (offset % 8);
+
+	return !!val;
+}
+
+static void exar_set_value(struct gpio_chip *chip, unsigned int offset,
+			   int value)
+{
+	unsigned int bank = offset / 8;
+	unsigned int addr;
+
+	addr = bank ? EXAR_OFFSET_MPIOLVL_HI : EXAR_OFFSET_MPIOLVL_LO;
+	exar_update(chip, addr, value, offset % 8);
+}
+
+static int gpio_exar_probe(struct platform_device *pdev)
+{
+	struct pci_dev *pcidev = platform_get_drvdata(pdev);
+	struct exar_gpio_chip *exar_gpio;
+	void __iomem *p;
+	int index, ret;
+
+	if (pcidev->vendor != PCI_VENDOR_ID_EXAR)
+		return -ENODEV;
+
+	/*
+	 * Map the pci device to get the register addresses.
+	 * We will need to read and write those registers to control
+	 * the GPIO pins.
+	 * Using managed functions will save us from unmaping on exit.
+	 * As the device is enabled using managed functions by the
+	 * UART driver we can also use managed functions here.
+	 */
+	p = pcim_iomap(pcidev, 0, 0);
+	if (!p)
+		return -ENOMEM;
+
+	exar_gpio = devm_kzalloc(&pcidev->dev, sizeof(*exar_gpio), GFP_KERNEL);
+	if (!exar_gpio)
+		return -ENOMEM;
+
+	mutex_init(&exar_gpio->lock);
+
+	index = ida_simple_get(&ida_index, 0, 0, GFP_KERNEL);
+
+	sprintf(exar_gpio->name, "exar_gpio%d", index);
+	exar_gpio->gpio_chip.label = exar_gpio->name;
+	exar_gpio->gpio_chip.parent = &pcidev->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;
+
+	ret = devm_gpiochip_add_data(&pcidev->dev,
+				     &exar_gpio->gpio_chip, exar_gpio);
+	if (ret)
+		goto err_destroy;
+
+	platform_set_drvdata(pdev, exar_gpio);
+
+	return 0;
+
+err_destroy:
+	ida_simple_remove(&ida_index, index);
+	mutex_destroy(&exar_gpio->lock);
+	return ret;
+}
+
+static int gpio_exar_remove(struct platform_device *pdev)
+{
+	struct exar_gpio_chip *exar_gpio = platform_get_drvdata(pdev);
+
+	ida_simple_remove(&ida_index, exar_gpio->index);
+	mutex_destroy(&exar_gpio->lock);
+
+	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 <sudip.mukherjee@codethink.co.uk>");
+MODULE_LICENSE("GPL");
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v10 2/3] serial: exar: split out the exar code from 8250_pci
  2017-01-19 22:23 [PATCH v10 1/3] gpio: exar: add gpio for exar cards Sudip Mukherjee
@ 2017-01-19 22:23 ` Sudip Mukherjee
  2017-01-20 22:25   ` Andy Shevchenko
  2017-01-19 22:23 ` [PATCH v10 3/3] serial: 8250_pci: remove exar code Sudip Mukherjee
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 10+ messages in thread
From: Sudip Mukherjee @ 2017-01-19 22:23 UTC (permalink / raw)
  To: Linus Walleij, Alexandre Courbot, Greg Kroah-Hartman, Jiri Slaby,
	Andy Shevchenko, gnomes
  Cc: linux-kernel, linux-serial, linux-gpio

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.

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

Andy,
Having 3 setup hooks and assigning everything there was becoming too
complicated for me to follow, so i left it as it is. Though some more
parts from the board setup has been removed. uart_offset has been
completely removed.

To answer your questions:
1. In pci_xr17v35x_exit() pdev can be NULL if xr17v35x_register_gpio()
failed while it was called. And if the gpio fails to register then we
should not try to unregister it afterwards.
2. A working GPIO driver without serial does not make sense. I think
I will send a patch afterwards to change that.

 drivers/tty/serial/8250/8250_exar.c | 394 ++++++++++++++++++++++++++++++++++++
 drivers/tty/serial/8250/Kconfig     |   4 +
 drivers/tty/serial/8250/Makefile    |   1 +
 3 files changed, 399 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..fd5598d
--- /dev/null
+++ b/drivers/tty/serial/8250/8250_exar.c
@@ -0,0 +1,394 @@
+/*
+ *  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
+ * @base_baud: baudrate
+ * @reg_shift: describes UART register mapping in PCI memory
+ */
+struct exar8250_board {
+	unsigned int num_ports;
+	unsigned int base_baud;
+	unsigned int reg_shift;
+	bool has_slave;
+	int	(*setup)(struct exar8250 *, struct pci_dev *,
+			 const struct exar8250_board *,
+			 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,
+			 const struct exar8250_board *board, int idx,
+			 unsigned int offset, struct uart_8250_port *port)
+{
+	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_xr17c154_setup(struct exar8250 *priv, struct pci_dev *pcidev,
+		   const struct exar8250_board *board,
+		   struct uart_8250_port *port, int idx)
+{
+	unsigned int offset = idx * 0x200;
+
+	port->port.flags |= UPF_EXAR_EFR;
+	return default_setup(priv, pcidev, board, 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,
+		   const struct exar8250_board *board,
+		   struct uart_8250_port *port, int idx)
+{
+	unsigned int offset = idx * 0x400;
+	u8 __iomem *p;
+	int ret;
+
+	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 (board->has_slave && idx >= 8)
+		port->port.uartclk = (7812500 * 16 / 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, board, 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;
+
+	if (!pdev)
+		return;
+
+	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;
+
+	rc = pcim_enable_device(pcidev);
+	if (rc)
+		return rc;
+
+	board = (struct exar8250_board *)ent->driver_data;
+	if (!board) {
+		board = devm_kzalloc(&pcidev->dev, sizeof(*board),
+				     GFP_KERNEL);
+		board->base_baud = 1843200;
+	}
+
+	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;
+	uart.port.uartclk = board->base_baud * 16;
+	uart.port.irq = pcidev->irq;
+	uart.port.dev = &pcidev->dev;
+
+	for (i = 0; i < nr_ports && i < maxnr; i++) {
+		if (board->setup)
+			rc = board->setup(priv, pcidev, board, &uart, i);
+		else
+			rc = pci_xr17c154_setup(priv, pcidev, board, &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_exar_ibm_saturn = {
+	.num_ports	= 1,
+	.base_baud	= 921600,
+};
+
+static const struct exar8250_board pbn_exar_XR17C15x = {
+	.base_baud	= 921600,
+};
+
+static const struct exar8250_board pbn_exar_XR17V35x = {
+	.base_baud	= 7812500,
+	.setup		= pci_xr17v35x_setup,
+	.exit		= pci_xr17v35x_exit,
+};
+
+static const struct exar8250_board pbn_exar_XR17V4358 = {
+	.num_ports	= 12,
+	.base_baud	= 7812500,
+	.has_slave	= true,
+	.setup		= pci_xr17v35x_setup,
+	.exit		= pci_xr17v35x_exit,
+};
+
+static const struct exar8250_board pbn_exar_XR17V8358 = {
+	.num_ports	= 16,
+	.base_baud	= 7812500,
+	.has_slave	= true,
+	.setup		= pci_xr17v35x_setup,
+	.exit		= pci_xr17v35x_exit,
+};
+
+#define CONNECT_DEVICE(devid, sdevid) {				\
+	PCI_DEVICE_SUB(						\
+		PCI_VENDOR_ID_EXAR,				\
+		PCI_DEVICE_ID_EXAR_##devid,			\
+		PCI_SUBVENDOR_ID_CONNECT_TECH,			\
+		PCI_SUBDEVICE_ID_CONNECT_TECH_PCI_##sdevid)	\
+	}
+
+#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),
+	CONNECT_DEVICE(XR17C154, UART_4_232),
+	CONNECT_DEVICE(XR17C158, UART_8_232),
+	CONNECT_DEVICE(XR17C152, UART_1_1),
+	CONNECT_DEVICE(XR17C154, UART_2_2),
+	CONNECT_DEVICE(XR17C158, UART_4_4),
+	CONNECT_DEVICE(XR17C152, UART_2),
+	CONNECT_DEVICE(XR17C154, UART_4),
+	CONNECT_DEVICE(XR17C158, UART_8),
+	CONNECT_DEVICE(XR17C152, UART_2_485),
+	CONNECT_DEVICE(XR17C154, UART_4_485),
+	CONNECT_DEVICE(XR17C158, UART_8_485),
+
+	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 8998347..62a3766 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 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

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v10 3/3] serial: 8250_pci: remove exar code
  2017-01-19 22:23 [PATCH v10 1/3] gpio: exar: add gpio for exar cards Sudip Mukherjee
  2017-01-19 22:23 ` [PATCH v10 2/3] serial: exar: split out the exar code from 8250_pci Sudip Mukherjee
@ 2017-01-19 22:23 ` Sudip Mukherjee
  2017-01-20 22:29   ` Andy Shevchenko
  2017-01-20 22:02 ` [PATCH v10 1/3] gpio: exar: add gpio for exar cards Andy Shevchenko
  2017-01-26 10:16 ` Linus Walleij
  3 siblings, 1 reply; 10+ messages in thread
From: Sudip Mukherjee @ 2017-01-19 22:23 UTC (permalink / raw)
  To: Linus Walleij, Alexandre Courbot, Greg Kroah-Hartman, Jiri Slaby,
	Andy Shevchenko, gnomes
  Cc: linux-kernel, linux-serial, linux-gpio

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>
---

patch4? but those are from different vendor. They are with
PCI_VENDOR_ID_COMMTECH. Then we should not name the file as 8250_exar.
But if needed I can send a separate patch for that after this series is
complete.

 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 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 },
diff --git a/drivers/tty/serial/8250/Kconfig b/drivers/tty/serial/8250/Kconfig
index 62a3766..9f8bd0a 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

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v10 1/3] gpio: exar: add gpio for exar cards
  2017-01-19 22:23 [PATCH v10 1/3] gpio: exar: add gpio for exar cards Sudip Mukherjee
  2017-01-19 22:23 ` [PATCH v10 2/3] serial: exar: split out the exar code from 8250_pci Sudip Mukherjee
  2017-01-19 22:23 ` [PATCH v10 3/3] serial: 8250_pci: remove exar code Sudip Mukherjee
@ 2017-01-20 22:02 ` Andy Shevchenko
  2017-01-20 23:50   ` Sudip Mukherjee
  2017-01-26 10:16 ` Linus Walleij
  3 siblings, 1 reply; 10+ messages in thread
From: Andy Shevchenko @ 2017-01-20 22:02 UTC (permalink / raw)
  To: Sudip Mukherjee
  Cc: Linus Walleij, Alexandre Courbot, Greg Kroah-Hartman, Jiri Slaby,
	One Thousand Gnomes, linux-kernel, linux-serial, linux-gpio

On Fri, Jan 20, 2017 at 12:23 AM, Sudip Mukherjee
<sudipm.mukherjee@gmail.com> wrote:
> From: Sudip Mukherjee <sudip.mukherjee@codethink.co.uk>
>
> Exar XR17V352/354/358 chips have 16 multi-purpose inputs/outputs which
> can be controlled using gpio interface.
>
> Add the gpio specific code.
>

Looks nice and clean!

FWIW:
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

> Signed-off-by: Sudip Mukherjee <sudip.mukherjee@codethink.co.uk>
> ---
>  drivers/gpio/Kconfig     |   7 ++
>  drivers/gpio/Makefile    |   1 +
>  drivers/gpio/gpio-exar.c | 200 +++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 208 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..05c8946
> --- /dev/null
> +++ b/drivers/gpio/gpio-exar.c
> @@ -0,0 +1,200 @@
> +/*
> + * GPIO driver for Exar XR17V35X chip
> + *
> + * Copyright (C) 2015 Sudip Mukherjee <sudip.mukherjee@codethink.co.uk>
> + *
> + * 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.
> + */
> +#include <linux/bitops.h>
> +#include <linux/device.h>
> +#include <linux/gpio/driver.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 DEFINE_IDA(ida_index);
> +
> +struct exar_gpio_chip {
> +       struct gpio_chip gpio_chip;
> +       struct mutex lock;
> +       int index;
> +       void __iomem *regs;
> +       char name[20];
> +};
> +
> +static void exar_update(struct gpio_chip *chip, unsigned int reg, int val,
> +                       unsigned int offset)
> +{
> +       struct exar_gpio_chip *exar_gpio = gpiochip_get_data(chip);
> +       int temp;
> +
> +       mutex_lock(&exar_gpio->lock);
> +       temp = readb(exar_gpio->regs + reg);
> +       temp &= ~BIT(offset);
> +       if (val)
> +               temp |= BIT(offset);
> +       writeb(temp, exar_gpio->regs + reg);
> +       mutex_unlock(&exar_gpio->lock);
> +}
> +
> +static int exar_set_direction(struct gpio_chip *chip, int direction,
> +                             unsigned int offset)
> +{
> +       unsigned int bank = offset / 8;
> +       unsigned int addr;
> +
> +       addr = bank ? EXAR_OFFSET_MPIOSEL_HI : EXAR_OFFSET_MPIOSEL_LO;
> +       exar_update(chip, addr, 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 = gpiochip_get_data(chip);
> +       int value;
> +
> +       mutex_lock(&exar_gpio->lock);
> +       value = readb(exar_gpio->regs + reg);
> +       mutex_unlock(&exar_gpio->lock);
> +
> +       return !!value;
> +}
> +
> +static int exar_get_direction(struct gpio_chip *chip, unsigned int offset)
> +{
> +       unsigned int bank = offset / 8;
> +       unsigned int addr;
> +       int val;
> +
> +       addr = bank ? EXAR_OFFSET_MPIOSEL_HI : EXAR_OFFSET_MPIOSEL_LO;
> +       val = exar_get(chip, addr) >> (offset % 8);
> +
> +       return !!val;
> +}
> +
> +static int exar_get_value(struct gpio_chip *chip, unsigned int offset)
> +{
> +       unsigned int bank = offset / 8;
> +       unsigned int addr;
> +       int val;
> +
> +       addr = bank ? EXAR_OFFSET_MPIOLVL_LO : EXAR_OFFSET_MPIOLVL_HI;
> +       val = exar_get(chip, addr) >> (offset % 8);
> +
> +       return !!val;
> +}
> +
> +static void exar_set_value(struct gpio_chip *chip, unsigned int offset,
> +                          int value)
> +{
> +       unsigned int bank = offset / 8;
> +       unsigned int addr;
> +
> +       addr = bank ? EXAR_OFFSET_MPIOLVL_HI : EXAR_OFFSET_MPIOLVL_LO;
> +       exar_update(chip, addr, value, offset % 8);
> +}
> +
> +static int gpio_exar_probe(struct platform_device *pdev)
> +{
> +       struct pci_dev *pcidev = platform_get_drvdata(pdev);
> +       struct exar_gpio_chip *exar_gpio;
> +       void __iomem *p;
> +       int index, ret;
> +
> +       if (pcidev->vendor != PCI_VENDOR_ID_EXAR)
> +               return -ENODEV;
> +
> +       /*
> +        * Map the pci device to get the register addresses.
> +        * We will need to read and write those registers to control
> +        * the GPIO pins.
> +        * Using managed functions will save us from unmaping on exit.
> +        * As the device is enabled using managed functions by the
> +        * UART driver we can also use managed functions here.
> +        */
> +       p = pcim_iomap(pcidev, 0, 0);
> +       if (!p)
> +               return -ENOMEM;
> +
> +       exar_gpio = devm_kzalloc(&pcidev->dev, sizeof(*exar_gpio), GFP_KERNEL);
> +       if (!exar_gpio)
> +               return -ENOMEM;
> +
> +       mutex_init(&exar_gpio->lock);
> +
> +       index = ida_simple_get(&ida_index, 0, 0, GFP_KERNEL);
> +
> +       sprintf(exar_gpio->name, "exar_gpio%d", index);
> +       exar_gpio->gpio_chip.label = exar_gpio->name;
> +       exar_gpio->gpio_chip.parent = &pcidev->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;
> +
> +       ret = devm_gpiochip_add_data(&pcidev->dev,
> +                                    &exar_gpio->gpio_chip, exar_gpio);
> +       if (ret)
> +               goto err_destroy;
> +
> +       platform_set_drvdata(pdev, exar_gpio);
> +
> +       return 0;
> +
> +err_destroy:
> +       ida_simple_remove(&ida_index, index);
> +       mutex_destroy(&exar_gpio->lock);
> +       return ret;
> +}
> +
> +static int gpio_exar_remove(struct platform_device *pdev)
> +{
> +       struct exar_gpio_chip *exar_gpio = platform_get_drvdata(pdev);
> +
> +       ida_simple_remove(&ida_index, exar_gpio->index);
> +       mutex_destroy(&exar_gpio->lock);
> +
> +       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 <sudip.mukherjee@codethink.co.uk>");
> +MODULE_LICENSE("GPL");
> --
> 1.9.1
>



-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v10 2/3] serial: exar: split out the exar code from 8250_pci
  2017-01-19 22:23 ` [PATCH v10 2/3] serial: exar: split out the exar code from 8250_pci Sudip Mukherjee
@ 2017-01-20 22:25   ` Andy Shevchenko
  2017-01-20 23:55     ` Sudip Mukherjee
  0 siblings, 1 reply; 10+ messages in thread
From: Andy Shevchenko @ 2017-01-20 22:25 UTC (permalink / raw)
  To: Sudip Mukherjee
  Cc: Linus Walleij, Alexandre Courbot, Greg Kroah-Hartman, Jiri Slaby,
	One Thousand Gnomes, linux-kernel, linux-serial, linux-gpio

On Fri, Jan 20, 2017 at 12:23 AM, Sudip Mukherjee
<sudipm.mukherjee@gmail.com> 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.

Thanks, this is indeed much cleaner that v6 I saw couple of weeks before!

> Andy,
> Having 3 setup hooks and assigning everything there was becoming too
> complicated for me to follow, so i left it as it is. Though some more
> parts from the board setup has been removed. uart_offset has been
> completely removed.

Yes, what about baudrate?
I will check later, but for me it seems other way, i.e. simpler to
have just 3 setup functions in a row than one is lurking somewhere
with allocation et al.

> To answer your questions:
> 1. In pci_xr17v35x_exit() pdev can be NULL if xr17v35x_register_gpio()
> failed while it was called. And if the gpio fails to register then we
> should not try to unregister it afterwards.

See below.

> 2. A working GPIO driver without serial does not make sense. I think
> I will send a patch afterwards to change that.

OK

> +/**
> + * struct exar8250_board - board information
> + * @num_ports: number of serial ports
> + * @base_baud: baudrate
> + * @reg_shift: describes UART register mapping in PCI memory
> + */

Good!

> +static int
> +pci_xr17c154_setup(struct exar8250 *priv, struct pci_dev *pcidev,
> +                  const struct exar8250_board *board,
> +                  struct uart_8250_port *port, int idx)
> +{
> +       unsigned int offset = idx * 0x200;

Yep.

> +
> +       port->port.flags |= UPF_EXAR_EFR;
> +       return default_setup(priv, pcidev, board, idx, offset, port);
> +}

> +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;
> +

> +       if (!pdev)
> +               return;

Just checked, platform_device_unregister() is NULL-aware, so, please
remove this check.

> +
> +       platform_device_unregister(pdev);
> +       port->port.private_data = NULL;
> +}

> +exar_pci_probe(struct pci_dev *pcidev, const struct pci_device_id *ent)
> +{

> +               board = devm_kzalloc(&pcidev->dev, sizeof(*board),
> +                                    GFP_KERNEL);
> +               board->base_baud = 1843200;

That's candidate for
pci_connect_tech_setup();

But let me still look and compare.

> +       for (i = 0; i < nr_ports && i < maxnr; i++) {
> +               if (board->setup)
> +                       rc = board->setup(priv, pcidev, board, &uart, i);

> +               else
> +                       rc = pci_xr17c154_setup(priv, pcidev, board, &uart, i);

> +static const struct exar8250_board pbn_exar_ibm_saturn = {
> +       .num_ports      = 1,
> +       .base_baud      = 921600,
> +};
> +
> +static const struct exar8250_board pbn_exar_XR17C15x = {
> +       .base_baud      = 921600,
> +};

You see, that no setup has been using same baud rate. Just replace
baud rate with setup. In that case you will remove above
if (board->setup)

> +static const struct exar8250_board pbn_exar_XR17V35x = {

> +       .base_baud      = 7812500,

> +       .setup          = pci_xr17v35x_setup,
> +       .exit           = pci_xr17v35x_exit,

> +static const struct exar8250_board pbn_exar_XR17V4358 = {

> +       .base_baud      = 7812500,

> +       .setup          = pci_xr17v35x_setup,
> +       .exit           = pci_xr17v35x_exit,

> +static const struct exar8250_board pbn_exar_XR17V8358 = {
> +       .base_baud      = 7812500,

> +       .setup          = pci_xr17v35x_setup,
> +       .exit           = pci_xr17v35x_exit,
> +};

In all above you call same setup with same baud rate. Move it there.

> +#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                     \
> +       }

Yep!

> +static struct pci_device_id exar_pci_tbl[] = {
> +       CONNECT_DEVICE(XR17C152, UART_2_232),
> +       CONNECT_DEVICE(XR17C154, UART_4_232),
> +       CONNECT_DEVICE(XR17C158, UART_8_232),
> +       CONNECT_DEVICE(XR17C152, UART_1_1),
> +       CONNECT_DEVICE(XR17C154, UART_2_2),
> +       CONNECT_DEVICE(XR17C158, UART_4_4),
> +       CONNECT_DEVICE(XR17C152, UART_2),
> +       CONNECT_DEVICE(XR17C154, UART_4),
> +       CONNECT_DEVICE(XR17C158, UART_8),
> +       CONNECT_DEVICE(XR17C152, UART_2_485),
> +       CONNECT_DEVICE(XR17C154, UART_4_485),
> +       CONNECT_DEVICE(XR17C158, UART_8_485),

Perhaps it takes few LOC more, but having same defined struct here
with dedicated ->setup() looks cleaner to me.

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v10 3/3] serial: 8250_pci: remove exar code
  2017-01-19 22:23 ` [PATCH v10 3/3] serial: 8250_pci: remove exar code Sudip Mukherjee
@ 2017-01-20 22:29   ` Andy Shevchenko
  0 siblings, 0 replies; 10+ messages in thread
From: Andy Shevchenko @ 2017-01-20 22:29 UTC (permalink / raw)
  To: Sudip Mukherjee
  Cc: Linus Walleij, Alexandre Courbot, Greg Kroah-Hartman, Jiri Slaby,
	One Thousand Gnomes, linux-kernel, linux-serial, linux-gpio

On Fri, Jan 20, 2017 at 12:23 AM, Sudip Mukherjee
<sudipm.mukherjee@gmail.com> wrote:
> 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.

> patch4? but those are from different vendor. They are with
> PCI_VENDOR_ID_COMMTECH. Then we should not name the file as 8250_exar.

But it seems it is using Exar behind the scenes. No?

> But if needed I can send a separate patch for that after this series is
> complete.

For me looks like logical continuation to this series. I'm fine with
separate patch later on.

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v10 1/3] gpio: exar: add gpio for exar cards
  2017-01-20 22:02 ` [PATCH v10 1/3] gpio: exar: add gpio for exar cards Andy Shevchenko
@ 2017-01-20 23:50   ` Sudip Mukherjee
  2017-01-26 10:17     ` Linus Walleij
  0 siblings, 1 reply; 10+ messages in thread
From: Sudip Mukherjee @ 2017-01-20 23:50 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Andy Shevchenko, Alexandre Courbot, Greg Kroah-Hartman,
	Jiri Slaby, One Thousand Gnomes, linux-kernel, linux-serial,
	linux-gpio

On Friday 20 January 2017 10:02 PM, Andy Shevchenko wrote:
> On Fri, Jan 20, 2017 at 12:23 AM, Sudip Mukherjee
> <sudipm.mukherjee@gmail.com> wrote:
>> From: Sudip Mukherjee <sudip.mukherjee@codethink.co.uk>
>>
>> Exar XR17V352/354/358 chips have 16 multi-purpose inputs/outputs which
>> can be controlled using gpio interface.
>>
>> Add the gpio specific code.
>>
>
> Looks nice and clean!
>
> FWIW:
> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

Hi Linus,

It looks like the patch 2/3 will have one more revision. Do you want me 
to send this patch again as part of v11 series or will you take it in 
your tree and I send a v11 series of the remaining 2 patches?

Regards
Sudip

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v10 2/3] serial: exar: split out the exar code from 8250_pci
  2017-01-20 22:25   ` Andy Shevchenko
@ 2017-01-20 23:55     ` Sudip Mukherjee
  0 siblings, 0 replies; 10+ messages in thread
From: Sudip Mukherjee @ 2017-01-20 23:55 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Linus Walleij, Alexandre Courbot, Greg Kroah-Hartman, Jiri Slaby,
	One Thousand Gnomes, linux-kernel, linux-serial, linux-gpio

On Friday 20 January 2017 10:25 PM, Andy Shevchenko wrote:
> On Fri, Jan 20, 2017 at 12:23 AM, Sudip Mukherjee
> <sudipm.mukherjee@gmail.com> 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.
>
> Thanks, this is indeed much cleaner that v6 I saw couple of weeks before!
>
>> Andy,
>> Having 3 setup hooks and assigning everything there was becoming too
>> complicated for me to follow, so i left it as it is. Though some more
>> parts from the board setup has been removed. uart_offset has been
>> completely removed.
>
> Yes, what about baudrate?
> I will check later, but for me it seems other way, i.e. simpler to
> have just 3 setup functions in a row than one is lurking somewhere
> with allocation et al.
>
<snip>
>> +       CONNECT_DEVICE(XR17C154, UART_4_485),
>> +       CONNECT_DEVICE(XR17C158, UART_8_485),
>
> Perhaps it takes few LOC more, but having same defined struct here
> with dedicated ->setup() looks cleaner to me.
>

I think now I understand the 3 different setup functions. Let me try v11 
(hopefully the last one).

Regards
Sudip

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v10 1/3] gpio: exar: add gpio for exar cards
  2017-01-19 22:23 [PATCH v10 1/3] gpio: exar: add gpio for exar cards Sudip Mukherjee
                   ` (2 preceding siblings ...)
  2017-01-20 22:02 ` [PATCH v10 1/3] gpio: exar: add gpio for exar cards Andy Shevchenko
@ 2017-01-26 10:16 ` Linus Walleij
  3 siblings, 0 replies; 10+ messages in thread
From: Linus Walleij @ 2017-01-26 10:16 UTC (permalink / raw)
  To: Sudip Mukherjee
  Cc: Alexandre Courbot, Greg Kroah-Hartman, Jiri Slaby,
	Andy Shevchenko, One Thousand Gnomes, linux-kernel, linux-serial,
	linux-gpio

On Thu, Jan 19, 2017 at 11:23 PM, Sudip Mukherjee
<sudipm.mukherjee@gmail.com> wrote:

> From: Sudip Mukherjee <sudip.mukherjee@codethink.co.uk>
>
> 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>

Patch applied.

I think maybe we can simplify the patch even more using
select GPIO_GENERIC and just specify the different
registers to bgpio_init().

But that can be done as a separate improvement patch, this
definately looks fine.

Maybe you also want something like:

default SERIAL_8250_EXAR

so that the module is by default added as "y" or "m" depending
on what is selected for the parent device?

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v10 1/3] gpio: exar: add gpio for exar cards
  2017-01-20 23:50   ` Sudip Mukherjee
@ 2017-01-26 10:17     ` Linus Walleij
  0 siblings, 0 replies; 10+ messages in thread
From: Linus Walleij @ 2017-01-26 10:17 UTC (permalink / raw)
  To: Sudip Mukherjee
  Cc: Andy Shevchenko, Alexandre Courbot, Greg Kroah-Hartman,
	Jiri Slaby, One Thousand Gnomes, linux-kernel, linux-serial,
	linux-gpio

On Sat, Jan 21, 2017 at 12:50 AM, Sudip Mukherjee
<sudipm.mukherjee@gmail.com> wrote:

> It looks like the patch 2/3 will have one more revision. Do you want me to
> send this patch again as part of v11 series or will you take it in your tree
> and I send a v11 series of the remaining 2 patches?

I just applied 1/3, so just send the remaining patches on next iteration.

Plus/minus the improvements I suggested :)

Thanks,
Linus Walleij

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2017-01-26 10:17 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-01-19 22:23 [PATCH v10 1/3] gpio: exar: add gpio for exar cards Sudip Mukherjee
2017-01-19 22:23 ` [PATCH v10 2/3] serial: exar: split out the exar code from 8250_pci Sudip Mukherjee
2017-01-20 22:25   ` Andy Shevchenko
2017-01-20 23:55     ` Sudip Mukherjee
2017-01-19 22:23 ` [PATCH v10 3/3] serial: 8250_pci: remove exar code Sudip Mukherjee
2017-01-20 22:29   ` Andy Shevchenko
2017-01-20 22:02 ` [PATCH v10 1/3] gpio: exar: add gpio for exar cards Andy Shevchenko
2017-01-20 23:50   ` Sudip Mukherjee
2017-01-26 10:17     ` Linus Walleij
2017-01-26 10:16 ` Linus Walleij

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).