----- Original Message -----
> From: "Andy Shevchenko" <andriy.shevchenko@xxxxxxxxxxxxxxx>
> Sent: Monday, July 16, 2018 8:35:39 AM
> 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.
>
> While the commit
>
> c7e1b4059075 ("tty: serial: exar: Relocate sleep wake-up handling")
>
> already moved IRQ handler and even improved it, this change is about
> port type detection and power management. The fractional divisor support
> will be moved later on.
>
> Cc: Aaron Sierra <asierra@xxxxxxxxxxx>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
> ---
> drivers/tty/serial/8250/8250_exar.c | 32 +++++++++++++++++--
> drivers/tty/serial/8250/8250_port.c | 48 +----------------------------
> 2 files changed, 31 insertions(+), 49 deletions(-)
Hi Andy,
I've only managed a little testing so far. I appear to be hung up on an
issue with an unrelated patch. I just wanted to point out a couple issues
that I've run into already.
> diff --git a/drivers/tty/serial/8250/8250_exar.c
> b/drivers/tty/serial/8250/8250_exar.c
> index 102e059472ee..ab14072e7210 100644
> --- a/drivers/tty/serial/8250/8250_exar.c
> +++ b/drivers/tty/serial/8250/8250_exar.c
> @@ -36,6 +36,8 @@
>
> #define UART_EXAR_INT0 0x80
> #define UART_EXAR_8XMODE 0x88 /* 8X sampling rate select */
> +#define UART_EXAR_SLEEP 0x8b /* Sleep mode */
> +#define UART_EXAR_DVID 0x8d /* Device identification */
>
> #define UART_EXAR_FCTR 0x08 /* Feature Control Register */
> #define UART_FCTR_EXAR_IRDA 0x10 /* IrDa data encode select */
> @@ -126,18 +128,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);
> +}
> +
> 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;
>
> port->port.iotype = UPIO_MEM;
> port->port.mapbase = pci_resource_start(pcidev, bar) + offset;
> port->port.membase = priv->virt + 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);
Using serial_in() here results in a NULL pointer dereference. Replacing it
with this worked around the issue:
status = ioread8(port->port.membase + UART_EXAR_DVID);
> + if (status == 0x82 || status == 0x84 || status == 0x88)
> + port->port.type = PORT_XR17V35X;
> + else
> + port->port.type = PORT_XR17D15X;
> +
> + port->port.pm = exar_pm;
> return 0;
> }
>
[ snip ]
> @@ -710,19 +708,8 @@ 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.
> - */
> +
> serial8250_rpm_get(p);
> - if ((p->port.type == PORT_XR17V35X) ||
> - (p->port.type == PORT_XR17D15X)) {
> - serial_out(p, UART_EXAR_SLEEP, sleep ? 0xff : 0);
> - goto out;
Removing this goto without taking the corresponding label with it, leaves
around a compile warning :)
-Aaron S.
[ snip ]
--
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
