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

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

[Jon Hunter]

Hi John,

On 24/07/2026 11:36, 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>


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 the 
only interesting thing I have observed is that if I add 'keep_bootcon' 
to the command line the board does boot.

Jon

-- 
nvpublic

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

[Andy Shevchenko]

On Tue, Aug 18, 2026 at 06:32:12AM +0100, Jon Hunter wrote:
> On 24/07/2026 11:36, 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.
> 
> 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.

Wasn't v11 applied and not v9?

> So far the only
> interesting thing I have observed is that if I add 'keep_bootcon' to the
> command line the board does boot.

keep_bootcon defers driver taking over, that's why it works, it uses just
simple primitives instead of full-featured driver.

-- 
With Best Regards,
Andy Shevchenko

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

[Jon Hunter]

On 18/08/2026 07:14, Andy Shevchenko wrote:
> On Tue, Aug 18, 2026 at 06:32:12AM +0100, Jon Hunter wrote:
>> On 24/07/2026 11:36, 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.
>>
>> 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.
> 
> Wasn't v11 applied and not v9?

Ah may be I replied to the wrong version. However, I am testing 
next-20260817. So whatever is in -next.
>> So far the only
>> interesting thing I have observed is that if I add 'keep_bootcon' to the
>> command line the board does boot.
> 
> keep_bootcon defers driver taking over, that's why it works, it uses just
> simple primitives instead of full-featured driver.

Yes that makes sense then.
Jon

-- 
nvpublic

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

[Petr Mladek]

On Tue 2026-08-18 09:14:51, Andy Shevchenko wrote:
> On Tue, Aug 18, 2026 at 06:32:12AM +0100, Jon Hunter wrote:
> > On 24/07/2026 11:36, 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.
> > 
> > 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.
> 
> Wasn't v11 applied and not v9?
> 
> > So far the only
> > interesting thing I have observed is that if I add 'keep_bootcon' to the
> > command line the board does boot.
> 
> keep_bootcon defers driver taking over, that's why it works, it uses just
> simple primitives instead of full-featured driver.

It is the other way. The full-featured driver is used even with
keep_bootcon. The option causes that the simple boot driver stays
registered in parallel. You probably see duplicated lines because
of this.

Anyway, the most important effect of "keep_bootcon" is that
boot consoles prevent using the printk kthreads. Even the nbcon
consoles need to be handled in the legacy loop.

There reason is a combination of two things:

  1. Boot consoles are synchronized only using the legacy
     console_lock (console_sem). port->lock does not exist
     at boot time.

  2. There is no simple way how to match boot console drivers
     with the full-featured drivers. They use a different interface
     to access the same HW.

As a result, even the full featured console drivers have to rely
on the legacy console_lock as long as any boot console is used.

The regression seems to be when the uart 8250 port become handled
by the printk kthread.

Best Regards,
Petr

PS: I am not sure whether to continue the discussion here.
    It might be better to move it to v11. see
    https://lore.kernel.org/all/20260729120439.281252-1-john.ogness@linutronix.de/
    I am going to reply there as well.