[PATCH] Make the Startech UART detection 'more correct'.

public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed

* [PATCH] Make the Startech UART detection 'more correct'.
@ 2003-09-08 20:54 Tom Rini
  2003-09-09 16:18 ` Russell King
  0 siblings, 1 reply; 6+ messages in thread
From: Tom Rini @ 2003-09-08 20:54 UTC (permalink / raw)
  To: Russell King, tytso; +Cc: Kernel Mailing List, linux-serial, gallen

[-- Attachment #1: Type: text/plain, Size: 2019 bytes --]

Hello.  The following patches (vs 2.4 and 2.6) make the Startech UART
detection 'more correct'  The problem is that on with the Motorola
MPC82xx line (8245 for example) it has an internal DUART that it claims
to be PC16550D compatible, and it has an additional EFR (Enhanced
Feature Register) at offset 0x2, like on the Startech UARTS.  However,
it is not a Startech, and when it's detected as such, FIFOs don't work.
The fix for this is that the Startech UARTs have a 32 byte FIFO [1] and
the MPC82xx DUARTs have a 16-byte FIFO [2], to check that the FIFO size
is correct for a Startech.

2.4:
===== drivers/char/serial.c 1.34 vs edited =====
--- 1.34/drivers/char/serial.c	Sun Jul  6 22:33:28 2003
+++ edited/drivers/char/serial.c	Fri Sep  5 15:44:11 2003
@@ -3740,7 +3740,7 @@
 	if (state->type == PORT_16550A) {
 		/* Check for Startech UART's */
 		serial_outp(info, UART_LCR, UART_LCR_DLAB);
-		if (serial_in(info, UART_EFR) == 0) {
+		if (serial_in(info, UART_EFR) == 0 && size_fifo(info) != 16) {
 			state->type = PORT_16650;
 		} else {
 			serial_outp(info, UART_LCR, 0xBF);

2.6:
===== drivers/serial/8250.c 1.34 vs edited =====
--- 1.34/drivers/serial/8250.c	Sun Jun 15 02:21:11 2003
+++ edited/drivers/serial/8250.c	Fri Sep  5 15:43:23 2003
@@ -467,7 +467,7 @@
 	 * Only ST16C650V1 UARTs pass this test.
 	 */
 	serial_outp(up, UART_LCR, UART_LCR_DLAB);
-	if (serial_in(up, UART_EFR) == 0) {
+	if (serial_in(up, UART_EFR) == 0 && size_fifo(up) != 16) {
 		DEBUG_AUTOCONF("EFRv1 ");
 		up->port.type = PORT_16650;
 		return;

I must admit however, that after reading both datasheets, I'm not sure
if the correct test is for && size_fifo() == 32 or != 16, or if it
matters.  Comments?

This was originally fixed by Greg Allen[3].

[1]: http://www.exar.com/products/uartnote.pdf
[2]: http://e-www.motorola.com/brdata/PDFDB/docs/MPC8245UM_CH12.pdf
[3]: http://www.geocrawler.com/archives/3/8358/2002/3/0/8228902/
-- 
Tom Rini
http://gate.crashing.org/~trini/

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Make the Startech UART detection 'more correct'.
  2003-09-08 20:54 [PATCH] Make the Startech UART detection 'more correct' Tom Rini
@ 2003-09-09 16:18 ` Russell King
  2003-09-09 19:12   ` Stuart MacDonald
  2003-09-09 23:51   ` Tom Rini
  0 siblings, 2 replies; 6+ messages in thread
From: Russell King @ 2003-09-09 16:18 UTC (permalink / raw)
  To: Tom Rini; +Cc: tytso, Kernel Mailing List, linux-serial, gallen

On Mon, Sep 08, 2003 at 01:54:31PM -0700, Tom Rini wrote:
> Hello.  The following patches (vs 2.4 and 2.6) make the Startech UART
> detection 'more correct'  The problem is that on with the Motorola
> MPC82xx line (8245 for example) it has an internal DUART that it claims
> to be PC16550D compatible, and it has an additional EFR (Enhanced
> Feature Register) at offset 0x2, like on the Startech UARTS.  However,
> it is not a Startech, and when it's detected as such, FIFOs don't work.
> The fix for this is that the Startech UARTs have a 32 byte FIFO [1] and
> the MPC82xx DUARTs have a 16-byte FIFO [2], to check that the FIFO size
> is correct for a Startech.

size_fifo() is claimed to be unreliable at detecting the FIFO size,
so I don't feel safe about using it here.

