public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
* [U-Boot] [PATCH 0/5] PL01x: baudrate & line control fixes
@ 2014-11-18  0:17 Vikas Manocha
  2014-11-18  0:17 ` [U-Boot] [PATCH 1/5] serial: pl01x: pass pl01x_type to set baudrate Vikas Manocha
                   ` (5 more replies)
  0 siblings, 6 replies; 17+ messages in thread
From: Vikas Manocha @ 2014-11-18  0:17 UTC (permalink / raw)
  To: u-boot

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

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 |   46 +++++++++++++++++++++--------------------
 1 file changed, 24 insertions(+), 22 deletions(-)

-- 
1.7.9.5

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [U-Boot] [PATCH 1/5] serial: pl01x: pass pl01x_type to set baudrate
  2014-11-18  0:17 [U-Boot] [PATCH 0/5] PL01x: baudrate & line control fixes Vikas Manocha
@ 2014-11-18  0:17 ` Vikas Manocha
  2014-11-18  5:25   ` Simon Glass
  2014-11-18  0:17 ` [U-Boot] [PATCH 2/5] serial: pl01x: fix pl011 baud rate configuration Vikas Manocha
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 17+ messages in thread
From: Vikas Manocha @ 2014-11-18  0:17 UTC (permalink / raw)
  To: u-boot

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

Signed-off-by: Vikas Manocha <vikas.manocha@st.com>
---
 drivers/serial/serial_pl01x.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/serial/serial_pl01x.c b/drivers/serial/serial_pl01x.c
index 38dda91..1860289 100644
--- a/drivers/serial/serial_pl01x.c
+++ b/drivers/serial/serial_pl01x.c
@@ -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);
 }
 
 /*
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [U-Boot] [PATCH 2/5] serial: pl01x: fix pl011 baud rate configuration
  2014-11-18  0:17 [U-Boot] [PATCH 0/5] PL01x: baudrate & line control fixes Vikas Manocha
  2014-11-18  0:17 ` [U-Boot] [PATCH 1/5] serial: pl01x: pass pl01x_type to set baudrate Vikas Manocha
@ 2014-11-18  0:17 ` Vikas Manocha
  2014-11-18  5:26   ` Simon Glass
  2014-11-18  0:17 ` [U-Boot] [PATCH 3/5] serial: pl01x: move all line control at same place Vikas Manocha
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 17+ messages in thread
From: Vikas Manocha @ 2014-11-18  0:17 UTC (permalink / raw)
  To: u-boot

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.

Signed-off-by: Vikas Manocha <vikas.manocha@st.com>
---
 drivers/serial/serial_pl01x.c |    6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/serial/serial_pl01x.c b/drivers/serial/serial_pl01x.c
index 1860289..c0531ca 100644
--- a/drivers/serial/serial_pl01x.c
+++ b/drivers/serial/serial_pl01x.c
@@ -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,11 @@ 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);
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [U-Boot] [PATCH 3/5] serial: pl01x: move all line control at same place
  2014-11-18  0:17 [U-Boot] [PATCH 0/5] PL01x: baudrate & line control fixes Vikas Manocha
  2014-11-18  0:17 ` [U-Boot] [PATCH 1/5] serial: pl01x: pass pl01x_type to set baudrate Vikas Manocha
  2014-11-18  0:17 ` [U-Boot] [PATCH 2/5] serial: pl01x: fix pl011 baud rate configuration Vikas Manocha
@ 2014-11-18  0:17 ` Vikas Manocha
  2014-11-18  5:27   ` Simon Glass
  2014-11-18  0:17 ` [U-Boot] [PATCH 4/5] serial: pl01x: disable as per type of pl01x Vikas Manocha
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 17+ messages in thread
From: Vikas Manocha @ 2014-11-18  0:17 UTC (permalink / raw)
  To: u-boot

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.

Signed-off-by: Vikas Manocha <vikas.manocha@st.com>
---
 drivers/serial/serial_pl01x.c |   40 +++++++++++++++++++---------------------
 1 file changed, 19 insertions(+), 21 deletions(-)

