Re: [PATCH tty v11 1/2] serial: 8250: Switch to nbcon console, take 2

Re: [PATCH tty v11 1/2] serial: 8250: Switch to nbcon console, take 2

[Jon Hunter]

Hi John,

On 29/07/2026 13:04, John Ogness wrote:
> Implement the necessary callbacks to switch the 8250 console driver
> to perform as an nbcon console.
> 
> Add implementations for the nbcon console callbacks:
> 
>    ->write_atomic()
>    ->write_thread()
>    ->device_lock()
>    ->device_unlock()
> 
> and add CON_NBCON to the initial @flags.
> 
> All hardware access in the callbacks is within unsafe sections.
> The ->write_atomic() and ->write_thread() callbacks allow safe
> handover/takeover per byte and add a preceding newline if they
> take over from another context mid-line.
> 
> For the ->write_atomic() callback, a new irq_work is used to defer
> modem control since it may be called from a context that does not
> allow waking up tasks. During suspend/resume the irq_work is not
> used as this has been shown to cause suspend problems for some
> hardware. Upon resume, any pending modem control is performed.
> 
> Note: A new __serial8250_clear_IER() is introduced for direct
> clearing of UART_IER during console writing (which will not be
> holding the port lock for atomic printing or KDB/KGDB). This
> allows restoring a lockdep check to serial8250_clear_IER() in
> a follow-up commit.
> 
> Signed-off-by: John Ogness <john.ogness@linutronix.de>

I accidently responded to V9, but responding here with the same report 
for completeness.

This change is causing a boot regression for our Tegra20 and Tegra30 
platforms. Reverting this on top of -next fixes the issue. Previously 
with V5 I did not see a boot issue only an issue in suspend. So far I 
have not had chance to dig any further.

Jon

-- 
nvpublic

Re: [PATCH tty v11 1/2] serial: 8250: Switch to nbcon console, take 2

[Sebastian Andrzej Siewior]

On 2026-08-18 07:25:13 [+0100], Jon Hunter wrote:
> Hi John,
Hi,

> This change is causing a boot regression for our Tegra20 and Tegra30
> platforms. Reverting this on top of -next fixes the issue. Previously with
> V5 I did not see a boot issue only an issue in suspend. So far I have not
> had chance to dig any further.

If my counting is correct: v5 is from last year
	https://lore.kernel.org/all/20250107212702.169493-1-john.ogness@linutronix.de/

and part of v6.14 however the nbcon bits itself were reverted before the
final release. That is why v6 till the applied v11 has only two
patches.

> Jon

Sebastian

Re: [PATCH tty v11 1/2] serial: 8250: Switch to nbcon console, take 2

[Petr Mladek]

On Tue 2026-08-18 07:25:13, Jon Hunter wrote:
> Hi John,
> 
> On 29/07/2026 13:04, John Ogness wrote:
> > Implement the necessary callbacks to switch the 8250 console driver
> > to perform as an nbcon console.
> > 
> > Add implementations for the nbcon console callbacks:
> > 
> >    ->write_atomic()
> >    ->write_thread()
> >    ->device_lock()
> >    ->device_unlock()
> > 
> > and add CON_NBCON to the initial @flags.
> > 
> > All hardware access in the callbacks is within unsafe sections.
> > The ->write_atomic() and ->write_thread() callbacks allow safe
> > handover/takeover per byte and add a preceding newline if they
> > take over from another context mid-line.
> > 
> > For the ->write_atomic() callback, a new irq_work is used to defer
> > modem control since it may be called from a context that does not
> > allow waking up tasks. During suspend/resume the irq_work is not
> > used as this has been shown to cause suspend problems for some
> > hardware. Upon resume, any pending modem control is performed.
> > 
> > Note: A new __serial8250_clear_IER() is introduced for direct
> > clearing of UART_IER during console writing (which will not be
> > holding the port lock for atomic printing or KDB/KGDB). This
> > allows restoring a lockdep check to serial8250_clear_IER() in
> > a follow-up commit.
> > 
> > Signed-off-by: John Ogness <john.ogness@linutronix.de>
> 
> I accidently responded to V9, but responding here with the same report for
> completeness.
> 
> This change is causing a boot regression for our Tegra20 and Tegra30
> platforms. Reverting this on top of -next fixes the issue. Previously with
> V5 I did not see a boot issue only an issue in suspend. So far I have not
> had chance to dig any further.

Interesting.

Another clue, mentioned in the v9 thread [1], is that the boot
regression does not happen with v11 when "keep_bootcon" option
is used.

