Re: [PATCH] tty/serial/8250: make UART_MCR register access consistent

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

 



Hi Yegor,

On 03/11/2016 05:46 AM, yegorslists@xxxxxxxxxxxxxx wrote:
> From: Yegor Yefremov <yegorslists@xxxxxxxxxxxxxx>
> 
> Introduce serial8250_out_MCR() and serial8250_in_MCR() routines, that
> replace following calls:
> 
> serial_out(port, UART_MCR, val)
> serial_port_out(up, UART_MCR, val)
> serial_in(port, UART_MCR)

I assume that this patch would be the first in a series that adds
mctrl gpio support to the 8250 driver (specifically to omap).

A couple of comments.
1) I think the commit log should note the purpose of the abstraction;
   ie, to enable gpio modem control lines.
2) You'll want to rebase on top of Greg's tty-next branch of his tty tree
   git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty.git

Regards,
Peter Hurley


> CC: Peter Hurley <peter@xxxxxxxxxxxxxxxxxx>
> Signed-off-by: Yegor Yefremov <yegorslists@xxxxxxxxxxxxxx>
> ---
>  drivers/tty/serial/8250/8250.h      | 10 ++++++++++
>  drivers/tty/serial/8250/8250_omap.c |  4 ++--
>  drivers/tty/serial/8250/8250_port.c | 39 ++++++++++++++++++-------------------
>  3 files changed, 31 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/tty/serial/8250/8250.h b/drivers/tty/serial/8250/8250.h
> index d54dcd8..5469bef 100644
> --- a/drivers/tty/serial/8250/8250.h
> +++ b/drivers/tty/serial/8250/8250.h
> @@ -118,6 +118,16 @@ struct uart_8250_port *serial8250_get_port(int line);
>  void serial8250_rpm_get(struct uart_8250_port *p);
>  void serial8250_rpm_put(struct uart_8250_port *p);
>  
> +static inline void serial8250_out_MCR(struct uart_8250_port *up, int value)
> +{
> +	serial_out(up, UART_MCR, value);
> +}
> +
> +static inline int serial8250_in_MCR(struct uart_8250_port *up)
> +{
> +	return serial_in(up, UART_MCR);
> +}
> +
>  #if defined(__alpha__) && !defined(CONFIG_PCI)
>  /*
>   * Digital did something really horribly wrong with the OUT1 and OUT2
> diff --git a/drivers/tty/serial/8250/8250_omap.c b/drivers/tty/serial/8250/8250_omap.c
> index a2c0734..bb431e3 100644
> --- a/drivers/tty/serial/8250/8250_omap.c
> +++ b/drivers/tty/serial/8250/8250_omap.c
> @@ -274,7 +274,7 @@ static void omap8250_restore_regs(struct uart_8250_port *up)
>  	serial_out(up, UART_EFR, UART_EFR_ECB);
>  
>  	serial_out(up, UART_LCR, UART_LCR_CONF_MODE_A);
> -	serial_out(up, UART_MCR, UART_MCR_TCRTLR);
> +	serial8250_out_MCR(up, UART_MCR_TCRTLR);
>  	serial_out(up, UART_FCR, up->fcr);
>  
>  	omap8250_update_scr(up, priv);
> @@ -290,7 +290,7 @@ static void omap8250_restore_regs(struct uart_8250_port *up)
>  	serial_out(up, UART_LCR, 0);
>  
>  	/* drop TCR + TLR access, we setup XON/XOFF later */
> -	serial_out(up, UART_MCR, up->mcr);
> +	serial8250_out_MCR(up, up->mcr);
>  	serial_out(up, UART_IER, up->ier);
>  
>  	serial_out(up, UART_LCR, UART_LCR_CONF_MODE_B);
> diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
> index 8d262bc..ad87495c 100644
> --- a/drivers/tty/serial/8250/8250_port.c
> +++ b/drivers/tty/serial/8250/8250_port.c
> @@ -701,10 +701,10 @@ static int size_fifo(struct uart_8250_port *up)
>  	old_lcr = serial_in(up, UART_LCR);
>  	serial_out(up, UART_LCR, 0);
>  	old_fcr = serial_in(up, UART_FCR);
> -	old_mcr = serial_in(up, UART_MCR);
> +	old_mcr = serial8250_in_MCR(up);
>  	serial_out(up, UART_FCR, UART_FCR_ENABLE_FIFO |
>  		    UART_FCR_CLEAR_RCVR | UART_FCR_CLEAR_XMIT);
> -	serial_out(up, UART_MCR, UART_MCR_LOOP);
> +	serial8250_out_MCR(up, UART_MCR_LOOP);
>  	serial_out(up, UART_LCR, UART_LCR_CONF_MODE_A);
>  	old_dl = serial_dl_read(up);
>  	serial_dl_write(up, 0x0001);
> @@ -716,7 +716,7 @@ static int size_fifo(struct uart_8250_port *up)
>  	     (count < 256); count++)
>  		serial_in(up, UART_RX);
>  	serial_out(up, UART_FCR, old_fcr);
> -	serial_out(up, UART_MCR, old_mcr);
> +	serial8250_out_MCR(up, old_mcr);
>  	serial_out(up, UART_LCR, UART_LCR_CONF_MODE_A);
>  	serial_dl_write(up, old_dl);
>  	serial_out(up, UART_LCR, old_lcr);
> @@ -962,17 +962,17 @@ static void autoconfig_16550a(struct uart_8250_port *up)
>  	 * it's changed. If so, set baud_base in EXCR2 to 921600. -- dwmw2
>  	 */
>  	serial_out(up, UART_LCR, 0);
> -	status1 = serial_in(up, UART_MCR);
> +	status1 = serial8250_in_MCR(up);
>  	serial_out(up, UART_LCR, 0xE0);
>  	status2 = serial_in(up, 0x02); /* EXCR1 */
>  
>  	if (!((status2 ^ status1) & UART_MCR_LOOP)) {
>  		serial_out(up, UART_LCR, 0);
> -		serial_out(up, UART_MCR, status1 ^ UART_MCR_LOOP);
> +		serial8250_out_MCR(up, status1 ^ UART_MCR_LOOP);
>  		serial_out(up, UART_LCR, 0xE0);
>  		status2 = serial_in(up, 0x02); /* EXCR1 */
>  		serial_out(up, UART_LCR, 0);
> -		serial_out(up, UART_MCR, status1);
> +		serial8250_out_MCR(up, status1);
>  
>  		if ((status2 ^ status1) & UART_MCR_LOOP) {
>  			unsigned short quot;
> @@ -1146,7 +1146,7 @@ static void autoconfig(struct uart_8250_port *up)
>  		}
>  	}
>  
> -	save_mcr = serial_in(up, UART_MCR);
> +	save_mcr = serial8250_in_MCR(up);
>  	save_lcr = serial_in(up, UART_LCR);
>  
>  	/*
> @@ -1159,9 +1159,9 @@ static void autoconfig(struct uart_8250_port *up)
>  	 * that conflicts with COM 1-4 --- we hope!
>  	 */
>  	if (!(port->flags & UPF_SKIP_TEST)) {
> -		serial_out(up, UART_MCR, UART_MCR_LOOP | 0x0A);
> +		serial8250_out_MCR(up, UART_MCR_LOOP | 0x0A);
>  		status1 = serial_in(up, UART_MSR) & 0xF0;
> -		serial_out(up, UART_MCR, save_mcr);
> +		serial8250_out_MCR(up, save_mcr);
>  		if (status1 != 0x90) {
>  			spin_unlock_irqrestore(&port->lock, flags);
>  			DEBUG_AUTOCONF("LOOP test failed (%02x) ",
> @@ -1227,7 +1227,7 @@ static void autoconfig(struct uart_8250_port *up)
>  	if (port->type == PORT_RSA)
>  		serial_out(up, UART_RSA_FRR, 0);
>  #endif
> -	serial_out(up, UART_MCR, save_mcr);
> +	serial8250_out_MCR(up, save_mcr);
>  	serial8250_clear_fifos(up);
>  	serial_in(up, UART_RX);
>  	if (up->capabilities & UART_CAP_UUE)
> @@ -1269,19 +1269,18 @@ static void autoconfig_irq(struct uart_8250_port *up)
>  
>  	/* forget possible initially masked and pending IRQ */
>  	probe_irq_off(probe_irq_on());
> -	save_mcr = serial_in(up, UART_MCR);
> +	save_mcr = serial8250_in_MCR(up);
>  	save_ier = serial_in(up, UART_IER);
> -	serial_out(up, UART_MCR, UART_MCR_OUT1 | UART_MCR_OUT2);
> +	serial8250_out_MCR(up, UART_MCR_OUT1 | UART_MCR_OUT2);
>  
>  	irqs = probe_irq_on();
> -	serial_out(up, UART_MCR, 0);
> +	serial8250_out_MCR(up, 0);
>  	udelay(10);
>  	if (port->flags & UPF_FOURPORT) {
> -		serial_out(up, UART_MCR,
> -			    UART_MCR_DTR | UART_MCR_RTS);
> +		serial8250_out_MCR(up, UART_MCR_DTR | UART_MCR_RTS);
>  	} else {
> -		serial_out(up, UART_MCR,
> -			    UART_MCR_DTR | UART_MCR_RTS | UART_MCR_OUT2);
> +		serial8250_out_MCR(up,
> +			UART_MCR_DTR | UART_MCR_RTS | UART_MCR_OUT2);
>  	}
>  	serial_out(up, UART_IER, 0x0f);	/* enable all intrs */
>  	serial_in(up, UART_LSR);
> @@ -1292,7 +1291,7 @@ static void autoconfig_irq(struct uart_8250_port *up)
>  	udelay(20);
>  	irq = probe_irq_off(irqs);
>  
> -	serial_out(up, UART_MCR, save_mcr);
> +	serial8250_out_MCR(up, save_mcr);
>  	serial_out(up, UART_IER, save_ier);
>  
>  	if (port->flags & UPF_FOURPORT)
> @@ -1701,7 +1700,7 @@ void serial8250_do_set_mctrl(struct uart_port *port, unsigned int mctrl)
>  
>  	mcr = (mcr & up->mcr_mask) | up->mcr_force | up->mcr;
>  
> -	serial_port_out(port, UART_MCR, mcr);
> +	serial8250_out_MCR(up, mcr);
>  }
>  EXPORT_SYMBOL_GPL(serial8250_do_set_mctrl);
>  
> @@ -2848,7 +2847,7 @@ static void serial8250_console_restore(struct uart_8250_port *up)
>  
>  	serial8250_set_divisor(port, baud, quot, frac);
>  	serial_port_out(port, UART_LCR, up->lcr);
> -	serial_port_out(port, UART_MCR, UART_MCR_DTR | UART_MCR_RTS);
> +	serial8250_out_MCR(up, UART_MCR_DTR | UART_MCR_RTS);
>  }
>  
>  /*
> 

--
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