diff --git a/drivers/serial/serial_pl01x.c b/drivers/serial/serial_pl01x.c
index c0531ca..3155840 100644
--- a/drivers/serial/serial_pl01x.c
+++ b/drivers/serial/serial_pl01x.c
@@ -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,26 @@ 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 +116,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,11 +178,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);
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [U-Boot] [PATCH 4/5] serial: pl01x: disable as per type of pl01x
  2014-11-18  0:17 [U-Boot] [PATCH 0/5] PL01x: baudrate & line control fixes Vikas Manocha
                   ` (2 preceding siblings ...)
  2014-11-18  0:17 ` [U-Boot] [PATCH 3/5] serial: pl01x: move all line control at same place Vikas Manocha
@ 2014-11-18  0:17 ` Vikas Manocha
  2014-11-18  5:27   ` Simon Glass
  2014-11-18  0:17 ` [U-Boot] [PATCH 5/5] serial: pl01x: avoid pl01x type check two times Vikas Manocha
  2014-11-18  5:30 ` [U-Boot] [PATCH 0/5] PL01x: baudrate & line control fixes Simon Glass
  5 siblings, 1 reply; 17+ messages in thread
From: Vikas Manocha @ 2014-11-18  0:17 UTC (permalink / raw)
  To: u-boot

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

Signed-off-by: Vikas Manocha <vikas.manocha@st.com>
---
 drivers/serial/serial_pl01x.c |    7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/serial/serial_pl01x.c b/drivers/serial/serial_pl01x.c
index 3155840..758684f 100644
--- a/drivers/serial/serial_pl01x.c
+++ b/drivers/serial/serial_pl01x.c
@@ -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;
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [U-Boot] [PATCH 5/5] serial: pl01x: avoid pl01x type check two times
  2014-11-18  0:17 [U-Boot] [PATCH 0/5] PL01x: baudrate & line control fixes Vikas Manocha
                   ` (3 preceding siblings ...)
  2014-11-18  0:17 ` [U-Boot] [PATCH 4/5] serial: pl01x: disable as per type of pl01x Vikas Manocha
@ 2014-11-18  0:17 ` Vikas Manocha
  2014-11-18  5:28   ` Simon Glass
  2014-11-18  5:30 ` [U-Boot] [PATCH 0/5] PL01x: baudrate & line control fixes Simon Glass
  5 siblings, 1 reply; 17+ messages in thread
From: Vikas Manocha @ 2014-11-18  0:17 UTC (permalink / raw)
  To: u-boot

Signed-off-by: Vikas Manocha <vikas.manocha@st.com>
---
 drivers/serial/serial_pl01x.c |   15 ++++++---------
 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/drivers/serial/serial_pl01x.c b/drivers/serial/serial_pl01x.c
index 758684f..3fc1db5 100644
--- a/drivers/serial/serial_pl01x.c
+++ b/drivers/serial/serial_pl01x.c
@@ -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;
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [U-Boot] [PATCH 1/5] serial: pl01x: pass pl01x_type to set baudrate
  2014-11-18  0:17 ` [U-Boot] [PATCH 1/5] serial: pl01x: pass pl01x_type to set baudrate Vikas Manocha
@ 2014-11-18  5:25   ` Simon Glass
  0 siblings, 0 replies; 17+ messages in thread
From: Simon Glass @ 2014-11-18  5:25 UTC (permalink / raw)
  To: u-boot

On 18 November 2014 00:17, Vikas Manocha <vikas.manocha@st.com> wrote:
> Although we were checking the pl01x type, seems like PL010 type was being
> passed by mistake.
>
> Signed-off-by: Vikas Manocha <vikas.manocha@st.com>

Acked-by: Simon Glass <sjg@chromium.org>

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [U-Boot] [PATCH 2/5] serial: pl01x: fix pl011 baud rate configuration
  2014-11-18  0:17 ` [U-Boot] [PATCH 2/5] serial: pl01x: fix pl011 baud rate configuration Vikas Manocha