The "keep_bootcon" option causes that the boot console driver stays
registered even when the full featured driver gets registered
later.

The most important effect is that the printk kthreads can't
be used as long as any boot console driver is registered.
All drivers need to be called in the legacy loop in this case.
There are two reasons for this:

  1. Boot console drivers are synchronized only by
     the legacy console_lock (console_sem). port->lock
     is available only for the full featured driver.

  2. There is no easy way to match boot console and
     full featured console drivers working on the same
     HW.

So, the regression seems to happen when the printk kthreads
start being used.

Jon, could you please share the full log when "keep_bootcon"
is used?

[1] https://lore.kernel.org/all/20260724103654.133654-1-john.ogness@linutronix.de/

Best Regards,
Petr

Re: [PATCH tty v11 1/2] serial: 8250: Switch to nbcon console, take 2

[Jon Hunter]

Hi Petr,

On 18/08/2026 08:51, Petr Mladek wrote:

...

>> This change is causing a boot regression for our Tegra20 and Tegra30
>> platforms. Reverting this on top of -next fixes the issue. Previously with
>> V5 I did not see a boot issue only an issue in suspend. So far I have not
>> had chance to dig any further.
> 
> Interesting.
> 
> Another clue, mentioned in the v9 thread [1], is that the boot
> regression does not happen with v11 when "keep_bootcon" option
> is used.
> 
> The "keep_bootcon" option causes that the boot console driver stays
> registered even when the full featured driver gets registered
> later.
> 
> The most important effect is that the printk kthreads can't
> be used as long as any boot console driver is registered.
> All drivers need to be called in the legacy loop in this case.
> There are two reasons for this:
> 
>    1. Boot console drivers are synchronized only by
>       the legacy console_lock (console_sem). port->lock
>       is available only for the full featured driver.
> 
>    2. There is no easy way to match boot console and
>       full featured console drivers working on the same
>       HW.
> 
> So, the regression seems to happen when the printk kthreads
> start being used.
> 
> Jon, could you please share the full log when "keep_bootcon"
> is used?


Yes absolutely. You can find the boot log here [0]. So far nothing 
really stands out to me but let me know if you see anything.

Jon

[0] https://pastebin.com/FhQVSqfy

-- 
nvpublic

Re: [PATCH tty v11 1/2] serial: 8250: Switch to nbcon console, take 2

[Petr Mladek]

On Tue 2026-08-18 13:19:12, Jon Hunter wrote:
> Hi Petr,
> 
> On 18/08/2026 08:51, Petr Mladek wrote:
> 
> ...
> 
> > > This change is causing a boot regression for our Tegra20 and Tegra30
> > > platforms. Reverting this on top of -next fixes the issue. Previously with
> > > V5 I did not see a boot issue only an issue in suspend. So far I have not
> > > had chance to dig any further.
> > 
> > Interesting.
> > 
> > Another clue, mentioned in the v9 thread [1], is that the boot
> > regression does not happen with v11 when "keep_bootcon" option
> > is used.
> > 
> > The "keep_bootcon" option causes that the boot console driver stays
> > registered even when the full featured driver gets registered
> > later.
> > 
> > The most important effect is that the printk kthreads can't
> > be used as long as any boot console driver is registered.
> > All drivers need to be called in the legacy loop in this case.
> > There are two reasons for this:
> > 
> >    1. Boot console drivers are synchronized only by
> >       the legacy console_lock (console_sem). port->lock
> >       is available only for the full featured driver.
> > 
> >    2. There is no easy way to match boot console and
> >       full featured console drivers working on the same
> >       HW.
> > 
> > So, the regression seems to happen when the printk kthreads
> > start being used.
> > 
> > Jon, could you please share the full log when "keep_bootcon"
> > is used?
> 
> 
> Yes absolutely. You can find the boot log here [0]. So far nothing really
> stands out to me but let me know if you see anything.
> 
> [0] https://pastebin.com/FhQVSqfy

Thanks for the log.

One or two things look strange/important to me.
But let me show all important parts:

[    0.000000] Kernel command line: console=ttyS0,115200n8 console=tty1 earlycon ignore_loglevel root=/dev/nfs rw ip=192.168.99.2:192.168.99.1:192.168.99.1:255.255.255.0::eth0:off nfsroot=192.168.99.1:/home/ausvrl81292/nfsroot,tcp rootwait keep_bootcon

