[PATCH v6 tty-next 0/4] serial: 8250_pci1xxxx: Add driver for the pci1xxxx's quad-uart function

[PATCH v6 tty-next 0/4] serial: 8250_pci1xxxx: Add driver for the pci1xxxx's quad-uart function

[Kumaravel Thiagarajan]

pci1xxxx is a PCIe switch with a multi-function endpoint on one of its
downstream ports. Quad-uart is one of the functions in the multi-function
endpoint. This patch adds device driver for the quad-uart function and
enumerates between 1 to 4 instances of uarts based on the PCIe subsystem
device ID.

The changes from v1->v2->v3->v4->v5->v6 are mentioned in each patch in
the patchset.

Thanks to Andy Shevchenko, Ilpo Jarvinen, Chritophe JAILLET, Geert
Uytterhoeven, Greg KH for their review comments.

Kumaravel Thiagarajan (4):
  serial: 8250_pci: Add serial8250_pci_setup_port definition in
    8250_pcilib.c
  serial: 8250_pci1xxxx: Add driver for quad-uart support.
  serial: 8250_pci1xxxx: Add RS485 support to quad-uart driver
  serial: 8250_pci1xxxx: Add power management functions to quad-uart
    driver

 MAINTAINERS                             |   7 +
 drivers/tty/serial/8250/8250_pci.c      |  24 +-
 drivers/tty/serial/8250/8250_pci1xxxx.c | 463 ++++++++++++++++++++++++
 drivers/tty/serial/8250/8250_pcilib.c   |  38 ++
 drivers/tty/serial/8250/8250_pcilib.h   |   9 +
 drivers/tty/serial/8250/8250_port.c     |   8 +
 drivers/tty/serial/8250/Kconfig         |  15 +
 drivers/tty/serial/8250/Makefile        |   2 +
 include/uapi/linux/serial_core.h        |   3 +
 9 files changed, 547 insertions(+), 22 deletions(-)
 create mode 100644 drivers/tty/serial/8250/8250_pci1xxxx.c
 create mode 100644 drivers/tty/serial/8250/8250_pcilib.c
 create mode 100644 drivers/tty/serial/8250/8250_pcilib.h

-- 
2.25.1

[PATCH v6 tty-next 1/4] serial: 8250_pci: Add serial8250_pci_setup_port definition in 8250_pcilib.c

[Kumaravel Thiagarajan]

Move implementation of setup_port API to serial8250_pci_setup_port

Co-developed-by: Tharun Kumar P <tharunkumar.pasumarthi@microchip.com>
Signed-off-by: Tharun Kumar P <tharunkumar.pasumarthi@microchip.com>
Signed-off-by: Kumaravel Thiagarajan <kumaravel.thiagarajan@microchip.com>
---
Changes in v6:
- Made this patch first patch of the patch series

Changes in v5:
- This is the new patch added in v5 version of this patchset
- Moved implementation of setup_port from 8250_pci.c to 8250_pcilib.c

---
 drivers/tty/serial/8250/8250_pci.c    | 24 ++---------------
 drivers/tty/serial/8250/8250_pcilib.c | 38 +++++++++++++++++++++++++++
 drivers/tty/serial/8250/8250_pcilib.h |  9 +++++++
 drivers/tty/serial/8250/Kconfig       |  4 +++
 drivers/tty/serial/8250/Makefile      |  1 +
 5 files changed, 54 insertions(+), 22 deletions(-)
 create mode 100644 drivers/tty/serial/8250/8250_pcilib.c
 create mode 100644 drivers/tty/serial/8250/8250_pcilib.h

diff --git a/drivers/tty/serial/8250/8250_pci.c b/drivers/tty/serial/8250/8250_pci.c
index 6f66dc2ebacc..69ff367b08de 100644
--- a/drivers/tty/serial/8250/8250_pci.c
+++ b/drivers/tty/serial/8250/8250_pci.c
@@ -24,6 +24,7 @@
 #include <asm/io.h>
 
 #include "8250.h"
+#include "8250_pcilib.h"
 
 /*
  * init function returns:
@@ -89,28 +90,7 @@ static int
 setup_port(struct serial_private *priv, struct uart_8250_port *port,
 	   u8 bar, unsigned int offset, int regshift)
 {
-	struct pci_dev *dev = priv->dev;
-
-	if (bar >= PCI_STD_NUM_BARS)
-		return -EINVAL;
-
-	if (pci_resource_flags(dev, bar) & IORESOURCE_MEM) {
-		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 = regshift;
-	} else {
-		port->port.iotype = UPIO_PORT;
-		port->port.iobase = pci_resource_start(dev, bar) + offset;
-		port->port.mapbase = 0;
-		port->port.membase = NULL;
-		port->port.regshift = 0;
-	}
-	return 0;
+	return serial8250_pci_setup_port(priv->dev, port, bar, offset, regshift);
 }
 
 /*
diff --git a/drivers/tty/serial/8250/8250_pcilib.c b/drivers/tty/serial/8250/8250_pcilib.c
new file mode 100644
index 000000000000..e5a4a9b22c81
--- /dev/null
+++ b/drivers/tty/serial/8250/8250_pcilib.c
@@ -0,0 +1,38 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * 8250 PCI library.
+ *
+ * Copyright (C) 2001 Russell King, All Rights Reserved.
+ */
+#include <linux/errno.h>
+#include <linux/ioport.h>
+#include <linux/pci.h>
+#include <linux/types.h>
+
+#include "8250.h"
+
+int serial8250_pci_setup_port(struct pci_dev *dev, struct uart_8250_port *port,
+		   u8 bar, unsigned int offset, int regshift)
+{
+	if (bar >= PCI_STD_NUM_BARS)
+		return -EINVAL;
+
+	if (pci_resource_flags(dev, bar) & IORESOURCE_MEM) {
+		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 = regshift;
+	} else {
+		port->port.iotype = UPIO_PORT;
+		port->port.iobase = pci_resource_start(dev, bar) + offset;
+		port->port.mapbase = 0;
+		port->port.membase = NULL;
+		port->port.regshift = 0;
+	}
+	return 0;
+}
+EXPORT_SYMBOL_GPL(serial8250_pci_setup_port);
diff --git a/drivers/tty/serial/8250/8250_pcilib.h b/drivers/tty/serial/8250/8250_pcilib.h
new file mode 100644
index 000000000000..41ef01d5c3c5
--- /dev/null
+++ b/drivers/tty/serial/8250/8250_pcilib.h
@@ -0,0 +1,9 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * 8250 PCI library header file.
+ *
+ * Copyright (C) 2001 Russell King, All Rights Reserved.
+ */
+
+int serial8250_pci_setup_port(struct pci_dev *dev, struct uart_8250_port *port, u8 bar,
+		   unsigned int offset, int regshift);
diff --git a/drivers/tty/serial/8250/Kconfig b/drivers/tty/serial/8250/Kconfig
index d0b49e15fbf5..3088eaff3ad0 100644
--- a/drivers/tty/serial/8250/Kconfig
+++ b/drivers/tty/serial/8250/Kconfig
@@ -132,6 +132,7 @@ config SERIAL_8250_DMA
 config SERIAL_8250_PCI
 	tristate "8250/16550 PCI device support"
 	depends on SERIAL_8250 && PCI
+	select SERIAL_8250_PCILIB
 	default SERIAL_8250
 	help
 	  This builds standard PCI serial support. You may be able to
@@ -500,6 +501,9 @@ config SERIAL_8250_MID
 	  Intel Medfield SOC and various other Intel platforms that is not
 	  covered by the more generic SERIAL_8250_PCI option.
 
+config SERIAL_8250_PCILIB
+	bool
+
 config SERIAL_8250_PERICOM
 	tristate "Support for Pericom and Acces I/O serial ports"
 	default SERIAL_8250
diff --git a/drivers/tty/serial/8250/Makefile b/drivers/tty/serial/8250/Makefile
index bee908f99ea0..b9179d1f104b 100644
--- a/drivers/tty/serial/8250/Makefile
+++ b/drivers/tty/serial/8250/Makefile
@@ -12,6 +12,7 @@ obj-$(CONFIG_SERIAL_8250)		+= 8250.o 8250_base.o
 8250_base-$(CONFIG_SERIAL_8250_DMA)	+= 8250_dma.o
 8250_base-$(CONFIG_SERIAL_8250_DWLIB)	+= 8250_dwlib.o
 8250_base-$(CONFIG_SERIAL_8250_FINTEK)	+= 8250_fintek.o
+8250_base-$(CONFIG_SERIAL_8250_PCILIB)	+= 8250_pcilib.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
-- 
2.25.1

[PATCH v6 tty-next 2/4] serial: 8250_pci1xxxx: Add driver for quad-uart support.

[Kumaravel Thiagarajan]

pci1xxxx is a PCIe switch with a multi-function endpoint on one of
its downstream ports. Quad-uart is one of the functions in the
multi-function endpoint. This driver loads for the quad-uart and
enumerates single or multiple instances of uart based on the PCIe
subsystem device ID.

Co-developed-by: Tharun Kumar P <tharunkumar.pasumarthi@microchip.com>
Signed-off-by: Tharun Kumar P <tharunkumar.pasumarthi@microchip.com>
Signed-off-by: Kumaravel Thiagarajan <kumaravel.thiagarajan@microchip.com>
---
Changes in v6:
- Removed un-necessary paranthesis
- Used array and removed switch cases to reduce complexity
- Handled failure case of pcim_iomap

Changes in v5:
- Used tabs instead of spaces in MACRO definitions for readability
- Removed assignments that are not required
- Removed redundant blank lines

