[PATCH] serial: 8250 dw: clear FIFO before writting LCR

[PATCH] serial: 8250 dw: clear FIFO before writting LCR

[Adriana Nicolae]

This patch is proposing a custom configuration for Synopsys DesignWare
serial to be used by products with associated compatible string in the
device tree.

The PORT_DWAPB config will be used instead of the default PORT_16550A
which does not include the UART_FCR_CLEAR_RCVR and UART_FCR_CLEAR_XMIT
bits for the FIFO configuration register. Having those flags is necessary
to clear FIFO when the serial port is reconfigured with do_set_termios.

Additionally, inside the do_set_termios we use the LCR (Line Control
Register) to enable DLAB bit in order to access DLL/DLM (Divisor Latch
Low/High) registers for baud rate setting. These 2 registers are sharing
the same address space with UART_TX/UART_RX and UART_IER. The sequence is:

(1) enable DLAB -> (2) set baud -> (3) disable DLAB -> (4) reset FCR

When there is a TX or RX flow on the serial while we attempt to set/clear
DLAB, the LCR write will be ignored and we will get a IIR_BUSY interrupt
afterwards which is cleared by only reading the USR (UART status register).

The sequence above can leave the serial in an unstable state in two cases:

 - if UART is busy while (1), then LCR is still pointing to the normal set of
registers, which means the code setting DLL/DLM is actually writing into TX or
modifying interrupts in UART_IER which may end with either a garbage char '\'
on the console or with serial interrupts disabled.

 - if UART is busy while (3), then LCR remains pointing to DLL/DLM instead of
moving back to RX/TX. The first transfer on the serial will be stuck because
the transmit/receive registers are not accessible unless the DLAB bit
is cleared.

The changes in this patch include a specific serial_out function for this UART
type similar to the one for Armada-38x devices in commit
b7639b0b15ddd1a4686b0142e70dfb122eefc88f with some changes in the tx_wait_empty
function to check the UART status by looking at the USR register and actively
try to clear FIFO to reduce time before a LCR write since the characters will
be lost otherwise after baud rate change.

The USR register may report that UART is busy even if TX/TX FIFO is already
empty so we will loop until all USR[0] (UART busy status) is cleared and USR[1]
TX FIFO is empty (RX FIFO bits should be 0 in this case).
Keeping the same timeout of 20ms as measurements with the 9600 baud when
the console was busy it took max 1.9ms to get the UART free state.

Signed-off-by: Adriana Nicolae <adriana@arista.com>
---
 drivers/tty/serial/8250/8250_dw.c   |   52 +++++++++++++++++++++++++++++++++++
 drivers/tty/serial/8250/8250_port.c |    8 +++++
 include/uapi/linux/serial_core.h    |    3 ++
 3 files changed, 63 insertions(+)

diff --git a/drivers/tty/serial/8250/8250_dw.c
b/drivers/tty/serial/8250/8250_dw.c
index ace221afe..337b9a8bf 100644
--- a/drivers/tty/serial/8250/8250_dw.c
+++ b/drivers/tty/serial/8250/8250_dw.c
@@ -33,10 +33,16 @@

 /* Offsets for the DesignWare specific registers */
 #define DW_UART_USR 0x1f /* UART Status Register */
+#define DW_UART_USR_BUSY 0x1 /* UART Busy status */
+#define DW_UART_USR_TFNF 0x2 /* UART TX FIFO not full */
+#define DW_UART_USR_TFE  0x4 /* UART TX FIFO empty */

 /* DesignWare specific register fields */
 #define DW_UART_MCR_SIRE BIT(6)

+#define DW8250_REG( p, reg ) \
+ ((void __iomem *)(p->membase + ((reg) << p->regshift)))
+
 struct dw8250_data {
  struct dw8250_port_data data;

@@ -159,6 +165,46 @@ static void dw8250_serial_out38x(struct uart_port
*p, int offset, int value)
  dw8250_check_lcr(p, value);
 }

+/* Drain FIFO and wait for USR to be not busy and TX/RX FIFO empty */
+static void dw8250_tx_wait_empty_apb(struct uart_port *p)
+{
+ unsigned int tries = 20000;
+ unsigned int delay_threshold = tries - 1000;
+ unsigned int usr;
+
+ while (tries--) {
+ usr = readl(DW8250_REG(p, DW_UART_USR));
+
+ /* Check UART free and TX/RX FIFO empty */
+ if ((usr & ~DW_UART_USR_TFNF) == DW_UART_USR_TFE)
+ break;
+
+ /* FIFO is still not empty, try to clear it */
+ if (tries < delay_threshold) {
+ writel(UART_FCR_ENABLE_FIFO, DW8250_REG(p, UART_FCR));
+ writel(UART_FCR_ENABLE_FIFO | UART_FCR_CLEAR_RCVR |
+ UART_FCR_CLEAR_XMIT, DW8250_REG(p, UART_FCR));
+ writel(0, DW8250_REG(p, UART_FCR));
+ udelay (1);
+ }
+ }
+}
+
+static void dw8250_serial_outapb(struct uart_port *p, int offset, int value)
+{
+       struct dw8250_data *d = to_dw8250_data(p->private_data);
+
+ if(offset == UART_LCR && !d->uart_16550_compatible)
+ dw8250_tx_wait_empty_apb(p);
+
+ writel(value, DW8250_REG(p, offset));
+
+ if (offset == UART_LCR && !d->uart_16550_compatible) {
+ /* Check FIFO is left enabled and LCR was written */
+ writel(UART_FCR_ENABLE_FIFO, DW8250_REG(p, UART_FCR));
+ dw8250_check_lcr(p, value);
+ }
+}

 static void dw8250_serial_out(struct uart_port *p, int offset, int value)
 {
@@ -421,6 +467,12 @@ static void dw8250_quirks(struct uart_port *p,
struct dw8250_data *data)
  }
  if (of_device_is_compatible(np, "marvell,armada-38x-uart"))
  p->serial_out = dw8250_serial_out38x;