The last "console=" parameter is "console=tty1". It is a so called
preferred console. It has several effects:

   + it should get associated with /dev/console

   + it does not replay the log from the beginning when
     registered. Only newer messages are shown.

   + Boot consoles should get unregistered when this console
     gets registered (unless keep_bootcon is defined).

Note that "ttyS0" is _not_ the _preferred_console. As a result:

   + it will replay all messages when registered

   + boot console won't get unregistered when this one
     is registered


Now, the ordering is:

[    0.000000] earlycon: uart0 MMIO:0x70006300 (options '115200n8')
[    0.000000] printk: legacy bootconsole [uart0] enabled

   First, earlycon is registered thanks because of the "earlycon"
   parameter.

[    0.036461] Console: colour dummy device 80x30
[    0.041020] printk: legacy console [tty1] enabled

   Second, the graphical "tty1" gets registered because
   of the "console=tty1" parameter.

   Normally, the boot console should get unregistered at
   this point. But it stays because of the "keep_bootcon"
   parameter.

[    0.645646] Serial: 8250/16550 driver, 4 ports, IRQ sharing disabled
[    0.655230] printk: console [ttyS0] disabled

   IMPORTANT: This is the weird thing! I do not understand why "ttySO"
	gets disabled when it has not been registered yet.

[    0.659964] 70006300.serial: ttyS0 MMIO32:0x70006300 (irq = 51, base_baud = 13500000) is a Tegra
[    0.000000] Booting Linux on physical CPU 0x0
[    0.669026] printk: console [ttyS0] enabled

   The real "ttyS0" driver has been registered (added to
   console_list) and the legacy loop started flushing
   the messages in console_unlock().

   The real "ttyS0" console driver started emitting messages
   from the beginning.

   The boot console driver emits only the newly added messages
   "printk: console [ttyS0] enabled".

[    0.000000] Linux version 7.2.0-next-20260817 (jonathanh@build-jonathanh-noble-20260527) (arm-buildroot-linux-gnueabihf-gcc.br_real (Buildroot 2021.11-11272-ge2962af) 13.2.0, GNU ld (GNU Binutils) 2.42) #15 SMP PREEMPT Tue Aug 18 04:56:56 UTC 2026
[    0.000000] CPU: ARMv7 Processor [411fc090] revision 0 (ARMv7), cr=10c5387d

   The real "ttyS0" console driver replays the entire log.

