make 16C950 UARTs work

make 16C950 UARTs work

[Dave Jones]

This patch has been submitted a number of times, and doesn't seem
to get any upstream traction, which is a shame, as it seems to work
for users, and I keep inadvertantly dropping it from the Fedora
kernel everytime I rebase it.

http://marc.theaimsgroup.com/?l=linux-kernel&m=112687270832687&w=2
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=126403

From: Mathias Adam <a2@adamis.de>
Signed-off-by: Dave Jones <davej@redhat.com>


--- linux-2.6.13-org/drivers/serial/8250.c	2005-08-29 01:41:01.000000000 +0200
+++ linux-2.6.13/drivers/serial/8250.c	2005-09-16 12:18:14.000000000 +0200
@@ -7,6 +7,9 @@
  *
  *  Copyright (C) 2001 Russell King.
  *
+ *  2005/09/16: Enabled higher baud rates for 16C95x.
+ *		(Mathias Adam <a2@adamis.de>)
+ *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
  * the Free Software Foundation; either version 2 of the License, or
@@ -1652,6 +1655,14 @@
 	else if ((port->flags & UPF_MAGIC_MULTIPLIER) &&
 		 baud == (port->uartclk/8))
 		quot = 0x8002;
+	/*
+	 * For 16C950s UART_TCR is used in combination with divisor==1
+	 * to achieve baud rates up to baud_base*4.
+	 */
+	else if ((port->type == PORT_16C950) &&
+		 baud > (port->uartclk/16))
+		quot = 1;
+
 	else
 		quot = uart_get_divisor(port, baud);
 
@@ -1665,7 +1676,7 @@
 	struct uart_8250_port *up = (struct uart_8250_port *)port;
 	unsigned char cval, fcr = 0;
 	unsigned long flags;
-	unsigned int baud, quot;
+	unsigned int baud, quot, max_baud;
 
 	switch (termios->c_cflag & CSIZE) {
 	case CS5:
@@ -1697,7 +1708,8 @@
 	/*
 	 * Ask the core to calculate the divisor for us.
 	 */
-	baud = uart_get_baud_rate(port, termios, old, 0, port->uartclk/16); 
+	max_baud = (up->port.type == PORT_16C950 ? port->uartclk/4 : port->uartclk/16);
+	baud = uart_get_baud_rate(port, termios, old, 0, max_baud); 
 	quot = serial8250_get_divisor(port, baud);
 
 	/*
@@ -1733,6 +1745,19 @@
 	 */
 	spin_lock_irqsave(&up->port.lock, flags);
 
+	/* 
+	 * 16C950 supports additional prescaler ratios between 1:16 and 1:4
+	 * thus increasing max baud rate to uartclk/4.
+	 */
+	if (up->port.type == PORT_16C950) {
+		if (baud == port->uartclk/4)
+			serial_icr_write(up, UART_TCR, 0x4);
+		else if (baud == port->uartclk/8)
+			serial_icr_write(up, UART_TCR, 0x8);
+		else
+			serial_icr_write(up, UART_TCR, 0);
+	}
+	
 	/*
 	 * Update the per-port timeout.
 	 */


-- 
http://www.codemonkey.org.uk

Re: make 16C950 UARTs work

[Russell King]

On Wed, Aug 02, 2006 at 03:49:38PM -0400, Dave Jones wrote:
> This patch has been submitted a number of times, and doesn't seem
> to get any upstream traction, which is a shame, as it seems to work
> for users, and I keep inadvertantly dropping it from the Fedora
> kernel everytime I rebase it.

As I've said, I'm ignoring all 950 patches because I don't know what
works and what doesn't, and it's highly likely that applying one fix
for one card breaks already working fixes for other cards because
they have different crystals fitted, thereby requiring different
register settings.

My requests for the broken BT hardware which dwmw2 promised me which
the original requests were based upon have consistently resulted in
promise of action but no real action.

Basically, either I need 950 based hardware so I can at least validate
that new fixes don't break existing setups, or someone else needs to
be in this position and take on the responsibility for reviewing and
testing future 950 based patches.

I don't believe that there are sufficient users out there who follow
the kernel to allow the "dump in -mm and wait a bit to see if anyone
complains" solution will work.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 Serial core

Re: make 16C950 UARTs work

[Dave Jones]

On Wed, Aug 02, 2006 at 09:17:23PM +0100, Russell King wrote:
 > On Wed, Aug 02, 2006 at 03:49:38PM -0400, Dave Jones wrote:
 > > This patch has been submitted a number of times, and doesn't seem
 > > to get any upstream traction, which is a shame, as it seems to work
 > > for users, and I keep inadvertantly dropping it from the Fedora
 > > kernel everytime I rebase it.
 > 
 > As I've said, I'm ignoring all 950 patches because I don't know what
 > works and what doesn't

Well, thanks for the reply.

 > , and it's highly likely that applying one fix
 > for one card breaks already working fixes for other cards because
 > they have different crystals fitted, thereby requiring different
 > register settings.

FWIW, we've had that patch in Fedora (most of the time), since 2.6.13,
and I don't recall seeing any other serial breakage ports (on this,
or any UART for that matter).

 > My requests for the broken BT hardware which dwmw2 promised me which
 > the original requests were based upon have consistently resulted in
 > promise of action but no real action.
 > 
 > Basically, either I need 950 based hardware so I can at least validate
 > that new fixes don't break existing setups, or someone else needs to
 > be in this position and take on the responsibility for reviewing and
 > testing future 950 based patches.
 > 
 > I don't believe that there are sufficient users out there who follow
 > the kernel to allow the "dump in -mm and wait a bit to see if anyone
 > complains" solution will work.