+ if (of_device_is_compatible(np, "snps,dw-apb-uart")) {
+ p->type = PORT_DWAPB;
+ p->flags |= UPF_FIXED_TYPE;
+ p->serial_out = dw8250_serial_outapb;
+ data->skip_autocfg = true;
+ }

  } else if (acpi_dev_present("APMC0D08", NULL, -1)) {
  p->iotype = UPIO_MEM32;
diff --git a/drivers/tty/serial/8250/8250_port.c
b/drivers/tty/serial/8250/8250_port.c
index 0cc4dcf23..d627d85a0 100644
--- a/drivers/tty/serial/8250/8250_port.c
+++ b/drivers/tty/serial/8250/8250_port.c
@@ -307,6 +307,14 @@ static const struct serial8250_config uart_config[] = {
  .rxtrig_bytes = {1, 32, 64, 112},
  .flags = UART_CAP_FIFO | UART_CAP_SLEEP,
  },
+ [PORT_DWAPB] = {
+ .name = "Synopsys DesignWare",
+ .fifo_size = 16,
+ .tx_loadsz = 16,
+ .fcr = UART_FCR_ENABLE_FIFO |
+   UART_FCR_CLEAR_RCVR | UART_FCR_CLEAR_XMIT,
+ .flags = UART_CAP_FIFO | UART_CAP_AFE | UART_CAP_IRDA,
+ },
 };

 static const char *platform_list[] = {
diff --git a/include/uapi/linux/serial_core.h b/include/uapi/linux/serial_core.h
index 851b982f8..279fc4c98 100644
--- a/include/uapi/linux/serial_core.h
+++ b/include/uapi/linux/serial_core.h
@@ -279,4 +279,7 @@
 /* Freescale LINFlexD UART */
 #define PORT_LINFLEXUART 122

+/* Synopsys DesignWare */
+#define PORT_DWAPB 123
+
 #endif /* _UAPILINUX_SERIAL_CORE_H */

Re: [PATCH] serial: 8250 dw: clear FIFO before writting LCR

[Greg KH]

On Tue, Aug 19, 2025 at 06:06:55PM +0300, Adriana Nicolae wrote:
> This patch is proposing a custom configuration for Synopsys DesignWare
> serial to be used by products with associated compatible string in the
> device tree.
> 
> The PORT_DWAPB config will be used instead of the default PORT_16550A
> which does not include the UART_FCR_CLEAR_RCVR and UART_FCR_CLEAR_XMIT
> bits for the FIFO configuration register. Having those flags is necessary
> to clear FIFO when the serial port is reconfigured with do_set_termios.
> 
> Additionally, inside the do_set_termios we use the LCR (Line Control
> Register) to enable DLAB bit in order to access DLL/DLM (Divisor Latch
> Low/High) registers for baud rate setting. These 2 registers are sharing
> the same address space with UART_TX/UART_RX and UART_IER. The sequence is:
> 
> (1) enable DLAB -> (2) set baud -> (3) disable DLAB -> (4) reset FCR
> 
> When there is a TX or RX flow on the serial while we attempt to set/clear
> DLAB, the LCR write will be ignored and we will get a IIR_BUSY interrupt
> afterwards which is cleared by only reading the USR (UART status register).
> 
> The sequence above can leave the serial in an unstable state in two cases:
> 
>  - if UART is busy while (1), then LCR is still pointing to the normal set of
> registers, which means the code setting DLL/DLM is actually writing into TX or
> modifying interrupts in UART_IER which may end with either a garbage char '\'
> on the console or with serial interrupts disabled.
> 
>  - if UART is busy while (3), then LCR remains pointing to DLL/DLM instead of
> moving back to RX/TX. The first transfer on the serial will be stuck because
> the transmit/receive registers are not accessible unless the DLAB bit
> is cleared.
> 
> The changes in this patch include a specific serial_out function for this UART
> type similar to the one for Armada-38x devices in commit
> b7639b0b15ddd1a4686b0142e70dfb122eefc88f with some changes in the tx_wait_empty
> function to check the UART status by looking at the USR register and actively
> try to clear FIFO to reduce time before a LCR write since the characters will
> be lost otherwise after baud rate change.
> 
> The USR register may report that UART is busy even if TX/TX FIFO is already
> empty so we will loop until all USR[0] (UART busy status) is cleared and USR[1]
> TX FIFO is empty (RX FIFO bits should be 0 in this case).
> Keeping the same timeout of 20ms as measurements with the 9600 baud when
> the console was busy it took max 1.9ms to get the UART free state.
> 
> Signed-off-by: Adriana Nicolae <adriana@arista.com>
> ---
>  drivers/tty/serial/8250/8250_dw.c   |   52 +++++++++++++++++++++++++++++++++++
>  drivers/tty/serial/8250/8250_port.c |    8 +++++
>  include/uapi/linux/serial_core.h    |    3 ++
>  3 files changed, 63 insertions(+)
> 
> diff --git a/drivers/tty/serial/8250/8250_dw.c
> b/drivers/tty/serial/8250/8250_dw.c
> index ace221afe..337b9a8bf 100644
> --- a/drivers/tty/serial/8250/8250_dw.c
> +++ b/drivers/tty/serial/8250/8250_dw.c
> @@ -33,10 +33,16 @@
> 
>  /* Offsets for the DesignWare specific registers */
>  #define DW_UART_USR 0x1f /* UART Status Register */
> +#define DW_UART_USR_BUSY 0x1 /* UART Busy status */
> +#define DW_UART_USR_TFNF 0x2 /* UART TX FIFO not full */
> +#define DW_UART_USR_TFE  0x4 /* UART TX FIFO empty */
> 
>  /* DesignWare specific register fields */
>  #define DW_UART_MCR_SIRE BIT(6)
> 
> +#define DW8250_REG( p, reg ) \
> + ((void __iomem *)(p->membase + ((reg) << p->regshift)))
> +

Hi,

This is the friendly patch-bot of Greg Kroah-Hartman.  You have sent him
a patch that has triggered this response.  He used to manually respond
to these common problems, but in order to save his sanity (he kept
writing the same thing over and over, yet to different people), I was
created.  Hopefully you will not take offence and will fix the problem
in your patch and resubmit it so that it can be accepted into the Linux
kernel tree.

You are receiving this message because of the following common error(s)
as indicated below:

- Your patch is malformed (tabs converted to spaces, linewrapped, etc.)
  and can not be applied.  Please read the file,
  Documentation/process/email-clients.rst in order to fix this.

If you wish to discuss this problem further, or you have questions about
how to resolve this issue, please feel free to respond to this email and
Greg will reply once he has dug out from the pending patches received
from other developers.

thanks,

greg k-h's patch email bot

[PATCH] serial: 8250 dw: clear FIFO before writting LCR

[adriana]

This patch is proposing a custom configuration for Synopsys DesignWare
serial to be used by products with associated compatible string in the
device tree.

The PORT_DWAPB config will be used instead of the default PORT_16550A
which does not include the UART_FCR_CLEAR_RCVR and UART_FCR_CLEAR_XMIT
bits for the FIFO configuration register. Having those flags is necessary
to clear FIFO when the serial port is reconfigured with do_set_termios.

Additionally, inside the do_set_termios we use the LCR (Line Control
Register) to enable DLAB bit in order to access DLL/DLM (Divisor Latch
Low/High) registers for baud rate setting. These 2 registers are sharing
the same address space with UART_TX/UART_RX and UART_IER. The sequence is:

(1) enable DLAB -> (2) set baud -> (3) disable DLAB -> (4) reset FCR

When there is a TX or RX flow on the serial while we attempt to set/clear
DLAB, the LCR write will be ignored and we will get a IIR_BUSY interrupt
afterwards which is cleared by only reading the USR (UART status register).

The sequence above can leave the serial in an unstable state in two cases:

- if UART is busy while (1), then LCR is still pointing to the normal set of
registers, which means the code setting DLL/DLM is actually writing into TX or
modifying interrupts in UART_IER which may end with either a garbage character
on the console or with serial interrupts disabled.

- if UART is busy while (3), then LCR remains pointing to DLL/DLM instead of
moving back to RX/TX. The first transfer on the serial will be stuck because
the transmit/receive registers are not accessible unless the DLAB bit
is cleared.

The changes in this patch include a specific serial_out function for this UART
type similar to the one for Armada-38x devices in commit
b7639b0b15ddd1a4686b0142e70dfb122eefc88f with some changes in the tx_wait_empty
function to check the UART status by looking at the USR register and actively
try to clear FIFO to reduce time before a LCR write since the characters will
be lost otherwise after baud rate change.

The USR register may report that UART is busy even if TX/TX FIFO is already
empty so we will loop until all USR[0] (UART busy status) is cleared and USR[1]
TX FIFO is empty (RX FIFO bits should be 0 in this case).
Keeping the same timeout of 20ms as measurements with the 9600 baud when
the console was busy it took max 1.9ms to get the UART free state.

Signed-off-by: Adriana Nicolae <adriana@arista.com>
---
 drivers/tty/serial/8250/8250_dw.c   |   52 +++++++++++++++++++++++++++++++++++
 drivers/tty/serial/8250/8250_port.c |    8 +++++
 include/uapi/linux/serial_core.h    |    3 ++
 3 files changed, 63 insertions(+)

diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c
index a53ba04d9770..985a2650f3f3 100644
--- a/drivers/tty/serial/8250/8250_dw.c
+++ b/drivers/tty/serial/8250/8250_dw.c
@@ -33,6 +33,9 @@
 /* Offsets for the DesignWare specific registers */
 #define DW_UART_USR	0x1f /* UART Status Register */
 #define DW_UART_DMASA	0xa8 /* DMA Software Ack */
+#define DW_UART_USR_BUSY 0x1 /* UART Busy status */
+#define DW_UART_USR_TFNF 0x2 /* UART TX FIFO not full */
+#define DW_UART_USR_TFE  0x4 /* UART TX FIFO empty */
 
 #define OCTEON_UART_USR	0x27 /* UART Status Register */
 
@@ -56,6 +59,10 @@
 #define DW_UART_QUIRK_IS_DMA_FC		BIT(3)
 #define DW_UART_QUIRK_APMC0D08		BIT(4)
 #define DW_UART_QUIRK_CPR_VALUE		BIT(5)
+#define DW_UART_QUIRK_APB		BIT(6)
+
+#define DW8250_REG( p, reg ) \
+	((void __iomem *)(p->membase + ((reg) << p->regshift)))
 
 struct dw8250_platform_data {
 	u8 usr_reg;
@@ -220,6 +227,47 @@ static void dw8250_serial_out38x(struct uart_port *p, unsigned int offset, u32 v
 	dw8250_serial_out(p, offset, value);
 }
 
+/* Drain FIFO and wait for USR to be not busy and TX/RX FIFO empty */
+static void dw8250_tx_wait_empty_apb(struct uart_port *p)
+{
+	unsigned int tries = 20000;
+	unsigned int delay_threshold = tries - 1000;
+	unsigned int usr;
+
+	while (tries--) {
+		usr = readl(DW8250_REG(p, DW_UART_USR));
+
+		/* Check UART free and TX/RX FIFO empty */
+		if ((usr & ~DW_UART_USR_TFNF) == DW_UART_USR_TFE)
+			break;
+
+		/* FIFO is still not empty, try to clear it */
+		if (tries < delay_threshold) {
+			writel(UART_FCR_ENABLE_FIFO, DW8250_REG(p, UART_FCR));
+			writel(UART_FCR_ENABLE_FIFO | UART_FCR_CLEAR_RCVR |
+			UART_FCR_CLEAR_XMIT, DW8250_REG(p, UART_FCR));
+			writel(0, DW8250_REG(p, UART_FCR));
+			udelay (1);
+		}
+	}
+}
+
+static void dw8250_serial_outapb(struct uart_port *p, int offset, int value)
+{
+       struct dw8250_data *d = to_dw8250_data(p->private_data);
+
+	if(offset == UART_LCR && !d->uart_16550_compatible)
+		dw8250_tx_wait_empty_apb(p);
+
+	writel(value, DW8250_REG(p, offset));
+
+	if (offset == UART_LCR && !d->uart_16550_compatible) {
+		/* Check FIFO is left enabled and LCR was written */
+		writel(UART_FCR_ENABLE_FIFO, DW8250_REG(p, UART_FCR));
+		dw8250_check_lcr(p, value);
+	}
+}
+
 static u32 dw8250_serial_in(struct uart_port *p, unsigned int offset)
 {
 	u32 value = readb(p->membase + (offset << p->regshift));
@@ -520,6 +568,12 @@ static void dw8250_quirks(struct uart_port *p, struct dw8250_data *data)
 		p->serial_in = dw8250_serial_in32;
 		data->uart_16550_compatible = true;
 	}
+	if (quirks & DW_UART_QUIRK_DWAPB) {
+		p->type = PORT_DWAPB;
+		p->flags |= UPF_FIXED_TYPE;
+		p->serial_out = dw8250_serial_outapb;
+		data->skip_autocfg = true;
+	}
 }
 
 static void dw8250_reset_control_assert(void *data)
@@ -755,6 +809,7 @@ static const struct dev_pm_ops dw8250_pm_ops = {
 
 static const struct dw8250_platform_data dw8250_dw_apb = {
 	.usr_reg = DW_UART_USR,
+	.quirks = DW_UART_QUIRK_APB,
 };
 
 static const struct dw8250_platform_data dw8250_octeon_3860_data = {
diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
index 2da9db960d09..3882a71920f6 100644
--- a/drivers/tty/serial/8250/8250_port.c
+++ b/drivers/tty/serial/8250/8250_port.c
@@ -310,6 +310,14 @@ static const struct serial8250_config uart_config[] = {
 		.rxtrig_bytes	= {1, 8, 16, 30},
 		.flags		= UART_CAP_FIFO | UART_CAP_AFE,
 	},
+	[PORT_DWAPB] = {
+		.name		= "Synopsys DesignWare",
+		.fifo_size	= 16,
+		.tx_loadsz	= 16,
+		.fcr		= UART_FCR_ENABLE_FIFO |
+				  UART_FCR_CLEAR_RCVR | UART_FCR_CLEAR_XMIT,
+		.flags		= UART_CAP_FIFO | UART_CAP_AFE | UART_CAP_IRDA,
+	},
 };
 
 /* Uart divisor latch read */
diff --git a/include/uapi/linux/serial_core.h b/include/uapi/linux/serial_core.h
index 9c007a106330..8386436b813f 100644
--- a/include/uapi/linux/serial_core.h
+++ b/include/uapi/linux/serial_core.h
@@ -231,6 +231,9 @@
 /* Sunplus UART */
 #define PORT_SUNPLUS	123
 
+/* Synopsys DesignWare */
+#define PORT_DWAPB		124
+
 /* Generic type identifier for ports which type is not important to userspace. */
 #define PORT_GENERIC	(-1)
 

Re: [PATCH] serial: 8250 dw: clear FIFO before writting LCR

[Greg KH]

On Tue, Aug 19, 2025 at 11:23:22AM -0700, adriana@arista.com wrote:
> This patch is proposing a custom configuration for Synopsys DesignWare
> serial to be used by products with associated compatible string in the
> device tree.
> 
> The PORT_DWAPB config will be used instead of the default PORT_16550A
> which does not include the UART_FCR_CLEAR_RCVR and UART_FCR_CLEAR_XMIT
> bits for the FIFO configuration register. Having those flags is necessary
> to clear FIFO when the serial port is reconfigured with do_set_termios.
> 
> Additionally, inside the do_set_termios we use the LCR (Line Control
> Register) to enable DLAB bit in order to access DLL/DLM (Divisor Latch
> Low/High) registers for baud rate setting. These 2 registers are sharing
> the same address space with UART_TX/UART_RX and UART_IER. The sequence is:
> 
> (1) enable DLAB -> (2) set baud -> (3) disable DLAB -> (4) reset FCR
> 
> When there is a TX or RX flow on the serial while we attempt to set/clear
> DLAB, the LCR write will be ignored and we will get a IIR_BUSY interrupt
> afterwards which is cleared by only reading the USR (UART status register).
> 
> The sequence above can leave the serial in an unstable state in two cases:
> 
> - if UART is busy while (1), then LCR is still pointing to the normal set of
> registers, which means the code setting DLL/DLM is actually writing into TX or
> modifying interrupts in UART_IER which may end with either a garbage character
> on the console or with serial interrupts disabled.
> 
> - if UART is busy while (3), then LCR remains pointing to DLL/DLM instead of
> moving back to RX/TX. The first transfer on the serial will be stuck because
> the transmit/receive registers are not accessible unless the DLAB bit
> is cleared.
> 
> The changes in this patch include a specific serial_out function for this UART
> type similar to the one for Armada-38x devices in commit
> b7639b0b15ddd1a4686b0142e70dfb122eefc88f with some changes in the tx_wait_empty
> function to check the UART status by looking at the USR register and actively
> try to clear FIFO to reduce time before a LCR write since the characters will
> be lost otherwise after baud rate change.
> 
> The USR register may report that UART is busy even if TX/TX FIFO is already
> empty so we will loop until all USR[0] (UART busy status) is cleared and USR[1]
> TX FIFO is empty (RX FIFO bits should be 0 in this case).
> Keeping the same timeout of 20ms as measurements with the 9600 baud when
> the console was busy it took max 1.9ms to get the UART free state.
> 
> Signed-off-by: Adriana Nicolae <adriana@arista.com>
> ---
>  drivers/tty/serial/8250/8250_dw.c   |   52 +++++++++++++++++++++++++++++++++++
>  drivers/tty/serial/8250/8250_port.c |    8 +++++
>  include/uapi/linux/serial_core.h    |    3 ++
>  3 files changed, 63 insertions(+)
> 
> diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c
> index a53ba04d9770..985a2650f3f3 100644
> --- a/drivers/tty/serial/8250/8250_dw.c
> +++ b/drivers/tty/serial/8250/8250_dw.c
> @@ -33,6 +33,9 @@
>  /* Offsets for the DesignWare specific registers */
>  #define DW_UART_USR	0x1f /* UART Status Register */
>  #define DW_UART_DMASA	0xa8 /* DMA Software Ack */
> +#define DW_UART_USR_BUSY 0x1 /* UART Busy status */
> +#define DW_UART_USR_TFNF 0x2 /* UART TX FIFO not full */
> +#define DW_UART_USR_TFE  0x4 /* UART TX FIFO empty */
>  
>  #define OCTEON_UART_USR	0x27 /* UART Status Register */
>  
> @@ -56,6 +59,10 @@
>  #define DW_UART_QUIRK_IS_DMA_FC		BIT(3)
>  #define DW_UART_QUIRK_APMC0D08		BIT(4)
>  #define DW_UART_QUIRK_CPR_VALUE		BIT(5)
> +#define DW_UART_QUIRK_APB		BIT(6)
> +
> +#define DW8250_REG( p, reg ) \
> +	((void __iomem *)(p->membase + ((reg) << p->regshift)))
>  
>  struct dw8250_platform_data {
>  	u8 usr_reg;
> @@ -220,6 +227,47 @@ static void dw8250_serial_out38x(struct uart_port *p, unsigned int offset, u32 v
>  	dw8250_serial_out(p, offset, value);
>  }
>  
> +/* Drain FIFO and wait for USR to be not busy and TX/RX FIFO empty */
> +static void dw8250_tx_wait_empty_apb(struct uart_port *p)
> +{
> +	unsigned int tries = 20000;
> +	unsigned int delay_threshold = tries - 1000;
> +	unsigned int usr;
> +
> +	while (tries--) {
> +		usr = readl(DW8250_REG(p, DW_UART_USR));
> +
> +		/* Check UART free and TX/RX FIFO empty */
> +		if ((usr & ~DW_UART_USR_TFNF) == DW_UART_USR_TFE)
> +			break;
> +
> +		/* FIFO is still not empty, try to clear it */
> +		if (tries < delay_threshold) {
> +			writel(UART_FCR_ENABLE_FIFO, DW8250_REG(p, UART_FCR));
> +			writel(UART_FCR_ENABLE_FIFO | UART_FCR_CLEAR_RCVR |
> +			UART_FCR_CLEAR_XMIT, DW8250_REG(p, UART_FCR));
> +			writel(0, DW8250_REG(p, UART_FCR));
> +			udelay (1);
> +		}
> +	}
> +}
> +
> +static void dw8250_serial_outapb(struct uart_port *p, int offset, int value)
> +{
> +       struct dw8250_data *d = to_dw8250_data(p->private_data);
> +
> +	if(offset == UART_LCR && !d->uart_16550_compatible)
> +		dw8250_tx_wait_empty_apb(p);
> +
> +	writel(value, DW8250_REG(p, offset));
> +
> +	if (offset == UART_LCR && !d->uart_16550_compatible) {
> +		/* Check FIFO is left enabled and LCR was written */
> +		writel(UART_FCR_ENABLE_FIFO, DW8250_REG(p, UART_FCR));
> +		dw8250_check_lcr(p, value);
> +	}
> +}
> +
>  static u32 dw8250_serial_in(struct uart_port *p, unsigned int offset)
>  {
>  	u32 value = readb(p->membase + (offset << p->regshift));
> @@ -520,6 +568,12 @@ static void dw8250_quirks(struct uart_port *p, struct dw8250_data *data)
>  		p->serial_in = dw8250_serial_in32;
>  		data->uart_16550_compatible = true;
>  	}
> +	if (quirks & DW_UART_QUIRK_DWAPB) {
> +		p->type = PORT_DWAPB;
> +		p->flags |= UPF_FIXED_TYPE;
> +		p->serial_out = dw8250_serial_outapb;
> +		data->skip_autocfg = true;
> +	}
>  }
>  
>  static void dw8250_reset_control_assert(void *data)
> @@ -755,6 +809,7 @@ static const struct dev_pm_ops dw8250_pm_ops = {
>  
>  static const struct dw8250_platform_data dw8250_dw_apb = {
>  	.usr_reg = DW_UART_USR,
> +	.quirks = DW_UART_QUIRK_APB,
>  };
>  
>  static const struct dw8250_platform_data dw8250_octeon_3860_data = {
> diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
> index 2da9db960d09..3882a71920f6 100644
> --- a/drivers/tty/serial/8250/8250_port.c
> +++ b/drivers/tty/serial/8250/8250_port.c
> @@ -310,6 +310,14 @@ static const struct serial8250_config uart_config[] = {
>  		.rxtrig_bytes	= {1, 8, 16, 30},
>  		.flags		= UART_CAP_FIFO | UART_CAP_AFE,
>  	},
> +	[PORT_DWAPB] = {
> +		.name		= "Synopsys DesignWare",
> +		.fifo_size	= 16,
> +		.tx_loadsz	= 16,
> +		.fcr		= UART_FCR_ENABLE_FIFO |
> +				  UART_FCR_CLEAR_RCVR | UART_FCR_CLEAR_XMIT,
> +		.flags		= UART_CAP_FIFO | UART_CAP_AFE | UART_CAP_IRDA,
> +	},
>  };
>  
>  /* Uart divisor latch read */
> diff --git a/include/uapi/linux/serial_core.h b/include/uapi/linux/serial_core.h
> index 9c007a106330..8386436b813f 100644
> --- a/include/uapi/linux/serial_core.h
> +++ b/include/uapi/linux/serial_core.h
> @@ -231,6 +231,9 @@
>  /* Sunplus UART */
>  #define PORT_SUNPLUS	123
>  
> +/* Synopsys DesignWare */
> +#define PORT_DWAPB		124
> +
>  /* Generic type identifier for ports which type is not important to userspace. */
>  #define PORT_GENERIC	(-1)
>  
> 

Hi,

This is the friendly patch-bot of Greg Kroah-Hartman.  You have sent him
a patch that has triggered this response.  He used to manually respond
to these common problems, but in order to save his sanity (he kept
writing the same thing over and over, yet to different people), I was
created.  Hopefully you will not take offence and will fix the problem
in your patch and resubmit it so that it can be accepted into the Linux
kernel tree.

You are receiving this message because of the following common error(s)
as indicated below:

- This looks like a new version of a previously submitted patch, but you
  did not list below the --- line any changes from the previous version.
  Please read the section entitled "The canonical patch format" in the
  kernel file, Documentation/process/submitting-patches.rst for what
  needs to be done here to properly describe this.

If you wish to discuss this problem further, or you have questions about
how to resolve this issue, please feel free to respond to this email and
Greg will reply once he has dug out from the pending patches received
from other developers.

thanks,

greg k-h's patch email bot

Re: [PATCH] serial: 8250 dw: clear FIFO before writting LCR

[Greg KH]

On Tue, Aug 19, 2025 at 11:23:22AM -0700, adriana@arista.com wrote:
> This patch is proposing a custom configuration for Synopsys DesignWare
> serial to be used by products with associated compatible string in the
> device tree.

Any reason why you are ignoring the maintainer list that
scripts/get_maintainer.pl spits out?

thanks,

greg k-h

Re: [PATCH] serial: 8250 dw: clear FIFO before writting LCR

[Ilpo Järvinen]

On Tue, 19 Aug 2025, adriana@arista.com wrote:

> This patch is proposing a custom configuration for Synopsys DesignWare

Please try to avoid starting sentences in the changelog with "This patch" 
or other wordings with similar meaning. Write imperatively instead.

Preferrable, describe the problem first, then the solution.

> serial to be used by products with associated compatible string in the
> device tree.
> 
> The PORT_DWAPB config will be used instead of the default PORT_16550A
> which does not include the UART_FCR_CLEAR_RCVR and UART_FCR_CLEAR_XMIT
> bits for the FIFO configuration register. Having those flags is necessary
> to clear FIFO when the serial port is reconfigured with do_set_termios.
> 
> Additionally, inside the do_set_termios we use the LCR (Line Control
> Register) to enable DLAB bit in order to access DLL/DLM (Divisor Latch
> Low/High) registers for baud rate setting. These 2 registers are sharing
> the same address space with UART_TX/UART_RX and UART_IER. The sequence is:
> 
> (1) enable DLAB -> (2) set baud -> (3) disable DLAB -> (4) reset FCR
> 
> When there is a TX or RX flow on the serial while we attempt to set/clear
> DLAB, the LCR write will be ignored and we will get a IIR_BUSY interrupt
> afterwards which is cleared by only reading the USR (UART status register).
> 
> The sequence above can leave the serial in an unstable state in two cases:
> 
> - if UART is busy while (1), then LCR is still pointing to the normal set of
> registers, which means the code setting DLL/DLM is actually writing into TX or
> modifying interrupts in UART_IER which may end with either a garbage character
> on the console or with serial interrupts disabled.
> 
> - if UART is busy while (3), then LCR remains pointing to DLL/DLM instead of
> moving back to RX/TX. The first transfer on the serial will be stuck because
> the transmit/receive registers are not accessible unless the DLAB bit
> is cleared.
> 
> The changes in this patch include a specific serial_out function for this UART
> type similar to the one for Armada-38x devices in commit
> b7639b0b15ddd1a4686b0142e70dfb122eefc88f with some changes in the tx_wait_empty
> function to check the UART status by looking at the USR register and actively
> try to clear FIFO to reduce time before a LCR write since the characters will
> be lost otherwise after baud rate change.
> 
> The USR register may report that UART is busy even if TX/TX FIFO is already
> empty so we will loop until all USR[0] (UART busy status) is cleared and USR[1]
> TX FIFO is empty (RX FIFO bits should be 0 in this case).
> Keeping the same timeout of 20ms as measurements with the 9600 baud when
> the console was busy it took max 1.9ms to get the UART free state.
> 
> Signed-off-by: Adriana Nicolae <adriana@arista.com>
> ---
>  drivers/tty/serial/8250/8250_dw.c   |   52 +++++++++++++++++++++++++++++++++++
>  drivers/tty/serial/8250/8250_port.c |    8 +++++
>  include/uapi/linux/serial_core.h    |    3 ++
>  3 files changed, 63 insertions(+)
> 
> diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c
> index a53ba04d9770..985a2650f3f3 100644
> --- a/drivers/tty/serial/8250/8250_dw.c
> +++ b/drivers/tty/serial/8250/8250_dw.c
> @@ -33,6 +33,9 @@
>  /* Offsets for the DesignWare specific registers */
>  #define DW_UART_USR	0x1f /* UART Status Register */
>  #define DW_UART_DMASA	0xa8 /* DMA Software Ack */
> +#define DW_UART_USR_BUSY 0x1 /* UART Busy status */
> +#define DW_UART_USR_TFNF 0x2 /* UART TX FIFO not full */
> +#define DW_UART_USR_TFE  0x4 /* UART TX FIFO empty */
>  
>  #define OCTEON_UART_USR	0x27 /* UART Status Register */
>  
> @@ -56,6 +59,10 @@
>  #define DW_UART_QUIRK_IS_DMA_FC		BIT(3)
>  #define DW_UART_QUIRK_APMC0D08		BIT(4)
>  #define DW_UART_QUIRK_CPR_VALUE		BIT(5)
> +#define DW_UART_QUIRK_APB		BIT(6)
> +
> +#define DW8250_REG( p, reg ) \
> +	((void __iomem *)(p->membase + ((reg) << p->regshift)))

What's wrong with dw8250_readl_ext() and dw8250_writel_ext()?

>  struct dw8250_platform_data {
>  	u8 usr_reg;
> @@ -220,6 +227,47 @@ static void dw8250_serial_out38x(struct uart_port *p, unsigned int offset, u32 v
>  	dw8250_serial_out(p, offset, value);
>  }
>  
> +/* Drain FIFO and wait for USR to be not busy and TX/RX FIFO empty */
> +static void dw8250_tx_wait_empty_apb(struct uart_port *p)
> +{
> +	unsigned int tries = 20000;
> +	unsigned int delay_threshold = tries - 1000;
> +	unsigned int usr;
> +
> +	while (tries--) {
> +		usr = readl(DW8250_REG(p, DW_UART_USR));
> +
> +		/* Check UART free and TX/RX FIFO empty */
> +		if ((usr & ~DW_UART_USR_TFNF) == DW_UART_USR_TFE)
> +			break;
> +
> +		/* FIFO is still not empty, try to clear it */
> +		if (tries < delay_threshold) {
> +			writel(UART_FCR_ENABLE_FIFO, DW8250_REG(p, UART_FCR));
> +			writel(UART_FCR_ENABLE_FIFO | UART_FCR_CLEAR_RCVR |
> +			UART_FCR_CLEAR_XMIT, DW8250_REG(p, UART_FCR));

Please indent any continuation lines properly, in this case to the char 
after the opening parenthesis.

> +			writel(0, DW8250_REG(p, UART_FCR));
> +			udelay (1);
> +		}
> +	}

This seems to be just rehashing the same non-robust algorithm. There's no 
way to ensure UART wouldn't become BUSY again because of Rx at any time. 
Thus, any amount of clearing FIFOs is just never going be fully safe.

Long time ago, I attempted to create a more robust solution to this BUSY
problem by temporarily enabling loopback which should prevent any new Rx 
from reaching the UART.

Could you please try my patch that is attached to:

https://lore.kernel.org/linux-serial/079c8fe6-9ce4-fa59-4b44-93e27dd376d6@linux.intel.com/

(I haven't found a way to reproduce this myself and so far all the 
reporters of this problem have gone oddly quiet when asked to test this 
patch so it's hasn't moved forward for fea years.)

There are small Tx DMA related bits to add to the patch from robustness 
perspective (but a sane communication pattern shouldn't need those 
anyway, ie., no application should be sending something while trying to 
change these registers).

> +}
> +
> +static void dw8250_serial_outapb(struct uart_port *p, int offset, int value)
> +{
> +       struct dw8250_data *d = to_dw8250_data(p->private_data);
> +
> +	if(offset == UART_LCR && !d->uart_16550_compatible)
> +		dw8250_tx_wait_empty_apb(p);
> +
> +	writel(value, DW8250_REG(p, offset));
> +
> +	if (offset == UART_LCR && !d->uart_16550_compatible) {
> +		/* Check FIFO is left enabled and LCR was written */
> +		writel(UART_FCR_ENABLE_FIFO, DW8250_REG(p, UART_FCR));
> +		dw8250_check_lcr(p, value);
> +	}
> +}
> +
>  static u32 dw8250_serial_in(struct uart_port *p, unsigned int offset)
>  {
>  	u32 value = readb(p->membase + (offset << p->regshift));
> @@ -520,6 +568,12 @@ static void dw8250_quirks(struct uart_port *p, struct dw8250_data *data)
>  		p->serial_in = dw8250_serial_in32;
>  		data->uart_16550_compatible = true;
>  	}
> +	if (quirks & DW_UART_QUIRK_DWAPB) {
> +		p->type = PORT_DWAPB;
> +		p->flags |= UPF_FIXED_TYPE;
> +		p->serial_out = dw8250_serial_outapb;
> +		data->skip_autocfg = true;
> +	}
>  }
>  
>  static void dw8250_reset_control_assert(void *data)
> @@ -755,6 +809,7 @@ static const struct dev_pm_ops dw8250_pm_ops = {
>  
>  static const struct dw8250_platform_data dw8250_dw_apb = {
>  	.usr_reg = DW_UART_USR,
> +	.quirks = DW_UART_QUIRK_APB,
>  };
>  
>  static const struct dw8250_platform_data dw8250_octeon_3860_data = {
> diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
> index 2da9db960d09..3882a71920f6 100644
> --- a/drivers/tty/serial/8250/8250_port.c
> +++ b/drivers/tty/serial/8250/8250_port.c
> @@ -310,6 +310,14 @@ static const struct serial8250_config uart_config[] = {
>  		.rxtrig_bytes	= {1, 8, 16, 30},
>  		.flags		= UART_CAP_FIFO | UART_CAP_AFE,
>  	},
> +	[PORT_DWAPB] = {
> +		.name		= "Synopsys DesignWare",
> +		.fifo_size	= 16,
> +		.tx_loadsz	= 16,
> +		.fcr		= UART_FCR_ENABLE_FIFO |
> +				  UART_FCR_CLEAR_RCVR | UART_FCR_CLEAR_XMIT,
> +		.flags		= UART_CAP_FIFO | UART_CAP_AFE | UART_CAP_IRDA,
> +	},
>  };
>  
>  /* Uart divisor latch read */
> diff --git a/include/uapi/linux/serial_core.h b/include/uapi/linux/serial_core.h
> index 9c007a106330..8386436b813f 100644
> --- a/include/uapi/linux/serial_core.h
> +++ b/include/uapi/linux/serial_core.h
> @@ -231,6 +231,9 @@
>  /* Sunplus UART */
>  #define PORT_SUNPLUS	123
>  
> +/* Synopsys DesignWare */
> +#define PORT_DWAPB		124
> +
>  /* Generic type identifier for ports which type is not important to userspace. */
>  #define PORT_GENERIC	(-1)
>  
> 

-- 
 i.

Re: [PATCH] serial: 8250 dw: clear FIFO before writting LCR

[Adriana Nicolae]

On Wed, Aug 20, 2025 at 1:02 PM Ilpo Järvinen
<ilpo.jarvinen@linux.intel.com> wrote:
>
> On Tue, 19 Aug 2025, adriana@arista.com wrote:
>
> > This patch is proposing a custom configuration for Synopsys DesignWare
>
> Please try to avoid starting sentences in the changelog with "This patch"
> or other wordings with similar meaning. Write imperatively instead.
>
> Preferrable, describe the problem first, then the solution.
>
> > serial to be used by products with associated compatible string in the
> > device tree.
> >
> > The PORT_DWAPB config will be used instead of the default PORT_16550A
> > which does not include the UART_FCR_CLEAR_RCVR and UART_FCR_CLEAR_XMIT
> > bits for the FIFO configuration register. Having those flags is necessary
> > to clear FIFO when the serial port is reconfigured with do_set_termios.
> >
> > Additionally, inside the do_set_termios we use the LCR (Line Control
> > Register) to enable DLAB bit in order to access DLL/DLM (Divisor Latch
> > Low/High) registers for baud rate setting. These 2 registers are sharing
> > the same address space with UART_TX/UART_RX and UART_IER. The sequence is:
> >
> > (1) enable DLAB -> (2) set baud -> (3) disable DLAB -> (4) reset FCR
> >
> > When there is a TX or RX flow on the serial while we attempt to set/clear
> > DLAB, the LCR write will be ignored and we will get a IIR_BUSY interrupt
> > afterwards which is cleared by only reading the USR (UART status register).
> >
> > The sequence above can leave the serial in an unstable state in two cases:
> >
> > - if UART is busy while (1), then LCR is still pointing to the normal set of
> > registers, which means the code setting DLL/DLM is actually writing into TX or
> > modifying interrupts in UART_IER which may end with either a garbage character
> > on the console or with serial interrupts disabled.
> >
> > - if UART is busy while (3), then LCR remains pointing to DLL/DLM instead of
> > moving back to RX/TX. The first transfer on the serial will be stuck because
> > the transmit/receive registers are not accessible unless the DLAB bit
> > is cleared.
> >
> > The changes in this patch include a specific serial_out function for this UART
> > type similar to the one for Armada-38x devices in commit
> > b7639b0b15ddd1a4686b0142e70dfb122eefc88f with some changes in the tx_wait_empty
> > function to check the UART status by looking at the USR register and actively
> > try to clear FIFO to reduce time before a LCR write since the characters will
> > be lost otherwise after baud rate change.
> >
> > The USR register may report that UART is busy even if TX/TX FIFO is already
> > empty so we will loop until all USR[0] (UART busy status) is cleared and USR[1]
> > TX FIFO is empty (RX FIFO bits should be 0 in this case).
> > Keeping the same timeout of 20ms as measurements with the 9600 baud when
> > the console was busy it took max 1.9ms to get the UART free state.
> >
> > Signed-off-by: Adriana Nicolae <adriana@arista.com>
> > ---
> >  drivers/tty/serial/8250/8250_dw.c   |   52 +++++++++++++++++++++++++++++++++++
> >  drivers/tty/serial/8250/8250_port.c |    8 +++++
> >  include/uapi/linux/serial_core.h    |    3 ++
> >  3 files changed, 63 insertions(+)
> >
> > diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c
> > index a53ba04d9770..985a2650f3f3 100644
> > --- a/drivers/tty/serial/8250/8250_dw.c
> > +++ b/drivers/tty/serial/8250/8250_dw.c
> > @@ -33,6 +33,9 @@
> >  /* Offsets for the DesignWare specific registers */
> >  #define DW_UART_USR  0x1f /* UART Status Register */
> >  #define DW_UART_DMASA        0xa8 /* DMA Software Ack */
> > +#define DW_UART_USR_BUSY 0x1 /* UART Busy status */
> > +#define DW_UART_USR_TFNF 0x2 /* UART TX FIFO not full */
> > +#define DW_UART_USR_TFE  0x4 /* UART TX FIFO empty */
> >
> >  #define OCTEON_UART_USR      0x27 /* UART Status Register */
> >
> > @@ -56,6 +59,10 @@
> >  #define DW_UART_QUIRK_IS_DMA_FC              BIT(3)
> >  #define DW_UART_QUIRK_APMC0D08               BIT(4)
> >  #define DW_UART_QUIRK_CPR_VALUE              BIT(5)
> > +#define DW_UART_QUIRK_APB            BIT(6)
> > +
> > +#define DW8250_REG( p, reg ) \
> > +     ((void __iomem *)(p->membase + ((reg) << p->regshift)))
>
> What's wrong with dw8250_readl_ext() and dw8250_writel_ext()?
>
> >  struct dw8250_platform_data {
> >       u8 usr_reg;
> > @@ -220,6 +227,47 @@ static void dw8250_serial_out38x(struct uart_port *p, unsigned int offset, u32 v
> >       dw8250_serial_out(p, offset, value);
> >  }
> >
> > +/* Drain FIFO and wait for USR to be not busy and TX/RX FIFO empty */
> > +static void dw8250_tx_wait_empty_apb(struct uart_port *p)
> > +{
> > +     unsigned int tries = 20000;
> > +     unsigned int delay_threshold = tries - 1000;
> > +     unsigned int usr;
> > +
> > +     while (tries--) {
> > +             usr = readl(DW8250_REG(p, DW_UART_USR));
> > +
> > +             /* Check UART free and TX/RX FIFO empty */
> > +             if ((usr & ~DW_UART_USR_TFNF) == DW_UART_USR_TFE)
> > +                     break;
> > +
> > +             /* FIFO is still not empty, try to clear it */
> > +             if (tries < delay_threshold) {
> > +                     writel(UART_FCR_ENABLE_FIFO, DW8250_REG(p, UART_FCR));
> > +                     writel(UART_FCR_ENABLE_FIFO | UART_FCR_CLEAR_RCVR |
> > +                     UART_FCR_CLEAR_XMIT, DW8250_REG(p, UART_FCR));
>
> Please indent any continuation lines properly, in this case to the char
> after the opening parenthesis.
>
> > +                     writel(0, DW8250_REG(p, UART_FCR));
> > +                     udelay (1);
> > +             }
> > +     }
>
> This seems to be just rehashing the same non-robust algorithm. There's no
> way to ensure UART wouldn't become BUSY again because of Rx at any time.
> Thus, any amount of clearing FIFOs is just never going be fully safe.
>
> Long time ago, I attempted to create a more robust solution to this BUSY
> problem by temporarily enabling loopback which should prevent any new Rx
> from reaching the UART.
>
> Could you please try my patch that is attached to:
>
> https://lore.kernel.org/linux-serial/079c8fe6-9ce4-fa59-4b44-93e27dd376d6@linux.intel.com/
Thanks! I've applied this patch on the device I'm using and ran the
test that attempts
to write a large buffer while do_set_termios is called. I don't see
any softlockup
anymore, but during one test iteration the console hangs.

I could ssh to the host, `stty -F /dev/ttyS0 sane` hangs and same if I try to
write anything `echo test > /dev/ttyS0`. There are no errors in dmesg, and
tty interrupts are not triggered anymore. I have kernel watchdog enabled but
it didn't trigger.  I'm going to add some traces in dw8250_idle_enter and
dw8250_idle_exit and update this thread. It takes a while to reproduce,
sorry for the delayed response.

Thanks,
Adriana


>
> (I haven't found a way to reproduce this myself and so far all the
> reporters of this problem have gone oddly quiet when asked to test this
> patch so it's hasn't moved forward for fea years.)
>
> There are small Tx DMA related bits to add to the patch from robustness
> perspective (but a sane communication pattern shouldn't need those
> anyway, ie., no application should be sending something while trying to
> change these registers).
>
> > +}
> > +
> > +static void dw8250_serial_outapb(struct uart_port *p, int offset, int value)
> > +{
> > +       struct dw8250_data *d = to_dw8250_data(p->private_data);
> > +
> > +     if(offset == UART_LCR && !d->uart_16550_compatible)
> > +             dw8250_tx_wait_empty_apb(p);
> > +
> > +     writel(value, DW8250_REG(p, offset));
> > +
> > +     if (offset == UART_LCR && !d->uart_16550_compatible) {
> > +             /* Check FIFO is left enabled and LCR was written */
> > +             writel(UART_FCR_ENABLE_FIFO, DW8250_REG(p, UART_FCR));
> > +             dw8250_check_lcr(p, value);
> > +     }
> > +}
> > +
> >  static u32 dw8250_serial_in(struct uart_port *p, unsigned int offset)
> >  {
> >       u32 value = readb(p->membase + (offset << p->regshift));
> > @@ -520,6 +568,12 @@ static void dw8250_quirks(struct uart_port *p, struct dw8250_data *data)
> >               p->serial_in = dw8250_serial_in32;
> >               data->uart_16550_compatible = true;
> >       }
> > +     if (quirks & DW_UART_QUIRK_DWAPB) {
> > +             p->type = PORT_DWAPB;
> > +             p->flags |= UPF_FIXED_TYPE;
> > +             p->serial_out = dw8250_serial_outapb;
> > +             data->skip_autocfg = true;
> > +     }
> >  }
> >
> >  static void dw8250_reset_control_assert(void *data)
> > @@ -755,6 +809,7 @@ static const struct dev_pm_ops dw8250_pm_ops = {
> >
> >  static const struct dw8250_platform_data dw8250_dw_apb = {
> >       .usr_reg = DW_UART_USR,
> > +     .quirks = DW_UART_QUIRK_APB,
> >  };
> >
> >  static const struct dw8250_platform_data dw8250_octeon_3860_data = {
> > diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
> > index 2da9db960d09..3882a71920f6 100644
> > --- a/drivers/tty/serial/8250/8250_port.c
> > +++ b/drivers/tty/serial/8250/8250_port.c
> > @@ -310,6 +310,14 @@ static const struct serial8250_config uart_config[] = {
> >               .rxtrig_bytes   = {1, 8, 16, 30},
> >               .flags          = UART_CAP_FIFO | UART_CAP_AFE,
> >       },
> > +     [PORT_DWAPB] = {
> > +             .name           = "Synopsys DesignWare",
> > +             .fifo_size      = 16,
> > +             .tx_loadsz      = 16,
> > +             .fcr            = UART_FCR_ENABLE_FIFO |
> > +                               UART_FCR_CLEAR_RCVR | UART_FCR_CLEAR_XMIT,
> > +             .flags          = UART_CAP_FIFO | UART_CAP_AFE | UART_CAP_IRDA,
> > +     },
> >  };
> >
> >  /* Uart divisor latch read */
> > diff --git a/include/uapi/linux/serial_core.h b/include/uapi/linux/serial_core.h
> > index 9c007a106330..8386436b813f 100644
> > --- a/include/uapi/linux/serial_core.h
> > +++ b/include/uapi/linux/serial_core.h
> > @@ -231,6 +231,9 @@
> >  /* Sunplus UART */
> >  #define PORT_SUNPLUS 123
> >
> > +/* Synopsys DesignWare */
> > +#define PORT_DWAPB           124
> > +
> >  /* Generic type identifier for ports which type is not important to userspace. */
> >  #define PORT_GENERIC (-1)
> >
> >
>
> --
>  i.
>

Re: [PATCH] serial: 8250 dw: clear FIFO before writting LCR

[Ilpo Järvinen]

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

On Fri, 22 Aug 2025, Adriana Nicolae wrote:

> On Wed, Aug 20, 2025 at 1:02 PM Ilpo Järvinen
> <ilpo.jarvinen@linux.intel.com> wrote:
> >
> > On Tue, 19 Aug 2025, adriana@arista.com wrote:
> >
> > > This patch is proposing a custom configuration for Synopsys DesignWare
> >
> > Please try to avoid starting sentences in the changelog with "This patch"
> > or other wordings with similar meaning. Write imperatively instead.
> >
> > Preferrable, describe the problem first, then the solution.
> >
> > > serial to be used by products with associated compatible string in the
> > > device tree.
> > >
> > > The PORT_DWAPB config will be used instead of the default PORT_16550A
> > > which does not include the UART_FCR_CLEAR_RCVR and UART_FCR_CLEAR_XMIT
> > > bits for the FIFO configuration register. Having those flags is necessary
> > > to clear FIFO when the serial port is reconfigured with do_set_termios.
> > >
> > > Additionally, inside the do_set_termios we use the LCR (Line Control
> > > Register) to enable DLAB bit in order to access DLL/DLM (Divisor Latch
> > > Low/High) registers for baud rate setting. These 2 registers are sharing
> > > the same address space with UART_TX/UART_RX and UART_IER. The sequence is:
> > >
> > > (1) enable DLAB -> (2) set baud -> (3) disable DLAB -> (4) reset FCR
> > >
> > > When there is a TX or RX flow on the serial while we attempt to set/clear
> > > DLAB, the LCR write will be ignored and we will get a IIR_BUSY interrupt
> > > afterwards which is cleared by only reading the USR (UART status register).
> > >
> > > The sequence above can leave the serial in an unstable state in two cases:
> > >
> > > - if UART is busy while (1), then LCR is still pointing to the normal set of
> > > registers, which means the code setting DLL/DLM is actually writing into TX or
> > > modifying interrupts in UART_IER which may end with either a garbage character
> > > on the console or with serial interrupts disabled.
> > >
> > > - if UART is busy while (3), then LCR remains pointing to DLL/DLM instead of
> > > moving back to RX/TX. The first transfer on the serial will be stuck because
> > > the transmit/receive registers are not accessible unless the DLAB bit
> > > is cleared.
> > >
> > > The changes in this patch include a specific serial_out function for this UART
> > > type similar to the one for Armada-38x devices in commit
> > > b7639b0b15ddd1a4686b0142e70dfb122eefc88f with some changes in the tx_wait_empty
> > > function to check the UART status by looking at the USR register and actively
> > > try to clear FIFO to reduce time before a LCR write since the characters will
> > > be lost otherwise after baud rate change.
> > >
> > > The USR register may report that UART is busy even if TX/TX FIFO is already
> > > empty so we will loop until all USR[0] (UART busy status) is cleared and USR[1]
> > > TX FIFO is empty (RX FIFO bits should be 0 in this case).
> > > Keeping the same timeout of 20ms as measurements with the 9600 baud when
> > > the console was busy it took max 1.9ms to get the UART free state.
> > >
> > > Signed-off-by: Adriana Nicolae <adriana@arista.com>
> > > ---
> > >  drivers/tty/serial/8250/8250_dw.c   |   52 +++++++++++++++++++++++++++++++++++
> > >  drivers/tty/serial/8250/8250_port.c |    8 +++++
> > >  include/uapi/linux/serial_core.h    |    3 ++
> > >  3 files changed, 63 insertions(+)
> > >
> > > diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c
> > > index a53ba04d9770..985a2650f3f3 100644
> > > --- a/drivers/tty/serial/8250/8250_dw.c
> > > +++ b/drivers/tty/serial/8250/8250_dw.c
> > > @@ -33,6 +33,9 @@
> > >  /* Offsets for the DesignWare specific registers */
> > >  #define DW_UART_USR  0x1f /* UART Status Register */
> > >  #define DW_UART_DMASA        0xa8 /* DMA Software Ack */
> > > +#define DW_UART_USR_BUSY 0x1 /* UART Busy status */
> > > +#define DW_UART_USR_TFNF 0x2 /* UART TX FIFO not full */
> > > +#define DW_UART_USR_TFE  0x4 /* UART TX FIFO empty */
> > >
> > >  #define OCTEON_UART_USR      0x27 /* UART Status Register */
> > >
> > > @@ -56,6 +59,10 @@
> > >  #define DW_UART_QUIRK_IS_DMA_FC              BIT(3)
> > >  #define DW_UART_QUIRK_APMC0D08               BIT(4)
> > >  #define DW_UART_QUIRK_CPR_VALUE              BIT(5)
> > > +#define DW_UART_QUIRK_APB            BIT(6)
> > > +
> > > +#define DW8250_REG( p, reg ) \
> > > +     ((void __iomem *)(p->membase + ((reg) << p->regshift)))
> >
> > What's wrong with dw8250_readl_ext() and dw8250_writel_ext()?
> >
> > >  struct dw8250_platform_data {
> > >       u8 usr_reg;
> > > @@ -220,6 +227,47 @@ static void dw8250_serial_out38x(struct uart_port *p, unsigned int offset, u32 v
> > >       dw8250_serial_out(p, offset, value);
> > >  }
> > >
> > > +/* Drain FIFO and wait for USR to be not busy and TX/RX FIFO empty */
> > > +static void dw8250_tx_wait_empty_apb(struct uart_port *p)
> > > +{
> > > +     unsigned int tries = 20000;
> > > +     unsigned int delay_threshold = tries - 1000;
> > > +     unsigned int usr;
> > > +
> > > +     while (tries--) {
> > > +             usr = readl(DW8250_REG(p, DW_UART_USR));
> > > +
> > > +             /* Check UART free and TX/RX FIFO empty */
> > > +             if ((usr & ~DW_UART_USR_TFNF) == DW_UART_USR_TFE)
> > > +                     break;
> > > +
> > > +             /* FIFO is still not empty, try to clear it */
> > > +             if (tries < delay_threshold) {
> > > +                     writel(UART_FCR_ENABLE_FIFO, DW8250_REG(p, UART_FCR));
> > > +                     writel(UART_FCR_ENABLE_FIFO | UART_FCR_CLEAR_RCVR |
> > > +                     UART_FCR_CLEAR_XMIT, DW8250_REG(p, UART_FCR));
> >
> > Please indent any continuation lines properly, in this case to the char
> > after the opening parenthesis.
> >
> > > +                     writel(0, DW8250_REG(p, UART_FCR));
> > > +                     udelay (1);
> > > +             }
> > > +     }
> >
> > This seems to be just rehashing the same non-robust algorithm. There's no
> > way to ensure UART wouldn't become BUSY again because of Rx at any time.
> > Thus, any amount of clearing FIFOs is just never going be fully safe.
> >
> > Long time ago, I attempted to create a more robust solution to this BUSY
> > problem by temporarily enabling loopback which should prevent any new Rx
> > from reaching the UART.
> >
> > Could you please try my patch that is attached to:
> >
> > https://lore.kernel.org/linux-serial/079c8fe6-9ce4-fa59-4b44-93e27dd376d6@linux.intel.com/
>
> Thanks! I've applied this patch on the device I'm using and ran the
> test that attempts
> to write a large buffer while do_set_termios is called. I don't see
> any softlockup
> anymore, but during one test iteration the console hangs.

Can't think of anything obvious, idle enter and exit seem to always pair 
and idle exit should restore everything, AFAICT.

Maybe add an asserts using lockdep to ensure that the port lock is held.
While it looks ->in_idle should never remain set forever in case of a 
race, I'm not sure how DW UART takes two racing idle enter and/or exit 
calls.

With console, DMA shouldn't be relevant as DMA is not used for consoles.
If there are non-console UARTs involved, then Tx DMA might come into the 
picture if something is written to UART. Pausing DMA Tx is bit problematic 
though as (IIRC) pausing DMA was not always supported so the entire Tx 
would have to be terminated which obviously is going to lose characters 
(potentially quite many).

> I could ssh to the host, `stty -F /dev/ttyS0 sane` hangs and same if I try to
> write anything `echo test > /dev/ttyS0`. There are no errors in dmesg, and
> tty interrupts are not triggered anymore. I have kernel watchdog enabled but
> it didn't trigger.  I'm going to add some traces in dw8250_idle_enter and
> dw8250_idle_exit and update this thread.

If IER doesn't get messed up by something it looks a bit odd, checking LSR 
in dw8250_idle_exit() might provide some clues though that feels long 
shot to me. If DR is set, maybe try to read one char inside 
dw8250_idle_exit(), I'd think the best place for that is before IER is 
written but I'm not sure.

> It takes a while to reproduce, sorry for the delayed response.

Don't worry :-), your help is really appreciated.

-- 
 i.

> Thanks,
> Adriana
> 
> 
> >
> > (I haven't found a way to reproduce this myself and so far all the
> > reporters of this problem have gone oddly quiet when asked to test this
> > patch so it's hasn't moved forward for fea years.)
> >
> > There are small Tx DMA related bits to add to the patch from robustness
> > perspective (but a sane communication pattern shouldn't need those
> > anyway, ie., no application should be sending something while trying to
> > change these registers).
> >
> > > +}
> > > +
> > > +static void dw8250_serial_outapb(struct uart_port *p, int offset, int value)
> > > +{
> > > +       struct dw8250_data *d = to_dw8250_data(p->private_data);
> > > +
> > > +     if(offset == UART_LCR && !d->uart_16550_compatible)
> > > +             dw8250_tx_wait_empty_apb(p);
> > > +
> > > +     writel(value, DW8250_REG(p, offset));
> > > +
> > > +     if (offset == UART_LCR && !d->uart_16550_compatible) {
> > > +             /* Check FIFO is left enabled and LCR was written */
> > > +             writel(UART_FCR_ENABLE_FIFO, DW8250_REG(p, UART_FCR));
> > > +             dw8250_check_lcr(p, value);
> > > +     }
> > > +}
> > > +
> > >  static u32 dw8250_serial_in(struct uart_port *p, unsigned int offset)
> > >  {
> > >       u32 value = readb(p->membase + (offset << p->regshift));
> > > @@ -520,6 +568,12 @@ static void dw8250_quirks(struct uart_port *p, struct dw8250_data *data)
> > >               p->serial_in = dw8250_serial_in32;
> > >               data->uart_16550_compatible = true;
> > >       }
> > > +     if (quirks & DW_UART_QUIRK_DWAPB) {
> > > +             p->type = PORT_DWAPB;
> > > +             p->flags |= UPF_FIXED_TYPE;
> > > +             p->serial_out = dw8250_serial_outapb;
> > > +             data->skip_autocfg = true;
> > > +     }
> > >  }
> > >
> > >  static void dw8250_reset_control_assert(void *data)
> > > @@ -755,6 +809,7 @@ static const struct dev_pm_ops dw8250_pm_ops = {
> > >
> > >  static const struct dw8250_platform_data dw8250_dw_apb = {
> > >       .usr_reg = DW_UART_USR,
> > > +     .quirks = DW_UART_QUIRK_APB,
> > >  };
> > >
> > >  static const struct dw8250_platform_data dw8250_octeon_3860_data = {
> > > diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
> > > index 2da9db960d09..3882a71920f6 100644
> > > --- a/drivers/tty/serial/8250/8250_port.c
> > > +++ b/drivers/tty/serial/8250/8250_port.c
> > > @@ -310,6 +310,14 @@ static const struct serial8250_config uart_config[] = {
> > >               .rxtrig_bytes   = {1, 8, 16, 30},
> > >               .flags          = UART_CAP_FIFO | UART_CAP_AFE,
> > >       },
> > > +     [PORT_DWAPB] = {
> > > +             .name           = "Synopsys DesignWare",
> > > +             .fifo_size      = 16,
> > > +             .tx_loadsz      = 16,
> > > +             .fcr            = UART_FCR_ENABLE_FIFO |
> > > +                               UART_FCR_CLEAR_RCVR | UART_FCR_CLEAR_XMIT,
> > > +             .flags          = UART_CAP_FIFO | UART_CAP_AFE | UART_CAP_IRDA,
> > > +     },
> > >  };
> > >
> > >  /* Uart divisor latch read */
> > > diff --git a/include/uapi/linux/serial_core.h b/include/uapi/linux/serial_core.h
> > > index 9c007a106330..8386436b813f 100644
> > > --- a/include/uapi/linux/serial_core.h
> > > +++ b/include/uapi/linux/serial_core.h
> > > @@ -231,6 +231,9 @@
> > >  /* Sunplus UART */
> > >  #define PORT_SUNPLUS 123
> > >
> > > +/* Synopsys DesignWare */
> > > +#define PORT_DWAPB           124
> > > +
> > >  /* Generic type identifier for ports which type is not important to userspace. */
> > >  #define PORT_GENERIC (-1)
> > >
> > >
> >
> > --
> >  i.
> >
> 

Re: [PATCH] serial: 8250 dw: clear FIFO before writting LCR

[Ilpo Järvinen]

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

On Fri, 22 Aug 2025, Adriana Nicolae wrote:

> On Wed, Aug 20, 2025 at 1:02 PM Ilpo Järvinen
> <ilpo.jarvinen@linux.intel.com> wrote:
> >
> > On Tue, 19 Aug 2025, adriana@arista.com wrote:
> >
> > > This patch is proposing a custom configuration for Synopsys DesignWare
> >
> > Please try to avoid starting sentences in the changelog with "This patch"
> > or other wordings with similar meaning. Write imperatively instead.
> >
> > Preferrable, describe the problem first, then the solution.
> >
> > > serial to be used by products with associated compatible string in the
> > > device tree.
> > >
> > > The PORT_DWAPB config will be used instead of the default PORT_16550A
> > > which does not include the UART_FCR_CLEAR_RCVR and UART_FCR_CLEAR_XMIT
> > > bits for the FIFO configuration register. Having those flags is necessary
> > > to clear FIFO when the serial port is reconfigured with do_set_termios.
> > >
> > > Additionally, inside the do_set_termios we use the LCR (Line Control
> > > Register) to enable DLAB bit in order to access DLL/DLM (Divisor Latch
> > > Low/High) registers for baud rate setting. These 2 registers are sharing
> > > the same address space with UART_TX/UART_RX and UART_IER. The sequence is:
> > >
> > > (1) enable DLAB -> (2) set baud -> (3) disable DLAB -> (4) reset FCR
> > >
> > > When there is a TX or RX flow on the serial while we attempt to set/clear
> > > DLAB, the LCR write will be ignored and we will get a IIR_BUSY interrupt
> > > afterwards which is cleared by only reading the USR (UART status register).
> > >
> > > The sequence above can leave the serial in an unstable state in two cases:
> > >
> > > - if UART is busy while (1), then LCR is still pointing to the normal set of
> > > registers, which means the code setting DLL/DLM is actually writing into TX or
> > > modifying interrupts in UART_IER which may end with either a garbage character
> > > on the console or with serial interrupts disabled.
> > >
> > > - if UART is busy while (3), then LCR remains pointing to DLL/DLM instead of
> > > moving back to RX/TX. The first transfer on the serial will be stuck because
> > > the transmit/receive registers are not accessible unless the DLAB bit
> > > is cleared.
> > >
> > > The changes in this patch include a specific serial_out function for this UART
> > > type similar to the one for Armada-38x devices in commit
> > > b7639b0b15ddd1a4686b0142e70dfb122eefc88f with some changes in the tx_wait_empty
> > > function to check the UART status by looking at the USR register and actively
> > > try to clear FIFO to reduce time before a LCR write since the characters will
> > > be lost otherwise after baud rate change.
> > >
> > > The USR register may report that UART is busy even if TX/TX FIFO is already
> > > empty so we will loop until all USR[0] (UART busy status) is cleared and USR[1]
> > > TX FIFO is empty (RX FIFO bits should be 0 in this case).
> > > Keeping the same timeout of 20ms as measurements with the 9600 baud when
> > > the console was busy it took max 1.9ms to get the UART free state.
> > >
> > > Signed-off-by: Adriana Nicolae <adriana@arista.com>
> > > ---
> > >  drivers/tty/serial/8250/8250_dw.c   |   52 +++++++++++++++++++++++++++++++++++
> > >  drivers/tty/serial/8250/8250_port.c |    8 +++++
> > >  include/uapi/linux/serial_core.h    |    3 ++
> > >  3 files changed, 63 insertions(+)
> > >
> > > diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c
> > > index a53ba04d9770..985a2650f3f3 100644
> > > --- a/drivers/tty/serial/8250/8250_dw.c
> > > +++ b/drivers/tty/serial/8250/8250_dw.c
> > > @@ -33,6 +33,9 @@
> > >  /* Offsets for the DesignWare specific registers */
> > >  #define DW_UART_USR  0x1f /* UART Status Register */
> > >  #define DW_UART_DMASA        0xa8 /* DMA Software Ack */
> > > +#define DW_UART_USR_BUSY 0x1 /* UART Busy status */
> > > +#define DW_UART_USR_TFNF 0x2 /* UART TX FIFO not full */
> > > +#define DW_UART_USR_TFE  0x4 /* UART TX FIFO empty */
> > >
> > >  #define OCTEON_UART_USR      0x27 /* UART Status Register */
> > >
> > > @@ -56,6 +59,10 @@
> > >  #define DW_UART_QUIRK_IS_DMA_FC              BIT(3)
> > >  #define DW_UART_QUIRK_APMC0D08               BIT(4)
> > >  #define DW_UART_QUIRK_CPR_VALUE              BIT(5)
> > > +#define DW_UART_QUIRK_APB            BIT(6)
> > > +
> > > +#define DW8250_REG( p, reg ) \
> > > +     ((void __iomem *)(p->membase + ((reg) << p->regshift)))
> >
> > What's wrong with dw8250_readl_ext() and dw8250_writel_ext()?
> >
> > >  struct dw8250_platform_data {
> > >       u8 usr_reg;
> > > @@ -220,6 +227,47 @@ static void dw8250_serial_out38x(struct uart_port *p, unsigned int offset, u32 v
> > >       dw8250_serial_out(p, offset, value);
> > >  }
> > >
> > > +/* Drain FIFO and wait for USR to be not busy and TX/RX FIFO empty */
> > > +static void dw8250_tx_wait_empty_apb(struct uart_port *p)
> > > +{
> > > +     unsigned int tries = 20000;
> > > +     unsigned int delay_threshold = tries - 1000;
> > > +     unsigned int usr;
> > > +
> > > +     while (tries--) {
> > > +             usr = readl(DW8250_REG(p, DW_UART_USR));
> > > +
> > > +             /* Check UART free and TX/RX FIFO empty */
> > > +             if ((usr & ~DW_UART_USR_TFNF) == DW_UART_USR_TFE)
> > > +                     break;
> > > +
> > > +             /* FIFO is still not empty, try to clear it */
> > > +             if (tries < delay_threshold) {
> > > +                     writel(UART_FCR_ENABLE_FIFO, DW8250_REG(p, UART_FCR));
> > > +                     writel(UART_FCR_ENABLE_FIFO | UART_FCR_CLEAR_RCVR |
> > > +                     UART_FCR_CLEAR_XMIT, DW8250_REG(p, UART_FCR));
> >
> > Please indent any continuation lines properly, in this case to the char
> > after the opening parenthesis.
> >
> > > +                     writel(0, DW8250_REG(p, UART_FCR));
> > > +                     udelay (1);
> > > +             }
> > > +     }
> >
> > This seems to be just rehashing the same non-robust algorithm. There's no
> > way to ensure UART wouldn't become BUSY again because of Rx at any time.
> > Thus, any amount of clearing FIFOs is just never going be fully safe.
> >
> > Long time ago, I attempted to create a more robust solution to this BUSY
> > problem by temporarily enabling loopback which should prevent any new Rx
> > from reaching the UART.
> >
> > Could you please try my patch that is attached to:
> >
> > https://lore.kernel.org/linux-serial/079c8fe6-9ce4-fa59-4b44-93e27dd376d6@linux.intel.com/
> Thanks! I've applied this patch on the device I'm using and ran the
> test that attempts
> to write a large buffer while do_set_termios is called. I don't see
> any softlockup
> anymore, but during one test iteration the console hangs.
> 
> I could ssh to the host, `stty -F /dev/ttyS0 sane` hangs and same if I try to
> write anything `echo test > /dev/ttyS0`. There are no errors in dmesg, and
> tty interrupts are not triggered anymore. I have kernel watchdog enabled but
> it didn't trigger.  I'm going to add some traces in dw8250_idle_enter and
> dw8250_idle_exit and update this thread. It takes a while to reproduce,
> sorry for the delayed response.

Hi Adriana,

Did you ever manage to figure out what goes wrong with the idle enter/exit 
patch?

-- 
 i.

Re: [PATCH] serial: 8250 dw: clear FIFO before writting LCR

[Ilpo Järvinen]

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

On Wed, 8 Oct 2025, Adriana Nicolae wrote:
> Hello, Sorry for missing an update, the exact root cause is not clear, currently the
> brute force method of draining FIFO right before setting or clearing DLAB was stable
> during tests.
> 
> The serial stuck seems to be a failed attempt to clear the DLAB.
> This operation fails because the USR indicates the hardware is
> still busy, even though the UART is in loopback mode and should
> be inactive.
> 
> To isolate the issue, I tried the following without success:
> - Added delays: I inserted 100 repeated ndelay(p->frame_time)
> calls before and after enabling loopback mode to allow the FIFO
> to clear.
> - Cleared FIFO: I explicitly cleared the FIFO in addition to
> adding the delay.
> - Checked status: I printed the LSR just before the DLAB clear
> attempt and checked the USB busy bit.

Okay, so the BUSY must be stuck asserted.

Another idea, maybe test tx + rx over loopback to see if that manages to 
de-freeze the BUSY bit. A patch to that effect below.

(I've only added the new block into dw8250_idle_enter() compared with the 
old patch but rebasing to tty-next resulted in some other changes due to 
conflicts.)


Only thing besides BUSY being stuck asserted is something Tx'ing after the 
idle enter sequence. I think we could trap/check that too in 
dw8250_serial_out() by using something along these lines:

	if (d->in_idle && offset == UART_TX) {
		WARN_ON_ONCE(1);
		return;
	}

(I think that should catch even console writes but I'm not 100% sure of 
that and it will should get us where the rogue Tx originates from).

> The critical finding was that immediately before the DLAB clear
> operation (p->serial_out(p, UART_LCR, up->lcr);), the LSR value
> was 0x6a and the USR busy bit [0] was set. This confirms the UART
> is busy, which blocks the DLAB modification.
> 
> This is occurring on a device with a single UART console. The setup
> does not use DMA or modem control; only the Rx/Tx lines are connected.
> 
> The trace below, from a single process, shows one successful DLAB
> clear followed by a failing one. The second attempt repeatedly logs
> "USR still busy" before eventually succeeding. This can lead to
> subsequent failures in dw8250_check_lcr: dw8250_idle_entered.
> 
> Trace Log:
> 
> <...>-15440  8583.592533: dw8250_idle_enter: in_idle = 1
> login-15440  8583.713817: dw8250_idle_enter: in loopback mode
> login-15440  8583.835099: dw8250_idle_enter: LSR in loopback mode is 0x63
> login-15440  8583.835103: dw8250_set_divisor: UART_LCR_DLAB cleared 13
> login-15440  8583.835104: dw8250_idle_exit: idle exit
> login-15440  8583.835105: dw8250_idle_exit: out of loopback mode
> login-15440  8583.835105: dw8250_idle_exit: in_idle = 0
> login-15440  8583.835352: dw8250_idle_enter: in_idle = 1
> login-15440  8583.956633: dw8250_idle_enter: in loopback mode
> login-15440  8583.956638: dw8250_idle_enter: LSR in loopback mode is 0x6a
> login-15440  8583.963918: dw8250_set_divisor: USR still busy dl_write      
> login-15440  8584.000332: dw8250_set_divisor: USR still busy dl_write
> login-15440  8584.040385: dw8250_set_divisor: USR still busy dl_write
> login-15440  8584.078012: dw8250_set_divisor: UART_LCR_DLAB cleared 93
> login-15440  8584.078013: dw8250_idle_exit: idle exit
> login-15440  8584.078014: dw8250_idle_exit: out of loopback mode
> login-15440  8584.078015: dw8250_idle_exit: in_idle = 0



--
From 01df58736a10f7f34aca895ef08e5519953f8572 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ilpo=20J=C3=A4rvinen?= <ilpo.jarvinen@linux.intel.com>
Date: Wed, 8 Oct 2025 15:40:19 +0300
Subject: [PATCH 1/1] serial: 8250_dw: Ensure BUSY is deasserted
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

DW UART cannot write to LCR, DLL, and DLH while BUSY is asserted.
Existance of BUSY depends on uart_16550_compatible, if UART HW is
configured with 16550 compatible those registers can always be
written.

There currently is dw8250_force_idle() which attempts to archive
non-BUSY state by disabling FIFO, however, the solution is unreliable
when Rx keeps getting more and more characters.

Create a sequence of operations to enforce that ensures UART cannot
keep BUSY asserted indefinitely. The new sequence relies on enabling
loopback mode temporarily to prevent incoming Rx characters keeping
UART BUSY.

Use the new dw8250_idle_enter/exit() to do divisor writes and LCR
writes.

This issue was reported by qianfan Zhao who put lots of debugging
effort into understanding the solution space.

Reported-by: qianfan Zhao <qianfanguijin@163.com>
Reported-by: Adriana Nicolae <adriana@arista.com>
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
 drivers/tty/serial/8250/8250_dw.c | 159 +++++++++++++++++++++---------
 1 file changed, 115 insertions(+), 44 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c
index a53ba04d9770..8e25dfe8e653 100644
--- a/drivers/tty/serial/8250/8250_dw.c
+++ b/drivers/tty/serial/8250/8250_dw.c
@@ -42,6 +42,8 @@
 /* DesignWare specific register fields */
 #define DW_UART_MCR_SIRE		BIT(6)
 
+#define DW_UART_USR_BUSY		BIT(0)
+
 /* Renesas specific register fields */
 #define RZN1_UART_xDMACR_DMA_EN		BIT(0)
 #define RZN1_UART_xDMACR_1_WORD_BURST	(0 << 1)
@@ -77,6 +79,7 @@ struct dw8250_data {
 
 	unsigned int		skip_autocfg:1;
 	unsigned int		uart_16550_compatible:1;
+	unsigned int		in_idle:1;
 };
 
 static inline struct dw8250_data *to_dw8250_data(struct dw8250_port_data *data)
@@ -108,36 +111,103 @@ static inline u32 dw8250_modify_msr(struct uart_port *p, unsigned int offset, u3
 }
 
 /*
- * This function is being called as part of the uart_port::serial_out()
- * routine. Hence, it must not call serial_port_out() or serial_out()
- * against the modified registers here, i.e. LCR.
+ * Ensure BUSY is not asserted. If DW UART is configured with
+ * !uart_16550_compatible, the writes to LCR, DLL, and DLH fail while
+ * BUSY is asserted.
+ *
+ * Context: port's lock must be held
  */
-static void dw8250_force_idle(struct uart_port *p)
+static int dw8250_idle_enter(struct uart_port *p)
 {
+	struct dw8250_data *d = to_dw8250_data(p->private_data);
 	struct uart_8250_port *up = up_to_u8250p(p);
-	unsigned int lsr;
+	u32 lsr;
 
-	/*
-	 * The following call currently performs serial_out()
-	 * against the FCR register. Because it differs to LCR
-	 * there will be no infinite loop, but if it ever gets
-	 * modified, we might need a new custom version of it
-	 * that avoids infinite recursion.
-	 */
-	serial8250_clear_and_reinit_fifos(up);
+	if (d->uart_16550_compatible)
+		return 0;
+
+	d->in_idle = 1;
+
+	/* Prevent triggering interrupt from RBR filling */
+	p->serial_out(p, UART_IER, 0);
+
+	serial8250_rx_dma_flush(up);
+	// What about Tx DMA? Should probably pause that too and resume
+	// afterwards.
+
+	p->serial_out(p, UART_MCR, up->mcr | UART_MCR_LOOP);
+	if (up->capabilities & UART_CAP_FIFO)
+		p->serial_out(p, UART_FCR, 0);
+
+	if (p->serial_in(p, d->pdata->usr_reg) & DW_UART_USR_BUSY)
+		ndelay(p->frame_time);
+
+	lsr = serial_lsr_in(up);
+	if (lsr & UART_LSR_DR) {
+		p->serial_in(p, UART_RX);
+		up->lsr_saved_flags = 0;
+	}
 
 	/*
-	 * With PSLVERR_RESP_EN parameter set to 1, the device generates an
-	 * error response when an attempt to read an empty RBR with FIFO
-	 * enabled.
+	 * BUSY might still be frozen to asserted, try to de-freeze it by
+	 * sending a character over the loopback and receiving it.
 	 */
-	if (up->fcr & UART_FCR_ENABLE_FIFO) {
-		lsr = serial_port_in(p, UART_LSR);
-		if (!(lsr & UART_LSR_DR))
-			return;
+	if (p->serial_in(p, d->pdata->usr_reg) & DW_UART_USR_BUSY) {
+		p->serial_out(p, UART_TX, 0);
+		ndelay(p->frame_time);
+		lsr = serial_lsr_in(up);
+		if (lsr & UART_LSR_DR) {
+			p->serial_in(p, UART_RX);
+			up->lsr_saved_flags = 0;
+		}
 	}
 
-	serial_port_in(p, UART_RX);
+	/* Now guaranteed to have BUSY deasserted? Just sanity check */
+	if (p->serial_in(p, d->pdata->usr_reg) & DW_UART_USR_BUSY)
+		return -EBUSY;
+
+	return 0;
+}
+
+static void dw8250_idle_exit(struct uart_port *p)
+{
+	struct dw8250_data *d = to_dw8250_data(p->private_data);
+	struct uart_8250_port *up = up_to_u8250p(p);
+
+	if (d->uart_16550_compatible)
+		return;
+
+	if (up->capabilities & UART_CAP_FIFO)
+		p->serial_out(p, UART_FCR, up->fcr);
+	p->serial_out(p, UART_MCR, up->mcr);
+	p->serial_out(p, UART_IER, up->ier);
+
+	// Maybe move the DMA Rx restart check in dma_rx_complete() to own
+	// function (serial8250_rx_dma_restart()) and call it from here.
+	// DMA Tx resume
+
+	d->in_idle = 0;
+}
+
+static void dw8250_set_divisor(struct uart_port *p, unsigned int baud,
+			       unsigned int quot, unsigned int quot_frac)
+{
+	struct uart_8250_port *up = up_to_u8250p(p);
+	int ret;
+
+	ret = dw8250_idle_enter(p);
+	if (ret < 0)
+		goto idle_failed;
+
+	p->serial_out(p, UART_LCR, up->lcr | UART_LCR_DLAB);
+	if (!(p->serial_in(p, UART_LCR) & UART_LCR_DLAB))
+		goto idle_failed;
+
+	serial_dl_write(up, quot);
+	p->serial_out(p, UART_LCR, up->lcr);
+
+idle_failed:
+	dw8250_idle_exit(p);
 }
 
 /*
@@ -148,37 +218,37 @@ static void dw8250_force_idle(struct uart_port *p)
 static void dw8250_check_lcr(struct uart_port *p, unsigned int offset, u32 value)
 {
 	struct dw8250_data *d = to_dw8250_data(p->private_data);
-	void __iomem *addr = p->membase + (offset << p->regshift);
-	int tries = 1000;
+	u32 lcr = p->serial_in(p, UART_LCR);
+	int ret;
 
 	if (offset != UART_LCR || d->uart_16550_compatible)
 		return;
 
 	/* Make sure LCR write wasn't ignored */
-	while (tries--) {
-		u32 lcr = serial_port_in(p, offset);
-
-		if ((value & ~UART_LCR_SPAR) == (lcr & ~UART_LCR_SPAR))
-			return;
+	if ((value & ~UART_LCR_SPAR) == (lcr & ~UART_LCR_SPAR))
+		return;
 
-		dw8250_force_idle(p);
+	if (d->in_idle) {
+		/*
+		 * FIXME: this deadlocks if port->lock is already held
+		 * dev_err(p->dev, "Couldn't set LCR to %d\n", value);
+		 */
+		return;
+	}
 
-#ifdef CONFIG_64BIT
-		if (p->type == PORT_OCTEON)
-			__raw_writeq(value & 0xff, addr);
-		else
-#endif
-		if (p->iotype == UPIO_MEM32)
-			writel(value, addr);
-		else if (p->iotype == UPIO_MEM32BE)
-			iowrite32be(value, addr);
-		else
-			writeb(value, addr);
+	ret = dw8250_idle_enter(p);
+	if (ret < 0) {
+		/*
+		 * FIXME: this deadlocks if port->lock is already held
+		 * dev_err(p->dev, "Couldn't set LCR to %d\n", value);
+		 */
+		goto idle_failed;
 	}
-	/*
-	 * FIXME: this deadlocks if port->lock is already held
-	 * dev_err(p->dev, "Couldn't set LCR to %d\n", value);
-	 */
+
+	p->serial_out(p, UART_LCR, value);
+
+idle_failed:
+	dw8250_idle_exit(p);
 }
 
 /* Returns once the transmitter is empty or we run out of retries */
@@ -547,6 +617,7 @@ static int dw8250_probe(struct platform_device *pdev)
 	p->dev		= dev;
 	p->set_ldisc	= dw8250_set_ldisc;
 	p->set_termios	= dw8250_set_termios;
+	p->set_divisor	= dw8250_set_divisor;
 
 	data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
 	if (!data)

base-commit: d21b26cad33250be758ea9d860ff9d5c3992c459
-- 
2.39.5

Re: [PATCH] serial: 8250 dw: clear FIFO before writting LCR

[Adriana Nicolae]

On Wed, Oct 8, 2025 at 4:12 PM Ilpo Järvinen
<ilpo.jarvinen@linux.intel.com> wrote:
>
> On Wed, 8 Oct 2025, Adriana Nicolae wrote:
> > Hello, Sorry for missing an update, the exact root cause is not clear, currently the
> > brute force method of draining FIFO right before setting or clearing DLAB was stable
> > during tests.
> >
> > The serial stuck seems to be a failed attempt to clear the DLAB.
> > This operation fails because the USR indicates the hardware is
> > still busy, even though the UART is in loopback mode and should
> > be inactive.
> >
> > To isolate the issue, I tried the following without success:
> > - Added delays: I inserted 100 repeated ndelay(p->frame_time)
> > calls before and after enabling loopback mode to allow the FIFO
> > to clear.
> > - Cleared FIFO: I explicitly cleared the FIFO in addition to
> > adding the delay.
> > - Checked status: I printed the LSR just before the DLAB clear
> > attempt and checked the USB busy bit.
>
> Okay, so the BUSY must be stuck asserted.
>
> Another idea, maybe test tx + rx over loopback to see if that manages to
> de-freeze the BUSY bit. A patch to that effect below.
>
> (I've only added the new block into dw8250_idle_enter() compared with the
> old patch but rebasing to tty-next resulted in some other changes due to
> conflicts.)
>
I've tested the new dw8250_idle_enter() sequence, and you're right,
the BUSY bit remains set after entering loopback mode.

However, the sequence in the patch (including the single loopback
tx/rx) wasn't quite enough. I didn't see any kernel panics or console
stuck anymore, but I've monitored the traces and there were cases when
the trace after "p->serial_out(p, UART_LCR, up->lcr);" showed both
BUSY bit set and DLAB bit still enabled.

>
> Only thing besides BUSY being stuck asserted is something Tx'ing after the
> idle enter sequence. I think we could trap/check that too in
> dw8250_serial_out() by using something along these lines:
>
>         if (d->in_idle && offset == UART_TX) {
>                 WARN_ON_ONCE(1);
>                 return;
>         }
>
> (I think that should catch even console writes but I'm not 100% sure of
> that and it will should get us where the rogue Tx originates from).

I also added the WARN_ON_ONCE check you suggested in
dw8250_serial_out(). The warning has not triggered, so it seems we
don't have a rogue Tx firing while in_idle is set.

>
> > The critical finding was that immediately before the DLAB clear
> > operation (p->serial_out(p, UART_LCR, up->lcr);), the LSR value
> > was 0x6a and the USR busy bit [0] was set. This confirms the UART
> > is busy, which blocks the DLAB modification.
> >
> > This is occurring on a device with a single UART console. The setup
> > does not use DMA or modem control; only the Rx/Tx lines are connected.
> >
> > The trace below, from a single process, shows one successful DLAB
> > clear followed by a failing one. The second attempt repeatedly logs
> > "USR still busy" before eventually succeeding. This can lead to
> > subsequent failures in dw8250_check_lcr: dw8250_idle_entered.
> >
> > Trace Log:
> >
> > <...>-15440  8583.592533: dw8250_idle_enter: in_idle = 1
> > login-15440  8583.713817: dw8250_idle_enter: in loopback mode
> > login-15440  8583.835099: dw8250_idle_enter: LSR in loopback mode is 0x63
> > login-15440  8583.835103: dw8250_set_divisor: UART_LCR_DLAB cleared 13
> > login-15440  8583.835104: dw8250_idle_exit: idle exit
> > login-15440  8583.835105: dw8250_idle_exit: out of loopback mode
> > login-15440  8583.835105: dw8250_idle_exit: in_idle = 0
> > login-15440  8583.835352: dw8250_idle_enter: in_idle = 1
> > login-15440  8583.956633: dw8250_idle_enter: in loopback mode
> > login-15440  8583.956638: dw8250_idle_enter: LSR in loopback mode is 0x6a
> > login-15440  8583.963918: dw8250_set_divisor: USR still busy dl_write
> > login-15440  8584.000332: dw8250_set_divisor: USR still busy dl_write
> > login-15440  8584.040385: dw8250_set_divisor: USR still busy dl_write
> > login-15440  8584.078012: dw8250_set_divisor: UART_LCR_DLAB cleared 93
> > login-15440  8584.078013: dw8250_idle_exit: idle exit
> > login-15440  8584.078014: dw8250_idle_exit: out of loopback mode
> > login-15440  8584.078015: dw8250_idle_exit: in_idle = 0
>
>
>
> --
> From 01df58736a10f7f34aca895ef08e5519953f8572 Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?Ilpo=20J=C3=A4rvinen?= <ilpo.jarvinen@linux.intel.com>
> Date: Wed, 8 Oct 2025 15:40:19 +0300
> Subject: [PATCH 1/1] serial: 8250_dw: Ensure BUSY is deasserted
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
>
> DW UART cannot write to LCR, DLL, and DLH while BUSY is asserted.
> Existance of BUSY depends on uart_16550_compatible, if UART HW is
> configured with 16550 compatible those registers can always be
> written.
>
> There currently is dw8250_force_idle() which attempts to archive
> non-BUSY state by disabling FIFO, however, the solution is unreliable
> when Rx keeps getting more and more characters.
>
> Create a sequence of operations to enforce that ensures UART cannot
> keep BUSY asserted indefinitely. The new sequence relies on enabling
> loopback mode temporarily to prevent incoming Rx characters keeping
> UART BUSY.
>
> Use the new dw8250_idle_enter/exit() to do divisor writes and LCR
> writes.
>
> This issue was reported by qianfan Zhao who put lots of debugging
> effort into understanding the solution space.
>
> Reported-by: qianfan Zhao <qianfanguijin@163.com>
> Reported-by: Adriana Nicolae <adriana@arista.com>
> Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> ---
>  drivers/tty/serial/8250/8250_dw.c | 159 +++++++++++++++++++++---------
>  1 file changed, 115 insertions(+), 44 deletions(-)
>
> diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c
> index a53ba04d9770..8e25dfe8e653 100644
> --- a/drivers/tty/serial/8250/8250_dw.c
> +++ b/drivers/tty/serial/8250/8250_dw.c
> @@ -42,6 +42,8 @@
>  /* DesignWare specific register fields */
>  #define DW_UART_MCR_SIRE               BIT(6)
>
> +#define DW_UART_USR_BUSY               BIT(0)
> +
>  /* Renesas specific register fields */
>  #define RZN1_UART_xDMACR_DMA_EN                BIT(0)
>  #define RZN1_UART_xDMACR_1_WORD_BURST  (0 << 1)
> @@ -77,6 +79,7 @@ struct dw8250_data {
>
>         unsigned int            skip_autocfg:1;
>         unsigned int            uart_16550_compatible:1;
> +       unsigned int            in_idle:1;
>  };
>
>  static inline struct dw8250_data *to_dw8250_data(struct dw8250_port_data *data)
> @@ -108,36 +111,103 @@ static inline u32 dw8250_modify_msr(struct uart_port *p, unsigned int offset, u3
>  }
>
>  /*
> - * This function is being called as part of the uart_port::serial_out()
> - * routine. Hence, it must not call serial_port_out() or serial_out()
> - * against the modified registers here, i.e. LCR.
> + * Ensure BUSY is not asserted. If DW UART is configured with
> + * !uart_16550_compatible, the writes to LCR, DLL, and DLH fail while
> + * BUSY is asserted.
> + *
> + * Context: port's lock must be held
>   */
> -static void dw8250_force_idle(struct uart_port *p)
> +static int dw8250_idle_enter(struct uart_port *p)
>  {
> +       struct dw8250_data *d = to_dw8250_data(p->private_data);
>         struct uart_8250_port *up = up_to_u8250p(p);
> -       unsigned int lsr;
> +       u32 lsr;
>
> -       /*
> -        * The following call currently performs serial_out()
> -        * against the FCR register. Because it differs to LCR
> -        * there will be no infinite loop, but if it ever gets
> -        * modified, we might need a new custom version of it
> -        * that avoids infinite recursion.
> -        */
> -       serial8250_clear_and_reinit_fifos(up);
> +       if (d->uart_16550_compatible)
> +               return 0;
> +
> +       d->in_idle = 1;
> +
> +       /* Prevent triggering interrupt from RBR filling */
> +       p->serial_out(p, UART_IER, 0);
> +
> +       serial8250_rx_dma_flush(up);
> +       // What about Tx DMA? Should probably pause that too and resume
> +       // afterwards.
> +
> +       p->serial_out(p, UART_MCR, up->mcr | UART_MCR_LOOP);
> +       if (up->capabilities & UART_CAP_FIFO)
> +               p->serial_out(p, UART_FCR, 0);

Changing this to repeatedly clear the FIFO turned out to reliably
clear the BUSY bit , also no kernel panic or device stuck in busy
mode.

On the device I tested the first clear is not always enough, under
high load I saw it cleared on the second iteration. I'm thinking it
might be some particular issue with the device I'm using where the
first FIFO clear might fail. I never encountered more than 2
iterations with a "ndelay(p->frame_time);" in between here to get out
of BUSY state.

> +
> +       if (p->serial_in(p, d->pdata->usr_reg) & DW_UART_USR_BUSY)
> +               ndelay(p->frame_time);
> +
> +       lsr = serial_lsr_in(up);
> +       if (lsr & UART_LSR_DR) {
> +               p->serial_in(p, UART_RX);
> +               up->lsr_saved_flags = 0;
> +       }
>
>         /*
> -        * With PSLVERR_RESP_EN parameter set to 1, the device generates an
> -        * error response when an attempt to read an empty RBR with FIFO
> -        * enabled.
> +        * BUSY might still be frozen to asserted, try to de-freeze it by
> +        * sending a character over the loopback and receiving it.
>          */
> -       if (up->fcr & UART_FCR_ENABLE_FIFO) {
> -               lsr = serial_port_in(p, UART_LSR);
> -               if (!(lsr & UART_LSR_DR))
> -                       return;
> +       if (p->serial_in(p, d->pdata->usr_reg) & DW_UART_USR_BUSY) {
> +               p->serial_out(p, UART_TX, 0);
> +               ndelay(p->frame_time);
> +               lsr = serial_lsr_in(up);
> +               if (lsr & UART_LSR_DR) {
> +                       p->serial_in(p, UART_RX);
> +                       up->lsr_saved_flags = 0;
> +               }
>         }
>
> -       serial_port_in(p, UART_RX);
> +       /* Now guaranteed to have BUSY deasserted? Just sanity check */
> +       if (p->serial_in(p, d->pdata->usr_reg) & DW_UART_USR_BUSY)
> +               return -EBUSY;
> +
> +       return 0;
> +}
> +
> +static void dw8250_idle_exit(struct uart_port *p)
> +{
> +       struct dw8250_data *d = to_dw8250_data(p->private_data);
> +       struct uart_8250_port *up = up_to_u8250p(p);
> +
> +       if (d->uart_16550_compatible)
> +               return;
> +
> +       if (up->capabilities & UART_CAP_FIFO)
> +               p->serial_out(p, UART_FCR, up->fcr);
> +       p->serial_out(p, UART_MCR, up->mcr);
> +       p->serial_out(p, UART_IER, up->ier);
> +
> +       // Maybe move the DMA Rx restart check in dma_rx_complete() to own
> +       // function (serial8250_rx_dma_restart()) and call it from here.
> +       // DMA Tx resume
> +
> +       d->in_idle = 0;
> +}
> +
> +static void dw8250_set_divisor(struct uart_port *p, unsigned int baud,
> +                              unsigned int quot, unsigned int quot_frac)
> +{
> +       struct uart_8250_port *up = up_to_u8250p(p);
> +       int ret;
> +
> +       ret = dw8250_idle_enter(p);
> +       if (ret < 0)
> +               goto idle_failed;
> +
> +       p->serial_out(p, UART_LCR, up->lcr | UART_LCR_DLAB);
> +       if (!(p->serial_in(p, UART_LCR) & UART_LCR_DLAB))
> +               goto idle_failed;
> +
> +       serial_dl_write(up, quot);
> +       p->serial_out(p, UART_LCR, up->lcr);
> +
> +idle_failed:
> +       dw8250_idle_exit(p);
>  }
>
>  /*
> @@ -148,37 +218,37 @@ static void dw8250_force_idle(struct uart_port *p)
>  static void dw8250_check_lcr(struct uart_port *p, unsigned int offset, u32 value)
>  {
>         struct dw8250_data *d = to_dw8250_data(p->private_data);
> -       void __iomem *addr = p->membase + (offset << p->regshift);
> -       int tries = 1000;
> +       u32 lcr = p->serial_in(p, UART_LCR);
> +       int ret;
>
>         if (offset != UART_LCR || d->uart_16550_compatible)
>                 return;
>
>         /* Make sure LCR write wasn't ignored */
> -       while (tries--) {
> -               u32 lcr = serial_port_in(p, offset);
> -
> -               if ((value & ~UART_LCR_SPAR) == (lcr & ~UART_LCR_SPAR))
> -                       return;
> +       if ((value & ~UART_LCR_SPAR) == (lcr & ~UART_LCR_SPAR))
> +               return;
>
> -               dw8250_force_idle(p);
> +       if (d->in_idle) {
> +               /*
> +                * FIXME: this deadlocks if port->lock is already held
> +                * dev_err(p->dev, "Couldn't set LCR to %d\n", value);
> +                */
> +               return;
> +       }
>
> -#ifdef CONFIG_64BIT
> -               if (p->type == PORT_OCTEON)
> -                       __raw_writeq(value & 0xff, addr);
> -               else
> -#endif
> -               if (p->iotype == UPIO_MEM32)
> -                       writel(value, addr);
> -               else if (p->iotype == UPIO_MEM32BE)
> -                       iowrite32be(value, addr);
> -               else
> -                       writeb(value, addr);
> +       ret = dw8250_idle_enter(p);
> +       if (ret < 0) {
> +               /*
> +                * FIXME: this deadlocks if port->lock is already held
> +                * dev_err(p->dev, "Couldn't set LCR to %d\n", value);
> +                */
> +               goto idle_failed;
>         }
> -       /*
> -        * FIXME: this deadlocks if port->lock is already held
> -        * dev_err(p->dev, "Couldn't set LCR to %d\n", value);
> -        */
> +
> +       p->serial_out(p, UART_LCR, value);
> +
> +idle_failed:
> +       dw8250_idle_exit(p);
>  }
>
>  /* Returns once the transmitter is empty or we run out of retries */
> @@ -547,6 +617,7 @@ static int dw8250_probe(struct platform_device *pdev)
>         p->dev          = dev;
>         p->set_ldisc    = dw8250_set_ldisc;
>         p->set_termios  = dw8250_set_termios;
> +       p->set_divisor  = dw8250_set_divisor;
>
>         data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
>         if (!data)
>
> base-commit: d21b26cad33250be758ea9d860ff9d5c3992c459
> --
> 2.39.5

Re: [PATCH] serial: 8250 dw: clear FIFO before writting LCR

[Ilpo Järvinen]

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

On Fri, 31 Oct 2025, Adriana Nicolae wrote:

> On Wed, Oct 8, 2025 at 4:12 PM Ilpo Järvinen
> <ilpo.jarvinen@linux.intel.com> wrote:
> >
> > On Wed, 8 Oct 2025, Adriana Nicolae wrote:
> > > Hello, Sorry for missing an update, the exact root cause is not clear, currently the
> > > brute force method of draining FIFO right before setting or clearing DLAB was stable
> > > during tests.
> > >
> > > The serial stuck seems to be a failed attempt to clear the DLAB.
> > > This operation fails because the USR indicates the hardware is
> > > still busy, even though the UART is in loopback mode and should
> > > be inactive.
> > >
> > > To isolate the issue, I tried the following without success:
> > > - Added delays: I inserted 100 repeated ndelay(p->frame_time)
> > > calls before and after enabling loopback mode to allow the FIFO
> > > to clear.
> > > - Cleared FIFO: I explicitly cleared the FIFO in addition to
> > > adding the delay.
> > > - Checked status: I printed the LSR just before the DLAB clear
> > > attempt and checked the USB busy bit.
> >
> > Okay, so the BUSY must be stuck asserted.
> >
> > Another idea, maybe test tx + rx over loopback to see if that manages to
> > de-freeze the BUSY bit. A patch to that effect below.
> >
> > (I've only added the new block into dw8250_idle_enter() compared with the
> > old patch but rebasing to tty-next resulted in some other changes due to
> > conflicts.)
>
> I've tested the new dw8250_idle_enter() sequence, and you're right,
> the BUSY bit remains set after entering loopback mode.
> 
> However, the sequence in the patch (including the single loopback
> tx/rx) wasn't quite enough. I didn't see any kernel panics or console
> stuck anymore, but I've monitored the traces and there were cases when
> the trace after "p->serial_out(p, UART_LCR, up->lcr);" showed both
> BUSY bit set and DLAB bit still enabled.
> 
> >
> > Only thing besides BUSY being stuck asserted is something Tx'ing after the
> > idle enter sequence. I think we could trap/check that too in
> > dw8250_serial_out() by using something along these lines:
> >
> >         if (d->in_idle && offset == UART_TX) {
> >                 WARN_ON_ONCE(1);
> >                 return;
> >         }
> >
> > (I think that should catch even console writes but I'm not 100% sure of
> > that and it will should get us where the rogue Tx originates from).
> 
> I also added the WARN_ON_ONCE check you suggested in
> dw8250_serial_out(). The warning has not triggered, so it seems we
> don't have a rogue Tx firing while in_idle is set.
> 
> >
> > > The critical finding was that immediately before the DLAB clear
> > > operation (p->serial_out(p, UART_LCR, up->lcr);), the LSR value
> > > was 0x6a and the USR busy bit [0] was set. This confirms the UART
> > > is busy, which blocks the DLAB modification.
> > >
> > > This is occurring on a device with a single UART console. The setup
> > > does not use DMA or modem control; only the Rx/Tx lines are connected.
> > >
> > > The trace below, from a single process, shows one successful DLAB
> > > clear followed by a failing one. The second attempt repeatedly logs
> > > "USR still busy" before eventually succeeding. This can lead to
> > > subsequent failures in dw8250_check_lcr: dw8250_idle_entered.
> > >
> > > Trace Log:
> > >
> > > <...>-15440  8583.592533: dw8250_idle_enter: in_idle = 1
> > > login-15440  8583.713817: dw8250_idle_enter: in loopback mode
> > > login-15440  8583.835099: dw8250_idle_enter: LSR in loopback mode is 0x63
> > > login-15440  8583.835103: dw8250_set_divisor: UART_LCR_DLAB cleared 13
> > > login-15440  8583.835104: dw8250_idle_exit: idle exit
> > > login-15440  8583.835105: dw8250_idle_exit: out of loopback mode
> > > login-15440  8583.835105: dw8250_idle_exit: in_idle = 0
> > > login-15440  8583.835352: dw8250_idle_enter: in_idle = 1
> > > login-15440  8583.956633: dw8250_idle_enter: in loopback mode
> > > login-15440  8583.956638: dw8250_idle_enter: LSR in loopback mode is 0x6a
> > > login-15440  8583.963918: dw8250_set_divisor: USR still busy dl_write
> > > login-15440  8584.000332: dw8250_set_divisor: USR still busy dl_write
> > > login-15440  8584.040385: dw8250_set_divisor: USR still busy dl_write
> > > login-15440  8584.078012: dw8250_set_divisor: UART_LCR_DLAB cleared 93
> > > login-15440  8584.078013: dw8250_idle_exit: idle exit
> > > login-15440  8584.078014: dw8250_idle_exit: out of loopback mode
> > > login-15440  8584.078015: dw8250_idle_exit: in_idle = 0
> >
> >
> >
> > --
> > From 01df58736a10f7f34aca895ef08e5519953f8572 Mon Sep 17 00:00:00 2001
> > From: =?UTF-8?q?Ilpo=20J=C3=A4rvinen?= <ilpo.jarvinen@linux.intel.com>
> > Date: Wed, 8 Oct 2025 15:40:19 +0300
> > Subject: [PATCH 1/1] serial: 8250_dw: Ensure BUSY is deasserted
> > MIME-Version: 1.0
> > Content-Type: text/plain; charset=UTF-8
> > Content-Transfer-Encoding: 8bit
> >
> > DW UART cannot write to LCR, DLL, and DLH while BUSY is asserted.
> > Existance of BUSY depends on uart_16550_compatible, if UART HW is
> > configured with 16550 compatible those registers can always be
> > written.
> >
> > There currently is dw8250_force_idle() which attempts to archive
> > non-BUSY state by disabling FIFO, however, the solution is unreliable
> > when Rx keeps getting more and more characters.
> >
> > Create a sequence of operations to enforce that ensures UART cannot
> > keep BUSY asserted indefinitely. The new sequence relies on enabling
> > loopback mode temporarily to prevent incoming Rx characters keeping
> > UART BUSY.
> >
> > Use the new dw8250_idle_enter/exit() to do divisor writes and LCR
> > writes.
> >
> > This issue was reported by qianfan Zhao who put lots of debugging
> > effort into understanding the solution space.
> >
> > Reported-by: qianfan Zhao <qianfanguijin@163.com>
> > Reported-by: Adriana Nicolae <adriana@arista.com>
> > Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> > ---
> >  drivers/tty/serial/8250/8250_dw.c | 159 +++++++++++++++++++++---------
> >  1 file changed, 115 insertions(+), 44 deletions(-)
> >
> > diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c
> > index a53ba04d9770..8e25dfe8e653 100644
> > --- a/drivers/tty/serial/8250/8250_dw.c
> > +++ b/drivers/tty/serial/8250/8250_dw.c
> > @@ -42,6 +42,8 @@
> >  /* DesignWare specific register fields */
> >  #define DW_UART_MCR_SIRE               BIT(6)
> >
> > +#define DW_UART_USR_BUSY               BIT(0)
> > +
> >  /* Renesas specific register fields */
> >  #define RZN1_UART_xDMACR_DMA_EN                BIT(0)
> >  #define RZN1_UART_xDMACR_1_WORD_BURST  (0 << 1)
> > @@ -77,6 +79,7 @@ struct dw8250_data {
> >
> >         unsigned int            skip_autocfg:1;
> >         unsigned int            uart_16550_compatible:1;
> > +       unsigned int            in_idle:1;
> >  };
> >
> >  static inline struct dw8250_data *to_dw8250_data(struct dw8250_port_data *data)
> > @@ -108,36 +111,103 @@ static inline u32 dw8250_modify_msr(struct uart_port *p, unsigned int offset, u3
> >  }
> >
> >  /*
> > - * This function is being called as part of the uart_port::serial_out()
> > - * routine. Hence, it must not call serial_port_out() or serial_out()
> > - * against the modified registers here, i.e. LCR.
> > + * Ensure BUSY is not asserted. If DW UART is configured with
> > + * !uart_16550_compatible, the writes to LCR, DLL, and DLH fail while
> > + * BUSY is asserted.
> > + *
> > + * Context: port's lock must be held
> >   */
> > -static void dw8250_force_idle(struct uart_port *p)
> > +static int dw8250_idle_enter(struct uart_port *p)
> >  {
> > +       struct dw8250_data *d = to_dw8250_data(p->private_data);
> >         struct uart_8250_port *up = up_to_u8250p(p);
> > -       unsigned int lsr;
> > +       u32 lsr;
> >
> > -       /*
> > -        * The following call currently performs serial_out()
> > -        * against the FCR register. Because it differs to LCR
> > -        * there will be no infinite loop, but if it ever gets
> > -        * modified, we might need a new custom version of it
> > -        * that avoids infinite recursion.
> > -        */
> > -       serial8250_clear_and_reinit_fifos(up);
> > +       if (d->uart_16550_compatible)
> > +               return 0;
> > +
> > +       d->in_idle = 1;
> > +
> > +       /* Prevent triggering interrupt from RBR filling */
> > +       p->serial_out(p, UART_IER, 0);
> > +
> > +       serial8250_rx_dma_flush(up);
> > +       // What about Tx DMA? Should probably pause that too and resume
> > +       // afterwards.
> > +
> > +       p->serial_out(p, UART_MCR, up->mcr | UART_MCR_LOOP);
> > +       if (up->capabilities & UART_CAP_FIFO)
> > +               p->serial_out(p, UART_FCR, 0);
> 
> Changing this to repeatedly clear the FIFO turned out to reliably
> clear the BUSY bit , also no kernel panic or device stuck in busy
> mode.
> 
> On the device I tested the first clear is not always enough, under
> high load I saw it cleared on the second iteration. I'm thinking it
> might be some particular issue with the device I'm using where the
> first FIFO clear might fail. I never encountered more than 2
> iterations with a "ndelay(p->frame_time);" in between here to get out
> of BUSY state.

Hi,

I seem to have missed this email until now (I'm sorry about that, though 
to my defence, IIRC, I was quite sick around that timeframe it was sent 
and clear the email backlog isn't ever fun and may end up missing 
something).

Do you mean changing this to a simple loop or writing something else than 
just 0 to FCR (or perhaps calling serial8250_clear_fifos())?

What is the exact code that you found working?

So when you fixed this FIFO clearing thing, everything seemed to work okay 
after that?

In the meantime, this issue has once again been reported to me by somebody 
else, and I've done improvements to shutdown code as well to address a 
few BUSY related problems (I'll be posting a series that solved that 
case soon but I suppose this patch needs amendments based on input from
your case).

-- 
 i.

> > +       if (p->serial_in(p, d->pdata->usr_reg) & DW_UART_USR_BUSY)
> > +               ndelay(p->frame_time);
> > +
> > +       lsr = serial_lsr_in(up);
> > +       if (lsr & UART_LSR_DR) {
> > +               p->serial_in(p, UART_RX);
> > +               up->lsr_saved_flags = 0;
> > +       }
> >
> >         /*
> > -        * With PSLVERR_RESP_EN parameter set to 1, the device generates an
> > -        * error response when an attempt to read an empty RBR with FIFO
> > -        * enabled.
> > +        * BUSY might still be frozen to asserted, try to de-freeze it by
> > +        * sending a character over the loopback and receiving it.
> >          */
> > -       if (up->fcr & UART_FCR_ENABLE_FIFO) {
> > -               lsr = serial_port_in(p, UART_LSR);
> > -               if (!(lsr & UART_LSR_DR))
> > -                       return;
> > +       if (p->serial_in(p, d->pdata->usr_reg) & DW_UART_USR_BUSY) {
> > +               p->serial_out(p, UART_TX, 0);
> > +               ndelay(p->frame_time);
> > +               lsr = serial_lsr_in(up);
> > +               if (lsr & UART_LSR_DR) {
> > +                       p->serial_in(p, UART_RX);
> > +                       up->lsr_saved_flags = 0;
> > +               }
> >         }
> >
> > -       serial_port_in(p, UART_RX);
> > +       /* Now guaranteed to have BUSY deasserted? Just sanity check */
> > +       if (p->serial_in(p, d->pdata->usr_reg) & DW_UART_USR_BUSY)
> > +               return -EBUSY;
> > +
> > +       return 0;
> > +}
> > +
> > +static void dw8250_idle_exit(struct uart_port *p)
> > +{
> > +       struct dw8250_data *d = to_dw8250_data(p->private_data);
> > +       struct uart_8250_port *up = up_to_u8250p(p);
> > +
> > +       if (d->uart_16550_compatible)
> > +               return;
> > +
> > +       if (up->capabilities & UART_CAP_FIFO)
> > +               p->serial_out(p, UART_FCR, up->fcr);
> > +       p->serial_out(p, UART_MCR, up->mcr);
> > +       p->serial_out(p, UART_IER, up->ier);
> > +
> > +       // Maybe move the DMA Rx restart check in dma_rx_complete() to own
> > +       // function (serial8250_rx_dma_restart()) and call it from here.
> > +       // DMA Tx resume
> > +
> > +       d->in_idle = 0;
> > +}
> > +
> > +static void dw8250_set_divisor(struct uart_port *p, unsigned int baud,
> > +                              unsigned int quot, unsigned int quot_frac)
> > +{
> > +       struct uart_8250_port *up = up_to_u8250p(p);
> > +       int ret;
> > +
> > +       ret = dw8250_idle_enter(p);
> > +       if (ret < 0)
> > +               goto idle_failed;
> > +
> > +       p->serial_out(p, UART_LCR, up->lcr | UART_LCR_DLAB);
> > +       if (!(p->serial_in(p, UART_LCR) & UART_LCR_DLAB))
> > +               goto idle_failed;
> > +
> > +       serial_dl_write(up, quot);
> > +       p->serial_out(p, UART_LCR, up->lcr);
> > +
> > +idle_failed:
> > +       dw8250_idle_exit(p);
> >  }
> >
> >  /*
> > @@ -148,37 +218,37 @@ static void dw8250_force_idle(struct uart_port *p)
> >  static void dw8250_check_lcr(struct uart_port *p, unsigned int offset, u32 value)
> >  {
> >         struct dw8250_data *d = to_dw8250_data(p->private_data);
> > -       void __iomem *addr = p->membase + (offset << p->regshift);
> > -       int tries = 1000;
> > +       u32 lcr = p->serial_in(p, UART_LCR);
> > +       int ret;
> >
> >         if (offset != UART_LCR || d->uart_16550_compatible)
> >                 return;
> >
> >         /* Make sure LCR write wasn't ignored */
> > -       while (tries--) {
> > -               u32 lcr = serial_port_in(p, offset);
> > -
> > -               if ((value & ~UART_LCR_SPAR) == (lcr & ~UART_LCR_SPAR))
> > -                       return;
> > +       if ((value & ~UART_LCR_SPAR) == (lcr & ~UART_LCR_SPAR))
> > +               return;
> >
> > -               dw8250_force_idle(p);
> > +       if (d->in_idle) {
> > +               /*
> > +                * FIXME: this deadlocks if port->lock is already held
> > +                * dev_err(p->dev, "Couldn't set LCR to %d\n", value);
> > +                */
> > +               return;
> > +       }
> >
> > -#ifdef CONFIG_64BIT
> > -               if (p->type == PORT_OCTEON)
> > -                       __raw_writeq(value & 0xff, addr);
> > -               else
> > -#endif
> > -               if (p->iotype == UPIO_MEM32)
> > -                       writel(value, addr);
> > -               else if (p->iotype == UPIO_MEM32BE)
> > -                       iowrite32be(value, addr);
> > -               else
> > -                       writeb(value, addr);
> > +       ret = dw8250_idle_enter(p);
> > +       if (ret < 0) {
> > +               /*
> > +                * FIXME: this deadlocks if port->lock is already held
> > +                * dev_err(p->dev, "Couldn't set LCR to %d\n", value);
> > +                */
> > +               goto idle_failed;
> >         }
> > -       /*
> > -        * FIXME: this deadlocks if port->lock is already held
> > -        * dev_err(p->dev, "Couldn't set LCR to %d\n", value);
> > -        */
> > +
> > +       p->serial_out(p, UART_LCR, value);
> > +
> > +idle_failed:
> > +       dw8250_idle_exit(p);
> >  }
> >
> >  /* Returns once the transmitter is empty or we run out of retries */
> > @@ -547,6 +617,7 @@ static int dw8250_probe(struct platform_device *pdev)
> >         p->dev          = dev;
> >         p->set_ldisc    = dw8250_set_ldisc;
> >         p->set_termios  = dw8250_set_termios;
> > +       p->set_divisor  = dw8250_set_divisor;
> >
> >         data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
> >         if (!data)
> >
> > base-commit: d21b26cad33250be758ea9d860ff9d5c3992c459
> > --
> > 2.39.5
> 

Re: [PATCH] serial: 8250 dw: clear FIFO before writting LCR

[Adriana Nicolae]

On Wed, Jan 21, 2026 at 4:31 PM Ilpo Järvinen
<ilpo.jarvinen@linux.intel.com> wrote:
>
> On Fri, 31 Oct 2025, Adriana Nicolae wrote:
>
> > On Wed, Oct 8, 2025 at 4:12 PM Ilpo Järvinen
> > <ilpo.jarvinen@linux.intel.com> wrote:
> > >
> > > On Wed, 8 Oct 2025, Adriana Nicolae wrote:
> > > > Hello, Sorry for missing an update, the exact root cause is not clear, currently the
> > > > brute force method of draining FIFO right before setting or clearing DLAB was stable
> > > > during tests.
> > > >
> > > > The serial stuck seems to be a failed attempt to clear the DLAB.
> > > > This operation fails because the USR indicates the hardware is
> > > > still busy, even though the UART is in loopback mode and should
> > > > be inactive.
> > > >
> > > > To isolate the issue, I tried the following without success:
> > > > - Added delays: I inserted 100 repeated ndelay(p->frame_time)
> > > > calls before and after enabling loopback mode to allow the FIFO
> > > > to clear.
> > > > - Cleared FIFO: I explicitly cleared the FIFO in addition to
> > > > adding the delay.
> > > > - Checked status: I printed the LSR just before the DLAB clear
> > > > attempt and checked the USB busy bit.
> > >
> > > Okay, so the BUSY must be stuck asserted.
> > >
> > > Another idea, maybe test tx + rx over loopback to see if that manages to
> > > de-freeze the BUSY bit. A patch to that effect below.
> > >
> > > (I've only added the new block into dw8250_idle_enter() compared with the
> > > old patch but rebasing to tty-next resulted in some other changes due to
> > > conflicts.)
> >
> > I've tested the new dw8250_idle_enter() sequence, and you're right,
> > the BUSY bit remains set after entering loopback mode.
> >
> > However, the sequence in the patch (including the single loopback
> > tx/rx) wasn't quite enough. I didn't see any kernel panics or console
> > stuck anymore, but I've monitored the traces and there were cases when
> > the trace after "p->serial_out(p, UART_LCR, up->lcr);" showed both
> > BUSY bit set and DLAB bit still enabled.
> >
> > >
> > > Only thing besides BUSY being stuck asserted is something Tx'ing after the
> > > idle enter sequence. I think we could trap/check that too in
> > > dw8250_serial_out() by using something along these lines:
> > >
> > >         if (d->in_idle && offset == UART_TX) {
> > >                 WARN_ON_ONCE(1);
> > >                 return;
> > >         }
> > >
> > > (I think that should catch even console writes but I'm not 100% sure of
> > > that and it will should get us where the rogue Tx originates from).
> >
> > I also added the WARN_ON_ONCE check you suggested in
> > dw8250_serial_out(). The warning has not triggered, so it seems we
> > don't have a rogue Tx firing while in_idle is set.
> >
> > >
> > > > The critical finding was that immediately before the DLAB clear
> > > > operation (p->serial_out(p, UART_LCR, up->lcr);), the LSR value
> > > > was 0x6a and the USR busy bit [0] was set. This confirms the UART
> > > > is busy, which blocks the DLAB modification.
> > > >
> > > > This is occurring on a device with a single UART console. The setup
> > > > does not use DMA or modem control; only the Rx/Tx lines are connected.
> > > >
> > > > The trace below, from a single process, shows one successful DLAB
> > > > clear followed by a failing one. The second attempt repeatedly logs
> > > > "USR still busy" before eventually succeeding. This can lead to
> > > > subsequent failures in dw8250_check_lcr: dw8250_idle_entered.
> > > >
> > > > Trace Log:
> > > >
> > > > <...>-15440  8583.592533: dw8250_idle_enter: in_idle = 1
> > > > login-15440  8583.713817: dw8250_idle_enter: in loopback mode
> > > > login-15440  8583.835099: dw8250_idle_enter: LSR in loopback mode is 0x63
> > > > login-15440  8583.835103: dw8250_set_divisor: UART_LCR_DLAB cleared 13
> > > > login-15440  8583.835104: dw8250_idle_exit: idle exit
> > > > login-15440  8583.835105: dw8250_idle_exit: out of loopback mode
> > > > login-15440  8583.835105: dw8250_idle_exit: in_idle = 0
> > > > login-15440  8583.835352: dw8250_idle_enter: in_idle = 1
> > > > login-15440  8583.956633: dw8250_idle_enter: in loopback mode
> > > > login-15440  8583.956638: dw8250_idle_enter: LSR in loopback mode is 0x6a
> > > > login-15440  8583.963918: dw8250_set_divisor: USR still busy dl_write
> > > > login-15440  8584.000332: dw8250_set_divisor: USR still busy dl_write
> > > > login-15440  8584.040385: dw8250_set_divisor: USR still busy dl_write
> > > > login-15440  8584.078012: dw8250_set_divisor: UART_LCR_DLAB cleared 93
> > > > login-15440  8584.078013: dw8250_idle_exit: idle exit
> > > > login-15440  8584.078014: dw8250_idle_exit: out of loopback mode
> > > > login-15440  8584.078015: dw8250_idle_exit: in_idle = 0
> > >
> > >
> > >
> > > --
> > > From 01df58736a10f7f34aca895ef08e5519953f8572 Mon Sep 17 00:00:00 2001
> > > From: =?UTF-8?q?Ilpo=20J=C3=A4rvinen?= <ilpo.jarvinen@linux.intel.com>
> > > Date: Wed, 8 Oct 2025 15:40:19 +0300
> > > Subject: [PATCH 1/1] serial: 8250_dw: Ensure BUSY is deasserted
> > > MIME-Version: 1.0
> > > Content-Type: text/plain; charset=UTF-8
> > > Content-Transfer-Encoding: 8bit
> > >
> > > DW UART cannot write to LCR, DLL, and DLH while BUSY is asserted.
> > > Existance of BUSY depends on uart_16550_compatible, if UART HW is
> > > configured with 16550 compatible those registers can always be
> > > written.
> > >
> > > There currently is dw8250_force_idle() which attempts to archive
> > > non-BUSY state by disabling FIFO, however, the solution is unreliable
> > > when Rx keeps getting more and more characters.
> > >
> > > Create a sequence of operations to enforce that ensures UART cannot
> > > keep BUSY asserted indefinitely. The new sequence relies on enabling
> > > loopback mode temporarily to prevent incoming Rx characters keeping
> > > UART BUSY.
> > >
> > > Use the new dw8250_idle_enter/exit() to do divisor writes and LCR
> > > writes.
> > >
> > > This issue was reported by qianfan Zhao who put lots of debugging
> > > effort into understanding the solution space.
> > >
> > > Reported-by: qianfan Zhao <qianfanguijin@163.com>
> > > Reported-by: Adriana Nicolae <adriana@arista.com>
> > > Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> > > ---
> > >  drivers/tty/serial/8250/8250_dw.c | 159 +++++++++++++++++++++---------
> > >  1 file changed, 115 insertions(+), 44 deletions(-)
> > >
> > > diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c
> > > index a53ba04d9770..8e25dfe8e653 100644
> > > --- a/drivers/tty/serial/8250/8250_dw.c
> > > +++ b/drivers/tty/serial/8250/8250_dw.c
> > > @@ -42,6 +42,8 @@
> > >  /* DesignWare specific register fields */
> > >  #define DW_UART_MCR_SIRE               BIT(6)
> > >
> > > +#define DW_UART_USR_BUSY               BIT(0)
> > > +
> > >  /* Renesas specific register fields */
> > >  #define RZN1_UART_xDMACR_DMA_EN                BIT(0)
> > >  #define RZN1_UART_xDMACR_1_WORD_BURST  (0 << 1)
> > > @@ -77,6 +79,7 @@ struct dw8250_data {
> > >
> > >         unsigned int            skip_autocfg:1;
> > >         unsigned int            uart_16550_compatible:1;
> > > +       unsigned int            in_idle:1;
> > >  };
> > >
> > >  static inline struct dw8250_data *to_dw8250_data(struct dw8250_port_data *data)
> > > @@ -108,36 +111,103 @@ static inline u32 dw8250_modify_msr(struct uart_port *p, unsigned int offset, u3
> > >  }
> > >
> > >  /*
> > > - * This function is being called as part of the uart_port::serial_out()
> > > - * routine. Hence, it must not call serial_port_out() or serial_out()
> > > - * against the modified registers here, i.e. LCR.
> > > + * Ensure BUSY is not asserted. If DW UART is configured with
> > > + * !uart_16550_compatible, the writes to LCR, DLL, and DLH fail while
> > > + * BUSY is asserted.
> > > + *
> > > + * Context: port's lock must be held
> > >   */
> > > -static void dw8250_force_idle(struct uart_port *p)
> > > +static int dw8250_idle_enter(struct uart_port *p)
> > >  {
> > > +       struct dw8250_data *d = to_dw8250_data(p->private_data);
> > >         struct uart_8250_port *up = up_to_u8250p(p);
> > > -       unsigned int lsr;
> > > +       u32 lsr;
> > >
> > > -       /*
> > > -        * The following call currently performs serial_out()
> > > -        * against the FCR register. Because it differs to LCR
> > > -        * there will be no infinite loop, but if it ever gets
> > > -        * modified, we might need a new custom version of it
> > > -        * that avoids infinite recursion.
> > > -        */
> > > -       serial8250_clear_and_reinit_fifos(up);
> > > +       if (d->uart_16550_compatible)
> > > +               return 0;
> > > +
> > > +       d->in_idle = 1;
> > > +
> > > +       /* Prevent triggering interrupt from RBR filling */
> > > +       p->serial_out(p, UART_IER, 0);
> > > +
> > > +       serial8250_rx_dma_flush(up);
> > > +       // What about Tx DMA? Should probably pause that too and resume
> > > +       // afterwards.
> > > +
> > > +       p->serial_out(p, UART_MCR, up->mcr | UART_MCR_LOOP);
> > > +       if (up->capabilities & UART_CAP_FIFO)
> > > +               p->serial_out(p, UART_FCR, 0);
> >
> > Changing this to repeatedly clear the FIFO turned out to reliably
> > clear the BUSY bit , also no kernel panic or device stuck in busy
> > mode.
> >
> > On the device I tested the first clear is not always enough, under
> > high load I saw it cleared on the second iteration. I'm thinking it
> > might be some particular issue with the device I'm using where the
> > first FIFO clear might fail. I never encountered more than 2
> > iterations with a "ndelay(p->frame_time);" in between here to get out
> > of BUSY state.
>
> Hi,
>
> I seem to have missed this email until now (I'm sorry about that, though
> to my defence, IIRC, I was quite sick around that timeframe it was sent
> and clear the email backlog isn't ever fun and may end up missing
> something).
>
> Do you mean changing this to a simple loop or writing something else than
> just 0 to FCR (or perhaps calling serial8250_clear_fifos())?
>
> What is the exact code that you found working?
>
Yes, everything worked ok for me after changing the dw8250_idle_enter
function with the one below. From traces added in the function, it
sometimes reported iterations_in_busy = 2 but never higher than that.
The function only has the prepended "while(p->serial_in(p,
d->pdata->usr_reg) & DW_UART_USR_BUSY) {" to iterate forever, although
it was at most 2 iterations when serial was stressed:
static int dw8250_idle_enter(struct uart_port *p)
{
    struct dw8250_data *d = to_dw8250_data(p->private_data);
    struct uart_8250_port *up = up_to_u8250p(p);
    u32 lsr, iterations_in_busy = 0;

    if (d->uart_16550_compatible)
        return 0;

    d->in_idle = 1;

    /* Prevent triggering interrupt from RBR filling */
    p->serial_out(p, UART_IER, 0);

    serial8250_rx_dma_flush(up);
    // What about Tx DMA? Should probably pause that too and resume
    // afterwards.
    p->serial_out(p, UART_MCR, up->mcr | UART_MCR_LOOP);

    while(p->serial_in(p, d->pdata->usr_reg) & DW_UART_USR_BUSY) {
        if (up->capabilities & UART_CAP_FIFO) {
            p->serial_out(p, UART_FCR, UART_FCR_ENABLE_FIFO);
            p->serial_out(p, UART_FCR, UART_FCR_ENABLE_FIFO |
                UART_FCR_CLEAR_RCVR | UART_FCR_CLEAR_XMIT);
            p->serial_out(p, UART_FCR, 0);
        }
        ndelay(p->frame_time);
        iterations_in_busy++;
    }

    trace_printk("Not busy got after %d\n", iterations_in_busy);
    lsr = serial_lsr_in(up);
    if (lsr & UART_LSR_DR) {
        p->serial_in(p, UART_RX);
        up->lsr_saved_flags = 0;
    }

        /*
     * BUSY might still be frozen to asserted, try to de-freeze it by
     * sending a character over the loopback and receiving it.
         */
    if (p->serial_in(p, d->pdata->usr_reg) & DW_UART_USR_BUSY) {
        trace_printk("Serial USR still busy\n");
        p->serial_out(p, UART_TX, 0);
        ndelay(1000);
        lsr = serial_lsr_in(up);

        if (lsr & UART_LSR_DR) {
            p->serial_in(p, UART_RX);
            up->lsr_saved_flags = 0;
        }
        }

     /* Now guaranteed to have BUSY deasserted? Just sanity check */
    if (p->serial_in(p, d->pdata->usr_reg) & DW_UART_USR_BUSY) {
        trace_printk("BUSY\n");
        return -EBUSY;
    }

    return 0;
}
> So when you fixed this FIFO clearing thing, everything seemed to work okay
> after that?
>
> In the meantime, this issue has once again been reported to me by somebody
> else, and I've done improvements to shutdown code as well to address a
> few BUSY related problems (I'll be posting a series that solved that
> case soon but I suppose this patch needs amendments based on input from
> your case).
>
> --
>  i.
>
> > > +       if (p->serial_in(p, d->pdata->usr_reg) & DW_UART_USR_BUSY)
> > > +               ndelay(p->frame_time);
> > > +
> > > +       lsr = serial_lsr_in(up);
> > > +       if (lsr & UART_LSR_DR) {
> > > +               p->serial_in(p, UART_RX);
> > > +               up->lsr_saved_flags = 0;
> > > +       }
> > >
> > >         /*
> > > -        * With PSLVERR_RESP_EN parameter set to 1, the device generates an
> > > -        * error response when an attempt to read an empty RBR with FIFO
> > > -        * enabled.
> > > +        * BUSY might still be frozen to asserted, try to de-freeze it by
> > > +        * sending a character over the loopback and receiving it.
> > >          */
> > > -       if (up->fcr & UART_FCR_ENABLE_FIFO) {
> > > -               lsr = serial_port_in(p, UART_LSR);
> > > -               if (!(lsr & UART_LSR_DR))
> > > -                       return;
> > > +       if (p->serial_in(p, d->pdata->usr_reg) & DW_UART_USR_BUSY) {
> > > +               p->serial_out(p, UART_TX, 0);
> > > +               ndelay(p->frame_time);
> > > +               lsr = serial_lsr_in(up);
> > > +               if (lsr & UART_LSR_DR) {
> > > +                       p->serial_in(p, UART_RX);
> > > +                       up->lsr_saved_flags = 0;
> > > +               }
> > >         }
> > >
> > > -       serial_port_in(p, UART_RX);
> > > +       /* Now guaranteed to have BUSY deasserted? Just sanity check */
> > > +       if (p->serial_in(p, d->pdata->usr_reg) & DW_UART_USR_BUSY)
> > > +               return -EBUSY;
> > > +
> > > +       return 0;
> > > +}
> > > +
> > > +static void dw8250_idle_exit(struct uart_port *p)
> > > +{
> > > +       struct dw8250_data *d = to_dw8250_data(p->private_data);
> > > +       struct uart_8250_port *up = up_to_u8250p(p);
> > > +
> > > +       if (d->uart_16550_compatible)
> > > +               return;
> > > +
> > > +       if (up->capabilities & UART_CAP_FIFO)
> > > +               p->serial_out(p, UART_FCR, up->fcr);
> > > +       p->serial_out(p, UART_MCR, up->mcr);
> > > +       p->serial_out(p, UART_IER, up->ier);
> > > +
> > > +       // Maybe move the DMA Rx restart check in dma_rx_complete() to own
> > > +       // function (serial8250_rx_dma_restart()) and call it from here.
> > > +       // DMA Tx resume
> > > +
> > > +       d->in_idle = 0;
> > > +}
> > > +
> > > +static void dw8250_set_divisor(struct uart_port *p, unsigned int baud,
> > > +                              unsigned int quot, unsigned int quot_frac)
> > > +{
> > > +       struct uart_8250_port *up = up_to_u8250p(p);
> > > +       int ret;
> > > +
> > > +       ret = dw8250_idle_enter(p);
> > > +       if (ret < 0)
> > > +               goto idle_failed;
> > > +
> > > +       p->serial_out(p, UART_LCR, up->lcr | UART_LCR_DLAB);
> > > +       if (!(p->serial_in(p, UART_LCR) & UART_LCR_DLAB))
> > > +               goto idle_failed;
> > > +
> > > +       serial_dl_write(up, quot);
> > > +       p->serial_out(p, UART_LCR, up->lcr);
> > > +
> > > +idle_failed:
> > > +       dw8250_idle_exit(p);
> > >  }
> > >
> > >  /*
> > > @@ -148,37 +218,37 @@ static void dw8250_force_idle(struct uart_port *p)
> > >  static void dw8250_check_lcr(struct uart_port *p, unsigned int offset, u32 value)
> > >  {
> > >         struct dw8250_data *d = to_dw8250_data(p->private_data);
> > > -       void __iomem *addr = p->membase + (offset << p->regshift);
> > > -       int tries = 1000;
> > > +       u32 lcr = p->serial_in(p, UART_LCR);
> > > +       int ret;
> > >
> > >         if (offset != UART_LCR || d->uart_16550_compatible)
> > >                 return;
> > >
> > >         /* Make sure LCR write wasn't ignored */
> > > -       while (tries--) {
> > > -               u32 lcr = serial_port_in(p, offset);
> > > -
> > > -               if ((value & ~UART_LCR_SPAR) == (lcr & ~UART_LCR_SPAR))
> > > -                       return;
> > > +       if ((value & ~UART_LCR_SPAR) == (lcr & ~UART_LCR_SPAR))
> > > +               return;
> > >
> > > -               dw8250_force_idle(p);
> > > +       if (d->in_idle) {
> > > +               /*
> > > +                * FIXME: this deadlocks if port->lock is already held
> > > +                * dev_err(p->dev, "Couldn't set LCR to %d\n", value);
> > > +                */
> > > +               return;
> > > +       }
> > >
> > > -#ifdef CONFIG_64BIT
> > > -               if (p->type == PORT_OCTEON)
> > > -                       __raw_writeq(value & 0xff, addr);
> > > -               else
> > > -#endif
> > > -               if (p->iotype == UPIO_MEM32)
> > > -                       writel(value, addr);
> > > -               else if (p->iotype == UPIO_MEM32BE)
> > > -                       iowrite32be(value, addr);
> > > -               else
> > > -                       writeb(value, addr);
> > > +       ret = dw8250_idle_enter(p);
> > > +       if (ret < 0) {
> > > +               /*
> > > +                * FIXME: this deadlocks if port->lock is already held
> > > +                * dev_err(p->dev, "Couldn't set LCR to %d\n", value);
> > > +                */
> > > +               goto idle_failed;
> > >         }
> > > -       /*
> > > -        * FIXME: this deadlocks if port->lock is already held
> > > -        * dev_err(p->dev, "Couldn't set LCR to %d\n", value);
> > > -        */
> > > +
> > > +       p->serial_out(p, UART_LCR, value);
> > > +
> > > +idle_failed:
> > > +       dw8250_idle_exit(p);
> > >  }
> > >
> > >  /* Returns once the transmitter is empty or we run out of retries */
> > > @@ -547,6 +617,7 @@ static int dw8250_probe(struct platform_device *pdev)
> > >         p->dev          = dev;
> > >         p->set_ldisc    = dw8250_set_ldisc;
> > >         p->set_termios  = dw8250_set_termios;
> > > +       p->set_divisor  = dw8250_set_divisor;
> > >
> > >         data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
> > >         if (!data)
> > >
> > > base-commit: d21b26cad33250be758ea9d860ff9d5c3992c459
> > > --
> > > 2.39.5
> >

Re: [PATCH] serial: 8250 dw: clear FIFO before writting LCR

[Ilpo Järvinen]

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

On Wed, 21 Jan 2026, Adriana Nicolae wrote:
> On Wed, Jan 21, 2026 at 4:31 PM Ilpo Järvinen
> <ilpo.jarvinen@linux.intel.com> wrote:
> > On Fri, 31 Oct 2025, Adriana Nicolae wrote:
> > > On Wed, Oct 8, 2025 at 4:12 PM Ilpo Järvinen
> > > <ilpo.jarvinen@linux.intel.com> wrote:
> > > > On Wed, 8 Oct 2025, Adriana Nicolae wrote:
> > > > > Hello, Sorry for missing an update, the exact root cause is not clear, currently the
> > > > > brute force method of draining FIFO right before setting or clearing DLAB was stable
> > > > > during tests.
> > > > >
> > > > > The serial stuck seems to be a failed attempt to clear the DLAB.
> > > > > This operation fails because the USR indicates the hardware is
> > > > > still busy, even though the UART is in loopback mode and should
> > > > > be inactive.
> > > > >
> > > > > To isolate the issue, I tried the following without success:
> > > > > - Added delays: I inserted 100 repeated ndelay(p->frame_time)
> > > > > calls before and after enabling loopback mode to allow the FIFO
> > > > > to clear.
> > > > > - Cleared FIFO: I explicitly cleared the FIFO in addition to
> > > > > adding the delay.
> > > > > - Checked status: I printed the LSR just before the DLAB clear
> > > > > attempt and checked the USB busy bit.
> > > >
> > > > Okay, so the BUSY must be stuck asserted.
> > > >
> > > > Another idea, maybe test tx + rx over loopback to see if that manages to
> > > > de-freeze the BUSY bit. A patch to that effect below.
> > > >
> > > > (I've only added the new block into dw8250_idle_enter() compared with the
> > > > old patch but rebasing to tty-next resulted in some other changes due to
> > > > conflicts.)
> > >
> > > I've tested the new dw8250_idle_enter() sequence, and you're right,
> > > the BUSY bit remains set after entering loopback mode.
> > >
> > > However, the sequence in the patch (including the single loopback
> > > tx/rx) wasn't quite enough. I didn't see any kernel panics or console
> > > stuck anymore, but I've monitored the traces and there were cases when
> > > the trace after "p->serial_out(p, UART_LCR, up->lcr);" showed both
> > > BUSY bit set and DLAB bit still enabled.
> > >
> > > >
> > > > Only thing besides BUSY being stuck asserted is something Tx'ing after the
> > > > idle enter sequence. I think we could trap/check that too in
> > > > dw8250_serial_out() by using something along these lines:
> > > >
> > > >         if (d->in_idle && offset == UART_TX) {
> > > >                 WARN_ON_ONCE(1);
> > > >                 return;
> > > >         }
> > > >
> > > > (I think that should catch even console writes but I'm not 100% sure of
> > > > that and it will should get us where the rogue Tx originates from).
> > >
> > > I also added the WARN_ON_ONCE check you suggested in
> > > dw8250_serial_out(). The warning has not triggered, so it seems we
> > > don't have a rogue Tx firing while in_idle is set.
> > >
> > > >
> > > > > The critical finding was that immediately before the DLAB clear
> > > > > operation (p->serial_out(p, UART_LCR, up->lcr);), the LSR value
> > > > > was 0x6a and the USR busy bit [0] was set. This confirms the UART
> > > > > is busy, which blocks the DLAB modification.
> > > > >
> > > > > This is occurring on a device with a single UART console. The setup
> > > > > does not use DMA or modem control; only the Rx/Tx lines are connected.
> > > > >
> > > > > The trace below, from a single process, shows one successful DLAB
> > > > > clear followed by a failing one. The second attempt repeatedly logs
> > > > > "USR still busy" before eventually succeeding. This can lead to
> > > > > subsequent failures in dw8250_check_lcr: dw8250_idle_entered.
> > > > >
> > > > > Trace Log:
> > > > >
> > > > > <...>-15440  8583.592533: dw8250_idle_enter: in_idle = 1
> > > > > login-15440  8583.713817: dw8250_idle_enter: in loopback mode
> > > > > login-15440  8583.835099: dw8250_idle_enter: LSR in loopback mode is 0x63
> > > > > login-15440  8583.835103: dw8250_set_divisor: UART_LCR_DLAB cleared 13
> > > > > login-15440  8583.835104: dw8250_idle_exit: idle exit
> > > > > login-15440  8583.835105: dw8250_idle_exit: out of loopback mode
> > > > > login-15440  8583.835105: dw8250_idle_exit: in_idle = 0
> > > > > login-15440  8583.835352: dw8250_idle_enter: in_idle = 1
> > > > > login-15440  8583.956633: dw8250_idle_enter: in loopback mode
> > > > > login-15440  8583.956638: dw8250_idle_enter: LSR in loopback mode is 0x6a
> > > > > login-15440  8583.963918: dw8250_set_divisor: USR still busy dl_write
> > > > > login-15440  8584.000332: dw8250_set_divisor: USR still busy dl_write
> > > > > login-15440  8584.040385: dw8250_set_divisor: USR still busy dl_write
> > > > > login-15440  8584.078012: dw8250_set_divisor: UART_LCR_DLAB cleared 93
> > > > > login-15440  8584.078013: dw8250_idle_exit: idle exit
> > > > > login-15440  8584.078014: dw8250_idle_exit: out of loopback mode
> > > > > login-15440  8584.078015: dw8250_idle_exit: in_idle = 0
> > > >
> > > >
> > > >
> > > > --
> > > > From 01df58736a10f7f34aca895ef08e5519953f8572 Mon Sep 17 00:00:00 2001
> > > > From: =?UTF-8?q?Ilpo=20J=C3=A4rvinen?= <ilpo.jarvinen@linux.intel.com>
> > > > Date: Wed, 8 Oct 2025 15:40:19 +0300
> > > > Subject: [PATCH 1/1] serial: 8250_dw: Ensure BUSY is deasserted
> > > > MIME-Version: 1.0
> > > > Content-Type: text/plain; charset=UTF-8
> > > > Content-Transfer-Encoding: 8bit
> > > >
> > > > DW UART cannot write to LCR, DLL, and DLH while BUSY is asserted.
> > > > Existance of BUSY depends on uart_16550_compatible, if UART HW is
> > > > configured with 16550 compatible those registers can always be
> > > > written.
> > > >
> > > > There currently is dw8250_force_idle() which attempts to archive
> > > > non-BUSY state by disabling FIFO, however, the solution is unreliable
> > > > when Rx keeps getting more and more characters.
> > > >
> > > > Create a sequence of operations to enforce that ensures UART cannot
> > > > keep BUSY asserted indefinitely. The new sequence relies on enabling
> > > > loopback mode temporarily to prevent incoming Rx characters keeping
> > > > UART BUSY.
> > > >
> > > > Use the new dw8250_idle_enter/exit() to do divisor writes and LCR
> > > > writes.
> > > >
> > > > This issue was reported by qianfan Zhao who put lots of debugging
> > > > effort into understanding the solution space.
> > > >
> > > > Reported-by: qianfan Zhao <qianfanguijin@163.com>
> > > > Reported-by: Adriana Nicolae <adriana@arista.com>
> > > > Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> > > > ---
> > > >  drivers/tty/serial/8250/8250_dw.c | 159 +++++++++++++++++++++---------
> > > >  1 file changed, 115 insertions(+), 44 deletions(-)
> > > >
> > > > diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c
> > > > index a53ba04d9770..8e25dfe8e653 100644
> > > > --- a/drivers/tty/serial/8250/8250_dw.c
> > > > +++ b/drivers/tty/serial/8250/8250_dw.c
> > > > @@ -42,6 +42,8 @@
> > > >  /* DesignWare specific register fields */
> > > >  #define DW_UART_MCR_SIRE               BIT(6)
> > > >
> > > > +#define DW_UART_USR_BUSY               BIT(0)
> > > > +
> > > >  /* Renesas specific register fields */
> > > >  #define RZN1_UART_xDMACR_DMA_EN                BIT(0)
> > > >  #define RZN1_UART_xDMACR_1_WORD_BURST  (0 << 1)
> > > > @@ -77,6 +79,7 @@ struct dw8250_data {
> > > >
> > > >         unsigned int            skip_autocfg:1;
> > > >         unsigned int            uart_16550_compatible:1;
> > > > +       unsigned int            in_idle:1;
> > > >  };
> > > >
> > > >  static inline struct dw8250_data *to_dw8250_data(struct dw8250_port_data *data)
> > > > @@ -108,36 +111,103 @@ static inline u32 dw8250_modify_msr(struct uart_port *p, unsigned int offset, u3
> > > >  }
> > > >
> > > >  /*
> > > > - * This function is being called as part of the uart_port::serial_out()
> > > > - * routine. Hence, it must not call serial_port_out() or serial_out()
> > > > - * against the modified registers here, i.e. LCR.
> > > > + * Ensure BUSY is not asserted. If DW UART is configured with
> > > > + * !uart_16550_compatible, the writes to LCR, DLL, and DLH fail while
> > > > + * BUSY is asserted.
> > > > + *
> > > > + * Context: port's lock must be held
> > > >   */
> > > > -static void dw8250_force_idle(struct uart_port *p)
> > > > +static int dw8250_idle_enter(struct uart_port *p)
> > > >  {
> > > > +       struct dw8250_data *d = to_dw8250_data(p->private_data);
> > > >         struct uart_8250_port *up = up_to_u8250p(p);
> > > > -       unsigned int lsr;
> > > > +       u32 lsr;
> > > >
> > > > -       /*
> > > > -        * The following call currently performs serial_out()
> > > > -        * against the FCR register. Because it differs to LCR
> > > > -        * there will be no infinite loop, but if it ever gets
> > > > -        * modified, we might need a new custom version of it
> > > > -        * that avoids infinite recursion.
> > > > -        */
> > > > -       serial8250_clear_and_reinit_fifos(up);
> > > > +       if (d->uart_16550_compatible)
> > > > +               return 0;
> > > > +
> > > > +       d->in_idle = 1;
> > > > +
> > > > +       /* Prevent triggering interrupt from RBR filling */
> > > > +       p->serial_out(p, UART_IER, 0);
> > > > +
> > > > +       serial8250_rx_dma_flush(up);
> > > > +       // What about Tx DMA? Should probably pause that too and resume
> > > > +       // afterwards.
> > > > +
> > > > +       p->serial_out(p, UART_MCR, up->mcr | UART_MCR_LOOP);
> > > > +       if (up->capabilities & UART_CAP_FIFO)
> > > > +               p->serial_out(p, UART_FCR, 0);
> > >
> > > Changing this to repeatedly clear the FIFO turned out to reliably
> > > clear the BUSY bit , also no kernel panic or device stuck in busy
> > > mode.
> > >
> > > On the device I tested the first clear is not always enough, under
> > > high load I saw it cleared on the second iteration. I'm thinking it
> > > might be some particular issue with the device I'm using where the
> > > first FIFO clear might fail. I never encountered more than 2
> > > iterations with a "ndelay(p->frame_time);" in between here to get out
> > > of BUSY state.
> >
> > Hi,
> >
> > I seem to have missed this email until now (I'm sorry about that, though
> > to my defence, IIRC, I was quite sick around that timeframe it was sent
> > and clear the email backlog isn't ever fun and may end up missing
> > something).
> >
> > Do you mean changing this to a simple loop or writing something else than
> > just 0 to FCR (or perhaps calling serial8250_clear_fifos())?
> >
> > What is the exact code that you found working?
> >
> Yes, everything worked ok for me after changing the dw8250_idle_enter
> function with the one below. From traces added in the function, it
> sometimes reported iterations_in_busy = 2 but never higher than that.
> The function only has the prepended "while(p->serial_in(p,
> d->pdata->usr_reg) & DW_UART_USR_BUSY) {" to iterate forever, although
> it was at most 2 iterations when serial was stressed:
> static int dw8250_idle_enter(struct uart_port *p)
> {
>     struct dw8250_data *d = to_dw8250_data(p->private_data);
>     struct uart_8250_port *up = up_to_u8250p(p);
>     u32 lsr, iterations_in_busy = 0;
> 
>     if (d->uart_16550_compatible)
>         return 0;
> 
>     d->in_idle = 1;
> 
>     /* Prevent triggering interrupt from RBR filling */
>     p->serial_out(p, UART_IER, 0);
> 
>     serial8250_rx_dma_flush(up);
>     // What about Tx DMA? Should probably pause that too and resume
>     // afterwards.
>     p->serial_out(p, UART_MCR, up->mcr | UART_MCR_LOOP);
> 
>     while(p->serial_in(p, d->pdata->usr_reg) & DW_UART_USR_BUSY) {
>         if (up->capabilities & UART_CAP_FIFO) {
>             p->serial_out(p, UART_FCR, UART_FCR_ENABLE_FIFO);
>             p->serial_out(p, UART_FCR, UART_FCR_ENABLE_FIFO |
>                 UART_FCR_CLEAR_RCVR | UART_FCR_CLEAR_XMIT);
>             p->serial_out(p, UART_FCR, 0);

Thanks for the information!

Okay, so this is same as calling serial8250_clear_fifos() which is what 
I'll do here.

>         }
>         ndelay(p->frame_time);
>         iterations_in_busy++;
>     }
> 
>     trace_printk("Not busy got after %d\n", iterations_in_busy);
>     lsr = serial_lsr_in(up);
>     if (lsr & UART_LSR_DR) {
>         p->serial_in(p, UART_RX);
>         up->lsr_saved_flags = 0;
>     }
> 
>         /*
>      * BUSY might still be frozen to asserted, try to de-freeze it by
>      * sending a character over the loopback and receiving it.
>          */
>     if (p->serial_in(p, d->pdata->usr_reg) & DW_UART_USR_BUSY) {
>         trace_printk("Serial USR still busy\n");
>         p->serial_out(p, UART_TX, 0);
>         ndelay(1000);
>         lsr = serial_lsr_in(up);
> 
>         if (lsr & UART_LSR_DR) {
>             p->serial_in(p, UART_RX);
>             up->lsr_saved_flags = 0;
>         }
>         }

So I think this entire tx+rx de-freezing wasn't required at all? Adding 
this was based on my guess how the driver could try to force BUSY 
deassertion but if the FIFOs were the real culprit to the BUSY remaining 
asserted, I'd prefer to remove this block entirely to not add random 
complexity just for the sake of doing everything imaginable.

>      /* Now guaranteed to have BUSY deasserted? Just sanity check */
>     if (p->serial_in(p, d->pdata->usr_reg) & DW_UART_USR_BUSY) {
>         trace_printk("BUSY\n");
>         return -EBUSY;
>     }
> 
>     return 0;
> }
> > So when you fixed this FIFO clearing thing, everything seemed to work okay
> > after that?
> >
> > In the meantime, this issue has once again been reported to me by somebody
> > else, and I've done improvements to shutdown code as well to address a
> > few BUSY related problems (I'll be posting a series that solved that
> > case soon but I suppose this patch needs amendments based on input from
> > your case).
> >
> > --
> >  i.
> >
> > > > +       if (p->serial_in(p, d->pdata->usr_reg) & DW_UART_USR_BUSY)
> > > > +               ndelay(p->frame_time);
> > > > +
> > > > +       lsr = serial_lsr_in(up);
> > > > +       if (lsr & UART_LSR_DR) {
> > > > +               p->serial_in(p, UART_RX);
> > > > +               up->lsr_saved_flags = 0;
> > > > +       }
> > > >
> > > >         /*
> > > > -        * With PSLVERR_RESP_EN parameter set to 1, the device generates an
> > > > -        * error response when an attempt to read an empty RBR with FIFO
> > > > -        * enabled.
> > > > +        * BUSY might still be frozen to asserted, try to de-freeze it by
> > > > +        * sending a character over the loopback and receiving it.
> > > >          */
> > > > -       if (up->fcr & UART_FCR_ENABLE_FIFO) {
> > > > -               lsr = serial_port_in(p, UART_LSR);
> > > > -               if (!(lsr & UART_LSR_DR))
> > > > -                       return;
> > > > +       if (p->serial_in(p, d->pdata->usr_reg) & DW_UART_USR_BUSY) {
> > > > +               p->serial_out(p, UART_TX, 0);
> > > > +               ndelay(p->frame_time);
> > > > +               lsr = serial_lsr_in(up);
> > > > +               if (lsr & UART_LSR_DR) {
> > > > +                       p->serial_in(p, UART_RX);
> > > > +                       up->lsr_saved_flags = 0;
> > > > +               }
> > > >         }
> > > >
> > > > -       serial_port_in(p, UART_RX);
> > > > +       /* Now guaranteed to have BUSY deasserted? Just sanity check */
> > > > +       if (p->serial_in(p, d->pdata->usr_reg) & DW_UART_USR_BUSY)
> > > > +               return -EBUSY;
> > > > +
> > > > +       return 0;
> > > > +}


-- 
 i.