[    0.659964] 70006300.serial: ttyS0 MMIO32:0x70006300 (irq = 51, base_baud = 13500000) is a Tegra
[    0.669026] printk: console [ttyS0] enabled


   And then all messages are emitted twice (by the boot console
   driver and by the real console driver:

[    1.574761] loop: module loaded
[    1.574761] loop: module loaded
[    1.584820] CAN device driver interface
[    1.584820] CAN device driver interface

   The real console driver would normally emit these messages
   from the printk kthread. But it does it in the legacy loop
   because the boot console driver is still registered.

[    2.904178] ------------[ cut here ]------------
[    2.904178] ------------[ cut here ]------------
[    2.913507] WARNING: drivers/soc/tegra/pmc.c:4999 at tegra_pmc_enter_suspend_mode+0x168/0x178, CPU#0: swapper/0/0
[    2.913507] WARNING: drivers/soc/tegra/pmc.c:4999 at tegra_pmc_enter_suspend_mode+0x168/0x178, CPU#0: swapper/0/0

  IMPORTANT: This is a warning. It is printed with
	NBCON_PRIO_EMERGENCY. These messages would normally
	get flushed by nbcon_atomic_flush_pending() directly
	from printk() using con->write_atomic(). It would
	take over the console ownership from the kthread
	when needed.

	In this particular log, it is emitted from the legacy
	loop in console_unlock() because the boot console
	is still registered.


Summary:

Almost everything works as expected except for:

  1. I am not sure why "printk: console [ttyS0] disabled" is printed.
     It does not make any sense to me.

  2. The WARNING would be handled with NBCON_PRIO_EMERGENCY.
     I wonder if this warning happened also with "v5" of this
     patchset.

Why is the WARNING important?

   If the WARNING happened also with v5 of this patchset
   then it tested emergency mode as well. But the system
   booted with v5. So that a difference between v5 and v11
   patchset might be important.

   If The WARNING did _not_ happen with v5 then we probably did
   not test the emergency mode in this version. So that
   the problem might be in the emergency mode handling.



Ideas for testing:

1. I wonder if adding a WARN() with v5 of this patchset
   would make v5 fail as well.

2. If wonder if boot_delay=10 makes any difference. It might
   prevent some races.

Best Regards,
Petr

Re: [PATCH tty v11 1/2] serial: 8250: Switch to nbcon console, take 2

[Jon Hunter]

Hi Petr,

On 18/08/2026 16:53, Petr Mladek wrote:

...

> Summary:
> 
> Almost everything works as expected except for:
> 
>    1. I am not sure why "printk: console [ttyS0] disabled" is printed.
>       It does not make any sense to me.
> 
>    2. The WARNING would be handled with NBCON_PRIO_EMERGENCY.
>       I wonder if this warning happened also with "v5" of this
>       patchset.
> 
> Why is the WARNING important?
> 
>     If the WARNING happened also with v5 of this patchset
>     then it tested emergency mode as well. But the system
>     booted with v5. So that a difference between v5 and v11
>     patchset might be important.
> 
>     If The WARNING did _not_ happen with v5 then we probably did
>     not test the emergency mode in this version. So that
>     the problem might be in the emergency mode handling.
> 
> 
> 
> Ideas for testing:
> 
> 1. I wonder if adding a WARN() with v5 of this patchset
>     would make v5 fail as well.
Thanks for the analysis. I don't think that WARN occurred back in V5 
because this is something I believe is fairly recent and we need to fix. 
Anyway, for testing, I simply removed the warning to see if this has an 
impact and it does. So you are right, this appears to be the cause of 
the issue.

It would be interesting to know if anyone else sees this if an WARNING 
is inserted somewhere during boot.

Cheers
Jon	

-- 
nvpublic

Re: [PATCH tty v11 1/2] serial: 8250: Switch to nbcon console, take 2

[John Ogness]

On 2026-08-18, Petr Mladek <pmladek@suse.com> wrote:
>> [0] https://pastebin.com/FhQVSqfy

...

> Almost everything works as expected except for:
>
>   1. I am not sure why "printk: console [ttyS0] disabled" is printed.
>      It does not make any sense to me.

I have seen this in the past. It can happens unregister_console_locked()
has always printed the disabled message before actually checking if the
console is registered.

IIRC it has something to do with serial8250_register_8250_port() calling
uart_remove_one_port(). I remember starting to look into how to fix it
but then was going down a rabbit hole that exceeded my scope at the
time.

>   2. The WARNING would be handled with NBCON_PRIO_EMERGENCY.
>      I wonder if this warning happened also with "v5" of this
>      patchset.

The tegra issue we had with v5 was related to irqwork being scheduled
when suspending. When keep_bootcon is specified, there will be no
irqwork scheduled during boot because legacy_direct=true.

From the backtrace in the posted boot log we see that the warning is
within tegra_pmc_enter_suspend_mode(). I am wondering if this is the
same issue we saw with v5... an irqwork getting scheduled when the tegra
is entering some suspend state.

Back then I was able to reproduce the suspend issue on a Phytec-WEGA
board. I will see if I can reproduce this report on it as well.

John

Re: [PATCH tty v11 1/2] serial: 8250: Switch to nbcon console, take 2

[John Ogness]

Hi Jon,

On 2026-08-20, Jon Hunter <jonathanh@nvidia.com> wrote:
>> 1. I wonder if adding a WARN() with v5 of this patchset
>>     would make v5 fail as well.
>
> Thanks for the analysis. I don't think that WARN occurred back in V5 
> because this is something I believe is fairly recent and we need to fix. 
> Anyway, for testing, I simply removed the warning to see if this has an 
> impact and it does. So you are right, this appears to be the cause of 
> the issue.

Instead of the WARN, could you insert a pr_err() call? I would like to
remove the EMERGENCY printing from the picture if we can. If it hangs
due to NORMAL console printing from that suspend function, I expect it
is irqwork related.

John

Re: [PATCH tty v11 1/2] serial: 8250: Switch to nbcon console, take 2

[Jon Hunter]

Hi John,

On 20/08/2026 13:40, John Ogness wrote:
> Hi Jon,
> 
> On 2026-08-20, Jon Hunter <jonathanh@nvidia.com> wrote:
>>> 1. I wonder if adding a WARN() with v5 of this patchset
>>>      would make v5 fail as well.
>>
>> Thanks for the analysis. I don't think that WARN occurred back in V5
>> because this is something I believe is fairly recent and we need to fix.
>> Anyway, for testing, I simply removed the warning to see if this has an
>> impact and it does. So you are right, this appears to be the cause of
>> the issue.
> 
> Instead of the WARN, could you insert a pr_err() call? I would like to
> remove the EMERGENCY printing from the picture if we can. If it hangs
> due to NORMAL console printing from that suspend function, I expect it
> is irqwork related.
I replaced the WARN with a pr_err() and I see the same issue.

Thanks
Jon

-- 
nvpublic

Re: [PATCH tty v11 1/2] serial: 8250: Switch to nbcon console, take 2

[John Ogness]

On 2026-08-21, Jon Hunter <jonathanh@nvidia.com> wrote:
> I replaced the WARN with a pr_err() and I see the same issue.

Thanks for testing. But just to be certain, can you restore the original
code (with the warning) and apply this horrible hack to force atomic
printing? I want to be certain that the problem goes away when nbcon
deferred printing is not active. For thet test you should boot normally
(without keep_bootcon).

===== BEGIN HACK =====
diff --git a/kernel/printk/internal.h b/kernel/printk/internal.h
index 85fbf1801cbe0..5806815fa11b2 100644
--- a/kernel/printk/internal.h
+++ b/kernel/printk/internal.h
@@ -202,6 +202,7 @@ static inline void printk_get_console_flush_type(struct console_flush_type *ft)
 				ft->nbcon_offload = true;
 			else
 				ft->nbcon_atomic = true;
+			ft->nbcon_atomic = true;
 		}
 
 		/* Legacy consoles are flushed directly when possible. */
