diff mbox series

[v2,2/2] bcm283x_pl011: Flush RX queue after setting baud rate

Message ID 20180307210825.50709-3-agraf@suse.de
State Accepted
Commit c9bf43dd9d49c5024a9a361b5168ef664186c22a
Headers show
Series RPi: Drain RX queue on setbrg | expand

Commit Message

Alexander Graf March 7, 2018, 9:08 p.m. UTC
After the UART was initialized, we may still have bogus data in the
RX queue if it was enabled with incorrect pin muxing before.

So let's flush the RX queue whenever we initialize baud rates.

This fixes a regression with the dynamic pinmuxing code when enable_uart=1
is not set in config.txt on Raspberry Pis that use pl011 for serial.

Fixes: caf2233b28 ("bcm283x: Add pinctrl driver")
Reported-by: Göran Lundberg <goran@lundberg.email>
Reported-by: Peter Robinson <pbrobinson@gmail.com>
Signed-off-by: Alexander Graf <agraf@suse.de>

---

v1 -> v2:

  - correctly drain the queue
---
 drivers/serial/serial_bcm283x_pl011.c  | 25 ++++++++++++++++++++++++-
 drivers/serial/serial_pl01x.c          | 10 +++++-----
 drivers/serial/serial_pl01x_internal.h |  7 ++++++-
 3 files changed, 35 insertions(+), 7 deletions(-)

Comments

Peter Robinson March 8, 2018, 10:06 a.m. UTC | #1
On Wed, Mar 7, 2018 at 9:08 PM, Alexander Graf <agraf@suse.de> wrote:
> After the UART was initialized, we may still have bogus data in the
> RX queue if it was enabled with incorrect pin muxing before.
>
> So let's flush the RX queue whenever we initialize baud rates.
>
> This fixes a regression with the dynamic pinmuxing code when enable_uart=1
> is not set in config.txt on Raspberry Pis that use pl011 for serial.
>
> Fixes: caf2233b28 ("bcm283x: Add pinctrl driver")
> Reported-by: Göran Lundberg <goran@lundberg.email>
> Reported-by: Peter Robinson <pbrobinson@gmail.com>
> Signed-off-by: Alexander Graf <agraf@suse.de>
Tested-by: Peter Robinson <pbrobinson@gmail.com>

Tested on RPi2 and works a lot better, I can boot, interact with the
console etc :-)


