Re: [RFC, wip][PATCH v1] serial: 8250_exar: Move the Exar bits from 8250_port

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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?

In return, I'll finally send the patches to enable our device's EXAR
with upstream. They seem ready now.

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);
> +	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;
>  }
> 

-- 
Siemens AG, Corporate Technology, CT RDA ITP SES-DE
Corporate Competence Center Embedded Linux
--
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



[Index of Archives]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux PPP]     [Linux FS]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Linmodem]     [Device Mapper]     [Linux Kernel for ARM]

  Powered by Linux