[PATCH] serial: serial_msm: Ensure BAM/single character mode are disabled

[PATCH] serial: serial_msm: Ensure BAM/single character mode are disabled

[Stephan Gerhold]

At the moment, the U-Boot serial_msm driver does not initialize the
UART_DM_DMEN register with the required value. Usually this does not
cause any problems, because there is Qualcomm's LK bootloader running
before U-Boot which initializes the register with the correct value.

It's important that this register is initialized correctly, because
the U-Boot driver does not make use of the BAM/DMA or single character
mode functionality of the UART controller. A different bootloader
before U-Boot might initialize the register differently.

For example, on DragonBoard 410c U-Boot can also be installed to the
"aboot" partition (replacing LK entirely). In this case U-Boot is
loaded directly by SBL, which seems to use the single-character mode
for some reason. In single character mode there is always just one
char in the FIFO, instead of the 4 characters expected by
msm_serial_fetch(). It also causes issues with "earlycon" later in
the Linux kernel, which tries to output 4 chars at once,
but only the first char will be written.

This causes early UART log in Linux to be corrupted like this:

    [ 00ano:ameoi .Q1B[ 00ac _idaM00080oo'ahani-lcle._20). 15NdNii 5 SPMSJ20:U2
    [ 00rkoolmsamel
    [ 00Fw ]elamletopsioble
    [ 00ore

instead of

    [    0.000000] Booting Linux on physical CPU 0x0000000000 [0x410fd030]
    [    0.000000] Machine model: Qualcomm Technologies, Inc. APQ 8016 SBC
    [    0.000000] earlycon: msm_serial_dm0 at MMIO 0x00000000078b0000 (options '')
    [    0.000000] printk: bootconsole [msm_serial_dm0] enabled

Make sure to initialize UART_DM_DMEN correctly to fix this issue
when loading U-Boot directly after SBL (instead of through LK).

There is no functional difference when loading U-Boot through LK
since LK also initializes UART_DM_DMEN to 0x0. [1]

[1]: https://git.linaro.org/landing-teams/working/qualcomm/lk.git/tree/platform/msm_shared/uart_dm.c?h=dragonboard410c-LA.BR.1.2.7-03810-8x16.0-linaro3#n203

Cc: Ramon Fried <rfried.dev@gmail.com>
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 d8c6c2f6b5..d8dd5c1104 100644
--- a/drivers/serial/serial_msm.c
+++ b/drivers/serial/serial_msm.c
@@ -23,6 +23,7 @@
 /* Serial registers - this driver works in uartdm mode*/
 
 #define UARTDM_DMRX             0x34 /* Max RX transfer length */
+#define UARTDM_DMEN             0x3C /* DMA/data-packing mode */
 #define UARTDM_NCF_TX           0x40 /* Number of chars to TX */
 
 #define UARTDM_RXFS             0x50 /* RX channel status register */
@@ -197,6 +198,9 @@ static void uart_dm_init(struct msm_serial_data *priv)
 	writel(MSM_BOOT_UART_DM_8_N_1_MODE, priv->base + UARTDM_MR2);
 	writel(MSM_BOOT_UART_DM_CMD_RESET_RX, priv->base + UARTDM_CR);
 	writel(MSM_BOOT_UART_DM_CMD_RESET_TX, priv->base + UARTDM_CR);
+
+	/* Make sure BAM/single character mode is disabled */
+	writel(0x0, priv->base + UARTDM_DMEN);
 }
 static int msm_serial_probe(struct udevice *dev)
 {
-- 
2.32.0

Re: [PATCH] serial: serial_msm: Ensure BAM/single character mode are disabled

[Ramon Fried]

On Mon, Jun 28, 2021 at 11:40 AM Stephan Gerhold <stephan@gerhold.net> wrote:
>
> At the moment, the U-Boot serial_msm driver does not initialize the
> UART_DM_DMEN register with the required value. Usually this does not
> cause any problems, because there is Qualcomm's LK bootloader running
> before U-Boot which initializes the register with the correct value.
>
> It's important that this register is initialized correctly, because
> the U-Boot driver does not make use of the BAM/DMA or single character
> mode functionality of the UART controller. A different bootloader
> before U-Boot might initialize the register differently.
>
> For example, on DragonBoard 410c U-Boot can also be installed to the
> "aboot" partition (replacing LK entirely). In this case U-Boot is
> loaded directly by SBL, which seems to use the single-character mode
> for some reason. In single character mode there is always just one
> char in the FIFO, instead of the 4 characters expected by
> msm_serial_fetch(). It also causes issues with "earlycon" later in
> the Linux kernel, which tries to output 4 chars at once,
> but only the first char will be written.
>
> This causes early UART log in Linux to be corrupted like this:
>
>     [ 00ano:ameoi .Q1B[ 00ac _idaM00080oo'ahani-lcle._20). 15NdNii 5 SPMSJ20:U2
>     [ 00rkoolmsamel
>     [ 00Fw ]elamletopsioble
>     [ 00ore
>
> instead of
>
>     [    0.000000] Booting Linux on physical CPU 0x0000000000 [0x410fd030]
>     [    0.000000] Machine model: Qualcomm Technologies, Inc. APQ 8016 SBC
>     [    0.000000] earlycon: msm_serial_dm0 at MMIO 0x00000000078b0000 (options '')
>     [    0.000000] printk: bootconsole [msm_serial_dm0] enabled
>
> Make sure to initialize UART_DM_DMEN correctly to fix this issue
> when loading U-Boot directly after SBL (instead of through LK).
>
> There is no functional difference when loading U-Boot through LK
> since LK also initializes UART_DM_DMEN to 0x0. [1]
>
> [1]: https://git.linaro.org/landing-teams/working/qualcomm/lk.git/tree/platform/msm_shared/uart_dm.c?h=dragonboard410c-LA.BR.1.2.7-03810-8x16.0-linaro3#n203
>
> Cc: Ramon Fried <rfried.dev@gmail.com>
> 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 d8c6c2f6b5..d8dd5c1104 100644
> --- a/drivers/serial/serial_msm.c
> +++ b/drivers/serial/serial_msm.c
> @@ -23,6 +23,7 @@
>  /* Serial registers - this driver works in uartdm mode*/
>
>  #define UARTDM_DMRX             0x34 /* Max RX transfer length */
> +#define UARTDM_DMEN             0x3C /* DMA/data-packing mode */
>  #define UARTDM_NCF_TX           0x40 /* Number of chars to TX */
>
>  #define UARTDM_RXFS             0x50 /* RX channel status register */
> @@ -197,6 +198,9 @@ static void uart_dm_init(struct msm_serial_data *priv)
>         writel(MSM_BOOT_UART_DM_8_N_1_MODE, priv->base + UARTDM_MR2);
>         writel(MSM_BOOT_UART_DM_CMD_RESET_RX, priv->base + UARTDM_CR);
>         writel(MSM_BOOT_UART_DM_CMD_RESET_TX, priv->base + UARTDM_CR);
> +
> +       /* Make sure BAM/single character mode is disabled */
> +       writel(0x0, priv->base + UARTDM_DMEN);
>  }
>  static int msm_serial_probe(struct udevice *dev)
>  {
> --
> 2.32.0
>
Hi.
This is strange, I never encountered the behaviour, and I did boot
Linux after U-boot without LK in the way.

Re: [PATCH] serial: serial_msm: Ensure BAM/single character mode are disabled

[Stephan Gerhold]

On Mon, Jun 28, 2021 at 08:15:28PM +0300, Ramon Fried wrote:
> On Mon, Jun 28, 2021 at 11:40 AM Stephan Gerhold <stephan@gerhold.net> wrote:
> >
> > At the moment, the U-Boot serial_msm driver does not initialize the
> > UART_DM_DMEN register with the required value. Usually this does not
> > cause any problems, because there is Qualcomm's LK bootloader running
> > before U-Boot which initializes the register with the correct value.
> >
> > It's important that this register is initialized correctly, because
> > the U-Boot driver does not make use of the BAM/DMA or single character
> > mode functionality of the UART controller. A different bootloader
> > before U-Boot might initialize the register differently.
> >
> > For example, on DragonBoard 410c U-Boot can also be installed to the
> > "aboot" partition (replacing LK entirely). In this case U-Boot is
> > loaded directly by SBL, which seems to use the single-character mode
> > for some reason. In single character mode there is always just one
> > char in the FIFO, instead of the 4 characters expected by
> > msm_serial_fetch(). It also causes issues with "earlycon" later in
> > the Linux kernel, which tries to output 4 chars at once,
> > but only the first char will be written.
> >
> > This causes early UART log in Linux to be corrupted like this:
> >
> >     [ 00ano:ameoi .Q1B[ 00ac _idaM00080oo'ahani-lcle._20). 15NdNii 5 SPMSJ20:U2
> >     [ 00rkoolmsamel
> >     [ 00Fw ]elamletopsioble
> >     [ 00ore
> >
> > instead of
> >
> >     [    0.000000] Booting Linux on physical CPU 0x0000000000 [0x410fd030]
> >     [    0.000000] Machine model: Qualcomm Technologies, Inc. APQ 8016 SBC
> >     [    0.000000] earlycon: msm_serial_dm0 at MMIO 0x00000000078b0000 (options '')
> >     [    0.000000] printk: bootconsole [msm_serial_dm0] enabled
> >
> > Make sure to initialize UART_DM_DMEN correctly to fix this issue
> > when loading U-Boot directly after SBL (instead of through LK).
> >
> > There is no functional difference when loading U-Boot through LK
> > since LK also initializes UART_DM_DMEN to 0x0. [1]
> >
> > [1]: https://git.linaro.org/landing-teams/working/qualcomm/lk.git/tree/platform/msm_shared/uart_dm.c?h=dragonboard410c-LA.BR.1.2.7-03810-8x16.0-linaro3#n203
> >
> > Cc: Ramon Fried <rfried.dev@gmail.com>
> > 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 d8c6c2f6b5..d8dd5c1104 100644
> > --- a/drivers/serial/serial_msm.c
> > +++ b/drivers/serial/serial_msm.c
> > @@ -23,6 +23,7 @@
> >  /* Serial registers - this driver works in uartdm mode*/
> >
> >  #define UARTDM_DMRX             0x34 /* Max RX transfer length */
> > +#define UARTDM_DMEN             0x3C /* DMA/data-packing mode */
> >  #define UARTDM_NCF_TX           0x40 /* Number of chars to TX */
> >
> >  #define UARTDM_RXFS             0x50 /* RX channel status register */
> > @@ -197,6 +198,9 @@ static void uart_dm_init(struct msm_serial_data *priv)
> >         writel(MSM_BOOT_UART_DM_8_N_1_MODE, priv->base + UARTDM_MR2);
> >         writel(MSM_BOOT_UART_DM_CMD_RESET_RX, priv->base + UARTDM_CR);
> >         writel(MSM_BOOT_UART_DM_CMD_RESET_TX, priv->base + UARTDM_CR);
> > +
> > +       /* Make sure BAM/single character mode is disabled */
> > +       writel(0x0, priv->base + UARTDM_DMEN);
> >  }
> >  static int msm_serial_probe(struct udevice *dev)
> >  {
> > --
> > 2.32.0
> >
> Hi.
> This is strange, I never encountered the behaviour, and I did boot
> Linux after U-boot without LK in the way.

It happens for me if the boot flow is SBL -> U-Boot -> Linux instead of
SBL -> LK -> U-Boot -> Linux (The latter is the suggested setup
according to dragonboard410c_defconfig and the dragonboard410c
readme.txt, but I wanted to eliminate LK entirely).

If you tried the same, perhaps you didn't have earlycon enabled?
It also happens only during early boot with earlycon enabled
("earlycon" in kernel parameters). It stops happening later on boot
when the kernel fully re-initializes the UART controller. (The idea of
earlycon is to reuse the existing UART configuration to report errors
that occur very early during boot.)

Thanks,
Stephan

Re: [PATCH] serial: serial_msm: Ensure BAM/single character mode are disabled

[Ramon Fried]

On Mon, Jun 28, 2021 at 9:59 PM Stephan Gerhold <stephan@gerhold.net> wrote:
>
> On Mon, Jun 28, 2021 at 08:15:28PM +0300, Ramon Fried wrote:
> > On Mon, Jun 28, 2021 at 11:40 AM Stephan Gerhold <stephan@gerhold.net> wrote:
> > >
> > > At the moment, the U-Boot serial_msm driver does not initialize the
> > > UART_DM_DMEN register with the required value. Usually this does not
> > > cause any problems, because there is Qualcomm's LK bootloader running
> > > before U-Boot which initializes the register with the correct value.
> > >
> > > It's important that this register is initialized correctly, because
> > > the U-Boot driver does not make use of the BAM/DMA or single character
> > > mode functionality of the UART controller. A different bootloader
> > > before U-Boot might initialize the register differently.
> > >
> > > For example, on DragonBoard 410c U-Boot can also be installed to the
> > > "aboot" partition (replacing LK entirely). In this case U-Boot is
> > > loaded directly by SBL, which seems to use the single-character mode
> > > for some reason. In single character mode there is always just one
> > > char in the FIFO, instead of the 4 characters expected by
> > > msm_serial_fetch(). It also causes issues with "earlycon" later in
> > > the Linux kernel, which tries to output 4 chars at once,
> > > but only the first char will be written.
> > >
> > > This causes early UART log in Linux to be corrupted like this:
> > >
> > >     [ 00ano:ameoi .Q1B[ 00ac _idaM00080oo'ahani-lcle._20). 15NdNii 5 SPMSJ20:U2
> > >     [ 00rkoolmsamel
> > >     [ 00Fw ]elamletopsioble
> > >     [ 00ore
> > >
> > > instead of
> > >
> > >     [    0.000000] Booting Linux on physical CPU 0x0000000000 [0x410fd030]
> > >     [    0.000000] Machine model: Qualcomm Technologies, Inc. APQ 8016 SBC
> > >     [    0.000000] earlycon: msm_serial_dm0 at MMIO 0x00000000078b0000 (options '')
> > >     [    0.000000] printk: bootconsole [msm_serial_dm0] enabled
> > >
> > > Make sure to initialize UART_DM_DMEN correctly to fix this issue
> > > when loading U-Boot directly after SBL (instead of through LK).
> > >
> > > There is no functional difference when loading U-Boot through LK
> > > since LK also initializes UART_DM_DMEN to 0x0. [1]
> > >
> > > [1]: https://git.linaro.org/landing-teams/working/qualcomm/lk.git/tree/platform/msm_shared/uart_dm.c?h=dragonboard410c-LA.BR.1.2.7-03810-8x16.0-linaro3#n203
> > >
> > > Cc: Ramon Fried <rfried.dev@gmail.com>
> > > 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 d8c6c2f6b5..d8dd5c1104 100644
> > > --- a/drivers/serial/serial_msm.c
> > > +++ b/drivers/serial/serial_msm.c
> > > @@ -23,6 +23,7 @@
> > >  /* Serial registers - this driver works in uartdm mode*/
> > >
> > >  #define UARTDM_DMRX             0x34 /* Max RX transfer length */
> > > +#define UARTDM_DMEN             0x3C /* DMA/data-packing mode */
> > >  #define UARTDM_NCF_TX           0x40 /* Number of chars to TX */
> > >
> > >  #define UARTDM_RXFS             0x50 /* RX channel status register */
> > > @@ -197,6 +198,9 @@ static void uart_dm_init(struct msm_serial_data *priv)
> > >         writel(MSM_BOOT_UART_DM_8_N_1_MODE, priv->base + UARTDM_MR2);
> > >         writel(MSM_BOOT_UART_DM_CMD_RESET_RX, priv->base + UARTDM_CR);
> > >         writel(MSM_BOOT_UART_DM_CMD_RESET_TX, priv->base + UARTDM_CR);
> > > +
> > > +       /* Make sure BAM/single character mode is disabled */
> > > +       writel(0x0, priv->base + UARTDM_DMEN);
> > >  }
> > >  static int msm_serial_probe(struct udevice *dev)
> > >  {
> > > --
> > > 2.32.0
> > >
> > Hi.
> > This is strange, I never encountered the behaviour, and I did boot
> > Linux after U-boot without LK in the way.
>
> It happens for me if the boot flow is SBL -> U-Boot -> Linux instead of
> SBL -> LK -> U-Boot -> Linux (The latter is the suggested setup
> according to dragonboard410c_defconfig and the dragonboard410c
> readme.txt, but I wanted to eliminate LK entirely).
>
> If you tried the same, perhaps you didn't have earlycon enabled?
> It also happens only during early boot with earlycon enabled
> ("earlycon" in kernel parameters). It stops happening later on boot
> when the kernel fully re-initializes the UART controller. (The idea of
> earlycon is to reuse the existing UART configuration to report errors
> that occur very early during boot.)
>
> Thanks,
> Stephan
It's interesting, I wanted to eliminate LK entirely, but I encountered
an error I couldn't understand.
You can run U-boot compiled to 64bit and 32bit, in 64bit the boot is
stuck for some reason, and Tom refused to merge a patch where a 32bit
U-boot would boot a 64bit Linux.
Did you compile U-boot for 64 or 32 ? How far did you get ?
Thnks,
Ramon

Re: [PATCH] serial: serial_msm: Ensure BAM/single character mode are disabled

[Stephan Gerhold]

On Mon, Jun 28, 2021 at 10:04:29PM +0300, Ramon Fried wrote:
> On Mon, Jun 28, 2021 at 9:59 PM Stephan Gerhold <stephan@gerhold.net> wrote:
> >
> > On Mon, Jun 28, 2021 at 08:15:28PM +0300, Ramon Fried wrote:
> > > On Mon, Jun 28, 2021 at 11:40 AM Stephan Gerhold <stephan@gerhold.net> wrote:
> > > >
> > > > At the moment, the U-Boot serial_msm driver does not initialize the
> > > > UART_DM_DMEN register with the required value. Usually this does not
> > > > cause any problems, because there is Qualcomm's LK bootloader running
> > > > before U-Boot which initializes the register with the correct value.
> > > >
> > > > It's important that this register is initialized correctly, because
> > > > the U-Boot driver does not make use of the BAM/DMA or single character
> > > > mode functionality of the UART controller. A different bootloader
> > > > before U-Boot might initialize the register differently.
> > > >
> > > > For example, on DragonBoard 410c U-Boot can also be installed to the
> > > > "aboot" partition (replacing LK entirely). In this case U-Boot is
> > > > loaded directly by SBL, which seems to use the single-character mode
> > > > for some reason. In single character mode there is always just one
> > > > char in the FIFO, instead of the 4 characters expected by
> > > > msm_serial_fetch(). It also causes issues with "earlycon" later in
> > > > the Linux kernel, which tries to output 4 chars at once,
> > > > but only the first char will be written.
> > > >
> > > > This causes early UART log in Linux to be corrupted like this:
> > > >
> > > >     [ 00ano:ameoi .Q1B[ 00ac _idaM00080oo'ahani-lcle._20). 15NdNii 5 SPMSJ20:U2
> > > >     [ 00rkoolmsamel
> > > >     [ 00Fw ]elamletopsioble
> > > >     [ 00ore
> > > >
> > > > instead of
> > > >
> > > >     [    0.000000] Booting Linux on physical CPU 0x0000000000 [0x410fd030]
> > > >     [    0.000000] Machine model: Qualcomm Technologies, Inc. APQ 8016 SBC
> > > >     [    0.000000] earlycon: msm_serial_dm0 at MMIO 0x00000000078b0000 (options '')
> > > >     [    0.000000] printk: bootconsole [msm_serial_dm0] enabled
> > > >
> > > > Make sure to initialize UART_DM_DMEN correctly to fix this issue
> > > > when loading U-Boot directly after SBL (instead of through LK).
> > > >
> > > > There is no functional difference when loading U-Boot through LK
> > > > since LK also initializes UART_DM_DMEN to 0x0. [1]
> > > >
> > > > [1]: https://git.linaro.org/landing-teams/working/qualcomm/lk.git/tree/platform/msm_shared/uart_dm.c?h=dragonboard410c-LA.BR.1.2.7-03810-8x16.0-linaro3#n203
> > > >
> > > > Cc: Ramon Fried <rfried.dev@gmail.com>
> > > > 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 d8c6c2f6b5..d8dd5c1104 100644
> > > > --- a/drivers/serial/serial_msm.c
> > > > +++ b/drivers/serial/serial_msm.c
> > > > @@ -23,6 +23,7 @@
> > > >  /* Serial registers - this driver works in uartdm mode*/
> > > >
> > > >  #define UARTDM_DMRX             0x34 /* Max RX transfer length */
> > > > +#define UARTDM_DMEN             0x3C /* DMA/data-packing mode */
> > > >  #define UARTDM_NCF_TX           0x40 /* Number of chars to TX */
> > > >
> > > >  #define UARTDM_RXFS             0x50 /* RX channel status register */
> > > > @@ -197,6 +198,9 @@ static void uart_dm_init(struct msm_serial_data *priv)
> > > >         writel(MSM_BOOT_UART_DM_8_N_1_MODE, priv->base + UARTDM_MR2);
> > > >         writel(MSM_BOOT_UART_DM_CMD_RESET_RX, priv->base + UARTDM_CR);
> > > >         writel(MSM_BOOT_UART_DM_CMD_RESET_TX, priv->base + UARTDM_CR);
> > > > +
> > > > +       /* Make sure BAM/single character mode is disabled */
> > > > +       writel(0x0, priv->base + UARTDM_DMEN);
> > > >  }
> > > >  static int msm_serial_probe(struct udevice *dev)
> > > >  {
> > > > --
> > > > 2.32.0
> > > >
> > > Hi.
> > > This is strange, I never encountered the behaviour, and I did boot
> > > Linux after U-boot without LK in the way.
> >
> > It happens for me if the boot flow is SBL -> U-Boot -> Linux instead of
> > SBL -> LK -> U-Boot -> Linux (The latter is the suggested setup
> > according to dragonboard410c_defconfig and the dragonboard410c
> > readme.txt, but I wanted to eliminate LK entirely).
> >
> > If you tried the same, perhaps you didn't have earlycon enabled?
> > It also happens only during early boot with earlycon enabled
> > ("earlycon" in kernel parameters). It stops happening later on boot
> > when the kernel fully re-initializes the UART controller. (The idea of
> > earlycon is to reuse the existing UART configuration to report errors
> > that occur very early during boot.)
> >
> > Thanks,
> > Stephan
> It's interesting, I wanted to eliminate LK entirely, but I encountered
> an error I couldn't understand.
> You can run U-boot compiled to 64bit and 32bit, in 64bit the boot is
> stuck for some reason, and Tom refused to merge a patch where a 32bit
> U-boot would boot a 64bit Linux.
> Did you compile U-boot for 64 or 32 ? How far did you get ?

Ah, right. I run U-Boot in 64-bit mode. But I think I'm familiar with
the problem you are talking about (Linux getting stuck when booting
U-Boot directly in 64-bit mode).

Basically it's a bug in the PSCI implementation on DragonBoard 410c
(part of the TrustZone/tz firmware). Unless you have *something* that
triggers the 32-bit -> 64-bit switch via the TZ syscalls, the PSCI
implementation will boot all other CPU cores in 32-bit mode (since it
does not know the switch to 64-bit has occurred).

I have a workaround for this in my open-source "hyp" firmware
replacement for DB410c: https://github.com/msm8916-mainline/qhypstub
(The README has further detailed information about the bug and the
 workaround, it's a bit hard to explain shortly...)

If you use this "hyp" firmware it shouldn't get stuck anymore.
As another nice side effect both U-Boot and Linux will start in EL2
so you can use KVM or other hypervisors. :)

Stephan

Re: [PATCH] serial: serial_msm: Ensure BAM/single character mode are disabled

[Ramon Fried]

On Mon, Jun 28, 2021 at 10:16 PM Stephan Gerhold <stephan@gerhold.net> wrote:
>
> On Mon, Jun 28, 2021 at 10:04:29PM +0300, Ramon Fried wrote:
> > On Mon, Jun 28, 2021 at 9:59 PM Stephan Gerhold <stephan@gerhold.net> wrote:
> > >
> > > On Mon, Jun 28, 2021 at 08:15:28PM +0300, Ramon Fried wrote:
> > > > On Mon, Jun 28, 2021 at 11:40 AM Stephan Gerhold <stephan@gerhold.net> wrote:
> > > > >
> > > > > At the moment, the U-Boot serial_msm driver does not initialize the
> > > > > UART_DM_DMEN register with the required value. Usually this does not
> > > > > cause any problems, because there is Qualcomm's LK bootloader running
> > > > > before U-Boot which initializes the register with the correct value.
> > > > >
> > > > > It's important that this register is initialized correctly, because
> > > > > the U-Boot driver does not make use of the BAM/DMA or single character
> > > > > mode functionality of the UART controller. A different bootloader
> > > > > before U-Boot might initialize the register differently.
> > > > >
> > > > > For example, on DragonBoard 410c U-Boot can also be installed to the
> > > > > "aboot" partition (replacing LK entirely). In this case U-Boot is
> > > > > loaded directly by SBL, which seems to use the single-character mode
> > > > > for some reason. In single character mode there is always just one
> > > > > char in the FIFO, instead of the 4 characters expected by
> > > > > msm_serial_fetch(). It also causes issues with "earlycon" later in
> > > > > the Linux kernel, which tries to output 4 chars at once,
> > > > > but only the first char will be written.
> > > > >
> > > > > This causes early UART log in Linux to be corrupted like this:
> > > > >
> > > > >     [ 00ano:ameoi .Q1B[ 00ac _idaM00080oo'ahani-lcle._20). 15NdNii 5 SPMSJ20:U2
> > > > >     [ 00rkoolmsamel
> > > > >     [ 00Fw ]elamletopsioble
> > > > >     [ 00ore
> > > > >
> > > > > instead of
> > > > >
> > > > >     [    0.000000] Booting Linux on physical CPU 0x0000000000 [0x410fd030]
> > > > >     [    0.000000] Machine model: Qualcomm Technologies, Inc. APQ 8016 SBC
> > > > >     [    0.000000] earlycon: msm_serial_dm0 at MMIO 0x00000000078b0000 (options '')
> > > > >     [    0.000000] printk: bootconsole [msm_serial_dm0] enabled
> > > > >
> > > > > Make sure to initialize UART_DM_DMEN correctly to fix this issue
> > > > > when loading U-Boot directly after SBL (instead of through LK).
> > > > >
> > > > > There is no functional difference when loading U-Boot through LK
> > > > > since LK also initializes UART_DM_DMEN to 0x0. [1]
> > > > >
> > > > > [1]: https://git.linaro.org/landing-teams/working/qualcomm/lk.git/tree/platform/msm_shared/uart_dm.c?h=dragonboard410c-LA.BR.1.2.7-03810-8x16.0-linaro3#n203
> > > > >
> > > > > Cc: Ramon Fried <rfried.dev@gmail.com>
> > > > > 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 d8c6c2f6b5..d8dd5c1104 100644
> > > > > --- a/drivers/serial/serial_msm.c
> > > > > +++ b/drivers/serial/serial_msm.c
> > > > > @@ -23,6 +23,7 @@
> > > > >  /* Serial registers - this driver works in uartdm mode*/
> > > > >
> > > > >  #define UARTDM_DMRX             0x34 /* Max RX transfer length */
> > > > > +#define UARTDM_DMEN             0x3C /* DMA/data-packing mode */
> > > > >  #define UARTDM_NCF_TX           0x40 /* Number of chars to TX */
> > > > >
> > > > >  #define UARTDM_RXFS             0x50 /* RX channel status register */
> > > > > @@ -197,6 +198,9 @@ static void uart_dm_init(struct msm_serial_data *priv)
> > > > >         writel(MSM_BOOT_UART_DM_8_N_1_MODE, priv->base + UARTDM_MR2);
> > > > >         writel(MSM_BOOT_UART_DM_CMD_RESET_RX, priv->base + UARTDM_CR);
> > > > >         writel(MSM_BOOT_UART_DM_CMD_RESET_TX, priv->base + UARTDM_CR);
> > > > > +
> > > > > +       /* Make sure BAM/single character mode is disabled */
> > > > > +       writel(0x0, priv->base + UARTDM_DMEN);
> > > > >  }
> > > > >  static int msm_serial_probe(struct udevice *dev)
> > > > >  {
> > > > > --
> > > > > 2.32.0
> > > > >
> > > > Hi.
> > > > This is strange, I never encountered the behaviour, and I did boot
> > > > Linux after U-boot without LK in the way.
> > >
> > > It happens for me if the boot flow is SBL -> U-Boot -> Linux instead of
> > > SBL -> LK -> U-Boot -> Linux (The latter is the suggested setup
> > > according to dragonboard410c_defconfig and the dragonboard410c
> > > readme.txt, but I wanted to eliminate LK entirely).
> > >
> > > If you tried the same, perhaps you didn't have earlycon enabled?
> > > It also happens only during early boot with earlycon enabled
> > > ("earlycon" in kernel parameters). It stops happening later on boot
> > > when the kernel fully re-initializes the UART controller. (The idea of
> > > earlycon is to reuse the existing UART configuration to report errors
> > > that occur very early during boot.)
> > >
> > > Thanks,
> > > Stephan
> > It's interesting, I wanted to eliminate LK entirely, but I encountered
> > an error I couldn't understand.
> > You can run U-boot compiled to 64bit and 32bit, in 64bit the boot is
> > stuck for some reason, and Tom refused to merge a patch where a 32bit
> > U-boot would boot a 64bit Linux.
> > Did you compile U-boot for 64 or 32 ? How far did you get ?
>
> Ah, right. I run U-Boot in 64-bit mode. But I think I'm familiar with
> the problem you are talking about (Linux getting stuck when booting
> U-Boot directly in 64-bit mode).
>
> Basically it's a bug in the PSCI implementation on DragonBoard 410c
> (part of the TrustZone/tz firmware). Unless you have *something* that
> triggers the 32-bit -> 64-bit switch via the TZ syscalls, the PSCI
> implementation will boot all other CPU cores in 32-bit mode (since it
> does not know the switch to 64-bit has occurred).
Wow, I was completely puzzled by that, thanks for sharing the info,
Maybe I'll add a quirk to U-boot so we can ditch LK for good.
Thanks,
Ramon
>
> I have a workaround for this in my open-source "hyp" firmware
> replacement for DB410c: https://github.com/msm8916-mainline/qhypstub
> (The README has further detailed information about the bug and the
>  workaround, it's a bit hard to explain shortly...)
Cool, thanks.
>
> If you use this "hyp" firmware it shouldn't get stuck anymore.
> As another nice side effect both U-Boot and Linux will start in EL2
> so you can use KVM or other hypervisors. :)
>
> Stephan
Beaut

Re: [PATCH] serial: serial_msm: Ensure BAM/single character mode are disabled

[Stephan Gerhold]

On Tue, Jun 29, 2021 at 08:16:09AM +0300, Ramon Fried wrote:
> On Mon, Jun 28, 2021 at 10:16 PM Stephan Gerhold <stephan@gerhold.net> wrote:
> >
> > On Mon, Jun 28, 2021 at 10:04:29PM +0300, Ramon Fried wrote:
> > > On Mon, Jun 28, 2021 at 9:59 PM Stephan Gerhold <stephan@gerhold.net> wrote:
> > > >
> > > > On Mon, Jun 28, 2021 at 08:15:28PM +0300, Ramon Fried wrote:
> > > > > On Mon, Jun 28, 2021 at 11:40 AM Stephan Gerhold <stephan@gerhold.net> wrote:
> > > > > >
> > > > > > At the moment, the U-Boot serial_msm driver does not initialize the
> > > > > > UART_DM_DMEN register with the required value. Usually this does not
> > > > > > cause any problems, because there is Qualcomm's LK bootloader running
> > > > > > before U-Boot which initializes the register with the correct value.
> > > > > >
> > > > > > It's important that this register is initialized correctly, because
> > > > > > the U-Boot driver does not make use of the BAM/DMA or single character
> > > > > > mode functionality of the UART controller. A different bootloader
> > > > > > before U-Boot might initialize the register differently.
> > > > > >
> > > > > > For example, on DragonBoard 410c U-Boot can also be installed to the
> > > > > > "aboot" partition (replacing LK entirely). In this case U-Boot is
> > > > > > loaded directly by SBL, which seems to use the single-character mode
> > > > > > for some reason. In single character mode there is always just one
> > > > > > char in the FIFO, instead of the 4 characters expected by
> > > > > > msm_serial_fetch(). It also causes issues with "earlycon" later in
> > > > > > the Linux kernel, which tries to output 4 chars at once,
> > > > > > but only the first char will be written.
> > > > > >
> > > > > > This causes early UART log in Linux to be corrupted like this:
> > > > > >
> > > > > >     [ 00ano:ameoi .Q1B[ 00ac _idaM00080oo'ahani-lcle._20). 15NdNii 5 SPMSJ20:U2
> > > > > >     [ 00rkoolmsamel
> > > > > >     [ 00Fw ]elamletopsioble
> > > > > >     [ 00ore
> > > > > >
> > > > > > instead of
> > > > > >
> > > > > >     [    0.000000] Booting Linux on physical CPU 0x0000000000 [0x410fd030]
> > > > > >     [    0.000000] Machine model: Qualcomm Technologies, Inc. APQ 8016 SBC
> > > > > >     [    0.000000] earlycon: msm_serial_dm0 at MMIO 0x00000000078b0000 (options '')
> > > > > >     [    0.000000] printk: bootconsole [msm_serial_dm0] enabled
> > > > > >
> > > > > > Make sure to initialize UART_DM_DMEN correctly to fix this issue
> > > > > > when loading U-Boot directly after SBL (instead of through LK).
> > > > > >
> > > > > > There is no functional difference when loading U-Boot through LK
> > > > > > since LK also initializes UART_DM_DMEN to 0x0. [1]
> > > > > >
> > > > > > [1]: https://git.linaro.org/landing-teams/working/qualcomm/lk.git/tree/platform/msm_shared/uart_dm.c?h=dragonboard410c-LA.BR.1.2.7-03810-8x16.0-linaro3#n203
> > > > > >
> > > > > > Cc: Ramon Fried <rfried.dev@gmail.com>
> > > > > > 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 d8c6c2f6b5..d8dd5c1104 100644
> > > > > > --- a/drivers/serial/serial_msm.c
> > > > > > +++ b/drivers/serial/serial_msm.c
> > > > > > @@ -23,6 +23,7 @@
> > > > > >  /* Serial registers - this driver works in uartdm mode*/
> > > > > >
> > > > > >  #define UARTDM_DMRX             0x34 /* Max RX transfer length */
> > > > > > +#define UARTDM_DMEN             0x3C /* DMA/data-packing mode */
> > > > > >  #define UARTDM_NCF_TX           0x40 /* Number of chars to TX */
> > > > > >
> > > > > >  #define UARTDM_RXFS             0x50 /* RX channel status register */
> > > > > > @@ -197,6 +198,9 @@ static void uart_dm_init(struct msm_serial_data *priv)
> > > > > >         writel(MSM_BOOT_UART_DM_8_N_1_MODE, priv->base + UARTDM_MR2);
> > > > > >         writel(MSM_BOOT_UART_DM_CMD_RESET_RX, priv->base + UARTDM_CR);
> > > > > >         writel(MSM_BOOT_UART_DM_CMD_RESET_TX, priv->base + UARTDM_CR);
> > > > > > +
> > > > > > +       /* Make sure BAM/single character mode is disabled */
> > > > > > +       writel(0x0, priv->base + UARTDM_DMEN);
> > > > > >  }
> > > > > >  static int msm_serial_probe(struct udevice *dev)
> > > > > >  {
> > > > > > --
> > > > > > 2.32.0
> > > > > >
> > > > > Hi.
> > > > > This is strange, I never encountered the behaviour, and I did boot
> > > > > Linux after U-boot without LK in the way.
> > > >
> > > > It happens for me if the boot flow is SBL -> U-Boot -> Linux instead of
> > > > SBL -> LK -> U-Boot -> Linux (The latter is the suggested setup
> > > > according to dragonboard410c_defconfig and the dragonboard410c
> > > > readme.txt, but I wanted to eliminate LK entirely).
> > > >
> > > > If you tried the same, perhaps you didn't have earlycon enabled?
> > > > It also happens only during early boot with earlycon enabled
> > > > ("earlycon" in kernel parameters). It stops happening later on boot
> > > > when the kernel fully re-initializes the UART controller. (The idea of
> > > > earlycon is to reuse the existing UART configuration to report errors
> > > > that occur very early during boot.)
> > > >
> > > > Thanks,
> > > > Stephan
> > > It's interesting, I wanted to eliminate LK entirely, but I encountered
> > > an error I couldn't understand.
> > > You can run U-boot compiled to 64bit and 32bit, in 64bit the boot is
> > > stuck for some reason, and Tom refused to merge a patch where a 32bit
> > > U-boot would boot a 64bit Linux.
> > > Did you compile U-boot for 64 or 32 ? How far did you get ?
> >
> > Ah, right. I run U-Boot in 64-bit mode. But I think I'm familiar with
> > the problem you are talking about (Linux getting stuck when booting
> > U-Boot directly in 64-bit mode).
> >
> > Basically it's a bug in the PSCI implementation on DragonBoard 410c
> > (part of the TrustZone/tz firmware). Unless you have *something* that
> > triggers the 32-bit -> 64-bit switch via the TZ syscalls, the PSCI
> > implementation will boot all other CPU cores in 32-bit mode (since it
> > does not know the switch to 64-bit has occurred).
>
> Wow, I was completely puzzled by that, thanks for sharing the info,
> Maybe I'll add a quirk to U-boot so we can ditch LK for good.
>

It should be possible to add a similar workaround in U-Boot directly,
although I suspect this might conflict with the one in qhypstub.
I realize qhypstub is unofficial but on the other hand I'm not aware of
any disadvantages when using it. So I'm not sure if duplicating the
workaround into U-Boot is worth it.

But I'm open for discussion on this, we can probably find a way to make
it work in all cases if that's what we want.

I was going to make a separate mail to discuss making U-Boot independent
of LK. Will probably do that later and document the exact steps I'm
using there so we don't have all the discussion on this simple patch. :)

Thanks!
Stephan

Re: [PATCH] serial: serial_msm: Ensure BAM/single character mode are disabled

[Ramon Fried]

Sounds good to me.

On Tue, Jun 29, 2021 at 2:06 PM Stephan Gerhold <stephan@gerhold.net> wrote:
>
> On Tue, Jun 29, 2021 at 08:16:09AM +0300, Ramon Fried wrote:
> > On Mon, Jun 28, 2021 at 10:16 PM Stephan Gerhold <stephan@gerhold.net> wrote:
> > >
> > > On Mon, Jun 28, 2021 at 10:04:29PM +0300, Ramon Fried wrote:
> > > > On Mon, Jun 28, 2021 at 9:59 PM Stephan Gerhold <stephan@gerhold.net> wrote:
> > > > >
> > > > > On Mon, Jun 28, 2021 at 08:15:28PM +0300, Ramon Fried wrote:
> > > > > > On Mon, Jun 28, 2021 at 11:40 AM Stephan Gerhold <stephan@gerhold.net> wrote:
> > > > > > >
> > > > > > > At the moment, the U-Boot serial_msm driver does not initialize the
> > > > > > > UART_DM_DMEN register with the required value. Usually this does not
> > > > > > > cause any problems, because there is Qualcomm's LK bootloader running
> > > > > > > before U-Boot which initializes the register with the correct value.
> > > > > > >
> > > > > > > It's important that this register is initialized correctly, because
> > > > > > > the U-Boot driver does not make use of the BAM/DMA or single character
> > > > > > > mode functionality of the UART controller. A different bootloader
> > > > > > > before U-Boot might initialize the register differently.
> > > > > > >
> > > > > > > For example, on DragonBoard 410c U-Boot can also be installed to the
> > > > > > > "aboot" partition (replacing LK entirely). In this case U-Boot is
> > > > > > > loaded directly by SBL, which seems to use the single-character mode
> > > > > > > for some reason. In single character mode there is always just one
> > > > > > > char in the FIFO, instead of the 4 characters expected by
> > > > > > > msm_serial_fetch(). It also causes issues with "earlycon" later in
> > > > > > > the Linux kernel, which tries to output 4 chars at once,
> > > > > > > but only the first char will be written.
> > > > > > >
> > > > > > > This causes early UART log in Linux to be corrupted like this:
> > > > > > >
> > > > > > >     [ 00ano:ameoi .Q1B[ 00ac _idaM00080oo'ahani-lcle._20). 15NdNii 5 SPMSJ20:U2
> > > > > > >     [ 00rkoolmsamel
> > > > > > >     [ 00Fw ]elamletopsioble
> > > > > > >     [ 00ore
> > > > > > >
> > > > > > > instead of
> > > > > > >
> > > > > > >     [    0.000000] Booting Linux on physical CPU 0x0000000000 [0x410fd030]
> > > > > > >     [    0.000000] Machine model: Qualcomm Technologies, Inc. APQ 8016 SBC
> > > > > > >     [    0.000000] earlycon: msm_serial_dm0 at MMIO 0x00000000078b0000 (options '')
> > > > > > >     [    0.000000] printk: bootconsole [msm_serial_dm0] enabled
> > > > > > >
> > > > > > > Make sure to initialize UART_DM_DMEN correctly to fix this issue
> > > > > > > when loading U-Boot directly after SBL (instead of through LK).
> > > > > > >
> > > > > > > There is no functional difference when loading U-Boot through LK
> > > > > > > since LK also initializes UART_DM_DMEN to 0x0. [1]
> > > > > > >
> > > > > > > [1]: https://git.linaro.org/landing-teams/working/qualcomm/lk.git/tree/platform/msm_shared/uart_dm.c?h=dragonboard410c-LA.BR.1.2.7-03810-8x16.0-linaro3#n203
> > > > > > >
> > > > > > > Cc: Ramon Fried <rfried.dev@gmail.com>
> > > > > > > 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 d8c6c2f6b5..d8dd5c1104 100644
> > > > > > > --- a/drivers/serial/serial_msm.c
> > > > > > > +++ b/drivers/serial/serial_msm.c
> > > > > > > @@ -23,6 +23,7 @@
> > > > > > >  /* Serial registers - this driver works in uartdm mode*/
> > > > > > >
> > > > > > >  #define UARTDM_DMRX             0x34 /* Max RX transfer length */
> > > > > > > +#define UARTDM_DMEN             0x3C /* DMA/data-packing mode */
> > > > > > >  #define UARTDM_NCF_TX           0x40 /* Number of chars to TX */
> > > > > > >
> > > > > > >  #define UARTDM_RXFS             0x50 /* RX channel status register */
> > > > > > > @@ -197,6 +198,9 @@ static void uart_dm_init(struct msm_serial_data *priv)
> > > > > > >         writel(MSM_BOOT_UART_DM_8_N_1_MODE, priv->base + UARTDM_MR2);
> > > > > > >         writel(MSM_BOOT_UART_DM_CMD_RESET_RX, priv->base + UARTDM_CR);
> > > > > > >         writel(MSM_BOOT_UART_DM_CMD_RESET_TX, priv->base + UARTDM_CR);
> > > > > > > +
> > > > > > > +       /* Make sure BAM/single character mode is disabled */
> > > > > > > +       writel(0x0, priv->base + UARTDM_DMEN);
> > > > > > >  }
> > > > > > >  static int msm_serial_probe(struct udevice *dev)
> > > > > > >  {
> > > > > > > --
> > > > > > > 2.32.0
> > > > > > >
> > > > > > Hi.
> > > > > > This is strange, I never encountered the behaviour, and I did boot
> > > > > > Linux after U-boot without LK in the way.
> > > > >
> > > > > It happens for me if the boot flow is SBL -> U-Boot -> Linux instead of
> > > > > SBL -> LK -> U-Boot -> Linux (The latter is the suggested setup
> > > > > according to dragonboard410c_defconfig and the dragonboard410c
> > > > > readme.txt, but I wanted to eliminate LK entirely).
> > > > >
> > > > > If you tried the same, perhaps you didn't have earlycon enabled?
> > > > > It also happens only during early boot with earlycon enabled
> > > > > ("earlycon" in kernel parameters). It stops happening later on boot
> > > > > when the kernel fully re-initializes the UART controller. (The idea of
> > > > > earlycon is to reuse the existing UART configuration to report errors
> > > > > that occur very early during boot.)
> > > > >
> > > > > Thanks,
> > > > > Stephan
> > > > It's interesting, I wanted to eliminate LK entirely, but I encountered
> > > > an error I couldn't understand.
> > > > You can run U-boot compiled to 64bit and 32bit, in 64bit the boot is
> > > > stuck for some reason, and Tom refused to merge a patch where a 32bit
> > > > U-boot would boot a 64bit Linux.
> > > > Did you compile U-boot for 64 or 32 ? How far did you get ?
> > >
> > > Ah, right. I run U-Boot in 64-bit mode. But I think I'm familiar with
> > > the problem you are talking about (Linux getting stuck when booting
> > > U-Boot directly in 64-bit mode).
> > >
> > > Basically it's a bug in the PSCI implementation on DragonBoard 410c
> > > (part of the TrustZone/tz firmware). Unless you have *something* that
> > > triggers the 32-bit -> 64-bit switch via the TZ syscalls, the PSCI
> > > implementation will boot all other CPU cores in 32-bit mode (since it
> > > does not know the switch to 64-bit has occurred).
> >
> > Wow, I was completely puzzled by that, thanks for sharing the info,
> > Maybe I'll add a quirk to U-boot so we can ditch LK for good.
> >
>
> It should be possible to add a similar workaround in U-Boot directly,
> although I suspect this might conflict with the one in qhypstub.
> I realize qhypstub is unofficial but on the other hand I'm not aware of
> any disadvantages when using it. So I'm not sure if duplicating the
> workaround into U-Boot is worth it.
>
> But I'm open for discussion on this, we can probably find a way to make
> it work in all cases if that's what we want.
>
> I was going to make a separate mail to discuss making U-Boot independent
> of LK. Will probably do that later and document the exact steps I'm
> using there so we don't have all the discussion on this simple patch. :)
>
> Thanks!
> Stephan

Re: [PATCH] serial: serial_msm: Ensure BAM/single character mode are disabled

[Ramon Fried]

On Mon, Jun 28, 2021 at 11:40 AM Stephan Gerhold <stephan@gerhold.net> wrote:
>
> At the moment, the U-Boot serial_msm driver does not initialize the
> UART_DM_DMEN register with the required value. Usually this does not
> cause any problems, because there is Qualcomm's LK bootloader running
> before U-Boot which initializes the register with the correct value.
>
> It's important that this register is initialized correctly, because
> the U-Boot driver does not make use of the BAM/DMA or single character
> mode functionality of the UART controller. A different bootloader
> before U-Boot might initialize the register differently.
>
> For example, on DragonBoard 410c U-Boot can also be installed to the
> "aboot" partition (replacing LK entirely). In this case U-Boot is
> loaded directly by SBL, which seems to use the single-character mode
> for some reason. In single character mode there is always just one
> char in the FIFO, instead of the 4 characters expected by
> msm_serial_fetch(). It also causes issues with "earlycon" later in
> the Linux kernel, which tries to output 4 chars at once,
> but only the first char will be written.
>
> This causes early UART log in Linux to be corrupted like this:
>
>     [ 00ano:ameoi .Q1B[ 00ac _idaM00080oo'ahani-lcle._20). 15NdNii 5 SPMSJ20:U2
>     [ 00rkoolmsamel
>     [ 00Fw ]elamletopsioble
>     [ 00ore
>
> instead of
>
>     [    0.000000] Booting Linux on physical CPU 0x0000000000 [0x410fd030]
>     [    0.000000] Machine model: Qualcomm Technologies, Inc. APQ 8016 SBC
>     [    0.000000] earlycon: msm_serial_dm0 at MMIO 0x00000000078b0000 (options '')
>     [    0.000000] printk: bootconsole [msm_serial_dm0] enabled
>
> Make sure to initialize UART_DM_DMEN correctly to fix this issue
> when loading U-Boot directly after SBL (instead of through LK).
>
> There is no functional difference when loading U-Boot through LK
> since LK also initializes UART_DM_DMEN to 0x0. [1]
>
> [1]: https://git.linaro.org/landing-teams/working/qualcomm/lk.git/tree/platform/msm_shared/uart_dm.c?h=dragonboard410c-LA.BR.1.2.7-03810-8x16.0-linaro3#n203
>
> Cc: Ramon Fried <rfried.dev@gmail.com>
> 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 d8c6c2f6b5..d8dd5c1104 100644
> --- a/drivers/serial/serial_msm.c
> +++ b/drivers/serial/serial_msm.c
> @@ -23,6 +23,7 @@
>  /* Serial registers - this driver works in uartdm mode*/
>
>  #define UARTDM_DMRX             0x34 /* Max RX transfer length */
> +#define UARTDM_DMEN             0x3C /* DMA/data-packing mode */
>  #define UARTDM_NCF_TX           0x40 /* Number of chars to TX */
>
>  #define UARTDM_RXFS             0x50 /* RX channel status register */
> @@ -197,6 +198,9 @@ static void uart_dm_init(struct msm_serial_data *priv)
>         writel(MSM_BOOT_UART_DM_8_N_1_MODE, priv->base + UARTDM_MR2);
>         writel(MSM_BOOT_UART_DM_CMD_RESET_RX, priv->base + UARTDM_CR);
>         writel(MSM_BOOT_UART_DM_CMD_RESET_TX, priv->base + UARTDM_CR);
> +
> +       /* Make sure BAM/single character mode is disabled */
> +       writel(0x0, priv->base + UARTDM_DMEN);
>  }
>  static int msm_serial_probe(struct udevice *dev)
>  {
> --
> 2.32.0
>
Reviewed-by: Ramon Fried <rfried.dev@gmail.com>

Re: [PATCH] serial: serial_msm: Ensure BAM/single character mode are disabled

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 2345 bytes --]

On Mon, Jun 28, 2021 at 10:40:09AM +0200, Stephan Gerhold wrote:

> At the moment, the U-Boot serial_msm driver does not initialize the
> UART_DM_DMEN register with the required value. Usually this does not
> cause any problems, because there is Qualcomm's LK bootloader running
> before U-Boot which initializes the register with the correct value.
> 
> It's important that this register is initialized correctly, because
> the U-Boot driver does not make use of the BAM/DMA or single character
> mode functionality of the UART controller. A different bootloader
> before U-Boot might initialize the register differently.
> 
> For example, on DragonBoard 410c U-Boot can also be installed to the
> "aboot" partition (replacing LK entirely). In this case U-Boot is
> loaded directly by SBL, which seems to use the single-character mode
> for some reason. In single character mode there is always just one
> char in the FIFO, instead of the 4 characters expected by
> msm_serial_fetch(). It also causes issues with "earlycon" later in
> the Linux kernel, which tries to output 4 chars at once,
> but only the first char will be written.
> 
> This causes early UART log in Linux to be corrupted like this:
> 
>     [ 00ano:ameoi .Q1B[ 00ac _idaM00080oo'ahani-lcle._20). 15NdNii 5 SPMSJ20:U2
>     [ 00rkoolmsamel
>     [ 00Fw ]elamletopsioble
>     [ 00ore
> 
> instead of
> 
>     [    0.000000] Booting Linux on physical CPU 0x0000000000 [0x410fd030]
>     [    0.000000] Machine model: Qualcomm Technologies, Inc. APQ 8016 SBC
>     [    0.000000] earlycon: msm_serial_dm0 at MMIO 0x00000000078b0000 (options '')
>     [    0.000000] printk: bootconsole [msm_serial_dm0] enabled
> 
> Make sure to initialize UART_DM_DMEN correctly to fix this issue
> when loading U-Boot directly after SBL (instead of through LK).
> 
> There is no functional difference when loading U-Boot through LK
> since LK also initializes UART_DM_DMEN to 0x0. [1]
> 
> [1]: https://git.linaro.org/landing-teams/working/qualcomm/lk.git/tree/platform/msm_shared/uart_dm.c?h=dragonboard410c-LA.BR.1.2.7-03810-8x16.0-linaro3#n203
> 
> Cc: Ramon Fried <rfried.dev@gmail.com>
> Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
> Reviewed-by: Ramon Fried <rfried.dev@gmail.com>

Applied to u-boot/master, thanks!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]