On Sun 2025-01-05 02:03:41, John Ogness wrote:
> On 2025-01-03, Petr Mladek <pmladek@xxxxxxxx> wrote:
> >> --- a/drivers/tty/serial/8250/8250_port.c
> >> +++ b/drivers/tty/serial/8250/8250_port.c
> >> @@ -1406,9 +1416,6 @@ void serial8250_em485_stop_tx(struct uart_8250_port *p, bool toggle_ier)
> >> {
> >> unsigned char mcr = serial8250_in_MCR(p);
> >>
> >> - /* Port locked to synchronize UART_IER access against the console. */
> >> - lockdep_assert_held_once(&p->port.lock);
> >
> > We should explain why it is OK to move the assert.
> >
> > IMHO, most poeple would not understand the port lock is needed only
> > for UART_IER manipulation and not for UART_MCR manipulation.
> >
> > We should probably explain that even the UART_MCR manipulation
> > is synchronized either by the port lock or by nbcon context
> > ownership. Where the nbcon context owner ship actually provides
> > synchronization against the port lock in all situations
> > except for the final unsafe flush in panic().
>
> Correct, although the "except for the final unsafe flush in panic()" is
> the reason that even an nbcon context ownership assert could not be used
> here.
Good point. Well, lockdep should be disabled by debug_locks_off()
before nbcon_atomic_flush_unsafe() is called. I hope that we could
keep the assert.
> > A comment might be enough.
>
> OK. I will extend the comment at the new lockdep_assert site explaining
> why the port lock is only guaranteed to be held for the toggle_ier==true
> situation.
Thanks.
> >> + bool console_line_ended; /* line fully output */
> >
> > I wonder if the following is better:
> >
> > bool console_line_ended; /* finished writing full line */
>
> I would prefer to make it more technically exact. It would require a
> multi-line comment and this header uses 2 different styles for
> multi-line field comments.
>
> How about:
>
> /*
> * Track when a console line has been fully written to the
> * hardware, i.e. true when the most recent byte written by
> * the console to UART_TX was '\n'.
> */
> bool console_line_ended;
Looks good to me.
Best Regards,
Petr
