From: Marcos Paulo de Souza <mpdesouza@suse.com>
To: Doug Anderson <dianders@chromium.org>
Cc: Petr Mladek <pmladek@suse.com>,
Steven Rostedt <rostedt@goodmis.org>,
John Ogness <john.ogness@linutronix.de>,
Sergey Senozhatsky <senozhatsky@chromium.org>,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
Jiri Slaby <jirislaby@kernel.org>,
Jason Wessel <jason.wessel@windriver.com>,
Daniel Thompson <danielt@kernel.org>,
Richard Weinberger <richard@nod.at>,
Anton Ivanov <anton.ivanov@cambridgegreys.com>,
Johannes Berg <johannes@sipsolutions.net>,
linux-kernel@vger.kernel.org, linux-serial@vger.kernel.org,
kgdb-bugreport@lists.sourceforge.net,
linux-um@lists.infradead.org
Subject: Re: [PATCH 4/7] drivers: serial: kgdboc: Check CON_SUSPENDED instead of CON_ENABLED
Date: Tue, 10 Jun 2025 17:03:02 -0300 [thread overview]
Message-ID: <f962e9bab3dc8bf5cae1c7e187a54fb96a543d51.camel@suse.com> (raw)
In-Reply-To: <CAD=FV=XXQyZLYKoszj68ZGFDY=9-cmEUp406WeOeSBVZOHyUHw@mail.gmail.com>
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
next prev parent reply other threads:[~2025-06-10 22:49 UTC|newest]
Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-06-07 2:53 [PATCH 0/7] printk cleanup - part 2 Marcos Paulo de Souza
2025-06-07 2:53 ` [PATCH 1/7] printk: Make console_{suspend,resume} handle CON_SUSPENDED Marcos Paulo de Souza
2025-06-12 11:46 ` Petr Mladek
2025-06-23 18:45 ` Marcos Paulo de Souza
2025-06-07 2:53 ` [PATCH 2/7] printk: Use consoles_suspended flag when suspending/resuming all consoles Marcos Paulo de Souza
2025-06-13 15:20 ` Petr Mladek
2025-06-20 14:43 ` John Ogness
2025-06-24 8:40 ` Petr Mladek
2025-06-24 11:04 ` John Ogness
2025-06-25 8:48 ` Petr Mladek
2025-06-23 18:53 ` Marcos Paulo de Souza
2025-06-07 2:53 ` [PATCH 3/7] drivers: tty: Check CON_SUSPENDED instead of CON_ENABLED Marcos Paulo de Souza
2025-06-12 11:48 ` Petr Mladek
2025-06-07 2:53 ` [PATCH 4/7] drivers: serial: kgdboc: " Marcos Paulo de Souza
2025-06-09 20:13 ` Doug Anderson
2025-06-10 20:03 ` Marcos Paulo de Souza [this message]
2025-06-10 23:18 ` Doug Anderson
2025-06-12 13:57 ` Petr Mladek
2025-06-12 23:16 ` Doug Anderson
2025-06-13 10:52 ` Petr Mladek
2025-06-13 16:44 ` Doug Anderson
2025-06-07 2:53 ` [PATCH 5/7] arch: um: kmsg_dump: Don't check for CON_ENABLED Marcos Paulo de Souza
2025-06-16 13:33 ` Petr Mladek
2025-06-20 15:45 ` John Ogness
2025-06-07 2:53 ` [PATCH 6/7] debug: kgd_io: " Marcos Paulo de Souza
2025-06-16 13:56 ` Petr Mladek
2025-06-30 0:31 ` Marcos Paulo de Souza
2025-06-07 2:53 ` [PATCH 7/7] printk: Don't check for CON_ENABLED on console_unblank Marcos Paulo de Souza
2025-06-16 14:02 ` Petr Mladek
2025-06-17 9:32 ` Geert Uytterhoeven
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=f962e9bab3dc8bf5cae1c7e187a54fb96a543d51.camel@suse.com \
--to=mpdesouza@suse.com \
--cc=anton.ivanov@cambridgegreys.com \
--cc=danielt@kernel.org \
--cc=dianders@chromium.org \
--cc=gregkh@linuxfoundation.org \
--cc=jason.wessel@windriver.com \
--cc=jirislaby@kernel.org \
--cc=johannes@sipsolutions.net \
--cc=john.ogness@linutronix.de \
--cc=kgdb-bugreport@lists.sourceforge.net \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-serial@vger.kernel.org \
--cc=linux-um@lists.infradead.org \
--cc=pmladek@suse.com \
--cc=richard@nod.at \
--cc=rostedt@goodmis.org \
--cc=senozhatsky@chromium.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).