[U-Boot-Users] [PATCH] Fix integer overflow warning in calc_divisor()
Fix the integer overflow warning when rounding the serial port clock divisor value in calc_divisor().
Signed-off-by: Hugo Villeneuve hugo.villeneuve@lyrtech.com
---
drivers/serial/serial.c | 11 +++++++++-- 1 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/drivers/serial/serial.c b/drivers/serial/serial.c index 182ca2d..4ccaee2 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) { + 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)) { @@ -147,10 +149,15 @@ 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 or 8/16. + * but we need to round that value by adding 0.5 (2/4). * Rounding is especially important at high baud rates. */ - return (((16 * CFG_NS16550_CLK) / MODE_X_DIV / gd->baudrate) + 8) / 16; + clk_divisor = (((4 * CFG_NS16550_CLK) / + (MODE_X_DIV * gd->baudrate)) + 2) / 4; + + debug("NS16550 clock divisor = %d\n", clk_divisor); + + return clk_divisor; }
#if !defined(CONFIG_SERIAL_MULTI)
In message 1215786255-4241-1-git-send-email-hugo.villeneuve@lyrtech.com you wrote:
Fix the integer overflow warning when rounding the serial port clock divisor value in calc_divisor().
Signed-off-by: Hugo Villeneuve hugo.villeneuve@lyrtech.com
drivers/serial/serial.c | 11 +++++++++-- 1 files changed, 9 insertions(+), 2 deletions(-)
Applied, thanks, but I really wish you had kept the subject unchanged, and ideally even threading correct.
Best regards,
Wolfgang Denk
On Sun, 2008-07-13 at 15:14 +0200, Wolfgang Denk wrote:
In message 1215786255-4241-1-git-send-email-hugo.villeneuve@lyrtech.com you wrote:
Fix the integer overflow warning when rounding the serial port clock divisor value in calc_divisor().
Signed-off-by: Hugo Villeneuve hugo.villeneuve@lyrtech.com
drivers/serial/serial.c | 11 +++++++++-- 1 files changed, 9 insertions(+), 2 deletions(-)
Applied, thanks, but I really wish you had kept the subject unchanged, and ideally even threading correct.
Hi Wolfgang, since you already applied the first patch, I simply thought that the fix for the first patch should have a different subject.
I will keep the same subject in the future
Hugo V.
In message 1216047507.12227.2.camel@server.hugovil.com you wrote:
since you already applied the first patch, I simply thought that the fix for the first patch should have a different subject.
I will keep the same subject in the future
Thanks. You have another try anyway ;-)
The bug is not fixed yet:
Configuring for mpc7448hpc2 board... serial.c: In function 'calc_divisor': serial.c:155: warning: integer overflow in expression
Best regards,
Wolfgang Denk
This formula is better at avoiding integer overflow.
Signed-off-by: Gerald Van Baren vanbaren@cideas.com ---
This is my latest entry in the baud rate rounding dual. Since it doesn't multiply the master BRG clock but instead adds the baud rate scaled by 1/2 the clock multiplier, it should not overflow (for a master clock right at the edge of overflowing itself, it still will overflow, but that is pretty unlikely).
This compiles OK on the mpc7448hpc2. I have only tested it on a calculator. I have NOT tested it on real hardware.
drivers/serial/serial.c | 16 +++++----------- 1 files changed, 5 insertions(+), 11 deletions(-)
diff --git a/drivers/serial/serial.c b/drivers/serial/serial.c index 4ccaee2..7f43540 100644 --- a/drivers/serial/serial.c +++ b/drivers/serial/serial.c @@ -124,7 +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 */ @@ -147,17 +146,12 @@ static int calc_divisor (NS16550_t port) #define MODE_X_DIV 16 #endif
- /* 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). - * Rounding is especially important at high baud rates. + /* + * Compute divisor value, rounding it properly. 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)
u-boot-users-bounces@lists.sourceforge.net wrote:
This formula is better at avoiding integer overflow.
Signed-off-by: Gerald Van Baren vanbaren@cideas.com
This is my latest entry in the baud rate rounding dual. Since it doesn't multiply the master BRG clock but instead adds the baud rate scaled by 1/2 the clock multiplier, it should not overflow (for a master clock right at the edge of overflowing itself, it still will overflow, but that is pretty unlikely).
This compiles OK on the mpc7448hpc2. I have only tested it on a calculator. I have NOT tested it on real hardware.
I have tested it on my board and it works.
I also tested it on my board by using the clock value of the mpc7448hpc2 board, and printing the computed value which was correct.
drivers/serial/serial.c | 16 +++++----------- 1 files changed, 5 insertions(+), 11 deletions(-) ...
- /* 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).* Rounding is especially important at high baud rates.
- /*
* Compute divisor value, rounding it properly. Rounding is* especially important at high baud rates.
Please keep the comments explaining the base formula: it is not easy to reverse-engineer an optimized formula to figure out what is the base formula.
I will resubmit a patch with your fix and my comments.
Hugo V.
participants (3)
-
Hugo Villeneuve -
Jerry Van Baren -
Wolfgang Denk