Still, leaving a patch out in the cold for 11 months, when we know it
at least makes things work for some users strikes me as wrong.
If we took this approach with every driver, we'd end up not supporting
the majority of things we support today.

		Dave

-- 
http://www.codemonkey.org.uk

Re: make 16C950 UARTs work

[Russell King]

On Wed, Aug 02, 2006 at 04:31:46PM -0400, Dave Jones wrote:
> Still, leaving a patch out in the cold for 11 months, when we know it
> at least makes things work for some users strikes me as wrong.
> If we took this approach with every driver, we'd end up not supporting
> the majority of things we support today.

Define "majority".  How do we know whether what's merged works for
the majority, and this fix breaks it for one type of card.

Eg, there's another 950 setup which I received a report back in May
which had a 16MHz crystal, and the _only_ way to get that to work
reliably was to use setserial with spd_cust to specify 104 for 9600
baud, etc.  We never got to work out exactly what was going on.

However, based on my experience with dwmw2's card, registers such as
the TCR are programmed on card powerup to have some non-default state
depending on the manufacturers knowledge of the card.  This means if
we start fscking around with them in order to support these "wizzy
new features" other normal things will break.

Let me repeat what I want - I want some way that any changes which are
proposed in this area get tested against some 950-based implementation
which we call "the control implementation".  That way we get to know if
we're going forwards, backwards or sideways.

Blindly applying patches which mess around with 950 clocking registers
based upon what one random card does just isn't acceptable.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 Serial core

Re: make 16C950 UARTs work

[Theodore Tso]

On Wed, Aug 02, 2006 at 09:17:23PM +0100, Russell King wrote:
> On Wed, Aug 02, 2006 at 03:49:38PM -0400, Dave Jones wrote:
> > This patch has been submitted a number of times, and doesn't seem
> > to get any upstream traction, which is a shame, as it seems to work
> > for users, and I keep inadvertantly dropping it from the Fedora
> > kernel everytime I rebase it.
> 
> As I've said, I'm ignoring all 950 patches because I don't know what
> works and what doesn't, and it's highly likely that applying one fix
> for one card breaks already working fixes for other cards because
> they have different crystals fitted, thereby requiring different
> register settings.

Actually, this particular one is probably safe, because it doesn't
depend on what crystal is installed, but rather works by using a
documented feature in the Oxford 950 UART to oversample the clock
signal.  In addition, the patch only activates UART_TCR if the user
requests the higher baud rates, so the patch only does something if
the user requests a baud rate that would have been previously rejected
by the driver.  Worse case, if the UART oscillator is badly
implemented, it might make the UART behave out of spec when supporting
an "overclocked" baud rate, but only when trying to support 234000 bps
on hardware where the stock driver would only support 115200 bps.  The
patch programs the UART identically for 115200bps, so it's pretty easy
to prove by inspection that it won't break existing setups.

> Basically, either I need 950 based hardware so I can at least validate
> that new fixes don't break existing setups, or someone else needs to
> be in this position and take on the responsibility for reviewing and
> testing future 950 based patches.

I'll have to wait until I get back home to make sure, but I think have
a 950 based card around somewhere that I'd be happy to give to you on
extended loan.  I'm not sure whether I have an PCI card; it might
actually be part of my ISA serial card collection....

						- Ted

Re: make 16C950 UARTs work

[Mathias Adam]

Hello,

