On Wed, 2018-01-24 at 18:19 -0600, Aaron Sierra wrote:
> Exar sleep wake-up handling has been done on a per-channel basis by
> virtue of INT0 being accessible from each channel's address space. I
> believe this was initially done out of necessity, but now that Exar
> devices have their own driver, we can do things more efficiently by
> registering a dedicated INT0 handler at the PCI device level.
>
> I see this change providing the following benefits:
>
> 1. If more than one port is active, eliminates the redundant bus
> cycles for reading INT0 on every interrupt.
> 2. This note associated with hooking in the per-channel handler in
> 8250_port.c is resolved:
> /* Fixme: probably not the best place for this */
>
+Cc: Jan, Sudip
> Cc: Matt Schulte <matts@xxxxxxxxxxxxxxxxxxxx>
> Signed-off-by: Aaron Sierra <asierra@xxxxxxxxxxx>
> ---
> drivers/tty/serial/8250/8250_exar.c | 34
> ++++++++++++++++++++++++++++++----
> drivers/tty/serial/8250/8250_port.c | 26 --------------------------
> 2 files changed, 30 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/tty/serial/8250/8250_exar.c
> b/drivers/tty/serial/8250/8250_exar.c
> index a402878..38af306 100644
> --- a/drivers/tty/serial/8250/8250_exar.c
> +++ b/drivers/tty/serial/8250/8250_exar.c
> @@ -34,6 +34,7 @@
> #define PCI_DEVICE_ID_EXAR_XR17V4358 0x4358
> #define PCI_DEVICE_ID_EXAR_XR17V8358 0x8358
>
> +#define UART_EXAR_INT0 0x80
> #define UART_EXAR_8XMODE 0x88 /* 8X sampling rate
> select */
>
> #define UART_EXAR_FCTR 0x08 /* Feature Control
> Register */
> @@ -121,6 +122,7 @@ struct exar8250_board {
> struct exar8250 {
> unsigned int nr;
> struct exar8250_board *board;
> + void __iomem *virt;
> int line[0];
> };
>
> @@ -131,12 +133,9 @@ static int default_setup(struct exar8250 *priv,
> struct pci_dev *pcidev,
> const struct exar8250_board *board = priv->board;
> unsigned int bar = 0;
>
> - if (!pcim_iomap_table(pcidev)[bar] && !pcim_iomap(pcidev,
> bar, 0))
> - return -ENOMEM;
> -
> port->port.iotype = UPIO_MEM;
> port->port.mapbase = pci_resource_start(pcidev, bar) +
> offset;
> - port->port.membase = pcim_iomap_table(pcidev)[bar] + offset;
> + port->port.membase = priv->virt + offset;
> port->port.regshift = board->reg_shift;
>
> return 0;
> @@ -420,6 +419,25 @@ static void pci_xr17v35x_exit(struct pci_dev
> *pcidev)
> port->port.private_data = NULL;
> }
>
> +/*
> + * These Exar UARTs have an extra interrupt indicator that could fire
> for a
> + * few interrupts that are not presented/cleared through IIR. One of
> which is
> + * a wakeup interrupt when coming out of sleep. These interrupts are
> only
> + * cleared by reading global INT0 or INT1 registers as interrupts are
> + * associated with channel 0. The INT[3:0] registers _are_ accessible
> from each
> + * channel's address space, but for the sake of bus efficiency we
> register a
> + * dedicated handler at the PCI device level to handle them.
> + */
> +static irqreturn_t exar_misc_handler(int irq, void *data)
> +{
> + struct exar8250 *priv = data;
> +
> + /* Clear all PCI interrupts by reading INT0. No effect on IIR
> */
> + ioread8(priv->virt + UART_EXAR_INT0);
> +
> + return IRQ_HANDLED;
> +}
> +
> static int
> exar_pci_probe(struct pci_dev *pcidev, const struct pci_device_id
> *ent)
> {
> @@ -448,6 +466,9 @@ exar_pci_probe(struct pci_dev *pcidev, const
> struct pci_device_id *ent)
> return -ENOMEM;
>
> priv->board = board;
> + priv->virt = pcim_iomap(pcidev, bar, 0);
> + if (!priv->virt)
> + return -ENOMEM;
>
> pci_set_master(pcidev);
>
> @@ -461,6 +482,11 @@ exar_pci_probe(struct pci_dev *pcidev, const
> struct pci_device_id *ent)
> uart.port.irq = pci_irq_vector(pcidev, 0);
> uart.port.dev = &pcidev->dev;
>
> + rc = devm_request_irq(&pcidev->dev, uart.port.irq,
> exar_misc_handler,
> + IRQF_SHARED, "exar_uart", priv);
> + if (rc)
> + return rc;
> +
> for (i = 0; i < nr_ports && i < maxnr; i++) {
> rc = board->setup(priv, pcidev, &uart, i);
> if (rc) {
> diff --git a/drivers/tty/serial/8250/8250_port.c
> b/drivers/tty/serial/8250/8250_port.c
> index 1143455..1328c7e 100644
> --- a/drivers/tty/serial/8250/8250_port.c
> +++ b/drivers/tty/serial/8250/8250_port.c
> @@ -441,7 +441,6 @@ static void io_serial_out(struct uart_port *p, int
> offset, int value)
> }
>
> static int serial8250_default_handle_irq(struct uart_port *port);
> -static int exar_handle_irq(struct uart_port *port);
>
> static void set_io_from_upio(struct uart_port *p)
> {
> @@ -1884,26 +1883,6 @@ static int serial8250_default_handle_irq(struct
> uart_port *port)
> }
>
> /*
> - * These Exar UARTs have an extra interrupt indicator that could
> - * fire for a few unimplemented interrupts. One of which is a
> - * wakeup event when coming out of sleep. Put this here just
> - * to be on the safe side that these interrupts don't go unhandled.
> - */
> -static int exar_handle_irq(struct uart_port *port)
> -{
> - unsigned int iir = serial_port_in(port, UART_IIR);
> - int ret = 0;
> -
> - if (((port->type == PORT_XR17V35X) || (port->type ==
> PORT_XR17D15X)) &&
> - serial_port_in(port, UART_EXAR_INT0) != 0)
> - ret = 1;
> -
> - ret |= serial8250_handle_irq(port, iir);
> -
> - return ret;
> -}
> -
> -/*
> * Newer 16550 compatible parts such as the SC16C650 & Altera 16550
> Soft IP
> * have a programmable TX threshold that triggers the THRE interrupt
> in
> * the IIR register. In this case, the THRE interrupt indicates the
> FIFO
> @@ -3067,11 +3046,6 @@ static void serial8250_config_port(struct
> uart_port *port, int flags)
> if (port->type == PORT_UNKNOWN)
> serial8250_release_std_resource(up);
>
> - /* Fixme: probably not the best place for this */
> - if ((port->type == PORT_XR17V35X) ||
> - (port->type == PORT_XR17D15X))
> - port->handle_irq = exar_handle_irq;
> -
> register_dev_spec_attr_grp(up);
> up->fcr = uart_config[up->port.type].fcr;
> }
--
Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
Intel Finland Oy
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
