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

[Liew Tsi Chung]

Jerry,

	The calculation you provided does not work, I tried it before.
The best approach is reverse calculation after finding the closer
counter (or divider), then find the smaller gap to the bus frequency.

Using the calculation without the fix: counter = ((bus_freq / baudrate)
+ 31 )/ 32
Bus freq 140Mhz and baudrate 115200

counter = ((140000000 / 115200) + 31) / 32
counter = 38

However, the 38 divider shows partial garbage message when output to the
terminal.

Now, perform reverse calculation to obtain bus frequency.
bus_freq = ((counter * 32) - 31) * 115200

bus_freq = ((38 * 32) - 31) * 115200
bus_freq = 136512000
diff = 140000000 - 136512000 = 3488000

counter	bus_freq	diff
=======	=========	=======
36		129139200	way too off!
37		132825600	7174400
38		136512000	3488000
39		140198400	198400	<=== Work at this counter! Small
diff.
40		143884800	3884800

Regards,
TsiChung


-----Original Message-----
From: Jerry Van Baren [mailto:gerald.vanbaren at ge.com] 
Sent: Wednesday, May 28, 2008 1:45 PM
To: Liew Tsi Chung
Cc: U-Boot-Users; Wolfgang Denx; Rigby John
Subject: Re: [U-Boot-Users] [PATCH] ColdFire: Fix UART baudrate at
115200

Tsi-Chung.Liew wrote:
> From: TsiChung Liew <Tsi-Chung.Liew at 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 at 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