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

[Jerry Van Baren]

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.

Best regards,
gvb