[PATCH 2/2] serial/8250_pci: Need to clear FIFOs for KT serial on BI

[PATCH 2/2] serial/8250_pci: Need to clear FIFOs for KT serial on BI

[sudhakar]

From: Sudhakar Mamillapalli <sudhakar@fb.com>

When using SOL thru a KT serial device and Intel ME gets reset
the serial FIFOs need to be cleared for sane SOL output.  On
a reset the device assertes BI, so using that as a cue FIFOs
are cleared.  One other problem is that the serial registers
might temporarily go to 0 on reset for this device. So
instead of restoring IER register from read value in
poll_char and other functions we get the value from
uart_8250_port which should have the same value.

Signed-off-by: Sudhakar Mamillapalli <sudhakar@fb.com>
Acked-by: Dan Williams <dan.j.williams@intel.com>
Acked-by: Nhan H Mai <nhan.h.mai@intel.com>
---
 drivers/tty/serial/8250/8250.c     |   37 +++++++++++++++++++++++++++++++++--
 drivers/tty/serial/8250/8250_pci.c |    3 +-
 include/linux/serial_core.h        |    3 +-
 3 files changed, 38 insertions(+), 5 deletions(-)

diff --git a/drivers/tty/serial/8250/8250.c b/drivers/tty/serial/8250/8250.c
index 5fb0157..c61799e 100644
--- a/drivers/tty/serial/8250/8250.c
+++ b/drivers/tty/serial/8250/8250.c
@@ -282,6 +282,13 @@ static const struct serial8250_config uart_config[] = {
 		.fcr		= UART_FCR_ENABLE_FIFO | UART_FCR_R_TRIG_10,
 		.flags		= UART_CAP_FIFO | UART_CAP_AFE | UART_CAP_EFR,
 	},
+	[PORT_KT_SERIAL] = {
+		.name		= "KTSERIAL",
+		.fifo_size	= 16,
+		.tx_loadsz	= 16,
+		.fcr		= UART_FCR_ENABLE_FIFO | UART_FCR_R_TRIG_10,
+		.flags		= UART_CAP_FIFO,
+	},
 };
 
 #if defined(CONFIG_MIPS_ALCHEMY)
@@ -1363,6 +1370,7 @@ serial8250_rx_chars(struct uart_8250_port *up, unsigned char lsr)
 	struct uart_port *port = &up->port;
 	struct tty_struct *tty = port->state->port.tty;
 	unsigned char ch;
+	unsigned char fcr;
 	int max_count = 256;
 	char flag;
 
@@ -1385,6 +1393,16 @@ serial8250_rx_chars(struct uart_8250_port *up, unsigned char lsr)
 		lsr |= up->lsr_saved_flags;
 		up->lsr_saved_flags = 0;
 
