Collection of ns16550 UART driver refactoring

PATCH New Masahiro Yamada 3 participants 9 messages Jul 11, 2014 – Jul 22, 2014

Share & tools

Masahiro Yamada

author

2014-07-11 11:29 UTC #0 lore raw

cover Collection of ns16550 UART driver refactoring

Masahiro Yamada (3):
serial: ns16550: drop CONFIG_OMAP1610 from the special case
serial: ns16550: use DIV_ROUND_CLOSEST macro to compute the divisor
serial: ns16550: use a const variable instead of macro

 drivers/serial/serial_ns16550.c | 18 ++++--------------
 1 file changed, 4 insertions(+), 14 deletions(-)

Signature · 1 line

1.9.1

Masahiro Yamada

author

2014-07-11 11:29 UTC #1 lore raw

PATCH 1/3 serial: ns16550: drop CONFIG_OMAP1610 from the special case

If CONFIG_OMAP1610 is defined, the code returning the fixed value (26) is enabled. But this case is covered by the following code.

(CONFIG_SYS_NS16550_CLK + (gd->baudrate * (MODE_X_DIV / 2))) /

    (MODE_X_DIV * gd->baudrate)

= (48000000 + (115200 * (16 / 2))) / (16 * 115200) = 48921600 / 1843200
= 26

The "#ifdef CONFIG_OMAP1610" was added by commit 6f21347d more than ten years ago. In those days, the divide-and-round was not used. I guess that is why this weird code was added here.

Signed-off-by Masahiro Yamada <[email protected]>

Cc Tom Rini <[email protected]>

Cc Rishi Bhattacharya <[email protected]>

---

 drivers/serial/serial_ns16550.c | 6 ------
 1 file changed, 6 deletions(-)

drivers/serial/serial_ns16550.c +0 -6

index ba68d46..056ef2a 100644
@@ -128,12 +128,6 @@ static int calc_divisor (NS16550_t port)
 	}
 	port->osc_12m_sel = 0;			/* clear if previsouly set */
 #endif
-#ifdef CONFIG_OMAP1610
-	/* If can't cleanly clock 115200 set div to 1 */
-	if ((CONFIG_SYS_NS16550_CLK == 48000000) && (gd->baudrate == 115200)) {
-		return (26);		/* return 26 for base divisor */
-	}
-#endif
 
 #define MODE_X_DIV 16
 	/* Compute divisor value. Normally, we should simply return:

Signature · 1 line

1.9.1

Masahiro Yamada

author

2014-07-11 11:52 UTC #2 lore raw

PATCH 1/3 serial: ns16550: drop CONFIG_OMAP1610 from the special case

Hi Tom,

Quoted 9 lines · On Fri, 11 Jul 2014 20:29:02 +0900 Masahiro Yamada <[email protected]>…

If CONFIG_OMAP1610 is defined, the code returning the fixed value (26) is enabled. But this case is covered by the following code.
(CONFIG_SYS_NS16550_CLK + (gd->baudrate * (MODE_X_DIV / 2))) /
    (MODE_X_DIV * gd->baudrate)
= (48000000 + (115200 * (16 / 2))) / (16 * 115200) = 48921600 / 1843200
= 26

Quoted 7 lines

The "#ifdef CONFIG_OMAP1610" was added by commit 6f21347d more than ten years ago. In those days, the divide-and-round was not used. I guess that is why this weird code was added here.
Signed-off-by Masahiro Yamada <[email protected]>
Cc Tom Rini <[email protected]>
Cc Rishi Bhattacharya <[email protected]>

The mail to "Rishi Bhattacharya <[email protected]>"
has been bouncing.

Could you assign a new maintainer for omap5912osk board? Or is this already a dead board?

Best Regards
Masahiro Yamada

Tom Rini

2014-07-22 19:22 UTC #3 lore raw

PATCH 1/3 serial: ns16550: drop CONFIG_OMAP1610 from the special case

In reply to Masahiro Yamada

Quoted 9 lines · Masahiro Yamada

If CONFIG_OMAP1610 is defined, the code returning the fixed value (26) is enabled. But this case is covered by the following code.
(CONFIG_SYS_NS16550_CLK + (gd->baudrate * (MODE_X_DIV / 2))) /
    (MODE_X_DIV * gd->baudrate)
= (48000000 + (115200 * (16 / 2))) / (16 * 115200) = 48921600 / 1843200
= 26

Quoted 7 lines

The "#ifdef CONFIG_OMAP1610" was added by commit 6f21347d more than ten years ago. In those days, the divide-and-round was not used. I guess that is why this weird code was added here.
Signed-off-by Masahiro Yamada <[email protected]>
Cc Tom Rini <[email protected]>
Cc Rishi Bhattacharya <[email protected]>

Applied to u-boot/master, thanks!

Signature · 8 lines

Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20140722/690d155e/attachment.pgp>

Masahiro Yamada

author

2014-07-11 11:29 UTC #4 lore raw

PATCH 2/3 serial: ns16550: use DIV_ROUND_CLOSEST macro to compute the divisor

In reply to Masahiro Yamada

The function still returns the same value.

The comment block is no longer necessary because our intention is clear enough by using DIV_ROUND_CLOSEST() macro.

Signed-off-by Masahiro Yamada <[email protected]>

---

 drivers/serial/serial_ns16550.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

drivers/serial/serial_ns16550.c +3 -7

index 056ef2a..49e2c1f 100644
@@ -130,13 +130,9 @@ static int calc_divisor (NS16550_t port)
 #endif
 
 #define MODE_X_DIV 16
-	/* Compute divisor value. Normally, we should simply return:
-	 *   CONFIG_SYS_NS16550_CLK) / MODE_X_DIV / gd->baudrate
-	 * but we need to round that value by adding 0.5.
-	 * Rounding is especially important at high baud rates.
-	 */
-	return (CONFIG_SYS_NS16550_CLK + (gd->baudrate * (MODE_X_DIV / 2))) /
-		(MODE_X_DIV * gd->baudrate);
+
+	return DIV_ROUND_CLOSEST(CONFIG_SYS_NS16550_CLK,
+						MODE_X_DIV * gd->baudrate);
 }
 
 void

