Re: [PATCH v2 tty-next 1/3] 8250: microchip: pci1xxxx: Add driver for quad-uart support.

From: Ilpo Järvinen
Date: Mon Oct 03 2022 - 05:19:01 EST

On Sat, 1 Oct 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.
>
> Signed-off-by: Kumaravel Thiagarajan <kumaravel.thiagarajan@xxxxxxxxxxxxx>
> ---
> 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 | 6 +
> drivers/tty/serial/8250/8250_pci1xxxx.c | 394 ++++++++++++++++++++++++
> drivers/tty/serial/8250/8250_port.c | 8 +
> drivers/tty/serial/8250/Kconfig | 10 +
> drivers/tty/serial/8250/Makefile | 1 +
> include/uapi/linux/serial_core.h | 3 +
> 6 files changed, 422 insertions(+)
> create mode 100644 drivers/tty/serial/8250/8250_pci1xxxx.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index d30f26e07cd3..3390693d57ae 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -218,6 +218,12 @@ T: git git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty.git
> F: drivers/tty/serial/8250*
> F: include/linux/serial_8250.h
>
> +MICROCHIP PCIe UART DRIVER
> +M: Kumaravel Thiagarajan <kumaravel.thiagarajan@xxxxxxxxxxxxx>
> +L: linux-serial@xxxxxxxxxxxxxxx
> +S: Maintained
> +F: drivers/tty/serial/8250/8250_pci1xxxx.c
> +
> 8390 NETWORK DRIVERS [WD80x3/SMC-ELITE, SMC-ULTRA, NE2000, 3C503, etc.]
> L: netdev@xxxxxxxxxxxxxxx
> S: Orphan / Obsolete
> diff --git a/drivers/tty/serial/8250/8250_pci1xxxx.c b/drivers/tty/serial/8250/8250_pci1xxxx.c
> new file mode 100644
> index 000000000000..41a4b94f52b4
> --- /dev/null
> +++ b/drivers/tty/serial/8250/8250_pci1xxxx.c
> @@ -0,0 +1,394 @@

> +#define PCI_VENDOR_ID_MCHP_PCI1XXXX 0x1055
> +
> +#define PCI_DEVICE_ID_MCHP_PCI12000 0xA002
> +#define PCI_DEVICE_ID_MCHP_PCI11010 0xA012
> +#define PCI_DEVICE_ID_MCHP_PCI11101 0xA022
> +#define PCI_DEVICE_ID_MCHP_PCI11400 0xA032
> +#define PCI_DEVICE_ID_MCHP_PCI11414 0xA042
> +
> +#define PCI_SUBDEVICE_ID_MCHP_PCI1XXXX_4p 0x0001
> +#define PCI_SUBDEVICE_ID_MCHP_PCI1XXXX_3p012 0x0002
> +#define PCI_SUBDEVICE_ID_MCHP_PCI1XXXX_3p013 0x0003
> +#define PCI_SUBDEVICE_ID_MCHP_PCI1XXXX_3p023 0x0004
> +#define PCI_SUBDEVICE_ID_MCHP_PCI1XXXX_3p123 0x0005
> +#define PCI_SUBDEVICE_ID_MCHP_PCI1XXXX_2p01 0x0006
> +#define PCI_SUBDEVICE_ID_MCHP_PCI1XXXX_2p02 0x0007
> +#define PCI_SUBDEVICE_ID_MCHP_PCI1XXXX_2p03 0x0008
> +#define PCI_SUBDEVICE_ID_MCHP_PCI1XXXX_2p12 0x0009
> +#define PCI_SUBDEVICE_ID_MCHP_PCI1XXXX_2p13 0x000A
> +#define PCI_SUBDEVICE_ID_MCHP_PCI1XXXX_2p23 0x000B
> +#define PCI_SUBDEVICE_ID_MCHP_PCI1XXXX_1p0 0x000C
> +#define PCI_SUBDEVICE_ID_MCHP_PCI1XXXX_1p1 0x000D
> +#define PCI_SUBDEVICE_ID_MCHP_PCI1XXXX_1p2 0x000E
> +#define PCI_SUBDEVICE_ID_MCHP_PCI1XXXX_1p3 0x000F
> +
> +#define PCI_SUBDEVICE_ID_MCHP_PCI12000 0xA002
> +#define PCI_SUBDEVICE_ID_MCHP_PCI11010 0xA012
> +#define PCI_SUBDEVICE_ID_MCHP_PCI11101 0xA022
> +#define PCI_SUBDEVICE_ID_MCHP_PCI11400 0xA032
> +#define PCI_SUBDEVICE_ID_MCHP_PCI11414 0xA042

Usually lowercase is used for hexadecimal letters.

> +#define UART_ACTV_REG 0x11
> +#define UART_ACTV_SET_ACTIVE 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 CLK_SEL_REG 0x50
> +#define CLK_SEL_MASK GENMASK(1, 0)
> +#define CLK_SEL_166MHZ 0x01
> +#define CLK_SEL_500MHZ 0x02

FIELD_PREP(CLK_SEL_MASK, ..) for thse two.

> +
> +#define CLK_DIVISOR_REG 0x54
> +
> +#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 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_RESET_REG 0x94
> +#define UART_RESET_D3_RESET_DISABLE BIT(16)
> +
> +#define UART_BIT_SAMPLE_CNT 16
> +
> +struct pci1xxxx_8250 {
> + struct pci_dev *dev;
> + unsigned int nr;
> + void __iomem *membase;
> + int line[];
> +};

> +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 nano seconds.
> + * Fractional part x denotes x/255 parts of a nano second.
> + */
> +
> + quot = ((1000000000) / (baud * UART_BIT_SAMPLE_CNT));
> + *frac = (((1000000000 - (quot * baud * UART_BIT_SAMPLE_CNT)) /

NSEC_PER_SEC

> + 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((quot << 8) | frac, (port->membase + CLK_DIVISOR_REG));

Define mask for quotient part and use FIELD_PREP().

Remove extra parenthesis.

> +static int pci1xxxx_serial_probe(struct pci_dev *dev,
> + const struct pci_device_id *ent)
> +{
> + struct pci1xxxx_8250 *priv;
> + struct uart_8250_port uart;
> + unsigned int nr_ports, i;
> + int num_vectors = 0;
> + int rc;
> +
> + rc = pcim_enable_device(dev);
> + if (rc)
> + return rc;
> +
> + nr_ports = pci1xxxx_get_num_ports(dev);
> +
> + priv = devm_kzalloc(&dev->dev, struct_size(priv, line, nr_ports), GFP_KERNEL);
> + if (!priv)
> + return -ENOMEM;
> +
> + priv->membase = pcim_iomap(dev, 0, 0);
> + priv->dev = dev;
> + priv->nr = nr_ports;

Extra space.

> +
> + pci_set_master(dev);
> +
> + num_vectors = pci_alloc_irq_vectors(dev, 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_TYPE | UPF_FIXED_PORT;
> + uart.port.uartclk = 62500000;
> + uart.port.dev = &dev->dev;
> +
> + if (num_vectors == 4)
> + writeb(UART_PCI_CTRL_SET_MULTIPLE_MSI, (priv->membase + UART_PCI_CTRL_REG));

Extra parenthesis.

--
i.