[PATCH v5 2/2] serial: mxc: have putc use the TXFIFO
TERTYCHNYI Grygorii
grygorii.tertychnyi at leica-geosystems.com
Thu Nov 3 09:35:27 CET 2022
I think Pali is right,
mxc_serial_setbrg() is called _after_ board_init(), and because setbrg()
touches bmr and cr registers it causes loosing of FIFO - even for the
very long string, "only" last 32 chars are potentially corrupted.
Here I printed bmr value before and after (looks like setbrg() is called twice after board_init):
==========================================
U-Boot SPL 2022.10-dirty (Nov 03 2022 - 09:21:16 +0100)
No pmic
SEC0: RNG instantiated
Normal Boot
WDT: Started watchdog at 30280000 with servicing (60s timeout)
Trying to boot from MMC1
NOTICE: BL31: v2.2(release):rel_imx_5.4.47_2.2.0-0-gc949a888e909
NOTICE: BL31: Built : 15:21:46, Nov 2 2022
before: 0x0
after: 0x68
before: 0x68
after: 0x68
U-Boot 2022.10-dirty (Nov 03 2022 - 09:21:16 +0100)
CPU: Freescale i.MX8MMQ rev1.0 at 1200 MHz
Reset cause: POR
Model: FSL i.MX8MM EVK board
DRAM: 2 GiB
1:12345678901234567890123456789012:FIFO*****+
2:12345678901234567890123456789012:FIFO*****+
3:12345678901234567890123456789012:FIFO*****+
4:12345678901234567890123456789012:FIFO
before: 0x0
after: 0x68
before: 0x68
after: 0x68
Core: 160 devices, 20 uclasses, devicetree: separate
WDT: Started watchdog at 30280000 with servicing (60s timeout)
MMC: FSL_SDHC: 1, FSL_SDHC: 2
Loading Environment from MMC... *** Warning - bad CRC, using default environment
In: serial at 30890000
Out: serial at 30890000
Err: serial at 30890000
SEC0: RNG instantiated
Net: eth0: ethernet at 30be0000
Hit any key to stop autoboot: 0
==========================================
diff --git a/board/freescale/imx8mm_evk/imx8mm_evk.c b/board/freescale/imx8mm_evk/imx8mm_evk.c
index e0975fcda705..837f1286b4e8 100644
--- a/board/freescale/imx8mm_evk/imx8mm_evk.c
+++ b/board/freescale/imx8mm_evk/imx8mm_evk.c
@@ -50,6 +50,11 @@ int board_init(void)
if (IS_ENABLED(CONFIG_FEC_MXC))
setup_fec();
+ puts("1:12345678901234567890123456789012:FIFO*****+\n");
+ puts("2:12345678901234567890123456789012:FIFO*****+\n");
+ puts("3:12345678901234567890123456789012:FIFO*****+\n");
+ puts("4:12345678901234567890123456789012:FIFO*****+\n");
+
return 0;
}
diff --git a/drivers/serial/serial_mxc.c b/drivers/serial/serial_mxc.c
index af1fd1ea9bc7..daadd0605123 100644
--- a/drivers/serial/serial_mxc.c
+++ b/drivers/serial/serial_mxc.c
@@ -164,10 +164,25 @@ static void _mxc_serial_init(struct mxc_uart *base, int use_dte)
writel(0, &base->ts);
}
+static void print_str(struct mxc_uart *base, const char *str)
+{
+ while (*str) {
+ writel(*str, &base->txd);
+ while (!(readl(&base->ts) & UTS_TXEMPTY)) {
+ ; /* wait */
+ }
+ ++str;
+ }
+}
+
static void _mxc_serial_setbrg(struct mxc_uart *base, unsigned long clk,
unsigned long baudrate, bool use_dte)
{
u32 tmp;
+ u32 bmr;
+ char bmrstr[16];
+
+ bmr = readl(&base->bmr);
tmp = RFDIV << UFCR_RFDIV_SHF;
if (use_dte)
@@ -190,6 +205,15 @@ static void _mxc_serial_setbrg(struct mxc_uart *base, unsigned long clk,
writel(readl(&base->cr3) | UCR3_RXDMUXSEL, &base->cr3);
writel(UCR1_UARTEN, &base->cr1);
+
+ sprintf(bmrstr, "0x%x", bmr);
+ print_str(base, "\nbefore: ");
+ print_str(base, bmrstr);
+
+ sprintf(bmrstr, "0x%x", readl(&base->bmr));
+ print_str(base, "\nafter: ");
+ print_str(base, bmrstr);
+ print_str(base, "\n");
}
#if !CONFIG_IS_ENABLED(DM_SERIAL)
If board_init() flushes FIFO, everything works. Unfortunately, it means,
FIFO cannot be used unless CONFIG_SERIAL_PUTS is enabled?
diff --git a/board/freescale/imx8mm_evk/imx8mm_evk.c b/board/freescale/imx8mm_evk/imx8mm_evk.c
index e0975fcda705..1c50db7789ec 100644
--- a/board/freescale/imx8mm_evk/imx8mm_evk.c
+++ b/board/freescale/imx8mm_evk/imx8mm_evk.c
@@ -50,6 +50,12 @@ int board_init(void)
if (IS_ENABLED(CONFIG_FEC_MXC))
setup_fec();
+ puts("12345678901234567890123456789012:");
+ puts("12345678901234567890123456789012:");
+ puts("12345678901234567890123456789012:");
+ puts("12345678901234567890123456789012:");
+ puts("%");
+
return 0;
}
diff --git a/drivers/serial/serial_mxc.c b/drivers/serial/serial_mxc.c
index af1fd1ea9bc7..0e85ecaae9c0 100644
--- a/drivers/serial/serial_mxc.c
+++ b/drivers/serial/serial_mxc.c
@@ -316,6 +316,13 @@ static int mxc_serial_putc(struct udevice *dev, const char ch)
writel(ch, &uart->txd);
+ if (ch == '%') {
+ /* flush FIFO */
+ while (!(readl(&uart->ts) & UTS_TXEMPTY)) {
+ ; /* wait */
+ }
+ }
+
return 0;
}
Regards,
Grygorii
________________________________________
From: SCHNEIDER Johannes <johannes.schneider at leica-geosystems.com>
Sent: Thursday, November 3, 2022 07:17
To: Fabio Estevam; Pali Rohár
Cc: Fabio Estevam; Tim Harvey; u-boot at lists.denx.de; peng.fan at oss.nxp.com; sbabic at denx.de; trini; GEO-CHHER-bsp-development; Peng Fan
Subject: Re: [PATCH v5 2/2] serial: mxc: have putc use the TXFIFO
Hi all,
flushing and waiting... maybe you're onto something: what if one printf races another since it thinks considers its buffer handed to the FIFO as "done" a bit too soon?
might the below variation on "waiting for the fifo" solve the symptoms on imx6?
regards
Johannes
diff --git a/drivers/serial/serial_mxc.c b/drivers/serial/serial_mxc.c
index 4207650503..dfd7670f7e 100644
--- a/drivers/serial/serial_mxc.c
+++ b/drivers/serial/serial_mxc.c
@@ -329,8 +329,23 @@ static int mxc_serial_pending(struct udevice *dev, bool input)
return sr2 & USR2_TXDC ? 0 : 1;
}
+static ssize_t mxc_serial_puts(struct udevice *dev, const char *s, size_t len)
+{
+ struct mxc_serial_plat *plat = dev_get_plat(dev);
+ struct mxc_uart *const uart = plat->reg;
+
+ while (*s)
+ mcx_serial_putc(dev, *s++);
+
+ // flush the TXFIFO
+ while(mxc_serial_pending(dev, false));
+
+ return len;
+}
+
static const struct dm_serial_ops mxc_serial_ops = {
.putc = mxc_serial_putc,
+ .puts = mxc_serial_puts,
.pending = mxc_serial_pending,
.getc = mxc_serial_getc,
.setbrg = mxc_serial_setbrg,
________________________________________
From: Fabio Estevam <festevam at gmail.com>
Sent: Wednesday, November 2, 2022 19:37
To: Pali Rohár
Cc: Fabio Estevam; SCHNEIDER Johannes; Tim Harvey; u-boot at lists.denx.de; peng.fan at oss.nxp.com; sbabic at denx.de; trini; GEO-CHHER-bsp-development; Peng Fan
Subject: Re: [PATCH v5 2/2] serial: mxc: have putc use the TXFIFO
This email is not from Hexagon’s Office 365 instance. Please be careful while clicking links, opening attachments, or replying to this email.
On Wed, Nov 2, 2022 at 2:24 PM Pali Rohár <pali at kernel.org> wrote:
> Hello! This log just cleanly shows that UART TX FIFO is either broken or
> something drops its content prior all bytes are properly transmitted.
> Dropping HW TX FIFO is on most UARTs possible by resetting registers or
> reconfiguring buadrate.
>
> I have an idea, cannot some u-boot code calls some mxc function which
> changes parameters of UART and this will cause loosing of FIFO?
>
> For example I see two functions which are doing it. Could you try to add
> code which waits until content of FIFO is transmitted prior changing
> UART parameters?
>
> diff --git a/drivers/serial/serial_mxc.c b/drivers/serial/serial_mxc.c
> index 4cf79c1ca24f..9611d9bc8a00 100644
> --- a/drivers/serial/serial_mxc.c
> +++ b/drivers/serial/serial_mxc.c
> @@ -146,6 +146,9 @@ struct mxc_uart {
>
> static void _mxc_serial_init(struct mxc_uart *base, int use_dte)
> {
> + while (!(readl(&base->ts) & UTS_TXEMPTY))
> + ;
> +
> writel(0, &base->cr1);
> writel(0, &base->cr2);
>
> @@ -169,6 +172,9 @@ static void _mxc_serial_setbrg(struct mxc_uart *base, unsigned long clk,
> {
> u32 tmp;
>
> + while (!(readl(&base->ts) & UTS_TXEMPTY))
> + ;
> +
Tried your suggestion, but the print() content I added inside
board_init() is no longer printed.
More information about the U-Boot
mailing list