Signature · 1 line

1.9.1

Tom Rini

2014-07-22 19:22 UTC #5 lore raw

PATCH 2/3 serial: ns16550: use DIV_ROUND_CLOSEST macro to compute the divisor

Quoted 6 lines · Masahiro Yamada

The function still returns the same value.
The comment block is no longer necessary because our intention is clear enough by using DIV_ROUND_CLOSEST() macro.
Signed-off-by Masahiro Yamada <[email protected]>

Applied to u-boot/master, thanks!

Signature · 8 lines

Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20140722/4fa9269a/attachment.pgp>

Masahiro Yamada

author

2014-07-11 11:29 UTC #6 lore raw

PATCH 3/3 serial: ns16550: use a const variable instead of macro

In reply to Masahiro Yamada

Just for type checking.

Signed-off-by Masahiro Yamada <[email protected]>

Cc Marek Vasut <[email protected]>

---

 drivers/serial/serial_ns16550.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

drivers/serial/serial_ns16550.c +3 -3

index 49e2c1f..4413e69 100644
@@ -120,6 +120,8 @@ static NS16550_t serial_ports[6] = {
 
 static int calc_divisor (NS16550_t port)
 {
+	const unsigned int mode_x_div = 16;
+
 #ifdef CONFIG_OMAP1510
 	/* If can't cleanly clock 115200 set div to 1 */
 	if ((CONFIG_SYS_NS16550_CLK == 12000000) && (gd->baudrate == 115200)) {

@@ -129,10 +131,8 @@ static int calc_divisor (NS16550_t port)
 	port->osc_12m_sel = 0;			/* clear if previsouly set */
 #endif
 
-#define MODE_X_DIV 16
-
 	return DIV_ROUND_CLOSEST(CONFIG_SYS_NS16550_CLK,
-						MODE_X_DIV * gd->baudrate);
+						mode_x_div * gd->baudrate);
 }
 
 void

Signature · 1 line

1.9.1

Marek Vasut

2014-07-11 11:50 UTC #7 lore raw

PATCH 3/3 serial: ns16550: use a const variable instead of macro

Quoted 4 lines · Masahiro Yamada

Just for type checking.
Signed-off-by Masahiro Yamada <[email protected]>
Cc Marek Vasut <[email protected]>

Acked-by Marek Vasut <[email protected]>

Thanks !

Best regards,
Marek Vasut

Tom Rini

2014-07-22 19:22 UTC #8 lore raw

PATCH 3/3 serial: ns16550: use a const variable instead of macro

In reply to Masahiro Yamada

Quoted 5 lines · Masahiro Yamada

Just for type checking.
Signed-off-by Masahiro Yamada <[email protected]>
Cc Marek Vasut <[email protected]>
Acked-by Marek Vasut <[email protected]>

Applied to u-boot/master, thanks!

Signature · 8 lines

Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20140722/0f1f819d/attachment.pgp>