@ 2014-11-18  5:26   ` Simon Glass
  0 siblings, 0 replies; 17+ messages in thread
From: Simon Glass @ 2014-11-18  5:26 UTC (permalink / raw)
  To: u-boot

On 18 November 2014 00:17, Vikas Manocha <vikas.manocha@st.com> wrote:
> 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.
>
> Signed-off-by: Vikas Manocha <vikas.manocha@st.com>

Acked-by: Simon Glass <sjg@chromium.org>

but see below.

> ---
>  drivers/serial/serial_pl01x.c |    6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/drivers/serial/serial_pl01x.c b/drivers/serial/serial_pl01x.c
> index 1860289..c0531ca 100644
> --- a/drivers/serial/serial_pl01x.c
> +++ b/drivers/serial/serial_pl01x.c
> @@ -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,11 @@ 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 */

You may as well fix the comment style at the same time:

/*
 * Internal update...
 * control ...
 */

> +               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);
> --

Regards,
Simon

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [U-Boot] [PATCH 3/5] serial: pl01x: move all line control at same place
  2014-11-18  0:17 ` [U-Boot] [PATCH 3/5] serial: pl01x: move all line control at same place Vikas Manocha
@ 2014-11-18  5:27   ` Simon Glass
  0 siblings, 0 replies; 17+ messages in thread
From: Simon Glass @ 2014-11-18  5:27 UTC (permalink / raw)
  To: u-boot

On 18 November 2014 00:17, Vikas Manocha <vikas.manocha@st.com> wrote:
> 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.
>
> Signed-off-by: Vikas Manocha <vikas.manocha@st.com>

Seems cleaner to me.

Acked-by: Simon Glass <sjg@chromium.org>

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [U-Boot] [PATCH 4/5] serial: pl01x: disable as per type of pl01x
  2014-11-18  0:17 ` [U-Boot] [PATCH 4/5] serial: pl01x: disable as per type of pl01x Vikas Manocha
@ 2014-11-18  5:27   ` Simon Glass
  0 siblings, 0 replies; 17+ messages in thread
From: Simon Glass @ 2014-11-18  5:27 UTC (permalink / raw)
  To: u-boot

On 18 November 2014 00:17, Vikas Manocha <vikas.manocha@st.com> wrote:
> pl010 & pl011 have different control register offsets, setting it as per
> the pl01x type.
>
> Signed-off-by: Vikas Manocha <vikas.manocha@st.com>

Acked-by: Simon Glass <sjg@chromium.org>

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [U-Boot] [PATCH 5/5] serial: pl01x: avoid pl01x type check two times
  2014-11-18  0:17 ` [U-Boot] [PATCH 5/5] serial: pl01x: avoid pl01x type check two times Vikas Manocha
@ 2014-11-18  5:28   ` Simon Glass
  0 siblings, 0 replies; 17+ messages in thread
From: Simon Glass @ 2014-11-18  5:28 UTC (permalink / raw)
  To: u-boot

On 18 November 2014 00:17, Vikas Manocha <vikas.manocha@st.com> wrote:
> Signed-off-by: Vikas Manocha <vikas.manocha@st.com>

Acked-by: Simon Glass <sjg@chromium.org>

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [U-Boot] [PATCH 0/5] PL01x: baudrate & line control fixes
  2014-11-18  0:17 [U-Boot] [PATCH 0/5] PL01x: baudrate & line control fixes Vikas Manocha
                   ` (4 preceding siblings ...)
  2014-11-18  0:17 ` [U-Boot] [PATCH 5/5] serial: pl01x: avoid pl01x type check two times Vikas Manocha
@ 2014-11-18  5:30 ` Simon Glass
  2014-11-18 18:59   ` vikasm
  5 siblings, 1 reply; 17+ messages in thread
From: Simon Glass @ 2014-11-18  5:30 UTC (permalink / raw)
  To: u-boot

Hi Vikas,

On 18 November 2014 00:17, Vikas Manocha <vikas.manocha@st.com> wrote:
> This patchset fixes the pl01x driver esp for pl011 baudrate & line
> control.
>
> 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 |   46 +++++++++++++++++++++--------------------
>  1 file changed, 24 insertions(+), 22 deletions(-)

Great to see this series. If you have not tested on p1010 I can do
this in a week or so (am travelling). At least a few of these looks
like bug fixes to my recent refactor so I could bring them through the
DM tree if no one else picks them up.

Regards,
Simon

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [U-Boot] [PATCH 0/5] PL01x: baudrate & line control fixes
  2014-11-18  5:30 ` [U-Boot] [PATCH 0/5] PL01x: baudrate & line control fixes Simon Glass
