Skip to contentSkip to conversation

PL01x: baudrate & line control fixes

PATCH New Vikas Manocha 2 participants 11 messages Nov 21, 2014 – Dec 8, 2014

lore
Share & tools
lore
#0 lore raw

v2 cover PL01x: baudrate & line control fixes

This patchset fixes the pl01x driver esp for pl011 baudrate & line control.

Changes in v2:
- fix the comment style

Vikas Manocha (5):
serial: pl01x: pass pl01x_type to set baudrate
serial: pl01x: fix pl011 baud rate configuration
serial: pl01x: move all line control at same place
serial: pl01x: disable as per type of pl01x
serial: pl01x: avoid pl01x type check two times

 drivers/serial/serial_pl01x.c |   48 ++++++++++++++++++++++-------------------
 1 file changed, 26 insertions(+), 22 deletions(-)
Signature · 1 line
1.7.9.5
#1 lore raw

PATCH v2 1/5 serial: pl01x: pass pl01x_type to set baudrate

Although we were checking the pl01x type, seems like PL010 type was being passed by mistake.

---

Changes in v2: None

 drivers/serial/serial_pl01x.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/serial/serial_pl01x.c +1 -1
index 38dda91..1860289 100644
@@ -201,7 +201,7 @@ static void pl01x_serial_init_baud(int baudrate)
 	base_regs = (struct pl01x_regs *)port[CONFIG_CONS_INDEX];
 
 	pl01x_generic_serial_init(base_regs, pl01x_type);
-	pl01x_generic_setbrg(base_regs, TYPE_PL010, clock, baudrate);
+	pl01x_generic_setbrg(base_regs, pl01x_type, clock, baudrate);
 }
 
 /*
Signature · 1 line
1.7.9.5
#2 lore raw

PATCH v2 1/5 serial: pl01x: pass pl01x_type to set baudrate

Quoted 5 lines · Vikas Manocha

Although we were checking the pl01x type, seems like PL010 type was being passed by mistake.

Applied to u-boot/master, thanks!

Signature · 8 lines
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20141208/7a667a13/attachment.pgp>
#3 lore raw

PATCH v2 2/5 serial: pl01x: fix pl011 baud rate configuration

In reply to Vikas Manocha

UART_IBRD, UART_FBRD, and UART_LCR_H form a single 30-bit wide register which is updated on a single write strobe generated by a UART_LCR_H write. So, to internally update the content of UART_IBRD or UART_FBRD, a write to UART_LCR_H must always be performed at the end.

---

Changes in v2:
- fixed comment style

 drivers/serial/serial_pl01x.c |    8 ++++++++
 1 file changed, 8 insertions(+)
drivers/serial/serial_pl01x.c +8 -0
index 1860289..a58ad8a 100644
@@ -122,6 +122,7 @@ static int pl01x_generic_serial_init(struct pl01x_regs *regs,
 static int pl01x_generic_setbrg(struct pl01x_regs *regs, enum pl01x_type type,
 				int clock, int baudrate)
 {
+	unsigned int lcr;
 	switch (type) {
 	case TYPE_PL010: {
 		unsigned int divisor;
@@ -175,6 +176,13 @@ static int pl01x_generic_setbrg(struct pl01x_regs *regs, enum pl01x_type type,
 		writel(divider, &regs->pl011_ibrd);
 		writel(fraction, &regs->pl011_fbrd);
 
+		/*
+		 * Internal update of baud rate register require line
+		 * control register write
+		 */
+		lcr = UART_PL011_LCRH_WLEN_8 | UART_PL011_LCRH_FEN;
+		writel(lcr, &regs->pl011_lcrh);
+
 		/* Finally, enable the UART */
 		writel(UART_PL011_CR_UARTEN | UART_PL011_CR_TXE |
 		       UART_PL011_CR_RXE | UART_PL011_CR_RTS, &regs->pl011_cr);
Signature · 1 line
1.7.9.5
#4 lore raw

PATCH v2 2/5 serial: pl01x: fix pl011 baud rate configuration

Quoted 7 lines · Vikas Manocha

UART_IBRD, UART_FBRD, and UART_LCR_H form a single 30-bit wide register which is updated on a single write strobe generated by a UART_LCR_H write. So, to internally update the content of UART_IBRD or UART_FBRD, a write to UART_LCR_H must always be performed at the end.

Applied to u-boot/master, thanks!

Signature · 8 lines
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20141208/c1fd91f1/attachment.pgp>
#5 lore raw

PATCH v2 3/5 serial: pl01x: move all line control at same place

In reply to Vikas Manocha

Receive line control uses same setting as transmit line control, also one lcrh write is effective for both baud rate & receive line control internal update.

---

Changes in v2:
- fixed comment style

 drivers/serial/serial_pl01x.c |   44 ++++++++++++++++++++---------------------
 1 file changed, 21 insertions(+), 23 deletions(-)
