lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:	Fri, 8 Apr 2016 15:44:21 -0700
From:	Peter Hurley <peter@...leysoftware.com>
To:	Andy Shevchenko <andy.shevchenko@...il.com>
Cc:	Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
	Vinod Koul <vinod.koul@...el.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	dmaengine <dmaengine@...r.kernel.org>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	"Puustinen, Ismo" <ismo.puustinen@...el.com>,
	Heikki Krogerus <heikki.krogerus@...ux.intel.com>,
	"linux-serial@...r.kernel.org" <linux-serial@...r.kernel.org>
Subject: Re: [PATCH v1 09/12] serial: 8250_lpss: split LPSS driver to separate
 module

On 04/08/2016 01:17 AM, Andy Shevchenko wrote:
> On Fri, Apr 8, 2016 at 4:42 AM, Peter Hurley <peter@...leysoftware.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@...ux.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@...ux.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
>>>
>>
> 
> 
> 

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