I'd suggest something like:

	serial_outp(port, UART_LCR, UART_LCR_DLAB);
	efr = serial_in(port, UART_EFR);
	if ((efr & 0xfc) == 0) {
		serial_out(port, UART_EFR, 0xac | (efr & 3));
		/* if top 6 bits return zero, its motorola */
		if (serial_in(port, UART_EFR) == (efr & 3)) {
			/* motorola port */
		} else {
			/* ST16C650V1 port */
		}
		/* restore old value */
		serial_outb(port, UART_EFR, efr);
	}

If you can guarantee that the lower two bits will always be zero, you can
drop the frobbing to ignore/preseve the lower two bits.

-- 
Russell King (rmk@arm.linux.org.uk)	http://www.arm.linux.org.uk/personal/
Linux kernel maintainer of:
  2.6 ARM Linux   - http://www.arm.linux.org.uk/
  2.6 PCMCIA      - http://pcmcia.arm.linux.org.uk/
  2.6 Serial core

^ permalink raw reply	[flat|nested] 6+ messages in thread

* RE: [PATCH] Make the Startech UART detection 'more correct'.
  2003-09-09 16:18 ` Russell King
@ 2003-09-09 19:12   ` Stuart MacDonald
  2003-09-09 19:23     ` Tom Rini
  2003-09-09 23:51   ` Tom Rini
  1 sibling, 1 reply; 6+ messages in thread
From: Stuart MacDonald @ 2003-09-09 19:12 UTC (permalink / raw)
  To: 'Russell King', 'Tom Rini'
  Cc: tytso, 'Kernel Mailing List', linux-serial, gallen

From: linux-kernel-owner@vger.kernel.org 
> size_fifo() is claimed to be unreliable at detecting the FIFO size,
> so I don't feel safe about using it here.

"Claimed" being the operative word. This claim predates my involvement
with the serial driver (IIRC tytso is reporting someone's claim
second-hand), however, my company has always used this test in various
test apps, and never had it fail, and this predates my employment with
them.

So. A number of possibilities: someone thought it was unreliable, but
they were incorrect; it was unreliable, but because the software was
wrong; it was unreliable, but only on certain very old hardware that I
personally have no knowledge of its existance.

I've never seen any concrete details about what goes wrong either,
just a simple claim of unreliability.

The premise is simple enough. Put the uart into internal loopback mode
(the rx and tx pins on the chip become on-die connected to each other,
date never leaves the chip), transmit 256 characters at a known baud
rate, wait for at least the time it takes to transmit these characters
plus some to be safe, and see how many are left in the rx fifo, which
should be full.

> I'd suggest something like:
> 
> 	serial_outp(port, UART_LCR, UART_LCR_DLAB);
> 	efr = serial_in(port, UART_EFR);
> 	if ((efr & 0xfc) == 0) {
> 		serial_out(port, UART_EFR, 0xac | (efr & 3));
> 		/* if top 6 bits return zero, its motorola */
> 		if (serial_in(port, UART_EFR) == (efr & 3)) {
> 			/* motorola port */
> 		} else {
> 			/* ST16C650V1 port */
> 		}
> 		/* restore old value */
> 		serial_outb(port, UART_EFR, efr);
> 	}
> 
> If you can guarantee that the lower two bits will always be 
> zero, you can
> drop the frobbing to ignore/preseve the lower two bits.

Does the Motorola chip have an ID register at all? Certainly using the
fifo size is a weak test and should only be a last resort.

..Stu

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Make the Startech UART detection 'more correct'.
  2003-09-09 19:12   ` Stuart MacDonald
@ 2003-09-09 19:23     ` Tom Rini
  0 siblings, 0 replies; 6+ messages in thread
From: Tom Rini @ 2003-09-09 19:23 UTC (permalink / raw)
  To: Stuart MacDonald
  Cc: 'Russell King', tytso, 'Kernel Mailing List',
	linux-serial, gallen