drivers/serial/serial_pl01x.c +21 -23
index a58ad8a..fb11210 100644
@@ -72,8 +72,6 @@ static int pl01x_tstc(struct pl01x_regs *regs)
 static int pl01x_generic_serial_init(struct pl01x_regs *regs,
 				     enum pl01x_type type)
 {
-	unsigned int lcr;
-
 #ifdef CONFIG_PL011_SERIAL_FLUSH_ON_INIT
 	if (type == TYPE_PL011) {
 		/* Empty RX fifo if necessary */
@@ -87,15 +85,28 @@ static int pl01x_generic_serial_init(struct pl01x_regs *regs,
 	/* First, disable everything */
 	writel(0, &regs->pl010_cr);
 
-	/* Set the UART to be 8 bits, 1 stop bit, no parity, fifo enabled */
-	lcr = UART_PL011_LCRH_WLEN_8 | UART_PL011_LCRH_FEN;
-	writel(lcr, &regs->pl011_lcrh);
-
 	switch (type) {
 	case TYPE_PL010:
 		break;
-	case TYPE_PL011: {
+	case TYPE_PL011:
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int set_line_control(struct pl01x_regs *regs)
+{
+	unsigned int lcr;
+	/*
+	 * Internal update of baud rate register require line
+	 * control register write
+	 */
+	lcr = UART_PL011_LCRH_WLEN_8 | UART_PL011_LCRH_FEN;
 #ifdef CONFIG_PL011_SERIAL_RLCR
+	{
 		int i;
 
 		/*
@@ -107,22 +118,15 @@ static int pl01x_generic_serial_init(struct pl01x_regs *regs,
 			writel(lcr, &regs->fr);
 
 		writel(lcr, &regs->pl011_rlcr);
-		/* lcrh needs to be set again for change to be effective */
-		writel(lcr, &regs->pl011_lcrh);
-#endif
-		break;
-	}
-	default:
-		return -EINVAL;
 	}
-
+#endif
+	writel(lcr, &regs->pl011_lcrh);
 	return 0;
 }
 
 static int pl01x_generic_setbrg(struct pl01x_regs *regs, enum pl01x_type type,
 				int clock, int baudrate)
 {
-	unsigned int lcr;
 	switch (type) {
 	case TYPE_PL010: {
 		unsigned int divisor;
@@ -176,13 +180,7 @@ static int pl01x_generic_setbrg(struct pl01x_regs *regs, enum pl01x_type type,
 		writel(divider, &regs->pl011_ibrd);
 		writel(fraction, &regs->pl011_fbrd);
 
-		/*
-		 * Internal update of baud rate register require line
-		 * control register write
-		 */
-		lcr = UART_PL011_LCRH_WLEN_8 | UART_PL011_LCRH_FEN;
-		writel(lcr, &regs->pl011_lcrh);
-
+		set_line_control(regs);
 		/* Finally, enable the UART */
 		writel(UART_PL011_CR_UARTEN | UART_PL011_CR_TXE |
 		       UART_PL011_CR_RXE | UART_PL011_CR_RTS, &regs->pl011_cr);
Signature · 1 line
1.7.9.5
#6 lore raw

PATCH v2 3/5 serial: pl01x: move all line control at same place

Quoted 5 lines · Vikas Manocha

Receive line control uses same setting as transmit line control, also one lcrh write is effective for both baud rate & receive line control internal update.

Applied to u-boot/master, thanks!

Signature · 8 lines
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20141208/1006c04d/attachment.pgp>
#7 lore raw

PATCH v2 4/5 serial: pl01x: disable as per type of pl01x

In reply to Vikas Manocha

pl010 & pl011 have different control register offsets, setting it as per the pl01x type.

---

Changes in v2: None

 drivers/serial/serial_pl01x.c |    7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)
drivers/serial/serial_pl01x.c +4 -3
index fb11210..2a6b61d 100644
@@ -82,13 +82,14 @@ static int pl01x_generic_serial_init(struct pl01x_regs *regs,
 	}
 #endif
 
-	/* First, disable everything */
-	writel(0, &regs->pl010_cr);
-
 	switch (type) {
 	case TYPE_PL010:
+		/* disable everything */
+		writel(0, &regs->pl010_cr);
 		break;
 	case TYPE_PL011:
+		/* disable everything */
+		writel(0, &regs->pl011_cr);
 		break;
 	default:
 		return -EINVAL;
Signature · 1 line
1.7.9.5
#8 lore raw

PATCH v2 4/5 serial: pl01x: disable as per type of pl01x

Quoted 5 lines · Vikas Manocha

pl010 & pl011 have different control register offsets, setting it as per the pl01x type.

Applied to u-boot/master, thanks!

Signature · 8 lines
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20141208/c8004bce/attachment.pgp>
#9 lore raw

PATCH v2 5/5 serial: pl01x: avoid pl01x type check two times

In reply to Vikas Manocha

---

Changes in v2: None

 drivers/serial/serial_pl01x.c |   15 ++++++---------
 1 file changed, 6 insertions(+), 9 deletions(-)
drivers/serial/serial_pl01x.c +6 -9
index 2a6b61d..e1bf496 100644
@@ -72,22 +72,19 @@ static int pl01x_tstc(struct pl01x_regs *regs)
 static int pl01x_generic_serial_init(struct pl01x_regs *regs,
 				     enum pl01x_type type)
 {
+	switch (type) {
+	case TYPE_PL010:
+		/* disable everything */
+		writel(0, &regs->pl010_cr);
+		break;
+	case TYPE_PL011:
 #ifdef CONFIG_PL011_SERIAL_FLUSH_ON_INIT
-	if (type == TYPE_PL011) {
 		/* Empty RX fifo if necessary */
 		if (readl(&regs->pl011_cr) & UART_PL011_CR_UARTEN) {
 			while (!(readl(&regs->fr) & UART_PL01x_FR_RXFE))
 				readl(&regs->dr);
 		}
-	}
 #endif
-
-	switch (type) {
-	case TYPE_PL010:
-		/* disable everything */
-		writel(0, &regs->pl010_cr);
-		break;
-	case TYPE_PL011:
 		/* disable everything */
 		writel(0, &regs->pl011_cr);
 		break;
Signature · 1 line
1.7.9.5
#10 lore raw

PATCH v2 5/5 serial: pl01x: avoid pl01x type check two times

Quoted 2 lines · Vikas Manocha

Applied to u-boot/master, thanks!

Signature · 8 lines
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20141208/49b15a5e/attachment.pgp>