@ 2014-11-18 18:59   ` vikasm
  2014-11-24 15:51     ` Simon Glass
  0 siblings, 1 reply; 17+ messages in thread
From: vikasm @ 2014-11-18 18:59 UTC (permalink / raw)
  To: u-boot

Thanks Simon,

On 11/17/2014 09:30 PM, Simon Glass wrote:
> Hi Vikas,
>
> On 18 November 2014 00:17, Vikas Manocha <vikas.manocha@st.com> wrote:
>> This patchset fixes the pl01x driver esp for pl011 baudrate & line
>> control.
>>
>> 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 |   46 +++++++++++++++++++++--------------------
>>   1 file changed, 24 insertions(+), 22 deletions(-)
> Great to see this series. If you have not tested on p1010 I can do
> this in a week or so (am travelling). At least a few of these looks
> like bug fixes to my recent refactor so I could bring them through the
> DM tree if no one else picks them up.

I was able to test it only for pl011, don't have any board with pl010.

>
> Regards,
> Simon

Rgds,
Vikas

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [U-Boot] [PATCH 0/5] PL01x: baudrate & line control fixes
  2014-11-18 18:59   ` vikasm
@ 2014-11-24 15:51     ` Simon Glass
  2014-11-25  0:40       ` vikasm
  0 siblings, 1 reply; 17+ messages in thread
From: Simon Glass @ 2014-11-24 15:51 UTC (permalink / raw)
  To: u-boot

Hi Vikas,

On 18 November 2014 at 11:59, vikasm <vikas.manocha@st.com> wrote:
> Thanks Simon,
>
>
> On 11/17/2014 09:30 PM, Simon Glass wrote:
>>
>> Hi Vikas,
>>
>> On 18 November 2014 00:17, Vikas Manocha <vikas.manocha@st.com> wrote:
>>>
>>> This patchset fixes the pl01x driver esp for pl011 baudrate & line
>>> control.
>>>
>>> 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 |   46
>>> +++++++++++++++++++++--------------------
>>>   1 file changed, 24 insertions(+), 22 deletions(-)
>>
>> Great to see this series. If you have not tested on p1010 I can do
>> this in a week or so (am travelling). At least a few of these looks
>> like bug fixes to my recent refactor so I could bring them through the
>> DM tree if no one else picks them up.
>
>
> I was able to test it only for pl011, don't have any board with pl010.

Sadly I was wrong, I only have pl011 also. It makes me wonder how it
worked so nicely for me without your patches.

Regards,
Simon

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [U-Boot] [PATCH 0/5] PL01x: baudrate & line control fixes
  2014-11-24 15:51     ` Simon Glass
@ 2014-11-25  0:40       ` vikasm
  2014-11-25  3:51         ` Simon Glass
  0 siblings, 1 reply; 17+ messages in thread
From: vikasm @ 2014-11-25  0:40 UTC (permalink / raw)
  To: u-boot

Hi Simon,

On 11/24/2014 07:51 AM, Simon Glass wrote:
> Hi Vikas,
>
> On 18 November 2014 at 11:59, vikasm <vikas.manocha@st.com> wrote:
>> Thanks Simon,
>>
>>
>> On 11/17/2014 09:30 PM, Simon Glass wrote:
>>> Hi Vikas,
>>>
>>> On 18 November 2014 00:17, Vikas Manocha <vikas.manocha@st.com> wrote:
>>>> This patchset fixes the pl01x driver esp for pl011 baudrate & line
>>>> control.
>>>>
>>>> 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 |   46
>>>> +++++++++++++++++++++--------------------
>>>>    1 file changed, 24 insertions(+), 22 deletions(-)
>>> Great to see this series. If you have not tested on p1010 I can do
>>> this in a week or so (am travelling). At least a few of these looks
>>> like bug fixes to my recent refactor so I could bring them through the
>>> DM tree if no one else picks them up.
>>
>> I was able to test it only for pl011, don't have any board with pl010.
> Sadly I was wrong, I only have pl011 also. It makes me wonder how it
> worked so nicely for me without your patches.
Please find below the possible explanation for all patches:

