[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <Y5DxEszrq0rXVqvl@smile.fi.intel.com>
Date: Wed, 7 Dec 2022 22:01:22 +0200
From: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
To: Kumaravel Thiagarajan <kumaravel.thiagarajan@...rochip.com>
Cc: linux-kernel@...r.kernel.org, linux-serial@...r.kernel.org,
gregkh@...uxfoundation.org, jirislaby@...nel.org,
ilpo.jarvinen@...ux.intel.com, macro@...am.me.uk, cang1@...e.co.uk,
colin.i.king@...il.com, phil.edworthy@...esas.com,
biju.das.jz@...renesas.com, geert+renesas@...der.be,
lukas@...ner.de, u.kleine-koenig@...gutronix.de, wander@...hat.com,
etremblay@...tech-controls.com, jk@...abs.org,
Tharun Kumar P <tharunkumar.pasumarthi@...rochip.com>
Subject: Re: [PATCH v7 tty-next 2/4] serial: 8250_pci1xxxx: Add driver for
quad-uart support
On Thu, Dec 08, 2022 at 05:23:03AM +0530, Kumaravel Thiagarajan wrote:
> pci1xxxx is a PCIe switch with a multi-function endpoint on one of
> its downstream ports. Quad-uart is one of the functions in the
> multi-function endpoint. This driver loads for the quad-uart and
> enumerates single or multiple instances of uart based on the PCIe
> subsystem device ID.
...
> +static int pci1xxxx_get_physical_port(int subsys_dev, int port)
> +{
> + static int logical_to_physical_port_idx[][MAX_PORTS] = {
> + {0, 1, 2, 3}, /* PCI12000 PCI11010 PCI11101 PCI11400 PCI11414 */
> + {0, 1, 2, 3}, /* PCI4p */
> + {0, 1, 2, -1}, /* PCI3p012 */
> + {0, 1, 3, -1}, /* PCI3p013 */
> + {0, 2, 3, -1}, /* PCI3p023 */
> + {1, 2, 3, -1}, /* PCI3p123 */
> + {0, 1, -1, -1}, /* PCI2p01 */
> + {0, 2, -1, -1}, /* PCI2p02 */
> + {0, 3, -1, -1}, /* PCI2p03 */
> + {1, 2, -1, -1}, /* PCI2p12 */
> + {1, 3, -1, -1}, /* PCI2p13 */
> + {2, 3, -1, -1}, /* PCI2p23 */
> + {0, -1, -1, -1},/* PCI1p0 */
> + {1, -1, -1, -1},/* PCI1p1 */
> + {2, -1, -1, -1},/* PCI1p2 */
> + {3, -1, -1, -1},/* PCI1p3 */
You can make columns nicely indented, but it is up to you,
...
{0, 1, 2, 3}, /* PCI4p */
...
{1, 2, 3, -1}, /* PCI3p123 */
...
{2, 3, -1, -1}, /* PCI2p23 */
{0, -1, -1, -1}, /* PCI1p0 */
...
> + };
> +
> + return logical_to_physical_port_idx[subsys_dev][port];
> +}
...
> + priv->membase = pcim_iomap(pdev, 0, 0);
Is any of those card can have an IO bar (instead of MEM)?
> + if (!priv->membase)
> + return -ENOMEM;
...
> + if (num_vectors == 4)
> + writeb(UART_PCI_CTRL_SET_MULTIPLE_MSI,
> + priv->membase + UART_PCI_CTRL_REG);
If you set this unconditionally when num_vectors < 4, would it still work?
> + else
> + uart.port.irq = pci_irq_vector(pdev, 0);
I would move this to the below...
...
> + if (num_vectors == 4)
> + uart.port.irq = pci_irq_vector(priv->pdev, port_idx);
...here as IRQ assignment to be seen at one place.
--
With Best Regards,
Andy Shevchenko
Powered by blists - more mailing lists