[U-Boot-Users] [PATCH] Round the serial port clock divisor value returned by calc_divisor()

[Hugo Villeneuve]

Jerry Van Baren wrote:
> Hugo Villeneuve wrote:
>> Jerry Van Baren wrote:
>>> Hugo Villeneuve wrote:
>>>> Round the serial port clock divisor value returned by
>>>> calc_divisor(). 
>>>> 
>>>> Signed-off-by: Hugo Villeneuve <hugo.villeneuve at lyrtech.com>
>>>> 
>>>> ---
>>>> 
>>>> Rounding is important, especially when using high baud rates
>>>> values like 115200bps. When using the non-rounded value, some
>>>> boards will work and some won't.
>>>> 
>>>>  drivers/serial/serial.c |    7 ++++++-
>>>>  1 files changed, 6 insertions(+), 1 deletions(-)
>>>> 
>>>> diff --git a/drivers/serial/serial.c b/drivers/serial/serial.c
>>>> index 76425d8..7e315ad 100644 --- a/drivers/serial/serial.c +++
>>>> b/drivers/serial/serial.c @@ -124,6 +124,8 @@ static NS16550_t
>>>> serial_ports[4] = { 
>>>> 
>>>>  static int calc_divisor (NS16550_t port)
>>>>  {
>>>> +	u32 divisor_x10;
>>>> +
>>>>  #ifdef CONFIG_OMAP1510
>>>>  	/* If can't cleanly clock 115200 set div to 1 */
>>>>  	if ((CFG_NS16550_CLK == 12000000) && (gd->baudrate == 115200)) {
>>>> @@ -144,8 +146,11 @@ static int calc_divisor (NS16550_t port) 
>>>>  #else  #define MODE_X_DIV 16 #endif
>>>> -	return (CFG_NS16550_CLK / MODE_X_DIV / gd->baudrate);
>>>> 
>>>> +	/* Compute divisor value with rounding by adding 0.5 */
>>>> +	divisor_x10 = (10 * CFG_NS16550_CLK) / MODE_X_DIV /
>>>> gd->baudrate; + +	return (divisor_x10 + 5) / 10;
>>>>  }
>>>> 
>>>>  #if !defined(CONFIG_SERIAL_MULTI)
>>> Hi Hugo,
>>> 
>>> Will a real rounding work?  Work better?  If I got my mental math
>>> and parenthesis right and the resulting math doesn't overflow your
>>> registers, the following will add 1/2 the baud rate scaled by the
>>> MODE_X_DIV and then perform the divide which will do full rounding.
>>> 
>>> 	return (((CFG_NS16550_CLK + ((gd->baudrate / 2)* MODE_X_DIV))
>                                  oops, I missed a space ^
>>> 		/ MODE_X_DIV) / gd->baudrate);
>> I tested it and it works for me.
>> But maybe the code is less obvious that way?
> 
> Obviousness is relative.  :-)
> This will be more accurate, but accuracy is relative too.  :-/  It
> only has to be close.
> 
> Since CFG_NS16550_CLK is very likely a multiple of MODE_X_DIV or it
> doesn't matter if it isn't because it is much, much larger than
> MODE_X_DIV,  the formula could be simplified slightly without
> substantially hurting the accuracy...
> 
> 	return (((CFG_NS16550_CLK + (gd->baudrate / 2))
> 		/ MODE_X_DIV) / gd->baudrate);
> 
> A slight bit of added perceived complexity may be coming from my
> addition of parenthesis to emphasize the evaluation order.  The
> following (should be) equivalent and reads better to my brain.
> 
> 	return (CFG_NS16550_CLK + (gd->baudrate / 2)) /
> 		(MODE_X_DIV * gd->baudrate);
> 
> 
>>> Alternately, I prefer to scale up by 16 and then divide by 8 since
>>> processors can do that very efficiently ( << 4 followed by >> 3).
>>> Right. 
>> 
>> I prefer the second solution for its simplicity, but the first one
>> also works. 
> 
> Bottom line: it's your call IMHO.

Ok then, let´s use the second solution, updated patch will follow.

Hugo V.

Hugo Villeneuve
Hardware developer | Concepteur matériel
Lyrtech
Phone/Tél. : (1) (418) 877-4644 #2395
Toll-free/Sans frais - Canada & USA : (1) (888) 922-4644 #2395
Fax/Téléc. : (1) (418) 877-7710
www.lyrtech.com
Infinite possibilities...TM