serial: pl01x: pass pl01x_type to set baudrate
- This patch only affect if DM is not defined.
If you are using driver model for pl011, pl011 will work without this patch.

serial: pl01x: fix pl011 baud rate configuration
- If pl011 is already configured to correct baudrate or LCR register is 
being written later.

serial: pl01x: move all line control at same place
- this patch does not affect the functionality.

serial: pl01x: disable as per type of pl01x
- pl011 might work without disabling it properly at start.
pl010 control register(disable bit) location is reserved in case of pl011.

serial: pl01x: avoid pl01x type check two times
- does not affect the functionality.

>
> Regards,
> Simon

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [U-Boot] [PATCH 0/5] PL01x: baudrate & line control fixes
  2014-11-25  0:40       ` vikasm
@ 2014-11-25  3:51         ` Simon Glass
  2014-11-25 19:33           ` vikasm
  0 siblings, 1 reply; 17+ messages in thread
From: Simon Glass @ 2014-11-25  3:51 UTC (permalink / raw)
  To: u-boot

Hi Vikas,

On 24 November 2014 at 17:40, vikasm <vikas.manocha@st.com> wrote:
> Hi Simon,
>
>
> On 11/24/2014 07:51 AM, Simon Glass wrote:
>>
>> Hi Vikas,
>>
>> On 18 November 2014 at 11:59, vikasm <vikas.manocha@st.com> wrote:
>>>
>>> Thanks Simon,
>>>
>>>
>>> On 11/17/2014 09:30 PM, Simon Glass wrote:
>>>>
>>>> Hi Vikas,
>>>>
>>>> On 18 November 2014 00:17, Vikas Manocha <vikas.manocha@st.com> wrote:
>>>>>
>>>>> This patchset fixes the pl01x driver esp for pl011 baudrate & line
>>>>> control.
>>>>>
>>>>> 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 |   46
>>>>> +++++++++++++++++++++--------------------
>>>>>    1 file changed, 24 insertions(+), 22 deletions(-)
>>>>
>>>> Great to see this series. If you have not tested on p1010 I can do
>>>> this in a week or so (am travelling). At least a few of these looks
>>>> like bug fixes to my recent refactor so I could bring them through the
>>>> DM tree if no one else picks them up.
>>>
>>>
>>> I was able to test it only for pl011, don't have any board with pl010.
>>
>> Sadly I was wrong, I only have pl011 also. It makes me wonder how it
>> worked so nicely for me without your patches.
>
> Please find below the possible explanation for all patches:
>
> serial: pl01x: pass pl01x_type to set baudrate
> - This patch only affect if DM is not defined.
> If you are using driver model for pl011, pl011 will work without this patch.
>
> serial: pl01x: fix pl011 baud rate configuration
> - If pl011 is already configured to correct baudrate or LCR register is
> being written later.
>
> serial: pl01x: move all line control at same place
> - this patch does not affect the functionality.
>
> serial: pl01x: disable as per type of pl01x
> - pl011 might work without disabling it properly at start.
> pl010 control register(disable bit) location is reserved in case of pl011.
>
> serial: pl01x: avoid pl01x type check two times
> - does not affect the functionality.

I didn't mean for you to go to that much trouble, but thank you.

Raspberry Pi is running without driver model (I tested it both ways)
so I guess I'm just lucky (or actually unlucky since I might have
noticed these problems).

When Stephen ACKs my Raspberry Pi driver rmodel series then I'll pull
this series in to DM.

Regards,
Simon

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [U-Boot] [PATCH 0/5] PL01x: baudrate & line control fixes
  2014-11-25  3:51         ` Simon Glass
