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

[Jerry Van Baren]

Liew Tsi Chung wrote:
> 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

Where are you getting the +31 from?  Is this in the User's Manual?

> 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

Computations are for mathematicians.  Real engineers measure the actual 
frequency.  ;-)

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

> 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

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

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?  Ahh, MFC5249 has a 140MHz bus.  Still is a straight 
forward divide-by n*32, no "- 31".

gvb

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