Changes in v4:
- Renamed pci_setup_port to serial8250_pci_setup_port
- Added Copyright information to 8250_pcilib.c

Changes in v3:
- Used NSEC_PER_SEC, HZ_PER_MHZ, FIELD_PREP, FIELD_GET MACROs wherever
  necessary
- Handled failure case of serial8250_register_8250_port properly
- Moved pci_setup_port to 8250_pcilib.c

Changes in v2:
- Use only the 62.5 MHz for baud clock.
- Define custom implementation for get_divisor and set_divisor.
- Use BOTHER instead of UPF_SPD_CUST for non standard baud rates
  (untested).
- Correct indentation in clock divisor computation.
- Remove unnecessary call to pci_save_state in probe function.
- Fix null pointer dereference in probe function.
- Move pci1xxxx_rs485_config to a separate patch.
- Depends on SERIAL_8250_PCI & default to SERIAL_8250.
- Change PORT_MCHP16550A to 100 from 124.

---
 MAINTAINERS                             |   7 +
 drivers/tty/serial/8250/8250_pci1xxxx.c | 298 ++++++++++++++++++++++++
 drivers/tty/serial/8250/8250_port.c     |   8 +
 drivers/tty/serial/8250/Kconfig         |  11 +
 drivers/tty/serial/8250/Makefile        |   1 +
 include/uapi/linux/serial_core.h        |   3 +
 6 files changed, 328 insertions(+)
 create mode 100644 drivers/tty/serial/8250/8250_pci1xxxx.c

diff --git a/MAINTAINERS b/MAINTAINERS
index d30f26e07cd3..20c868776a5e 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -13434,6 +13434,13 @@ F:	Documentation/devicetree/bindings/nvmem/microchip,sama7g5-otpc.yaml
 F:	drivers/nvmem/microchip-otpc.c
 F:	include/dt-bindings/nvmem/microchip,sama7g5-otpc.h
 
+MICROCHIP PCIe UART DRIVER
+M:     Kumaravel Thiagarajan <kumaravel.thiagarajan@microchip.com>
+M:     Tharun Kumar P <tharunkumar.pasumarthi@microchip.com>
+L:     linux-serial@vger.kernel.org
+S:     Maintained
+F:     drivers/tty/serial/8250/8250_pci1xxxx.c
+
 MICROCHIP PWM DRIVER
 M:	Claudiu Beznea <claudiu.beznea@microchip.com>
 L:	linux-arm-kernel@lists.infradead.org (moderated for non-subscribers)
