[SERIAL] Tidy up 8250 port type detection

The original port detection code was one large function with many
tests without any clear structure.

This cset cleans this up, splitting out the tests for 8250/16450
vs 16550 vs 16550A vs later ports, and adds comments about why
we test what we're testing.

drivers/serial/8250.c

@@ -344,10 +344,9 @@ static int size_fifo(struct uart_8250_port *up)
  * 
  * What evil have men's minds wrought...
  */
-static void
-autoconfig_startech_uarts(struct uart_8250_port *up)
+static void autoconfig_has_efr(struct uart_8250_port *up)
 {
-	unsigned char scratch, scratch2, scratch3, scratch4;
+	unsigned char id1, id2, id3, rev, saved_dll, saved_dlm;
 
 	/*
 	 * First we check to see if it's an Oxford Semiconductor UART.
@@ -356,31 +355,32 @@ autoconfig_startech_uarts(struct uart_8250_port *up)
 	 * Semiconductor clone chips lock up if you try writing to the
 	 * LSR register (which serial_icr_read does)
 	 */
-	if (up->port.type == PORT_16550A) {
-		/*
-		 * EFR [4] must be set else this test fails
-		 *
-		 * This shouldn't be necessary, but Mike Hudson
-		 * (Exoray@isys.ca) claims that it's needed for 952
-		 * dual UART's (which are not recommended for new designs).
-		 */
-		up->acr = 0;
-		serial_out(up, UART_LCR, 0xBF);
-		serial_out(up, UART_EFR, 0x10);
-		serial_out(up, UART_LCR, 0x00);
-		/* Check for Oxford Semiconductor 16C950 */
-		scratch = serial_icr_read(up, UART_ID1);
-		scratch2 = serial_icr_read(up, UART_ID2);
-		scratch3 = serial_icr_read(up, UART_ID3);
-		
-		if (scratch == 0x16 && scratch2 == 0xC9 &&
-		    (scratch3 == 0x50 || scratch3 == 0x52 ||
-		     scratch3 == 0x54)) {
-			up->port.type = PORT_16C950;
-			up->rev = serial_icr_read(up, UART_REV) |
-						  (scratch3 << 8);
-			return;
-		}
+
+	/*
+	 * Check for Oxford Semiconductor 16C950.
+	 *
+	 * EFR [4] must be set else this test fails.
+	 *
+	 * This shouldn't be necessary, but Mike Hudson (Exoray@isys.ca)
+	 * claims that it's needed for 952 dual UART's (which are not
+	 * recommended for new designs).
+	 */
+	up->acr = 0;
+	serial_out(up, UART_LCR, 0xBF);
+	serial_out(up, UART_EFR, UART_EFR_ECB);
+	serial_out(up, UART_LCR, 0x00);
+	id1 = serial_icr_read(up, UART_ID1);
+	id2 = serial_icr_read(up, UART_ID2);
+	id3 = serial_icr_read(up, UART_ID3);
+	rev = serial_icr_read(up, UART_REV);
+
+	DEBUG_AUTOCONF("950id=%02x:%02x:%02x:%02x ", id1, id2, id3, rev);
+
+	if (id1 == 0x16 && id2 == 0xC9 &&
+	    (id3 == 0x50 || id3 == 0x52 || id3 == 0x54)) {
+		up->port.type = PORT_16C950;
+		up->rev = rev | (scratch3 << 8);
+		return;
 	}
 	
 	/*
@@ -391,34 +391,28 @@ autoconfig_startech_uarts(struct uart_8250_port *up)
 	 *  0x12 - XR16C2850.
 	 *  0x14 - XR16C854.
 	 */
-
-	/* Save the DLL and DLM */
-
 	serial_outp(up, UART_LCR, UART_LCR_DLAB);
-	scratch3 = serial_inp(up, UART_DLL);
-	scratch4 = serial_inp(up, UART_DLM);
-
+	saved_dll = serial_inp(up, UART_DLL);
+	saved_dlm = serial_inp(up, UART_DLM);
 	serial_outp(up, UART_DLL, 0);
 	serial_outp(up, UART_DLM, 0);
-	scratch2 = serial_inp(up, UART_DLL);
-	scratch = serial_inp(up, UART_DLM);
-	serial_outp(up, UART_LCR, 0);
+	id2 = serial_inp(up, UART_DLL);
+	id1 = serial_inp(up, UART_DLM);
+	serial_outp(up, UART_DLL, saved_dll);
+	serial_outp(up, UART_DLM, saved_dlm);
+
+	DEBUG_AUTOCONF("850id=%02x:%02x ", id1, id2);
 
-	if (scratch == 0x10 || scratch == 0x12 || scratch == 0x14) {
-		if (scratch == 0x10)
-			up->rev = scratch2;
+	if (id1 == 0x10 || id1 == 0x12 || id1 == 0x14) {
+		if (id1 == 0x10)
+			up->rev = id2;
 		up->port.type = PORT_16850;
 		return;
 	}
 
-	/* Restore the DLL and DLM */
-
-	serial_outp(up, UART_LCR, UART_LCR_DLAB);
-	serial_outp(up, UART_DLL, scratch3);
-	serial_outp(up, UART_DLM, scratch4);
-	serial_outp(up, UART_LCR, 0);
-
 	/*
+	 * It wasn't an XR16C850.
+	 *
 	 * We distinguish between the '654 and the '650 by counting
 	 * how many bytes are in the FIFO.  I'm using this for now,
 	 * since that's the technique that was sent to me in the
@@ -431,6 +425,85 @@ autoconfig_startech_uarts(struct uart_8250_port *up)
 		up->port.type = PORT_16650V2;
 }
 
+/*
+ * We detected a chip without a FIFO.  Only two fall into
+ * this category - the original 8250 and the 16450.  The
+ * 16450 has a scratch register (accessible with LCR=0)
+ */
+static void autoconfig_8250(struct uart_8250_port *up)
+{
+	unsigned char scratch, status1, status2;
+
+	up->port.type = PORT_8250;
+
+	scratch = serial_in(up, UART_SCR);
+	serial_outp(up, UART_SCR, 0xa5);
+	status1 = serial_in(up, UART_SCR);
+	serial_outp(up, UART_SCR, 0x5a);
+	status2 = serial_in(up, UART_SCR);
+	serial_outp(up, UART_SCR, scratch);
+
+	if (status1 == 0xa5 && status2 == 0x5a)
+		up->port.type = PORT_16450;
+}
+
+/*
+ * We know that the chip has FIFOs.  Does it have an EFR?  The
+ * EFR is located in the same register position as the IIR and
+ * we know the top two bits of the IIR are currently set.  The
+ * EFR should contain zero.  Try to read the EFR.
+ */
+static void autoconfig_16550a(struct uart_8250_port *up)
+{
+	unsigned char status1, status2;
+
+	up->port.type = PORT_16550A;
+
+	/*
+	 * Check for presence of the EFR when DLAB is set.
+	 * Only ST16C650V1 UARTs pass this test.
+	 */
+	serial_outp(up, UART_LCR, UART_LCR_DLAB);
+	if (serial_in(up, UART_EFR) == 0) {
+		DEBUG_AUTOCONF("EFRv1 ");
+		up->port.type = PORT_16650;
+		return;
+	}
+
+	/*
+	 * Maybe it requires 0xbf to be written to the LCR.
+	 * (other ST16C650V2 UARTs, TI16C752A, etc)
+	 */
+	serial_outp(up, UART_LCR, 0xBF);
+	if (serial_in(up, UART_EFR) == 0) {
+		DEBUG_AUTOCONF("EFRv2 ");
+		autoconfig_has_efr(up);
+		return;
+	}
+
+	/*
+	 * No EFR.  Try to detect a TI16750, which only sets bit 5 of
+	 * the IIR when 64 byte FIFO mode is enabled when DLAB is set.
+	 * Try setting it with and without DLAB set.  Cheap clones
+	 * set bit 5 without DLAB set.
+	 */
+	serial_outp(up, UART_LCR, 0);
+	serial_outp(up, UART_FCR, UART_FCR_ENABLE_FIFO | UART_FCR7_64BYTE);
+	status1 = serial_in(up, UART_IIR) >> 5;
+	serial_outp(up, UART_FCR, UART_FCR_ENABLE_FIFO);
+	serial_outp(up, UART_LCR, UART_LCR_DLAB);
+	serial_outp(up, UART_FCR, UART_FCR_ENABLE_FIFO | UART_FCR7_64BYTE);
+	status2 = serial_in(up, UART_IIR) >> 5;
+	serial_outp(up, UART_FCR, UART_FCR_ENABLE_FIFO);
+
+	DEBUG_AUTOCONF("iir1=%d iir2=%d ", status1, status2);
+
+	if (status1 == 6 && status2 == 7) {
+		up->port.type = PORT_16750;
+		return;
+	}
+}
+
 /*
  * This routine is called by rs_init() to initialize a specific serial
  * port.  It determines what type of UART chip this serial port is
@@ -440,16 +513,16 @@ autoconfig_startech_uarts(struct uart_8250_port *up)
  */
 static void autoconfig(struct uart_8250_port *up, unsigned int probeflags)
 {
-	unsigned char status1, status2, scratch, scratch2, scratch3;
+	unsigned char status1, scratch, scratch2, scratch3;
 	unsigned char save_lcr, save_mcr;
 	unsigned long flags;
 
-	DEBUG_AUTOCONF("Testing ttyS%d (0x%04x, 0x%08lx)...\n",
-			up->port.line, up->port.iobase, up->port.membase);
-
 	if (!up->port.iobase && !up->port.membase)
 		return;
 
+	DEBUG_AUTOCONF("ttyS%d: autoconf (0x%04x, 0x%08lx): ",
+			up->port.line, up->port.iobase, up->port.membase);
+
 	/*
 	 * We really do need global IRQs disabled here - we're going to
 	 * be frobbing the chips IRQ enable register to see if it exists.
@@ -467,6 +540,9 @@ static void autoconfig(struct uart_8250_port *up, unsigned int probeflags)
 		 * assumption is that 0x80 is a non-existent port;
 		 * which should be safe since include/asm/io.h also
 		 * makes this assumption.
+		 *
+		 * Note: this is safe as long as MCR bit 4 is clear
+		 * and the device is in "PC" mode.
 		 */
 		scratch = serial_inp(up, UART_IER);
 		serial_outp(up, UART_IER, 0);
@@ -484,9 +560,8 @@ static void autoconfig(struct uart_8250_port *up, unsigned int probeflags)
 			/*
 			 * We failed; there's nothing here
 			 */
-			DEBUG_AUTOCONF("serial: ttyS%d: simple autoconfig "
-				       "failed (%02x, %02x)\n",
-				       up->port.line, scratch2, scratch3);
+			DEBUG_AUTOCONF("IER test failed (%02x, %02x) ",
+				       scratch2, scratch3);
 			goto out;
 		}
 	}
@@ -508,64 +583,45 @@ static void autoconfig(struct uart_8250_port *up, unsigned int probeflags)
 		status1 = serial_inp(up, UART_MSR) & 0xF0;
 		serial_outp(up, UART_MCR, save_mcr);
 		if (status1 != 0x90) {
-			DEBUG_AUTOCONF("serial: ttyS%d: no UART loopback "
-				       "failed\n", up->port.line);
+			DEBUG_AUTOCONF("LOOP test failed (%02x) ",
+				       status1);
 			goto out;
 		}
 	}
-	serial_outp(up, UART_LCR, 0xBF); /* set up for StarTech test */
-	serial_outp(up, UART_EFR, 0);	/* EFR is the same as FCR */
+
+	/*
+	 * We're pretty sure there's a port here.  Lets find out what
+	 * type of port it is.  The IIR top two bits allows us to find
+	 * out if its 8250 or 16450, 16550, 16550A or later.  This
+	 * determines what we test for next.
+	 *
+	 * We also initialise the EFR (if any) to zero for later.  The
+	 * EFR occupies the same register location as the FCR and IIR.
+	 */
+	serial_outp(up, UART_LCR, 0xBF);
+	serial_outp(up, UART_EFR, 0);
 	serial_outp(up, UART_LCR, 0);
+
 	serial_outp(up, UART_FCR, UART_FCR_ENABLE_FIFO);
 	scratch = serial_in(up, UART_IIR) >> 6;
+
+	DEBUG_AUTOCONF("iir=%d ", scratch);
+
 	switch (scratch) {
-		case 0:
-			up->port.type = PORT_16450;
-			break;
-		case 1:
-			up->port.type = PORT_UNKNOWN;
-			break;
-		case 2:
-			up->port.type = PORT_16550;
-			break;
-		case 3:
-			up->port.type = PORT_16550A;
-			break;
-	}
-	if (up->port.type == PORT_16550A) {
-		/* Check for Startech UART's */
-		serial_outp(up, UART_LCR, UART_LCR_DLAB);
-		if (serial_in(up, UART_EFR) == 0) {
-			up->port.type = PORT_16650;
-		} else {
-			serial_outp(up, UART_LCR, 0xBF);
-			if (serial_in(up, UART_EFR) == 0)
-				autoconfig_startech_uarts(up);
-		}
-	}
-	if (up->port.type == PORT_16550A) {
-		/* Check for TI 16750 */
-		serial_outp(up, UART_LCR, save_lcr | UART_LCR_DLAB);
-		serial_outp(up, UART_FCR,
-			    UART_FCR_ENABLE_FIFO | UART_FCR7_64BYTE);
-		scratch = serial_in(up, UART_IIR) >> 5;
-		if (scratch == 7) {
-			/*
-			 * If this is a 16750, and not a cheap UART
-			 * clone, then it should only go into 64 byte
-			 * mode if the UART_FCR7_64BYTE bit was set
-			 * while UART_LCR_DLAB was latched.
-			 */
- 			serial_outp(up, UART_FCR, UART_FCR_ENABLE_FIFO);
-			serial_outp(up, UART_LCR, 0);
-			serial_outp(up, UART_FCR,
-				    UART_FCR_ENABLE_FIFO | UART_FCR7_64BYTE);
-			scratch = serial_in(up, UART_IIR) >> 5;
-			if (scratch == 6)
-				up->port.type = PORT_16750;
-		}
-		serial_outp(up, UART_FCR, UART_FCR_ENABLE_FIFO);
+	case 0:
+		autoconfig_8250(up);
+		break;
+	case 1:
+		up->port.type = PORT_UNKNOWN;
+		break;
+	case 2:
+		up->port.type = PORT_16550;
+		break;
+	case 3:
+		autoconfig_16550a(up);
+		break;
 	}
+
 #if defined(CONFIG_SERIAL_8250_RSA) && defined(MODULE)
 	/*
 	 * Only probe for RSA ports if we got the region.
@@ -588,17 +644,6 @@ static void autoconfig(struct uart_8250_port *up, unsigned int probeflags)
 	}
 #endif
 	serial_outp(up, UART_LCR, save_lcr);
-	if (up->port.type == PORT_16450) {
-		scratch = serial_in(up, UART_SCR);
-		serial_outp(up, UART_SCR, 0xa5);
-		status1 = serial_in(up, UART_SCR);
-		serial_outp(up, UART_SCR, 0x5a);
-		status2 = serial_in(up, UART_SCR);
-		serial_outp(up, UART_SCR, scratch);
-
-		if ((status1 != 0xa5) || (status2 != 0x5a))
-			up->port.type = PORT_8250;
-	}
 
 	up->port.fifosize = uart_config[up->port.type].dfl_xmit_fifo_size;
 
@@ -630,6 +675,7 @@ static void autoconfig(struct uart_8250_port *up, unsigned int probeflags)
 			       "serial_rsa");
 	}
 #endif
+	DEBUG_AUTOCONF("type=%s\n", uart_config[up->port.type].name);
 }
 
 static void autoconfig_irq(struct uart_8250_port *up)