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

[John Ogness <john.ogness@linutronix.de>]

On 2026-07-20, Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
>> +	/* irq_work allowed again. Handle MSR now if pending. */
>> +	up->avoid_modem_status_work = false;
>> +	guard(uart_port_lock_irqsave)(port);
>
> To make guard()() visible, we usually add a blank line before and after
> guard()() line. I dunno if scoped_guard() makes more sense here as Petr
> proposed, I'm fine with either.

Ack.

>> +static void serial8250_console_byte_write(struct uart_8250_port *up,
>> +					  struct nbcon_write_context *wctxt)
>> +{
>> +	struct uart_port *port = &up->port;
>> +	const char *s = wctxt->outbuf;
>> +	const char *end = s + wctxt->len;
>> +
>> +	/*
>> +	 * Write out the message. If a handover or takeover occurs, writing
>> +	 * must be aborted since wctxt->outbuf and wctxt->len are no longer
>> +	 * valid.
>> +	 */
>> +	while (s != end) {
>
> Can len == 0? If not, I would write this as do {} while ().

printk should never make such requests (len=0 means it is a skipped
record) but the driver should not rely on that. Also, this pattern is
the same used by serial8250_console_fifo_write()

>> +/*
>> + * irq_work handler to perform modem control. Only triggered via
>> + * ->write_atomic() callback because it may be in a scheduler or
>> + * NMI context, unable to wake tasks.
>> + */
>> +static void modem_status_handler(struct irq_work *iwp)
>> +{
>> +	struct uart_8250_port *up = container_of(iwp, struct uart_8250_port, modem_status_work);
>> +	struct uart_port *port = &up->port;
>> +
>> +	guard(uart_port_lock)(port);
>
> + blank line.

Ack.

John