[-- Attachment #1: Type: text/plain, Size: 994 bytes --]

On Tue, Sep 09, 2003 at 03:12:30PM -0400, Stuart MacDonald wrote:
> From: linux-kernel-owner@vger.kernel.org 
[snip]
> > I'd suggest something like:
> > 
> > 	serial_outp(port, UART_LCR, UART_LCR_DLAB);
> > 	efr = serial_in(port, UART_EFR);
> > 	if ((efr & 0xfc) == 0) {
> > 		serial_out(port, UART_EFR, 0xac | (efr & 3));
> > 		/* if top 6 bits return zero, its motorola */
> > 		if (serial_in(port, UART_EFR) == (efr & 3)) {
> > 			/* motorola port */
> > 		} else {
> > 			/* ST16C650V1 port */
> > 		}
> > 		/* restore old value */
> > 		serial_outb(port, UART_EFR, efr);
> > 	}
> > 
> > If you can guarantee that the lower two bits will always be 
> > zero, you can
> > drop the frobbing to ignore/preseve the lower two bits.
> 
> Does the Motorola chip have an ID register at all? Certainly using the
> fifo size is a weak test and should only be a last resort.

No, there is not an ID on the motorola duarts.

-- 
Tom Rini
http://gate.crashing.org/~trini/

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Make the Startech UART detection 'more correct'.
  2003-09-09 16:18 ` Russell King
  2003-09-09 19:12   ` Stuart MacDonald
@ 2003-09-09 23:51   ` Tom Rini
  2003-09-24 22:40     ` Kumar Gala
  1 sibling, 1 reply; 6+ messages in thread
From: Tom Rini @ 2003-09-09 23:51 UTC (permalink / raw)
  To: tytso, Kernel Mailing List, linux-serial, gallen; +Cc: Kumar Gala

[-- Attachment #1: Type: text/plain, Size: 2691 bytes --]

On Tue, Sep 09, 2003 at 05:18:59PM +0100, Russell King wrote:

> On Mon, Sep 08, 2003 at 01:54:31PM -0700, Tom Rini wrote:
> > Hello.  The following patches (vs 2.4 and 2.6) make the Startech UART
> > detection 'more correct'  The problem is that on with the Motorola
> > MPC82xx line (8245 for example) it has an internal DUART that it claims
> > to be PC16550D compatible, and it has an additional EFR (Enhanced
> > Feature Register) at offset 0x2, like on the Startech UARTS.  However,
> > it is not a Startech, and when it's detected as such, FIFOs don't work.
> > The fix for this is that the Startech UARTs have a 32 byte FIFO [1] and
> > the MPC82xx DUARTs have a 16-byte FIFO [2], to check that the FIFO size
> > is correct for a Startech.
> 
> size_fifo() is claimed to be unreliable at detecting the FIFO size,
> so I don't feel safe about using it here.
> 
> I'd suggest something like:
> 
> 	serial_outp(port, UART_LCR, UART_LCR_DLAB);
> 	efr = serial_in(port, UART_EFR);
> 	if ((efr & 0xfc) == 0) {
> 		serial_out(port, UART_EFR, 0xac | (efr & 3));
> 		/* if top 6 bits return zero, its motorola */
> 		if (serial_in(port, UART_EFR) == (efr & 3)) {
> 			/* motorola port */
> 		} else {
> 			/* ST16C650V1 port */
> 		}
> 		/* restore old value */
> 		serial_outb(port, UART_EFR, efr);
> 	}

Okay, from Kumar Gala (cc'ed) I've got the following patch for 2.4:
===== drivers/char/serial.c 1.34 vs edited =====
--- 1.34/drivers/char/serial.c	Sun Jul  6 22:33:28 2003
+++ edited/drivers/char/serial.c	Tue Sep  9 16:50:22 2003
@@ -3741,7 +3741,10 @@
 		/* Check for Startech UART's */
 		serial_outp(info, UART_LCR, UART_LCR_DLAB);
 		if (serial_in(info, UART_EFR) == 0) {
-			state->type = PORT_16650;
+			serial_outp(info, UART_EFR, 0xA8);
+			if (serial_in(info, UART_EFR) != 0)
+				state->type = PORT_16650;
+			serial_outp(info, UART_EFR, 0);
 		} else {
 			serial_outp(info, UART_LCR, 0xBF);
 			if (serial_in(info, UART_EFR) == 0)

And I forward ported this up to 2.6 as:
===== drivers/serial/8250.c 1.36 vs edited =====
--- 1.36/drivers/serial/8250.c	Tue Sep  9 07:22:12 2003
+++ edited/drivers/serial/8250.c	Tue Sep  9 16:49:07 2003
@@ -469,8 +469,13 @@
 	 */
 	serial_outp(up, UART_LCR, UART_LCR_DLAB);
 	if (serial_in(up, UART_EFR) == 0) {
-		DEBUG_AUTOCONF("EFRv1 ");
-		up->port.type = PORT_16650;
+		serial_outp(up, UART_EFR, 0xA8);
+		if (serial_in(up, UART_EFR) != 0) {
+			DEBUG_AUTOCONF("EFRv1 ");
+			up->port.type = PORT_16650;
+		} else
+			DEBUG_AUTOCONF("Motorola 8xxx DUART ");
+		serial_outp(up, UART_EFR, 0);
 		return;
 	}

Better?

-- 
Tom Rini
http://gate.crashing.org/~trini/

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Make the Startech UART detection 'more correct'.
  2003-09-09 23:51   ` Tom Rini
@ 2003-09-24 22:40     ` Kumar Gala
  0 siblings, 0 replies; 6+ messages in thread
From: Kumar Gala @ 2003-09-24 22:40 UTC (permalink / raw)
  To: tytso; +Cc: kernel Mailing List, linux-serial, gallen, Tom Rini

I was wondering what the status of this patch was for the 2.4 side?

thanks

- kumar

On Tuesday, September 9, 2003, at 06:51 PM, Tom Rini wrote:

> On Tue, Sep 09, 2003 at 05:18:59PM +0100, Russell King wrote:
>
>> On Mon, Sep 08, 2003 at 01:54:31PM -0700, Tom Rini wrote:
>>> Hello.  The following patches (vs 2.4 and 2.6) make the Startech UART
>>> detection 'more correct'  The problem is that on with the Motorola
>>> MPC82xx line (8245 for example) it has an internal DUART that it 
>>> claims
>>> to be PC16550D compatible, and it has an additional EFR (Enhanced
>>> Feature Register) at offset 0x2, like on the Startech UARTS.  
>>> However,
>>> it is not a Startech, and when it's detected as such, FIFOs don't 
>>> work.
>>> The fix for this is that the Startech UARTs have a 32 byte FIFO [1] 
>>> and
>>> the MPC82xx DUARTs have a 16-byte FIFO [2], to check that the FIFO 
>>> size
>>> is correct for a Startech.
>>
>> size_fifo() is claimed to be unreliable at detecting the FIFO size,
>> so I don't feel safe about using it here.
>>
>> I'd suggest something like:
>>
>> 	serial_outp(port, UART_LCR, UART_LCR_DLAB);
>> 	efr = serial_in(port, UART_EFR);
>> 	if ((efr & 0xfc) == 0) {
>> 		serial_out(port, UART_EFR, 0xac | (efr & 3));
>> 		/* if top 6 bits return zero, its motorola */
>> 		if (serial_in(port, UART_EFR) == (efr & 3)) {
>> 			/* motorola port */
>> 		} else {
>> 			/* ST16C650V1 port */
>> 		}
>> 		/* restore old value */
>> 		serial_outb(port, UART_EFR, efr);
>> 	}
>
> Okay, from Kumar Gala (cc'ed) I've got the following patch for 2.4:
> ===== drivers/char/serial.c 1.34 vs edited =====
> --- 1.34/drivers/char/serial.c	Sun Jul  6 22:33:28 2003
> +++ edited/drivers/char/serial.c	Tue Sep  9 16:50:22 2003
> @@ -3741,7 +3741,10 @@
>  		/* Check for Startech UART's */
>  		serial_outp(info, UART_LCR, UART_LCR_DLAB);
>  		if (serial_in(info, UART_EFR) == 0) {
> -			state->type = PORT_16650;
> +			serial_outp(info, UART_EFR, 0xA8);
> +			if (serial_in(info, UART_EFR) != 0)
> +				state->type = PORT_16650;
> +			serial_outp(info, UART_EFR, 0);
>  		} else {
>  			serial_outp(info, UART_LCR, 0xBF);
>  			if (serial_in(info, UART_EFR) == 0)
>
> And I forward ported this up to 2.6 as:
> ===== drivers/serial/8250.c 1.36 vs edited =====
> --- 1.36/drivers/serial/8250.c	Tue Sep  9 07:22:12 2003
> +++ edited/drivers/serial/8250.c	Tue Sep  9 16:49:07 2003
> @@ -469,8 +469,13 @@
>  	 */
>  	serial_outp(up, UART_LCR, UART_LCR_DLAB);
>  	if (serial_in(up, UART_EFR) == 0) {
> -		DEBUG_AUTOCONF("EFRv1 ");
> -		up->port.type = PORT_16650;
> +		serial_outp(up, UART_EFR, 0xA8);
> +		if (serial_in(up, UART_EFR) != 0) {
> +			DEBUG_AUTOCONF("EFRv1 ");
> +			up->port.type = PORT_16650;
> +		} else
> +			DEBUG_AUTOCONF("Motorola 8xxx DUART ");
> +		serial_outp(up, UART_EFR, 0);
>  		return;
>  	}
>
> Better?
>
> -- 
> Tom Rini
> http://gate.crashing.org/~trini/
> <mime-attachment>


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2003-09-24 22:40 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2003-09-08 20:54 [PATCH] Make the Startech UART detection 'more correct' Tom Rini
2003-09-09 16:18 ` Russell King
2003-09-09 19:12   ` Stuart MacDonald
2003-09-09 19:23     ` Tom Rini
2003-09-09 23:51   ` Tom Rini
2003-09-24 22:40     ` Kumar Gala

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox