Re: Regression: serial: imx: overrun errors on debug UART

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Uwe,

Am 25.03.23 um 16:11 schrieb Uwe Kleine-König:
Hello,

On Sat, Mar 25, 2023 at 12:31:01PM +0100, Stefan Wahren wrote:
Am 24.03.23 um 16:00 schrieb Stefan Wahren:
Am 24.03.23 um 13:57 schrieb Fabio Estevam:
On Fri, Mar 24, 2023 at 8:48 AM Ilpo Järvinen
<ilpo.jarvinen@xxxxxxxxxxxxxxx> wrote:

This has come up earlier, see e.g.:

https://lore.kernel.org/linux-serial/20221003110850.GA28338@xxxxxxxxxxxxxxxxxxxxxxxxxxxx/

My somewhat uninformed suggestion: if the overrun problems
mostly show up
with console ports, maybe the trigger level could depend on the port
being a console or not?
Does the change below help? Taking Ilpo's suggestion into account:
this breaks the boot / debug console completely, but i got the idea.


based on your patch, i successfully tested this:

diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
index f07c4f9ff13c..1aacaa637ede 100644
--- a/drivers/tty/serial/imx.c
+++ b/drivers/tty/serial/imx.c
@@ -1277,6 +1277,7 @@ static void imx_uart_clear_rx_errors(struct imx_port
*sport)
  }

  #define TXTL_DEFAULT 2 /* reset default */
+#define RXTL_DEFAULT_CONSOLE 1 /* reset default */
  #define RXTL_DEFAULT 8 /* 8 characters or aging timer */
  #define TXTL_DMA 8 /* DMA burst setting */
  #define RXTL_DMA 9 /* DMA burst setting */
@@ -1286,6 +1287,9 @@ static void imx_uart_setup_ufcr(struct imx_port
*sport,
  {
  	unsigned int val;

+	if (uart_console(&sport->port))
+		rxwl = RXTL_DEFAULT_CONSOLE; // fallback
+
  	/* set receiver / transmitter trigger level */
  	val = imx_uart_readl(sport, UFCR) & (UFCR_RFDIV | UFCR_DCEDTE);
  	val |= txwl << UFCR_TXTL_SHF | rxwl;

So the current theory that the issue occurs because of a combination of:

  - With a higher watermark value the irq triggers later and so there is
    less time the until the ISR must run before an overflow happens; and

  - serial console activity disables irqs for a (relative) long time

right?

So on an UP system the problem should occur also on a non-console port?

This is less likely, because UART applications usually need some kind of flow control (either from hardware or protocol side). For a non-console application the receiver usually wait until the end and then starts to transmit.

Sure you can flood the UART with characters and it's only a question of time until the RX FIFO is full and data get lost. But i think we should focus on the real use case and don't try find the perfect solution. At the end it's always a compromise between latency and throughput.

Local irqs are only disabled if some printk is about to be emitted,
isn't it? Does this match the error you're seeing?

Yes, that's the typical "problem" of a console application.

That makes me wonder if the error doesn't relate to the UART being a
console port, but the UART being used without DMA?! (So the patch above
fixes the problem for you because on the console port no DMA is used?)

As i said the issue only occured on the console. My problem is that the other UARTs on Tarragon are used for RS485 which means they are just half duplex.

According to these lines in imx.c DMA is never used for console:

  /* Can we enable the DMA support? */
  if (!uart_console(port) && imx_uart_dma_init(sport) == 0)
    dma_is_inited = 1;

At the end the patch above only restores the old console behavior, but keep Tomasz Moń's optimization for non-console (which was indented for).

Best regards
Stefan


Best regards
Uwe


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel



[Index of Archives]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux PPP]     [Linux FS]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Linmodem]     [Device Mapper]     [Linux Kernel for ARM]

  Powered by Linux