> ---
>
> v1 -> v2:
>
>   - correctly drain the queue
> ---
>  drivers/serial/serial_bcm283x_pl011.c  | 25 ++++++++++++++++++++++++-
>  drivers/serial/serial_pl01x.c          | 10 +++++-----
>  drivers/serial/serial_pl01x_internal.h |  7 ++++++-
>  3 files changed, 35 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/serial/serial_bcm283x_pl011.c b/drivers/serial/serial_bcm283x_pl011.c
> index bfd39f84f3..dad7236895 100644
> --- a/drivers/serial/serial_bcm283x_pl011.c
> +++ b/drivers/serial/serial_bcm283x_pl011.c
> @@ -9,6 +9,7 @@
>  #include <asm/gpio.h>
>  #include <dm/pinctrl.h>
>  #include <dm/platform_data/serial_pl01x.h>
> +#include <serial.h>
>  #include "serial_pl01x_internal.h"
>
>  /*
> @@ -55,6 +56,28 @@ static int bcm283x_pl011_serial_ofdata_to_platdata(struct udevice *dev)
>         return 0;
>  }
>
> +static int bcm283x_pl011_serial_setbrg(struct udevice *dev, int baudrate)
> +{
> +       int r;
> +
> +       r = pl01x_serial_setbrg(dev, baudrate);
> +
> +       /*
> +        * We may have been muxed to a bogus line before. Drain the RX
> +        * queue so we start at a clean slate.
> +        */
> +       while (pl01x_serial_getc(dev) != -EAGAIN) ;
> +
> +       return r;
> +}
> +
> +static const struct dm_serial_ops bcm283x_pl011_serial_ops = {
> +       .putc = pl01x_serial_putc,
> +       .pending = pl01x_serial_pending,
> +       .getc = pl01x_serial_getc,
> +       .setbrg = bcm283x_pl011_serial_setbrg,
> +};
> +
>  static const struct udevice_id bcm283x_pl011_serial_id[] = {
>         {.compatible = "brcm,bcm2835-pl011", .data = TYPE_PL011},
>         {}
> @@ -67,7 +90,7 @@ U_BOOT_DRIVER(bcm283x_pl011_uart) = {
>         .ofdata_to_platdata = of_match_ptr(bcm283x_pl011_serial_ofdata_to_platdata),
>         .platdata_auto_alloc_size = sizeof(struct pl01x_serial_platdata),
>         .probe  = pl01x_serial_probe,
> -       .ops    = &pl01x_serial_ops,
> +       .ops    = &bcm283x_pl011_serial_ops,
>         .flags  = DM_FLAG_PRE_RELOC,
>         .priv_auto_alloc_size = sizeof(struct pl01x_priv),
>  };
> diff --git a/drivers/serial/serial_pl01x.c b/drivers/serial/serial_pl01x.c
> index 23d9d839cb..45f1282770 100644
> --- a/drivers/serial/serial_pl01x.c
> +++ b/drivers/serial/serial_pl01x.c
> @@ -273,7 +273,7 @@ __weak struct serial_device *default_serial_console(void)
>
>  #ifdef CONFIG_DM_SERIAL
>
> -static int pl01x_serial_setbrg(struct udevice *dev, int baudrate)
> +int pl01x_serial_setbrg(struct udevice *dev, int baudrate)
>  {
>         struct pl01x_serial_platdata *plat = dev_get_platdata(dev);
>         struct pl01x_priv *priv = dev_get_priv(dev);
> @@ -299,21 +299,21 @@ int pl01x_serial_probe(struct udevice *dev)
>                 return 0;
>  }
>
> -static int pl01x_serial_getc(struct udevice *dev)
> +int pl01x_serial_getc(struct udevice *dev)
>  {
>         struct pl01x_priv *priv = dev_get_priv(dev);
>
>         return pl01x_getc(priv->regs);
>  }
>
> -static int pl01x_serial_putc(struct udevice *dev, const char ch)
> +int pl01x_serial_putc(struct udevice *dev, const char ch)
>  {
>         struct pl01x_priv *priv = dev_get_priv(dev);
>
>         return pl01x_putc(priv->regs, ch);
>  }
>
> -static int pl01x_serial_pending(struct udevice *dev, bool input)
> +int pl01x_serial_pending(struct udevice *dev, bool input)
>  {
>         struct pl01x_priv *priv = dev_get_priv(dev);
>         unsigned int fr = readl(&priv->regs->fr);
> @@ -324,7 +324,7 @@ static int pl01x_serial_pending(struct udevice *dev, bool input)
>                 return fr & UART_PL01x_FR_TXFF ? 0 : 1;
>  }
>
> -const struct dm_serial_ops pl01x_serial_ops = {
> +static const struct dm_serial_ops pl01x_serial_ops = {
>         .putc = pl01x_serial_putc,
>         .pending = pl01x_serial_pending,
>         .getc = pl01x_serial_getc,
> diff --git a/drivers/serial/serial_pl01x_internal.h b/drivers/serial/serial_pl01x_internal.h
> index c56dd54c7b..d4605f24a3 100644
> --- a/drivers/serial/serial_pl01x_internal.h
> +++ b/drivers/serial/serial_pl01x_internal.h
> @@ -43,7 +43,12 @@ struct pl01x_regs {
>
>  int pl01x_serial_ofdata_to_platdata(struct udevice *dev);
>  int pl01x_serial_probe(struct udevice *dev);
> -extern const struct dm_serial_ops pl01x_serial_ops;
> +
> +/* Needed for external pl01x_serial_ops drivers */
> +int pl01x_serial_putc(struct udevice *dev, const char ch);
> +int pl01x_serial_pending(struct udevice *dev, bool input);
> +int pl01x_serial_getc(struct udevice *dev);
> +int pl01x_serial_setbrg(struct udevice *dev, int baudrate);
>
>  struct pl01x_priv {
>         struct pl01x_regs *regs;
> --
> 2.12.3
>
Tuomas Tynkkynen March 9, 2018, 4:04 p.m. UTC | #2
On Wed, 7 Mar 2018 22:08:25 +0100
Alexander Graf <agraf@suse.de> wrote:

> After the UART was initialized, we may still have bogus data in the
> RX queue if it was enabled with incorrect pin muxing before.
> 
> So let's flush the RX queue whenever we initialize baud rates.
> 
> This fixes a regression with the dynamic pinmuxing code when enable_uart=1
> is not set in config.txt on Raspberry Pis that use pl011 for serial.
> 
> Fixes: caf2233b28 ("bcm283x: Add pinctrl driver")
> Reported-by: Göran Lundberg <goran@lundberg.email>
> Reported-by: Peter Robinson <pbrobinson@gmail.com>
> Signed-off-by: Alexander Graf <agraf@suse.de>
> 
> ---
> 
> v1 -> v2:
> 
>   - correctly drain the queue
> ---
>  drivers/serial/serial_bcm283x_pl011.c  | 25 ++++++++++++++++++++++++-
>  drivers/serial/serial_pl01x.c          | 10 +++++-----
>  drivers/serial/serial_pl01x_internal.h |  7 ++++++-
>  3 files changed, 35 insertions(+), 7 deletions(-)
> 

On a RPi 1, with no enable_uart in config.txt:

Tested-by: Tuomas Tynkkynen <tuomas@tuxera.com>
Tom Rini March 9, 2018, 6:29 p.m. UTC | #3
On Wed, Mar 07, 2018 at 10:08:25PM +0100, Alexander Graf wrote:

> After the UART was initialized, we may still have bogus data in the

> RX queue if it was enabled with incorrect pin muxing before.

> 

> So let's flush the RX queue whenever we initialize baud rates.

> 

> This fixes a regression with the dynamic pinmuxing code when enable_uart=1

> is not set in config.txt on Raspberry Pis that use pl011 for serial.

> 

> Fixes: caf2233b28 ("bcm283x: Add pinctrl driver")

> Reported-by: Göran Lundberg <goran@lundberg.email>

> Reported-by: Peter Robinson <pbrobinson@gmail.com>

> Signed-off-by: Alexander Graf <agraf@suse.de>

> Tested-by: Peter Robinson <pbrobinson@gmail.com>

> Tested-by: Tuomas Tynkkynen <tuomas@tuxera.com>


Applied to u-boot/master, thanks!

-- 
Tom
diff mbox series

Patch

diff --git a/drivers/serial/serial_bcm283x_pl011.c b/drivers/serial/serial_bcm283x_pl011.c
index bfd39f84f3..dad7236895 100644
--- a/drivers/serial/serial_bcm283x_pl011.c
+++ b/drivers/serial/serial_bcm283x_pl011.c
@@ -9,6 +9,7 @@ 
 #include <asm/gpio.h>
 #include <dm/pinctrl.h>
 #include <dm/platform_data/serial_pl01x.h>
+#include <serial.h>
 #include "serial_pl01x_internal.h"
 
 /*
@@ -55,6 +56,28 @@  static int bcm283x_pl011_serial_ofdata_to_platdata(struct udevice *dev)
 	return 0;
 }
 
+static int bcm283x_pl011_serial_setbrg(struct udevice *dev, int baudrate)
+{
+	int r;
+
+	r = pl01x_serial_setbrg(dev, baudrate);
+
+	/*
+	 * We may have been muxed to a bogus line before. Drain the RX
+	 * queue so we start at a clean slate.
+	 */
+	while (pl01x_serial_getc(dev) != -EAGAIN) ;
+
+	return r;
+}
+
+static const struct dm_serial_ops bcm283x_pl011_serial_ops = {
+	.putc = pl01x_serial_putc,
+	.pending = pl01x_serial_pending,
+	.getc = pl01x_serial_getc,
+	.setbrg = bcm283x_pl011_serial_setbrg,
+};
+
 static const struct udevice_id bcm283x_pl011_serial_id[] = {
 	{.compatible = "brcm,bcm2835-pl011", .data = TYPE_PL011},
 	{}
@@ -67,7 +90,7 @@  U_BOOT_DRIVER(bcm283x_pl011_uart) = {
 	.ofdata_to_platdata = of_match_ptr(bcm283x_pl011_serial_ofdata_to_platdata),
 	.platdata_auto_alloc_size = sizeof(struct pl01x_serial_platdata),
 	.probe	= pl01x_serial_probe,
-	.ops	= &pl01x_serial_ops,
+	.ops	= &bcm283x_pl011_serial_ops,
 	.flags	= DM_FLAG_PRE_RELOC,
 	.priv_auto_alloc_size = sizeof(struct pl01x_priv),
 };
diff --git a/drivers/serial/serial_pl01x.c b/drivers/serial/serial_pl01x.c
index 23d9d839cb..45f1282770 100644
--- a/drivers/serial/serial_pl01x.c
+++ b/drivers/serial/serial_pl01x.c
@@ -273,7 +273,7 @@  __weak struct serial_device *default_serial_console(void)
 
 #ifdef CONFIG_DM_SERIAL
 
-static int pl01x_serial_setbrg(struct udevice *dev, int baudrate)
+int pl01x_serial_setbrg(struct udevice *dev, int baudrate)
 {
 	struct pl01x_serial_platdata *plat = dev_get_platdata(dev);
 	struct pl01x_priv *priv = dev_get_priv(dev);
@@ -299,21 +299,21 @@  int pl01x_serial_probe(struct udevice *dev)
 		return 0;
 }
 
-static int pl01x_serial_getc(struct udevice *dev)
+int pl01x_serial_getc(struct udevice *dev)
 {
 	struct pl01x_priv *priv = dev_get_priv(dev);
 
 	return pl01x_getc(priv->regs);
 }
 
-static int pl01x_serial_putc(struct udevice *dev, const char ch)
+int pl01x_serial_putc(struct udevice *dev, const char ch)
 {
 	struct pl01x_priv *priv = dev_get_priv(dev);
 
 	return pl01x_putc(priv->regs, ch);
 }
 
-static int pl01x_serial_pending(struct udevice *dev, bool input)
+int pl01x_serial_pending(struct udevice *dev, bool input)
 {
 	struct pl01x_priv *priv = dev_get_priv(dev);
 	unsigned int fr = readl(&priv->regs->fr);
@@ -324,7 +324,7 @@  static int pl01x_serial_pending(struct udevice *dev, bool input)
 		return fr & UART_PL01x_FR_TXFF ? 0 : 1;
 }
 
-const struct dm_serial_ops pl01x_serial_ops = {
+static const struct dm_serial_ops pl01x_serial_ops = {
 	.putc = pl01x_serial_putc,
 	.pending = pl01x_serial_pending,
 	.getc = pl01x_serial_getc,
diff --git a/drivers/serial/serial_pl01x_internal.h b/drivers/serial/serial_pl01x_internal.h
index c56dd54c7b..d4605f24a3 100644
--- a/drivers/serial/serial_pl01x_internal.h
+++ b/drivers/serial/serial_pl01x_internal.h
@@ -43,7 +43,12 @@  struct pl01x_regs {
 
 int pl01x_serial_ofdata_to_platdata(struct udevice *dev);
 int pl01x_serial_probe(struct udevice *dev);
-extern const struct dm_serial_ops pl01x_serial_ops;
+
+/* Needed for external pl01x_serial_ops drivers */
+int pl01x_serial_putc(struct udevice *dev, const char ch);
+int pl01x_serial_pending(struct udevice *dev, bool input);
+int pl01x_serial_getc(struct udevice *dev);
+int pl01x_serial_setbrg(struct udevice *dev, int baudrate);
 
 struct pl01x_priv {
 	struct pl01x_regs *regs;