On Mon, 2017-04-24 at 21:26 +0200, Jan Kiszka wrote:
> On 2017-04-24 19:29, Andy Shevchenko wrote:
> > There are Exar quirks in 8250_port which belong to 8250_exar module.
> > Move them to the correct module and do not contaminate generic code
> > with it.
> >
> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
> > ---
> > Just compile tested. I have no hardware to test this approach. It
> > would
> > be nice if you guys (Jan, Sudip) can take it and continue on real
> > hardware. Why am I interested in this? It would make my life
> > slightly
> > easier for runtime PM support developing for 8250.
>
> I will try to through this on the IOT2040 tomorrow. What's the best
> way
> to test the PM paths?
Open port, close port. Do it again.
It will crash on probe stage :-( I forgot to replace serial_in() by
readl() or readb(). I think the port type would be assigned in ->setup()
hooks directly without this detection (detection seems not working in
IBM case, I dunno if it worked before).
>
> In return, I'll finally send the patches to enable our device's EXAR
> with upstream. They seem ready now.
Good!
>
> Jan
>
> >
> > TODO:
> > - move get_divisor() / set_divisor() quirks as well
> > - perhaps move startup() / shutdown() either
> > - patch can be split a bit since there is at least one useless
> > check
> > in exar_handle_irq() for port type (this might be separate clean
> > up)
> >
> > I would be pleased if this just works after being applied.
> >
> > drivers/tty/serial/8250/8250_exar.c | 57
> > ++++++++++++++++++++++++++++-
> > drivers/tty/serial/8250/8250_port.c | 73 --------------------------
> > -----------
> > 2 files changed, 56 insertions(+), 74 deletions(-)
> >
> > diff --git a/drivers/tty/serial/8250/8250_exar.c
> > b/drivers/tty/serial/8250/8250_exar.c
> > index 2a8f00d96e6b..ea963c0b8697 100644
> > --- a/drivers/tty/serial/8250/8250_exar.c
> > +++ b/drivers/tty/serial/8250/8250_exar.c
> > @@ -47,6 +47,13 @@
> > #define UART_EXAR_TXTRG 0x0a /* Tx FIFO
> > trigger level write-only */
> > #define UART_EXAR_RXTRG 0x0b /* Rx FIFO
> > trigger level write-only */
> >
> > +/*
> > + * These are definitions for the Exar XR17V35X and XR17(C|D)15X
> > + */
> > +#define UART_EXAR_INT0 0x80
> > +#define UART_EXAR_SLEEP 0x8b /* Sleep mode */
> > +#define UART_EXAR_DVID 0x8d /* Device
> > identification */
> > +
> > #define UART_EXAR_MPIOINT_7_0 0x8f /* MPIOINT[7:0] */
> > #define UART_EXAR_MPIOLVL_7_0 0x90 /* MPIOLVL[7:0] */
> > #define UART_EXAR_MPIO3T_7_0 0x91 /* MPIO3T[7:0] */
> > @@ -82,12 +89,44 @@ struct exar8250 {
> > int line[0];
> > };
> >
> > +static void exar_pm(struct uart_port *port, unsigned int state,
> > unsigned int old)
> > +{
> > + /*
> > + * Exar UARTs have a SLEEP register that enables or
> > disables
> > + * each UART to enter sleep mode separately. On the
> > XR17V35x the
> > + * register is accessible to each UART at the
> > UART_EXAR_SLEEP
> > + * offset but the UART channel may only write to the
> > corresponding
> > + * bit.
> > + */
> > + serial_port_out(port, UART_EXAR_SLEEP, state ? 0xff : 0);
> > +}
> > +
> > +/*
> > + * 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 (serial_port_in(port, UART_EXAR_INT0) != 0)
> > + ret = 1;
> > +
> > + ret |= serial8250_handle_irq(port, iir);
> > +
> > + return ret;
> > +}
> > +
> > static int default_setup(struct exar8250 *priv, struct pci_dev
> > *pcidev,
> > int idx, unsigned int offset,
> > struct uart_8250_port *port)
> > {
> > const struct exar8250_board *board = priv->board;
> > unsigned int bar = 0;
> > + unsigned char status;
> >
> > if (!pcim_iomap_table(pcidev)[bar] && !pcim_iomap(pcidev,
> > bar, 0))
> > return -ENOMEM;
> > @@ -97,6 +136,22 @@ static int default_setup(struct exar8250 *priv,
> > struct pci_dev *pcidev,
> > port->port.membase = pcim_iomap_table(pcidev)[bar] +
> > offset;
> > port->port.regshift = board->reg_shift;
> >
> > + /*
> > + * XR17V35x UARTs have an extra divisor register, DLD that
> > gets
> > + * enabled with when DLAB is set which will cause the
> > device to
> > + * incorrectly match and assign port type to PORT_16650.
> > The EFR
> > + * for this UART is found at offset 0x09. Instead check the
> > + * Deice ID (DVID) register for a 2, 4 or 8 port UART.
> > + */
> > + status = serial_in(port, UART_EXAR_DVID);
Kaboom!
> > + if (status == 0x82 || status == 0x84 || status == 0x88)
> > + port->port.type = PORT_XR17V35X;
> > + else
> > + port->port.type = PORT_XR17D15X;
> > +
> > + port->port.pm = exar_pm;
> > + port->port.handle_irq = exar_handle_irq;
> > +
> > return 0;
> > }
> >
> > @@ -289,7 +344,7 @@ exar_pci_probe(struct pci_dev *pcidev, const
> > struct pci_device_id *ent)
> > return rc;
> >
> > memset(&uart, 0, sizeof(uart));
> > - uart.port.flags = UPF_SKIP_TEST | UPF_BOOT_AUTOCONF |
> > UPF_SHARE_IRQ
> > + uart.port.flags = UPF_SHARE_IRQ | UPF_FIXED_PORT |
> > UPF_FIXED_TYPE
> > | UPF_EXAR_EFR;
> > uart.port.irq = pci_irq_vector(pcidev, 0);
> > uart.port.dev = &pcidev->dev;
> > diff --git a/drivers/tty/serial/8250/8250_port.c
> > b/drivers/tty/serial/8250/8250_port.c
> > index ca9778fc32cb..db12f8cabe04 100644
> > --- a/drivers/tty/serial/8250/8250_port.c
> > +++ b/drivers/tty/serial/8250/8250_port.c
> > @@ -48,8 +48,6 @@
> > * These are definitions for the Exar XR17V35X and XR17(C|D)15X
> > */
> > #define UART_EXAR_INT0 0x80
> > -#define UART_EXAR_SLEEP 0x8b /* Sleep mode */
> > -#define UART_EXAR_DVID 0x8d /* Device
> > identification */
> >
> > /*
> > * Debugging.
> > @@ -437,7 +435,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)
> > {
> > @@ -684,18 +681,6 @@ EXPORT_SYMBOL_GPL(serial8250_rpm_put_tx);
> > static void serial8250_set_sleep(struct uart_8250_port *p, int
> > sleep)
> > {
> > unsigned char lcr = 0, efr = 0;
> > - /*
> > - * Exar UARTs have a SLEEP register that enables or
> > disables
> > - * each UART to enter sleep mode separately. On the
> > XR17V35x the
> > - * register is accessible to each UART at the
> > UART_EXAR_SLEEP
> > - * offset but the UART channel may only write to the
> > corresponding
> > - * bit.
> > - */
> > - if ((p->port.type == PORT_XR17V35X) ||
> > - (p->port.type == PORT_XR17D15X)) {
> > - serial_out(p, UART_EXAR_SLEEP, sleep ? 0xff : 0);
> > - return;
> > - }
> >
> > if (p->capabilities & UART_CAP_SLEEP) {
> > if (p->capabilities & UART_CAP_EFR) {
> > @@ -984,27 +969,6 @@ static void autoconfig_16550a(struct
> > uart_8250_port *up)
> > up->capabilities |= UART_CAP_FIFO;
> >
> > /*
> > - * XR17V35x UARTs have an extra divisor register, DLD
> > - * that gets enabled with when DLAB is set which will
> > - * cause the device to incorrectly match and assign
> > - * port type to PORT_16650. The EFR for this UART is
> > - * found at offset 0x09. Instead check the Deice ID (DVID)
> > - * register for a 2, 4 or 8 port UART.
> > - */
> > - if (up->port.flags & UPF_EXAR_EFR) {
> > - status1 = serial_in(up, UART_EXAR_DVID);
> > - if (status1 == 0x82 || status1 == 0x84 || status1
> > == 0x88) {
> > - DEBUG_AUTOCONF("Exar XR17V35x ");
> > - up->port.type = PORT_XR17V35X;
> > - up->capabilities |= UART_CAP_AFE |
> > UART_CAP_EFR |
> > - UART_CAP_SLEEP;
> > -
> > - return;
> > - }
> > -
> > - }
> > -
> > - /*
> > * Check for presence of the EFR when DLAB is set.
> > * Only ST16C650V1 UARTs pass this test.
> > */
> > @@ -1143,18 +1107,6 @@ static void autoconfig_16550a(struct
> > uart_8250_port *up)
> > serial_out(up, UART_IER, iersave);
> >
> > /*
> > - * Exar uarts have EFR in a weird location
> > - */
> > - if (up->port.flags & UPF_EXAR_EFR) {
> > - DEBUG_AUTOCONF("Exar XR17D15x ");
> > - up->port.type = PORT_XR17D15X;
> > - up->capabilities |= UART_CAP_AFE | UART_CAP_EFR |
> > - UART_CAP_SLEEP;
> > -
> > - return;
> > - }
> > -
> > - /*
> > * We distinguish between 16550A and U6 16550A by counting
> > * how many bytes are in the FIFO.
> > */
> > @@ -1850,26 +1802,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
> > @@ -3113,11 +3045,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