+		if ((up->port.type == PORT_KT_SERIAL) && (lsr & UART_LSR_BI)) {
+			/*
+			 * For KT serial device if break interrupt then got
+			 * to clear the fifos for sane SOL output.
+			 */
+			serial8250_clear_fifos(up);
+			fcr = uart_config[up->port.type].fcr;
+			serial_port_out(port, UART_FCR, fcr);
+		}
+
 		if (unlikely(lsr & UART_LSR_BRK_ERROR_BITS)) {
 			/*
 			 * For statistics only
@@ -1729,7 +1747,12 @@ static void serial8250_backup_timeout(unsigned long data)
 	 * based handler.
 	 */
 	if (up->port.irq) {
-		ier = serial_in(up, UART_IER);
+		/*
+		 * Get the ier value from up->ier rather than reading the
+		 * register, since some SOL uarts(for e.g. KT serial) it
+		 * goes to 0 momentarily on BMC reset.
+		 */
+		ier = up->ier;
 		serial_out(up, UART_IER, 0);
 	}
 
@@ -1896,8 +1919,12 @@ static void serial8250_put_poll_char(struct uart_port *port,
 
 	/*
 	 *	First save the IER then disable the interrupts
+	 *
+	 *      Get the ier value from up->ier rather than reading the
+	 *      register, since some SOL uarts(for e.g. KT serial) it
+	 *      goes to 0 momentarily on BMC reset.
 	 */
-	ier = serial_port_in(port, UART_IER);
+	ier = up->ier;
 	if (up->capabilities & UART_CAP_UUE)
 		serial_port_out(port, UART_IER, UART_IER_UUE);
 	else
@@ -2818,8 +2845,12 @@ serial8250_console_write(struct console *co, const char *s, unsigned int count)
 
 	/*
 	 *	First save the IER then disable the interrupts
+	 *
+	 *      Get the ier value from up->ier rather than reading the
+	 *      register, since some SOL uarts(for e.g. KT serial) it
+	 *      goes to 0 momentarily on BMC reset.
 	 */
-	ier = serial_port_in(port, UART_IER);
+	ier = up->ier;
 
 	if (up->capabilities & UART_CAP_UUE)
 		serial_port_out(port, UART_IER, UART_IER_UUE);
diff --git a/drivers/tty/serial/8250/8250_pci.c b/drivers/tty/serial/8250/8250_pci.c
index 858dca8..1aebe63 100644
--- a/drivers/tty/serial/8250/8250_pci.c
+++ b/drivers/tty/serial/8250/8250_pci.c
@@ -1096,7 +1096,8 @@ static int kt_serial_setup(struct serial_private *priv,
 			   const struct pciserial_board *board,
 			   struct uart_port *port, int idx)
 {
-	port->flags |= UPF_BUG_THRE;
+	port->flags |= UPF_BUG_THRE | UPF_FIXED_TYPE;
+	port->type = PORT_KT_SERIAL;
 	return skip_tx_en_setup(priv, board, port, idx);
 }
 
diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index 2db407a..efd7d0d 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -47,7 +47,8 @@
 #define PORT_U6_16550A	19	/* ST-Ericsson U6xxx internal UART */
 #define PORT_TEGRA	20	/* NVIDIA Tegra internal UART */
 #define PORT_XR17D15X	21	/* Exar XR17D15x UART */
-#define PORT_MAX_8250	21	/* max port ID */
+#define PORT_KT_SERIAL	22	/* KT Serial SOL device */
+#define PORT_MAX_8250	22	/* max port ID */
 
 /*
  * ARM specific type numbers.  These are not currently guaranteed
-- 
1.7.8.4

Re: [PATCH 2/2] serial/8250_pci: Need to clear FIFOs for KT serial on BI

[Alan Cox]

On Tue, 3 Apr 2012 12:47:21 -0700
sudhakar <sudhakar@fb.com> wrote:

> 
> From: Sudhakar Mamillapalli <sudhakar@fb.com>
> 
> When using SOL thru a KT serial device and Intel ME gets reset
> the serial FIFOs need to be cleared for sane SOL output.  On

Acronym failure.

Please remember that people looking at a patch and even more so people in
future maintaining the code will not have any idea wtf you are talking
about !

Expand the acronyms in a patch and include a bit of context.


> @@ -1385,6 +1393,16 @@ serial8250_rx_chars(struct uart_8250_port *up, unsigned char lsr)
>  		lsr |= up->lsr_saved_flags;
>  		up->lsr_saved_flags = 0;
>  
> +		if ((up->port.type == PORT_KT_SERIAL) && (lsr & UART_LSR_BI)) {
> +			/*
> +			 * For KT serial device if break interrupt then got
> +			 * to clear the fifos for sane SOL output.
> +			 */
> +			serial8250_clear_fifos(up);
> +			fcr = uart_config[up->port.type].fcr;
> +			serial_port_out(port, UART_FCR, fcr);
> +		}
> +

This wants to be some kind of call back handled case not more stuff in
the core 8250.c which we are trying to drive all the special cases back
out of.

>  		if (unlikely(lsr & UART_LSR_BRK_ERROR_BITS)) {
>  			/*
>  			 * For statistics only
> @@ -1729,7 +1747,12 @@ static void serial8250_backup_timeout(unsigned long data)
>  	 * based handler.
>  	 */
>  	if (up->port.irq) {
> -		ier = serial_in(up, UART_IER);
> +		/*
> +		 * Get the ier value from up->ier rather than reading the
> +		 * register, since some SOL uarts(for e.g. KT serial) it
> +		 * goes to 0 momentarily on BMC reset.
> +		 */
> +		ier = up->ier;
>  		serial_out(up, UART_IER, 0);

Surely you do this fixup in your own private serial_in method as various
other chips do for all sorts of brain damage.

>  	}
>  
> @@ -1896,8 +1919,12 @@ static void serial8250_put_poll_char(struct uart_port *port,
>  
>  	/*
>  	 *	First save the IER then disable the interrupts
> +	 *
> +	 *      Get the ier value from up->ier rather than reading the
> +	 *      register, since some SOL uarts(for e.g. KT serial) it
> +	 *      goes to 0 momentarily on BMC reset.
>  	 */
> -	ier = serial_port_in(port, UART_IER);
> +	ier = up->ier;
>  	if (up->capabilities & UART_CAP_UUE)

Ditto

>  		serial_port_out(port, UART_IER, UART_IER_UUE);
>  	else
> @@ -2818,8 +2845,12 @@ serial8250_console_write(struct console *co, const char *s, unsigned int count)
>  
>  	/*
>  	 *	First save the IER then disable the interrupts
> +	 *
> +	 *      Get the ier value from up->ier rather than reading the
> +	 *      register, since some SOL uarts(for e.g. KT serial) it
> +	 *      goes to 0 momentarily on BMC reset.
>  	 */
> -	ier = serial_port_in(port, UART_IER);
> +	ier = up->ier;
>  

Ditto

In fact as far as I can see this boils down to

- a private serial_in method
- possibly adding a callback for special break handling. You may even be
  able to hide that in serial_in methods, but its probably better
  explicit.

Alan