[U-Boot-Users] [PATCH] Round the serial port clock divisor value returned by calc_divisor()
Round the serial port clock divisor value returned by calc_divisor().
Signed-off-by: Hugo Villeneuve hugo.villeneuve@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)
Hugo Villeneuve wrote:
Round the serial port clock divisor value returned by calc_divisor().
Signed-off-by: Hugo Villeneuve hugo.villeneuve@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)) / 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).
Best regards, gvb
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@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)) / MODE_X_DIV) / gd->baudrate);
I tested it and it works for me. But maybe the code is less obvious that way?
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.
Hugo.
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
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@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
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@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
Round the serial port clock divisor value returned by calc_divisor()
Signed-off-by: Hugo Villeneuve hugo.villeneuve@lyrtech.com
---
drivers/serial/serial.c | 12 +++--------- 1 files changed, 3 insertions(+), 9 deletions(-)
diff --git a/drivers/serial/serial.c b/drivers/serial/serial.c index 4ccaee2..8bbfcf9 100644 --- a/drivers/serial/serial.c +++ b/drivers/serial/serial.c @@ -124,8 +124,6 @@ static NS16550_t serial_ports[4] = {
static int calc_divisor (NS16550_t port) { - uint32_t clk_divisor; - #ifdef CONFIG_OMAP1510 /* If can't cleanly clock 115200 set div to 1 */ if ((CFG_NS16550_CLK == 12000000) && (gd->baudrate == 115200)) { @@ -149,15 +147,11 @@ static int calc_divisor (NS16550_t port)
/* Compute divisor value. Normally, we should simply return: * CFG_NS16550_CLK) / MODE_X_DIV / gd->baudrate - * but we need to round that value by adding 0.5 (2/4). + * but we need to round that value by adding 0.5. * Rounding is especially important at high baud rates. */ - clk_divisor = (((4 * CFG_NS16550_CLK) / - (MODE_X_DIV * gd->baudrate)) + 2) / 4; - - debug("NS16550 clock divisor = %d\n", clk_divisor); - - return clk_divisor; + return (CFG_NS16550_CLK + (gd->baudrate * (MODE_X_DIV / 2))) / + (MODE_X_DIV * gd->baudrate); }
#if !defined(CONFIG_SERIAL_MULTI)
Hugo Villeneuve wrote:
Round the serial port clock divisor value returned by calc_divisor()
Signed-off-by: Hugo Villeneuve hugo.villeneuve@lyrtech.com
Acked-by: Gerald Van Baren vanbaren@cideas.com
(Hopefully this stops Wolfgang's pain!)
drivers/serial/serial.c | 12 +++--------- 1 files changed, 3 insertions(+), 9 deletions(-)
diff --git a/drivers/serial/serial.c b/drivers/serial/serial.c index 4ccaee2..8bbfcf9 100644 --- a/drivers/serial/serial.c +++ b/drivers/serial/serial.c @@ -124,8 +124,6 @@ static NS16550_t serial_ports[4] = {
static int calc_divisor (NS16550_t port) {
- uint32_t clk_divisor;
#ifdef CONFIG_OMAP1510 /* If can't cleanly clock 115200 set div to 1 */ if ((CFG_NS16550_CLK == 12000000) && (gd->baudrate == 115200)) { @@ -149,15 +147,11 @@ static int calc_divisor (NS16550_t port)
/* Compute divisor value. Normally, we should simply return: * CFG_NS16550_CLK) / MODE_X_DIV / gd->baudrate
* but we need to round that value by adding 0.5 (2/4).
* but we need to round that value by adding 0.5.*/
- Rounding is especially important at high baud rates.
- clk_divisor = (((4 * CFG_NS16550_CLK) /
(MODE_X_DIV * gd->baudrate)) + 2) / 4;- debug("NS16550 clock divisor = %d\n", clk_divisor);
- return clk_divisor;
- return (CFG_NS16550_CLK + (gd->baudrate * (MODE_X_DIV / 2))) /
(MODE_X_DIV * gd->baudrate);}
#if !defined(CONFIG_SERIAL_MULTI)
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge Build the coolest Linux based applications with Moblin SDK & win great prizes Grand prize is a trip for two to an Open Source event anywhere in the world http://moblin-contest.org/redirect.php?banner_id=100&url=/ _______________________________________________ U-Boot-Users mailing list U-Boot-Users@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/u-boot-users
In message 487CC7FF.2000700@ge.com you wrote:
Hugo Villeneuve wrote:
Round the serial port clock divisor value returned by calc_divisor()
Signed-off-by: Hugo Villeneuve hugo.villeneuve@lyrtech.com
Acked-by: Gerald Van Baren vanbaren@cideas.com
(Hopefully this stops Wolfgang's pain!)
Thanks for bringing this to a good end.
Best regards,
Wolfgang Denk
In message 1216135382-21987-1-git-send-email-hugo.villeneuve@lyrtech.com you wrote:
Round the serial port clock divisor value returned by calc_divisor()
Signed-off-by: Hugo Villeneuve hugo.villeneuve@lyrtech.com
drivers/serial/serial.c | 12 +++--------- 1 files changed, 3 insertions(+), 9 deletions(-)
Applied, thanks.
Best regards,
Wolfgang Denk
participants (3)
-
Hugo Villeneuve -
Jerry Van Baren -
Wolfgang Denk