diff --git a/kernel/printk/nbcon.c b/kernel/printk/nbcon.c
index 4b03b019cd5ee..152d9ac28176d 100644
--- a/kernel/printk/nbcon.c
+++ b/kernel/printk/nbcon.c
@@ -1185,6 +1185,7 @@ static bool nbcon_kthread_should_wakeup(struct console *con, struct nbcon_contex
 
 	if (kthread_should_stop())
 		return true;
+	return false;
 
 	/*
 	 * Block the kthread when the system is in an emergency or panic mode.
===== END HACK =====

I am expecting that this will work and the warning will be cleanly
visible on the UART console.

If the above test was successful, I am wondering if cpuidle should be
disabling the deferred printing as this seems quite similar to suspend.

Could you perform another test using the following hack _instead_ of the
above hack? This is probably a bit extreme since it does more than just
disable deferring, but it could help us figure out what is going on.

===== BEGIN HACK2 =====
diff --git a/drivers/cpuidle/cpuidle-tegra.c b/drivers/cpuidle/cpuidle-tegra.c
index aca907a62bb5d..0fb449e23f677 100644
--- a/drivers/cpuidle/cpuidle-tegra.c
+++ b/drivers/cpuidle/cpuidle-tegra.c
@@ -15,6 +15,7 @@
 #define pr_fmt(fmt)	"tegra-cpuidle: " fmt
 
 #include <linux/atomic.h>
+#include <linux/console.h>
 #include <linux/cpuidle.h>
 #include <linux/cpumask.h>
 #include <linux/cpu_pm.h>
@@ -165,6 +166,8 @@ static __cpuidle int tegra_cpuidle_state_enter(struct cpuidle_device *dev,
 {
 	int err;
 
+	console_suspend_all();
+
 	/*
 	 * CC6 state is the "CPU cluster power-off" state.  In order to
 	 * enter this state, at first the secondary CPU cores need to be
@@ -176,7 +179,7 @@ static __cpuidle int tegra_cpuidle_state_enter(struct cpuidle_device *dev,
 	if (index == TEGRA_CC6) {
 		err = tegra_cpuidle_coupled_barrier(dev);
 		if (err)
-			return err;
+			goto out;
 	}
 
 	local_fiq_disable();
@@ -204,6 +207,8 @@ static __cpuidle int tegra_cpuidle_state_enter(struct cpuidle_device *dev,
 	cpu_pm_exit();
 	tegra_pm_clear_cpu_in_lp2();
 	local_fiq_enable();
+out:
+	console_resume_all();
 
 	return err ?: index;
 }
diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index 3fcdf4b4e2e53..8d126b5068684 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -2787,7 +2787,7 @@ void console_suspend_all(void)
 	struct console *con;
 
 	if (console_suspend_enabled)
-		pr_info("Suspending console(s) (use no_console_suspend to debug)\n");
+		pr_info_once("Suspending console(s) (use no_console_suspend to debug)\n");
 
 	/*
 	 * Flush any console backlog and then avoid queueing irq_work until
===== END HACK2 =====

John Ogness