On 02.08.06 18:59:13, Theodore Tso wrote:
> On Wed, Aug 02, 2006 at 09:17:23PM +0100, Russell King wrote:
> > As I've said, I'm ignoring all 950 patches because I don't know what
> > works and what doesn't, and it's highly likely that applying one fix
> > for one card breaks already working fixes for other cards because
> > they have different crystals fitted, thereby requiring different
> > register settings.
> 
> Actually, this particular one is probably safe, because it doesn't
> depend on what crystal is installed, but rather works by using a
> documented feature in the Oxford 950 UART to oversample the clock
> signal.  In addition, the patch only activates UART_TCR if the user
> requests the higher baud rates, so the patch only does something if
> the user requests a baud rate that would have been previously rejected
> by the driver.  [...]

exactly, this is what the patch does. I implemented it according to
Oxford's 950 datasheet so it's not specific to the Socket BT card I have
used for testing. Furthermore, code similar to this has already been in
kernel 2.4 but got removed for some reason - perhaps there have been
other problems with 2.4's serial driver?

As far as I remember the current 2.6 serial driver uses a fixed value
for uartclk = 1843200 = 115200*16 while 2.4 was somewhat more general.
Together with 2.6's minimum divisor of 16 this gives the maximum baud
rate of 115200.

Are there any documents on why this was changed from 2.4 to 2.6?

Regards
Mathias

PS: currently I'm not subscribed to lkml so please CC me.

Re: make 16C950 UARTs work

[Russell King]

On Tue, Aug 08, 2006 at 01:20:32AM +0200, Mathias Adam wrote:
> Are there any documents on why this was changed from 2.4 to 2.6?

Probably because 2.4 came after 2.6?  Remember that some 2.4 development
was done after the 2.5 kernel tree was forked.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 Serial core

Re: make 16C950 UARTs work

[Alan Cox]

Ar Mer, 2006-08-02 am 15:49 -0400, ysgrifennodd Dave Jones:
> This patch has been submitted a number of times, and doesn't seem
> to get any upstream traction, which is a shame, as it seems to work
> for users, and I keep inadvertantly dropping it from the Fedora
> kernel everytime I rebase it.

We really ought to do that based on the PCI subvendor/subdevice id of
the boards in use if possible surely ? It ought to be safe for x86
because nobody is going to use anything but chip default values so they
can avoid needing a ROM.

Re: make 16C950 UARTs work

[Russell King]

On Wed, Aug 02, 2006 at 11:08:56PM +0100, Alan Cox wrote:
> Ar Mer, 2006-08-02 am 15:49 -0400, ysgrifennodd Dave Jones:
> > This patch has been submitted a number of times, and doesn't seem
> > to get any upstream traction, which is a shame, as it seems to work
> > for users, and I keep inadvertantly dropping it from the Fedora
> > kernel everytime I rebase it.
> 
> We really ought to do that based on the PCI subvendor/subdevice id of
> the boards in use if possible surely ? It ought to be safe for x86
> because nobody is going to use anything but chip default values so they
> can avoid needing a ROM.

Not correct - there are PCMCIA-based versions of these chips and they
do have weirdo values in the registers to cope with custom crystals
which magically vanish if you reset the UART.

dwmw2's 950-based bluetooth CF card does exactly this.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 Serial core

Re: make 16C950 UARTs work

[Petr Vandrovec]

Russell King wrote:
> On Wed, Aug 02, 2006 at 11:08:56PM +0100, Alan Cox wrote:
> 
>>Ar Mer, 2006-08-02 am 15:49 -0400, ysgrifennodd Dave Jones:
>>
>>>This patch has been submitted a number of times, and doesn't seem
>>>to get any upstream traction, which is a shame, as it seems to work
>>>for users, and I keep inadvertantly dropping it from the Fedora
>>>kernel everytime I rebase it.
>>
>>We really ought to do that based on the PCI subvendor/subdevice id of
>>the boards in use if possible surely ? It ought to be safe for x86
>>because nobody is going to use anything but chip default values so they
>>can avoid needing a ROM.
> 
> 
> Not correct - there are PCMCIA-based versions of these chips and they
> do have weirdo values in the registers to cope with custom crystals
> which magically vanish if you reset the UART.

But current serial.c IS resetting UART.  Unconditionally.  So I have no idea how 
is card setup supposed to survive...

> dwmw2's 950-based bluetooth CF card does exactly this.

To my knowledge right fix for this is allowing non-standard baud rates and then 
asking Dave to find which xtal his card uses.  I have to use baud_base 50400 
(xtal 806.4kHz) with one of my 16c950 pcmcia modems, and 1152000 with another 
(xtal 18.432MHz) one.  Under Windows both work "out of the box" as prescaller is 
programmed to 7/16 (resp. 10) by their EPROM, but Linux driver ditches this 
setting out of 16c950 registers (by resetting chip).  In second case fortunately 
code knows how to program 10 times bigger values to the registers, but with 
first modem it fails as it has no idea how to derive 115200Bd from 806kHz XTAL...
					Thanks,
						Petr Vandrovec