[PATCH v2] 8250: Now honours baud rate lower bounds

[Anton Vorontsov]

A platform clock drives 8250 ports in most SOC systems, the clock
might run at high frequencies, and so it's not always possible to
downscale uart clock to a desired value.

Currently the 8250 uart driver accepts not supported baud rates, and
what is worse, it is doing this silently, and then passes not accepted
values to a new termios, so userspace has no chance to catch this kind
of errors (userspace verifies that settings were accepted by reading
back and comparing the settings).

This patch fixes the issue by passing minimum baud rate to the
uart_get_baud_rate() call, the call should take care of all bounds,
so userspace should now report:

  # stty -F /dev/ttyS0 speed 300
  115200
  stty: /dev/ttyS0: unable to perform all requested operations

p.s. uart_get_baud_rate() falls back to 9600, which still might be too
     low for some 10 GHz platforms, but that's a separate issue, and
     we can wait with fixing this till we find such a platform.

Signed-off-by: Anton Vorontsov <avorontsov@xxxxxxxxxxxxx>
---

On Thu, Jul 02, 2009 at 10:53:12PM +0100, Alan Cox wrote:
> On Fri, 3 Jul 2009 01:36:24 +0400
> Anton Vorontsov <avorontsov@xxxxxxxxxxxxx> wrote:
> 
> > A platform clock drives 8250 ports in most SOC systems, the clock
> > might run at high frequencies, and so it's not always possible to
> > downscale uart clock to a desired value.
> 
> > This patch fixes the issue by passing minimum baud rate to the
> > uart_get_baud_rate() call, the call should take care of all bounds,
> > so userspace should now report:
> 
> We ought to be able to deduce the minimum baud rate from the divisor
> being bigger than the chip supports - or do some of these devices not
> support the full hw divisor range of the real chip which is what I assume
> from your patch ?

No, HW supports full hw divisor range, but uart clk is 533 MHz,
and 533 MHz / 16 / 0xffff = 508 bauds. But currently the driver
calculates divisor without min value, and 300 bauds ends up with
0x1b2071 div value which doesn't fit into 16 bits, and so we're
getting bogus baud rate and garbage on the console, and no single
message that something went wrong.

> > +static unsigned int _serial_dl_max(struct uart_8250_port *up)
> > +{
> > +	return 0xffff;
> > +}
> > +
> 
> We have far too many magic ifdef routines for this sort of stuff already.
> Is there any reason we can't put info on the true divisor or at least the
> baud rate range into the uart_8250_port structure so that we get rid of
> all of 

Heh. I just found specs for Au1100, and it appears they use 32
bit registers, but the divisor itself is still 0xffff (other bits
are reserved). So I don't think we need any special cases, at least
for now.

How about this patch down below?

Thanks.

 drivers/serial/8250.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/drivers/serial/8250.c b/drivers/serial/8250.c
index fb867a9..7720816 100644
--- a/drivers/serial/8250.c
+++ b/drivers/serial/8250.c
@@ -2272,7 +2272,9 @@ serial8250_set_termios(struct uart_port *port, struct ktermios *termios,
 	/*
 	 * Ask the core to calculate the divisor for us.
 	 */
-	baud = uart_get_baud_rate(port, termios, old, 0, port->uartclk/16);
+	baud = uart_get_baud_rate(port, termios, old,
+				  port->uartclk / 16 / 0xffff,
+				  port->uartclk / 16);
 	quot = serial8250_get_divisor(port, baud);
 
 	/*
-- 
1.6.3.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/