* Re: printk's threaded legacy console + fbcon => schedule where it should not
From: Sebastian Andrzej Siewior @ 2026-01-21 13:57 UTC (permalink / raw)
To: Petr Mladek
Cc: Steven Rostedt, linux-kernel, linux-serial, linux-fbdev,
John Ogness, Sergey Senozhatsky, Greg Kroah-Hartman, Jiri Slaby,
Simona Vetter, Helge Deller
In-Reply-To: <aXDYEVlkFgxdSVSG@pathway.suse.cz>
On 2026-01-21 14:43:45 [+0100], Petr Mladek wrote:
> I know that there was a plan to get rid of cond_resched().
> But what is the status now, please?
It is slowly moving => https://lore.kernel.org/all/20251219101502.GB1132199@noisy.programming.kicks-ass.net/
> I still see more that 1k cond_resched() calls in the code:
>
> $> git grep cond_resched\(\) | grep "\.c:" | wc -l
> 1263
>
> And config PREEMPT_VOLUNTARY still talks about the explicit
> preemption points.
>
> > Should we just remove it and see what breaks?
>
> Honestly, I do not feel comfortable with removing it. It is true that
> it has no effect in the printk() code path. But the vt code is used
> also when working on the terminal.
>
> The vt code still uses console_lock() because it was intertwined
> with printk console code since very old days. console_lock is a kind
> of big kernel lock there.
Do you a have path which loops and would mandate it? I found a few which
do not matter and have their own cond_resched() around. So I don't see a
reason to keep it. And I found one which breaks things so a removal
makes sense.
> Alternative solution is to get rid of the spin_trylock(). The only
> purpose is to prevent race in console_flush_on_panic(). It used
> to be a simple bit operation. The spin_lock() was added just to
> get barriers right. But we have a great atomic_t API these days.
>
> IMHO, it is a win-win solution because a preemptive context is
> always better.
So why do we keep warts again? Just because it *might* be required?
Keeping things preemptible makes sense but this is locking with no
annotation what so ever.
Again. printk has its cond_resched, the tty has it, too.
I'm with Steven on the removal side.
Sebastian
^ permalink raw reply
* Re: [PATCH] serial: 8250 dw: clear FIFO before writting LCR
From: Ilpo Järvinen @ 2026-01-21 14:31 UTC (permalink / raw)
To: Adriana Nicolae; +Cc: linux-serial, Andy Shevchenko, prasad
In-Reply-To: <CAERbo5yjKDrVdfAZEAzWqeTSXoZJE_GZqbwov-4KGs1BPpwCiQ@mail.gmail.com>
[-- 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
>
^ permalink raw reply
* [PATCH] serial: 8250_pci: Fix uninitialized variable use in pci_oxsemi_tornado_get_divisor
From: Lee Yongjun @ 2026-01-21 15:53 UTC (permalink / raw)
To: Greg Kroah-Hartman, Jiri Slaby
Cc: linux-serial, linux-kernel, Andy Shevchenko, Lee Yongjun
Smatch reports the following errors:
drivers/tty/serial/8250/8250_pci.c:1246 pci_oxsemi_tornado_get_divisor() error: uninitialized symbol 'tcr'.
drivers/tty/serial/8250/8250_pci.c:1247 pci_oxsemi_tornado_get_divisor() error: uninitialized symbol 'quot'.
drivers/tty/serial/8250/8250_pci.c:1251 pci_oxsemi_tornado_get_divisor() error: uninitialized symbol 'quot'.
drivers/tty/serial/8250/8250_pci.c:1255 pci_oxsemi_tornado_get_divisor() error: uninitialized symbol 'cpr'.
drivers/tty/serial/8250/8250_pci.c:1265 pci_oxsemi_tornado_get_divisor() error: uninitialized symbol 'cpr'.
These variables are initialized in the if branch or inside the loop in
the else branch. However, if the loop finishes without finding a suitable
divisor, they remain uninitialized.
Initialize 'quot', 'cpr', and 'tcr' to 0 to prevent undefined behavior.
Signed-off-by: Lee Yongjun <jun85566@gmail.com>
---
drivers/tty/serial/8250/8250_pci.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/tty/serial/8250/8250_pci.c b/drivers/tty/serial/8250/8250_pci.c
index c5a932f48f74..b32d7e03c29a 100644
--- a/drivers/tty/serial/8250/8250_pci.c
+++ b/drivers/tty/serial/8250/8250_pci.c
@@ -1200,9 +1200,9 @@ static unsigned int pci_oxsemi_tornado_get_divisor(struct uart_port *port,
unsigned int sdiv = DIV_ROUND_CLOSEST(sclk, baud);
unsigned int best_squot;
unsigned int squot;
- unsigned int quot;
- u16 cpr;
- u8 tcr;
+ unsigned int quot = 0;
+ u16 cpr = 0;
+ u8 tcr = 0;
int i;
/* Old custom speed handling. */
--
2.34.1
^ permalink raw reply related
* Re: printk's threaded legacy console + fbcon => schedule where it should not
From: Petr Mladek @ 2026-01-21 16:08 UTC (permalink / raw)
To: Sebastian Andrzej Siewior
Cc: Steven Rostedt, linux-kernel, linux-serial, linux-fbdev,
John Ogness, Sergey Senozhatsky, Greg Kroah-Hartman, Jiri Slaby,
Simona Vetter, Helge Deller
In-Reply-To: <20260121135737.K7b-4M5r@linutronix.de>
On Wed 2026-01-21 14:57:37, Sebastian Andrzej Siewior wrote:
> On 2026-01-21 14:43:45 [+0100], Petr Mladek wrote:
> > I know that there was a plan to get rid of cond_resched().
> > But what is the status now, please?
>
> It is slowly moving => https://lore.kernel.org/all/20251219101502.GB1132199@noisy.programming.kicks-ass.net/
Good to know.
> > I still see more that 1k cond_resched() calls in the code:
> >
> > $> git grep cond_resched\(\) | grep "\.c:" | wc -l
> > 1263
> >
> > And config PREEMPT_VOLUNTARY still talks about the explicit
> > preemption points.
> >
> > > Should we just remove it and see what breaks?
> >
> > Honestly, I do not feel comfortable with removing it. It is true that
> > it has no effect in the printk() code path. But the vt code is used
> > also when working on the terminal.
> >
> > The vt code still uses console_lock() because it was intertwined
> > with printk console code since very old days. console_lock is a kind
> > of big kernel lock there.
>
> Do you a have path which loops and would mandate it? I found a few which
> do not matter and have their own cond_resched() around. So I don't see a
> reason to keep it. And I found one which breaks things so a removal
> makes sense.
Could anyone from VT guys comment on it, please?
> > Alternative solution is to get rid of the spin_trylock(). The only
> > purpose is to prevent race in console_flush_on_panic(). It used
> > to be a simple bit operation. The spin_lock() was added just to
> > get barriers right. But we have a great atomic_t API these days.
> >
> > IMHO, it is a win-win solution because a preemptive context is
> > always better.
>
> So why do we keep warts again? Just because it *might* be required?
> Keeping things preemptible makes sense but this is locking with no
> annotation what so ever.
Well, the current locking is documented but it creates false
positives. The "printing_lock" is taken on a single place
using spin_trylock(). Nobody would ever spin on it. So
sleeping is perfectly fine.
> Again. printk has its cond_resched, the tty has it, too.
> I'm with Steven on the removal side.
As I said, the cond_resched() does not have any effect from
the printk() code path. But the other VT paths might rely on it.
If VT-guys are willing to take the risk and remove it
then I am fine with it.
Best Regards,
Petr
^ permalink raw reply
* Re: [PATCH] serial: 8250_pci: Fix uninitialized variable use in pci_oxsemi_tornado_get_divisor
From: Greg Kroah-Hartman @ 2026-01-21 16:36 UTC (permalink / raw)
To: Lee Yongjun; +Cc: Jiri Slaby, linux-serial, linux-kernel, Andy Shevchenko
In-Reply-To: <20260121155352.52489-1-jun85566@gmail.com>
On Thu, Jan 22, 2026 at 12:53:52AM +0900, Lee Yongjun wrote:
> Smatch reports the following errors:
>
> drivers/tty/serial/8250/8250_pci.c:1246 pci_oxsemi_tornado_get_divisor() error: uninitialized symbol 'tcr'.
> drivers/tty/serial/8250/8250_pci.c:1247 pci_oxsemi_tornado_get_divisor() error: uninitialized symbol 'quot'.
> drivers/tty/serial/8250/8250_pci.c:1251 pci_oxsemi_tornado_get_divisor() error: uninitialized symbol 'quot'.
> drivers/tty/serial/8250/8250_pci.c:1255 pci_oxsemi_tornado_get_divisor() error: uninitialized symbol 'cpr'.
> drivers/tty/serial/8250/8250_pci.c:1265 pci_oxsemi_tornado_get_divisor() error: uninitialized symbol 'cpr'.
>
> These variables are initialized in the if branch or inside the loop in
> the else branch. However, if the loop finishes without finding a suitable
> divisor, they remain uninitialized.
But can that ever really happen? Please step through it all with the
values that the hardware provides to see this.
> Initialize 'quot', 'cpr', and 'tcr' to 0 to prevent undefined behavior.
Why haven't we seen any real problems with this? I think this is ok
as-is. Maybe a little bit complex and non-intutive, but ok.
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH] serial: 8250 dw: clear FIFO before writting LCR
From: Adriana Nicolae @ 2026-01-21 16:55 UTC (permalink / raw)
To: Ilpo Järvinen; +Cc: linux-serial, Andy Shevchenko, prasad
In-Reply-To: <faf3bae5-4656-7c47-d875-9103a5f2079e@linux.intel.com>
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
> >
^ permalink raw reply
* Re: [PATCH] serial: 8250_pci: Fix uninitialized variable use in pci_oxsemi_tornado_get_divisor
From: Lee Yongjun @ 2026-01-22 0:18 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Jiri Slaby, linux-serial, linux-kernel, Andy Shevchenko
In-Reply-To: <2026012141-arrival-mounted-0d49@gregkh>
Hi Greg,
Thanks for the clarification.
I agree that this case cannot occur in practice and the current code is
fine as-is. I will drop this patch.
Thanks,
Lee Yongjun
Hi Greg,
Thanks for the clarification.
I agree that this case cannot occur in practice and the current code is
fine as-is. I will drop this patch.
Thanks,
Lee Yongjun
^ permalink raw reply
* Re: [PATCH v3 03/14] software node: Implement device_get_match_data fwnode callback
From: Andy Shevchenko @ 2026-01-22 8:51 UTC (permalink / raw)
To: Sui Jingfeng
Cc: manivannan.sadhasivam, Rob Herring, Greg Kroah-Hartman,
Jiri Slaby, Nathan Chancellor, Nicolas Schier, Hans de Goede,
Ilpo Järvinen, Mark Pearson, Derek J. Clark,
Manivannan Sadhasivam, Krzysztof Kozlowski, Conor Dooley,
Marcel Holtmann, Luiz Augusto von Dentz, Bartosz Golaszewski,
Daniel Scally, Heikki Krogerus, Sakari Ailus, Rafael J. Wysocki,
Danilo Krummrich, Bartosz Golaszewski, linux-serial, linux-kernel,
linux-kbuild, platform-driver-x86, linux-pci, devicetree,
linux-arm-msm, linux-bluetooth, linux-pm, Stephan Gerhold,
Dmitry Baryshkov, linux-acpi
In-Reply-To: <26a001c3-2140-4241-87dd-604eab3f827b@linux.dev>
On Wed, Jan 14, 2026 at 11:21:59AM +0800, Sui Jingfeng wrote:
> On 2026/1/12 15:56, Andy Shevchenko wrote:
> > On Sat, Jan 10, 2026 at 12:26:21PM +0530, Manivannan Sadhasivam via B4 Relay wrote:
> > > Because the software node backend of the fwnode API framework lacks an
> > > implementation for the .device_get_match_data function callback.
> > Maybe this is done on purpose.
>
> It is a *fact* that the broken swnode lacks an implementation for the .device_get_match_data stub.
No need to re-create board files when it's not needed or doesn't fit.
> Otherwise, If it is really done *on purpose*, the maintainers of swnode
> backend could/shall document it in the source file *explicitly*.
Probably it should be spoken in a better way.
> > Have you thought about this aspect?
>
> If you are sure, then stop telling us something start with "Maybe ..."
I wasn't the author of the swnode idea I can't read their minds. Please,
ask the respective people about this directly.
> > > This makes it difficult to use(and/or test) a few drivers that originates
> > > from DT world on the non-DT platform.
> > How difficult?
>
> The emphasis isn't on the 'difficult', it means that not convenient
>
> > DSA implementation went to the way of taking DT overlay
> > approach.
>
> Software node can do the same implementation just as what ACPI fwnode backend does.
>
> > Why that one can't be applied here?
>
> DT overlay requires the OS distribution(such as ubuntu) has theDT overlay
> config option selected. this is introduce extra overhead/side effects on the
> non-DT systems.
If we have hotpluggable or runtime reconfigurable devices this is the expected
option to support them. I don't see a problem here.
> > > Implement the .device_get_match_data fwnode callback, which helps to keep
> > > the three backends of the fwnode API aligned as much as possible. This is
> > > also a fundamental step to make a few drivers OF-independent truely
> > > possible.
> > >
> > > Device drivers or platform setup codes are expected to provide a software
> > > node string property, named as "compatible". At this moment, the value of
> > > this string property is being used to match against the compatible entries
> > > in the of_device_id table. It can be extended in the future though.
> > I really do not want to see this patch
>
> Whatever!
>
> Then just stop the endless, bruth-force ranting on such a straight-forward thing.
>
> > without very good justification
>
> Justifications has been provided over and over again.
>
> > (note, there were at least two attempts in the past
>
> This exactly saying that the implementation is missing.
Now you count a third one for your pleasure :-)
P.S.
We already had this discussion in the past and this attitude won't help
moving forward.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v3 00/14] Add support for handling PCIe M.2 Key E connectors in devicetree
From: Andy Shevchenko @ 2026-01-22 8:52 UTC (permalink / raw)
To: Herve Codina
Cc: manivannan.sadhasivam, Rob Herring, Greg Kroah-Hartman,
Jiri Slaby, Nathan Chancellor, Nicolas Schier, Hans de Goede,
Ilpo Järvinen, Mark Pearson, Derek J. Clark,
Manivannan Sadhasivam, Krzysztof Kozlowski, Conor Dooley,
Marcel Holtmann, Luiz Augusto von Dentz, Bartosz Golaszewski,
Daniel Scally, Heikki Krogerus, Sakari Ailus, Rafael J. Wysocki,
Danilo Krummrich, Bartosz Golaszewski, linux-serial, linux-kernel,
linux-kbuild, platform-driver-x86, linux-pci, devicetree,
linux-arm-msm, linux-bluetooth, linux-pm, Stephan Gerhold,
Dmitry Baryshkov, linux-acpi, Bartosz Golaszewski, Sui Jingfeng
In-Reply-To: <20260114134004.11023a7e@bootlin.com>
On Wed, Jan 14, 2026 at 01:40:04PM +0100, Herve Codina wrote:
> On Mon, 12 Jan 2026 10:18:41 +0200
> Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
>
> > +Cc: Herve (btw, any news on LAN966x support?)
>
> Related to LAN966x support, I am still stucked on issues related to
> fw_devlink and DT overlays [1].
Thank you for the update and your reply here!
> [1] https://lore.kernel.org/all/20260112154731.6540453b@bootlin.com/
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v5 06/11] dt-bindings: pinctrl: pinctrl-microchip-sgpio: add LAN969x
From: Robert Marko @ 2026-01-22 9:35 UTC (permalink / raw)
To: Linus Walleij
Cc: robh, krzk+dt, conor+dt, nicolas.ferre, alexandre.belloni,
claudiu.beznea, herbert, davem, lee, andrew+netdev, edumazet,
kuba, pabeni, Steen.Hegelund, daniel.machon, UNGLinuxDriver,
olivia, richard.genoud, radu_nicolae.pirea, gregkh,
richardcochran, horatiu.vultur, Ryan.Wanner, tudor.ambarus,
kavyasree.kotagiri, lars.povlsen, devicetree, linux-arm-kernel,
linux-kernel, linux-crypto, netdev, linux-gpio, linux-spi,
linux-serial, luka.perkov, Conor Dooley
In-Reply-To: <CAD++jLmitNVhWmUf9BBqLR2_WsAR7V-+ykVJsLK3MuOSUKQF0A@mail.gmail.com>
On Wed, Jan 21, 2026 at 1:44 PM Linus Walleij <linusw@kernel.org> wrote:
>
> Hi Robert,
>
> On Thu, Jan 15, 2026 at 12:41 PM Robert Marko <robert.marko@sartura.hr> wrote:
>
> > Document LAN969x compatibles for SGPIO.
> >
> > Signed-off-by: Robert Marko <robert.marko@sartura.hr>
> > Acked-by: Conor Dooley <conor.dooley@microchip.com>
> > Reviewed-by: Claudiu Beznea <claudiu.beznea@tuxon.dev>
>
> Should I just merge this one patch to the pinctrl tree?
That would be great as other bindings are slowly being picked into
their respective trees.
Regards,
Robert
> Looks good to me.
>
> Yours,
> Linus Walleij
--
Robert Marko
Staff Embedded Linux Engineer
Sartura d.d.
Lendavska ulica 16a
10000 Zagreb, Croatia
Email: robert.marko@sartura.hr
Web: www.sartura.hr
^ permalink raw reply
* [PATCH v1 1/2] serial: 8250_omap: Remove custom deprecated baud setting routine
From: Andy Shevchenko @ 2026-01-22 10:19 UTC (permalink / raw)
To: Andy Shevchenko, Greg Kroah-Hartman, Jiri Slaby (SUSE), linux-doc,
linux-kernel, linux-serial
Cc: Jonathan Corbet, Shuah Khan, Kevin Hilman, Tony Lindgren
In-Reply-To: <20260122102349.2395423-1-andriy.shevchenko@linux.intel.com>
As comments mentioned this is old (and actually deprecated) interface
to set custom baud rates. This interface has limitations as it only
allows to set a single opaque value called "custom_divisor". If the HW
needs more complex settings (like fractional divisor) it must somehow
encode this. This is horrid interface that is very driver specific
and not flexible. Meanwhile Linux has established way to set free
baud rate settings via BOTHER [1]. With all this being said, remove
deprecated interface for good.
Link: https://stackoverflow.com/questions/12646324/how-can-i-set-a-custom-baud-rate-on-linux [1]
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
drivers/tty/serial/8250/8250_omap.c | 16 ----------------
1 file changed, 16 deletions(-)
diff --git a/drivers/tty/serial/8250/8250_omap.c b/drivers/tty/serial/8250/8250_omap.c
index 4496f692b86b..427719c158b7 100644
--- a/drivers/tty/serial/8250/8250_omap.c
+++ b/drivers/tty/serial/8250/8250_omap.c
@@ -243,22 +243,6 @@ static void omap_8250_get_divisor(struct uart_port *port, unsigned int baud,
unsigned int div_13, div_16;
unsigned int abs_d13, abs_d16;
- /*
- * Old custom speed handling.
- */
- if (baud == 38400 && (port->flags & UPF_SPD_MASK) == UPF_SPD_CUST) {
- priv->quot = port->custom_divisor & UART_DIV_MAX;
- /*
- * I assume that nobody is using this. But hey, if somebody
- * would like to specify the divisor _and_ the mode then the
- * driver is ready and waiting for it.
- */
- if (port->custom_divisor & (1 << 16))
- priv->mdr1 = UART_OMAP_MDR1_13X_MODE;
- else
- priv->mdr1 = UART_OMAP_MDR1_16X_MODE;
- return;
- }
div_13 = DIV_ROUND_CLOSEST(uartclk, 13 * baud);
div_16 = DIV_ROUND_CLOSEST(uartclk, 16 * baud);
--
2.50.1
^ permalink raw reply related
* [PATCH v1 2/2] serial: 8250_pci: Remove custom deprecated baud setting routine
From: Andy Shevchenko @ 2026-01-22 10:19 UTC (permalink / raw)
To: Andy Shevchenko, Greg Kroah-Hartman, Jiri Slaby (SUSE), linux-doc,
linux-kernel, linux-serial
Cc: Jonathan Corbet, Shuah Khan, Kevin Hilman, Tony Lindgren
In-Reply-To: <20260122102349.2395423-1-andriy.shevchenko@linux.intel.com>
As comments mentioned this is old (and actually deprecated) interface
to set custom baud rates. This interface has limitations as it only
allows to set a single opaque value called "custom_divisor". If the HW
needs more complex settings (like fractional divisor) it must somehow
encode this. This is horrid interface that is very driver specific
and not flexible. Meanwhile Linux has established way to set free
baud rate settings via BOTHER [1]. With all this being said, remove
deprecated interface for good.
Link: https://stackoverflow.com/questions/12646324/how-can-i-set-a-custom-baud-rate-on-linux [1]
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
Documentation/misc-devices/oxsemi-tornado.rst | 26 +-----
drivers/tty/serial/8250/8250_pci.c | 85 ++++++++-----------
2 files changed, 38 insertions(+), 73 deletions(-)
diff --git a/Documentation/misc-devices/oxsemi-tornado.rst b/Documentation/misc-devices/oxsemi-tornado.rst
index b33351bef6cf..fe2e5f726c2b 100644
--- a/Documentation/misc-devices/oxsemi-tornado.rst
+++ b/Documentation/misc-devices/oxsemi-tornado.rst
@@ -89,31 +89,7 @@ With the baud base set to 15625000 and the unsigned 16-bit UART_DIV_MAX
limitation imposed by ``serial8250_get_baud_rate`` standard baud rates
below 300bps become unavailable in the regular way, e.g. the rate of
200bps requires the baud base to be divided by 78125 and that is beyond
-the unsigned 16-bit range. The historic spd_cust feature can still be
-used by encoding the values for, the prescaler, the oversampling rate
-and the clock divisor (DLM/DLL) as follows to obtain such rates if so
-required:
-
-::
-
- 31 29 28 20 19 16 15 0
- +-----+-----------------+-------+-------------------------------+
- |0 0 0| CPR2:CPR | TCR | DLM:DLL |
- +-----+-----------------+-------+-------------------------------+
-
-Use a value such encoded for the ``custom_divisor`` field along with the
-ASYNC_SPD_CUST flag set in the ``flags`` field in ``struct serial_struct``
-passed with the TIOCSSERIAL ioctl(2), such as with the setserial(8)
-utility and its ``divisor`` and ``spd_cust`` parameters, and then select
-the baud rate of 38400bps. Note that the value of 0 in TCR sets the
-oversampling rate to 16 and prescaler values below 1 in CPR2/CPR are
-clamped by the driver to 1.
-
-For example the value of 0x1f4004e2 will set CPR2/CPR, TCR and DLM/DLL
-respectively to 0x1f4, 0x0 and 0x04e2, choosing the prescaler value,
-the oversampling rate and the clock divisor of 62.500, 16 and 1250
-respectively. These parameters will set the baud rate for the serial
-port to 62500000 / 62.500 / 1250 / 16 = 50bps.
+the unsigned 16-bit range.
Maciej W. Rozycki <macro@orcam.me.uk>
diff --git a/drivers/tty/serial/8250/8250_pci.c b/drivers/tty/serial/8250/8250_pci.c
index 3efe075ef7b2..6589bb531cc6 100644
--- a/drivers/tty/serial/8250/8250_pci.c
+++ b/drivers/tty/serial/8250/8250_pci.c
@@ -1205,60 +1205,49 @@ static unsigned int pci_oxsemi_tornado_get_divisor(struct uart_port *port,
u8 tcr;
int i;
- /* Old custom speed handling. */
- if (baud == 38400 && (port->flags & UPF_SPD_MASK) == UPF_SPD_CUST) {
- unsigned int cust_div = port->custom_divisor;
+ best_squot = quot_scale;
+ for (i = 0; i < ARRAY_SIZE(p); i++) {
+ unsigned int spre;
+ unsigned int srem;
+ u8 cp;
+ u8 tc;
- quot = cust_div & UART_DIV_MAX;
- tcr = (cust_div >> 16) & OXSEMI_TORNADO_TCR_MASK;
- cpr = (cust_div >> 20) & OXSEMI_TORNADO_CPR_MASK;
- if (cpr < OXSEMI_TORNADO_CPR_MIN)
- cpr = OXSEMI_TORNADO_CPR_DEF;
- } else {
- best_squot = quot_scale;
- for (i = 0; i < ARRAY_SIZE(p); i++) {
- unsigned int spre;
- unsigned int srem;
- u8 cp;
- u8 tc;
+ tc = p[i][0];
+ cp = p[i][1];
+ spre = tc * cp;
- tc = p[i][0];
- cp = p[i][1];
- spre = tc * cp;
+ srem = sdiv % spre;
+ if (srem > spre / 2)
+ srem = spre - srem;
+ squot = DIV_ROUND_CLOSEST(srem * quot_scale, spre);
- srem = sdiv % spre;
- if (srem > spre / 2)
- srem = spre - srem;
- squot = DIV_ROUND_CLOSEST(srem * quot_scale, spre);
-
- if (srem == 0) {
- tcr = tc;
- cpr = cp;
- quot = sdiv / spre;
- break;
- } else if (squot < best_squot) {
- best_squot = squot;
- tcr = tc;
- cpr = cp;
- quot = DIV_ROUND_CLOSEST(sdiv, spre);
- }
+ if (srem == 0) {
+ tcr = tc;
+ cpr = cp;
+ quot = sdiv / spre;
+ break;
+ } else if (squot < best_squot) {
+ best_squot = squot;
+ tcr = tc;
+ cpr = cp;
+ quot = DIV_ROUND_CLOSEST(sdiv, spre);
}
- while (tcr <= (OXSEMI_TORNADO_TCR_MASK + 1) >> 1 &&
- quot % 2 == 0) {
+ }
+ while (tcr <= (OXSEMI_TORNADO_TCR_MASK + 1) >> 1 &&
+ quot % 2 == 0) {
+ quot >>= 1;
+ tcr <<= 1;
+ }
+ while (quot > UART_DIV_MAX) {
+ if (tcr <= (OXSEMI_TORNADO_TCR_MASK + 1) >> 1) {
quot >>= 1;
tcr <<= 1;
- }
- while (quot > UART_DIV_MAX) {
- if (tcr <= (OXSEMI_TORNADO_TCR_MASK + 1) >> 1) {
- quot >>= 1;
- tcr <<= 1;
- } else if (cpr <= OXSEMI_TORNADO_CPR_MASK >> 1) {
- quot >>= 1;
- cpr <<= 1;
- } else {
- quot = quot * cpr / OXSEMI_TORNADO_CPR_MASK;
- cpr = OXSEMI_TORNADO_CPR_MASK;
- }
+ } else if (cpr <= OXSEMI_TORNADO_CPR_MASK >> 1) {
+ quot >>= 1;
+ cpr <<= 1;
+ } else {
+ quot = quot * cpr / OXSEMI_TORNADO_CPR_MASK;
+ cpr = OXSEMI_TORNADO_CPR_MASK;
}
}
--
2.50.1
^ permalink raw reply related
* [PATCH v1 0/2] serial: 8250: Remove highly customised deprecated interface
From: Andy Shevchenko @ 2026-01-22 10:19 UTC (permalink / raw)
To: Andy Shevchenko, Greg Kroah-Hartman, Jiri Slaby (SUSE), linux-doc,
linux-kernel, linux-serial
Cc: Jonathan Corbet, Shuah Khan, Kevin Hilman, Tony Lindgren
The custom divisor interface not only old and deprecated, it's too much
driver-dependent. For the user it's not easy to understand what to supply
as a value (usually encoded) via that interface. On top of it, there is
an existing BOTHER mechanism that allows to do the same and more without
any tribal knowledge.
Remove highly customised deprecated interface to set baud rates.
Note, the "standard" custom_divisor that is used as a plain value
for "quot" is still left and may be used by the respective drivers
including but not limited to 8250.
Andy Shevchenko (2):
serial: 8250_omap: Remove custom deprecated baud setting routine
serial: 8250_pci: Remove custom deprecated baud setting routine
Documentation/misc-devices/oxsemi-tornado.rst | 26 +-----
drivers/tty/serial/8250/8250_omap.c | 16 ----
drivers/tty/serial/8250/8250_pci.c | 85 ++++++++-----------
3 files changed, 38 insertions(+), 89 deletions(-)
--
2.50.1
^ permalink raw reply
* Re: [PATCH] serial: 8250 dw: clear FIFO before writting LCR
From: Ilpo Järvinen @ 2026-01-22 13:18 UTC (permalink / raw)
To: Adriana Nicolae; +Cc: linux-serial, Andy Shevchenko, prasad
In-Reply-To: <CAERbo5z2370GPWUNuSJuwZxga0se7CNH2XAnsnF-vtB7t6XREQ@mail.gmail.com>
[-- 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.
^ permalink raw reply
* Re: [PATCH 00/21] Adding support of ADI ARMv8 ADSP-SC598 SoC.
From: Arturs Artamonovs @ 2026-01-22 15:37 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: arturs.artamonovs, Catalin Marinas, Will Deacon, Greg Malysa,
Philipp Zabel, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Utsav Agarwal, Michael Turquette, Stephen Boyd, Linus Walleij,
Bartosz Golaszewski, Thomas Gleixner, Andi Shyti,
Greg Kroah-Hartman, Jiri Slaby, Arnd Bergmann, Olof Johansson,
soc, linux-arm-kernel, linux-kernel, devicetree, linux-clk,
linux-gpio, linux-i2c, linux-serial, adsp-linux,
Nathan Barrett-Morrison
Hi, big thanks to Krysztof, Arnd, Rob and Linus Walleij for the reviewing first set of patches that were sent some time ago. I have reviewed comments few times since then and Im currently at last stage of internal review for v2 series those are planned to be sent sometime in near future.
> On 16 Sep 2024, at 10:05, Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On 12/09/2024 20:24, Arturs Artamonovs via B4 Relay wrote:
>> This set of patches based on ADI fork of Linux Kerenl that support family of ADSP-SC5xx
>> SoC's and used by customers for some time . Patch series contains minimal set
>> of changes to add ADSP-SC598 support to upstream kernel. This series include
>> UART,I2C,IRQCHIP,RCU drivers and device-tree to be able boot on EV-SC598-SOM
>> board into serial shell and able to reset the board. Current SOM board
>> requires I2C expander to enable UART output.
>>
>> UART,I2C and PINCTRL drivers are based on old Blackfin drivers with
>> ADSP-SC5xx related bug fixes and improvments.
>>
>> Signed-off-by: Arturs Artamonovs <arturs.artamonovs@analog.com>
>> ---
>
> For new platform, be sure you have 0 warnings:
> 1. Please run standard kernel tools for static analysis, like
> coccinelle, smatch and sparse, and fix reported warnings.
>
> 2. Also check for warnings when building with W=1. Most of these
> commands (checks or W=1 build) can build specific targets, like some
> directory, to narrow the scope to only your code. The code here looks
> like it needs a fix. Feel free to get in touch if the warning is not clear.
>
> 3. Fix all compile test warning reported by LKP and check for common
> configs, regardless of reports.
>
> 4. Please run `make dtbs_check W=1` (see
> Documentation/devicetree/bindings/writing-schema.rst or
> https://www.linaro.org/blog/tips-and-tricks-for-validating-devicetree-sources-with-the-devicetree-schema/
> for instructions).
>
> 5. Please run scripts/checkpatch.pl and fix reported warnings. Then
> please run `scripts/checkpatch.pl --strict` and (probably) fix more
> warnings. Some warnings can be ignored, especially from --strict run.
>
>
> Best regards,
> Krzysztof
>
>
^ permalink raw reply
* [PATCH] serial: Fix not set tty->port race condition
From: Krzysztof Kozlowski @ 2026-01-22 17:00 UTC (permalink / raw)
To: Greg Kroah-Hartman, Jiri Slaby, linux-kernel, linux-serial
Cc: Krzysztof Kozlowski, stable
Revert commit bfc467db60b7 ("serial: remove redundant
tty_port_link_device()") because the tty_port_link_device() is not
redundant: the tty->port has to be confured before we call
uart_configure_port(), otherwise user-space can open console without TTY
linked to the driver.
This tty_port_link_device() was added explicitly to avoid this exact
issue in commit fb2b90014d78 ("tty: link tty and port before configuring
it as console"), so offending commit basically reverted the fix saying
it is redundant without addressing the actual race condition presented
there.
Reproducible always as tty->port warning on Qualcomm SoC with most of
devices disabled, so with very fast boot, and one serial device being
the console:
printk: legacy console [ttyMSM0] enabled
printk: legacy console [ttyMSM0] enabled
printk: legacy bootconsole [qcom_geni0] disabled
printk: legacy bootconsole [qcom_geni0] disabled
------------[ cut here ]------------
tty_init_dev: ttyMSM driver does not set tty->port. This would crash the kernel. Fix the driver!
WARNING: drivers/tty/tty_io.c:1414 at tty_init_dev.part.0+0x228/0x25c, CPU#2: systemd/1
Modules linked in: socinfo tcsrcc_eliza gcc_eliza sm3_ce fuse ipv6
CPU: 2 UID: 0 PID: 1 Comm: systemd Tainted: G S 6.19.0-rc4-next-20260108-00024-g2202f4d30aa8 #73 PREEMPT
Tainted: [S]=CPU_OUT_OF_SPEC
Hardware name: Qualcomm Technologies, Inc. Eliza (DT)
...
tty_init_dev.part.0 (drivers/tty/tty_io.c:1414 (discriminator 11)) (P)
tty_open (arch/arm64/include/asm/atomic_ll_sc.h:95 (discriminator 3) drivers/tty/tty_io.c:2073 (discriminator 3) drivers/tty/tty_io.c:2120 (discriminator 3))
chrdev_open (fs/char_dev.c:411)
do_dentry_open (fs/open.c:962)
vfs_open (fs/open.c:1094)
do_open (fs/namei.c:4634)
path_openat (fs/namei.c:4793)
do_filp_open (fs/namei.c:4820)
do_sys_openat2 (fs/open.c:1391 (discriminator 3))
...
Starting Network Name Resolution...
Apparently the flow with this small Yocto-based ramdisk user-space is:
driver (qcom_geni_serial.c): user-space:
============================ ===========
qcom_geni_serial_probe()
uart_add_one_port()
serial_core_register_port()
serial_core_add_one_port()
uart_configure_port()
register_console()
|
| open console
| ...
| tty_init_dev()
| driver->ports[idx] is NULL
|
tty_port_register_device_attr_serdev()
tty_port_link_device() <- set driver->ports[idx]
Fixes: bfc467db60b7 ("serial: remove redundant tty_port_link_device()")
Cc: <stable@vger.kernel.org>
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
---
drivers/tty/serial/serial_core.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index 0534b2eb1682..116f33f0643f 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -3077,6 +3077,7 @@ static int serial_core_add_one_port(struct uart_driver *drv, struct uart_port *u
if (uport->cons && uport->dev)
of_console_check(uport->dev->of_node, uport->cons->name, uport->line);
+ tty_port_link_device(port, drv->tty_driver, uport->line);
uart_configure_port(drv, state, uport);
port->console = uart_console(uport);
--
2.51.0
^ permalink raw reply related
* Re: [PATCH] serial: Fix not set tty->port race condition
From: Krzysztof Kozlowski @ 2026-01-22 17:11 UTC (permalink / raw)
To: Greg Kroah-Hartman, Jiri Slaby, linux-kernel, linux-serial; +Cc: stable
In-Reply-To: <20260122170031.433724-2-krzysztof.kozlowski@oss.qualcomm.com>
On 22/01/2026 18:00, Krzysztof Kozlowski wrote:
> Revert commit bfc467db60b7 ("serial: remove redundant
> tty_port_link_device()") because the tty_port_link_device() is not
And grumpy side note because I was looking at this for more than a day
blaming my new hardware:
I really wish commits (e.g. bfc467db60b7) calling something redundant
had that much of message written why something is redundant as the
commit (fb2b90014d78) which introduced that part of code.
If someone wrote one page of text why foo is needed, we should write not
less why it is not needed :)
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v2 00/16] MIPS: move pic32.h header file from asm to platform_data
From: Brian Masney @ 2026-01-23 0:49 UTC (permalink / raw)
To: Thomas Bogendoerfer, Claudiu Beznea
Cc: linux-mips, linux-kernel, Michael Turquette, Stephen Boyd,
linux-clk, Thomas Gleixner, Adrian Hunter, Ulf Hansson, linux-mmc,
Linus Walleij, linux-gpio, Alexandre Belloni, linux-rtc,
Greg Kroah-Hartman, Jiri Slaby, linux-serial, Guenter Roeck,
Wim Van Sebroeck, linux-watchdog
In-Reply-To: <20260112-mips-pic32-header-move-v2-0-927d516b1ff9@redhat.com>
Hi Thomas,
On Mon, Jan 12, 2026 at 05:47:54PM -0500, Brian Masney wrote:
> There are currently some pic32 MIPS drivers that are in tree, and are
> only configured to be compiled on the MIPS pic32 platform. There's a
> risk of breaking some of these drivers when migrating drivers away from
> legacy APIs. It happened to me with a pic32 clk driver.
>
> Let's go ahead and move the pic32.h from the asm to the platform_data
> include directory in the tree. This will make it easier, and cleaner to
> enable COMPILE_TEST for some of these pic32 drivers. To do this requires
> updating some includes, which I do at the beginning of this series.
>
> This series was compile tested on a centos-stream-10 arm64 host in two
> different configurations:
>
> - native arm64 build with COMPILE_TEST (via make allmodconfig)
> - MIPS cross compile on arm64 with:
> ARCH=mips CROSS_COMPILE=mips64-linux-gnu- make pic32mzda_defconfig
>
> Note that there is a separate MIPS compile error in linux-next, and I
> reported it at https://lore.kernel.org/all/aWVs2gVB418WiMVa@redhat.com/
>
> I included a patch at the end that shows enabling COMPILE_TEST for a
> pic32 clk driver.
>
> Merge Strategy
> ==============
> - Patches 1-15 can go through the MIPS tree.
I'm just checking if you'll be able to take patches 1-15 this
development cycle before the merge window opens?
If this series goes to Linus during this upcoming merge window, then
I have 17 patches ready to post in a month for the next development
cycle that can go to various individual subsystems that allows all
of these pic32 MIPS drivers to be compiled on all architectures. The
patches also fix some issues in those drivers that were found by
kernel CI.
This merge strategy makes it so that we won't have to deal with any
cross tree merge issues, or immutable branches.
Thanks,
Brian
^ permalink raw reply
* Re: [PATCH] serial: Fix not set tty->port race condition
From: Jiri Slaby @ 2026-01-23 5:55 UTC (permalink / raw)
To: Krzysztof Kozlowski, Greg Kroah-Hartman, linux-kernel,
linux-serial; +Cc: stable
In-Reply-To: <20260122170031.433724-2-krzysztof.kozlowski@oss.qualcomm.com>
On 22. 01. 26, 18:00, Krzysztof Kozlowski wrote:
> Revert commit bfc467db60b7 ("serial: remove redundant
> tty_port_link_device()") because the tty_port_link_device() is not
> redundant: the tty->port has to be confured before we call
> uart_configure_port(), otherwise user-space can open console without TTY
> linked to the driver.
>
> This tty_port_link_device() was added explicitly to avoid this exact
> issue in commit fb2b90014d78 ("tty: link tty and port before configuring
> it as console"), so offending commit basically reverted the fix saying
> it is redundant without addressing the actual race condition presented
> there.
>
> Reproducible always as tty->port warning on Qualcomm SoC with most of
> devices disabled, so with very fast boot, and one serial device being
> the console:
>
> printk: legacy console [ttyMSM0] enabled
> printk: legacy console [ttyMSM0] enabled
> printk: legacy bootconsole [qcom_geni0] disabled
> printk: legacy bootconsole [qcom_geni0] disabled
> ------------[ cut here ]------------
> tty_init_dev: ttyMSM driver does not set tty->port. This would crash the kernel. Fix the driver!
> WARNING: drivers/tty/tty_io.c:1414 at tty_init_dev.part.0+0x228/0x25c, CPU#2: systemd/1
> Modules linked in: socinfo tcsrcc_eliza gcc_eliza sm3_ce fuse ipv6
> CPU: 2 UID: 0 PID: 1 Comm: systemd Tainted: G S 6.19.0-rc4-next-20260108-00024-g2202f4d30aa8 #73 PREEMPT
> Tainted: [S]=CPU_OUT_OF_SPEC
> Hardware name: Qualcomm Technologies, Inc. Eliza (DT)
> ...
> tty_init_dev.part.0 (drivers/tty/tty_io.c:1414 (discriminator 11)) (P)
> tty_open (arch/arm64/include/asm/atomic_ll_sc.h:95 (discriminator 3) drivers/tty/tty_io.c:2073 (discriminator 3) drivers/tty/tty_io.c:2120 (discriminator 3))
> chrdev_open (fs/char_dev.c:411)
> do_dentry_open (fs/open.c:962)
> vfs_open (fs/open.c:1094)
> do_open (fs/namei.c:4634)
> path_openat (fs/namei.c:4793)
> do_filp_open (fs/namei.c:4820)
> do_sys_openat2 (fs/open.c:1391 (discriminator 3))
> ...
> Starting Network Name Resolution...
>
> Apparently the flow with this small Yocto-based ramdisk user-space is:
>
> driver (qcom_geni_serial.c): user-space:
> ============================ ===========
> qcom_geni_serial_probe()
> uart_add_one_port()
> serial_core_register_port()
> serial_core_add_one_port()
> uart_configure_port()
> register_console()
> |
> | open console
> | ...
> | tty_init_dev()
> | driver->ports[idx] is NULL
> |
> tty_port_register_device_attr_serdev()
> tty_port_link_device() <- set driver->ports[idx]
>
> Fixes: bfc467db60b7 ("serial: remove redundant tty_port_link_device()")
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
> ---
> drivers/tty/serial/serial_core.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
> index 0534b2eb1682..116f33f0643f 100644
> --- a/drivers/tty/serial/serial_core.c
> +++ b/drivers/tty/serial/serial_core.c
> @@ -3077,6 +3077,7 @@ static int serial_core_add_one_port(struct uart_driver *drv, struct uart_port *u
> if (uport->cons && uport->dev)
> of_console_check(uport->dev->of_node, uport->cons->name, uport->line);
>
> + tty_port_link_device(port, drv->tty_driver, uport->line);
Bah, so add a comment or I (or somebody) remove it again eventually :(.
thanks,
--
js
suse labs
^ permalink raw reply
* Re: [PATCH] serial: Fix not set tty->port race condition
From: Jiri Slaby @ 2026-01-23 5:59 UTC (permalink / raw)
To: Krzysztof Kozlowski, Greg Kroah-Hartman, linux-kernel,
linux-serial; +Cc: stable
In-Reply-To: <d86ad878-a125-41c9-a0a3-bc24c926ba55@oss.qualcomm.com>
On 22. 01. 26, 18:11, Krzysztof Kozlowski wrote:
> On 22/01/2026 18:00, Krzysztof Kozlowski wrote:
>> Revert commit bfc467db60b7 ("serial: remove redundant
>> tty_port_link_device()") because the tty_port_link_device() is not
>
> And grumpy side note because I was looking at this for more than a day
> blaming my new hardware:
>
> I really wish commits (e.g. bfc467db60b7) calling something redundant
> had that much of message written why something is redundant as the
> commit (fb2b90014d78) which introduced that part of code.
It was clear enough: because tty_port_register_device_attr_serdev()
links the port few lines below.
But it/I somehow didn't take the hidden uart_console() in
uart_configure_port() into account.
> If someone wrote one page of text why foo is needed, we should write not
> less why it is not needed :)
I think I could generate a bloat of text. But you will still have a
broken kernel the same way :)?
thanks,
--
js
suse labs
^ permalink raw reply
* Re: [PATCH] serial: Fix not set tty->port race condition
From: Krzysztof Kozlowski @ 2026-01-23 7:13 UTC (permalink / raw)
To: Jiri Slaby, Greg Kroah-Hartman, linux-kernel, linux-serial; +Cc: stable
In-Reply-To: <e0ee73fe-2b9e-4976-9648-35a6822b8ad1@kernel.org>
On 23/01/2026 06:55, Jiri Slaby wrote:
> On 22. 01. 26, 18:00, Krzysztof Kozlowski wrote:
>>
>> diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
>> index 0534b2eb1682..116f33f0643f 100644
>> --- a/drivers/tty/serial/serial_core.c
>> +++ b/drivers/tty/serial/serial_core.c
>> @@ -3077,6 +3077,7 @@ static int serial_core_add_one_port(struct uart_driver *drv, struct uart_port *u
>> if (uport->cons && uport->dev)
>> of_console_check(uport->dev->of_node, uport->cons->name, uport->line);
>>
>> + tty_port_link_device(port, drv->tty_driver, uport->line);
>
> Bah, so add a comment or I (or somebody) remove it again eventually :(.
>
Good point.
Best regards,
Krzysztof
^ permalink raw reply
* [PATCH v2] serial: Fix not set tty->port race condition
From: Krzysztof Kozlowski @ 2026-01-23 7:21 UTC (permalink / raw)
To: Greg Kroah-Hartman, Jiri Slaby, linux-kernel, linux-serial
Cc: Krzysztof Kozlowski, stable
Revert commit bfc467db60b7 ("serial: remove redundant
tty_port_link_device()") because the tty_port_link_device() is not
redundant: the tty->port has to be confured before we call
uart_configure_port(), otherwise user-space can open console without TTY
linked to the driver.
This tty_port_link_device() was added explicitly to avoid this exact
issue in commit fb2b90014d78 ("tty: link tty and port before configuring
it as console"), so offending commit basically reverted the fix saying
it is redundant without addressing the actual race condition presented
there.
Reproducible always as tty->port warning on Qualcomm SoC with most of
devices disabled, so with very fast boot, and one serial device being
the console:
printk: legacy console [ttyMSM0] enabled
printk: legacy console [ttyMSM0] enabled
printk: legacy bootconsole [qcom_geni0] disabled
printk: legacy bootconsole [qcom_geni0] disabled
------------[ cut here ]------------
tty_init_dev: ttyMSM driver does not set tty->port. This would crash the kernel. Fix the driver!
WARNING: drivers/tty/tty_io.c:1414 at tty_init_dev.part.0+0x228/0x25c, CPU#2: systemd/1
Modules linked in: socinfo tcsrcc_eliza gcc_eliza sm3_ce fuse ipv6
CPU: 2 UID: 0 PID: 1 Comm: systemd Tainted: G S 6.19.0-rc4-next-20260108-00024-g2202f4d30aa8 #73 PREEMPT
Tainted: [S]=CPU_OUT_OF_SPEC
Hardware name: Qualcomm Technologies, Inc. Eliza (DT)
...
tty_init_dev.part.0 (drivers/tty/tty_io.c:1414 (discriminator 11)) (P)
tty_open (arch/arm64/include/asm/atomic_ll_sc.h:95 (discriminator 3) drivers/tty/tty_io.c:2073 (discriminator 3) drivers/tty/tty_io.c:2120 (discriminator 3))
chrdev_open (fs/char_dev.c:411)
do_dentry_open (fs/open.c:962)
vfs_open (fs/open.c:1094)
do_open (fs/namei.c:4634)
path_openat (fs/namei.c:4793)
do_filp_open (fs/namei.c:4820)
do_sys_openat2 (fs/open.c:1391 (discriminator 3))
...
Starting Network Name Resolution...
Apparently the flow with this small Yocto-based ramdisk user-space is:
driver (qcom_geni_serial.c): user-space:
============================ ===========
qcom_geni_serial_probe()
uart_add_one_port()
serial_core_register_port()
serial_core_add_one_port()
uart_configure_port()
register_console()
|
| open console
| ...
| tty_init_dev()
| driver->ports[idx] is NULL
|
tty_port_register_device_attr_serdev()
tty_port_link_device() <- set driver->ports[idx]
Fixes: bfc467db60b7 ("serial: remove redundant tty_port_link_device()")
Cc: <stable@vger.kernel.org>
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
---
Changes in v2:
1. Add comment to the code.
---
drivers/tty/serial/serial_core.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index 9930023e924c..2805cad10511 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -3074,6 +3074,12 @@ static int serial_core_add_one_port(struct uart_driver *drv, struct uart_port *u
if (uport->cons && uport->dev)
of_console_check(uport->dev->of_node, uport->cons->name, uport->line);
+ /*
+ * TTY port has to be linked with the driver before register_console()
+ * in uart_configure_port(), because user-space could open the console
+ * immediately after.
+ */
+ tty_port_link_device(port, drv->tty_driver, uport->line);
uart_configure_port(drv, state, uport);
port->console = uart_console(uport);
--
2.51.0
^ permalink raw reply related
* Re: [PATCH v2] serial: Fix not set tty->port race condition
From: Jiri Slaby @ 2026-01-23 7:35 UTC (permalink / raw)
To: Krzysztof Kozlowski, Greg Kroah-Hartman, linux-kernel,
linux-serial; +Cc: stable
In-Reply-To: <20260123072139.53293-2-krzysztof.kozlowski@oss.qualcomm.com>
On 23. 01. 26, 8:21, Krzysztof Kozlowski wrote:
> Revert commit bfc467db60b7 ("serial: remove redundant
> tty_port_link_device()") because the tty_port_link_device() is not
> redundant: the tty->port has to be confured before we call
> uart_configure_port(), otherwise user-space can open console without TTY
> linked to the driver.
>
> This tty_port_link_device() was added explicitly to avoid this exact
> issue in commit fb2b90014d78 ("tty: link tty and port before configuring
> it as console"), so offending commit basically reverted the fix saying
> it is redundant without addressing the actual race condition presented
> there.
>
> Reproducible always as tty->port warning on Qualcomm SoC with most of
> devices disabled, so with very fast boot, and one serial device being
> the console:
>
> printk: legacy console [ttyMSM0] enabled
> printk: legacy console [ttyMSM0] enabled
> printk: legacy bootconsole [qcom_geni0] disabled
> printk: legacy bootconsole [qcom_geni0] disabled
> ------------[ cut here ]------------
> tty_init_dev: ttyMSM driver does not set tty->port. This would crash the kernel. Fix the driver!
> WARNING: drivers/tty/tty_io.c:1414 at tty_init_dev.part.0+0x228/0x25c, CPU#2: systemd/1
> Modules linked in: socinfo tcsrcc_eliza gcc_eliza sm3_ce fuse ipv6
> CPU: 2 UID: 0 PID: 1 Comm: systemd Tainted: G S 6.19.0-rc4-next-20260108-00024-g2202f4d30aa8 #73 PREEMPT
> Tainted: [S]=CPU_OUT_OF_SPEC
> Hardware name: Qualcomm Technologies, Inc. Eliza (DT)
> ...
> tty_init_dev.part.0 (drivers/tty/tty_io.c:1414 (discriminator 11)) (P)
> tty_open (arch/arm64/include/asm/atomic_ll_sc.h:95 (discriminator 3) drivers/tty/tty_io.c:2073 (discriminator 3) drivers/tty/tty_io.c:2120 (discriminator 3))
> chrdev_open (fs/char_dev.c:411)
> do_dentry_open (fs/open.c:962)
> vfs_open (fs/open.c:1094)
> do_open (fs/namei.c:4634)
> path_openat (fs/namei.c:4793)
> do_filp_open (fs/namei.c:4820)
> do_sys_openat2 (fs/open.c:1391 (discriminator 3))
> ...
> Starting Network Name Resolution...
>
> Apparently the flow with this small Yocto-based ramdisk user-space is:
>
> driver (qcom_geni_serial.c): user-space:
> ============================ ===========
> qcom_geni_serial_probe()
> uart_add_one_port()
> serial_core_register_port()
> serial_core_add_one_port()
> uart_configure_port()
> register_console()
> |
> | open console
> | ...
> | tty_init_dev()
> | driver->ports[idx] is NULL
> |
> tty_port_register_device_attr_serdev()
> tty_port_link_device() <- set driver->ports[idx]
>
> Fixes: bfc467db60b7 ("serial: remove redundant tty_port_link_device()")
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
Thanks for the update.
Reviewed-by: Jiri Slaby <jirislaby@kernel.org>
--
js
suse labs
^ permalink raw reply
* [PATCH 0/6] 8250 DW UART fixes when under constant Rx pressure
From: Ilpo Järvinen @ 2026-01-23 17:27 UTC (permalink / raw)
To: Greg Kroah-Hartman, Jiri Slaby, linux-serial, Andy Shevchenko,
qianfan Zhao, Adriana Nicolae
Cc: linux-kernel, Ilpo Järvinen
Hi all,
Here are fixes to 8250 DW UART conditions that mostly occur in scenarios
under constant Rx pressure which are made complicated by BUSY handling
of DW UARTs (used when !uart_16550_compatible).
A few of the changes touch also 8250_port but it's mostly moving existing
code around (except for the extra synchronize_irq() in shutdown).
I'll do UART_IIR_RX_TIMEOUT move to switch/case separately from this
fix series.
Ilpo Järvinen (6):
serial: 8250: Protect LCR write in shutdown
serial: 8250_dw: Avoid unnecessary LCR writes
serial: 8250_dw: Rework dw8250_handle_irq() locking and IIR handling
serial: 8250_dw: Rework IIR_NO_INT handling to stop interrupt storm
serial: 8250: Add late synchronize_irq() to shutdown to handle DW UART
BUSY
serial: 8250_dw: Ensure BUSY is deasserted
drivers/tty/serial/8250/8250.h | 25 +++
drivers/tty/serial/8250/8250_dw.c | 297 ++++++++++++++++++++++------
drivers/tty/serial/8250/8250_port.c | 69 ++++---
include/linux/serial_8250.h | 1 +
4 files changed, 301 insertions(+), 91 deletions(-)
base-commit: 8f0b4cce4481fb22653697cced8d0d04027cb1e8
--
2.39.5
^ permalink raw reply
* [PATCH 1/6] serial: 8250: Protect LCR write in shutdown
From: Ilpo Järvinen @ 2026-01-23 17:27 UTC (permalink / raw)
To: Greg Kroah-Hartman, Jiri Slaby, linux-serial, Andy Shevchenko,
qianfan Zhao, Adriana Nicolae, linux-kernel
Cc: Ilpo Järvinen, Bandal, Shankar, Murthy, Shanth
In-Reply-To: <20260123172739.13410-1-ilpo.jarvinen@linux.intel.com>
The 8250_dw driver needs to potentially perform very complex operations
during LCR writes because its BUSY handling prevents updates to LCR
while UART is BUSY (which is not fully under our control without those
complex operations). Thus, LCR writes should occur under port's lock.
Move LCR write under port's lock in serial8250_do_shutdown(). Also
split the LCR RMW so that the logic is on a separate line for clarity.
Tested-by: "Bandal, Shankar" <shankar.bandal@intel.com>
Tested-by: "Murthy, Shanth" <shanth.murthy@intel.com>
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
drivers/tty/serial/8250/8250_port.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
index 719faf92aa8a..f7a3c5555204 100644
--- a/drivers/tty/serial/8250/8250_port.c
+++ b/drivers/tty/serial/8250/8250_port.c
@@ -2350,6 +2350,7 @@ static int serial8250_startup(struct uart_port *port)
void serial8250_do_shutdown(struct uart_port *port)
{
struct uart_8250_port *up = up_to_u8250p(port);
+ u32 lcr;
serial8250_rpm_get(up);
/*
@@ -2376,13 +2377,13 @@ void serial8250_do_shutdown(struct uart_port *port)
port->mctrl &= ~TIOCM_OUT2;
serial8250_set_mctrl(port, port->mctrl);
+
+ /* Disable break condition */
+ lcr = serial_port_in(port, UART_LCR);
+ lcr &= ~UART_LCR_SBC;
+ serial_port_out(port, UART_LCR, lcr);
}
- /*
- * Disable break condition and FIFOs
- */
- serial_port_out(port, UART_LCR,
- serial_port_in(port, UART_LCR) & ~UART_LCR_SBC);
serial8250_clear_fifos(up);
rsa_disable(up);
--
2.39.5
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox