On Fri, 22 Aug 2025, Adriana Nicolae wrote: > On Wed, Aug 20, 2025 at 1:02 PM Ilpo Järvinen > 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 > > > --- > > > 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.