[U-Boot] [PATCH 0/3] Collection of ns16550 UART driver refactoring

[U-Boot] [PATCH 0/3] Collection of ns16550 UART driver refactoring

[Masahiro Yamada]

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(-)

-- 
1.9.1

[U-Boot] [PATCH 1/3] serial: ns16550: drop CONFIG_OMAP1610 from the special case

[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

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 <yamada.m@jp.panasonic.com>
Cc: Tom Rini <trini@ti.com>
Cc: Rishi Bhattacharya <rishi@ti.com>
---

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

diff --git a/drivers/serial/serial_ns16550.c b/drivers/serial/serial_ns16550.c
index ba68d46..056ef2a 100644
--- a/drivers/serial/serial_ns16550.c
+++ b/drivers/serial/serial_ns16550.c
@@ -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:
-- 
1.9.1

[U-Boot] [PATCH 2/3] serial: ns16550: use DIV_ROUND_CLOSEST macro to compute the divisor

[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 <yamada.m@jp.panasonic.com>
---

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

diff --git a/drivers/serial/serial_ns16550.c b/drivers/serial/serial_ns16550.c
index 056ef2a..49e2c1f 100644
--- a/drivers/serial/serial_ns16550.c
+++ b/drivers/serial/serial_ns16550.c
@@ -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
-- 
1.9.1

[U-Boot] [PATCH 3/3] serial: ns16550: use a const variable instead of macro

[Masahiro Yamada]

Just for type checking.

Signed-off-by: Masahiro Yamada <yamada.m@jp.panasonic.com>
Cc: Marek Vasut <marex@denx.de>
---

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

diff --git a/drivers/serial/serial_ns16550.c b/drivers/serial/serial_ns16550.c
index 49e2c1f..4413e69 100644
--- a/drivers/serial/serial_ns16550.c
+++ b/drivers/serial/serial_ns16550.c
@@ -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
-- 
1.9.1

[U-Boot] [PATCH 3/3] serial: ns16550: use a const variable instead of macro

[Marek Vasut]

On Friday, July 11, 2014 at 01:29:04 PM, Masahiro Yamada wrote:
> Just for type checking.
> 
> Signed-off-by: Masahiro Yamada <yamada.m@jp.panasonic.com>
> Cc: Marek Vasut <marex@denx.de>

Acked-by: Marek Vasut <marex@denx.de>

Thanks !

Best regards,
Marek Vasut

[U-Boot] [PATCH 1/3] serial: ns16550: drop CONFIG_OMAP1610 from the special case

[Masahiro Yamada]

Hi Tom,



On Fri, 11 Jul 2014 20:29:02 +0900
Masahiro Yamada <yamada.m@jp.panasonic.com> wrote:

> 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 <yamada.m@jp.panasonic.com>
> Cc: Tom Rini <trini@ti.com>
> Cc: Rishi Bhattacharya <rishi@ti.com>


The mail to "Rishi Bhattacharya <rishi@ti.com>"
has been bouncing.


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


Best Regards
Masahiro Yamada

[U-Boot] [U-Boot, 1/3] serial: ns16550: drop CONFIG_OMAP1610 from the special case

[Tom Rini]

On Fri, Jul 11, 2014 at 08:29:02PM +0900, Masahiro Yamada wrote:

> 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 <yamada.m@jp.panasonic.com>
> Cc: Tom Rini <trini@ti.com>
> Cc: Rishi Bhattacharya <rishi@ti.com>

Applied to u-boot/master, thanks!

-- 
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>

[U-Boot] [U-Boot, 2/3] serial: ns16550: use DIV_ROUND_CLOSEST macro to compute the divisor

[Tom Rini]

On Fri, Jul 11, 2014 at 08:29:03PM +0900, Masahiro Yamada wrote:

> 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 <yamada.m@jp.panasonic.com>

Applied to u-boot/master, thanks!

-- 
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>

[U-Boot] [U-Boot, 3/3] serial: ns16550: use a const variable instead of macro

[Tom Rini]

On Fri, Jul 11, 2014 at 08:29:04PM +0900, Masahiro Yamada wrote:

> Just for type checking.
> 
> Signed-off-by: Masahiro Yamada <yamada.m@jp.panasonic.com>
> Cc: Marek Vasut <marex@denx.de>
> Acked-by: Marek Vasut <marex@denx.de>

Applied to u-boot/master, thanks!

-- 
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>