Re: [PATCH 3/3] serial: serial_msm: Delay initialization to let pins stabilize

[Stephan Gerhold <stephan@gerhold.net>]

On Wed, Jul 14, 2021 at 11:39:34PM +0300, Ramon Fried wrote:
> On Wed, Jul 14, 2021 at 11:56 AM Stephan Gerhold <stephan@gerhold.net> wrote:
> >
> > For some reason, the DragonBoard 410c aborts autoboot immediately if
> > U-Boot is started without LK. It looks like it picks up a single broken
> > character via serial and therefore believes a key was pressed to abort
> > autoboot.
> >
> > After some debugging, it seems like adding some delay after pinctrl
> > setup but before UART initialization fixes the issue. It's also worth
> > mentioning that unlike when booting from LK, the pinctrl setup is
> > actually necessary when booting U-Boot without LK since UART is broken
> > if the pinctrl line is removed.
> >
> > I suspect that reconfiguring the pins might take some time to stabilize
> > and if the UART controller is enabled too quickly it will pick up some
> > random noise. Adding a few milliseconds of delay fixes the issue and
> > shouldn't have any other negative side effects.
> >
> > 3ms seems to be the minimum delay required in my tests, use 5ms instead
> > just to be sure.
> >
> > Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
> > ---
> >
> >  drivers/serial/serial_msm.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/drivers/serial/serial_msm.c b/drivers/serial/serial_msm.c
> > index d8dd5c1104..ac4d0824b9 100644
> > --- a/drivers/serial/serial_msm.c
> > +++ b/drivers/serial/serial_msm.c
> > @@ -18,6 +18,7 @@
> >  #include <asm/global_data.h>
> >  #include <asm/io.h>
> >  #include <linux/compiler.h>
> > +#include <linux/delay.h>
> >  #include <dm/pinctrl.h>
> >
> >  /* Serial registers - this driver works in uartdm mode*/
> > @@ -193,6 +194,9 @@ static int msm_uart_clk_init(struct udevice *dev)
> >
> >  static void uart_dm_init(struct msm_serial_data *priv)
> >  {
> > +       /* Delay initialization for a bit to let pins stabilize if necessary */
> > +       mdelay(5);
> > +
> >         writel(priv->clk_bit_rate, priv->base + UARTDM_CSR);
> >         writel(0x0, priv->base + UARTDM_MR1);
> >         writel(MSM_BOOT_UART_DM_8_N_1_MODE, priv->base + UARTDM_MR2);
> > --
> > 2.32.0
> >
> Eh...this is not a nice fix, perhaps just clearing the RX buffer after
> initialization will work better ?

I don't like it much either but this is the best I came up with even
after spending several hours trying to track down the problem. I also
tried porting more register initialization from LK, probably even
tried clearing the RX buffer again after initialization.

The problem is: In my tests there was no way to get rid of the delay,
because the character doesn't show up until a few milliseconds after
initialization. What you suggest would only work like this:

  <initialize UART>
  mdelay(5); // Wait for the weird character to show up
  <clear RX buffer>

Which is not much different (and in fact will take longer) than:

  mdelay(5);
  <initialize UART>

So unfortunately I can't think of a way to get rid of the delay. :(

I'm not sure if this is some strange problem of my DB410c, I'd actually
be curious if the problem can also be reproduced on others.

Stephan