[PATCH v3 1/2] serial: mxc: Wait for TX completion before reset
The u-boot console may show some corrupted characters when printing in board_init() due to reset of the UART (probe) before the TX FIFO has been completely drained.
To fix this issue, and in case UART is still running, we now try to flush the FIFO before proceeding to UART reinitialization. For this we're waiting for Transmitter Complete bit, indicating that the FIFO and the shift register are empty.
flushing has a 4ms timeout guard, which is normally more than enough to consume the FIFO @ low baudrate (9600bps).
Signed-off-by: Loic Poulain loic.poulain@linaro.org Tested-by: Lothar Waßmann LW@KARO-electronics.de --- v2: Add this commit to the series v3: Fix typo & reordering commits for good bisectability
drivers/serial/serial_mxc.c | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-)
diff --git a/drivers/serial/serial_mxc.c b/drivers/serial/serial_mxc.c index 82c0d84628..3c89781f2c 100644 --- a/drivers/serial/serial_mxc.c +++ b/drivers/serial/serial_mxc.c @@ -13,6 +13,7 @@ #include <dm/platform_data/serial_mxc.h> #include <serial.h> #include <linux/compiler.h> +#include <linux/delay.h>
/* UART Control Register Bit Fields.*/ #define URXD_CHARRDY (1<<15) @@ -144,8 +145,22 @@ struct mxc_uart { u32 ts; };
+static void _mxc_serial_flush(struct mxc_uart *base) +{ + unsigned int timeout = 4000; + + if (!(readl(&base->cr1) & UCR1_UARTEN) || + !(readl(&base->cr2) & UCR2_TXEN)) + return; + + while (!(readl(&base->sr2) & USR2_TXDC) && --timeout) + udelay(1); +} + static void _mxc_serial_init(struct mxc_uart *base, int use_dte) { + _mxc_serial_flush(base); + writel(0, &base->cr1); writel(0, &base->cr2);
@@ -252,10 +267,17 @@ static int mxc_serial_init(void) return 0; }
+static int mxc_serial_stop(void) +{ + _mxc_serial_flush(mxc_base); + + return 0; +} + static struct serial_device mxc_serial_drv = { .name = "mxc_serial", .start = mxc_serial_init, - .stop = NULL, + .stop = mxc_serial_stop, .setbrg = mxc_serial_setbrg, .putc = mxc_serial_putc, .puts = default_serial_puts,
Instead of waiting for empty FIFO condition before writing a character, wait for non-full FIFO condition.
This helps in saving several tens of milliseconds during boot (depending verbosity).
Signed-off-by: Loic Poulain loic.poulain@linaro.org Tested-by: Lothar Waßmann LW@KARO-electronics.de --- v2: fixing transfert abort & char corruption commit v3: Reordering commits for good bisectability
drivers/serial/serial_mxc.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/serial/serial_mxc.c b/drivers/serial/serial_mxc.c index 3c89781f2c..9899155c00 100644 --- a/drivers/serial/serial_mxc.c +++ b/drivers/serial/serial_mxc.c @@ -238,11 +238,11 @@ static void mxc_serial_putc(const char c) if (c == '\n') serial_putc('\r');
- writel(c, &mxc_base->txd); - /* wait for transmitter to be ready */ - while (!(readl(&mxc_base->ts) & UTS_TXEMPTY)) + while (readl(&mxc_base->ts) & UTS_TXFULL) schedule(); + + writel(c, &mxc_base->txd); }
/* Test whether a character is in the RX buffer */ @@ -333,7 +333,7 @@ static int mxc_serial_putc(struct udevice *dev, const char ch) struct mxc_serial_plat *plat = dev_get_plat(dev); struct mxc_uart *const uart = plat->reg;
- if (!(readl(&uart->ts) & UTS_TXEMPTY)) + if (readl(&uart->ts) & UTS_TXFULL) return -EAGAIN;
writel(ch, &uart->txd);
On Tue, Jan 10, 2023 at 4:24 PM Loic Poulain loic.poulain@linaro.org wrote:
Instead of waiting for empty FIFO condition before writing a character, wait for non-full FIFO condition.
This helps in saving several tens of milliseconds during boot (depending verbosity).
Signed-off-by: Loic Poulain loic.poulain@linaro.org Tested-by: Lothar Waßmann LW@KARO-electronics.de
v2: fixing transfert abort & char corruption commit v3: Reordering commits for good bisectability
Reviewed-by: Fabio Estevam festevam@denx.de
Thanks
On Tuesday 10 January 2023 20:24:07 Loic Poulain wrote:
Instead of waiting for empty FIFO condition before writing a character, wait for non-full FIFO condition.
This helps in saving several tens of milliseconds during boot (depending verbosity).
Signed-off-by: Loic Poulain loic.poulain@linaro.org Tested-by: Lothar Waßmann LW@KARO-electronics.de
v2: fixing transfert abort & char corruption commit v3: Reordering commits for good bisectability
drivers/serial/serial_mxc.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/serial/serial_mxc.c b/drivers/serial/serial_mxc.c index 3c89781f2c..9899155c00 100644 --- a/drivers/serial/serial_mxc.c +++ b/drivers/serial/serial_mxc.c @@ -238,11 +238,11 @@ static void mxc_serial_putc(const char c) if (c == '\n') serial_putc('\r');
- writel(c, &mxc_base->txd);
- /* wait for transmitter to be ready */
- while (!(readl(&mxc_base->ts) & UTS_TXEMPTY))
- while (readl(&mxc_base->ts) & UTS_TXFULL) schedule();
- writel(c, &mxc_base->txd);
}
/* Test whether a character is in the RX buffer */ @@ -333,7 +333,7 @@ static int mxc_serial_putc(struct udevice *dev, const char ch) struct mxc_serial_plat *plat = dev_get_plat(dev); struct mxc_uart *const uart = plat->reg;
- if (!(readl(&uart->ts) & UTS_TXEMPTY))
if (readl(&uart->ts) & UTS_TXFULL) return -EAGAIN;
writel(ch, &uart->txd);
-- 2.34.1
Hello!
In this driver there are two mxc_serial_putc() and two mxc_serial_getc() function. One for DM and one for non-DM version. It would be really nice to fix also second set of functions.
On Wed, 11 Jan 2023 at 00:55, Pali Rohár pali@kernel.org wrote:
On Tuesday 10 January 2023 20:24:07 Loic Poulain wrote:
Instead of waiting for empty FIFO condition before writing a character, wait for non-full FIFO condition.
This helps in saving several tens of milliseconds during boot (depending verbosity).
Signed-off-by: Loic Poulain loic.poulain@linaro.org Tested-by: Lothar Waßmann LW@KARO-electronics.de
v2: fixing transfert abort & char corruption commit v3: Reordering commits for good bisectability
drivers/serial/serial_mxc.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/serial/serial_mxc.c b/drivers/serial/serial_mxc.c index 3c89781f2c..9899155c00 100644 --- a/drivers/serial/serial_mxc.c +++ b/drivers/serial/serial_mxc.c @@ -238,11 +238,11 @@ static void mxc_serial_putc(const char c) if (c == '\n') serial_putc('\r');
writel(c, &mxc_base->txd);/* wait for transmitter to be ready */while (!(readl(&mxc_base->ts) & UTS_TXEMPTY))
while (readl(&mxc_base->ts) & UTS_TXFULL) schedule();writel(c, &mxc_base->txd);}
/* Test whether a character is in the RX buffer */ @@ -333,7 +333,7 @@ static int mxc_serial_putc(struct udevice *dev, const char ch) struct mxc_serial_plat *plat = dev_get_plat(dev); struct mxc_uart *const uart = plat->reg;
if (!(readl(&uart->ts) & UTS_TXEMPTY))
if (readl(&uart->ts) & UTS_TXFULL) return -EAGAIN; writel(ch, &uart->txd);-- 2.34.1
Hello!
In this driver there are two mxc_serial_putc() and two mxc_serial_getc() function. One for DM and one for non-DM version. It would be really nice to fix also second set of functions.
This commit changes both DM/non-DM putc() functions.
Regards, Loic
On Wednesday 11 January 2023 08:54:49 Loic Poulain wrote:
On Wed, 11 Jan 2023 at 00:55, Pali Rohár pali@kernel.org wrote:
On Tuesday 10 January 2023 20:24:07 Loic Poulain wrote:
Instead of waiting for empty FIFO condition before writing a character, wait for non-full FIFO condition.
This helps in saving several tens of milliseconds during boot (depending verbosity).
Signed-off-by: Loic Poulain loic.poulain@linaro.org Tested-by: Lothar Waßmann LW@KARO-electronics.de
v2: fixing transfert abort & char corruption commit v3: Reordering commits for good bisectability
drivers/serial/serial_mxc.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/serial/serial_mxc.c b/drivers/serial/serial_mxc.c index 3c89781f2c..9899155c00 100644 --- a/drivers/serial/serial_mxc.c +++ b/drivers/serial/serial_mxc.c @@ -238,11 +238,11 @@ static void mxc_serial_putc(const char c) if (c == '\n') serial_putc('\r');
writel(c, &mxc_base->txd);/* wait for transmitter to be ready */while (!(readl(&mxc_base->ts) & UTS_TXEMPTY))
while (readl(&mxc_base->ts) & UTS_TXFULL) schedule();writel(c, &mxc_base->txd);}
/* Test whether a character is in the RX buffer */ @@ -333,7 +333,7 @@ static int mxc_serial_putc(struct udevice *dev, const char ch) struct mxc_serial_plat *plat = dev_get_plat(dev); struct mxc_uart *const uart = plat->reg;
if (!(readl(&uart->ts) & UTS_TXEMPTY))
if (readl(&uart->ts) & UTS_TXFULL) return -EAGAIN; writel(ch, &uart->txd);-- 2.34.1
Hello!
In this driver there are two mxc_serial_putc() and two mxc_serial_getc() function. One for DM and one for non-DM version. It would be really nice to fix also second set of functions.
This commit changes both DM/non-DM putc() functions.
Regards, Loic
Ou, sorry for that. I have misread getc and putc strings, looks very similar for me. So it should be fine!
Acked-by: Pali Rohár pali@kernel.org
[Adding Johannes and Tim]
On Tue, Jan 10, 2023 at 4:24 PM Loic Poulain loic.poulain@linaro.org wrote:
The u-boot console may show some corrupted characters when printing in board_init() due to reset of the UART (probe) before the TX FIFO has been completely drained.
To fix this issue, and in case UART is still running, we now try to flush the FIFO before proceeding to UART reinitialization. For this we're waiting for Transmitter Complete bit, indicating that the FIFO and the shift register are empty.
flushing has a 4ms timeout guard, which is normally more than enough to consume the FIFO @ low baudrate (9600bps).
Signed-off-by: Loic Poulain loic.poulain@linaro.org Tested-by: Lothar Waßmann LW@KARO-electronics.de
v2: Add this commit to the series v3: Fix typo & reordering commits for good bisectability
Reviewed-by: Fabio Estevam festevam@denx.de
Thanks
On Tuesday 10 January 2023 20:24:06 Loic Poulain wrote:
The u-boot console may show some corrupted characters when printing in board_init() due to reset of the UART (probe) before the TX FIFO has been completely drained.
To fix this issue, and in case UART is still running, we now try to flush the FIFO before proceeding to UART reinitialization. For this we're waiting for Transmitter Complete bit, indicating that the FIFO and the shift register are empty.
flushing has a 4ms timeout guard, which is normally more than enough to consume the FIFO @ low baudrate (9600bps).
Signed-off-by: Loic Poulain loic.poulain@linaro.org Tested-by: Lothar Waßmann LW@KARO-electronics.de
Hello! Last time when I looked at this driver I was in impression that also _mxc_serial_setbrg() function requires calling some flush function at the beginning. Could you please check if it is needed or not? I'm really not sure.
v2: Add this commit to the series v3: Fix typo & reordering commits for good bisectability
drivers/serial/serial_mxc.c | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-)
diff --git a/drivers/serial/serial_mxc.c b/drivers/serial/serial_mxc.c index 82c0d84628..3c89781f2c 100644 --- a/drivers/serial/serial_mxc.c +++ b/drivers/serial/serial_mxc.c @@ -13,6 +13,7 @@ #include <dm/platform_data/serial_mxc.h> #include <serial.h> #include <linux/compiler.h> +#include <linux/delay.h>
/* UART Control Register Bit Fields.*/ #define URXD_CHARRDY (1<<15) @@ -144,8 +145,22 @@ struct mxc_uart { u32 ts; };
+static void _mxc_serial_flush(struct mxc_uart *base) +{
- unsigned int timeout = 4000;
- if (!(readl(&base->cr1) & UCR1_UARTEN) ||
!(readl(&base->cr2) & UCR2_TXEN))return;- while (!(readl(&base->sr2) & USR2_TXDC) && --timeout)
udelay(1);+}
static void _mxc_serial_init(struct mxc_uart *base, int use_dte) {
- _mxc_serial_flush(base);
- writel(0, &base->cr1); writel(0, &base->cr2);
@@ -252,10 +267,17 @@ static int mxc_serial_init(void) return 0; }
+static int mxc_serial_stop(void) +{
- _mxc_serial_flush(mxc_base);
- return 0;
+}
static struct serial_device mxc_serial_drv = { .name = "mxc_serial", .start = mxc_serial_init,
- .stop = NULL,
- .stop = mxc_serial_stop, .setbrg = mxc_serial_setbrg, .putc = mxc_serial_putc, .puts = default_serial_puts,
Anyway, this code touches _only_ non-DM version of the driver. So same fix should be implemented also for DM version because non-DM is now legacy and will be removed in the future from U-Boot. Please look at the DM version too.
-- 2.34.1
On Wed, 11 Jan 2023 at 00:53, Pali Rohár pali@kernel.org wrote:
On Tuesday 10 January 2023 20:24:06 Loic Poulain wrote:
The u-boot console may show some corrupted characters when printing in board_init() due to reset of the UART (probe) before the TX FIFO has been completely drained.
To fix this issue, and in case UART is still running, we now try to flush the FIFO before proceeding to UART reinitialization. For this we're waiting for Transmitter Complete bit, indicating that the FIFO and the shift register are empty.
flushing has a 4ms timeout guard, which is normally more than enough to consume the FIFO @ low baudrate (9600bps).
Signed-off-by: Loic Poulain loic.poulain@linaro.org Tested-by: Lothar Waßmann LW@KARO-electronics.de
Hello! Last time when I looked at this driver I was in impression that also _mxc_serial_setbrg() function requires calling some flush function at the beginning. Could you please check if it is needed or not? I'm really not sure.
_mxc_serial_setbrg is usually called after init, which now includes that flush.
v2: Add this commit to the series v3: Fix typo & reordering commits for good bisectability
drivers/serial/serial_mxc.c | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-)
diff --git a/drivers/serial/serial_mxc.c b/drivers/serial/serial_mxc.c index 82c0d84628..3c89781f2c 100644 --- a/drivers/serial/serial_mxc.c +++ b/drivers/serial/serial_mxc.c @@ -13,6 +13,7 @@ #include <dm/platform_data/serial_mxc.h> #include <serial.h> #include <linux/compiler.h> +#include <linux/delay.h>
/* UART Control Register Bit Fields.*/ #define URXD_CHARRDY (1<<15) @@ -144,8 +145,22 @@ struct mxc_uart { u32 ts; };
+static void _mxc_serial_flush(struct mxc_uart *base) +{
unsigned int timeout = 4000;if (!(readl(&base->cr1) & UCR1_UARTEN) ||!(readl(&base->cr2) & UCR2_TXEN))return;while (!(readl(&base->sr2) & USR2_TXDC) && --timeout)udelay(1);+}
static void _mxc_serial_init(struct mxc_uart *base, int use_dte) {
_mxc_serial_flush(base);writel(0, &base->cr1); writel(0, &base->cr2);@@ -252,10 +267,17 @@ static int mxc_serial_init(void) return 0; }
+static int mxc_serial_stop(void) +{
_mxc_serial_flush(mxc_base);return 0;+}
static struct serial_device mxc_serial_drv = { .name = "mxc_serial", .start = mxc_serial_init,
.stop = NULL,
.stop = mxc_serial_stop, .setbrg = mxc_serial_setbrg, .putc = mxc_serial_putc, .puts = default_serial_puts,Anyway, this code touches _only_ non-DM version of the driver. So same fix should be implemented also for DM version because non-DM is now legacy and will be removed in the future from U-Boot. Please look at the DM version too.
This code impacts both DM and non-DM, as _mxc_serial_init is a common version, and was the main issue (several init() leading to corrupted char).
Regards, Loic
On Wednesday 11 January 2023 08:53:30 Loic Poulain wrote:
On Wed, 11 Jan 2023 at 00:53, Pali Rohár pali@kernel.org wrote:
On Tuesday 10 January 2023 20:24:06 Loic Poulain wrote:
The u-boot console may show some corrupted characters when printing in board_init() due to reset of the UART (probe) before the TX FIFO has been completely drained.
To fix this issue, and in case UART is still running, we now try to flush the FIFO before proceeding to UART reinitialization. For this we're waiting for Transmitter Complete bit, indicating that the FIFO and the shift register are empty.
flushing has a 4ms timeout guard, which is normally more than enough to consume the FIFO @ low baudrate (9600bps).
Signed-off-by: Loic Poulain loic.poulain@linaro.org Tested-by: Lothar Waßmann LW@KARO-electronics.de
Hello! Last time when I looked at this driver I was in impression that also _mxc_serial_setbrg() function requires calling some flush function at the beginning. Could you please check if it is needed or not? I'm really not sure.
_mxc_serial_setbrg is usually called after init, which now includes that flush.
I'm looking at it and this function is called also from mxc_serial_setbrg() callback, which is used by u-boot to change baudrate after the init.
v2: Add this commit to the series v3: Fix typo & reordering commits for good bisectability
drivers/serial/serial_mxc.c | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-)
diff --git a/drivers/serial/serial_mxc.c b/drivers/serial/serial_mxc.c index 82c0d84628..3c89781f2c 100644 --- a/drivers/serial/serial_mxc.c +++ b/drivers/serial/serial_mxc.c @@ -13,6 +13,7 @@ #include <dm/platform_data/serial_mxc.h> #include <serial.h> #include <linux/compiler.h> +#include <linux/delay.h>
/* UART Control Register Bit Fields.*/ #define URXD_CHARRDY (1<<15) @@ -144,8 +145,22 @@ struct mxc_uart { u32 ts; };
+static void _mxc_serial_flush(struct mxc_uart *base) +{
unsigned int timeout = 4000;if (!(readl(&base->cr1) & UCR1_UARTEN) ||!(readl(&base->cr2) & UCR2_TXEN))return;while (!(readl(&base->sr2) & USR2_TXDC) && --timeout)udelay(1);+}
static void _mxc_serial_init(struct mxc_uart *base, int use_dte) {
_mxc_serial_flush(base);writel(0, &base->cr1); writel(0, &base->cr2);@@ -252,10 +267,17 @@ static int mxc_serial_init(void) return 0; }
+static int mxc_serial_stop(void) +{
_mxc_serial_flush(mxc_base);return 0;+}
static struct serial_device mxc_serial_drv = { .name = "mxc_serial", .start = mxc_serial_init,
.stop = NULL,
.stop = mxc_serial_stop, .setbrg = mxc_serial_setbrg, .putc = mxc_serial_putc, .puts = default_serial_puts,Anyway, this code touches _only_ non-DM version of the driver. So same fix should be implemented also for DM version because non-DM is now legacy and will be removed in the future from U-Boot. Please look at the DM version too.
This code impacts both DM and non-DM, as _mxc_serial_init is a common version, and was the main issue (several init() leading to corrupted char).
Regards, Loic
Yea, now I figured out.
On Wed, 11 Jan 2023 at 09:08, Pali Rohár pali@kernel.org wrote:
On Wednesday 11 January 2023 08:53:30 Loic Poulain wrote:
On Wed, 11 Jan 2023 at 00:53, Pali Rohár pali@kernel.org wrote:
On Tuesday 10 January 2023 20:24:06 Loic Poulain wrote:
The u-boot console may show some corrupted characters when printing in board_init() due to reset of the UART (probe) before the TX FIFO has been completely drained.
To fix this issue, and in case UART is still running, we now try to flush the FIFO before proceeding to UART reinitialization. For this we're waiting for Transmitter Complete bit, indicating that the FIFO and the shift register are empty.
flushing has a 4ms timeout guard, which is normally more than enough to consume the FIFO @ low baudrate (9600bps).
Signed-off-by: Loic Poulain loic.poulain@linaro.org Tested-by: Lothar Waßmann LW@KARO-electronics.de
Hello! Last time when I looked at this driver I was in impression that also _mxc_serial_setbrg() function requires calling some flush function at the beginning. Could you please check if it is needed or not? I'm really not sure.
_mxc_serial_setbrg is usually called after init, which now includes that flush.
I'm looking at it and this function is called also from mxc_serial_setbrg() callback, which is used by u-boot to change baudrate after the init.
Ok, good point, then it makes sense to add this guard here as well.
participants (3)
-
Fabio Estevam -
Loic Poulain -
Pali Rohár