@ 2014-11-25 19:33           ` vikasm
  0 siblings, 0 replies; 17+ messages in thread
From: vikasm @ 2014-11-25 19:33 UTC (permalink / raw)
  To: u-boot

Hello Simon,

On 11/24/2014 07:51 PM, Simon Glass wrote:
> Hi Vikas,
>
> On 24 November 2014 at 17:40, vikasm <vikas.manocha@st.com> wrote:
>> Hi Simon,
>>
>>
>> On 11/24/2014 07:51 AM, Simon Glass wrote:
>>> Hi Vikas,
>>>
>>> On 18 November 2014 at 11:59, vikasm <vikas.manocha@st.com> wrote:
>>>> Thanks Simon,
>>>>
>>>>
>>>> On 11/17/2014 09:30 PM, Simon Glass wrote:
>>>>> Hi Vikas,
>>>>>
>>>>> On 18 November 2014 00:17, Vikas Manocha <vikas.manocha@st.com> wrote:
>>>>>> This patchset fixes the pl01x driver esp for pl011 baudrate & line
>>>>>> control.
>>>>>>
>>>>>> 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 |   46
>>>>>> +++++++++++++++++++++--------------------
>>>>>>     1 file changed, 24 insertions(+), 22 deletions(-)
>>>>> Great to see this series. If you have not tested on p1010 I can do
>>>>> this in a week or so (am travelling). At least a few of these looks
>>>>> like bug fixes to my recent refactor so I could bring them through the
>>>>> DM tree if no one else picks them up.
>>>>
>>>> I was able to test it only for pl011, don't have any board with pl010.
>>> Sadly I was wrong, I only have pl011 also. It makes me wonder how it
>>> worked so nicely for me without your patches.
>> Please find below the possible explanation for all patches:
>>
>> serial: pl01x: pass pl01x_type to set baudrate
>> - This patch only affect if DM is not defined.
>> If you are using driver model for pl011, pl011 will work without this patch.
>>
>> serial: pl01x: fix pl011 baud rate configuration
>> - If pl011 is already configured to correct baudrate or LCR register is
>> being written later.
>>
>> serial: pl01x: move all line control at same place
>> - this patch does not affect the functionality.
>>
>> serial: pl01x: disable as per type of pl01x
>> - pl011 might work without disabling it properly at start.
>> pl010 control register(disable bit) location is reserved in case of pl011.
>>
>> serial: pl01x: avoid pl01x type check two times
>> - does not affect the functionality.
> I didn't mean for you to go to that much trouble, but thank you.

Not a problem, i was also curious to know it.

>
> Raspberry Pi is running without driver model (I tested it both ways)
> so I guess I'm just lucky (or actually unlucky since I might have
> noticed these problems).

just guessing... (don't want to bother you....don't reply if is not the 
case)
could it be possible uart is already initialized before u-boot like in 
xloader if it exists for Raspberry....

>
> When Stephen ACKs my Raspberry Pi driver rmodel series then I'll pull
> this series in to DM.

ok.

>
> Regards,
> Simon

Regards,
Vikas

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2014-11-25 19:33 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-11-18  0:17 [U-Boot] [PATCH 0/5] PL01x: baudrate & line control fixes Vikas Manocha
2014-11-18  0:17 ` [U-Boot] [PATCH 1/5] serial: pl01x: pass pl01x_type to set baudrate Vikas Manocha
2014-11-18  5:25   ` Simon Glass
2014-11-18  0:17 ` [U-Boot] [PATCH 2/5] serial: pl01x: fix pl011 baud rate configuration Vikas Manocha
2014-11-18  5:26   ` Simon Glass
2014-11-18  0:17 ` [U-Boot] [PATCH 3/5] serial: pl01x: move all line control at same place Vikas Manocha
2014-11-18  5:27   ` Simon Glass
2014-11-18  0:17 ` [U-Boot] [PATCH 4/5] serial: pl01x: disable as per type of pl01x Vikas Manocha
2014-11-18  5:27   ` Simon Glass
2014-11-18  0:17 ` [U-Boot] [PATCH 5/5] serial: pl01x: avoid pl01x type check two times Vikas Manocha
2014-11-18  5:28   ` Simon Glass
2014-11-18  5:30 ` [U-Boot] [PATCH 0/5] PL01x: baudrate & line control fixes Simon Glass
2014-11-18 18:59   ` vikasm
2014-11-24 15:51     ` Simon Glass
2014-11-25  0:40       ` vikasm
2014-11-25  3:51         ` Simon Glass
2014-11-25 19:33           ` vikasm

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox