| Message ID | 1460061433-63750-10-git-send-email-andriy.shevchenko@linux.intel.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
On 04/07/2016 01:37 PM, Andy Shevchenko wrote: > Intes SoCs, such as Braswell, have DesignWare UART. Split out to separate > module which also will be used for Intel Quark later. What's the rationale? And this really isn't a split; this patch introduces a number of significant changes from the pci version. > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > --- > drivers/tty/serial/8250/8250_lpss.c | 279 ++++++++++++++++++++++++++++++++++++ > drivers/tty/serial/8250/8250_pci.c | 227 ++--------------------------- > drivers/tty/serial/8250/Kconfig | 14 +- > drivers/tty/serial/8250/Makefile | 1 + > 4 files changed, 301 insertions(+), 220 deletions(-) > create mode 100644 drivers/tty/serial/8250/8250_lpss.c > > diff --git a/drivers/tty/serial/8250/8250_lpss.c b/drivers/tty/serial/8250/8250_lpss.c > new file mode 100644 > index 0000000..bca4adb > --- /dev/null > +++ b/drivers/tty/serial/8250/8250_lpss.c > @@ -0,0 +1,279 @@ > +/* > + * 8250_lpss.c - Driver for UART on Intel Braswell and various other Intel SoCs > + * > + * Copyright (C) 2016 Intel Corporation > + * Author: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 as > + * published by the Free Software Foundation. > + */ > + > +#include <linux/bitops.h> > +#include <linux/module.h> > +#include <linux/pci.h> > +#include <linux/rational.h> > + > +#include <linux/dmaengine.h> > +#include <linux/platform_data/dma-dw.h> > + > +#include "8250.h" > + > +#define PCI_DEVICE_ID_INTEL_BYT_UART1 0x0f0a > +#define PCI_DEVICE_ID_INTEL_BYT_UART2 0x0f0c > + > +#define PCI_DEVICE_ID_INTEL_BSW_UART1 0x228a > +#define PCI_DEVICE_ID_INTEL_BSW_UART2 0x228c > + > +#define PCI_DEVICE_ID_INTEL_BDW_UART1 0x9ce3 > +#define PCI_DEVICE_ID_INTEL_BDW_UART2 0x9ce4 > + > +/* Intel LPSS specific registers */ > + > +#define BYT_PRV_CLK 0x800 > +#define BYT_PRV_CLK_EN BIT(0) > +#define BYT_PRV_CLK_M_VAL_SHIFT 1 > +#define BYT_PRV_CLK_N_VAL_SHIFT 16 > +#define BYT_PRV_CLK_UPDATE BIT(31) > + > +#define BYT_TX_OVF_INT 0x820 > +#define BYT_TX_OVF_INT_MASK BIT(1) > + > +struct lpss8250; > + > +struct lpss8250_board { > + unsigned long freq; > + unsigned int base_baud; > + int (*setup)(struct lpss8250 *, struct uart_port *p); > +}; New concept. > + > +struct lpss8250 { > + int line; > + struct lpss8250_board *board; > + > + /* DMA parameters */ > + struct uart_8250_dma dma; > + struct dw_dma_slave dma_param; > + u8 dma_maxburst; > +}; > + > +/*****************************************************************************/ Please remove. > + > +static void lpss8250_set_termios(struct uart_port *p, > + struct ktermios *termios, > + struct ktermios *old) > +{ > + unsigned int baud = tty_termios_baud_rate(termios); > + struct lpss8250 *lpss = p->private_data; > + unsigned long fref = lpss->board->freq, fuart = baud * 16; > + unsigned long w = BIT(15) - 1; > + unsigned long m, n; > + u32 reg; > + > + /* Get Fuart closer to Fref */ > + fuart *= rounddown_pow_of_two(fref / fuart); > + > + /* > + * For baud rates 0.5M, 1M, 1.5M, 2M, 2.5M, 3M, 3.5M and 4M the > + * dividers must be adjusted. > + * > + * uartclk = (m / n) * 100 MHz, where m <= n > + */ > + rational_best_approximation(fuart, fref, w, w, &m, &n); > + p->uartclk = fuart; > + > + /* Reset the clock */ > + reg = (m << BYT_PRV_CLK_M_VAL_SHIFT) | (n << BYT_PRV_CLK_N_VAL_SHIFT); > + writel(reg, p->membase + BYT_PRV_CLK); > + reg |= BYT_PRV_CLK_EN | BYT_PRV_CLK_UPDATE; > + writel(reg, p->membase + BYT_PRV_CLK); > + > + p->status &= ~UPSTAT_AUTOCTS; > + if (termios->c_cflag & CRTSCTS) > + p->status |= UPSTAT_AUTOCTS; > + > + serial8250_do_set_termios(p, termios, old); > +} > + > +/*****************************************************************************/ Please remove. > + > +static int byt_serial_setup(struct lpss8250 *lpss, struct uart_port *port) This would have been much easier to review if you had simply moved it here and done the rework in a follow-on patch. > +{ > + struct dw_dma_slave *param = &lpss->dma_param; > + struct uart_8250_port *up = up_to_u8250p(port); > + struct pci_dev *pdev = to_pci_dev(port->dev); > + struct pci_dev *dma_dev = pci_get_slot(pdev->bus, PCI_DEVFN(PCI_SLOT(pdev->devfn), 0)); > + > + switch (pdev->device) { > + case PCI_DEVICE_ID_INTEL_BYT_UART1: > + case PCI_DEVICE_ID_INTEL_BSW_UART1: > + case PCI_DEVICE_ID_INTEL_BDW_UART1: > + param->src_id = 3; > + param->dst_id = 2; > + break; > + case PCI_DEVICE_ID_INTEL_BYT_UART2: > + case PCI_DEVICE_ID_INTEL_BSW_UART2: > + case PCI_DEVICE_ID_INTEL_BDW_UART2: > + param->src_id = 5; > + param->dst_id = 4; > + break; > + default: > + return -EINVAL; > + } > + > + param->dma_dev = &dma_dev->dev; > + param->m_master = 0; > + param->p_master = 1; > + > + /* TODO: Detect FIFO size automaticaly for DesignWare 8250 */ > + port->fifosize = 64; > + up->tx_loadsz = 64; > + > + lpss->dma_maxburst = 16; > + > + port->set_termios = lpss8250_set_termios; > + > + /* Disable TX counter interrupts */ > + writel(BYT_TX_OVF_INT_MASK, port->membase + BYT_TX_OVF_INT); > + > + return 0; > +} > + > +/*****************************************************************************/ Please remove. > + > +static bool lpss8250_dma_filter(struct dma_chan *chan, void *param) > +{ > + struct dw_dma_slave *dws = param; > + > + if (dws->dma_dev != chan->device->dev) > + return false; > + > + chan->private = dws; > + return true; > +} > + > +static int lpss8250_dma_setup(struct lpss8250 *lpss, struct uart_8250_port *port) > +{ > + struct uart_8250_dma *dma = &lpss->dma; > + struct dw_dma_slave *param = &lpss->dma_param; Unnecessary alias. > + struct dw_dma_slave *rx_param, *tx_param; > + struct device *dev = port->port.dev; > + > + rx_param = devm_kzalloc(dev, sizeof(*rx_param), GFP_KERNEL); > + if (!rx_param) > + return -ENOMEM; > + > + tx_param = devm_kzalloc(dev, sizeof(*tx_param), GFP_KERNEL); > + if (!tx_param) > + return -ENOMEM; > + > + *rx_param = *param; > + dma->rxconf.src_maxburst = lpss->dma_maxburst; > + > + *tx_param = *param; > + dma->txconf.dst_maxburst = lpss->dma_maxburst; > + > + dma->fn = lpss8250_dma_filter; > + dma->rx_param = rx_param; > + dma->tx_param = tx_param; > + > + port->dma = dma; > + return 0; > +} > + > +/*****************************************************************************/ Please remove. > + > +static int lpss8250_probe(struct pci_dev *pdev, const struct pci_device_id *id) > +{ > + struct uart_8250_port uart; > + struct lpss8250 *lpss; > + int ret; > + > + ret = pcim_enable_device(pdev); > + if (ret) > + return ret; > + > + pci_set_master(pdev); > + > + lpss = devm_kzalloc(&pdev->dev, sizeof(*lpss), GFP_KERNEL); > + if (!lpss) > + return -ENOMEM; > + > + lpss->board = (struct lpss8250_board *)id->driver_data; > + > + memset(&uart, 0, sizeof(struct uart_8250_port)); > + > + uart.port.dev = &pdev->dev; > + uart.port.irq = pdev->irq; > + uart.port.private_data = lpss; > + uart.port.type = PORT_16550A; > + uart.port.iotype = UPIO_MEM32; UPIO_MEM > + uart.port.regshift = 2; > + uart.port.uartclk = lpss->board->base_baud * 16; > + uart.port.flags = UPF_SHARE_IRQ | UPF_FIXED_PORT | UPF_FIXED_TYPE; > + Please remove empty line > + uart.capabilities = UART_CAP_FIFO | UART_CAP_AFE; > + Same > + uart.port.mapbase = pci_resource_start(pdev, 0); > + uart.port.membase = pcim_iomap(pdev, 0, 0); > + if (!uart.port.membase) > + return -ENOMEM; > + > + if (lpss->board->setup) { There's no design that doesn't have a setup method. > + ret = lpss->board->setup(lpss, &uart.port); > + if (ret) > + return ret; > + } > + > + ret = lpss8250_dma_setup(lpss, &uart); > + if (ret) > + return ret; I would fold this call into board setup which avoids the ugliness when this error pathway is reworked in the follow-on patches. > + > + ret = serial8250_register_8250_port(&uart); > + if (ret < 0) > + return ret; > + > + lpss->line = ret; > + > + pci_set_drvdata(pdev, lpss); > + return 0; > +} > + > +static void lpss8250_remove(struct pci_dev *pdev) > +{ > + struct lpss8250 *lpss = pci_get_drvdata(pdev); > + > + serial8250_unregister_port(lpss->line); > +} > + > +static const struct lpss8250_board byt_board = { > + .freq = 100000000, > + .base_baud = 2764800, > + .setup = byt_serial_setup, > +}; > + > +#define LPSS_DEVICE(id, board) { PCI_VDEVICE(INTEL, id), (kernel_ulong_t)&board } > + > +static const struct pci_device_id pci_ids[] = { > + LPSS_DEVICE(PCI_DEVICE_ID_INTEL_BYT_UART1, byt_board), > + LPSS_DEVICE(PCI_DEVICE_ID_INTEL_BYT_UART2, byt_board), > + LPSS_DEVICE(PCI_DEVICE_ID_INTEL_BSW_UART1, byt_board), > + LPSS_DEVICE(PCI_DEVICE_ID_INTEL_BSW_UART2, byt_board), > + LPSS_DEVICE(PCI_DEVICE_ID_INTEL_BDW_UART1, byt_board), > + LPSS_DEVICE(PCI_DEVICE_ID_INTEL_BDW_UART2, byt_board), > + { }, > +}; > +MODULE_DEVICE_TABLE(pci, pci_ids); > + > +static struct pci_driver lpss8250_pci_driver = { > + .name = "8250_lpss", > + .id_table = pci_ids, > + .probe = lpss8250_probe, > + .remove = lpss8250_remove, No power management? > +}; > + > +module_pci_driver(lpss8250_pci_driver); > + > +MODULE_AUTHOR("Intel Corporation"); > +MODULE_LICENSE("GPL v2"); > +MODULE_DESCRIPTION("Intel LPSS UART driver"); > diff --git a/drivers/tty/serial/8250/8250_pci.c b/drivers/tty/serial/8250/8250_pci.c > index 5eea74d..bb4df5d 100644 > --- a/drivers/tty/serial/8250/8250_pci.c > +++ b/drivers/tty/serial/8250/8250_pci.c > @@ -21,14 +21,10 @@ > #include <linux/serial_core.h> > #include <linux/8250_pci.h> > #include <linux/bitops.h> > -#include <linux/rational.h> > > #include <asm/byteorder.h> > #include <asm/io.h> > > -#include <linux/dmaengine.h> > -#include <linux/platform_data/dma-dw.h> > - > #include "8250.h" > > /* > @@ -1349,145 +1345,6 @@ ce4100_serial_setup(struct serial_private *priv, > return ret; > } > > -#define PCI_DEVICE_ID_INTEL_BYT_UART1 0x0f0a > -#define PCI_DEVICE_ID_INTEL_BYT_UART2 0x0f0c > - > -#define PCI_DEVICE_ID_INTEL_BSW_UART1 0x228a > -#define PCI_DEVICE_ID_INTEL_BSW_UART2 0x228c > - > -#define PCI_DEVICE_ID_INTEL_BDW_UART1 0x9ce3 > -#define PCI_DEVICE_ID_INTEL_BDW_UART2 0x9ce4 > - > -#define BYT_PRV_CLK 0x800 > -#define BYT_PRV_CLK_EN (1 << 0) > -#define BYT_PRV_CLK_M_VAL_SHIFT 1 > -#define BYT_PRV_CLK_N_VAL_SHIFT 16 > -#define BYT_PRV_CLK_UPDATE (1 << 31) > - > -#define BYT_TX_OVF_INT 0x820 > -#define BYT_TX_OVF_INT_MASK (1 << 1) > - > -static void > -byt_set_termios(struct uart_port *p, struct ktermios *termios, > - struct ktermios *old) > -{ > - unsigned int baud = tty_termios_baud_rate(termios); > - unsigned long fref = 100000000, fuart = baud * 16; > - unsigned long w = BIT(15) - 1; > - unsigned long m, n; > - u32 reg; > - > - /* Get Fuart closer to Fref */ > - fuart *= rounddown_pow_of_two(fref / fuart); > - > - /* > - * For baud rates 0.5M, 1M, 1.5M, 2M, 2.5M, 3M, 3.5M and 4M the > - * dividers must be adjusted. > - * > - * uartclk = (m / n) * 100 MHz, where m <= n > - */ > - rational_best_approximation(fuart, fref, w, w, &m, &n); > - p->uartclk = fuart; > - > - /* Reset the clock */ > - reg = (m << BYT_PRV_CLK_M_VAL_SHIFT) | (n << BYT_PRV_CLK_N_VAL_SHIFT); > - writel(reg, p->membase + BYT_PRV_CLK); > - reg |= BYT_PRV_CLK_EN | BYT_PRV_CLK_UPDATE; > - writel(reg, p->membase + BYT_PRV_CLK); > - > - p->status &= ~UPSTAT_AUTOCTS; > - if (termios->c_cflag & CRTSCTS) > - p->status |= UPSTAT_AUTOCTS; > - > - serial8250_do_set_termios(p, termios, old); > -} > - > -static bool byt_dma_filter(struct dma_chan *chan, void *param) > -{ > - struct dw_dma_slave *dws = param; > - > - if (dws->dma_dev != chan->device->dev) > - return false; > - > - chan->private = dws; > - return true; > -} > - > -static int > -byt_serial_setup(struct serial_private *priv, > - const struct pciserial_board *board, > - struct uart_8250_port *port, int idx) > -{ > - struct pci_dev *pdev = priv->dev; > - struct device *dev = port->port.dev; > - struct uart_8250_dma *dma; > - struct dw_dma_slave *tx_param, *rx_param; > - struct pci_dev *dma_dev; > - int ret; > - > - dma = devm_kzalloc(dev, sizeof(*dma), GFP_KERNEL); > - if (!dma) > - return -ENOMEM; > - > - tx_param = devm_kzalloc(dev, sizeof(*tx_param), GFP_KERNEL); > - if (!tx_param) > - return -ENOMEM; > - > - rx_param = devm_kzalloc(dev, sizeof(*rx_param), GFP_KERNEL); > - if (!rx_param) > - return -ENOMEM; > - > - switch (pdev->device) { > - case PCI_DEVICE_ID_INTEL_BYT_UART1: > - case PCI_DEVICE_ID_INTEL_BSW_UART1: > - case PCI_DEVICE_ID_INTEL_BDW_UART1: > - rx_param->src_id = 3; > - tx_param->dst_id = 2; > - break; > - case PCI_DEVICE_ID_INTEL_BYT_UART2: > - case PCI_DEVICE_ID_INTEL_BSW_UART2: > - case PCI_DEVICE_ID_INTEL_BDW_UART2: > - rx_param->src_id = 5; > - tx_param->dst_id = 4; > - break; > - default: > - return -EINVAL; > - } > - > - rx_param->m_master = 0; > - rx_param->p_master = 1; > - > - dma->rxconf.src_maxburst = 16; > - > - tx_param->m_master = 0; > - tx_param->p_master = 1; > - > - dma->txconf.dst_maxburst = 16; > - > - dma_dev = pci_get_slot(pdev->bus, PCI_DEVFN(PCI_SLOT(pdev->devfn), 0)); > - rx_param->dma_dev = &dma_dev->dev; > - tx_param->dma_dev = &dma_dev->dev; > - > - dma->fn = byt_dma_filter; > - dma->rx_param = rx_param; > - dma->tx_param = tx_param; > - > - ret = pci_default_setup(priv, board, port, idx); > - port->port.iotype = UPIO_MEM; > - port->port.type = PORT_16550A; > - port->port.flags = (port->port.flags | UPF_FIXED_PORT | UPF_FIXED_TYPE); > - port->port.set_termios = byt_set_termios; > - port->port.fifosize = 64; > - port->tx_loadsz = 64; > - port->dma = dma; > - port->capabilities = UART_CAP_FIFO | UART_CAP_AFE; > - > - /* Disable Tx counter interrupts */ > - writel(BYT_TX_OVF_INT_MASK, port->port.membase + BYT_TX_OVF_INT); > - > - return ret; > -} > - > static int > pci_omegapci_setup(struct serial_private *priv, > const struct pciserial_board *board, > @@ -2015,48 +1872,6 @@ static struct pci_serial_quirk pci_serial_quirks[] __refdata = { > .subdevice = PCI_ANY_ID, > .setup = kt_serial_setup, > }, > - { > - .vendor = PCI_VENDOR_ID_INTEL, > - .device = PCI_DEVICE_ID_INTEL_BYT_UART1, > - .subvendor = PCI_ANY_ID, > - .subdevice = PCI_ANY_ID, > - .setup = byt_serial_setup, > - }, > - { > - .vendor = PCI_VENDOR_ID_INTEL, > - .device = PCI_DEVICE_ID_INTEL_BYT_UART2, > - .subvendor = PCI_ANY_ID, > - .subdevice = PCI_ANY_ID, > - .setup = byt_serial_setup, > - }, > - { > - .vendor = PCI_VENDOR_ID_INTEL, > - .device = PCI_DEVICE_ID_INTEL_BSW_UART1, > - .subvendor = PCI_ANY_ID, > - .subdevice = PCI_ANY_ID, > - .setup = byt_serial_setup, > - }, > - { > - .vendor = PCI_VENDOR_ID_INTEL, > - .device = PCI_DEVICE_ID_INTEL_BSW_UART2, > - .subvendor = PCI_ANY_ID, > - .subdevice = PCI_ANY_ID, > - .setup = byt_serial_setup, > - }, > - { > - .vendor = PCI_VENDOR_ID_INTEL, > - .device = PCI_DEVICE_ID_INTEL_BDW_UART1, > - .subvendor = PCI_ANY_ID, > - .subdevice = PCI_ANY_ID, > - .setup = byt_serial_setup, > - }, > - { > - .vendor = PCI_VENDOR_ID_INTEL, > - .device = PCI_DEVICE_ID_INTEL_BDW_UART2, > - .subvendor = PCI_ANY_ID, > - .subdevice = PCI_ANY_ID, > - .setup = byt_serial_setup, > - }, > /* > * ITE > */ > @@ -2921,7 +2736,6 @@ enum pci_board_num_t { > pbn_ADDIDATA_PCIe_4_3906250, > pbn_ADDIDATA_PCIe_8_3906250, > pbn_ce4100_1_115200, > - pbn_byt, > pbn_qrk, > pbn_omegapci, > pbn_NETMOS9900_2s_115200, > @@ -3698,12 +3512,6 @@ static struct pciserial_board pci_boards[] = { > .base_baud = 921600, > .reg_shift = 2, > }, > - [pbn_byt] = { > - .flags = FL_BASE0, > - .num_ports = 1, > - .base_baud = 2764800, > - .reg_shift = 2, > - }, > [pbn_qrk] = { > .flags = FL_BASE0, > .num_ports = 1, > @@ -3820,6 +3628,14 @@ static const struct pci_device_id blacklist[] = { > { PCI_VDEVICE(INTEL, 0x081d), }, > { PCI_VDEVICE(INTEL, 0x1191), }, > { PCI_VDEVICE(INTEL, 0x19d8), }, > + > + /* Intel platforms with DesignWare UART */ > + { PCI_VDEVICE(INTEL, 0x0f0a), }, > + { PCI_VDEVICE(INTEL, 0x0f0c), }, > + { PCI_VDEVICE(INTEL, 0x228a), }, > + { PCI_VDEVICE(INTEL, 0x228c), }, > + { PCI_VDEVICE(INTEL, 0x9ce3), }, > + { PCI_VDEVICE(INTEL, 0x9ce4), }, > }; > > /* > @@ -5485,33 +5301,6 @@ static struct pci_device_id serial_pci_tbl[] = { > { PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_CE4100_UART, > PCI_ANY_ID, PCI_ANY_ID, 0, 0, > pbn_ce4100_1_115200 }, > - /* Intel BayTrail */ > - { PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_BYT_UART1, > - PCI_ANY_ID, PCI_ANY_ID, > - PCI_CLASS_COMMUNICATION_SERIAL << 8, 0xff0000, > - pbn_byt }, > - { PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_BYT_UART2, > - PCI_ANY_ID, PCI_ANY_ID, > - PCI_CLASS_COMMUNICATION_SERIAL << 8, 0xff0000, > - pbn_byt }, > - { PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_BSW_UART1, > - PCI_ANY_ID, PCI_ANY_ID, > - PCI_CLASS_COMMUNICATION_SERIAL << 8, 0xff0000, > - pbn_byt }, > - { PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_BSW_UART2, > - PCI_ANY_ID, PCI_ANY_ID, > - PCI_CLASS_COMMUNICATION_SERIAL << 8, 0xff0000, > - pbn_byt }, > - > - /* Intel Broadwell */ > - { PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_BDW_UART1, > - PCI_ANY_ID, PCI_ANY_ID, > - PCI_CLASS_COMMUNICATION_SERIAL << 8, 0xff0000, > - pbn_byt }, > - { PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_BDW_UART2, > - PCI_ANY_ID, PCI_ANY_ID, > - PCI_CLASS_COMMUNICATION_SERIAL << 8, 0xff0000, > - pbn_byt }, > > /* > * Intel Quark x1000 > diff --git a/drivers/tty/serial/8250/Kconfig b/drivers/tty/serial/8250/Kconfig > index 64742a0..6fde7d2 100644 > --- a/drivers/tty/serial/8250/Kconfig > +++ b/drivers/tty/serial/8250/Kconfig > @@ -108,7 +108,6 @@ config SERIAL_8250_PCI > tristate "8250/16550 PCI device support" if EXPERT > depends on SERIAL_8250 && PCI > default SERIAL_8250 > - select RATIONAL > help > This builds standard PCI serial support. You may be able to > disable this feature if you only need legacy serial support. > @@ -398,6 +397,19 @@ config SERIAL_8250_INGENIC > If you have a system using an Ingenic SoC and wish to make use of > its UARTs, say Y to this option. If unsure, say N. > > +config SERIAL_8250_LPSS > + tristate "Support for serial ports on Intel LPSS platforms" if EXPERT > + default SERIAL_8250 > + depends on SERIAL_8250 && PCI > + depends on X86 || COMPILE_TEST > + select DW_DMAC_CORE if SERIAL_8250_DMA > + select DW_DMAC_PCI if X86_INTEL_LPSS > + select RATIONAL > + help > + Selecting this option will enable handling of the extra features > + present on the UART found on Intel Braswell SoC and various other > + Intel platforms. > + > config SERIAL_8250_MID > tristate "Support for serial ports on Intel MID platforms" > depends on SERIAL_8250 && PCI > diff --git a/drivers/tty/serial/8250/Makefile b/drivers/tty/serial/8250/Makefile > index c9a2d6e..ca37d1c 100644 > --- a/drivers/tty/serial/8250/Makefile > +++ b/drivers/tty/serial/8250/Makefile > @@ -28,6 +28,7 @@ obj-$(CONFIG_SERIAL_8250_LPC18XX) += 8250_lpc18xx.o > obj-$(CONFIG_SERIAL_8250_MT6577) += 8250_mtk.o > obj-$(CONFIG_SERIAL_8250_UNIPHIER) += 8250_uniphier.o > obj-$(CONFIG_SERIAL_8250_INGENIC) += 8250_ingenic.o > +obj-$(CONFIG_SERIAL_8250_LPSS) += 8250_lpss.o > obj-$(CONFIG_SERIAL_8250_MID) += 8250_mid.o > obj-$(CONFIG_SERIAL_8250_MOXA) += 8250_moxa.o > obj-$(CONFIG_SERIAL_OF_PLATFORM) += 8250_of.o > -- To unsubscribe from this list: send the line "unsubscribe dmaengine" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Apr 8, 2016 at 4:42 AM, Peter Hurley <peter@hurleysoftware.com> wrote: > On 04/07/2016 01:37 PM, Andy Shevchenko wrote: >> Intes SoCs, such as Braswell, have DesignWare UART. Split out to separate >> module which also will be used for Intel Quark later. > > What's the rationale? 1. Not poison 8250_pci with too many quirks. 2. They all use same DMA engine, otherwise we might end up in all possible DMA engines included in one file. 3. All of them are actually DesignWare, so, in the future we might share code between 8250_dw and 8250_lpss. > > And this really isn't a split; this patch introduces a number of significant > changes from the pci version. Some style changes, yes, but "significant"? For example? > > >> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> >> --- >> drivers/tty/serial/8250/8250_lpss.c | 279 ++++++++++++++++++++++++++++++++++++ >> drivers/tty/serial/8250/8250_pci.c | 227 ++--------------------------- >> drivers/tty/serial/8250/Kconfig | 14 +- >> drivers/tty/serial/8250/Makefile | 1 + >> 4 files changed, 301 insertions(+), 220 deletions(-) >> create mode 100644 drivers/tty/serial/8250/8250_lpss.c >> >> diff --git a/drivers/tty/serial/8250/8250_lpss.c b/drivers/tty/serial/8250/8250_lpss.c >> new file mode 100644 >> index 0000000..bca4adb >> --- /dev/null >> +++ b/drivers/tty/serial/8250/8250_lpss.c >> @@ -0,0 +1,279 @@ >> +/* >> + * 8250_lpss.c - Driver for UART on Intel Braswell and various other Intel SoCs >> + * >> + * Copyright (C) 2016 Intel Corporation >> + * Author: Andy Shevchenko <andriy.shevchenko@linux.intel.com> >> + * >> + * This program is free software; you can redistribute it and/or modify >> + * it under the terms of the GNU General Public License version 2 as >> + * published by the Free Software Foundation. >> + */ >> + >> +#include <linux/bitops.h> >> +#include <linux/module.h> >> +#include <linux/pci.h> >> +#include <linux/rational.h> >> + >> +#include <linux/dmaengine.h> >> +#include <linux/platform_data/dma-dw.h> >> + >> +#include "8250.h" >> + >> +#define PCI_DEVICE_ID_INTEL_BYT_UART1 0x0f0a >> +#define PCI_DEVICE_ID_INTEL_BYT_UART2 0x0f0c >> + >> +#define PCI_DEVICE_ID_INTEL_BSW_UART1 0x228a >> +#define PCI_DEVICE_ID_INTEL_BSW_UART2 0x228c >> + >> +#define PCI_DEVICE_ID_INTEL_BDW_UART1 0x9ce3 >> +#define PCI_DEVICE_ID_INTEL_BDW_UART2 0x9ce4 >> + >> +/* Intel LPSS specific registers */ >> + >> +#define BYT_PRV_CLK 0x800 >> +#define BYT_PRV_CLK_EN BIT(0) >> +#define BYT_PRV_CLK_M_VAL_SHIFT 1 >> +#define BYT_PRV_CLK_N_VAL_SHIFT 16 >> +#define BYT_PRV_CLK_UPDATE BIT(31) >> + >> +#define BYT_TX_OVF_INT 0x820 >> +#define BYT_TX_OVF_INT_MASK BIT(1) >> + >> +struct lpss8250; >> + >> +struct lpss8250_board { >> + unsigned long freq; >> + unsigned int base_baud; >> + int (*setup)(struct lpss8250 *, struct uart_port *p); >> +}; > > New concept. > >> + >> +struct lpss8250 { >> + int line; >> + struct lpss8250_board *board; >> + >> + /* DMA parameters */ >> + struct uart_8250_dma dma; >> + struct dw_dma_slave dma_param; >> + u8 dma_maxburst; >> +}; >> + >> +/*****************************************************************************/ > > Please remove. > >> + >> +static void lpss8250_set_termios(struct uart_port *p, >> + struct ktermios *termios, >> + struct ktermios *old) >> +{ >> + unsigned int baud = tty_termios_baud_rate(termios); >> + struct lpss8250 *lpss = p->private_data; >> + unsigned long fref = lpss->board->freq, fuart = baud * 16; >> + unsigned long w = BIT(15) - 1; >> + unsigned long m, n; >> + u32 reg; >> + >> + /* Get Fuart closer to Fref */ >> + fuart *= rounddown_pow_of_two(fref / fuart); >> + >> + /* >> + * For baud rates 0.5M, 1M, 1.5M, 2M, 2.5M, 3M, 3.5M and 4M the >> + * dividers must be adjusted. >> + * >> + * uartclk = (m / n) * 100 MHz, where m <= n >> + */ >> + rational_best_approximation(fuart, fref, w, w, &m, &n); >> + p->uartclk = fuart; >> + >> + /* Reset the clock */ >> + reg = (m << BYT_PRV_CLK_M_VAL_SHIFT) | (n << BYT_PRV_CLK_N_VAL_SHIFT); >> + writel(reg, p->membase + BYT_PRV_CLK); >> + reg |= BYT_PRV_CLK_EN | BYT_PRV_CLK_UPDATE; >> + writel(reg, p->membase + BYT_PRV_CLK); >> + >> + p->status &= ~UPSTAT_AUTOCTS; >> + if (termios->c_cflag & CRTSCTS) >> + p->status |= UPSTAT_AUTOCTS; >> + >> + serial8250_do_set_termios(p, termios, old); >> +} >> + >> +/*****************************************************************************/ > > Please remove. > >> + >> +static int byt_serial_setup(struct lpss8250 *lpss, struct uart_port *port) > > This would have been much easier to review if you had simply moved it here > and done the rework in a follow-on patch. I didn't quite get this one. How series should look like? > > >> +{ >> + struct dw_dma_slave *param = &lpss->dma_param; >> + struct uart_8250_port *up = up_to_u8250p(port); >> + struct pci_dev *pdev = to_pci_dev(port->dev); >> + struct pci_dev *dma_dev = pci_get_slot(pdev->bus, PCI_DEVFN(PCI_SLOT(pdev->devfn), 0)); >> + >> + switch (pdev->device) { >> + case PCI_DEVICE_ID_INTEL_BYT_UART1: >> + case PCI_DEVICE_ID_INTEL_BSW_UART1: >> + case PCI_DEVICE_ID_INTEL_BDW_UART1: >> + param->src_id = 3; >> + param->dst_id = 2; >> + break; >> + case PCI_DEVICE_ID_INTEL_BYT_UART2: >> + case PCI_DEVICE_ID_INTEL_BSW_UART2: >> + case PCI_DEVICE_ID_INTEL_BDW_UART2: >> + param->src_id = 5; >> + param->dst_id = 4; >> + break; >> + default: >> + return -EINVAL; >> + } >> + >> + param->dma_dev = &dma_dev->dev; >> + param->m_master = 0; >> + param->p_master = 1; >> + >> + /* TODO: Detect FIFO size automaticaly for DesignWare 8250 */ >> + port->fifosize = 64; >> + up->tx_loadsz = 64; >> + >> + lpss->dma_maxburst = 16; >> + >> + port->set_termios = lpss8250_set_termios; >> + >> + /* Disable TX counter interrupts */ >> + writel(BYT_TX_OVF_INT_MASK, port->membase + BYT_TX_OVF_INT); >> + >> + return 0; >> +} >> + >> +/*****************************************************************************/ > > Please remove. > >> + >> +static bool lpss8250_dma_filter(struct dma_chan *chan, void *param) >> +{ >> + struct dw_dma_slave *dws = param; >> + >> + if (dws->dma_dev != chan->device->dev) >> + return false; >> + >> + chan->private = dws; >> + return true; >> +} >> + >> +static int lpss8250_dma_setup(struct lpss8250 *lpss, struct uart_8250_port *port) >> +{ >> + struct uart_8250_dma *dma = &lpss->dma; >> + struct dw_dma_slave *param = &lpss->dma_param; > > Unnecessary alias. > >> + struct dw_dma_slave *rx_param, *tx_param; >> + struct device *dev = port->port.dev; >> + >> + rx_param = devm_kzalloc(dev, sizeof(*rx_param), GFP_KERNEL); >> + if (!rx_param) >> + return -ENOMEM; >> + >> + tx_param = devm_kzalloc(dev, sizeof(*tx_param), GFP_KERNEL); >> + if (!tx_param) >> + return -ENOMEM; >> + >> + *rx_param = *param; >> + dma->rxconf.src_maxburst = lpss->dma_maxburst; >> + >> + *tx_param = *param; >> + dma->txconf.dst_maxburst = lpss->dma_maxburst; >> + >> + dma->fn = lpss8250_dma_filter; >> + dma->rx_param = rx_param; >> + dma->tx_param = tx_param; >> + >> + port->dma = dma; >> + return 0; >> +} >> + >> +/*****************************************************************************/ > > Please remove. > >> + >> +static int lpss8250_probe(struct pci_dev *pdev, const struct pci_device_id *id) >> +{ >> + struct uart_8250_port uart; >> + struct lpss8250 *lpss; >> + int ret; >> + >> + ret = pcim_enable_device(pdev); >> + if (ret) >> + return ret; >> + >> + pci_set_master(pdev); >> + >> + lpss = devm_kzalloc(&pdev->dev, sizeof(*lpss), GFP_KERNEL); >> + if (!lpss) >> + return -ENOMEM; >> + >> + lpss->board = (struct lpss8250_board *)id->driver_data; >> + >> + memset(&uart, 0, sizeof(struct uart_8250_port)); >> + >> + uart.port.dev = &pdev->dev; >> + uart.port.irq = pdev->irq; >> + uart.port.private_data = lpss; >> + uart.port.type = PORT_16550A; >> + uart.port.iotype = UPIO_MEM32; > > UPIO_MEM > > >> + uart.port.regshift = 2; >> + uart.port.uartclk = lpss->board->base_baud * 16; >> + uart.port.flags = UPF_SHARE_IRQ | UPF_FIXED_PORT | UPF_FIXED_TYPE; >> + > > Please remove empty line > >> + uart.capabilities = UART_CAP_FIFO | UART_CAP_AFE; >> + > > Same > >> + uart.port.mapbase = pci_resource_start(pdev, 0); >> + uart.port.membase = pcim_iomap(pdev, 0, 0); >> + if (!uart.port.membase) >> + return -ENOMEM; >> + >> + if (lpss->board->setup) { > > There's no design that doesn't have a setup method. For now, indeed. Okay, I will call it directly. > > >> + ret = lpss->board->setup(lpss, &uart.port); >> + if (ret) >> + return ret; >> + } >> + >> + ret = lpss8250_dma_setup(lpss, &uart); >> + if (ret) >> + return ret; > > I would fold this call into board setup which avoids the > ugliness when this error pathway is reworked in the > follow-on patches. Each of them? > > >> + >> + ret = serial8250_register_8250_port(&uart); >> + if (ret < 0) >> + return ret; >> + >> + lpss->line = ret; >> + >> + pci_set_drvdata(pdev, lpss); >> + return 0; >> +} >> + >> +static void lpss8250_remove(struct pci_dev *pdev) >> +{ >> + struct lpss8250 *lpss = pci_get_drvdata(pdev); >> + >> + serial8250_unregister_port(lpss->line); >> +} >> + >> +static const struct lpss8250_board byt_board = { >> + .freq = 100000000, >> + .base_baud = 2764800, >> + .setup = byt_serial_setup, >> +}; >> + >> +#define LPSS_DEVICE(id, board) { PCI_VDEVICE(INTEL, id), (kernel_ulong_t)&board } >> + >> +static const struct pci_device_id pci_ids[] = { >> + LPSS_DEVICE(PCI_DEVICE_ID_INTEL_BYT_UART1, byt_board), >> + LPSS_DEVICE(PCI_DEVICE_ID_INTEL_BYT_UART2, byt_board), >> + LPSS_DEVICE(PCI_DEVICE_ID_INTEL_BSW_UART1, byt_board), >> + LPSS_DEVICE(PCI_DEVICE_ID_INTEL_BSW_UART2, byt_board), >> + LPSS_DEVICE(PCI_DEVICE_ID_INTEL_BDW_UART1, byt_board), >> + LPSS_DEVICE(PCI_DEVICE_ID_INTEL_BDW_UART2, byt_board), >> + { }, >> +}; >> +MODULE_DEVICE_TABLE(pci, pci_ids); >> + >> +static struct pci_driver lpss8250_pci_driver = { >> + .name = "8250_lpss", >> + .id_table = pci_ids, >> + .probe = lpss8250_probe, >> + .remove = lpss8250_remove, > > No power management? PCI does the trick, no *special* power management treatment required, yes. > > >> +}; >> + >> +module_pci_driver(lpss8250_pci_driver); >> + >> +MODULE_AUTHOR("Intel Corporation"); >> +MODULE_LICENSE("GPL v2"); >> +MODULE_DESCRIPTION("Intel LPSS UART driver"); >> diff --git a/drivers/tty/serial/8250/8250_pci.c b/drivers/tty/serial/8250/8250_pci.c >> index 5eea74d..bb4df5d 100644 >> --- a/drivers/tty/serial/8250/8250_pci.c >> +++ b/drivers/tty/serial/8250/8250_pci.c >> @@ -21,14 +21,10 @@ >> #include <linux/serial_core.h> >> #include <linux/8250_pci.h> >> #include <linux/bitops.h> >> -#include <linux/rational.h> >> >> #include <asm/byteorder.h> >> #include <asm/io.h> >> >> -#include <linux/dmaengine.h> >> -#include <linux/platform_data/dma-dw.h> >> - >> #include "8250.h" >> >> /* >> @@ -1349,145 +1345,6 @@ ce4100_serial_setup(struct serial_private *priv, >> return ret; >> } >> >> -#define PCI_DEVICE_ID_INTEL_BYT_UART1 0x0f0a >> -#define PCI_DEVICE_ID_INTEL_BYT_UART2 0x0f0c >> - >> -#define PCI_DEVICE_ID_INTEL_BSW_UART1 0x228a >> -#define PCI_DEVICE_ID_INTEL_BSW_UART2 0x228c >> - >> -#define PCI_DEVICE_ID_INTEL_BDW_UART1 0x9ce3 >> -#define PCI_DEVICE_ID_INTEL_BDW_UART2 0x9ce4 >> - >> -#define BYT_PRV_CLK 0x800 >> -#define BYT_PRV_CLK_EN (1 << 0) >> -#define BYT_PRV_CLK_M_VAL_SHIFT 1 >> -#define BYT_PRV_CLK_N_VAL_SHIFT 16 >> -#define BYT_PRV_CLK_UPDATE (1 << 31) >> - >> -#define BYT_TX_OVF_INT 0x820 >> -#define BYT_TX_OVF_INT_MASK (1 << 1) >> - >> -static void >> -byt_set_termios(struct uart_port *p, struct ktermios *termios, >> - struct ktermios *old) >> -{ >> - unsigned int baud = tty_termios_baud_rate(termios); >> - unsigned long fref = 100000000, fuart = baud * 16; >> - unsigned long w = BIT(15) - 1; >> - unsigned long m, n; >> - u32 reg; >> - >> - /* Get Fuart closer to Fref */ >> - fuart *= rounddown_pow_of_two(fref / fuart); >> - >> - /* >> - * For baud rates 0.5M, 1M, 1.5M, 2M, 2.5M, 3M, 3.5M and 4M the >> - * dividers must be adjusted. >> - * >> - * uartclk = (m / n) * 100 MHz, where m <= n >> - */ >> - rational_best_approximation(fuart, fref, w, w, &m, &n); >> - p->uartclk = fuart; >> - >> - /* Reset the clock */ >> - reg = (m << BYT_PRV_CLK_M_VAL_SHIFT) | (n << BYT_PRV_CLK_N_VAL_SHIFT); >> - writel(reg, p->membase + BYT_PRV_CLK); >> - reg |= BYT_PRV_CLK_EN | BYT_PRV_CLK_UPDATE; >> - writel(reg, p->membase + BYT_PRV_CLK); >> - >> - p->status &= ~UPSTAT_AUTOCTS; >> - if (termios->c_cflag & CRTSCTS) >> - p->status |= UPSTAT_AUTOCTS; >> - >> - serial8250_do_set_termios(p, termios, old); >> -} >> - >> -static bool byt_dma_filter(struct dma_chan *chan, void *param) >> -{ >> - struct dw_dma_slave *dws = param; >> - >> - if (dws->dma_dev != chan->device->dev) >> - return false; >> - >> - chan->private = dws; >> - return true; >> -} >> - >> -static int >> -byt_serial_setup(struct serial_private *priv, >> - const struct pciserial_board *board, >> - struct uart_8250_port *port, int idx) >> -{ >> - struct pci_dev *pdev = priv->dev; >> - struct device *dev = port->port.dev; >> - struct uart_8250_dma *dma; >> - struct dw_dma_slave *tx_param, *rx_param; >> - struct pci_dev *dma_dev; >> - int ret; >> - >> - dma = devm_kzalloc(dev, sizeof(*dma), GFP_KERNEL); >> - if (!dma) >> - return -ENOMEM; >> - >> - tx_param = devm_kzalloc(dev, sizeof(*tx_param), GFP_KERNEL); >> - if (!tx_param) >> - return -ENOMEM; >> - >> - rx_param = devm_kzalloc(dev, sizeof(*rx_param), GFP_KERNEL); >> - if (!rx_param) >> - return -ENOMEM; >> - >> - switch (pdev->device) { >> - case PCI_DEVICE_ID_INTEL_BYT_UART1: >> - case PCI_DEVICE_ID_INTEL_BSW_UART1: >> - case PCI_DEVICE_ID_INTEL_BDW_UART1: >> - rx_param->src_id = 3; >> - tx_param->dst_id = 2; >> - break; >> - case PCI_DEVICE_ID_INTEL_BYT_UART2: >> - case PCI_DEVICE_ID_INTEL_BSW_UART2: >> - case PCI_DEVICE_ID_INTEL_BDW_UART2: >> - rx_param->src_id = 5; >> - tx_param->dst_id = 4; >> - break; >> - default: >> - return -EINVAL; >> - } >> - >> - rx_param->m_master = 0; >> - rx_param->p_master = 1; >> - >> - dma->rxconf.src_maxburst = 16; >> - >> - tx_param->m_master = 0; >> - tx_param->p_master = 1; >> - >> - dma->txconf.dst_maxburst = 16; >> - >> - dma_dev = pci_get_slot(pdev->bus, PCI_DEVFN(PCI_SLOT(pdev->devfn), 0)); >> - rx_param->dma_dev = &dma_dev->dev; >> - tx_param->dma_dev = &dma_dev->dev; >> - >> - dma->fn = byt_dma_filter; >> - dma->rx_param = rx_param; >> - dma->tx_param = tx_param; >> - >> - ret = pci_default_setup(priv, board, port, idx); >> - port->port.iotype = UPIO_MEM; >> - port->port.type = PORT_16550A; >> - port->port.flags = (port->port.flags | UPF_FIXED_PORT | UPF_FIXED_TYPE); >> - port->port.set_termios = byt_set_termios; >> - port->port.fifosize = 64; >> - port->tx_loadsz = 64; >> - port->dma = dma; >> - port->capabilities = UART_CAP_FIFO | UART_CAP_AFE; >> - >> - /* Disable Tx counter interrupts */ >> - writel(BYT_TX_OVF_INT_MASK, port->port.membase + BYT_TX_OVF_INT); >> - >> - return ret; >> -} >> - >> static int >> pci_omegapci_setup(struct serial_private *priv, >> const struct pciserial_board *board, >> @@ -2015,48 +1872,6 @@ static struct pci_serial_quirk pci_serial_quirks[] __refdata = { >> .subdevice = PCI_ANY_ID, >> .setup = kt_serial_setup, >> }, >> - { >> - .vendor = PCI_VENDOR_ID_INTEL, >> - .device = PCI_DEVICE_ID_INTEL_BYT_UART1, >> - .subvendor = PCI_ANY_ID, >> - .subdevice = PCI_ANY_ID, >> - .setup = byt_serial_setup, >> - }, >> - { >> - .vendor = PCI_VENDOR_ID_INTEL, >> - .device = PCI_DEVICE_ID_INTEL_BYT_UART2, >> - .subvendor = PCI_ANY_ID, >> - .subdevice = PCI_ANY_ID, >> - .setup = byt_serial_setup, >> - }, >> - { >> - .vendor = PCI_VENDOR_ID_INTEL, >> - .device = PCI_DEVICE_ID_INTEL_BSW_UART1, >> - .subvendor = PCI_ANY_ID, >> - .subdevice = PCI_ANY_ID, >> - .setup = byt_serial_setup, >> - }, >> - { >> - .vendor = PCI_VENDOR_ID_INTEL, >> - .device = PCI_DEVICE_ID_INTEL_BSW_UART2, >> - .subvendor = PCI_ANY_ID, >> - .subdevice = PCI_ANY_ID, >> - .setup = byt_serial_setup, >> - }, >> - { >> - .vendor = PCI_VENDOR_ID_INTEL, >> - .device = PCI_DEVICE_ID_INTEL_BDW_UART1, >> - .subvendor = PCI_ANY_ID, >> - .subdevice = PCI_ANY_ID, >> - .setup = byt_serial_setup, >> - }, >> - { >> - .vendor = PCI_VENDOR_ID_INTEL, >> - .device = PCI_DEVICE_ID_INTEL_BDW_UART2, >> - .subvendor = PCI_ANY_ID, >> - .subdevice = PCI_ANY_ID, >> - .setup = byt_serial_setup, >> - }, >> /* >> * ITE >> */ >> @@ -2921,7 +2736,6 @@ enum pci_board_num_t { >> pbn_ADDIDATA_PCIe_4_3906250, >> pbn_ADDIDATA_PCIe_8_3906250, >> pbn_ce4100_1_115200, >> - pbn_byt, >> pbn_qrk, >> pbn_omegapci, >> pbn_NETMOS9900_2s_115200, >> @@ -3698,12 +3512,6 @@ static struct pciserial_board pci_boards[] = { >> .base_baud = 921600, >> .reg_shift = 2, >> }, >> - [pbn_byt] = { >> - .flags = FL_BASE0, >> - .num_ports = 1, >> - .base_baud = 2764800, >> - .reg_shift = 2, >> - }, >> [pbn_qrk] = { >> .flags = FL_BASE0, >> .num_ports = 1, >> @@ -3820,6 +3628,14 @@ static const struct pci_device_id blacklist[] = { >> { PCI_VDEVICE(INTEL, 0x081d), }, >> { PCI_VDEVICE(INTEL, 0x1191), }, >> { PCI_VDEVICE(INTEL, 0x19d8), }, >> + >> + /* Intel platforms with DesignWare UART */ >> + { PCI_VDEVICE(INTEL, 0x0f0a), }, >> + { PCI_VDEVICE(INTEL, 0x0f0c), }, >> + { PCI_VDEVICE(INTEL, 0x228a), }, >> + { PCI_VDEVICE(INTEL, 0x228c), }, >> + { PCI_VDEVICE(INTEL, 0x9ce3), }, >> + { PCI_VDEVICE(INTEL, 0x9ce4), }, >> }; >> >> /* >> @@ -5485,33 +5301,6 @@ static struct pci_device_id serial_pci_tbl[] = { >> { PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_CE4100_UART, >> PCI_ANY_ID, PCI_ANY_ID, 0, 0, >> pbn_ce4100_1_115200 }, >> - /* Intel BayTrail */ >> - { PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_BYT_UART1, >> - PCI_ANY_ID, PCI_ANY_ID, >> - PCI_CLASS_COMMUNICATION_SERIAL << 8, 0xff0000, >> - pbn_byt }, >> - { PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_BYT_UART2, >> - PCI_ANY_ID, PCI_ANY_ID, >> - PCI_CLASS_COMMUNICATION_SERIAL << 8, 0xff0000, >> - pbn_byt }, >> - { PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_BSW_UART1, >> - PCI_ANY_ID, PCI_ANY_ID, >> - PCI_CLASS_COMMUNICATION_SERIAL << 8, 0xff0000, >> - pbn_byt }, >> - { PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_BSW_UART2, >> - PCI_ANY_ID, PCI_ANY_ID, >> - PCI_CLASS_COMMUNICATION_SERIAL << 8, 0xff0000, >> - pbn_byt }, >> - >> - /* Intel Broadwell */ >> - { PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_BDW_UART1, >> - PCI_ANY_ID, PCI_ANY_ID, >> - PCI_CLASS_COMMUNICATION_SERIAL << 8, 0xff0000, >> - pbn_byt }, >> - { PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_BDW_UART2, >> - PCI_ANY_ID, PCI_ANY_ID, >> - PCI_CLASS_COMMUNICATION_SERIAL << 8, 0xff0000, >> - pbn_byt }, >> >> /* >> * Intel Quark x1000 >> diff --git a/drivers/tty/serial/8250/Kconfig b/drivers/tty/serial/8250/Kconfig >> index 64742a0..6fde7d2 100644 >> --- a/drivers/tty/serial/8250/Kconfig >> +++ b/drivers/tty/serial/8250/Kconfig >> @@ -108,7 +108,6 @@ config SERIAL_8250_PCI >> tristate "8250/16550 PCI device support" if EXPERT >> depends on SERIAL_8250 && PCI >> default SERIAL_8250 >> - select RATIONAL >> help >> This builds standard PCI serial support. You may be able to >> disable this feature if you only need legacy serial support. >> @@ -398,6 +397,19 @@ config SERIAL_8250_INGENIC >> If you have a system using an Ingenic SoC and wish to make use of >> its UARTs, say Y to this option. If unsure, say N. >> >> +config SERIAL_8250_LPSS >> + tristate "Support for serial ports on Intel LPSS platforms" if EXPERT >> + default SERIAL_8250 >> + depends on SERIAL_8250 && PCI >> + depends on X86 || COMPILE_TEST >> + select DW_DMAC_CORE if SERIAL_8250_DMA >> + select DW_DMAC_PCI if X86_INTEL_LPSS >> + select RATIONAL >> + help >> + Selecting this option will enable handling of the extra features >> + present on the UART found on Intel Braswell SoC and various other >> + Intel platforms. >> + >> config SERIAL_8250_MID >> tristate "Support for serial ports on Intel MID platforms" >> depends on SERIAL_8250 && PCI >> diff --git a/drivers/tty/serial/8250/Makefile b/drivers/tty/serial/8250/Makefile >> index c9a2d6e..ca37d1c 100644 >> --- a/drivers/tty/serial/8250/Makefile >> +++ b/drivers/tty/serial/8250/Makefile >> @@ -28,6 +28,7 @@ obj-$(CONFIG_SERIAL_8250_LPC18XX) += 8250_lpc18xx.o >> obj-$(CONFIG_SERIAL_8250_MT6577) += 8250_mtk.o >> obj-$(CONFIG_SERIAL_8250_UNIPHIER) += 8250_uniphier.o >> obj-$(CONFIG_SERIAL_8250_INGENIC) += 8250_ingenic.o >> +obj-$(CONFIG_SERIAL_8250_LPSS) += 8250_lpss.o >> obj-$(CONFIG_SERIAL_8250_MID) += 8250_mid.o >> obj-$(CONFIG_SERIAL_8250_MOXA) += 8250_moxa.o >> obj-$(CONFIG_SERIAL_OF_PLATFORM) += 8250_of.o >> >
On 04/08/2016 01:17 AM, Andy Shevchenko wrote: > On Fri, Apr 8, 2016 at 4:42 AM, Peter Hurley <peter@hurleysoftware.com> wrote: >> On 04/07/2016 01:37 PM, Andy Shevchenko wrote: >>> Intes SoCs, such as Braswell, have DesignWare UART. Split out to separate >>> module which also will be used for Intel Quark later. >> >> What's the rationale? > > 1. Not poison 8250_pci with too many quirks. > 2. They all use same DMA engine, otherwise we might end up in all > possible DMA engines included in one file. > 3. All of them are actually DesignWare, so, in the future we might > share code between 8250_dw and 8250_lpss. Just my opinion, but I like to see the rationale in the changelog. >> And this really isn't a split; this patch introduces a number of significant >> changes from the pci version. > > Some style changes, yes, but "significant"? > For example? I'm just pointing out the changelog doesn't really match the commit. I'm not suggesting necessarily to redo the series, but just more adequately reflect the change. See below. >> >> >>> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> >>> --- >>> drivers/tty/serial/8250/8250_lpss.c | 279 ++++++++++++++++++++++++++++++++++++ >>> drivers/tty/serial/8250/8250_pci.c | 227 ++--------------------------- >>> drivers/tty/serial/8250/Kconfig | 14 +- >>> drivers/tty/serial/8250/Makefile | 1 + >>> 4 files changed, 301 insertions(+), 220 deletions(-) >>> create mode 100644 drivers/tty/serial/8250/8250_lpss.c >>> >>> diff --git a/drivers/tty/serial/8250/8250_lpss.c b/drivers/tty/serial/8250/8250_lpss.c >>> new file mode 100644 >>> index 0000000..bca4adb >>> --- /dev/null >>> +++ b/drivers/tty/serial/8250/8250_lpss.c >>> @@ -0,0 +1,279 @@ >>> +/* >>> + * 8250_lpss.c - Driver for UART on Intel Braswell and various other Intel SoCs >>> + * >>> + * Copyright (C) 2016 Intel Corporation >>> + * Author: Andy Shevchenko <andriy.shevchenko@linux.intel.com> >>> + * >>> + * This program is free software; you can redistribute it and/or modify >>> + * it under the terms of the GNU General Public License version 2 as >>> + * published by the Free Software Foundation. >>> + */ >>> + >>> +#include <linux/bitops.h> >>> +#include <linux/module.h> >>> +#include <linux/pci.h> >>> +#include <linux/rational.h> >>> + >>> +#include <linux/dmaengine.h> >>> +#include <linux/platform_data/dma-dw.h> >>> + >>> +#include "8250.h" >>> + >>> +#define PCI_DEVICE_ID_INTEL_BYT_UART1 0x0f0a >>> +#define PCI_DEVICE_ID_INTEL_BYT_UART2 0x0f0c >>> + >>> +#define PCI_DEVICE_ID_INTEL_BSW_UART1 0x228a >>> +#define PCI_DEVICE_ID_INTEL_BSW_UART2 0x228c >>> + >>> +#define PCI_DEVICE_ID_INTEL_BDW_UART1 0x9ce3 >>> +#define PCI_DEVICE_ID_INTEL_BDW_UART2 0x9ce4 >>> + >>> +/* Intel LPSS specific registers */ >>> + >>> +#define BYT_PRV_CLK 0x800 >>> +#define BYT_PRV_CLK_EN BIT(0) >>> +#define BYT_PRV_CLK_M_VAL_SHIFT 1 >>> +#define BYT_PRV_CLK_N_VAL_SHIFT 16 >>> +#define BYT_PRV_CLK_UPDATE BIT(31) >>> + >>> +#define BYT_TX_OVF_INT 0x820 >>> +#define BYT_TX_OVF_INT_MASK BIT(1) >>> + >>> +struct lpss8250; >>> + >>> +struct lpss8250_board { >>> + unsigned long freq; >>> + unsigned int base_baud; >>> + int (*setup)(struct lpss8250 *, struct uart_port *p); >>> +}; >> >> New concept. >> >>> + >>> +struct lpss8250 { >>> + int line; >>> + struct lpss8250_board *board; >>> + >>> + /* DMA parameters */ >>> + struct uart_8250_dma dma; >>> + struct dw_dma_slave dma_param; >>> + u8 dma_maxburst; >>> +}; >>> + >>> +/*****************************************************************************/ >> >> Please remove. >> >>> + >>> +static void lpss8250_set_termios(struct uart_port *p, >>> + struct ktermios *termios, >>> + struct ktermios *old) >>> +{ >>> + unsigned int baud = tty_termios_baud_rate(termios); >>> + struct lpss8250 *lpss = p->private_data; >>> + unsigned long fref = lpss->board->freq, fuart = baud * 16; >>> + unsigned long w = BIT(15) - 1; >>> + unsigned long m, n; >>> + u32 reg; >>> + >>> + /* Get Fuart closer to Fref */ >>> + fuart *= rounddown_pow_of_two(fref / fuart); >>> + >>> + /* >>> + * For baud rates 0.5M, 1M, 1.5M, 2M, 2.5M, 3M, 3.5M and 4M the >>> + * dividers must be adjusted. >>> + * >>> + * uartclk = (m / n) * 100 MHz, where m <= n >>> + */ >>> + rational_best_approximation(fuart, fref, w, w, &m, &n); >>> + p->uartclk = fuart; >>> + >>> + /* Reset the clock */ >>> + reg = (m << BYT_PRV_CLK_M_VAL_SHIFT) | (n << BYT_PRV_CLK_N_VAL_SHIFT); >>> + writel(reg, p->membase + BYT_PRV_CLK); >>> + reg |= BYT_PRV_CLK_EN | BYT_PRV_CLK_UPDATE; >>> + writel(reg, p->membase + BYT_PRV_CLK); >>> + >>> + p->status &= ~UPSTAT_AUTOCTS; >>> + if (termios->c_cflag & CRTSCTS) >>> + p->status |= UPSTAT_AUTOCTS; >>> + >>> + serial8250_do_set_termios(p, termios, old); >>> +} >>> + >>> +/*****************************************************************************/ >> >> Please remove. >> >>> + >>> +static int byt_serial_setup(struct lpss8250 *lpss, struct uart_port *port) >> >> This would have been much easier to review if you had simply moved it here >> and done the rework in a follow-on patch. > > I didn't quite get this one. Well, just comparing byt_serial_setup() from the two versions: 1) dma setup is in a completely separate function 2) the tx & rx dma parameters are folded together 3) the port setup is split out 4) introduction of struct lpss8250 ... > How series should look like? I would have just moved byt_serial_setup() without any of the other changes except perhaps replacing pciserial_board with lpss8250_board, and then made the other changes on top before the Quark patches. There is no changelog describing the purpose of struct lpss8250_board, or struct lpss8250. Or of the dma changes. Or why dma_maxburst was parameterized. ... Naturally, I can figure all of that out on my own, but it would have been better to read your reasoning. It looks alot of work to split out now, so I guess what's done is done, and I'll just review this *really* carefully. But imagine if you hadn't wrote it, and were reviewing this: it's very difficult to mentally separate out the changes and keep track of them while reviewing. Side-by-side diff is nearly useless... >> >> >>> +{ >>> + struct dw_dma_slave *param = &lpss->dma_param; >>> + struct uart_8250_port *up = up_to_u8250p(port); >>> + struct pci_dev *pdev = to_pci_dev(port->dev); >>> + struct pci_dev *dma_dev = pci_get_slot(pdev->bus, PCI_DEVFN(PCI_SLOT(pdev->devfn), 0)); >>> + >>> + switch (pdev->device) { >>> + case PCI_DEVICE_ID_INTEL_BYT_UART1: >>> + case PCI_DEVICE_ID_INTEL_BSW_UART1: >>> + case PCI_DEVICE_ID_INTEL_BDW_UART1: >>> + param->src_id = 3; >>> + param->dst_id = 2; >>> + break; >>> + case PCI_DEVICE_ID_INTEL_BYT_UART2: >>> + case PCI_DEVICE_ID_INTEL_BSW_UART2: >>> + case PCI_DEVICE_ID_INTEL_BDW_UART2: >>> + param->src_id = 5; >>> + param->dst_id = 4; >>> + break; >>> + default: >>> + return -EINVAL; >>> + } >>> + >>> + param->dma_dev = &dma_dev->dev; >>> + param->m_master = 0; >>> + param->p_master = 1; >>> + >>> + /* TODO: Detect FIFO size automaticaly for DesignWare 8250 */ >>> + port->fifosize = 64; >>> + up->tx_loadsz = 64; >>> + >>> + lpss->dma_maxburst = 16; >>> + >>> + port->set_termios = lpss8250_set_termios; >>> + >>> + /* Disable TX counter interrupts */ >>> + writel(BYT_TX_OVF_INT_MASK, port->membase + BYT_TX_OVF_INT); >>> + >>> + return 0; >>> +} >>> + >>> +/*****************************************************************************/ >> >> Please remove. >> >>> + >>> +static bool lpss8250_dma_filter(struct dma_chan *chan, void *param) >>> +{ >>> + struct dw_dma_slave *dws = param; >>> + >>> + if (dws->dma_dev != chan->device->dev) >>> + return false; >>> + >>> + chan->private = dws; >>> + return true; >>> +} >>> + >>> +static int lpss8250_dma_setup(struct lpss8250 *lpss, struct uart_8250_port *port) >>> +{ >>> + struct uart_8250_dma *dma = &lpss->dma; >>> + struct dw_dma_slave *param = &lpss->dma_param; >> >> Unnecessary alias. >> >>> + struct dw_dma_slave *rx_param, *tx_param; >>> + struct device *dev = port->port.dev; >>> + >>> + rx_param = devm_kzalloc(dev, sizeof(*rx_param), GFP_KERNEL); >>> + if (!rx_param) >>> + return -ENOMEM; >>> + >>> + tx_param = devm_kzalloc(dev, sizeof(*tx_param), GFP_KERNEL); >>> + if (!tx_param) >>> + return -ENOMEM; >>> + >>> + *rx_param = *param; >>> + dma->rxconf.src_maxburst = lpss->dma_maxburst; >>> + >>> + *tx_param = *param; >>> + dma->txconf.dst_maxburst = lpss->dma_maxburst; >>> + >>> + dma->fn = lpss8250_dma_filter; >>> + dma->rx_param = rx_param; >>> + dma->tx_param = tx_param; >>> + >>> + port->dma = dma; >>> + return 0; >>> +} >>> + >>> +/*****************************************************************************/ >> >> Please remove. >> >>> + >>> +static int lpss8250_probe(struct pci_dev *pdev, const struct pci_device_id *id) >>> +{ >>> + struct uart_8250_port uart; >>> + struct lpss8250 *lpss; >>> + int ret; >>> + >>> + ret = pcim_enable_device(pdev); >>> + if (ret) >>> + return ret; >>> + >>> + pci_set_master(pdev); >>> + >>> + lpss = devm_kzalloc(&pdev->dev, sizeof(*lpss), GFP_KERNEL); >>> + if (!lpss) >>> + return -ENOMEM; >>> + >>> + lpss->board = (struct lpss8250_board *)id->driver_data; >>> + >>> + memset(&uart, 0, sizeof(struct uart_8250_port)); >>> + >>> + uart.port.dev = &pdev->dev; >>> + uart.port.irq = pdev->irq; >>> + uart.port.private_data = lpss; >>> + uart.port.type = PORT_16550A; >>> + uart.port.iotype = UPIO_MEM32; >> >> UPIO_MEM >> >> >>> + uart.port.regshift = 2; >>> + uart.port.uartclk = lpss->board->base_baud * 16; >>> + uart.port.flags = UPF_SHARE_IRQ | UPF_FIXED_PORT | UPF_FIXED_TYPE; >>> + >> >> Please remove empty line >> >>> + uart.capabilities = UART_CAP_FIFO | UART_CAP_AFE; >>> + >> >> Same >> >>> + uart.port.mapbase = pci_resource_start(pdev, 0); >>> + uart.port.membase = pcim_iomap(pdev, 0, 0); >>> + if (!uart.port.membase) >>> + return -ENOMEM; >>> + >>> + if (lpss->board->setup) { >> >> There's no design that doesn't have a setup method. > > For now, indeed. Okay, I will call it directly. > >> >> >>> + ret = lpss->board->setup(lpss, &uart.port); >>> + if (ret) >>> + return ret; >>> + } >>> + >>> + ret = lpss8250_dma_setup(lpss, &uart); >>> + if (ret) >>> + return ret; >> >> I would fold this call into board setup which avoids the >> ugliness when this error pathway is reworked in the >> follow-on patches. > > Each of them? I'm assuming there's just the two: byt_serial_setup() and qrk_serial_setup()? Did I overlook something? Perhaps I missed some design goal? >> >> >>> + >>> + ret = serial8250_register_8250_port(&uart); >>> + if (ret < 0) >>> + return ret; >>> + >>> + lpss->line = ret; >>> + >>> + pci_set_drvdata(pdev, lpss); >>> + return 0; >>> +} >>> + >>> +static void lpss8250_remove(struct pci_dev *pdev) >>> +{ >>> + struct lpss8250 *lpss = pci_get_drvdata(pdev); >>> + >>> + serial8250_unregister_port(lpss->line); >>> +} >>> + >>> +static const struct lpss8250_board byt_board = { >>> + .freq = 100000000, >>> + .base_baud = 2764800, >>> + .setup = byt_serial_setup, >>> +}; >>> + >>> +#define LPSS_DEVICE(id, board) { PCI_VDEVICE(INTEL, id), (kernel_ulong_t)&board } >>> + >>> +static const struct pci_device_id pci_ids[] = { >>> + LPSS_DEVICE(PCI_DEVICE_ID_INTEL_BYT_UART1, byt_board), >>> + LPSS_DEVICE(PCI_DEVICE_ID_INTEL_BYT_UART2, byt_board), >>> + LPSS_DEVICE(PCI_DEVICE_ID_INTEL_BSW_UART1, byt_board), >>> + LPSS_DEVICE(PCI_DEVICE_ID_INTEL_BSW_UART2, byt_board), >>> + LPSS_DEVICE(PCI_DEVICE_ID_INTEL_BDW_UART1, byt_board), >>> + LPSS_DEVICE(PCI_DEVICE_ID_INTEL_BDW_UART2, byt_board), >>> + { }, >>> +}; >>> +MODULE_DEVICE_TABLE(pci, pci_ids); >>> + >>> +static struct pci_driver lpss8250_pci_driver = { >>> + .name = "8250_lpss", >>> + .id_table = pci_ids, >>> + .probe = lpss8250_probe, >>> + .remove = lpss8250_remove, >> >> No power management? > > PCI does the trick, no *special* power management treatment required, yes. I realized that later this am; sorry about that. [Maybe just put a small note in the changelog about that though?] Regards, Peter Hurley >> >> >>> +}; >>> + >>> +module_pci_driver(lpss8250_pci_driver); >>> + >>> +MODULE_AUTHOR("Intel Corporation"); >>> +MODULE_LICENSE("GPL v2"); >>> +MODULE_DESCRIPTION("Intel LPSS UART driver"); >>> diff --git a/drivers/tty/serial/8250/8250_pci.c b/drivers/tty/serial/8250/8250_pci.c >>> index 5eea74d..bb4df5d 100644 >>> --- a/drivers/tty/serial/8250/8250_pci.c >>> +++ b/drivers/tty/serial/8250/8250_pci.c >>> @@ -21,14 +21,10 @@ >>> #include <linux/serial_core.h> >>> #include <linux/8250_pci.h> >>> #include <linux/bitops.h> >>> -#include <linux/rational.h> >>> >>> #include <asm/byteorder.h> >>> #include <asm/io.h> >>> >>> -#include <linux/dmaengine.h> >>> -#include <linux/platform_data/dma-dw.h> >>> - >>> #include "8250.h" >>> >>> /* >>> @@ -1349,145 +1345,6 @@ ce4100_serial_setup(struct serial_private *priv, >>> return ret; >>> } >>> >>> -#define PCI_DEVICE_ID_INTEL_BYT_UART1 0x0f0a >>> -#define PCI_DEVICE_ID_INTEL_BYT_UART2 0x0f0c >>> - >>> -#define PCI_DEVICE_ID_INTEL_BSW_UART1 0x228a >>> -#define PCI_DEVICE_ID_INTEL_BSW_UART2 0x228c >>> - >>> -#define PCI_DEVICE_ID_INTEL_BDW_UART1 0x9ce3 >>> -#define PCI_DEVICE_ID_INTEL_BDW_UART2 0x9ce4 >>> - >>> -#define BYT_PRV_CLK 0x800 >>> -#define BYT_PRV_CLK_EN (1 << 0) >>> -#define BYT_PRV_CLK_M_VAL_SHIFT 1 >>> -#define BYT_PRV_CLK_N_VAL_SHIFT 16 >>> -#define BYT_PRV_CLK_UPDATE (1 << 31) >>> - >>> -#define BYT_TX_OVF_INT 0x820 >>> -#define BYT_TX_OVF_INT_MASK (1 << 1) >>> - >>> -static void >>> -byt_set_termios(struct uart_port *p, struct ktermios *termios, >>> - struct ktermios *old) >>> -{ >>> - unsigned int baud = tty_termios_baud_rate(termios); >>> - unsigned long fref = 100000000, fuart = baud * 16; >>> - unsigned long w = BIT(15) - 1; >>> - unsigned long m, n; >>> - u32 reg; >>> - >>> - /* Get Fuart closer to Fref */ >>> - fuart *= rounddown_pow_of_two(fref / fuart); >>> - >>> - /* >>> - * For baud rates 0.5M, 1M, 1.5M, 2M, 2.5M, 3M, 3.5M and 4M the >>> - * dividers must be adjusted. >>> - * >>> - * uartclk = (m / n) * 100 MHz, where m <= n >>> - */ >>> - rational_best_approximation(fuart, fref, w, w, &m, &n); >>> - p->uartclk = fuart; >>> - >>> - /* Reset the clock */ >>> - reg = (m << BYT_PRV_CLK_M_VAL_SHIFT) | (n << BYT_PRV_CLK_N_VAL_SHIFT); >>> - writel(reg, p->membase + BYT_PRV_CLK); >>> - reg |= BYT_PRV_CLK_EN | BYT_PRV_CLK_UPDATE; >>> - writel(reg, p->membase + BYT_PRV_CLK); >>> - >>> - p->status &= ~UPSTAT_AUTOCTS; >>> - if (termios->c_cflag & CRTSCTS) >>> - p->status |= UPSTAT_AUTOCTS; >>> - >>> - serial8250_do_set_termios(p, termios, old); >>> -} >>> - >>> -static bool byt_dma_filter(struct dma_chan *chan, void *param) >>> -{ >>> - struct dw_dma_slave *dws = param; >>> - >>> - if (dws->dma_dev != chan->device->dev) >>> - return false; >>> - >>> - chan->private = dws; >>> - return true; >>> -} >>> - >>> -static int >>> -byt_serial_setup(struct serial_private *priv, >>> - const struct pciserial_board *board, >>> - struct uart_8250_port *port, int idx) >>> -{ >>> - struct pci_dev *pdev = priv->dev; >>> - struct device *dev = port->port.dev; >>> - struct uart_8250_dma *dma; >>> - struct dw_dma_slave *tx_param, *rx_param; >>> - struct pci_dev *dma_dev; >>> - int ret; >>> - >>> - dma = devm_kzalloc(dev, sizeof(*dma), GFP_KERNEL); >>> - if (!dma) >>> - return -ENOMEM; >>> - >>> - tx_param = devm_kzalloc(dev, sizeof(*tx_param), GFP_KERNEL); >>> - if (!tx_param) >>> - return -ENOMEM; >>> - >>> - rx_param = devm_kzalloc(dev, sizeof(*rx_param), GFP_KERNEL); >>> - if (!rx_param) >>> - return -ENOMEM; >>> - >>> - switch (pdev->device) { >>> - case PCI_DEVICE_ID_INTEL_BYT_UART1: >>> - case PCI_DEVICE_ID_INTEL_BSW_UART1: >>> - case PCI_DEVICE_ID_INTEL_BDW_UART1: >>> - rx_param->src_id = 3; >>> - tx_param->dst_id = 2; >>> - break; >>> - case PCI_DEVICE_ID_INTEL_BYT_UART2: >>> - case PCI_DEVICE_ID_INTEL_BSW_UART2: >>> - case PCI_DEVICE_ID_INTEL_BDW_UART2: >>> - rx_param->src_id = 5; >>> - tx_param->dst_id = 4; >>> - break; >>> - default: >>> - return -EINVAL; >>> - } >>> - >>> - rx_param->m_master = 0; >>> - rx_param->p_master = 1; >>> - >>> - dma->rxconf.src_maxburst = 16; >>> - >>> - tx_param->m_master = 0; >>> - tx_param->p_master = 1; >>> - >>> - dma->txconf.dst_maxburst = 16; >>> - >>> - dma_dev = pci_get_slot(pdev->bus, PCI_DEVFN(PCI_SLOT(pdev->devfn), 0)); >>> - rx_param->dma_dev = &dma_dev->dev; >>> - tx_param->dma_dev = &dma_dev->dev; >>> - >>> - dma->fn = byt_dma_filter; >>> - dma->rx_param = rx_param; >>> - dma->tx_param = tx_param; >>> - >>> - ret = pci_default_setup(priv, board, port, idx); >>> - port->port.iotype = UPIO_MEM; >>> - port->port.type = PORT_16550A; >>> - port->port.flags = (port->port.flags | UPF_FIXED_PORT | UPF_FIXED_TYPE); >>> - port->port.set_termios = byt_set_termios; >>> - port->port.fifosize = 64; >>> - port->tx_loadsz = 64; >>> - port->dma = dma; >>> - port->capabilities = UART_CAP_FIFO | UART_CAP_AFE; >>> - >>> - /* Disable Tx counter interrupts */ >>> - writel(BYT_TX_OVF_INT_MASK, port->port.membase + BYT_TX_OVF_INT); >>> - >>> - return ret; >>> -} >>> - >>> static int >>> pci_omegapci_setup(struct serial_private *priv, >>> const struct pciserial_board *board, >>> @@ -2015,48 +1872,6 @@ static struct pci_serial_quirk pci_serial_quirks[] __refdata = { >>> .subdevice = PCI_ANY_ID, >>> .setup = kt_serial_setup, >>> }, >>> - { >>> - .vendor = PCI_VENDOR_ID_INTEL, >>> - .device = PCI_DEVICE_ID_INTEL_BYT_UART1, >>> - .subvendor = PCI_ANY_ID, >>> - .subdevice = PCI_ANY_ID, >>> - .setup = byt_serial_setup, >>> - }, >>> - { >>> - .vendor = PCI_VENDOR_ID_INTEL, >>> - .device = PCI_DEVICE_ID_INTEL_BYT_UART2, >>> - .subvendor = PCI_ANY_ID, >>> - .subdevice = PCI_ANY_ID, >>> - .setup = byt_serial_setup, >>> - }, >>> - { >>> - .vendor = PCI_VENDOR_ID_INTEL, >>> - .device = PCI_DEVICE_ID_INTEL_BSW_UART1, >>> - .subvendor = PCI_ANY_ID, >>> - .subdevice = PCI_ANY_ID, >>> - .setup = byt_serial_setup, >>> - }, >>> - { >>> - .vendor = PCI_VENDOR_ID_INTEL, >>> - .device = PCI_DEVICE_ID_INTEL_BSW_UART2, >>> - .subvendor = PCI_ANY_ID, >>> - .subdevice = PCI_ANY_ID, >>> - .setup = byt_serial_setup, >>> - }, >>> - { >>> - .vendor = PCI_VENDOR_ID_INTEL, >>> - .device = PCI_DEVICE_ID_INTEL_BDW_UART1, >>> - .subvendor = PCI_ANY_ID, >>> - .subdevice = PCI_ANY_ID, >>> - .setup = byt_serial_setup, >>> - }, >>> - { >>> - .vendor = PCI_VENDOR_ID_INTEL, >>> - .device = PCI_DEVICE_ID_INTEL_BDW_UART2, >>> - .subvendor = PCI_ANY_ID, >>> - .subdevice = PCI_ANY_ID, >>> - .setup = byt_serial_setup, >>> - }, >>> /* >>> * ITE >>> */ >>> @@ -2921,7 +2736,6 @@ enum pci_board_num_t { >>> pbn_ADDIDATA_PCIe_4_3906250, >>> pbn_ADDIDATA_PCIe_8_3906250, >>> pbn_ce4100_1_115200, >>> - pbn_byt, >>> pbn_qrk, >>> pbn_omegapci, >>> pbn_NETMOS9900_2s_115200, >>> @@ -3698,12 +3512,6 @@ static struct pciserial_board pci_boards[] = { >>> .base_baud = 921600, >>> .reg_shift = 2, >>> }, >>> - [pbn_byt] = { >>> - .flags = FL_BASE0, >>> - .num_ports = 1, >>> - .base_baud = 2764800, >>> - .reg_shift = 2, >>> - }, >>> [pbn_qrk] = { >>> .flags = FL_BASE0, >>> .num_ports = 1, >>> @@ -3820,6 +3628,14 @@ static const struct pci_device_id blacklist[] = { >>> { PCI_VDEVICE(INTEL, 0x081d), }, >>> { PCI_VDEVICE(INTEL, 0x1191), }, >>> { PCI_VDEVICE(INTEL, 0x19d8), }, >>> + >>> + /* Intel platforms with DesignWare UART */ >>> + { PCI_VDEVICE(INTEL, 0x0f0a), }, >>> + { PCI_VDEVICE(INTEL, 0x0f0c), }, >>> + { PCI_VDEVICE(INTEL, 0x228a), }, >>> + { PCI_VDEVICE(INTEL, 0x228c), }, >>> + { PCI_VDEVICE(INTEL, 0x9ce3), }, >>> + { PCI_VDEVICE(INTEL, 0x9ce4), }, >>> }; >>> >>> /* >>> @@ -5485,33 +5301,6 @@ static struct pci_device_id serial_pci_tbl[] = { >>> { PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_CE4100_UART, >>> PCI_ANY_ID, PCI_ANY_ID, 0, 0, >>> pbn_ce4100_1_115200 }, >>> - /* Intel BayTrail */ >>> - { PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_BYT_UART1, >>> - PCI_ANY_ID, PCI_ANY_ID, >>> - PCI_CLASS_COMMUNICATION_SERIAL << 8, 0xff0000, >>> - pbn_byt }, >>> - { PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_BYT_UART2, >>> - PCI_ANY_ID, PCI_ANY_ID, >>> - PCI_CLASS_COMMUNICATION_SERIAL << 8, 0xff0000, >>> - pbn_byt }, >>> - { PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_BSW_UART1, >>> - PCI_ANY_ID, PCI_ANY_ID, >>> - PCI_CLASS_COMMUNICATION_SERIAL << 8, 0xff0000, >>> - pbn_byt }, >>> - { PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_BSW_UART2, >>> - PCI_ANY_ID, PCI_ANY_ID, >>> - PCI_CLASS_COMMUNICATION_SERIAL << 8, 0xff0000, >>> - pbn_byt }, >>> - >>> - /* Intel Broadwell */ >>> - { PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_BDW_UART1, >>> - PCI_ANY_ID, PCI_ANY_ID, >>> - PCI_CLASS_COMMUNICATION_SERIAL << 8, 0xff0000, >>> - pbn_byt }, >>> - { PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_BDW_UART2, >>> - PCI_ANY_ID, PCI_ANY_ID, >>> - PCI_CLASS_COMMUNICATION_SERIAL << 8, 0xff0000, >>> - pbn_byt }, >>> >>> /* >>> * Intel Quark x1000 >>> diff --git a/drivers/tty/serial/8250/Kconfig b/drivers/tty/serial/8250/Kconfig >>> index 64742a0..6fde7d2 100644 >>> --- a/drivers/tty/serial/8250/Kconfig >>> +++ b/drivers/tty/serial/8250/Kconfig >>> @@ -108,7 +108,6 @@ config SERIAL_8250_PCI >>> tristate "8250/16550 PCI device support" if EXPERT >>> depends on SERIAL_8250 && PCI >>> default SERIAL_8250 >>> - select RATIONAL >>> help >>> This builds standard PCI serial support. You may be able to >>> disable this feature if you only need legacy serial support. >>> @@ -398,6 +397,19 @@ config SERIAL_8250_INGENIC >>> If you have a system using an Ingenic SoC and wish to make use of >>> its UARTs, say Y to this option. If unsure, say N. >>> >>> +config SERIAL_8250_LPSS >>> + tristate "Support for serial ports on Intel LPSS platforms" if EXPERT >>> + default SERIAL_8250 >>> + depends on SERIAL_8250 && PCI >>> + depends on X86 || COMPILE_TEST >>> + select DW_DMAC_CORE if SERIAL_8250_DMA >>> + select DW_DMAC_PCI if X86_INTEL_LPSS >>> + select RATIONAL >>> + help >>> + Selecting this option will enable handling of the extra features >>> + present on the UART found on Intel Braswell SoC and various other >>> + Intel platforms. >>> + >>> config SERIAL_8250_MID >>> tristate "Support for serial ports on Intel MID platforms" >>> depends on SERIAL_8250 && PCI >>> diff --git a/drivers/tty/serial/8250/Makefile b/drivers/tty/serial/8250/Makefile >>> index c9a2d6e..ca37d1c 100644 >>> --- a/drivers/tty/serial/8250/Makefile >>> +++ b/drivers/tty/serial/8250/Makefile >>> @@ -28,6 +28,7 @@ obj-$(CONFIG_SERIAL_8250_LPC18XX) += 8250_lpc18xx.o >>> obj-$(CONFIG_SERIAL_8250_MT6577) += 8250_mtk.o >>> obj-$(CONFIG_SERIAL_8250_UNIPHIER) += 8250_uniphier.o >>> obj-$(CONFIG_SERIAL_8250_INGENIC) += 8250_ingenic.o >>> +obj-$(CONFIG_SERIAL_8250_LPSS) += 8250_lpss.o >>> obj-$(CONFIG_SERIAL_8250_MID) += 8250_mid.o >>> obj-$(CONFIG_SERIAL_8250_MOXA) += 8250_moxa.o >>> obj-$(CONFIG_SERIAL_OF_PLATFORM) += 8250_of.o >>> >> > > > -- To unsubscribe from this list: send the line "unsubscribe dmaengine" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, 2016-04-08 at 15:44 -0700, Peter Hurley wrote: > On 04/08/2016 01:17 AM, Andy Shevchenko wrote: > > > > On Fri, Apr 8, 2016 at 4:42 AM, Peter Hurley <peter@hurleysoftware.c > > om> wrote: > > > > > > On 04/07/2016 01:37 PM, Andy Shevchenko wrote: > > > > > > > > Intes SoCs, such as Braswell, have DesignWare UART. Split out to > > > > separate > > > > module which also will be used for Intel Quark later. > > > What's the rationale? > > 1. Not poison 8250_pci with too many quirks. > > 2. They all use same DMA engine, otherwise we might end up in all > > possible DMA engines included in one file. > > 3. All of them are actually DesignWare, so, in the future we might > > share code between 8250_dw and 8250_lpss. > Just my opinion, but I like to see the rationale in the changelog. Agreed. Already did locally. > > > > And this really isn't a split; this patch introduces a number of > > > significant > > > changes from the pci version. > > Some style changes, yes, but "significant"? > > For example? > I'm just pointing out the changelog doesn't really match the > commit. I'm not suggesting necessarily to redo the series, but just > more > adequately reflect the change. See below. > +struct lpss8250_board { > > > > + unsigned long freq; > > > > + unsigned int base_baud; > > > > + int (*setup)(struct lpss8250 *, struct uart_port *p); > > > > +}; > > > New concept. > > > +struct lpss8250 { > > > > + int line; > > > > + struct lpss8250_board *board; > > > > + > > > > + /* DMA parameters */ > > > > + struct uart_8250_dma dma; > > > > + struct dw_dma_slave dma_param; > > > > + u8 dma_maxburst; > > > > +}; > > > > +static int byt_serial_setup(struct lpss8250 *lpss, struct > > > > uart_port *port) > > > This would have been much easier to review if you had simply moved > > > it here > > > and done the rework in a follow-on patch. > > I didn't quite get this one. > Well, just comparing byt_serial_setup() from the two versions: > 1) dma setup is in a completely separate function > 2) the tx & rx dma parameters are folded together > 3) the port setup is split out > 4) introduction of struct lpss8250 > ... > > > > > How series should look like? > I would have just moved byt_serial_setup() without any of the other > changes > except perhaps replacing pciserial_board with lpss8250_board, and > then made the other changes on top before the Quark patches. > > There is no changelog describing the purpose of struct lpss8250_board, > or > struct lpss8250. Or of the dma changes. Or why dma_maxburst was > parameterized. > ... > > Naturally, I can figure all of that out on my own, but it would have > been > better to read your reasoning. > > It looks alot of work to split out now, so I guess what's done is > done, and I'll > just review this *really* carefully. But imagine if you hadn't wrote > it, and > were reviewing this: it's very difficult to mentally separate out the > changes > and keep track of them while reviewing. Side-by-side diff is nearly > useless... > I sent new version, please, review. > > > > > > > > + ret = lpss->board->setup(lpss, &uart.port); > > > > + if (ret) > > > > + return ret; > > > > + } > > > > + > > > > + ret = lpss8250_dma_setup(lpss, &uart); > > > > + if (ret) > > > > + return ret; > > > I would fold this call into board setup which avoids the > > > ugliness when this error pathway is reworked in the > > > follow-on patches. > > Each of them? > I'm assuming there's just the two: byt_serial_setup() > and qrk_serial_setup()? For now yes. > > Did I overlook something? Perhaps I missed some design goal? Nope. But I still prefer to have separate _dma_setup() helper. I didn't see too much ugliness in the next patches. > > > > + .probe = lpss8250_probe, > > > > + .remove = lpss8250_remove, > > > No power management? > > PCI does the trick, no *special* power management treatment > > required, yes. > I realized that later this am; sorry about that. > [Maybe just put a small note in the changelog about that though?] OK.
diff --git a/drivers/tty/serial/8250/8250_lpss.c b/drivers/tty/serial/8250/8250_lpss.c new file mode 100644 index 0000000..bca4adb --- /dev/null +++ b/drivers/tty/serial/8250/8250_lpss.c @@ -0,0 +1,279 @@ +/* + * 8250_lpss.c - Driver for UART on Intel Braswell and various other Intel SoCs + * + * Copyright (C) 2016 Intel Corporation + * Author: Andy Shevchenko <andriy.shevchenko@linux.intel.com> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + */ + +#include <linux/bitops.h> +#include <linux/module.h> +#include <linux/pci.h> +#include <linux/rational.h> + +#include <linux/dmaengine.h> +#include <linux/platform_data/dma-dw.h> + +#include "8250.h" + +#define PCI_DEVICE_ID_INTEL_BYT_UART1 0x0f0a +#define PCI_DEVICE_ID_INTEL_BYT_UART2 0x0f0c + +#define PCI_DEVICE_ID_INTEL_BSW_UART1 0x228a +#define PCI_DEVICE_ID_INTEL_BSW_UART2 0x228c + +#define PCI_DEVICE_ID_INTEL_BDW_UART1 0x9ce3 +#define PCI_DEVICE_ID_INTEL_BDW_UART2 0x9ce4 + +/* Intel LPSS specific registers */ + +#define BYT_PRV_CLK 0x800 +#define BYT_PRV_CLK_EN BIT(0) +#define BYT_PRV_CLK_M_VAL_SHIFT 1 +#define BYT_PRV_CLK_N_VAL_SHIFT 16 +#define BYT_PRV_CLK_UPDATE BIT(31) + +#define BYT_TX_OVF_INT 0x820 +#define BYT_TX_OVF_INT_MASK BIT(1) + +struct lpss8250; + +struct lpss8250_board { + unsigned long freq; + unsigned int base_baud; + int (*setup)(struct lpss8250 *, struct uart_port *p); +}; + +struct lpss8250 { + int line; + struct lpss8250_board *board; + + /* DMA parameters */ + struct uart_8250_dma dma; + struct dw_dma_slave dma_param; + u8 dma_maxburst; +}; + +/*****************************************************************************/ + +static void lpss8250_set_termios(struct uart_port *p, + struct ktermios *termios, + struct ktermios *old) +{ + unsigned int baud = tty_termios_baud_rate(termios); + struct lpss8250 *lpss = p->private_data; + unsigned long fref = lpss->board->freq, fuart = baud * 16; + unsigned long w = BIT(15) - 1; + unsigned long m, n; + u32 reg; + + /* Get Fuart closer to Fref */ + fuart *= rounddown_pow_of_two(fref / fuart); + + /* + * For baud rates 0.5M, 1M, 1.5M, 2M, 2.5M, 3M, 3.5M and 4M the + * dividers must be adjusted. + * + * uartclk = (m / n) * 100 MHz, where m <= n + */ + rational_best_approximation(fuart, fref, w, w, &m, &n); + p->uartclk = fuart; + + /* Reset the clock */ + reg = (m << BYT_PRV_CLK_M_VAL_SHIFT) | (n << BYT_PRV_CLK_N_VAL_SHIFT); + writel(reg, p->membase + BYT_PRV_CLK); + reg |= BYT_PRV_CLK_EN | BYT_PRV_CLK_UPDATE; + writel(reg, p->membase + BYT_PRV_CLK); + + p->status &= ~UPSTAT_AUTOCTS; + if (termios->c_cflag & CRTSCTS) + p->status |= UPSTAT_AUTOCTS; + + serial8250_do_set_termios(p, termios, old); +} + +/*****************************************************************************/ + +static int byt_serial_setup(struct lpss8250 *lpss, struct uart_port *port) +{ + struct dw_dma_slave *param = &lpss->dma_param; + struct uart_8250_port *up = up_to_u8250p(port); + struct pci_dev *pdev = to_pci_dev(port->dev); + struct pci_dev *dma_dev = pci_get_slot(pdev->bus, PCI_DEVFN(PCI_SLOT(pdev->devfn), 0)); + + switch (pdev->device) { + case PCI_DEVICE_ID_INTEL_BYT_UART1: + case PCI_DEVICE_ID_INTEL_BSW_UART1: + case PCI_DEVICE_ID_INTEL_BDW_UART1: + param->src_id = 3; + param->dst_id = 2; + break; + case PCI_DEVICE_ID_INTEL_BYT_UART2: + case PCI_DEVICE_ID_INTEL_BSW_UART2: + case PCI_DEVICE_ID_INTEL_BDW_UART2: + param->src_id = 5; + param->dst_id = 4; + break; + default: + return -EINVAL; + } + + param->dma_dev = &dma_dev->dev; + param->m_master = 0; + param->p_master = 1; + + /* TODO: Detect FIFO size automaticaly for DesignWare 8250 */ + port->fifosize = 64; + up->tx_loadsz = 64; + + lpss->dma_maxburst = 16; + + port->set_termios = lpss8250_set_termios; + + /* Disable TX counter interrupts */ + writel(BYT_TX_OVF_INT_MASK, port->membase + BYT_TX_OVF_INT); + + return 0; +} + +/*****************************************************************************/ + +static bool lpss8250_dma_filter(struct dma_chan *chan, void *param) +{ + struct dw_dma_slave *dws = param; + + if (dws->dma_dev != chan->device->dev) + return false; + + chan->private = dws; + return true; +} + +static int lpss8250_dma_setup(struct lpss8250 *lpss, struct uart_8250_port *port) +{ + struct uart_8250_dma *dma = &lpss->dma; + struct dw_dma_slave *param = &lpss->dma_param; + struct dw_dma_slave *rx_param, *tx_param; + struct device *dev = port->port.dev; + + rx_param = devm_kzalloc(dev, sizeof(*rx_param), GFP_KERNEL); + if (!rx_param) + return -ENOMEM; + + tx_param = devm_kzalloc(dev, sizeof(*tx_param), GFP_KERNEL); + if (!tx_param) + return -ENOMEM; + + *rx_param = *param; + dma->rxconf.src_maxburst = lpss->dma_maxburst; + + *tx_param = *param; + dma->txconf.dst_maxburst = lpss->dma_maxburst; + + dma->fn = lpss8250_dma_filter; + dma->rx_param = rx_param; + dma->tx_param = tx_param; + + port->dma = dma; + return 0; +} + +/*****************************************************************************/ + +static int lpss8250_probe(struct pci_dev *pdev, const struct pci_device_id *id) +{ + struct uart_8250_port uart; + struct lpss8250 *lpss; + int ret; + + ret = pcim_enable_device(pdev); + if (ret) + return ret; + + pci_set_master(pdev); + + lpss = devm_kzalloc(&pdev->dev, sizeof(*lpss), GFP_KERNEL); + if (!lpss) + return -ENOMEM; + + lpss->board = (struct lpss8250_board *)id->driver_data; + + memset(&uart, 0, sizeof(struct uart_8250_port)); + + uart.port.dev = &pdev->dev; + uart.port.irq = pdev->irq; + uart.port.private_data = lpss; + uart.port.type = PORT_16550A; + uart.port.iotype = UPIO_MEM32; + uart.port.regshift = 2; + uart.port.uartclk = lpss->board->base_baud * 16; + uart.port.flags = UPF_SHARE_IRQ | UPF_FIXED_PORT | UPF_FIXED_TYPE; + + uart.capabilities = UART_CAP_FIFO | UART_CAP_AFE; + + uart.port.mapbase = pci_resource_start(pdev, 0); + uart.port.membase = pcim_iomap(pdev, 0, 0); + if (!uart.port.membase) + return -ENOMEM; + + if (lpss->board->setup) { + ret = lpss->board->setup(lpss, &uart.port); + if (ret) + return ret; + } + + ret = lpss8250_dma_setup(lpss, &uart); + if (ret) + return ret; + + ret = serial8250_register_8250_port(&uart); + if (ret < 0) + return ret; + + lpss->line = ret; + + pci_set_drvdata(pdev, lpss); + return 0; +} + +static void lpss8250_remove(struct pci_dev *pdev) +{ + struct lpss8250 *lpss = pci_get_drvdata(pdev); + + serial8250_unregister_port(lpss->line); +} + +static const struct lpss8250_board byt_board = { + .freq = 100000000, + .base_baud = 2764800, + .setup = byt_serial_setup, +}; + +#define LPSS_DEVICE(id, board) { PCI_VDEVICE(INTEL, id), (kernel_ulong_t)&board } + +static const struct pci_device_id pci_ids[] = { + LPSS_DEVICE(PCI_DEVICE_ID_INTEL_BYT_UART1, byt_board), + LPSS_DEVICE(PCI_DEVICE_ID_INTEL_BYT_UART2, byt_board), + LPSS_DEVICE(PCI_DEVICE_ID_INTEL_BSW_UART1, byt_board), + LPSS_DEVICE(PCI_DEVICE_ID_INTEL_BSW_UART2, byt_board), + LPSS_DEVICE(PCI_DEVICE_ID_INTEL_BDW_UART1, byt_board), + LPSS_DEVICE(PCI_DEVICE_ID_INTEL_BDW_UART2, byt_board), + { }, +}; +MODULE_DEVICE_TABLE(pci, pci_ids); + +static struct pci_driver lpss8250_pci_driver = { + .name = "8250_lpss", + .id_table = pci_ids, + .probe = lpss8250_probe, + .remove = lpss8250_remove, +}; + +module_pci_driver(lpss8250_pci_driver); + +MODULE_AUTHOR("Intel Corporation"); +MODULE_LICENSE("GPL v2"); +MODULE_DESCRIPTION("Intel LPSS UART driver"); diff --git a/drivers/tty/serial/8250/8250_pci.c b/drivers/tty/serial/8250/8250_pci.c index 5eea74d..bb4df5d 100644 --- a/drivers/tty/serial/8250/8250_pci.c +++ b/drivers/tty/serial/8250/8250_pci.c @@ -21,14 +21,10 @@ #include <linux/serial_core.h> #include <linux/8250_pci.h> #include <linux/bitops.h> -#include <linux/rational.h> #include <asm/byteorder.h> #include <asm/io.h> -#include <linux/dmaengine.h> -#include <linux/platform_data/dma-dw.h> - #include "8250.h" /* @@ -1349,145 +1345,6 @@ ce4100_serial_setup(struct serial_private *priv, return ret; } -#define PCI_DEVICE_ID_INTEL_BYT_UART1 0x0f0a -#define PCI_DEVICE_ID_INTEL_BYT_UART2 0x0f0c - -#define PCI_DEVICE_ID_INTEL_BSW_UART1 0x228a -#define PCI_DEVICE_ID_INTEL_BSW_UART2 0x228c - -#define PCI_DEVICE_ID_INTEL_BDW_UART1 0x9ce3 -#define PCI_DEVICE_ID_INTEL_BDW_UART2 0x9ce4 - -#define BYT_PRV_CLK 0x800 -#define BYT_PRV_CLK_EN (1 << 0) -#define BYT_PRV_CLK_M_VAL_SHIFT 1 -#define BYT_PRV_CLK_N_VAL_SHIFT 16 -#define BYT_PRV_CLK_UPDATE (1 << 31) - -#define BYT_TX_OVF_INT 0x820 -#define BYT_TX_OVF_INT_MASK (1 << 1) - -static void -byt_set_termios(struct uart_port *p, struct ktermios *termios, - struct ktermios *old) -{ - unsigned int baud = tty_termios_baud_rate(termios); - unsigned long fref = 100000000, fuart = baud * 16; - unsigned long w = BIT(15) - 1; - unsigned long m, n; - u32 reg; - - /* Get Fuart closer to Fref */ - fuart *= rounddown_pow_of_two(fref / fuart); - - /* - * For baud rates 0.5M, 1M, 1.5M, 2M, 2.5M, 3M, 3.5M and 4M the - * dividers must be adjusted. - * - * uartclk = (m / n) * 100 MHz, where m <= n - */ - rational_best_approximation(fuart, fref, w, w, &m, &n); - p->uartclk = fuart; - - /* Reset the clock */ - reg = (m << BYT_PRV_CLK_M_VAL_SHIFT) | (n << BYT_PRV_CLK_N_VAL_SHIFT); - writel(reg, p->membase + BYT_PRV_CLK); - reg |= BYT_PRV_CLK_EN | BYT_PRV_CLK_UPDATE; - writel(reg, p->membase + BYT_PRV_CLK); - - p->status &= ~UPSTAT_AUTOCTS; - if (termios->c_cflag & CRTSCTS) - p->status |= UPSTAT_AUTOCTS; - - serial8250_do_set_termios(p, termios, old); -} - -static bool byt_dma_filter(struct dma_chan *chan, void *param) -{ - struct dw_dma_slave *dws = param; - - if (dws->dma_dev != chan->device->dev) - return false; - - chan->private = dws; - return true; -} - -static int -byt_serial_setup(struct serial_private *priv, - const struct pciserial_board *board, - struct uart_8250_port *port, int idx) -{ - struct pci_dev *pdev = priv->dev; - struct device *dev = port->port.dev; - struct uart_8250_dma *dma; - struct dw_dma_slave *tx_param, *rx_param; - struct pci_dev *dma_dev; - int ret; - - dma = devm_kzalloc(dev, sizeof(*dma), GFP_KERNEL); - if (!dma) - return -ENOMEM; - - tx_param = devm_kzalloc(dev, sizeof(*tx_param), GFP_KERNEL); - if (!tx_param) - return -ENOMEM; - - rx_param = devm_kzalloc(dev, sizeof(*rx_param), GFP_KERNEL); - if (!rx_param) - return -ENOMEM; - - switch (pdev->device) { - case PCI_DEVICE_ID_INTEL_BYT_UART1: - case PCI_DEVICE_ID_INTEL_BSW_UART1: - case PCI_DEVICE_ID_INTEL_BDW_UART1: - rx_param->src_id = 3; - tx_param->dst_id = 2; - break; - case PCI_DEVICE_ID_INTEL_BYT_UART2: - case PCI_DEVICE_ID_INTEL_BSW_UART2: - case PCI_DEVICE_ID_INTEL_BDW_UART2: - rx_param->src_id = 5; - tx_param->dst_id = 4; - break; - default: - return -EINVAL; - } - - rx_param->m_master = 0; - rx_param->p_master = 1; - - dma->rxconf.src_maxburst = 16; - - tx_param->m_master = 0; - tx_param->p_master = 1; - - dma->txconf.dst_maxburst = 16; - - dma_dev = pci_get_slot(pdev->bus, PCI_DEVFN(PCI_SLOT(pdev->devfn), 0)); - rx_param->dma_dev = &dma_dev->dev; - tx_param->dma_dev = &dma_dev->dev; - - dma->fn = byt_dma_filter; - dma->rx_param = rx_param; - dma->tx_param = tx_param; - - ret = pci_default_setup(priv, board, port, idx); - port->port.iotype = UPIO_MEM; - port->port.type = PORT_16550A; - port->port.flags = (port->port.flags | UPF_FIXED_PORT | UPF_FIXED_TYPE); - port->port.set_termios = byt_set_termios; - port->port.fifosize = 64; - port->tx_loadsz = 64; - port->dma = dma; - port->capabilities = UART_CAP_FIFO | UART_CAP_AFE; - - /* Disable Tx counter interrupts */ - writel(BYT_TX_OVF_INT_MASK, port->port.membase + BYT_TX_OVF_INT); - - return ret; -} - static int pci_omegapci_setup(struct serial_private *priv, const struct pciserial_board *board, @@ -2015,48 +1872,6 @@ static struct pci_serial_quirk pci_serial_quirks[] __refdata = { .subdevice = PCI_ANY_ID, .setup = kt_serial_setup, }, - { - .vendor = PCI_VENDOR_ID_INTEL, - .device = PCI_DEVICE_ID_INTEL_BYT_UART1, - .subvendor = PCI_ANY_ID, - .subdevice = PCI_ANY_ID, - .setup = byt_serial_setup, - }, - { - .vendor = PCI_VENDOR_ID_INTEL, - .device = PCI_DEVICE_ID_INTEL_BYT_UART2, - .subvendor = PCI_ANY_ID, - .subdevice = PCI_ANY_ID, - .setup = byt_serial_setup, - }, - { - .vendor = PCI_VENDOR_ID_INTEL, - .device = PCI_DEVICE_ID_INTEL_BSW_UART1, - .subvendor = PCI_ANY_ID, - .subdevice = PCI_ANY_ID, - .setup = byt_serial_setup, - }, - { - .vendor = PCI_VENDOR_ID_INTEL, - .device = PCI_DEVICE_ID_INTEL_BSW_UART2, - .subvendor = PCI_ANY_ID, - .subdevice = PCI_ANY_ID, - .setup = byt_serial_setup, - }, - { - .vendor = PCI_VENDOR_ID_INTEL, - .device = PCI_DEVICE_ID_INTEL_BDW_UART1, - .subvendor = PCI_ANY_ID, - .subdevice = PCI_ANY_ID, - .setup = byt_serial_setup, - }, - { - .vendor = PCI_VENDOR_ID_INTEL, - .device = PCI_DEVICE_ID_INTEL_BDW_UART2, - .subvendor = PCI_ANY_ID, - .subdevice = PCI_ANY_ID, - .setup = byt_serial_setup, - }, /* * ITE */ @@ -2921,7 +2736,6 @@ enum pci_board_num_t { pbn_ADDIDATA_PCIe_4_3906250, pbn_ADDIDATA_PCIe_8_3906250, pbn_ce4100_1_115200, - pbn_byt, pbn_qrk, pbn_omegapci, pbn_NETMOS9900_2s_115200, @@ -3698,12 +3512,6 @@ static struct pciserial_board pci_boards[] = { .base_baud = 921600, .reg_shift = 2, }, - [pbn_byt] = { - .flags = FL_BASE0, - .num_ports = 1, - .base_baud = 2764800, - .reg_shift = 2, - }, [pbn_qrk] = { .flags = FL_BASE0, .num_ports = 1, @@ -3820,6 +3628,14 @@ static const struct pci_device_id blacklist[] = { { PCI_VDEVICE(INTEL, 0x081d), }, { PCI_VDEVICE(INTEL, 0x1191), }, { PCI_VDEVICE(INTEL, 0x19d8), }, + + /* Intel platforms with DesignWare UART */ + { PCI_VDEVICE(INTEL, 0x0f0a), }, + { PCI_VDEVICE(INTEL, 0x0f0c), }, + { PCI_VDEVICE(INTEL, 0x228a), }, + { PCI_VDEVICE(INTEL, 0x228c), }, + { PCI_VDEVICE(INTEL, 0x9ce3), }, + { PCI_VDEVICE(INTEL, 0x9ce4), }, }; /* @@ -5485,33 +5301,6 @@ static struct pci_device_id serial_pci_tbl[] = { { PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_CE4100_UART, PCI_ANY_ID, PCI_ANY_ID, 0, 0, pbn_ce4100_1_115200 }, - /* Intel BayTrail */ - { PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_BYT_UART1, - PCI_ANY_ID, PCI_ANY_ID, - PCI_CLASS_COMMUNICATION_SERIAL << 8, 0xff0000, - pbn_byt }, - { PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_BYT_UART2, - PCI_ANY_ID, PCI_ANY_ID, - PCI_CLASS_COMMUNICATION_SERIAL << 8, 0xff0000, - pbn_byt }, - { PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_BSW_UART1, - PCI_ANY_ID, PCI_ANY_ID, - PCI_CLASS_COMMUNICATION_SERIAL << 8, 0xff0000, - pbn_byt }, - { PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_BSW_UART2, - PCI_ANY_ID, PCI_ANY_ID, - PCI_CLASS_COMMUNICATION_SERIAL << 8, 0xff0000, - pbn_byt }, - - /* Intel Broadwell */ - { PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_BDW_UART1, - PCI_ANY_ID, PCI_ANY_ID, - PCI_CLASS_COMMUNICATION_SERIAL << 8, 0xff0000, - pbn_byt }, - { PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_BDW_UART2, - PCI_ANY_ID, PCI_ANY_ID, - PCI_CLASS_COMMUNICATION_SERIAL << 8, 0xff0000, - pbn_byt }, /* * Intel Quark x1000 diff --git a/drivers/tty/serial/8250/Kconfig b/drivers/tty/serial/8250/Kconfig index 64742a0..6fde7d2 100644 --- a/drivers/tty/serial/8250/Kconfig +++ b/drivers/tty/serial/8250/Kconfig @@ -108,7 +108,6 @@ config SERIAL_8250_PCI tristate "8250/16550 PCI device support" if EXPERT depends on SERIAL_8250 && PCI default SERIAL_8250 - select RATIONAL help This builds standard PCI serial support. You may be able to disable this feature if you only need legacy serial support. @@ -398,6 +397,19 @@ config SERIAL_8250_INGENIC If you have a system using an Ingenic SoC and wish to make use of its UARTs, say Y to this option. If unsure, say N. +config SERIAL_8250_LPSS + tristate "Support for serial ports on Intel LPSS platforms" if EXPERT + default SERIAL_8250 + depends on SERIAL_8250 && PCI + depends on X86 || COMPILE_TEST + select DW_DMAC_CORE if SERIAL_8250_DMA + select DW_DMAC_PCI if X86_INTEL_LPSS + select RATIONAL + help + Selecting this option will enable handling of the extra features + present on the UART found on Intel Braswell SoC and various other + Intel platforms. + config SERIAL_8250_MID tristate "Support for serial ports on Intel MID platforms" depends on SERIAL_8250 && PCI diff --git a/drivers/tty/serial/8250/Makefile b/drivers/tty/serial/8250/Makefile index c9a2d6e..ca37d1c 100644 --- a/drivers/tty/serial/8250/Makefile +++ b/drivers/tty/serial/8250/Makefile @@ -28,6 +28,7 @@ obj-$(CONFIG_SERIAL_8250_LPC18XX) += 8250_lpc18xx.o obj-$(CONFIG_SERIAL_8250_MT6577) += 8250_mtk.o obj-$(CONFIG_SERIAL_8250_UNIPHIER) += 8250_uniphier.o obj-$(CONFIG_SERIAL_8250_INGENIC) += 8250_ingenic.o +obj-$(CONFIG_SERIAL_8250_LPSS) += 8250_lpss.o obj-$(CONFIG_SERIAL_8250_MID) += 8250_mid.o obj-$(CONFIG_SERIAL_8250_MOXA) += 8250_moxa.o obj-$(CONFIG_SERIAL_OF_PLATFORM) += 8250_of.o
Intes SoCs, such as Braswell, have DesignWare UART. Split out to separate module which also will be used for Intel Quark later. Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> --- drivers/tty/serial/8250/8250_lpss.c | 279 ++++++++++++++++++++++++++++++++++++ drivers/tty/serial/8250/8250_pci.c | 227 ++--------------------------- drivers/tty/serial/8250/Kconfig | 14 +- drivers/tty/serial/8250/Makefile | 1 + 4 files changed, 301 insertions(+), 220 deletions(-) create mode 100644 drivers/tty/serial/8250/8250_lpss.c