Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1759248AbcDHWoa (ORCPT ); Fri, 8 Apr 2016 18:44:30 -0400 Received: from mail-pf0-f170.google.com ([209.85.192.170]:34458 "EHLO mail-pf0-f170.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1759222AbcDHWo1 (ORCPT ); Fri, 8 Apr 2016 18:44:27 -0400 Subject: Re: [PATCH v1 09/12] serial: 8250_lpss: split LPSS driver to separate module To: Andy Shevchenko References: <1460061433-63750-1-git-send-email-andriy.shevchenko@linux.intel.com> <1460061433-63750-10-git-send-email-andriy.shevchenko@linux.intel.com> <57070C93.2090000@hurleysoftware.com> Cc: Andy Shevchenko , Vinod Koul , "linux-kernel@vger.kernel.org" , dmaengine , Greg Kroah-Hartman , "Puustinen, Ismo" , Heikki Krogerus , "linux-serial@vger.kernel.org" From: Peter Hurley Message-ID: <57083445.9090009@hurleysoftware.com> Date: Fri, 8 Apr 2016 15:44:21 -0700 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.6.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 25926 Lines: 757 On 04/08/2016 01:17 AM, Andy Shevchenko wrote: > On Fri, Apr 8, 2016 at 4:42 AM, Peter Hurley 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 >>> --- >>> 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 >>> + * >>> + * 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 >>> +#include >>> +#include >>> +#include >>> + >>> +#include >>> +#include >>> + >>> +#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 >>> #include >>> #include >>> -#include >>> >>> #include >>> #include >>> >>> -#include >>> -#include >>> - >>> #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 >>> >> > > >