diff --git a/drivers/tty/serial/8250/8250_pci1xxxx.c b/drivers/tty/serial/8250/8250_pci1xxxx.c
new file mode 100644
index 000000000000..d5037e76b636
--- /dev/null
+++ b/drivers/tty/serial/8250/8250_pci1xxxx.c
@@ -0,0 +1,298 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ *  Probe module for 8250/16550-type MCHP PCI serial ports.
+ *
+ *  Based on drivers/tty/serial/8250/8250_pci.c,
+ *
+ *  Copyright (C) 2022 Microchip Technology Inc., All Rights Reserved.
+ */
+
+#include <linux/bitfield.h>
+#include <linux/bitops.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/pci.h>
+#include <linux/serial_core.h>
+#include <linux/slab.h>
+#include <linux/string.h>
+#include <linux/units.h>
+#include <linux/tty.h>
+
+#include <asm/byteorder.h>
+
+#include "8250.h"
+#include "8250_pcilib.h"
+
+#define PCI_DEVICE_ID_EFAR_PCI12000		0xa002
+#define PCI_DEVICE_ID_EFAR_PCI11010		0xa012
+#define PCI_DEVICE_ID_EFAR_PCI11101		0xa022
+#define PCI_DEVICE_ID_EFAR_PCI11400		0xa032
+#define PCI_DEVICE_ID_EFAR_PCI11414		0xa042
+
+#define PCI_SUBDEVICE_ID_EFAR_PCI1XXXX_4p	0x0001
+#define PCI_SUBDEVICE_ID_EFAR_PCI1XXXX_3p012	0x0002
+#define PCI_SUBDEVICE_ID_EFAR_PCI1XXXX_3p013	0x0003
+#define PCI_SUBDEVICE_ID_EFAR_PCI1XXXX_3p023	0x0004
+#define PCI_SUBDEVICE_ID_EFAR_PCI1XXXX_3p123	0x0005
+#define PCI_SUBDEVICE_ID_EFAR_PCI1XXXX_2p01	0x0006
+#define PCI_SUBDEVICE_ID_EFAR_PCI1XXXX_2p02	0x0007
+#define PCI_SUBDEVICE_ID_EFAR_PCI1XXXX_2p03	0x0008
+#define PCI_SUBDEVICE_ID_EFAR_PCI1XXXX_2p12	0x0009
+#define PCI_SUBDEVICE_ID_EFAR_PCI1XXXX_2p13	0x000a
+#define PCI_SUBDEVICE_ID_EFAR_PCI1XXXX_2p23	0x000b
+#define PCI_SUBDEVICE_ID_EFAR_PCI1XXXX_1p0	0x000c
+#define PCI_SUBDEVICE_ID_EFAR_PCI1XXXX_1p1	0x000d
+#define PCI_SUBDEVICE_ID_EFAR_PCI1XXXX_1p2	0x000e
+#define PCI_SUBDEVICE_ID_EFAR_PCI1XXXX_1p3	0x000f
+
+#define PCI_SUBDEVICE_ID_EFAR_PCI12000		0xa002
+#define PCI_SUBDEVICE_ID_EFAR_PCI11010		0xa012
+#define PCI_SUBDEVICE_ID_EFAR_PCI11101		0xa022
+#define PCI_SUBDEVICE_ID_EFAR_PCI11400		0xa032
+#define PCI_SUBDEVICE_ID_EFAR_PCI11414		0xa042
+
+#define UART_ACTV_REG				0x11
+#define UART_BLOCK_SET_ACTIVE			BIT(0)
+
+#define UART_PCI_CTRL_REG			0x80
+#define UART_PCI_CTRL_SET_MULTIPLE_MSI		BIT(4)
+#define UART_PCI_CTRL_D3_CLK_ENABLE		BIT(0)
+
+#define ADCL_CFG_REG				0x40
+#define ADCL_CFG_POL_SEL			BIT(2)
+#define ADCL_CFG_PIN_SEL			BIT(1)
+#define ADCL_CFG_EN				BIT(0)
+
+#define UART_BIT_SAMPLE_CNT			16
+#define BAUD_CLOCK_DIV_INT_MSK			GENMASK(31, 8)
+#define ADCL_CFG_RTS_DELAY_MASK			GENMASK(11, 8)
+#define UART_CLOCK_DEFAULT			(62.5 * HZ_PER_MHZ)
+
+#define UART_WAKE_REG				0x8C
+#define UART_WAKE_MASK_REG			0x90
+#define UART_WAKE_N_PIN				BIT(2)
+#define UART_WAKE_NCTS				BIT(1)
+#define UART_WAKE_INT				BIT(0)
+#define UART_WAKE_SRCS	\
+	(UART_WAKE_N_PIN | UART_WAKE_NCTS | UART_WAKE_INT)
+
+#define UART_BAUD_CLK_DIVISOR_REG		0x54
+
+#define UART_RESET_REG				0x94
+#define UART_RESET_D3_RESET_DISABLE		BIT(16)
+
+#define MAX_PORTS				4
+
+struct pci1xxxx_8250 {
+	struct pci_dev *pdev;
+	unsigned int nr;
+	void __iomem *membase;
+	int line[];
+};
+
+static int pci1xxxx_get_num_ports(struct pci_dev *dev)
+{
+	switch (dev->subsystem_device) {
+	case PCI_SUBDEVICE_ID_EFAR_PCI1XXXX_1p0:
+	case PCI_SUBDEVICE_ID_EFAR_PCI1XXXX_1p1:
+	case PCI_SUBDEVICE_ID_EFAR_PCI1XXXX_1p2:
+	case PCI_SUBDEVICE_ID_EFAR_PCI1XXXX_1p3:
+	case PCI_SUBDEVICE_ID_EFAR_PCI12000:
+	case PCI_SUBDEVICE_ID_EFAR_PCI11010:
+	case PCI_SUBDEVICE_ID_EFAR_PCI11101:
+	case PCI_SUBDEVICE_ID_EFAR_PCI11400:
+	default:
+		return 1;
+	case PCI_SUBDEVICE_ID_EFAR_PCI1XXXX_2p01:
+	case PCI_SUBDEVICE_ID_EFAR_PCI1XXXX_2p02:
+	case PCI_SUBDEVICE_ID_EFAR_PCI1XXXX_2p03:
+	case PCI_SUBDEVICE_ID_EFAR_PCI1XXXX_2p12:
+	case PCI_SUBDEVICE_ID_EFAR_PCI1XXXX_2p13:
+	case PCI_SUBDEVICE_ID_EFAR_PCI1XXXX_2p23:
+		return 2;
+	case PCI_SUBDEVICE_ID_EFAR_PCI1XXXX_3p012:
+	case PCI_SUBDEVICE_ID_EFAR_PCI1XXXX_3p123:
+	case PCI_SUBDEVICE_ID_EFAR_PCI1XXXX_3p013:
+	case PCI_SUBDEVICE_ID_EFAR_PCI1XXXX_3p023:
+		return 3;
+	case PCI_SUBDEVICE_ID_EFAR_PCI1XXXX_4p:
+	case PCI_SUBDEVICE_ID_EFAR_PCI11414:
+		return 4;
+	}
+}
+
+static unsigned int pci1xxxx_get_divisor(struct uart_port *port,
+					 unsigned int baud, unsigned int *frac)
+{
+	unsigned int quot;
+
+	/*
+	 * Calculate baud rate sampling period in nanoseconds.
+	 * Fractional part x denotes x/255 parts of a nanosecond.
+	 */
+	quot = NSEC_PER_SEC / (baud * UART_BIT_SAMPLE_CNT);
+	*frac = (NSEC_PER_SEC - quot * baud * UART_BIT_SAMPLE_CNT) /
+		  UART_BIT_SAMPLE_CNT * 255 / baud;
+
+	return quot;
+}
+
+static void pci1xxxx_set_divisor(struct uart_port *port, unsigned int baud,
+				 unsigned int quot, unsigned int frac)
+{
+	writel(FIELD_PREP(BAUD_CLOCK_DIV_INT_MSK, quot) | frac,
+	       port->membase + UART_BAUD_CLK_DIVISOR_REG);
+}
+
+static int pci1xxxx_setup(struct pci1xxxx_8250 *priv,
+			  struct uart_8250_port *port, int port_idx)
+{
+	int ret;
+
+	port->port.flags |= UPF_FIXED_TYPE | UPF_SKIP_TEST;
+	port->port.type = PORT_MCHP16550A;
+	port->port.set_termios = serial8250_do_set_termios;
+	port->port.get_divisor = pci1xxxx_get_divisor;
+	port->port.set_divisor = pci1xxxx_set_divisor;
+
+	ret = serial8250_pci_setup_port(priv->pdev, port, 0, port_idx * 256, 0);
+	if (ret < 0)
+		return ret;
+
+	writeb(UART_BLOCK_SET_ACTIVE, port->port.membase + UART_ACTV_REG);
+	writeb(UART_WAKE_SRCS, port->port.membase + UART_WAKE_REG);
+	writeb(UART_WAKE_N_PIN, port->port.membase + UART_WAKE_MASK_REG);
+
+	return 0;
+}
+
+static int pci1xxxx_serial_probe(struct pci_dev *pdev,
+				 const struct pci_device_id *id)
+{
+	static int logical_to_physical_port_idx[][MAX_PORTS] = {
+		{0, 1, 2, 3},	/* PCI12000 PCI11010 PCI11101 PCI11400 PCI11414 */
+		{0, 1, 2, 3},	/* PCI4p */
+		{0, 1, 2, -1},	/* PCI3p012 */
+		{0, 1, 3, -1},	/* PCI3p013 */
+		{0, 2, 3, -1},	/* PCI3p023 */
+		{1, 2, 3, -1},	/* PCI3p123 */
+		{0, 1, -1, -1},	/* PCI2p01 */
+		{0, 2, -1, -1},	/* PCI2p02 */
+		{0, 3, -1, -1},	/* PCI2p03 */
+		{1, 2, -1, -1},	/* PCI2p12 */
+		{1, 3, -1, -1},	/* PCI2p13 */
+		{2, 3, -1, -1},	/* PCI2p23 */
+		{0, -1, -1, -1},/* PCI1p0 */
+		{1, -1, -1, -1},/* PCI1p1 */
+		{2, -1, -1, -1},/* PCI1p2 */
+		{3, -1, -1, -1}	/* PCI1p3 */
+	};
+	struct device *dev = &pdev->dev;
+	struct pci1xxxx_8250 *priv;
+	struct uart_8250_port uart;
+	unsigned int nr_ports, i;
+	int num_vectors;
+	int subsys_dev;
+	int port_idx;
+	int rc;
+
+	rc = pcim_enable_device(pdev);
+	if (rc)
+		return rc;
+
+	nr_ports = pci1xxxx_get_num_ports(pdev);
+
+	priv = devm_kzalloc(dev, struct_size(priv, line, nr_ports), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	priv->membase = pcim_iomap(pdev, 0, 0);
+	if (!priv->membase)
+		return -ENOMEM;
+
+	priv->pdev = pdev;
+	subsys_dev = priv->pdev->subsystem_device;
+	priv->nr = nr_ports;
+	pci_set_master(pdev);
+
+	num_vectors = pci_alloc_irq_vectors(pdev, 1, 4, PCI_IRQ_ALL_TYPES);
+	if (num_vectors < 0)
+		return num_vectors;
+
+	memset(&uart, 0, sizeof(uart));
+	uart.port.flags = UPF_SHARE_IRQ | UPF_FIXED_PORT;
+	uart.port.uartclk = UART_CLOCK_DEFAULT;
+	uart.port.dev = dev;
+
+	if (num_vectors == 4)
+		writeb(UART_PCI_CTRL_SET_MULTIPLE_MSI,
+		       priv->membase + UART_PCI_CTRL_REG);
+	else
+		uart.port.irq = pci_irq_vector(pdev, 0);
+
+	for (i = 0; i < nr_ports; i++)
+		priv->line[i] = -ENOSPC;
+
+	for (i = 0; i < nr_ports; i++) {
+		if (subsys_dev > 0x0f)
+			port_idx = logical_to_physical_port_idx[0][i];
+		else
+			port_idx = logical_to_physical_port_idx[subsys_dev][i];
+
+		if (num_vectors == 4)
+			uart.port.irq = pci_irq_vector(priv->pdev, port_idx);
+
+		rc = pci1xxxx_setup(priv, &uart, port_idx);
+		if (rc) {
+			dev_warn(dev, "Failed to setup port %u\n", i);
+			continue;
+		}
+
+		priv->line[i] = serial8250_register_8250_port(&uart);
+		if (priv->line[i] < 0) {
+			dev_warn(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]);
+		}
+	}
+
+	pci_set_drvdata(pdev, priv);
+
+	return 0;
+}
+
+static void pci1xxxx_serial_remove(struct pci_dev *dev)
+{
+	struct pci1xxxx_8250 *priv = pci_get_drvdata(dev);
+	int i;
+
+	for (i = 0; i < priv->nr; i++) {
+		if (priv->line[i] >= 0)
+			serial8250_unregister_port(priv->line[i]);
+	}
+}
+
+static const struct pci_device_id pci1xxxx_pci_tbl[] = {
+	{ PCI_DEVICE(PCI_VENDOR_ID_EFAR, PCI_DEVICE_ID_EFAR_PCI11010) },
+	{ PCI_DEVICE(PCI_VENDOR_ID_EFAR, PCI_DEVICE_ID_EFAR_PCI11101) },
+	{ PCI_DEVICE(PCI_VENDOR_ID_EFAR, PCI_DEVICE_ID_EFAR_PCI11400) },
+	{ PCI_DEVICE(PCI_VENDOR_ID_EFAR, PCI_DEVICE_ID_EFAR_PCI11414) },
+	{ PCI_DEVICE(PCI_VENDOR_ID_EFAR, PCI_DEVICE_ID_EFAR_PCI12000) },
+	{}
+};
+MODULE_DEVICE_TABLE(pci, pci1xxxx_pci_tbl);
+
+static struct pci_driver pci1xxxx_pci_driver = {
+	.name = "pci1xxxx serial",
+	.probe = pci1xxxx_serial_probe,
+	.remove = pci1xxxx_serial_remove,
+	.id_table = pci1xxxx_pci_tbl,
+};
+module_pci_driver(pci1xxxx_pci_driver);
+
+MODULE_DESCRIPTION("Microchip Technology Inc. PCIe to UART module");
+MODULE_AUTHOR("Kumaravel Thiagarajan <kumaravel.thiagarajan@microchip.com>");
+MODULE_AUTHOR("Tharun Kumar P <tharunkumar.pasumarthi@microchip.com>");
+MODULE_LICENSE("GPL");
diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
index 1d2a43214b48..ec2fe5fd7b02 100644
--- a/drivers/tty/serial/8250/8250_port.c
+++ b/drivers/tty/serial/8250/8250_port.c
@@ -313,6 +313,14 @@ static const struct serial8250_config uart_config[] = {
 		.rxtrig_bytes	= {1, 4, 8, 14},
 		.flags		= UART_CAP_FIFO,
 	},
+	[PORT_MCHP16550A] = {
+		.name           = "MCHP16550A",
+		.fifo_size      = 256,
+		.tx_loadsz      = 256,
+		.fcr            = UART_FCR_ENABLE_FIFO | UART_FCR_R_TRIG_01,
+		.rxtrig_bytes   = {2, 66, 130, 194},
+		.flags          = UART_CAP_FIFO,
+	},
 };
 
 /* Uart divisor latch read */
diff --git a/drivers/tty/serial/8250/Kconfig b/drivers/tty/serial/8250/Kconfig
index 3088eaff3ad0..f67542470eae 100644
--- a/drivers/tty/serial/8250/Kconfig
+++ b/drivers/tty/serial/8250/Kconfig
@@ -292,6 +292,17 @@ config SERIAL_8250_HUB6
 	  To compile this driver as a module, choose M here: the module
 	  will be called 8250_hub6.
 
+config SERIAL_8250_PCI1XXXX
+	tristate "Microchip 8250 based serial port"
+	depends on SERIAL_8250_PCI
+	select SERIAL_8250_PCILIB
+	default SERIAL_8250
+	help
+	 Select this option if you have a setup with Microchip PCIe
+	 Switch with serial port enabled and wish to enable 8250
+	 serial driver for the serial interface. This driver support
+	 will ensure to support baud rates upto 1.5Mpbs.
+
 #
 # Misc. options/drivers.
 #
diff --git a/drivers/tty/serial/8250/Makefile b/drivers/tty/serial/8250/Makefile
index b9179d1f104b..98202fdf39f8 100644
--- a/drivers/tty/serial/8250/Makefile
+++ b/drivers/tty/serial/8250/Makefile
@@ -27,6 +27,7 @@ obj-$(CONFIG_SERIAL_8250_ACCENT)	+= 8250_accent.o
 obj-$(CONFIG_SERIAL_8250_BOCA)		+= 8250_boca.o
 obj-$(CONFIG_SERIAL_8250_EXAR_ST16C554)	+= 8250_exar_st16c554.o
 obj-$(CONFIG_SERIAL_8250_HUB6)		+= 8250_hub6.o
+obj-$(CONFIG_SERIAL_8250_PCI1XXXX)	+= 8250_pci1xxxx.o
 obj-$(CONFIG_SERIAL_8250_FSL)		+= 8250_fsl.o
 obj-$(CONFIG_SERIAL_8250_MEN_MCB)	+= 8250_men_mcb.o
 obj-$(CONFIG_SERIAL_8250_DW)		+= 8250_dw.o
diff --git a/include/uapi/linux/serial_core.h b/include/uapi/linux/serial_core.h
index 3ba34d8378bd..281fa286555c 100644
--- a/include/uapi/linux/serial_core.h
+++ b/include/uapi/linux/serial_core.h
@@ -207,6 +207,9 @@
 /* Atheros AR933X SoC */
 #define PORT_AR933X	99
 
+/* MCHP 16550A UART with 256 byte FIFOs */
+#define PORT_MCHP16550A	100
+
 /* ARC (Synopsys) on-chip UART */
 #define PORT_ARC       101
 
-- 
2.25.1

[PATCH v6 tty-next 3/4] serial: 8250_pci1xxxx: Add RS485 support to quad-uart driver

[Kumaravel Thiagarajan]

pci1xxxx uart supports RS485 mode of operation in the hardware with
auto-direction control with configurable delay for releasing RTS after
the transmission. This patch adds support for the RS485 mode.

Co-developed-by: Tharun Kumar P <tharunkumar.pasumarthi@microchip.com>
Signed-off-by: Tharun Kumar P <tharunkumar.pasumarthi@microchip.com>
Signed-off-by: Kumaravel Thiagarajan <kumaravel.thiagarajan@microchip.com>
---
Changes in v6:
- Modified datatype of delay_in_baud_periods to u64 to avoid overflows

Changes in v5:
- Removed unnecessary assignments
- Corrected styling issues in comments

Changes in v4:
- No Change

Changes in v3:
- Remove flags sanitization in driver which is taken care in core

Changes in v2:
- move pci1xxxx_rs485_config to a separate patch with
  pci1xxxx_rs485_supported.
---
 drivers/tty/serial/8250/8250_pci1xxxx.c | 49 +++++++++++++++++++++++++
 1 file changed, 49 insertions(+)

diff --git a/drivers/tty/serial/8250/8250_pci1xxxx.c b/drivers/tty/serial/8250/8250_pci1xxxx.c
index d5037e76b636..7585066d6baf 100644
--- a/drivers/tty/serial/8250/8250_pci1xxxx.c
+++ b/drivers/tty/serial/8250/8250_pci1xxxx.c
@@ -145,6 +145,53 @@ static void pci1xxxx_set_divisor(struct uart_port *port, unsigned int baud,
 	       port->membase + UART_BAUD_CLK_DIVISOR_REG);
 }
 
