Re: [PATCH v5 tty-next 2/4] 8250: microchip: pci1xxxx: Add serial8250_pci_setup_port definition in 8250_pcilib.c

["Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>]

On Thu, 17 Nov 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 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_pci1xxxx.c |  6 ++++
>  drivers/tty/serial/8250/8250_pcilib.c   | 38 +++++++++++++++++++++++++
>  drivers/tty/serial/8250/8250_pcilib.h   |  9 ++++++
>  drivers/tty/serial/8250/Kconfig         |  5 ++++
>  drivers/tty/serial/8250/Makefile        |  1 +
>  6 files changed, 61 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_pci1xxxx.c b/drivers/tty/serial/8250/8250_pci1xxxx.c
> index 9dd7aca76e58..02b9c6959dcc 100644
> --- a/drivers/tty/serial/8250/8250_pci1xxxx.c
> +++ b/drivers/tty/serial/8250/8250_pci1xxxx.c
> @@ -22,6 +22,7 @@
>  #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
> @@ -143,6 +144,7 @@ static int pci1xxxx_setup(struct pci1xxxx_8250 *priv,
>  {
>  	int first_offset;
>  	int offset;
> +	int ret;
>  
>  	switch (priv->dev->subsystem_device) {
>  	case PCI_SUBDEVICE_ID_EFAR_PCI1XXXX_1p1:
> @@ -191,6 +193,10 @@ 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;
> +	ret = serial8250_pci_setup_port(priv->dev, port, 0, offset, 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);
> 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 1c41722d8ac5..f67542470eae 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
> @@ -294,6 +295,7 @@ config SERIAL_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
> @@ -510,6 +512,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 fbc7d47c25bd..98202fdf39f8 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

You should swap patches 1/4 and 2/4 order so that the pcilib is added 
first so that you can use it directly when adding your driver.

-- 
 i.