[U-Boot-Users] [PATCH] ColdFire: Fix UART baudrate at 115200

[Jerry Van Baren <gerald.vanbaren@ge.com>]

Tsi-Chung.Liew wrote:
> From: TsiChung Liew <Tsi-Chung.Liew@freescale.com>
> 
> If bus frequency is larger than 133MHz, the UART cannot
> output baudrate at 115200 correctly.
> 
> Signed-off-by: TsiChung Liew <Tsi-Chung.Liew@freescale.com>
> ---
>  drivers/serial/mcfuart.c |    5 ++++-
>  1 files changed, 4 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/serial/mcfuart.c b/drivers/serial/mcfuart.c
> index 88f3eb1..fca76bd 100644
> --- a/drivers/serial/mcfuart.c
> +++ b/drivers/serial/mcfuart.c
> @@ -64,7 +64,10 @@ int serial_init(void)
>  
>  	/* Setting up BaudRate */
>  	counter = (u32) (gd->bus_clk / (gd->baudrate));
> -	counter >>= 5;
> +	counter = (counter + 31) >> 5;
> +
> +	if ((gd->bus_clk > 133333333) && (gd->baudrate >= 115200))
> +		counter++;
>  
>  	/* write to CTUR: divide counter upper byte */
>  	uart->ubg1 = (u8) ((counter & 0xff00) >> 8);

This doesn't look right at all.  It looks like you are patching up 
integer math problems by using different bad math and then special 
casing the result.  (In the metal working world, this is known as 
"measure with a micrometer, mark with chalk, cut with a torch, and grind 
to fit.")

Part A:
-------
 >  	counter = (u32) (gd->bus_clk / (gd->baudrate));

If you want this to be more accurate, you should round it:

 >  	counter = (u32) ((gd->bus_clk + (gd->baudrate / 2)) / (gd->baudrate));

Part B:
-------
 > +	counter = (counter + 31) >> 5;

This is not rounding properly, it is doing a "ceiling".

 > +	counter = (counter + 16) >> 5;

Part C:
-------

 > +	if ((gd->bus_clk > 133333333) && (gd->baudrate >= 115200))
 > +		counter++;

This looks totally bogus.  I very strongly suspect you need this because 
the above parts A: and B: are not rounding the division math properly, 
resulting in a wrong divisor *for your configuration*.  While the above 
conditional may result in the correct divisor *for your configuration,* 
it probably will be wrong for some finite set of *other* configurations.

If I got my algebra correct and my parenthesis balanced, I believe the 
above is trying to calculate the following formula:
	counter = (u32) (
		((gd->bus_clk + (gd->baudrate / 2) +
		 (gd->baudrate * 16))
		/ (gd->baudrate * 32);

Note, however, that (gd->baudrate / 2) is going to be relatively small 
compared to gd->bus_clk and (gd->baudrate * 16), so I suspect that the 
above formula could be changed to the following formula with no 
significant impact in accuracy:
	counter = (u32) (
		((gd->bus_clk + (gd->baudrate * 16))
		/ (gd->baudrate * 32);

Hmmmm, checking the math with 133e6 for the bus rate with full precision 
math (i.e. a calculator), I get a divisor value of 36.08 (truncated to 
an integer => 36).  Your hack-math results in a divisor of 38.  The last 
formula I proposed above  result in a divisor of 36.58 (truncated to an 
integer => 36).  That checks.

Real math    => 36.08
Hack-math    => 38    (109375 baud => 5.1% off)
Last formula => 36    (115451 baud => 0.2% off)

For 160e6 bus frequency:
Real math    => 43.40
Hack-math    => 44    (113636 baud => 1.4% off)
Last formula => 43    (116279 baud => 0.1% off)

I don't see how your formula works (well, actually it works but only 
because async serial can handle ~10% error).  In fact, it looks to me 
that your "correction" actually results in *more* error than the 
original calculation *without* rounding.  What is your actual bus speed? 
  What are to using on the other end of the serial line - I'm wondering 
if your receiver end is substantially off, causing a stack-up error that 
is causing your problem???  Is your board hardware marginal at 115200 
baud - what does it look like on a scope???

Best regards,
gvb