Re: [PATCH 4/7] drivers: serial: kgdboc: Check CON_SUSPENDED instead of CON_ENABLED

Marcos Paulo de Souza
2025-06-10T20:03:11Z

On Mon, 2025-06-09 at 13:13 -0700, Doug Anderson wrote:
> Hi,
>
> On Fri, Jun 6, 2025 at 7:54 PM Marcos Paulo de Souza
> <mpdesouza@suse.com> wrote:
> >
> > All consoles found on for_each_console are registered, meaning that
> > all of
> > them are CON_ENABLED. The code tries to find an active console, so
> > check if the
> > console is not suspended instead.
> >
> > Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com>
> > ---
> >  drivers/tty/serial/kgdboc.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/tty/serial/kgdboc.c
> > b/drivers/tty/serial/kgdboc.c
> > index
> > 85f6c5a76e0fff556f86f0d45ebc5aadf5b191e8..af6d2208b8ddb82d62f33292b
> > 006b2923583a0d2 100644
> > --- a/drivers/tty/serial/kgdboc.c
> > +++ b/drivers/tty/serial/kgdboc.c
> > @@ -577,7 +577,8 @@ static int __init kgdboc_earlycon_init(char
> > *opt)
> >         console_list_lock();
> >         for_each_console(con) {
> >                 if (con->write && con->read &&
> > -                   (con->flags & (CON_BOOT | CON_ENABLED)) &&
> > +                   (con->flags & CON_BOOT) &&
> > +                   ((con->flags & CON_SUSPENDED) == 0) &&
>
> I haven't tried running the code, so I could easily be mistaken,
> but...
>
> ...the above doesn't seem like the correct conversion. The old
> expression was:
>
> (con->flags & (CON_BOOT | CON_ENABLED))
>
> That would evaluate to non-zero (true) if the console was _either_
> "boot" or "enabled".
>
> The new expression is is:
>
> (con->flags & CON_BOOT) && ((con->flags & CON_SUSPENDED) == 0)
>
> That's only true if the console is _both_ "boot" and "not suspended".

My idea here was that the users of for_each_console would find the
first available console, and by available I would expect them to be
usable. In this case, is there any value for kgdboc to use a console
that is suspended? Would it work in this case?

I never really used kgdboc, but only checking if the console was
enabled (which it's always the case here) was something that needed to
be fixed.

Maybe I'm missing something here as well, so please let me know if I
should remove the new check.

>
> -Doug