+static int pci1xxxx_rs485_config(struct uart_port *port,
+				 struct ktermios *termios,
+				 struct serial_rs485 *rs485)
+{
+	u32 clock_div = readl(port->membase + UART_BAUD_CLK_DIVISOR_REG);
+	u64 delay_in_baud_periods;
+	u32 baud_period_in_ns;
+	u32 data = 0;
+
+	/*
+	 * pci1xxxx's uart hardware supports only RTS delay after
+	 * Tx and in units of bit times to a maximum of 15
+	 */
+	if (rs485->flags & SER_RS485_ENABLED) {
+		data = ADCL_CFG_EN | ADCL_CFG_PIN_SEL;
+
+		if (!(rs485->flags & SER_RS485_RTS_ON_SEND))
+			data |= ADCL_CFG_POL_SEL;
+
+		if (rs485->delay_rts_after_send) {
+			baud_period_in_ns =
+				FIELD_GET(BAUD_CLOCK_DIV_INT_MSK, clock_div) *
+				UART_BIT_SAMPLE_CNT;
+			delay_in_baud_periods =
+				rs485->delay_rts_after_send * NSEC_PER_MSEC /
+				baud_period_in_ns;
+			delay_in_baud_periods =
+				min_t(u64, delay_in_baud_periods,
+				      FIELD_MAX(ADCL_CFG_RTS_DELAY_MASK));
+			data |= FIELD_PREP(ADCL_CFG_RTS_DELAY_MASK,
+					   delay_in_baud_periods);
+			rs485->delay_rts_after_send =
+				baud_period_in_ns * delay_in_baud_periods /
+				NSEC_PER_MSEC;
+		}
+	}
+	writel(data, port->membase + ADCL_CFG_REG);
+	return 0;
+}
+
+static const struct serial_rs485 pci1xxxx_rs485_supported = {
+	.flags = SER_RS485_ENABLED | SER_RS485_RTS_ON_SEND |
+		 SER_RS485_RTS_AFTER_SEND,
+	.delay_rts_after_send = 1,
+	/* Delay RTS before send is not supported */
+};
+
 static int pci1xxxx_setup(struct pci1xxxx_8250 *priv,
 			  struct uart_8250_port *port, int port_idx)
 {
@@ -155,6 +202,8 @@ static int pci1xxxx_setup(struct pci1xxxx_8250 *priv,
 	port->port.set_termios = serial8250_do_set_termios;
 	port->port.get_divisor = pci1xxxx_get_divisor;
 	port->port.set_divisor = pci1xxxx_set_divisor;
+	port->port.rs485_config = pci1xxxx_rs485_config;
+	port->port.rs485_supported = pci1xxxx_rs485_supported;
 
 	ret = serial8250_pci_setup_port(priv->pdev, port, 0, port_idx * 256, 0);
 	if (ret < 0)
-- 
2.25.1

[PATCH v6 tty-next 4/4] serial: 8250_pci1xxxx: Add power management functions to quad-uart driver

[Kumaravel Thiagarajan]

pci1xxxx's quad-uart function has the capability to wake up UART
from suspend state. Enable wakeup before entering into suspend and
disable wakeup on resume.

Co-developed-by: Tharun Kumar P <tharunkumar.pasumarthi@microchip.com>
Signed-off-by: Tharun Kumar P <tharunkumar.pasumarthi@microchip.com>
Signed-off-by: Kumaravel Thiagarajan <kumaravel.thiagarajan@microchip.com>
---
Changes in v6:
- No Change

Changes in v5:
- Corrected commit message

Changes in v4:
- No Change

Changes in v3:
- Handled race condition in suspend and resume callbacks

Changes in v2:
- Use DEFINE_SIMPLE_DEV_PM_OPS instead of SIMPLE_DEV_PM_OPS.
- Use pm_sleep_ptr instead of CONFIG_PM_SLEEP.
- Change the return data type of pci1xxxx_port_suspend to bool from int.
---
 drivers/tty/serial/8250/8250_pci1xxxx.c | 116 ++++++++++++++++++++++++
 1 file changed, 116 insertions(+)

diff --git a/drivers/tty/serial/8250/8250_pci1xxxx.c b/drivers/tty/serial/8250/8250_pci1xxxx.c
index 7585066d6baf..594f0152daf2 100644
--- a/drivers/tty/serial/8250/8250_pci1xxxx.c
+++ b/drivers/tty/serial/8250/8250_pci1xxxx.c
@@ -192,6 +192,116 @@ static const struct serial_rs485 pci1xxxx_rs485_supported = {
 	/* Delay RTS before send is not supported */
 };
 
+static bool pci1xxxx_port_suspend(int line)
+{
+	struct uart_8250_port *up = serial8250_get_port(line);
+	struct uart_port *port = &up->port;
+	struct tty_port *tport = &port->state->port;
+	unsigned long flags;
+	bool ret = false;
+	u8 wakeup_mask;
+
+	mutex_lock(&tport->mutex);
+	if (port->suspended == 0 && port->dev) {
+		wakeup_mask = readb(up->port.membase + UART_WAKE_MASK_REG);
+
+		spin_lock_irqsave(&port->lock, flags);
+		port->mctrl &= ~TIOCM_OUT2;
+		port->ops->set_mctrl(port, port->mctrl);
+		spin_unlock_irqrestore(&port->lock, flags);
+
+		ret = (wakeup_mask & UART_WAKE_SRCS) != UART_WAKE_SRCS;
+	}
+
+	writeb(UART_WAKE_SRCS, port->membase + UART_WAKE_REG);
+	mutex_unlock(&tport->mutex);
+
+	return ret;
+}
+
+static void pci1xxxx_port_resume(int line)
+{
+	struct uart_8250_port *up = serial8250_get_port(line);
+	struct uart_port *port = &up->port;
+	struct tty_port *tport = &port->state->port;
+	unsigned long flags;
+
+	mutex_lock(&tport->mutex);
+	writeb(UART_BLOCK_SET_ACTIVE, port->membase + UART_ACTV_REG);
+	writeb(UART_WAKE_SRCS, port->membase + UART_WAKE_REG);
+
+	if (port->suspended == 0) {
+		spin_lock_irqsave(&port->lock, flags);
+		port->mctrl |= TIOCM_OUT2;
+		port->ops->set_mctrl(port, port->mctrl);
+		spin_unlock_irqrestore(&port->lock, flags);
+	}
+	mutex_unlock(&tport->mutex);
+}
+
+static int pci1xxxx_suspend(struct device *dev)
+{
+	struct pci1xxxx_8250 *priv = dev_get_drvdata(dev);
+	struct pci_dev *pcidev = to_pci_dev(dev);
+	bool wakeup = false;
+	unsigned int data;
+	void __iomem *p;
+	int i;
+
+	for (i = 0; i < priv->nr; i++) {
+		if (priv->line[i] >= 0) {
+			serial8250_suspend_port(priv->line[i]);
+			wakeup |= pci1xxxx_port_suspend(priv->line[i]);
+		}
+	}
+
+	p = pci_ioremap_bar(pcidev, 0);
+	if (!p) {
+		dev_err(dev, "remapping of bar 0 memory failed");
+		return -ENOMEM;
+	}
+
+	data = readl(p + UART_RESET_REG);
+	writel(data | UART_RESET_D3_RESET_DISABLE, p + UART_RESET_REG);
+
+	if (wakeup)
+		writeb(UART_PCI_CTRL_D3_CLK_ENABLE, p + UART_PCI_CTRL_REG);
+
+	iounmap(p);
+	device_set_wakeup_enable(dev, true);
+	pci_wake_from_d3(pcidev, true);
+
+	return 0;
+}
+
+static int pci1xxxx_resume(struct device *dev)
+{
+	struct pci1xxxx_8250 *priv = dev_get_drvdata(dev);
+	struct pci_dev *pcidev = to_pci_dev(dev);
+	unsigned int data;
+	void __iomem *p;
+	int i;
+
+	p = pci_ioremap_bar(pcidev, 0);
+	if (!p) {
+		dev_err(dev, "remapping of bar 0 memory failed");
+		return -ENOMEM;
+	}
+
+	data = readl(p + UART_RESET_REG);
+	writel(data & ~UART_RESET_D3_RESET_DISABLE, p + UART_RESET_REG);
+	iounmap(p);
+
+	for (i = 0; i < priv->nr; i++) {
+		if (priv->line[i] >= 0) {
+			pci1xxxx_port_resume(priv->line[i]);
+			serial8250_resume_port(priv->line[i]);
+		}
+	}
+
+	return 0;
+}
+
 static int pci1xxxx_setup(struct pci1xxxx_8250 *priv,
 			  struct uart_8250_port *port, int port_idx)
 {
@@ -323,6 +433,9 @@ static void pci1xxxx_serial_remove(struct pci_dev *dev)
 	}
 }
 
+static DEFINE_SIMPLE_DEV_PM_OPS(pci1xxxx_pm_ops, pci1xxxx_suspend,
+				pci1xxxx_resume);
+
 static const struct pci_device_id pci1xxxx_pci_tbl[] = {
 	{ PCI_DEVICE(PCI_VENDOR_ID_EFAR, PCI_DEVICE_ID_EFAR_PCI11010) },
 	{ PCI_DEVICE(PCI_VENDOR_ID_EFAR, PCI_DEVICE_ID_EFAR_PCI11101) },
@@ -337,6 +450,9 @@ static struct pci_driver pci1xxxx_pci_driver = {
 	.name = "pci1xxxx serial",
 	.probe = pci1xxxx_serial_probe,
 	.remove = pci1xxxx_serial_remove,
+	.driver = {
+		.pm     = pm_sleep_ptr(&pci1xxxx_pm_ops),
+	},
 	.id_table = pci1xxxx_pci_tbl,
 };
 module_pci_driver(pci1xxxx_pci_driver);
-- 
2.25.1

Re: [PATCH v6 tty-next 1/4] serial: 8250_pci: Add serial8250_pci_setup_port definition in 8250_pcilib.c

[Ilpo Järvinen]

[-- Attachment #1: Type: text/plain, Size: 1322 bytes --]

On Thu, 1 Dec 2022, Kumaravel Thiagarajan wrote:

> Move implementation of setup_port API to serial8250_pci_setup_port
> 
> Co-developed-by: Tharun Kumar P <tharunkumar.pasumarthi@microchip.com>
> Signed-off-by: Tharun Kumar P <tharunkumar.pasumarthi@microchip.com>
> Signed-off-by: Kumaravel Thiagarajan <kumaravel.thiagarajan@microchip.com>
> ---
> Changes in v6:
> - Made this patch first patch of the patch series
> 
> Changes in v5:
> - This is the new patch added in v5 version of this patchset
> - Moved implementation of setup_port from 8250_pci.c to 8250_pcilib.c
> 
> ---

> diff --git a/drivers/tty/serial/8250/8250_pcilib.h b/drivers/tty/serial/8250/8250_pcilib.h
> new file mode 100644
> index 000000000000..41ef01d5c3c5
> --- /dev/null
> +++ b/drivers/tty/serial/8250/8250_pcilib.h
> @@ -0,0 +1,9 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * 8250 PCI library header file.
> + *
> + * Copyright (C) 2001 Russell King, All Rights Reserved.
> + */

You shouldn't depend on .c file having things included for you. So 
please add these:

#include "8250.h"

struct pci_dev;

Other than that,
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>


> +
> +int serial8250_pci_setup_port(struct pci_dev *dev, struct uart_8250_port *port, u8 bar,
> +		   unsigned int offset, int regshift);


-- 
 i.

Re: [PATCH v6 tty-next 2/4] serial: 8250_pci1xxxx: Add driver for quad-uart support.

[Ilpo Järvinen]

On Thu, 1 Dec 2022, Kumaravel Thiagarajan wrote:

> pci1xxxx is a PCIe switch with a multi-function endpoint on one of
> its downstream ports. Quad-uart is one of the functions in the
> multi-function endpoint. This driver loads for the quad-uart and
> enumerates single or multiple instances of uart based on the PCIe
> subsystem device ID.
> 
> Co-developed-by: Tharun Kumar P <tharunkumar.pasumarthi@microchip.com>
> Signed-off-by: Tharun Kumar P <tharunkumar.pasumarthi@microchip.com>
> Signed-off-by: Kumaravel Thiagarajan <kumaravel.thiagarajan@microchip.com>
> ---
> Changes in v6:
> - Removed un-necessary paranthesis
> - Used array and removed switch cases to reduce complexity
> - Handled failure case of pcim_iomap
> 
> Changes in v5:
> - Used tabs instead of spaces in MACRO definitions for readability
> - Removed assignments that are not required
> - Removed redundant blank lines
> 
> Changes in v4:
> - Renamed pci_setup_port to serial8250_pci_setup_port
> - Added Copyright information to 8250_pcilib.c
> 
> Changes in v3:
> - Used NSEC_PER_SEC, HZ_PER_MHZ, FIELD_PREP, FIELD_GET MACROs wherever
>   necessary
> - Handled failure case of serial8250_register_8250_port properly
> - Moved pci_setup_port to 8250_pcilib.c
> 
> Changes in v2:
> - Use only the 62.5 MHz for baud clock.
> - Define custom implementation for get_divisor and set_divisor.
> - Use BOTHER instead of UPF_SPD_CUST for non standard baud rates
>   (untested).
> - Correct indentation in clock divisor computation.
> - Remove unnecessary call to pci_save_state in probe function.
> - Fix null pointer dereference in probe function.
> - Move pci1xxxx_rs485_config to a separate patch.
> - Depends on SERIAL_8250_PCI & default to SERIAL_8250.
> - Change PORT_MCHP16550A to 100 from 124.
> 
> ---
>  MAINTAINERS                             |   7 +
>  drivers/tty/serial/8250/8250_pci1xxxx.c | 298 ++++++++++++++++++++++++
>  drivers/tty/serial/8250/8250_port.c     |   8 +
>  drivers/tty/serial/8250/Kconfig         |  11 +
>  drivers/tty/serial/8250/Makefile        |   1 +
>  include/uapi/linux/serial_core.h        |   3 +
>  6 files changed, 328 insertions(+)
>  create mode 100644 drivers/tty/serial/8250/8250_pci1xxxx.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index d30f26e07cd3..20c868776a5e 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -13434,6 +13434,13 @@ F:	Documentation/devicetree/bindings/nvmem/microchip,sama7g5-otpc.yaml
>  F:	drivers/nvmem/microchip-otpc.c
>  F:	include/dt-bindings/nvmem/microchip,sama7g5-otpc.h
>  
> +MICROCHIP PCIe UART DRIVER
> +M:     Kumaravel Thiagarajan <kumaravel.thiagarajan@microchip.com>
> +M:     Tharun Kumar P <tharunkumar.pasumarthi@microchip.com>
> +L:     linux-serial@vger.kernel.org
> +S:     Maintained
> +F:     drivers/tty/serial/8250/8250_pci1xxxx.c
> +
>  MICROCHIP PWM DRIVER
>  M:	Claudiu Beznea <claudiu.beznea@microchip.com>
>  L:	linux-arm-kernel@lists.infradead.org (moderated for non-subscribers)
> diff --git a/drivers/tty/serial/8250/8250_pci1xxxx.c b/drivers/tty/serial/8250/8250_pci1xxxx.c
> new file mode 100644
> index 000000000000..d5037e76b636
> --- /dev/null
> +++ b/drivers/tty/serial/8250/8250_pci1xxxx.c
> @@ -0,0 +1,298 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + *  Probe module for 8250/16550-type MCHP PCI serial ports.
> + *
> + *  Based on drivers/tty/serial/8250/8250_pci.c,
> + *
> + *  Copyright (C) 2022 Microchip Technology Inc., All Rights Reserved.
> + */
> +
> +#include <linux/bitfield.h>
> +#include <linux/bitops.h>
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/pci.h>
> +#include <linux/serial_core.h>
> +#include <linux/slab.h>
> +#include <linux/string.h>
> +#include <linux/units.h>
> +#include <linux/tty.h>
> +
> +#include <asm/byteorder.h>
> +
> +#include "8250.h"
> +#include "8250_pcilib.h"
> +
> +#define PCI_DEVICE_ID_EFAR_PCI12000		0xa002
> +#define PCI_DEVICE_ID_EFAR_PCI11010		0xa012
> +#define PCI_DEVICE_ID_EFAR_PCI11101		0xa022
> +#define PCI_DEVICE_ID_EFAR_PCI11400		0xa032
> +#define PCI_DEVICE_ID_EFAR_PCI11414		0xa042
> +
> +#define PCI_SUBDEVICE_ID_EFAR_PCI1XXXX_4p	0x0001
> +#define PCI_SUBDEVICE_ID_EFAR_PCI1XXXX_3p012	0x0002
> +#define PCI_SUBDEVICE_ID_EFAR_PCI1XXXX_3p013	0x0003
> +#define PCI_SUBDEVICE_ID_EFAR_PCI1XXXX_3p023	0x0004
> +#define PCI_SUBDEVICE_ID_EFAR_PCI1XXXX_3p123	0x0005
> +#define PCI_SUBDEVICE_ID_EFAR_PCI1XXXX_2p01	0x0006
> +#define PCI_SUBDEVICE_ID_EFAR_PCI1XXXX_2p02	0x0007
> +#define PCI_SUBDEVICE_ID_EFAR_PCI1XXXX_2p03	0x0008
> +#define PCI_SUBDEVICE_ID_EFAR_PCI1XXXX_2p12	0x0009
> +#define PCI_SUBDEVICE_ID_EFAR_PCI1XXXX_2p13	0x000a
> +#define PCI_SUBDEVICE_ID_EFAR_PCI1XXXX_2p23	0x000b
> +#define PCI_SUBDEVICE_ID_EFAR_PCI1XXXX_1p0	0x000c
> +#define PCI_SUBDEVICE_ID_EFAR_PCI1XXXX_1p1	0x000d
> +#define PCI_SUBDEVICE_ID_EFAR_PCI1XXXX_1p2	0x000e
> +#define PCI_SUBDEVICE_ID_EFAR_PCI1XXXX_1p3	0x000f
> +
> +#define PCI_SUBDEVICE_ID_EFAR_PCI12000		0xa002
> +#define PCI_SUBDEVICE_ID_EFAR_PCI11010		0xa012
> +#define PCI_SUBDEVICE_ID_EFAR_PCI11101		0xa022
> +#define PCI_SUBDEVICE_ID_EFAR_PCI11400		0xa032
> +#define PCI_SUBDEVICE_ID_EFAR_PCI11414		0xa042

Since those numbers seems to match the others, could you just do:

#define PCI_SUBDEVICE_ID_EFAR_PCI11414 PCI_DEVICE_ID_EFAR_PCI11414

> +
> +#define UART_ACTV_REG				0x11
> +#define UART_BLOCK_SET_ACTIVE			BIT(0)
> +
> +#define UART_PCI_CTRL_REG			0x80
> +#define UART_PCI_CTRL_SET_MULTIPLE_MSI		BIT(4)
> +#define UART_PCI_CTRL_D3_CLK_ENABLE		BIT(0)
> +
> +#define ADCL_CFG_REG				0x40
> +#define ADCL_CFG_POL_SEL			BIT(2)
> +#define ADCL_CFG_PIN_SEL			BIT(1)
> +#define ADCL_CFG_EN				BIT(0)
> +
> +#define UART_BIT_SAMPLE_CNT			16
> +#define BAUD_CLOCK_DIV_INT_MSK			GENMASK(31, 8)
> +#define ADCL_CFG_RTS_DELAY_MASK			GENMASK(11, 8)
> +#define UART_CLOCK_DEFAULT			(62.5 * HZ_PER_MHZ)

I don't think you want to go into float realm. Just do 62500 * HZ_PER_KHZ

> +
> +#define UART_WAKE_REG				0x8C
> +#define UART_WAKE_MASK_REG			0x90
> +#define UART_WAKE_N_PIN				BIT(2)
> +#define UART_WAKE_NCTS				BIT(1)
> +#define UART_WAKE_INT				BIT(0)
> +#define UART_WAKE_SRCS	\
> +	(UART_WAKE_N_PIN | UART_WAKE_NCTS | UART_WAKE_INT)
> +
> +#define UART_BAUD_CLK_DIVISOR_REG		0x54
> +
> +#define UART_RESET_REG				0x94
> +#define UART_RESET_D3_RESET_DISABLE		BIT(16)
> +
> +#define MAX_PORTS				4
> +
> +struct pci1xxxx_8250 {
> +	struct pci_dev *pdev;
> +	unsigned int nr;
> +	void __iomem *membase;
> +	int line[];
> +};
> +
> +static int pci1xxxx_get_num_ports(struct pci_dev *dev)
> +{
> +	switch (dev->subsystem_device) {
> +	case PCI_SUBDEVICE_ID_EFAR_PCI1XXXX_1p0:
> +	case PCI_SUBDEVICE_ID_EFAR_PCI1XXXX_1p1:
> +	case PCI_SUBDEVICE_ID_EFAR_PCI1XXXX_1p2:
> +	case PCI_SUBDEVICE_ID_EFAR_PCI1XXXX_1p3:
> +	case PCI_SUBDEVICE_ID_EFAR_PCI12000:
> +	case PCI_SUBDEVICE_ID_EFAR_PCI11010:
> +	case PCI_SUBDEVICE_ID_EFAR_PCI11101:
> +	case PCI_SUBDEVICE_ID_EFAR_PCI11400:
> +	default:
> +		return 1;
> +	case PCI_SUBDEVICE_ID_EFAR_PCI1XXXX_2p01:
> +	case PCI_SUBDEVICE_ID_EFAR_PCI1XXXX_2p02:
> +	case PCI_SUBDEVICE_ID_EFAR_PCI1XXXX_2p03:
> +	case PCI_SUBDEVICE_ID_EFAR_PCI1XXXX_2p12:
> +	case PCI_SUBDEVICE_ID_EFAR_PCI1XXXX_2p13:
> +	case PCI_SUBDEVICE_ID_EFAR_PCI1XXXX_2p23:
> +		return 2;
> +	case PCI_SUBDEVICE_ID_EFAR_PCI1XXXX_3p012:
> +	case PCI_SUBDEVICE_ID_EFAR_PCI1XXXX_3p123:
> +	case PCI_SUBDEVICE_ID_EFAR_PCI1XXXX_3p013:
> +	case PCI_SUBDEVICE_ID_EFAR_PCI1XXXX_3p023:
> +		return 3;
> +	case PCI_SUBDEVICE_ID_EFAR_PCI1XXXX_4p:
> +	case PCI_SUBDEVICE_ID_EFAR_PCI11414:
> +		return 4;
> +	}
> +}
> +
> +static unsigned int pci1xxxx_get_divisor(struct uart_port *port,
> +					 unsigned int baud, unsigned int *frac)
> +{
> +	unsigned int quot;
> +
> +	/*
> +	 * Calculate baud rate sampling period in nanoseconds.
> +	 * Fractional part x denotes x/255 parts of a nanosecond.
> +	 */
> +	quot = NSEC_PER_SEC / (baud * UART_BIT_SAMPLE_CNT);
> +	*frac = (NSEC_PER_SEC - quot * baud * UART_BIT_SAMPLE_CNT) /
> +		  UART_BIT_SAMPLE_CNT * 255 / baud;

Is it fine to div first before * 255 from precision point of view?

> +
> +	return quot;
> +}
> +
> +static void pci1xxxx_set_divisor(struct uart_port *port, unsigned int baud,
> +				 unsigned int quot, unsigned int frac)
> +{
> +	writel(FIELD_PREP(BAUD_CLOCK_DIV_INT_MSK, quot) | frac,
> +	       port->membase + UART_BAUD_CLK_DIVISOR_REG);
> +}
> +
> +static int pci1xxxx_setup(struct pci1xxxx_8250 *priv,
> +			  struct uart_8250_port *port, int port_idx)
> +{
> +	int ret;
> +
> +	port->port.flags |= UPF_FIXED_TYPE | UPF_SKIP_TEST;
> +	port->port.type = PORT_MCHP16550A;
> +	port->port.set_termios = serial8250_do_set_termios;
> +	port->port.get_divisor = pci1xxxx_get_divisor;
> +	port->port.set_divisor = pci1xxxx_set_divisor;
> +
> +	ret = serial8250_pci_setup_port(priv->pdev, port, 0, port_idx * 256, 0);
> +	if (ret < 0)
> +		return ret;
> +
> +	writeb(UART_BLOCK_SET_ACTIVE, port->port.membase + UART_ACTV_REG);
> +	writeb(UART_WAKE_SRCS, port->port.membase + UART_WAKE_REG);
> +	writeb(UART_WAKE_N_PIN, port->port.membase + UART_WAKE_MASK_REG);
> +
> +	return 0;
> +}
> +
> +static int pci1xxxx_serial_probe(struct pci_dev *pdev,
> +				 const struct pci_device_id *id)
> +{
> +	static int logical_to_physical_port_idx[][MAX_PORTS] = {
> +		{0, 1, 2, 3},	/* PCI12000 PCI11010 PCI11101 PCI11400 PCI11414 */
> +		{0, 1, 2, 3},	/* PCI4p */
> +		{0, 1, 2, -1},	/* PCI3p012 */
> +		{0, 1, 3, -1},	/* PCI3p013 */
> +		{0, 2, 3, -1},	/* PCI3p023 */
> +		{1, 2, 3, -1},	/* PCI3p123 */
> +		{0, 1, -1, -1},	/* PCI2p01 */
> +		{0, 2, -1, -1},	/* PCI2p02 */
> +		{0, 3, -1, -1},	/* PCI2p03 */
> +		{1, 2, -1, -1},	/* PCI2p12 */
> +		{1, 3, -1, -1},	/* PCI2p13 */
> +		{2, 3, -1, -1},	/* PCI2p23 */
> +		{0, -1, -1, -1},/* PCI1p0 */
> +		{1, -1, -1, -1},/* PCI1p1 */
> +		{2, -1, -1, -1},/* PCI1p2 */
> +		{3, -1, -1, -1}	/* PCI1p3 */

Add comma also to the last item.

This array can go outside of function's scope.

> +	};
> +	struct device *dev = &pdev->dev;
> +	struct pci1xxxx_8250 *priv;
> +	struct uart_8250_port uart;
> +	unsigned int nr_ports, i;
> +	int num_vectors;
> +	int subsys_dev;
> +	int port_idx;
> +	int rc;
> +
> +	rc = pcim_enable_device(pdev);
> +	if (rc)
> +		return rc;
> +
> +	nr_ports = pci1xxxx_get_num_ports(pdev);
> +
> +	priv = devm_kzalloc(dev, struct_size(priv, line, nr_ports), GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +
> +	priv->membase = pcim_iomap(pdev, 0, 0);
> +	if (!priv->membase)
> +		return -ENOMEM;
> +
> +	priv->pdev = pdev;
> +	subsys_dev = priv->pdev->subsystem_device;
> +	priv->nr = nr_ports;
> +	pci_set_master(pdev);
> +
> +	num_vectors = pci_alloc_irq_vectors(pdev, 1, 4, PCI_IRQ_ALL_TYPES);
> +	if (num_vectors < 0)
> +		return num_vectors;
> +
> +	memset(&uart, 0, sizeof(uart));
> +	uart.port.flags = UPF_SHARE_IRQ | UPF_FIXED_PORT;
> +	uart.port.uartclk = UART_CLOCK_DEFAULT;
> +	uart.port.dev = dev;
> +
> +	if (num_vectors == 4)
> +		writeb(UART_PCI_CTRL_SET_MULTIPLE_MSI,
> +		       priv->membase + UART_PCI_CTRL_REG);
> +	else
> +		uart.port.irq = pci_irq_vector(pdev, 0);
> +
> +	for (i = 0; i < nr_ports; i++)
> +		priv->line[i] = -ENOSPC;

-ENODEV might be more appropriate.

> +
> +	for (i = 0; i < nr_ports; i++) {
> +		if (subsys_dev > 0x0f)

> PCI_SUBDEVICE_ID_EFAR_PCI1XXXX_1p3

> +			port_idx = logical_to_physical_port_idx[0][i];
> +		else
> +			port_idx = logical_to_physical_port_idx[subsys_dev][i];
> +
> +		if (num_vectors == 4)
> +			uart.port.irq = pci_irq_vector(priv->pdev, port_idx);
> +
> +		rc = pci1xxxx_setup(priv, &uart, port_idx);
> +		if (rc) {
> +			dev_warn(dev, "Failed to setup port %u\n", i);
> +			continue;
> +		}
> +
> +		priv->line[i] = serial8250_register_8250_port(&uart);
> +		if (priv->line[i] < 0) {
> +			dev_warn(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]);
> +		}
> +	}
> +
> +	pci_set_drvdata(pdev, priv);
> +
> +	return 0;
> +}
> +
> +static void pci1xxxx_serial_remove(struct pci_dev *dev)
> +{
> +	struct pci1xxxx_8250 *priv = pci_get_drvdata(dev);
> +	int i;
> +
> +	for (i = 0; i < priv->nr; i++) {
> +		if (priv->line[i] >= 0)
> +			serial8250_unregister_port(priv->line[i]);
> +	}
> +}
> +
> +static const struct pci_device_id pci1xxxx_pci_tbl[] = {
> +	{ PCI_DEVICE(PCI_VENDOR_ID_EFAR, PCI_DEVICE_ID_EFAR_PCI11010) },
> +	{ PCI_DEVICE(PCI_VENDOR_ID_EFAR, PCI_DEVICE_ID_EFAR_PCI11101) },
> +	{ PCI_DEVICE(PCI_VENDOR_ID_EFAR, PCI_DEVICE_ID_EFAR_PCI11400) },
> +	{ PCI_DEVICE(PCI_VENDOR_ID_EFAR, PCI_DEVICE_ID_EFAR_PCI11414) },
> +	{ PCI_DEVICE(PCI_VENDOR_ID_EFAR, PCI_DEVICE_ID_EFAR_PCI12000) },
> +	{}
> +};
> +MODULE_DEVICE_TABLE(pci, pci1xxxx_pci_tbl);
> +
> +static struct pci_driver pci1xxxx_pci_driver = {
> +	.name = "pci1xxxx serial",
> +	.probe = pci1xxxx_serial_probe,
> +	.remove = pci1xxxx_serial_remove,
> +	.id_table = pci1xxxx_pci_tbl,
> +};
> +module_pci_driver(pci1xxxx_pci_driver);
> +
> +MODULE_DESCRIPTION("Microchip Technology Inc. PCIe to UART module");
> +MODULE_AUTHOR("Kumaravel Thiagarajan <kumaravel.thiagarajan@microchip.com>");
> +MODULE_AUTHOR("Tharun Kumar P <tharunkumar.pasumarthi@microchip.com>");
> +MODULE_LICENSE("GPL");
> diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
> index 1d2a43214b48..ec2fe5fd7b02 100644
> --- a/drivers/tty/serial/8250/8250_port.c
> +++ b/drivers/tty/serial/8250/8250_port.c
> @@ -313,6 +313,14 @@ static const struct serial8250_config uart_config[] = {
>  		.rxtrig_bytes	= {1, 4, 8, 14},
>  		.flags		= UART_CAP_FIFO,
>  	},
> +	[PORT_MCHP16550A] = {
> +		.name           = "MCHP16550A",
> +		.fifo_size      = 256,
> +		.tx_loadsz      = 256,
> +		.fcr            = UART_FCR_ENABLE_FIFO | UART_FCR_R_TRIG_01,
> +		.rxtrig_bytes   = {2, 66, 130, 194},
> +		.flags          = UART_CAP_FIFO,
> +	},
>  };
>  
>  /* Uart divisor latch read */
> diff --git a/drivers/tty/serial/8250/Kconfig b/drivers/tty/serial/8250/Kconfig
> index 3088eaff3ad0..f67542470eae 100644
> --- a/drivers/tty/serial/8250/Kconfig
> +++ b/drivers/tty/serial/8250/Kconfig
> @@ -292,6 +292,17 @@ config SERIAL_8250_HUB6
>  	  To compile this driver as a module, choose M here: the module
>  	  will be called 8250_hub6.
>  
> +config SERIAL_8250_PCI1XXXX
> +	tristate "Microchip 8250 based serial port"
> +	depends on SERIAL_8250_PCI

Do you actually need this any more after pcilib? It's not like all PCI 
UARTs depends on SERIAL_8250_PCI. An alternative might be this:

depends on SERIAL_8250 && PCI


-- 
 i.

Re: [PATCH v6 tty-next 3/4] serial: 8250_pci1xxxx: Add RS485 support to quad-uart driver

[Ilpo Järvinen]

[-- Attachment #1: Type: text/plain, Size: 3808 bytes --]

On Thu, 1 Dec 2022, Kumaravel Thiagarajan wrote:

> pci1xxxx uart supports RS485 mode of operation in the hardware with
> auto-direction control with configurable delay for releasing RTS after
> the transmission. This patch adds support for the RS485 mode.
> 
> Co-developed-by: Tharun Kumar P <tharunkumar.pasumarthi@microchip.com>
> Signed-off-by: Tharun Kumar P <tharunkumar.pasumarthi@microchip.com>
> Signed-off-by: Kumaravel Thiagarajan <kumaravel.thiagarajan@microchip.com>
> ---
> Changes in v6:
> - Modified datatype of delay_in_baud_periods to u64 to avoid overflows
> 
> Changes in v5:
> - Removed unnecessary assignments
> - Corrected styling issues in comments
> 
> Changes in v4:
> - No Change
> 
> Changes in v3:
> - Remove flags sanitization in driver which is taken care in core
> 
> Changes in v2:
> - move pci1xxxx_rs485_config to a separate patch with
>   pci1xxxx_rs485_supported.
> ---
>  drivers/tty/serial/8250/8250_pci1xxxx.c | 49 +++++++++++++++++++++++++
>  1 file changed, 49 insertions(+)
> 
> diff --git a/drivers/tty/serial/8250/8250_pci1xxxx.c b/drivers/tty/serial/8250/8250_pci1xxxx.c
> index d5037e76b636..7585066d6baf 100644
> --- a/drivers/tty/serial/8250/8250_pci1xxxx.c
> +++ b/drivers/tty/serial/8250/8250_pci1xxxx.c
> @@ -145,6 +145,53 @@ static void pci1xxxx_set_divisor(struct uart_port *port, unsigned int baud,
>  	       port->membase + UART_BAUD_CLK_DIVISOR_REG);
>  }
>  
> +static int pci1xxxx_rs485_config(struct uart_port *port,
> +				 struct ktermios *termios,
> +				 struct serial_rs485 *rs485)
> +{
> +	u32 clock_div = readl(port->membase + UART_BAUD_CLK_DIVISOR_REG);

Maybe move this into the block where it's needed?

> +	u64 delay_in_baud_periods;
> +	u32 baud_period_in_ns;
> +	u32 data = 0;

data seems a bit too generic name for a variable? At minimum I'd suggest 
using cfg or mode_cfg (I couldn't guess where ADCL comes from, perhaps it
has some component which would make the variable name better).

> +
> +	/*
> +	 * pci1xxxx's uart hardware supports only RTS delay after
> +	 * Tx and in units of bit times to a maximum of 15
> +	 */
> +	if (rs485->flags & SER_RS485_ENABLED) {
> +		data = ADCL_CFG_EN | ADCL_CFG_PIN_SEL;
> +
> +		if (!(rs485->flags & SER_RS485_RTS_ON_SEND))
> +			data |= ADCL_CFG_POL_SEL;
> +
> +		if (rs485->delay_rts_after_send) {
> +			baud_period_in_ns =
> +				FIELD_GET(BAUD_CLOCK_DIV_INT_MSK, clock_div) *
> +				UART_BIT_SAMPLE_CNT;
> +			delay_in_baud_periods =
> +				rs485->delay_rts_after_send * NSEC_PER_MSEC /
> +				baud_period_in_ns;
> +			delay_in_baud_periods =
> +				min_t(u64, delay_in_baud_periods,
> +				      FIELD_MAX(ADCL_CFG_RTS_DELAY_MASK));
> +			data |= FIELD_PREP(ADCL_CFG_RTS_DELAY_MASK,
> +					   delay_in_baud_periods);
> +			rs485->delay_rts_after_send =
> +				baud_period_in_ns * delay_in_baud_periods /
> +				NSEC_PER_MSEC;
> +		}
> +	}
> +	writel(data, port->membase + ADCL_CFG_REG);
> +	return 0;
> +}
> +
> +static const struct serial_rs485 pci1xxxx_rs485_supported = {
> +	.flags = SER_RS485_ENABLED | SER_RS485_RTS_ON_SEND |
> +		 SER_RS485_RTS_AFTER_SEND,
> +	.delay_rts_after_send = 1,
> +	/* Delay RTS before send is not supported */
> +};
> +
>  static int pci1xxxx_setup(struct pci1xxxx_8250 *priv,
>  			  struct uart_8250_port *port, int port_idx)
>  {
> @@ -155,6 +202,8 @@ static int pci1xxxx_setup(struct pci1xxxx_8250 *priv,
>  	port->port.set_termios = serial8250_do_set_termios;
>  	port->port.get_divisor = pci1xxxx_get_divisor;
>  	port->port.set_divisor = pci1xxxx_set_divisor;
> +	port->port.rs485_config = pci1xxxx_rs485_config;
> +	port->port.rs485_supported = pci1xxxx_rs485_supported;
>  
>  	ret = serial8250_pci_setup_port(priv->pdev, port, 0, port_idx * 256, 0);
>  	if (ret < 0)
> 

Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>

-- 
 i.

Re: [PATCH v6 tty-next 1/4] serial: 8250_pci: Add serial8250_pci_setup_port definition in 8250_pcilib.c

[Andy Shevchenko]

On Thu, Dec 01, 2022 at 10:21:43AM +0530, Kumaravel Thiagarajan wrote:
> Move implementation of setup_port API to serial8250_pci_setup_port

We refer to the functions as func().
Moreover the grammatical period is what each end of sentence should have.

> +EXPORT_SYMBOL_GPL(serial8250_pci_setup_port);

Make it namespaced from day 1.

SERIAL_8250_PCI would be good name for symbol namespaces.


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v6 tty-next 1/4] serial: 8250_pci: Add serial8250_pci_setup_port definition in 8250_pcilib.c

[Andy Shevchenko]

On Thu, Dec 01, 2022 at 11:06:53AM +0200, Ilpo Järvinen wrote:
> On Thu, 1 Dec 2022, Kumaravel Thiagarajan wrote:
...

> > +/*
> > + * 8250 PCI library header file.
> > + *
> > + * Copyright (C) 2001 Russell King, All Rights Reserved.
> > + */
> 
> You shouldn't depend on .c file having things included for you. So
> please add these:

> #include "8250.h"

TBH I don't see how this is used here, but types.h for sure is missing.

> struct pci_dev;

+ blank line

struct uart_8250_port;

> Other than that,
> Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> 
> > +int serial8250_pci_setup_port(struct pci_dev *dev, struct uart_8250_port *port, u8 bar,
> > +		   unsigned int offset, int regshift);


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v6 tty-next 4/4] serial: 8250_pci1xxxx: Add power management functions to quad-uart driver

[Andy Shevchenko]

On Thu, Dec 01, 2022 at 10:21:46AM +0530, Kumaravel Thiagarajan wrote:
> pci1xxxx's quad-uart function has the capability to wake up UART
> from suspend state. Enable wakeup before entering into suspend and
> disable wakeup on resume.

...

> +static DEFINE_SIMPLE_DEV_PM_OPS(pci1xxxx_pm_ops, pci1xxxx_suspend,
> +				pci1xxxx_resume);

One line?


-- 
With Best Regards,
Andy Shevchenko

RE: [PATCH v6 tty-next 2/4] serial: 8250_pci1xxxx: Add driver for quad-uart support.

[Tharunkumar.Pasumarthi]

> From: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> Sent: Thursday, December 1, 2022 3:10 PM
> To: Kumaravel Thiagarajan - I21417
> <Kumaravel.Thiagarajan@microchip.com>
> Subject: Re: [PATCH v6 tty-next 2/4] serial: 8250_pci1xxxx: Add driver for
> quad-uart support.
> 
> > +             {3, -1, -1, -1} /* PCI1p3 */
> 
> Add comma also to the last item.
> 
> This array can go outside of function's scope.

Okay, I will create a new function get_physical_port and declare static array inside the function.
I will call the function to get value from the array whenever required. The scope of the function
Will be throughout the file.

Thanks,
Tharun Kumar P

RE: [PATCH v6 tty-next 1/4] serial: 8250_pci: Add serial8250_pci_setup_port definition in 8250_pcilib.c

[Tharunkumar.Pasumarthi]

> From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Sent: Thursday, December 1, 2022 5:42 PM
> To: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> Subject: Re: [PATCH v6 tty-next 1/4] serial: 8250_pci: Add
> serial8250_pci_setup_port definition in 8250_pcilib.c
> 
> > struct pci_dev;
> 
> + blank line
> 
> struct uart_8250_port;

Hi Andy,
Is the blank line required here?

Thanks,
Tharun Kumar P

Re: [PATCH v6 tty-next 1/4] serial: 8250_pci: Add serial8250_pci_setup_port definition in 8250_pcilib.c

[Andy Shevchenko]

On Tue, Dec 06, 2022 at 07:09:29PM +0000, Tharunkumar.Pasumarthi@microchip.com wrote:
> > From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > Sent: Thursday, December 1, 2022 5:42 PM

> > > struct pci_dev;
> > 
> > + blank line
> > 
> > struct uart_8250_port;
> 
> Is the blank line required here?

Strictly speaking no, it's not required. But it shows the group of generic
forward declarations and specific to the topic (driver / subsystem).

-- 
With Best Regards,
Andy Shevchenko

RE: [PATCH v6 tty-next 3/4] serial: 8250_pci1xxxx: Add RS485 support to quad-uart driver

[Kumaravel.Thiagarajan]

> -----Original Message-----
> From: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> Sent: Thursday, December 1, 2022 3:17 PM
> To: Kumaravel Thiagarajan - I21417 <Kumaravel.Thiagarajan@microchip.com>
> Subject: Re: [PATCH v6 tty-next 3/4] serial: 8250_pci1xxxx: Add RS485
> support to quad-uart driver
> 
> On Thu, 1 Dec 2022, Kumaravel Thiagarajan wrote:
> 
> > pci1xxxx uart supports RS485 mode of operation in the hardware with
> > auto-direction control with configurable delay for releasing RTS after
> > the transmission. This patch adds support for the RS485 mode.
> >
> >
> > +static int pci1xxxx_rs485_config(struct uart_port *port,
> > +                              struct ktermios *termios,
> > +                              struct serial_rs485 *rs485) {
> > +     u32 clock_div = readl(port->membase +
> > +UART_BAUD_CLK_DIVISOR_REG);
> 
> Maybe move this into the block where it's needed?
> 
> > +     u64 delay_in_baud_periods;
> > +     u32 baud_period_in_ns;
> > +     u32 data = 0;
> 
> data seems a bit too generic name for a variable? At minimum I'd suggest
> using cfg or mode_cfg (I couldn't guess where ADCL comes from, perhaps it
> has some component which would make the variable name better).

Hi Ilpo,

We just noticed after submitting v7 that this email of yours got forwarded to spam folder by our IT infrastructure automatically and we missed it.
We will take care of this in v8.

Thank You.

Regards,
Kumar