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

[Liew Tsi Chung]

I see your point. I use your formula if you have not other comments.

Regards,
TsiChung 

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

Liew Tsi Chung wrote:
>> Where are you getting the +31 from?  Is this in the User's Manual?
> No in user's manual. The +31 is rounding up purpose.

The formula (n + 31) / 32 is *NOT ROUNDING* the answer, it is doing a
"ceiling" function.  You need to add 1/2 the divisor to *ROUND*, i.e. (n

+ 16) / 32.

>> Computations are for mathematicians.  Real engineers measure the
> actual frequency.  ;-)
> Agree.
> 
>> If you change the " - 31" to " - 42", your computations will result
in
> a different divisor.  
>> Why are you subtracting what appears to be an arbitrary number (31)?
> Can't use 42 when the user's manual spec the divisor is 32. The "- 31"
> comes from counter = ((bus_freq / baudrate) + 31) / 32. Using the same
> calculation as mentioned here to find out bus_freq. bus_freq =
((counter
> * 32) - 31) * baudrate.

The 42 is an obscure reference to the Hitchhiker's Guide to the Galaxy -

the answer to the question of life, the universe and everything.  In 
other words, it is a totally arbitrary number.  Your use of +31 is not a

*ROUNDING*, it is *CEILING* and thus I claim it is arbitrary, especially

with the +1 to the divisor if your baud rate is suppose to be 115200 
baud.  You don't understand your hardware (if it makes you feel better, 
neither do I, but it is important that *you* understand it).

>> Are you sure your input frequency is 140MHz and not 144MHZ???  Have
> you read the itty-bitty 
>> numbers on the crystal?  Have you put a scope on it and actually
> measured it?  Never trust 
>> those hardware people.  ;-)
> Actually, the bus freq is 70Mhz, 140Mhz is cpu freq (scope
measurement).
> The input freq (crystal/oscillator) not always 1 to 1 ratio. Take
> M5253EVBE for example, the input clk is 11Mhz. Then, it multiply up
and
> divide down for CPU and bus frequency.

This *REALLY* makes your formula wrong.  According to the MCF5253 
Reference Manual Eqn. 15-1, the baud rate is
   fsys / (32 * divisor)
and fsys is 70MHz, *NOT* 140MHz.

baud = SysClk / (32 * divisor)

divisor = (70e6 / 32) / baud

divisor = 2187500 / 115200

divisor = 18.99 => 19

To *round*, add 1/2 the desired baud rate

divisor = ((70e6 / 32) * (baud / 2)) / baud

divisor = (2187500 + 57600) / 115200

divisor = 19.5 => integer truncation => 19

----

This means gd->bus_clk is *70MHz* (1/2 the CPU frequency), *NOT* 140MHz 
as you assume (note that this matches the Reference Manual).

I return to my contention:

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);

Simplifying the *32 factor to match my formula above:

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

	counter = ((70e6 / 32) + (115200 / 2)) / 115200
	counter = 19   (after inter truncation)

Note that, without adding (baudrate / 2) - rounding, the above formula 
results in a divisor of 18 rather than the proper 19.  THIS IS YOUR 
PROBLEM and also what your fix should be.

If you rounded properly, the baud rate calculation would work without 
all the magic.

Here is what your change actually does:

  	/* Setting up BaudRate */
	counter = (u32) (gd->bus_clk / (gd->baudrate));
This divide is doing an integer truncation and you are losing 
significant digits.
   70e6 / 115200 = 607.64 => integer truncation = 607

	counter = (counter + 31) >> 5;
This is "ceiling", but you've already lost the digits due to the 
previous divide with integer truncation (no rounding).
   (607 + 31) / 32 = 19.9 => integer truncation = 19

You get lucky and get the right answer even though you don't deserve it.
;-)

	if ((gd->bus_clk > 133333333) && (gd->baudrate >= 115200))
		counter++;
This is totally bogus.  gd->bus_clk == 70e6 so this is *always false.*

>> Looking at the MFC5407 (arbitrarily, not knowing what processor you
> have), the dividers are 
>> as expected and *do not* match your "- 31" 
>> calculations.  They have a simple example and simple math that
matches
> (no "- 31").
> 
>> Hmmm, the MFC5407 has a max CLKIN (and thus bus clock) of 54MHz.  You
> must have a different 
>> flavor.  MFC548x maxes at 50MHz.
>> What processor are you using?
> 52x2, 532x, 547x_8x, 5445x. The 5253EVB is having the issue where the
> calculation does not apply if baudrate is 115200.
> 
>> Ahh, MFC5249 has a 140MHz bus.  Still is a straight forward divide-by
> n*32, no "- 31".
> I thought the same too, but the 140Mhz is cpu freq.

It is clocked by the System Clock, not CPU frequency.  The CPU frequency

is 2x the System Clock.

> Regards,
> TsiChung

HTH,
gvb