Re: [PATCH next v1 1/2] serial: 8250: Switch to nbcon console

[Andy Shevchenko]

On Fri, Sep 06, 2024 at 06:44:41PM +0206, John Ogness wrote:
> On 2024-09-06, John Ogness <john.ogness@xxxxxxxxxxxxx> wrote:
> >> Wait! This makes the rs485 consoles much less usable for debugging.
> >> They might have troubles to see the emergency and panic messages.
> >>
> >> Is this acceptable? Why?
> >
> > It is not acceptable. I am looking into making the atomic part work for
> > RS485 as well.
> 
> So there are 2 things _not_ supported by the write_atomic() callback:
> 
> 1. RS485 mode. This is due to the need to start up TX for the
> write, which can lead to:
> 
> up->rs485_start_tx()
>   serial8250_em485_start_tx()
>     serial8250_stop_rx()
>       serial8250_rpm_get()
>         pm_runtime_get_sync()
>           __pm_runtime_resume()
>             spin_lock_irqsave()
> 
> Taking a spin lock is not safe from NMI and thus disqualifies this call
> chain for write_atomic().
> 
> If UART_CAP_RPM is not set, the pm_runtime_get_sync() is avoided. So I
> could only disable atomic RS485 if UART_CAP_RPM is set. But the OMAP
> variant of the 8250 does set this capability.

Please, don't add a new code which relies on UART_CAP_RPM.
The idea is to enable runtime PM for all users who provides respective
callbacks. Rather, you should ask runtime PM for this information.

> 2. Modem control. This is due to waiting for inputs, which can lead to:
> 
> serial8250_modem_status()
>   wake_up_interruptible()
> 
> Performing wakes is not safe from scheduler or NMI and thus disqualifies
> this call chain for write_atomic().
> 
> It would probably be acceptable to move serial8250_modem_status() into
> an irq_work.
> 
> I would be grateful for any insights on how best to handle these 2
> issues if we want full write_atomic() support for all 8250 variants.

-- 
With Best Regards,
Andy Shevchenko