The Linux Kernel Mailing List
 help / color / mirror / Atom feed
* [PATCH v4 00/11]  printk: Clean up preferred console handling
@ 2026-06-04 10:14 Petr Mladek
  2026-06-04 10:14 ` [PATCH v4 01/11] printk: Handle pre-enabled consoles in the top-level register_console() Petr Mladek
                   ` (11 more replies)
  0 siblings, 12 replies; 19+ messages in thread
From: Petr Mladek @ 2026-06-04 10:14 UTC (permalink / raw)
  To: John Ogness
  Cc: Sergey Senozhatsky, Steven Rostedt, Marcos Paulo de Souza,
	Chris Down, Naveen Kumar Chaudhary, linux-kernel, Petr Mladek

this patchset does some clean up of the code for handling preferred
consoles in the console registration code.

  + 1st and 2nd patch try to improve some naming.

  + 3rd patch removes some code duplication. It better defines and
    describes the rules for adding and updating preferred consoles.
    It uses a more defensive coding style.

  + 4-8th patch improve handling of Braille consoles.
    They are preferred via the command line but they do not get
    printk() messages and are not associated with /dev/console.

    The new code makes this more obvious. Also it explicitly
    defines the relation against default consoles and other
    non-Braille preferred consoles.

  + 9th patch removes a hidden side effect of
    try_enable_preferred_console()

  + 10th patch prevents adding more side effectes to
    try_enable_preferred_console(). It allows to call it
    only when there are any non-Braille preferred consoles.

  + 11th patch allows to catch an error when enabling a console
    driver by default.

This patchset is a prerequisite for Marcos' clean up of CON_ENABLE
flag handling. It should prevent regressions caused by the
hidden effects of try_enable_preferred_console(), for example,
see https://lore.kernel.org/r/89409a0f48e6998ff6dd2245691b9954f0e1e435.camel@suse.com

Also I am working on a feature which would allow to explicitly
enable/prefer consoles proposed by SPCR, device tree, or
platform-specific code using a generic "console=platform".
This clean up is a prerequisite, see
https://github.com/pmladek/linux/tree/console-platform-poc1-iter9

Changes against [v3]:

  + Move the handling of pre-enabled consoles into register_console()
    as 1st patch. It prevents a possible invalid access. [Petr]

  + Move the code for handling pre-enabled to console to
    try_enable_console() in 7th patch. The resulting code
    is the same as in v3.

    This 2nd move was not needed in v3 because the code
    was moved to this final location later. [Petr]

  + Switch order of the 10th and 11th patch. Aka make
    try_enable_default_console() return an err before
    refactoring try_enable_console() call
    try_enable_preferred_console() only when there is any.
    [Petr]

  + Initialize err value in try_enable_console() in 11th patch.
    It prevents using a non-initialized variable. [AI:gemini-3]

  + Make sure that pre-enabled consoles won't get registered
    when something went wrong in try_enable_console()
    in 11th patch [Petr]


Changes against [v2]:

  + Move static_assert() after declaration (1st patch) [Steven]

  + Rename update_preferred_console() to add_or_update_preferred_console() [John]
  + WARN_ON() when called without "name" and "devname" [John]
  + print error when reached MAX_PREFERRED_CONSOLES
  + Add comment above code adding new entries [John]
    (all in 4th patch)

  + Call console->setup() under console_lock() in braille_register_console()
    (new patch) [Sashiko]

  + Use "No functional changes." phrase in commit messages. [Steven]

  + Call try_enable_preferred_console() only when there is no
    preferred console (reworked and split 9th patch) [Petr]

  + Added taken Reviewed-by and Acked-by tags [John, Macros]


Changes against [v1]:

  + Fix stale comment (1st patch, Chris)

  + Add comment descibing update_preferred_console() behavior (3rd patch, John)

  + Better describe behavior changes caused by update_preferred_console()
    in the commit message (3rd patch, Chris)

  + Use unsigned int for iterating/indexing preferred_consoles[]
    table (3rd patch, John)

  + Add a check preventing an update via "devname" is already assigned
    to a real "name". (3rd patch, Petr)

  + Use only Braille entries when trying to enable Braille console (Chris)

  + Split all try_enable_*console() calls into new try_enable_console()
    and return early when try_enable_braille_console() succeeded.
    Note that we do not longer need to initialize "err" variable
    in this case. (new 5th patch, Chris, Petr)

  + Put back the explicit check of CON_BRL flag in register_console()
    so that it returns prelimitary only when the driver is used in Braille
    mode. (6th patch, Chris, Petr)

  + Add comments explaining that try_enable_braille_console() might succeeded
    also when newcon->match() succeeded. In this case, the Braille mode can't
    be used because the console device was already used as an early console.[*]
    (6th patch, Chris, Petr)

  + Reset preferred_dev_console_prev when used to revert the value.
    (7th patch, John)

  + Add Acked-by and Reviewed-by tags (1st, 2nd, 4th patch, Chris, Marcos)

[*] This is a questional behavior. I would personally prefer when
    the early console is unregistered at this point and the console
    device is converted into Braille console. I consider the Braille
    mode more important. But it might be changed/fixed later
    in another patchset.


[v1] https://lore.kernel.org/r/20260206165002.496724-1-pmladek@suse.com
[v2] https://lore.kernel.org/r/20260423130015.85175-1-pmladek@suse.com
[v3] https://lore.kernel.org/r/20260602085312.228251-1-pmladek@suse.com

Petr Mladek (11):
  printk: Handle pre-enabled consoles in the top-level
    register_console()
  printk: Rename struct console_cmdline to preferred_console
  printk: Rename preferred_console to preferred_dev_console
  printk: Separate code for adding/updating preferred console metadata
  printk: Cleanup _braille_(un)register_console() wrappers
  console/braille: Lock console->setup() call during the registration
  printk: Separate code for enabling console
  printk: Try to register each console as Braille first
  printk: Do not set Braille console as preferred_console
  printk: Modify try_enable_default_console() to return error/success
  printk: Try enable preferred consoles only when there are any

 .../accessibility/braille/braille_console.c   |   9 +-
 kernel/printk/braille.c                       |  20 +-
 kernel/printk/braille.h                       |  23 +-
 .../{console_cmdline.h => console_register.h} |   6 +-
 kernel/printk/printk.c                        | 402 ++++++++++++------
 5 files changed, 315 insertions(+), 145 deletions(-)
 rename kernel/printk/{console_cmdline.h => console_register.h} (83%)

-- 
2.54.0


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH v4 01/11] printk: Handle pre-enabled consoles in the top-level register_console()
  2026-06-04 10:14 [PATCH v4 00/11] printk: Clean up preferred console handling Petr Mladek
@ 2026-06-04 10:14 ` Petr Mladek
  2026-07-14 13:33   ` John Ogness
  2026-06-04 10:14 ` [PATCH v4 02/11] printk: Rename struct console_cmdline to preferred_console Petr Mladek
                   ` (10 subsequent siblings)
  11 siblings, 1 reply; 19+ messages in thread
From: Petr Mladek @ 2026-06-04 10:14 UTC (permalink / raw)
  To: John Ogness
  Cc: Sergey Senozhatsky, Steven Rostedt, Marcos Paulo de Souza,
	Chris Down, Naveen Kumar Chaudhary, linux-kernel, Petr Mladek

The function try_enable_preferred_console() currently has the
non-obvious side effect of returning success for consoles that are
already pre-enabled. This obscures the logic flow during console
registration.

Move the check for pre-enabled consoles directly into the top-level
register_console(). This change makes the handling of pre-enabled
consoles explicit and easier to follow.

Furthermore, this separation lays the groundwork for future cleanups
where try_enable_preferred_console() can be restricted to cases where
an entry actually exists in the console_cmdline[] array.

Also it fixes a possible out-of-bound access when the console_cmdline[]
array is full and no console matched. In fact, the check of
c->user_specified did not make much sense. The new console either
matched and was handled in the for-cycle. Or it did not match
and then *c pointed to an unused entry.

Possible behavior change:

try_enable_preferred_console() will newly be called also with
@user_specified parameter set to "false" when it failed with the "true"
variant. But it looks like the right way to do. It will allow to call
newcon->setup() when the console was preferred by some platform
specific code.

Reported-by: Naveen Kumar Chaudhary <naveen.osdev@gmail.com> # out-of-bound access
Closes: https://lore.kernel.org/r/7sq4tr2nmlz32tvkf6vpsghv6exvqfghsrlvywjcqihzsqqbf7@bspclmti5xg4
Signed-off-by: Petr Mladek <pmladek@suse.com>
---
 kernel/printk/printk.c | 19 ++++++++-----------
 1 file changed, 8 insertions(+), 11 deletions(-)

diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index 0323149548f6..aa53d57a524b 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -3890,9 +3890,6 @@ static int console_call_setup(struct console *newcon, char *options)
  * the newly registered console with any of the ones selected
  * by either the command line or add_preferred_console() and
  * setup/enable it.
- *
- * Care need to be taken with consoles that are statically
- * enabled such as netconsole
  */
 static int try_enable_preferred_console(struct console *newcon,
 					bool user_specified)
@@ -3933,14 +3930,6 @@ static int try_enable_preferred_console(struct console *newcon,
 		return 0;
 	}
 
-	/*
-	 * Some consoles, such as pstore and netconsole, can be enabled even
-	 * without matching. Accept the pre-enabled consoles only when match()
-	 * and setup() had a chance to be called.
-	 */
-	if (newcon->flags & CON_ENABLED && c->user_specified ==	user_specified)
-		return 0;
-
 	return -ENOENT;
 }
 
@@ -4123,6 +4112,14 @@ void register_console(struct console *newcon)
 	if (err == -ENOENT)
 		err = try_enable_preferred_console(newcon, false);
 
+	/*
+	 * Some consoles, such as pstore and netconsole, can be enabled even
+	 * without matching. Accept them at this stage when they had a chance
+	 * to match() and call setup().
+	 */
+	if (err == -ENOENT && (newcon->flags & CON_ENABLED))
+		err = 0;
+
 	/* printk() messages are not printed to the Braille console. */
 	if (err || newcon->flags & CON_BRL) {
 		if (newcon->flags & CON_NBCON)
-- 
2.54.0


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v4 02/11] printk: Rename struct console_cmdline to preferred_console
  2026-06-04 10:14 [PATCH v4 00/11] printk: Clean up preferred console handling Petr Mladek
  2026-06-04 10:14 ` [PATCH v4 01/11] printk: Handle pre-enabled consoles in the top-level register_console() Petr Mladek
@ 2026-06-04 10:14 ` Petr Mladek
  2026-06-04 10:14 ` [PATCH v4 03/11] printk: Rename preferred_console to preferred_dev_console Petr Mladek
                   ` (9 subsequent siblings)
  11 siblings, 0 replies; 19+ messages in thread
From: Petr Mladek @ 2026-06-04 10:14 UTC (permalink / raw)
  To: John Ogness
  Cc: Sergey Senozhatsky, Steven Rostedt, Marcos Paulo de Souza,
	Chris Down, Naveen Kumar Chaudhary, linux-kernel, Petr Mladek

The structure 'console_cmdline' was originally intended to store details
about consoles defined on the kernel command line. However, its usage
has since expanded; it now stores information for consoles preferred
via SPCR, device tree, or by particular platforms, e.g. XEN.

The current naming is misleading as it implies the configuration only
originates from the command line.

Rename the structure and associated artifacts to better reflect their
current purpose, for example:

  - struct console_cmdline c -> struct preferred_console pc
  - console_cmdline[]        -> preferred_consoles[]
  - console_cmdline.h        -> console_register.h
  - c			     -> pc

Additionally, renaming the header file to console_register.h would
eventually allow to decouple console registration logic from
the monolithic printk.c.

Finally, renaming the local variable from "c" to "pc" helps to distinguish
it from struct console variables. Note that "c" is used for struct console
in some code, for example see vt_console_device() function definition.

No functional changes.

Signed-off-by: Petr Mladek <pmladek@suse.com>
Acked-by: Chris Down <chris@chrisdown.name>
Acked-by: Marcos Paulo de Souza <mpdesouza@suse.com>
Reviewed-by: John Ogness <john.ogness@linutronix.de>
---
 kernel/printk/braille.c                       | 10 +--
 kernel/printk/braille.h                       | 10 +--
 .../{console_cmdline.h => console_register.h} |  6 +-
 kernel/printk/printk.c                        | 85 ++++++++++---------
 4 files changed, 57 insertions(+), 54 deletions(-)
 rename kernel/printk/{console_cmdline.h => console_register.h} (83%)

diff --git a/kernel/printk/braille.c b/kernel/printk/braille.c
index 17a9591e54ff..9d21a2bb1d38 100644
--- a/kernel/printk/braille.c
+++ b/kernel/printk/braille.c
@@ -6,7 +6,7 @@
 #include <linux/errno.h>
 #include <linux/string.h>
 
-#include "console_cmdline.h"
+#include "console_register.h"
 #include "braille.h"
 
 int _braille_console_setup(char **str, char **brl_options)
@@ -35,14 +35,14 @@ int _braille_console_setup(char **str, char **brl_options)
 }
 
 int
-_braille_register_console(struct console *console, struct console_cmdline *c)
+_braille_register_console(struct console *console, struct preferred_console *pc)
 {
 	int rtn = 0;
 
-	if (c->brl_options) {
+	if (pc->brl_options) {
 		console->flags |= CON_BRL;
-		rtn = braille_register_console(console, c->index, c->options,
-					       c->brl_options);
+		rtn = braille_register_console(console, pc->index, pc->options,
+					       pc->brl_options);
 	}
 
 	return rtn;
diff --git a/kernel/printk/braille.h b/kernel/printk/braille.h
index 123154f86304..55cd3178a17a 100644
--- a/kernel/printk/braille.h
+++ b/kernel/printk/braille.h
@@ -5,9 +5,9 @@
 #ifdef CONFIG_A11Y_BRAILLE_CONSOLE
 
 static inline void
-braille_set_options(struct console_cmdline *c, char *brl_options)
+braille_set_options(struct preferred_console *pc, char *brl_options)
 {
-	c->brl_options = brl_options;
+	pc->brl_options = brl_options;
 }
 
 /*
@@ -21,7 +21,7 @@ int
 _braille_console_setup(char **str, char **brl_options);
 
 int
-_braille_register_console(struct console *console, struct console_cmdline *c);
+_braille_register_console(struct console *console, struct preferred_console *pc);
 
 int
 _braille_unregister_console(struct console *console);
@@ -29,7 +29,7 @@ _braille_unregister_console(struct console *console);
 #else
 
 static inline void
-braille_set_options(struct console_cmdline *c, char *brl_options)
+braille_set_options(struct preferred_console *pc, char *brl_options)
 {
 }
 
@@ -40,7 +40,7 @@ _braille_console_setup(char **str, char **brl_options)
 }
 
 static inline int
-_braille_register_console(struct console *console, struct console_cmdline *c)
+_braille_register_console(struct console *console, struct preferred_console *pc)
 {
 	return 0;
 }
diff --git a/kernel/printk/console_cmdline.h b/kernel/printk/console_register.h
similarity index 83%
rename from kernel/printk/console_cmdline.h
rename to kernel/printk/console_register.h
index 0ab573b6d4dc..9ab3e1cc749b 100644
--- a/kernel/printk/console_cmdline.h
+++ b/kernel/printk/console_register.h
@@ -1,8 +1,8 @@
 /* SPDX-License-Identifier: GPL-2.0 */
-#ifndef _CONSOLE_CMDLINE_H
-#define _CONSOLE_CMDLINE_H
+#ifndef _CONSOLE_REGISTER_H
+#define _CONSOLE_REGISTER_H
 
-struct console_cmdline
+struct preferred_console
 {
 	char	name[16];			/* Name of the driver	    */
 	int	index;				/* Minor dev. to use	    */
diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index aa53d57a524b..0099e46a223b 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -58,7 +58,7 @@
 #include <trace/events/printk.h>
 
 #include "printk_ringbuffer.h"
-#include "console_cmdline.h"
+#include "console_register.h"
 #include "braille.h"
 #include "internal.h"
 
@@ -362,9 +362,9 @@ static int console_locked;
  *	Array of consoles built from command line options (console=)
  */
 
-#define MAX_CMDLINECONSOLES 8
+#define MAX_PREFERRED_CONSOLES 8
 
-static struct console_cmdline console_cmdline[MAX_CMDLINECONSOLES];
+static struct preferred_console preferred_consoles[MAX_PREFERRED_CONSOLES];
 
 static int preferred_console = -1;
 int console_set_on_cmdline;
@@ -2544,16 +2544,16 @@ asmlinkage __visible void early_printk(const char *fmt, ...)
 }
 #endif
 
-static void set_user_specified(struct console_cmdline *c, bool user_specified)
+static void set_user_specified(struct preferred_console *pc, bool user_specified)
 {
 	if (!user_specified)
 		return;
 
 	/*
-	 * @c console was defined by the user on the command line.
+	 * @pc console was defined by the user on the command line.
 	 * Do not clear when added twice also by SPCR or the device tree.
 	 */
-	c->user_specified = true;
+	pc->user_specified = true;
 	/* At least one console defined by the user on the command line. */
 	console_set_on_cmdline = 1;
 }
@@ -2562,7 +2562,7 @@ static int __add_preferred_console(const char *name, const short idx,
 				   const char *devname, char *options,
 				   char *brl_options, bool user_specified)
 {
-	struct console_cmdline *c;
+	struct preferred_console *pc;
 	int i;
 
 	if (!name && !devname)
@@ -2581,30 +2581,30 @@ static int __add_preferred_console(const char *name, const short idx,
 	 *	See if this tty is not yet registered, and
 	 *	if we have a slot free.
 	 */
-	for (i = 0, c = console_cmdline;
-	     i < MAX_CMDLINECONSOLES && (c->name[0] || c->devname[0]);
-	     i++, c++) {
-		if ((name && strcmp(c->name, name) == 0 && c->index == idx) ||
-		    (devname && strcmp(c->devname, devname) == 0)) {
+	for (i = 0, pc = preferred_consoles;
+	     i < MAX_PREFERRED_CONSOLES && (pc->name[0] || pc->devname[0]);
+	     i++, pc++) {
+		if ((name && strcmp(pc->name, name) == 0 && pc->index == idx) ||
+		    (devname && strcmp(pc->devname, devname) == 0)) {
 			if (!brl_options)
 				preferred_console = i;
-			set_user_specified(c, user_specified);
+			set_user_specified(pc, user_specified);
 			return 0;
 		}
 	}
-	if (i == MAX_CMDLINECONSOLES)
+	if (i == MAX_PREFERRED_CONSOLES)
 		return -E2BIG;
 	if (!brl_options)
 		preferred_console = i;
 	if (name)
-		strscpy(c->name, name);
+		strscpy(pc->name, name);
 	if (devname)
-		strscpy(c->devname, devname);
-	c->options = options;
-	set_user_specified(c, user_specified);
-	braille_set_options(c, brl_options);
+		strscpy(pc->devname, devname);
+	pc->options = options;
+	set_user_specified(pc, user_specified);
+	braille_set_options(pc, brl_options);
 
-	c->index = idx;
+	pc->index = idx;
 	return 0;
 }
 
@@ -2624,8 +2624,7 @@ __setup("console_msg_format=", console_msg_format_setup);
  */
 static int __init console_setup(char *str)
 {
-	static_assert(sizeof(console_cmdline[0].devname) >= sizeof(console_cmdline[0].name) + 4);
-	char buf[sizeof(console_cmdline[0].devname)];
+	char buf[sizeof(preferred_consoles[0].devname)];
 	char *brl_options = NULL;
 	char *ttyname = NULL;
 	char *devname = NULL;
@@ -2633,6 +2632,10 @@ static int __init console_setup(char *str)
 	char *s;
 	int idx;
 
+	static_assert(sizeof(preferred_consoles[0].devname) >=
+		      sizeof(preferred_consoles[0].name) + 4);
+
+
 	/*
 	 * console="" or console=null have been suggested as a way to
 	 * disable console output. Use ttynull that has been created
@@ -2730,19 +2733,19 @@ int match_devname_and_update_preferred_console(const char *devname,
 					       const char *name,
 					       const short idx)
 {
-	struct console_cmdline *c = console_cmdline;
+	struct preferred_console *pc = preferred_consoles;
 	int i;
 
 	if (!devname || !strlen(devname) || !name || !strlen(name) || idx < 0)
 		return -EINVAL;
 
-	for (i = 0; i < MAX_CMDLINECONSOLES && (c->name[0] || c->devname[0]);
-	     i++, c++) {
-		if (!strcmp(devname, c->devname)) {
+	for (i = 0; i < MAX_PREFERRED_CONSOLES && (pc->name[0] || pc->devname[0]);
+	     i++, pc++) {
+		if (!strcmp(devname, pc->devname)) {
 			pr_info("associate the preferred console \"%s\" with \"%s%d\"\n",
 				devname, name, idx);
-			strscpy(c->name, name);
-			c->index = idx;
+			strscpy(pc->name, name);
+			pc->index = idx;
 			return 0;
 		}
 	}
@@ -3894,33 +3897,33 @@ static int console_call_setup(struct console *newcon, char *options)
 static int try_enable_preferred_console(struct console *newcon,
 					bool user_specified)
 {
-	struct console_cmdline *c;
+	struct preferred_console *pc;
 	int i, err;
 
-	for (i = 0, c = console_cmdline;
-	     i < MAX_CMDLINECONSOLES && (c->name[0] || c->devname[0]);
-	     i++, c++) {
+	for (i = 0, pc = preferred_consoles;
+	     i < MAX_PREFERRED_CONSOLES && (pc->name[0] || pc->devname[0]);
+	     i++, pc++) {
 		/* Console not yet initialized? */
-		if (!c->name[0])
+		if (!pc->name[0])
 			continue;
-		if (c->user_specified != user_specified)
+		if (pc->user_specified != user_specified)
 			continue;
 		if (!newcon->match ||
-		    newcon->match(newcon, c->name, c->index, c->options) != 0) {
+		    newcon->match(newcon, pc->name, pc->index, pc->options) != 0) {
 			/* default matching */
-			BUILD_BUG_ON(sizeof(c->name) != sizeof(newcon->name));
-			if (strcmp(c->name, newcon->name) != 0)
+			BUILD_BUG_ON(sizeof(pc->name) != sizeof(newcon->name));
+			if (strcmp(pc->name, newcon->name) != 0)
 				continue;
 			if (newcon->index >= 0 &&
-			    newcon->index != c->index)
+			    newcon->index != pc->index)
 				continue;
 			if (newcon->index < 0)
-				newcon->index = c->index;
+				newcon->index = pc->index;
 
-			if (_braille_register_console(newcon, c))
+			if (_braille_register_console(newcon, pc))
 				return 0;
 
-			err = console_call_setup(newcon, c->options);
+			err = console_call_setup(newcon, pc->options);
 			if (err)
 				return err;
 		}
-- 
2.54.0


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v4 03/11] printk: Rename preferred_console to preferred_dev_console
  2026-06-04 10:14 [PATCH v4 00/11] printk: Clean up preferred console handling Petr Mladek
  2026-06-04 10:14 ` [PATCH v4 01/11] printk: Handle pre-enabled consoles in the top-level register_console() Petr Mladek
  2026-06-04 10:14 ` [PATCH v4 02/11] printk: Rename struct console_cmdline to preferred_console Petr Mladek
@ 2026-06-04 10:14 ` Petr Mladek
  2026-06-04 10:14 ` [PATCH v4 04/11] printk: Separate code for adding/updating preferred console metadata Petr Mladek
                   ` (8 subsequent siblings)
  11 siblings, 0 replies; 19+ messages in thread
From: Petr Mladek @ 2026-06-04 10:14 UTC (permalink / raw)
  To: John Ogness
  Cc: Sergey Senozhatsky, Steven Rostedt, Marcos Paulo de Souza,
	Chris Down, Naveen Kumar Chaudhary, linux-kernel, Petr Mladek

The preferred_consoles[] array stores information about consoles requested
via the command line, SPCR, Device Tree, or platform-specific code.
Within this array, the 'preferred_console' variable tracks the specific
index that should be associated with /dev/console (typically the last
non-braille console defined).

The current name "preferred_console" is ambiguous and leads to confusion.
It does not clearly communicate why one console is "more preferred" than
others in the array. Furthermore, entries for Braille consoles can exist
within the preferred_consoles[] array, yet they are never associated with
/dev/console and do not receive standard printk() output. Consequently,
the 'preferred_console' index must skip these entries, which is not
immediately obvious from the name.

Rename the variable to 'preferred_dev_console' to explicitly clarify its
role in identifying which entry is linked to /dev/console.

No behavior change.

Signed-off-by: Petr Mladek <pmladek@suse.com>
Acked-by: Chris Down <chris@chrisdown.name>
Acked-by: Marcos Paulo de Souza <mpdesouza@suse.com>
Reviewed-by: John Ogness <john.ogness@linutronix.de>
---
 kernel/printk/printk.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index 0099e46a223b..e162e8836683 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -366,7 +366,7 @@ static int console_locked;
 
 static struct preferred_console preferred_consoles[MAX_PREFERRED_CONSOLES];
 
-static int preferred_console = -1;
+static int preferred_dev_console = -1;
 int console_set_on_cmdline;
 EXPORT_SYMBOL(console_set_on_cmdline);
 
@@ -2587,7 +2587,7 @@ static int __add_preferred_console(const char *name, const short idx,
 		if ((name && strcmp(pc->name, name) == 0 && pc->index == idx) ||
 		    (devname && strcmp(pc->devname, devname) == 0)) {
 			if (!brl_options)
-				preferred_console = i;
+				preferred_dev_console = i;
 			set_user_specified(pc, user_specified);
 			return 0;
 		}
@@ -2595,7 +2595,7 @@ static int __add_preferred_console(const char *name, const short idx,
 	if (i == MAX_PREFERRED_CONSOLES)
 		return -E2BIG;
 	if (!brl_options)
-		preferred_console = i;
+		preferred_dev_console = i;
 	if (name)
 		strscpy(pc->name, name);
 	if (devname)
@@ -3928,7 +3928,7 @@ static int try_enable_preferred_console(struct console *newcon,
 				return err;
 		}
 		newcon->flags |= CON_ENABLED;
-		if (i == preferred_console)
+		if (i == preferred_dev_console)
 			newcon->flags |= CON_CONSDEV;
 		return 0;
 	}
@@ -4101,7 +4101,7 @@ void register_console(struct console *newcon)
 	 * Note that a console with tty binding will have CON_CONSDEV
 	 * flag set and will be first in the list.
 	 */
-	if (preferred_console < 0) {
+	if (preferred_dev_console < 0) {
 		if (hlist_empty(&console_list) || !console_first()->device ||
 		    console_first()->flags & CON_BOOT) {
 			try_enable_default_console(newcon);
-- 
2.54.0


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v4 04/11] printk: Separate code for adding/updating preferred console metadata
  2026-06-04 10:14 [PATCH v4 00/11] printk: Clean up preferred console handling Petr Mladek
                   ` (2 preceding siblings ...)
  2026-06-04 10:14 ` [PATCH v4 03/11] printk: Rename preferred_console to preferred_dev_console Petr Mladek
@ 2026-06-04 10:14 ` Petr Mladek
  2026-07-14 14:35   ` John Ogness
  2026-06-04 10:14 ` [PATCH v4 05/11] printk: Cleanup _braille_(un)register_console() wrappers Petr Mladek
                   ` (7 subsequent siblings)
  11 siblings, 1 reply; 19+ messages in thread
From: Petr Mladek @ 2026-06-04 10:14 UTC (permalink / raw)
  To: John Ogness
  Cc: Sergey Senozhatsky, Steven Rostedt, Marcos Paulo de Souza,
	Chris Down, Naveen Kumar Chaudhary, linux-kernel, Petr Mladek

The logic for adding or updating a preferred console is currently
duplicated within __add_preferred_console(), making the code difficult
to follow and prone to consistency issues.

Introduce update_preferred_console() to centralize the initialization
and updating of struct preferred_console entries. This refactoring
explicitly defines and enforces the following rules:

1. Console names and/or indexes are not set when a console is preferred
   via devname; these are resolved later during device matching.
2. Console names are only added alongside a valid index.
3. Only matching entries are updated.
4. Console and Braille options are never cleared. They are updated
   only via the command line.
5. The global 'preferred_dev_console' index and 'console_set_on_cmdline'
   flag are updated consistently.

Additionally, rename braille_set_options() to braille_update_options()
to better reflect its conditional behavior.

Behavior change:

The original code never updated the preferred console options
when it was preferred more times, e.g. via the command line
and/or some platform specific code, e.g. SPCR or device tree.

The new code explicitly allows to update the console options
when they are preferred over the command line.

It mostly worked even before but only because the command line was
processed early enough before handling SPCR, device tree, or other
platform specific init code.

The main behavior change is when the same console is preferred more
times on the command line. Newly, the later or Braille variant
wins. It is a more common and expected behavior.

Signed-off-by: Petr Mladek <pmladek@suse.com>
---
 kernel/printk/braille.h |   7 +-
 kernel/printk/printk.c  | 153 +++++++++++++++++++++++++++++++---------
 2 files changed, 122 insertions(+), 38 deletions(-)

diff --git a/kernel/printk/braille.h b/kernel/printk/braille.h
index 55cd3178a17a..0bdac303f8b1 100644
--- a/kernel/printk/braille.h
+++ b/kernel/printk/braille.h
@@ -5,9 +5,10 @@
 #ifdef CONFIG_A11Y_BRAILLE_CONSOLE
 
 static inline void
-braille_set_options(struct preferred_console *pc, char *brl_options)
+braille_update_options(struct preferred_console *pc, char *brl_options)
 {
-	pc->brl_options = brl_options;
+	if (brl_options)
+		pc->brl_options = brl_options;
 }
 
 /*
@@ -29,7 +30,7 @@ _braille_unregister_console(struct console *console);
 #else
 
 static inline void
-braille_set_options(struct preferred_console *pc, char *brl_options)
+braille_update_options(struct preferred_console *pc, char *brl_options)
 {
 }
 
diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index e162e8836683..b7dbbc1baeb1 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -2544,18 +2544,123 @@ asmlinkage __visible void early_printk(const char *fmt, ...)
 }
 #endif
 
-static void set_user_specified(struct preferred_console *pc, bool user_specified)
+/** add_or_update_preferred_console - Add or update a given entry in
+ *	the preferred_consoles[] table.
+ * @i: index of the entry in @preferred_consoles table which should get updated.
+ * @name: The name of the preferred console driver.
+ * @idx: Preferred console index, e.g. port number.
+ * @devname: The name of the preferred physical device.
+ * @options: Options used when setting up the console driver.
+ * @brl_options: Options used when setting up the console driver
+ *	as a braille console.
+ * @user_specified: True if preferred via the kernel command line.
+ *
+ * The function ensures that the given values are consistent. Also
+ * it updates some global variables which are used to make the right
+ * decisions in register_console().
+ *
+ * Rules:
+ *
+ *   1. Either @name and valid @idx OR @devname and @idx=-1 are allowed.
+ *	Note that a valid @name and @idx will get assigned later when
+ *	@devname matches during the device initialization.
+ *   2. Specify @brl_options if the console should be enabled as
+ *	a Braille console [*]
+ *   3. Only matching entries can be updated.
+ *   4. @options passed via the command line are used when the same
+ *	console is preferred also by some platform-specific code.
+ *
+ * [*] Braille console is using the mechanism for registering consoles
+ *     but it is very special. It is primarily used for user interaction
+ *     with the system. It neither gets printk() messages nor is associated
+ *     with /dev/console.
+ */
+static int add_or_update_preferred_console(unsigned int i,
+					   const char *name, const short idx,
+					   const char *devname, char *options,
+					   char *brl_options, bool user_specified)
 {
-	if (!user_specified)
-		return;
+	struct preferred_console *pc;
+
+	if (i >= MAX_PREFERRED_CONSOLES) {
+		pr_err_once("Reached maximal number of preferred consoles.\n");
+		return -E2BIG;
+	}
+
+	pc = &preferred_consoles[i];
+
+	if (WARN_ON(!name && !devname))
+		return -EINVAL;
+
+	if (devname) {
+		/*
+		 * A valid console name and index will get assigned when
+		 * a matching device gets registered.
+		 */
+		if (name) {
+			pr_err("Adding a preferred console devname with a hard-coded console name: %s, %s\n",
+			       devname, name);
+			return -EINVAL;
+		}
+		if (pc->name[0]) {
+			pr_err("Updating a preferred console entry with an already assigned console name via devname: %s, %s\n",
+			       devname, pc->name);
+			return -EINVAL;
+		}
+		if (idx != -1) {
+			pr_err("Adding a preferred console devname with a hard-coded index: %s, %d\n",
+			       devname, idx);
+			return -EINVAL;
+		}
+
+		if (!pc->devname[0]) {
+			/* This is a new entry. */
+			strscpy(pc->devname, devname);
+			pc->index = idx;
+		} else if (strcmp(pc->devname, devname) != 0) {
+			pr_err("Updating a preferred console with an invalid devname: %s vs. %s\n",
+			       pc->devname, devname);
+			return -EINVAL;
+		}
+	}
+
+	if (name) {
+		/* A console name must be defined with a valid index. */
+		if (idx < 0) {
+			pr_err("Adding a preferred console with an invalid index: %s, %d\n",
+			       name, idx);
+			return -EINVAL;
+		}
+
+		if (!pc->name[0]) {
+			/* This is a new entry. */
+			strscpy(pc->name, name);
+			pc->index = idx;
+		} else if (strcmp(pc->name, name) != 0 || pc->index != idx) {
+			pr_err("Updating a preferred console with an invalid name or index: %s%d vs. %s%d\n",
+			       pc->name, pc->index, name, idx);
+			return -EINVAL;
+		}
+	}
+
+	if (!pc->options || (user_specified && options))
+		pc->options = options;
+
+	braille_update_options(pc, brl_options);
+
+	if (!brl_options)
+		preferred_dev_console = i;
 
 	/*
 	 * @pc console was defined by the user on the command line.
 	 * Do not clear when added twice also by SPCR or the device tree.
 	 */
-	pc->user_specified = true;
-	/* At least one console defined by the user on the command line. */
-	console_set_on_cmdline = 1;
+	if (user_specified) {
+		pc->user_specified = true;
+		console_set_on_cmdline = 1;
+	}
+
+	return 0;
 }
 
 static int __add_preferred_console(const char *name, const short idx,
@@ -2563,19 +2668,11 @@ static int __add_preferred_console(const char *name, const short idx,
 				   char *brl_options, bool user_specified)
 {
 	struct preferred_console *pc;
-	int i;
+	unsigned int i;
 
 	if (!name && !devname)
 		return -EINVAL;
 
-	/*
-	 * We use a signed short index for struct console for device drivers to
-	 * indicate a not yet assigned index or port. However, a negative index
-	 * value is not valid when the console name and index are defined on
-	 * the command line.
-	 */
-	if (name && idx < 0)
-		return -EINVAL;
 
 	/*
 	 *	See if this tty is not yet registered, and
@@ -2584,28 +2681,14 @@ static int __add_preferred_console(const char *name, const short idx,
 	for (i = 0, pc = preferred_consoles;
 	     i < MAX_PREFERRED_CONSOLES && (pc->name[0] || pc->devname[0]);
 	     i++, pc++) {
-		if ((name && strcmp(pc->name, name) == 0 && pc->index == idx) ||
-		    (devname && strcmp(pc->devname, devname) == 0)) {
-			if (!brl_options)
-				preferred_dev_console = i;
-			set_user_specified(pc, user_specified);
-			return 0;
-		}
+		if (name && strcmp(pc->name, name) == 0 && pc->index == idx)
+			break;
+		if (devname && strcmp(pc->devname, devname) == 0)
+			break;
 	}
-	if (i == MAX_PREFERRED_CONSOLES)
-		return -E2BIG;
-	if (!brl_options)
-		preferred_dev_console = i;
-	if (name)
-		strscpy(pc->name, name);
-	if (devname)
-		strscpy(pc->devname, devname);
-	pc->options = options;
-	set_user_specified(pc, user_specified);
-	braille_set_options(pc, brl_options);
 
-	pc->index = idx;
-	return 0;
+	return add_or_update_preferred_console(i, name, idx, devname, options,
+					       brl_options, user_specified);
 }
 
 static int __init console_msg_format_setup(char *str)
-- 
2.54.0


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v4 05/11] printk: Cleanup _braille_(un)register_console() wrappers
  2026-06-04 10:14 [PATCH v4 00/11] printk: Clean up preferred console handling Petr Mladek
                   ` (3 preceding siblings ...)
  2026-06-04 10:14 ` [PATCH v4 04/11] printk: Separate code for adding/updating preferred console metadata Petr Mladek
@ 2026-06-04 10:14 ` Petr Mladek
  2026-06-04 10:14 ` [PATCH v4 06/11] console/braille: Lock console->setup() call during the registration Petr Mladek
                   ` (6 subsequent siblings)
  11 siblings, 0 replies; 19+ messages in thread
From: Petr Mladek @ 2026-06-04 10:14 UTC (permalink / raw)
  To: John Ogness
  Cc: Sergey Senozhatsky, Steven Rostedt, Marcos Paulo de Souza,
	Chris Down, Naveen Kumar Chaudhary, linux-kernel, Petr Mladek

The _braille_(un)register_console() wrappers currently attempt to hide
implementation details like the CON_BRL flag and pc->brl_options. This
forces callers to handle an unconventional tri-state return value (0 for
NOP, >0 for success, <0 for error), which makes the control flow harder
to follow and non-standard.

Refactor the wrappers to use standard kernel return codes (0 for success,
-ERRCODE on failure). Move the responsibility of checking brl_options
to the caller to make the logic more explicit.

Additionally, move the assignment of the CON_BRL flag from the internal
wrapper to braille_register_console(). This aligns it with how
CON_ENABLED is handled. To maintain symmetry and fix a potential bug
where flags might persist after removal, explicitly clear both
CON_ENABLED and CON_BRL in braille_unregister_console().

No behavior change.

Signed-off-by: Petr Mladek <pmladek@suse.com>
Acked-by: Chris Down <chris@chrisdown.name>
Reviewed-by: Marcos Paulo de Souza <mpdesouza@suse.com>
Reviewed-by: John Ogness <john.ogness@linutronix.de>
---
 drivers/accessibility/braille/braille_console.c |  7 ++++---
 kernel/printk/braille.c                         | 16 +++-------------
 kernel/printk/braille.h                         | 12 ++++++++++++
 kernel/printk/printk.c                          | 13 +++++--------
 4 files changed, 24 insertions(+), 24 deletions(-)

diff --git a/drivers/accessibility/braille/braille_console.c b/drivers/accessibility/braille/braille_console.c
index 06b43b678d6e..7b324329882f 100644
--- a/drivers/accessibility/braille/braille_console.c
+++ b/drivers/accessibility/braille/braille_console.c
@@ -360,12 +360,12 @@ int braille_register_console(struct console *console, int index,
 		if (ret != 0)
 			return ret;
 	}
-	console->flags |= CON_ENABLED;
+	console->flags |= CON_ENABLED | CON_BRL;
 	console->index = index;
 	braille_co = console;
 	register_keyboard_notifier(&keyboard_notifier_block);
 	register_vt_notifier(&vt_notifier_block);
-	return 1;
+	return 0;
 }
 
 int braille_unregister_console(struct console *console)
@@ -375,5 +375,6 @@ int braille_unregister_console(struct console *console)
 	unregister_keyboard_notifier(&keyboard_notifier_block);
 	unregister_vt_notifier(&vt_notifier_block);
 	braille_co = NULL;
-	return 1;
+	console->flags &= ~(CON_ENABLED | CON_BRL);
+	return 0;
 }
diff --git a/kernel/printk/braille.c b/kernel/printk/braille.c
index 9d21a2bb1d38..593f83eb0487 100644
--- a/kernel/printk/braille.c
+++ b/kernel/printk/braille.c
@@ -37,22 +37,12 @@ int _braille_console_setup(char **str, char **brl_options)
 int
 _braille_register_console(struct console *console, struct preferred_console *pc)
 {
-	int rtn = 0;
-
-	if (pc->brl_options) {
-		console->flags |= CON_BRL;
-		rtn = braille_register_console(console, pc->index, pc->options,
-					       pc->brl_options);
-	}
-
-	return rtn;
+	return braille_register_console(console, pc->index, pc->options,
+					pc->brl_options);
 }
 
 int
 _braille_unregister_console(struct console *console)
 {
-	if (console->flags & CON_BRL)
-		return braille_unregister_console(console);
-
-	return 0;
+	return braille_unregister_console(console);
 }
diff --git a/kernel/printk/braille.h b/kernel/printk/braille.h
index 0bdac303f8b1..ec5feac1f508 100644
--- a/kernel/printk/braille.h
+++ b/kernel/printk/braille.h
@@ -11,6 +11,12 @@ braille_update_options(struct preferred_console *pc, char *brl_options)
 		pc->brl_options = brl_options;
 }
 
+static inline bool
+is_braille_console_preferred(struct preferred_console *pc)
+{
+	return (!!pc->brl_options);
+}
+
 /*
  * Setup console according to braille options.
  * Return -EINVAL on syntax error, 0 on success (or no braille option was
@@ -34,6 +40,12 @@ braille_update_options(struct preferred_console *pc, char *brl_options)
 {
 }
 
+static inline bool
+is_braille_console_preferred(struct preferred_console *pc)
+{
+	return false;
+}
+
 static inline int
 _braille_console_setup(char **str, char **brl_options)
 {
diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index b7dbbc1baeb1..4ee98a4d2ce5 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -4003,8 +4003,8 @@ static int try_enable_preferred_console(struct console *newcon,
 			if (newcon->index < 0)
 				newcon->index = pc->index;
 
-			if (_braille_register_console(newcon, pc))
-				return 0;
+			if (is_braille_console_preferred(pc))
+				return _braille_register_console(newcon, pc);
 
 			err = console_call_setup(newcon, pc->options);
 			if (err)
@@ -4316,17 +4316,14 @@ static int unregister_console_locked(struct console *console)
 	bool found_boot_con = false;
 	unsigned long flags;
 	struct console *c;
-	int res;
+	int res = 0;
 
 	lockdep_assert_console_list_lock_held();
 
 	con_printk(KERN_INFO, console, "disabled\n");
 
-	res = _braille_unregister_console(console);
-	if (res < 0)
-		return res;
-	if (res > 0)
-		return 0;
+	if (console->flags & CON_BRL)
+		return _braille_unregister_console(console);
 
 	if (!console_is_registered_locked(console))
 		res = -ENODEV;
-- 
2.54.0


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v4 06/11] console/braille: Lock console->setup() call during the registration
  2026-06-04 10:14 [PATCH v4 00/11] printk: Clean up preferred console handling Petr Mladek
                   ` (4 preceding siblings ...)
  2026-06-04 10:14 ` [PATCH v4 05/11] printk: Cleanup _braille_(un)register_console() wrappers Petr Mladek
@ 2026-06-04 10:14 ` Petr Mladek
  2026-07-15 14:41   ` John Ogness
  2026-06-04 10:14 ` [PATCH v4 07/11] printk: Separate code for enabling console Petr Mladek
                   ` (5 subsequent siblings)
  11 siblings, 1 reply; 19+ messages in thread
From: Petr Mladek @ 2026-06-04 10:14 UTC (permalink / raw)
  To: John Ogness
  Cc: Sergey Senozhatsky, Steven Rostedt, Marcos Paulo de Souza,
	Chris Down, Naveen Kumar Chaudhary, linux-kernel, Petr Mladek,
	Sashiko

In braille_register_console(), the console->setup() callback must be
called while holding the console_lock. This prevents a race condition
with a possible active boot console that might be using the same hardware
registers.

This applies the same protection to the braille console registration
that was introduced for regular console registration in commit 801410b26a0e
("serial: Lock console when calling into driver before registration").

Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/#/patchset/20260423130015.85175-1-pmladek%40suse.com
Assisted-by: Gemini:gemini-3
Signed-off-by: Petr Mladek <pmladek@suse.com>
---
 drivers/accessibility/braille/braille_console.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/accessibility/braille/braille_console.c b/drivers/accessibility/braille/braille_console.c
index 7b324329882f..9ed3abffb687 100644
--- a/drivers/accessibility/braille/braille_console.c
+++ b/drivers/accessibility/braille/braille_console.c
@@ -356,7 +356,9 @@ int braille_register_console(struct console *console, int index,
 	if (braille_co)
 		return -ENODEV;
 	if (console->setup) {
+		console_lock();
 		ret = console->setup(console, console_options);
+		console_unlock();
 		if (ret != 0)
 			return ret;
 	}
-- 
2.54.0


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v4 07/11] printk: Separate code for enabling console
  2026-06-04 10:14 [PATCH v4 00/11] printk: Clean up preferred console handling Petr Mladek
                   ` (5 preceding siblings ...)
  2026-06-04 10:14 ` [PATCH v4 06/11] console/braille: Lock console->setup() call during the registration Petr Mladek
@ 2026-06-04 10:14 ` Petr Mladek
  2026-07-15 16:01   ` John Ogness
  2026-06-04 10:14 ` [PATCH v4 08/11] printk: Try to register each console as Braille first Petr Mladek
                   ` (4 subsequent siblings)
  11 siblings, 1 reply; 19+ messages in thread
From: Petr Mladek @ 2026-06-04 10:14 UTC (permalink / raw)
  To: John Ogness
  Cc: Sergey Senozhatsky, Steven Rostedt, Marcos Paulo de Souza,
	Chris Down, Naveen Kumar Chaudhary, linux-kernel, Petr Mladek

There are several code paths which try to enable a newly registered
console. Move the logic into a separate try_enable_console() function.

It simplifies a bit the long register_console() function definition.

Also followup patches are going to add even more code paths. And it will
be easier to use "return" when it does not make sense to try other
variants.

No behavior change.

Signed-off-by: Petr Mladek <pmladek@suse.com>
---
 kernel/printk/printk.c | 84 ++++++++++++++++++++++++------------------
 1 file changed, 48 insertions(+), 36 deletions(-)

diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index 4ee98a4d2ce5..5fb9581804c7 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -4034,6 +4034,53 @@ static void try_enable_default_console(struct console *newcon)
 		newcon->flags |= CON_CONSDEV;
 }
 
+#define console_first()				\
+	hlist_entry(console_list.first, struct console, node)
+
+static int try_enable_console(struct console *newcon)
+{
+	int err;
+
+	/*
+	 * See if we want to enable this console driver by default.
+	 *
+	 * Nope when a console is preferred by the command line, device
+	 * tree, or SPCR.
+	 *
+	 * The first real console with tty binding (driver) wins. More
+	 * consoles might get enabled before the right one is found.
+	 *
+	 * Note that a console with tty binding will have CON_CONSDEV
+	 * flag set and will be first in the list.
+	 */
+	if (preferred_dev_console < 0) {
+		if (hlist_empty(&console_list) || !console_first()->device ||
+		    console_first()->flags & CON_BOOT) {
+			try_enable_default_console(newcon);
+		}
+	}
+
+	/* See if this console matches one we selected on the command line */
+	err = try_enable_preferred_console(newcon, true);
+	if (err != -ENOENT)
+		return err;
+
+	/* If not, try to match against the platform default(s) */
+	err = try_enable_preferred_console(newcon, false);
+	if (err != -ENOENT)
+		return err;
+
+	/*
+	 * Some consoles, such as pstore and netconsole, can be enabled even
+	 * without matching. Accept them at this stage when they had a chance
+	 * to match() and call setup().
+	 */
+	if (newcon->flags & CON_ENABLED)
+		err = 0;
+
+	return err;
+}
+
 /* Return the starting sequence number for a newly registered console. */
 static u64 get_init_console_seq(struct console *newcon, bool bootcon_registered)
 {
@@ -4108,9 +4155,6 @@ static u64 get_init_console_seq(struct console *newcon, bool bootcon_registered)
 	return init_seq;
 }
 
-#define console_first()				\
-	hlist_entry(console_list.first, struct console, node)
-
 static int unregister_console_locked(struct console *console);
 
 /*
@@ -4172,39 +4216,7 @@ void register_console(struct console *newcon)
 			goto unlock;
 	}
 
-	/*
-	 * See if we want to enable this console driver by default.
-	 *
-	 * Nope when a console is preferred by the command line, device
-	 * tree, or SPCR.
-	 *
-	 * The first real console with tty binding (driver) wins. More
-	 * consoles might get enabled before the right one is found.
-	 *
-	 * Note that a console with tty binding will have CON_CONSDEV
-	 * flag set and will be first in the list.
-	 */
-	if (preferred_dev_console < 0) {
-		if (hlist_empty(&console_list) || !console_first()->device ||
-		    console_first()->flags & CON_BOOT) {
-			try_enable_default_console(newcon);
-		}
-	}
-
-	/* See if this console matches one we selected on the command line */
-	err = try_enable_preferred_console(newcon, true);
-
-	/* If not, try to match against the platform default(s) */
-	if (err == -ENOENT)
-		err = try_enable_preferred_console(newcon, false);
-
-	/*
-	 * Some consoles, such as pstore and netconsole, can be enabled even
-	 * without matching. Accept them at this stage when they had a chance
-	 * to match() and call setup().
-	 */
-	if (err == -ENOENT && (newcon->flags & CON_ENABLED))
-		err = 0;
+	err = try_enable_console(newcon);
 
 	/* printk() messages are not printed to the Braille console. */
 	if (err || newcon->flags & CON_BRL) {
-- 
2.54.0


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v4 08/11] printk: Try to register each console as Braille first
  2026-06-04 10:14 [PATCH v4 00/11] printk: Clean up preferred console handling Petr Mladek
                   ` (6 preceding siblings ...)
  2026-06-04 10:14 ` [PATCH v4 07/11] printk: Separate code for enabling console Petr Mladek
@ 2026-06-04 10:14 ` Petr Mladek
  2026-06-04 10:14 ` [PATCH v4 09/11] printk: Do not set Braille console as preferred_console Petr Mladek
                   ` (3 subsequent siblings)
  11 siblings, 0 replies; 19+ messages in thread
From: Petr Mladek @ 2026-06-04 10:14 UTC (permalink / raw)
  To: John Ogness
  Cc: Sergey Senozhatsky, Steven Rostedt, Marcos Paulo de Souza,
	Chris Down, Naveen Kumar Chaudhary, linux-kernel, Petr Mladek

Braille consoles are unique: they can only be enabled via the command
line and use the preferred console framework for initialization, yet
they are never added to the console_list or associated with
/dev/console. Because of this, the preferred_console variable remains
unset when only a Braille console is requested.

Currently, try_enable_preferred_console() must be called even when
"preferred_dev_console" variable is not set, just to catch these "hidden"
Braille requests.

Refactor the logic by adding a parameter to try_enable_preferred_console()
to explicitly handle Braille vs. non-Braille cases. It will eventually
allow to skip try_enable_preferred_console() when there are no preferred
consoles. This improves code robustness by ensuring the console
setup is explicit and only performed once.

Note that _braille_register_console() is skipped when the given console
driver has a match() callback and it succeeded. It means that it took
over an early console and can't be used as a Braille console.

As a result try_enable_braille_console() might return success even
when it did not enabled the console in the Braille mode. It is
the reason why register_console() has to explicitly check whether
CON_BRL flag was really set and could skip the further steps.

The refactoring even fixes two subtle bugs:

1. When only the Braille console is defined on the command line,
   the original code might attempt to enable the same console twice—once
   as a default and once as a Braille console. This results in calling
   the setup() callback twice and incorrectly setting the CON_CONSDEV flag.

2. When the same console is defined on the command line using devname
   and then as Braille console, for example:

      console=00:00:0.0,115200 console=brl,ttyS0,115200

   It would have two separate entries in preferred_consoles[] array.

   The 2nd (Braille) entry would be used when univ8250_console_init()
   tries to register the generic "ttyS" console driver. Note that
   the 1st entry still does not have defined the "name" entry at
   this stage.

   The 1st non-Braille entry would be used later when serial8250_init()
   registers all found devices, assigns "tty0" for the given "00:00:0.0"
   devname and tries to register the same struct console once again.

   The original code would call newcon->setup() twice in this scenario.
   It won't add the console into console_list only because the later
   check in register_console() would detect CON_BRL flag set from
   the 1st registration and return early.

Signed-off-by: Petr Mladek <pmladek@suse.com>
---
 kernel/printk/printk.c | 56 +++++++++++++++++++++++++++++++++++++-----
 1 file changed, 50 insertions(+), 6 deletions(-)

diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index 5fb9581804c7..3a645558c18a 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -367,6 +367,7 @@ static int console_locked;
 static struct preferred_console preferred_consoles[MAX_PREFERRED_CONSOLES];
 
 static int preferred_dev_console = -1;
+static bool want_braille_console;
 int console_set_on_cmdline;
 EXPORT_SYMBOL(console_set_on_cmdline);
 
@@ -2648,7 +2649,9 @@ static int add_or_update_preferred_console(unsigned int i,
 
 	braille_update_options(pc, brl_options);
 
-	if (!brl_options)
+	if (brl_options)
+		want_braille_console = true;
+	else
 		preferred_dev_console = i;
 
 	/*
@@ -3977,8 +3980,9 @@ static int console_call_setup(struct console *newcon, char *options)
  * by either the command line or add_preferred_console() and
  * setup/enable it.
  */
-static int try_enable_preferred_console(struct console *newcon,
-					bool user_specified)
+static int __try_enable_preferred_console(struct console *newcon,
+					  bool user_specified,
+					  bool try_only_braille)
 {
 	struct preferred_console *pc;
 	int i, err;
@@ -3989,8 +3993,19 @@ static int try_enable_preferred_console(struct console *newcon,
 		/* Console not yet initialized? */
 		if (!pc->name[0])
 			continue;
-		if (pc->user_specified != user_specified)
-			continue;
+
+		/*
+		 * @try_only_braille and @user_specifified define which
+		 * preferred console entries are handled in this round.
+		 */
+		if (try_only_braille) {
+			if (!is_braille_console_preferred(pc))
+				continue;
+		} else {
+			if (pc->user_specified != user_specified)
+				continue;
+		}
+
 		if (!newcon->match ||
 		    newcon->match(newcon, pc->name, pc->index, pc->options) != 0) {
 			/* default matching */
@@ -4003,7 +4018,7 @@ static int try_enable_preferred_console(struct console *newcon,
 			if (newcon->index < 0)
 				newcon->index = pc->index;
 
-			if (is_braille_console_preferred(pc))
+			if (try_only_braille)
 				return _braille_register_console(newcon, pc);
 
 			err = console_call_setup(newcon, pc->options);
@@ -4019,6 +4034,17 @@ static int try_enable_preferred_console(struct console *newcon,
 	return -ENOENT;
 }
 
+static int try_enable_preferred_console(struct console *newcon,
+					bool user_specified)
+{
+	return __try_enable_preferred_console(newcon, user_specified, false);
+}
+
+static int try_enable_braille_console(struct console *newcon)
+{
+	return __try_enable_preferred_console(newcon, true, true);
+}
+
 /* Try to enable the console unconditionally */
 static void try_enable_default_console(struct console *newcon)
 {
@@ -4041,6 +4067,24 @@ static int try_enable_console(struct console *newcon)
 {
 	int err;
 
+	/*
+	 * First, try to enable the console driver as a Braille console.
+	 * It would have metadata in the preferred_consoles[] array.
+	 * But it won't be counted as @preferred_console because
+	 * it does not get printk() messages and is not associated
+	 * with /dev/console.
+	 *
+	 * Note that it might succeed also when the driver has a match()
+	 * callback and it took over a boot console. In this case,
+	 * the driver will continue working as a classic non-Braille
+	 * console.
+	 */
+	if (want_braille_console) {
+		err = try_enable_braille_console(newcon);
+		if (err != -ENOENT)
+			return err;
+	}
+
 	/*
 	 * See if we want to enable this console driver by default.
 	 *
-- 
2.54.0


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v4 09/11] printk: Do not set Braille console as preferred_console
  2026-06-04 10:14 [PATCH v4 00/11] printk: Clean up preferred console handling Petr Mladek
                   ` (7 preceding siblings ...)
  2026-06-04 10:14 ` [PATCH v4 08/11] printk: Try to register each console as Braille first Petr Mladek
@ 2026-06-04 10:14 ` Petr Mladek
  2026-06-04 10:14 ` [PATCH v4 10/11] printk: Modify try_enable_default_console() to return error/success Petr Mladek
                   ` (2 subsequent siblings)
  11 siblings, 0 replies; 19+ messages in thread
From: Petr Mladek @ 2026-06-04 10:14 UTC (permalink / raw)
  To: John Ogness
  Cc: Sergey Senozhatsky, Steven Rostedt, Marcos Paulo de Souza,
	Chris Down, Naveen Kumar Chaudhary, linux-kernel, Petr Mladek

The Braille console reuses the framework for handling consoles preferred
via the command line. However, it is a special-purpose interface that
neither receives standard printk messages nor associates with
/dev/console.

Currently, the "preferred_dev_console" variable can point to a Braille
console entry in the "preferred_consoles[]" array. This occurs if an
entry was first created for a non-Braille console, but a later 'console='
parameter redefined it as a Braille console.

Since a Braille console will only ever be enabled as such, it should not
be tracked as the primary system console. Adjust the logic to ensure
"preferred_dev_console" continues to point to the previously designated
normal console instead.

Signed-off-by: Petr Mladek <pmladek@suse.com>
---
 kernel/printk/printk.c | 20 +++++++++++++++++---
 1 file changed, 17 insertions(+), 3 deletions(-)

diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index 3a645558c18a..a4eaf157cece 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -367,6 +367,7 @@ static int console_locked;
 static struct preferred_console preferred_consoles[MAX_PREFERRED_CONSOLES];
 
 static int preferred_dev_console = -1;
+static int preferred_dev_console_prev = -1;
 static bool want_braille_console;
 int console_set_on_cmdline;
 EXPORT_SYMBOL(console_set_on_cmdline);
@@ -2566,10 +2567,11 @@ asmlinkage __visible void early_printk(const char *fmt, ...)
  *	Note that a valid @name and @idx will get assigned later when
  *	@devname matches during the device initialization.
  *   2. Specify @brl_options if the console should be enabled as
- *	a Braille console [*]
+ *	a Braille console.
  *   3. Only matching entries can be updated.
  *   4. @options passed via the command line are used when the same
  *	console is preferred also by some platform-specific code.
+ *   5. Braille console is never associated with /dev/console.[*]
  *
  * [*] Braille console is using the mechanism for registering consoles
  *     but it is very special. It is primarily used for user interaction
@@ -2649,10 +2651,22 @@ static int add_or_update_preferred_console(unsigned int i,
 
 	braille_update_options(pc, brl_options);
 
-	if (brl_options)
+	/*
+	 * The last preferred console should get associated with /dev/console.
+	 * Except for the Braille console which can't get associated with
+	 * /dev/console. One level history should be enough because only one,
+	 * the VisioBraille device, is supported at the moment.
+	 */
+	if (brl_options) {
 		want_braille_console = true;
-	else
+		if (preferred_dev_console == i) {
+			preferred_dev_console = preferred_dev_console_prev;
+			preferred_dev_console_prev = -1;
+		}
+	} else if (!is_braille_console_preferred(pc)) {
+		preferred_dev_console_prev = preferred_dev_console;
 		preferred_dev_console = i;
+	}
 
 	/*
 	 * @pc console was defined by the user on the command line.
-- 
2.54.0


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v4 10/11] printk: Modify try_enable_default_console() to return error/success
  2026-06-04 10:14 [PATCH v4 00/11] printk: Clean up preferred console handling Petr Mladek
                   ` (8 preceding siblings ...)
  2026-06-04 10:14 ` [PATCH v4 09/11] printk: Do not set Braille console as preferred_console Petr Mladek
@ 2026-06-04 10:14 ` Petr Mladek
  2026-06-04 10:14 ` [PATCH v4 11/11] printk: Try enable preferred consoles only when there are any Petr Mladek
  2026-07-14  8:42 ` [PATCH v4 00/11] printk: Clean up preferred console handling Petr Mladek
  11 siblings, 0 replies; 19+ messages in thread
From: Petr Mladek @ 2026-06-04 10:14 UTC (permalink / raw)
  To: John Ogness
  Cc: Sergey Senozhatsky, Steven Rostedt, Marcos Paulo de Souza,
	Chris Down, Naveen Kumar Chaudhary, linux-kernel, Petr Mladek

Currently, try_enable_default_console() has a void return type, and
its potential failures (specifically when console_call_setup() fails)
are ignored by its caller, try_enable_console().

Modify try_enable_default_console() to return error/success, and update
try_enable_console() to capture and propagate this error.

With this change, try_enable_console() will no longer return success
for a pre-enabled console if try_enable_default_console() failed (such
as when newcon->setup() fails). While this might change existing behavior,
it is the correct approach to ensure setup errors are not silently ignored
or treated as successful registrations simply because the console was
marked pre-enabled.

Assisted-by: Gemini:gemini-3 # commit message
Signed-off-by: Petr Mladek <pmladek@suse.com>
---
 kernel/printk/printk.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index a4eaf157cece..b0df4a23252d 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -4060,18 +4060,23 @@ static int try_enable_braille_console(struct console *newcon)
 }
 
 /* Try to enable the console unconditionally */
-static void try_enable_default_console(struct console *newcon)
+static int try_enable_default_console(struct console *newcon)
 {
+	int err;
+
 	if (newcon->index < 0)
 		newcon->index = 0;
 
-	if (console_call_setup(newcon, NULL) != 0)
-		return;
+	err = console_call_setup(newcon, NULL);
+	if (err)
+		return err;
 
 	newcon->flags |= CON_ENABLED;
 
 	if (newcon->device)
 		newcon->flags |= CON_CONSDEV;
+
+	return 0;
 }
 
 #define console_first()				\
@@ -4114,7 +4119,9 @@ static int try_enable_console(struct console *newcon)
 	if (preferred_dev_console < 0) {
 		if (hlist_empty(&console_list) || !console_first()->device ||
 		    console_first()->flags & CON_BOOT) {
-			try_enable_default_console(newcon);
+			err = try_enable_default_console(newcon);
+			if (err != -ENOENT)
+				return err;
 		}
 	}
 
-- 
2.54.0


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v4 11/11] printk: Try enable preferred consoles only when there are any
  2026-06-04 10:14 [PATCH v4 00/11] printk: Clean up preferred console handling Petr Mladek
                   ` (9 preceding siblings ...)
  2026-06-04 10:14 ` [PATCH v4 10/11] printk: Modify try_enable_default_console() to return error/success Petr Mladek
@ 2026-06-04 10:14 ` Petr Mladek
  2026-07-14  8:42 ` [PATCH v4 00/11] printk: Clean up preferred console handling Petr Mladek
  11 siblings, 0 replies; 19+ messages in thread
From: Petr Mladek @ 2026-06-04 10:14 UTC (permalink / raw)
  To: John Ogness
  Cc: Sergey Senozhatsky, Steven Rostedt, Marcos Paulo de Souza,
	Chris Down, Naveen Kumar Chaudhary, linux-kernel, Petr Mladek

try_enable_preferred_console() used to be always called because it
had several hidden effects, namely:

- returned success when a console was pre-enabled using CON_ENABLED
  flag.

- enabled Braille consoles which were ignored by "preferred_dev_console"
  because they were not associated with /dev/console.

- returned success when a console was enabled by default because
  try_enable_default_console() did not return success.

The first two hidden effects were removed in previous patches. The already
fixed handling of pre-enabled consoles actually helps even the 3rd case.
try_enable_default_console() sets CON_ENABLED flag on success and
is later handled as pre-enabled.

Prevent any future hidden effects and call try_enable_preferred_console()
only when some console is preferred.

No behavior change.

Signed-off-by: Petr Mladek <pmladek@suse.com>
---
 kernel/printk/printk.c | 51 ++++++++++++++++++++++++------------------
 1 file changed, 29 insertions(+), 22 deletions(-)

diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index b0df4a23252d..21a9c78f4deb 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -4084,7 +4084,7 @@ static int try_enable_default_console(struct console *newcon)
 
 static int try_enable_console(struct console *newcon)
 {
-	int err;
+	int err = -ENOENT;
 
 	/*
 	 * First, try to enable the console driver as a Braille console.
@@ -4104,19 +4104,29 @@ static int try_enable_console(struct console *newcon)
 			return err;
 	}
 
-	/*
-	 * See if we want to enable this console driver by default.
-	 *
-	 * Nope when a console is preferred by the command line, device
-	 * tree, or SPCR.
-	 *
-	 * The first real console with tty binding (driver) wins. More
-	 * consoles might get enabled before the right one is found.
-	 *
-	 * Note that a console with tty binding will have CON_CONSDEV
-	 * flag set and will be first in the list.
-	 */
-	if (preferred_dev_console < 0) {
+	if (preferred_dev_console >= 0) {
+		/* See if this console matches one we selected on the command line */
+		err = try_enable_preferred_console(newcon, true);
+		if (err != -ENOENT)
+			return err;
+
+		/* If not, try to match against the platform default(s) */
+		err = try_enable_preferred_console(newcon, false);
+		if (err != -ENOENT)
+			return err;
+	} else {
+		/*
+		 * See if we want to enable this console driver by default.
+		 *
+		 * Nope when a console is preferred by the command line, device
+		 * tree, or SPCR.
+		 *
+		 * The first real console with tty binding (driver) wins. More
+		 * consoles might get enabled before the right one is found.
+		 *
+		 * Note that a console with tty binding will have CON_CONSDEV
+		 * flag set and will be first in the list.
+		 */
 		if (hlist_empty(&console_list) || !console_first()->device ||
 		    console_first()->flags & CON_BOOT) {
 			err = try_enable_default_console(newcon);
@@ -4125,14 +4135,11 @@ static int try_enable_console(struct console *newcon)
 		}
 	}
 
-	/* See if this console matches one we selected on the command line */
-	err = try_enable_preferred_console(newcon, true);
-	if (err != -ENOENT)
-		return err;
-
-	/* If not, try to match against the platform default(s) */
-	err = try_enable_preferred_console(newcon, false);
-	if (err != -ENOENT)
+	/*
+	 * Make sure that pre-enabled consoles won't get registered when
+	 * something went wrong.
+	 */
+	if (WARN_ON(err != -ENOENT))
 		return err;
 
 	/*
-- 
2.54.0


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH v4 00/11]  printk: Clean up preferred console handling
  2026-06-04 10:14 [PATCH v4 00/11] printk: Clean up preferred console handling Petr Mladek
                   ` (10 preceding siblings ...)
  2026-06-04 10:14 ` [PATCH v4 11/11] printk: Try enable preferred consoles only when there are any Petr Mladek
@ 2026-07-14  8:42 ` Petr Mladek
  11 siblings, 0 replies; 19+ messages in thread
From: Petr Mladek @ 2026-07-14  8:42 UTC (permalink / raw)
  To: John Ogness
  Cc: Sergey Senozhatsky, Steven Rostedt, Marcos Paulo de Souza,
	Chris Down, Naveen Kumar Chaudhary, linux-kernel

On Thu 2026-06-04 12:14:47, Petr Mladek wrote:
> this patchset does some clean up of the code for handling preferred
> consoles in the console registration code.
> 
> Petr Mladek (11):
>   printk: Handle pre-enabled consoles in the top-level
>     register_console()
>   printk: Rename struct console_cmdline to preferred_console
>   printk: Rename preferred_console to preferred_dev_console
>   printk: Separate code for adding/updating preferred console metadata
>   printk: Cleanup _braille_(un)register_console() wrappers
>   console/braille: Lock console->setup() call during the registration
>   printk: Separate code for enabling console
>   printk: Try to register each console as Braille first
>   printk: Do not set Braille console as preferred_console
>   printk: Modify try_enable_default_console() to return error/success
>   printk: Try enable preferred consoles only when there are any

Gently ping.

I know that this patchset is not easy for review. Well, if anyone
could look at the first patch at least. It prevents a potential
out-of-bound access, ...

Best Regards,
Petr

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v4 01/11] printk: Handle pre-enabled consoles in the top-level register_console()
  2026-06-04 10:14 ` [PATCH v4 01/11] printk: Handle pre-enabled consoles in the top-level register_console() Petr Mladek
@ 2026-07-14 13:33   ` John Ogness
  2026-07-15 14:43     ` Petr Mladek
  0 siblings, 1 reply; 19+ messages in thread
From: John Ogness @ 2026-07-14 13:33 UTC (permalink / raw)
  To: Petr Mladek
  Cc: Sergey Senozhatsky, Steven Rostedt, Marcos Paulo de Souza,
	Chris Down, Naveen Kumar Chaudhary, linux-kernel, Petr Mladek

On 2026-06-04, Petr Mladek <pmladek@suse.com> wrote:
> The function try_enable_preferred_console() currently has the
> non-obvious side effect of returning success for consoles that are
> already pre-enabled. This obscures the logic flow during console
> registration.
>
> Move the check for pre-enabled consoles directly into the top-level
> register_console(). This change makes the handling of pre-enabled
> consoles explicit and easier to follow.
>
> Furthermore, this separation lays the groundwork for future cleanups
> where try_enable_preferred_console() can be restricted to cases where
> an entry actually exists in the console_cmdline[] array.
>
> Also it fixes a possible out-of-bound access when the console_cmdline[]
> array is full and no console matched. In fact, the check of
> c->user_specified did not make much sense. The new console either
> matched and was handled in the for-cycle. Or it did not match
> and then *c pointed to an unused entry.
>
> Possible behavior change:
>
> try_enable_preferred_console() will newly be called also with
> @user_specified parameter set to "false" when it failed with the "true"
> variant. But it looks like the right way to do. It will allow to call
> newcon->setup() when the console was preferred by some platform
> specific code.

I agree that it is the right way to do it. But I am a bit confused about
the pre-enabled consoles.

I understood it that a driver manually sets CON_ENABLED because that
console is not enabled via normal methods ("console=" or "stdout" in
device-tree). There are several examples of this (netconsole, pstore,
nfcon, kgdb). But then I see drivers like:

- drivers/tty/ehv_bytechan.c
- drivers/tty/serial/ma35d1_serial.c
- drivers/tty/serial/mux.c

that are also setting CON_ENABLED, but seem that they should be
specified in the normal way.

Are these drivers falsely setting CON_ENABLED? Is there a clear
description about when a driver should manually set CON_ENABLED?

> Reported-by: Naveen Kumar Chaudhary <naveen.osdev@gmail.com> # out-of-bound access
> Closes: https://lore.kernel.org/r/7sq4tr2nmlz32tvkf6vpsghv6exvqfghsrlvywjcqihzsqqbf7@bspclmti5xg4
> Signed-off-by: Petr Mladek <pmladek@suse.com>

Despite my open questions about CON_ENABLED, this patch is simplifying
the pre-enabled path and correctly specifying the possible behavior
change.

Reviewed-by: John Ogness <john.ogness@linutronix.de>

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v4 04/11] printk: Separate code for adding/updating preferred console metadata
  2026-06-04 10:14 ` [PATCH v4 04/11] printk: Separate code for adding/updating preferred console metadata Petr Mladek
@ 2026-07-14 14:35   ` John Ogness
  2026-07-15 15:52     ` Petr Mladek
  0 siblings, 1 reply; 19+ messages in thread
From: John Ogness @ 2026-07-14 14:35 UTC (permalink / raw)
  To: Petr Mladek
  Cc: Sergey Senozhatsky, Steven Rostedt, Marcos Paulo de Souza,
	Chris Down, Naveen Kumar Chaudhary, linux-kernel, Petr Mladek

On 2026-06-04, Petr Mladek <pmladek@suse.com> wrote:
> The logic for adding or updating a preferred console is currently
> duplicated within __add_preferred_console(), making the code difficult
> to follow and prone to consistency issues.
>
> Introduce update_preferred_console() to centralize the initialization
> and updating of struct preferred_console entries. This refactoring
> explicitly defines and enforces the following rules:
>
> 1. Console names and/or indexes are not set when a console is preferred
>    via devname; these are resolved later during device matching.
> 2. Console names are only added alongside a valid index.
> 3. Only matching entries are updated.
> 4. Console and Braille options are never cleared. They are updated
>    only via the command line.
> 5. The global 'preferred_dev_console' index and 'console_set_on_cmdline'
>    flag are updated consistently.
>
> Additionally, rename braille_set_options() to braille_update_options()
> to better reflect its conditional behavior.
>
> Behavior change:
>
> The original code never updated the preferred console options
> when it was preferred more times, e.g. via the command line
> and/or some platform specific code, e.g. SPCR or device tree.
>
> The new code explicitly allows to update the console options
> when they are preferred over the command line.
>
> It mostly worked even before but only because the command line was
> processed early enough before handling SPCR, device tree, or other
> platform specific init code.

As you previously mentioned, this is a recipe for regressions. IMHO the
real issue is not that the behavior is changed, but rather that there is
no official ordering for SPCR/DT/command-line across all platforms.

Also, although a later specification can override previous options, it
cannot clear them.

I am just wondering if the previous behavior should be preserved until
someone implements a full solution with proper official option ordering
and override capabilities.

John

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v4 06/11] console/braille: Lock console->setup() call during the registration
  2026-06-04 10:14 ` [PATCH v4 06/11] console/braille: Lock console->setup() call during the registration Petr Mladek
@ 2026-07-15 14:41   ` John Ogness
  0 siblings, 0 replies; 19+ messages in thread
From: John Ogness @ 2026-07-15 14:41 UTC (permalink / raw)
  To: Petr Mladek
  Cc: Sergey Senozhatsky, Steven Rostedt, Marcos Paulo de Souza,
	Chris Down, Naveen Kumar Chaudhary, linux-kernel, Petr Mladek,
	Sashiko

On 2026-06-04, Petr Mladek <pmladek@suse.com> wrote:
> diff --git a/drivers/accessibility/braille/braille_console.c b/drivers/accessibility/braille/braille_console.c
> index 7b324329882f..9ed3abffb687 100644
> --- a/drivers/accessibility/braille/braille_console.c
> +++ b/drivers/accessibility/braille/braille_console.c
> @@ -356,7 +356,9 @@ int braille_register_console(struct console *console, int index,
>  	if (braille_co)
>  		return -ENODEV;
>  	if (console->setup) {
> +		console_lock();
>  		ret = console->setup(console, console_options);
> +		console_unlock();
>  		if (ret != 0)
>  			return ret;
>  	}

This block is open coding an existing helper that was created for
exactly this purpose. The code could look like:

	ret = console_call_setup(console, console_options);
	if (ret != 0)
		return ret;

...if console_call_setup() was (for example) a static inline in
console.h.

For this patch as-is:

Acked-by: John Ogness <john.ogness@linutronix.de>

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v4 01/11] printk: Handle pre-enabled consoles in the top-level register_console()
  2026-07-14 13:33   ` John Ogness
@ 2026-07-15 14:43     ` Petr Mladek
  0 siblings, 0 replies; 19+ messages in thread
From: Petr Mladek @ 2026-07-15 14:43 UTC (permalink / raw)
  To: John Ogness
  Cc: Sergey Senozhatsky, Steven Rostedt, Marcos Paulo de Souza,
	Chris Down, Naveen Kumar Chaudhary, linux-kernel

On Tue 2026-07-14 15:39:45, John Ogness wrote:
> On 2026-06-04, Petr Mladek <pmladek@suse.com> wrote:
> > The function try_enable_preferred_console() currently has the
> > non-obvious side effect of returning success for consoles that are
> > already pre-enabled. This obscures the logic flow during console
> > registration.
> >
> > Move the check for pre-enabled consoles directly into the top-level
> > register_console(). This change makes the handling of pre-enabled
> > consoles explicit and easier to follow.
> >
> > Furthermore, this separation lays the groundwork for future cleanups
> > where try_enable_preferred_console() can be restricted to cases where
> > an entry actually exists in the console_cmdline[] array.
> >
> > Also it fixes a possible out-of-bound access when the console_cmdline[]
> > array is full and no console matched. In fact, the check of
> > c->user_specified did not make much sense. The new console either
> > matched and was handled in the for-cycle. Or it did not match
> > and then *c pointed to an unused entry.
> >
> > Possible behavior change:
> >
> > try_enable_preferred_console() will newly be called also with
> > @user_specified parameter set to "false" when it failed with the "true"
> > variant. But it looks like the right way to do. It will allow to call
> > newcon->setup() when the console was preferred by some platform
> > specific code.
> 
> I agree that it is the right way to do it. But I am a bit confused about
> the pre-enabled consoles.

You are not alone ;-)

> I understood it that a driver manually sets CON_ENABLED because that
> console is not enabled via normal methods ("console=" or "stdout" in
> device-tree). There are several examples of this (netconsole, pstore,
> nfcon, kgdb).

My understanding is that these set CON_ENABLED because they
are enabled another way, e.g. netconsole=, ramoops, ...

And console_cmdline was originally only for consoles defined
via the console= parameter, ... And it probably does not make
sense to add a fake entry into console_cmdline[] array just
to make sure that the console driver would match something.

> - drivers/tty/ehv_bytechan.c
> - drivers/tty/serial/ma35d1_serial.c
> - drivers/tty/serial/mux.c

My expectation is that these are some special platforms where the
authors just wanted them enabled out-of-box.

BTW, similar situation is also in handling SPCR on different
architectures. I have investigated it some time ago and found
the following. The console proposed by SPCR is:

  + ignored on "loongarch" architecture.

  + requested on "x86" when "acpi=spcr" is defined on the command line,
    otherwise ignored.

  + requested on "arm" architecture unless "acpi=nospcr" is defined on
    the command line.

  + requested on "riscv" architecture unless acpi is disabled

So, again, some architectures get a particular console preferred
by the particular HW by default. Others get the kernel default
which is typically a graphical ttyX.

> that are also setting CON_ENABLED, but seem that they should be
> specified in the normal way.
> 
> Are these drivers falsely setting CON_ENABLED? Is there a clear
> description about when a driver should manually set CON_ENABLED?

I think that it is intentional. And Marcos is working on some
clean up to remove the hack with CON_ENABLED completely, see
https://lore.kernel.org/all/20251227-printk-cleanup-part3-v1-0-21a291bcf197@suse.com/

> > Reported-by: Naveen Kumar Chaudhary <naveen.osdev@gmail.com> # out-of-bound access
> > Closes: https://lore.kernel.org/r/7sq4tr2nmlz32tvkf6vpsghv6exvqfghsrlvywjcqihzsqqbf7@bspclmti5xg4
> > Signed-off-by: Petr Mladek <pmladek@suse.com>
> 
> Despite my open questions about CON_ENABLED, this patch is simplifying
> the pre-enabled path and correctly specifying the possible behavior
> change.
> 
> Reviewed-by: John Ogness <john.ogness@linutronix.de>

Thanks for review.

I am going to queue this patch for 7.3. I prevents a rather
theoretical out-of-bound access to console_cmdline array,

Best Regards,
Petr

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v4 04/11] printk: Separate code for adding/updating preferred console metadata
  2026-07-14 14:35   ` John Ogness
@ 2026-07-15 15:52     ` Petr Mladek
  0 siblings, 0 replies; 19+ messages in thread
From: Petr Mladek @ 2026-07-15 15:52 UTC (permalink / raw)
  To: John Ogness
  Cc: Sergey Senozhatsky, Steven Rostedt, Marcos Paulo de Souza,
	Chris Down, Naveen Kumar Chaudhary, linux-kernel

On Tue 2026-07-14 16:41:05, John Ogness wrote:
> On 2026-06-04, Petr Mladek <pmladek@suse.com> wrote:
> > The logic for adding or updating a preferred console is currently
> > duplicated within __add_preferred_console(), making the code difficult
> > to follow and prone to consistency issues.
> >
> > Introduce update_preferred_console() to centralize the initialization
> > and updating of struct preferred_console entries. This refactoring
> > explicitly defines and enforces the following rules:
> >
> > 1. Console names and/or indexes are not set when a console is preferred
> >    via devname; these are resolved later during device matching.
> > 2. Console names are only added alongside a valid index.
> > 3. Only matching entries are updated.
> > 4. Console and Braille options are never cleared. They are updated
> >    only via the command line.
> > 5. The global 'preferred_dev_console' index and 'console_set_on_cmdline'
> >    flag are updated consistently.
> >
> > Additionally, rename braille_set_options() to braille_update_options()
> > to better reflect its conditional behavior.
> >
> > Behavior change:
> >
> > The original code never updated the preferred console options
> > when it was preferred more times, e.g. via the command line
> > and/or some platform specific code, e.g. SPCR or device tree.
> >
> > The new code explicitly allows to update the console options
> > when they are preferred over the command line.
> >
> > It mostly worked even before but only because the command line was
> > processed early enough before handling SPCR, device tree, or other
> > platform specific init code.
> 
> As you previously mentioned, this is a recipe for regressions. IMHO the
> real issue is not that the behavior is changed, but rather that there is
> no official ordering for SPCR/DT/command-line across all platforms.
>
> Also, although a later specification can override previous options, it
> cannot clear them.

Good point!

> I am just wondering if the previous behavior should be preserved until
> someone implements a full solution with proper official option ordering
> and override capabilities.

Hmm, this patch was supposed to define some sane rules. For example,
options defined on the command line would be used even when
the same console was first added, e.g. by SPCR code without options.

Or at least it looked ugly to keep the original behavior.

But maybe, it was not that good idea:

  + It is always a bad idea to "hide" behavior changes into
    a patch which primary does "just" a code clean up.

  + We need to be careful when adding new rules. And it might
    need longer discussion.

I am going to keep the original behavior in v5. And likely even leave
the behavior change for another patchset.

Best Regards,
Petr

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v4 07/11] printk: Separate code for enabling console
  2026-06-04 10:14 ` [PATCH v4 07/11] printk: Separate code for enabling console Petr Mladek
@ 2026-07-15 16:01   ` John Ogness
  0 siblings, 0 replies; 19+ messages in thread
From: John Ogness @ 2026-07-15 16:01 UTC (permalink / raw)
  To: Petr Mladek
  Cc: Sergey Senozhatsky, Steven Rostedt, Marcos Paulo de Souza,
	Chris Down, Naveen Kumar Chaudhary, linux-kernel, Petr Mladek

On 2026-06-04, Petr Mladek <pmladek@suse.com> wrote:
> There are several code paths which try to enable a newly registered
> console. Move the logic into a separate try_enable_console() function.
>
> It simplifies a bit the long register_console() function definition.
>
> Also followup patches are going to add even more code paths. And it will
> be easier to use "return" when it does not make sense to try other
> variants.
>
> No behavior change.
>
> Signed-off-by: Petr Mladek <pmladek@suse.com>

Reviewed-by: John Ogness <john.ogness@linutronix.de>

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2026-07-15 16:01 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-04 10:14 [PATCH v4 00/11] printk: Clean up preferred console handling Petr Mladek
2026-06-04 10:14 ` [PATCH v4 01/11] printk: Handle pre-enabled consoles in the top-level register_console() Petr Mladek
2026-07-14 13:33   ` John Ogness
2026-07-15 14:43     ` Petr Mladek
2026-06-04 10:14 ` [PATCH v4 02/11] printk: Rename struct console_cmdline to preferred_console Petr Mladek
2026-06-04 10:14 ` [PATCH v4 03/11] printk: Rename preferred_console to preferred_dev_console Petr Mladek
2026-06-04 10:14 ` [PATCH v4 04/11] printk: Separate code for adding/updating preferred console metadata Petr Mladek
2026-07-14 14:35   ` John Ogness
2026-07-15 15:52     ` Petr Mladek
2026-06-04 10:14 ` [PATCH v4 05/11] printk: Cleanup _braille_(un)register_console() wrappers Petr Mladek
2026-06-04 10:14 ` [PATCH v4 06/11] console/braille: Lock console->setup() call during the registration Petr Mladek
2026-07-15 14:41   ` John Ogness
2026-06-04 10:14 ` [PATCH v4 07/11] printk: Separate code for enabling console Petr Mladek
2026-07-15 16:01   ` John Ogness
2026-06-04 10:14 ` [PATCH v4 08/11] printk: Try to register each console as Braille first Petr Mladek
2026-06-04 10:14 ` [PATCH v4 09/11] printk: Do not set Braille console as preferred_console Petr Mladek
2026-06-04 10:14 ` [PATCH v4 10/11] printk: Modify try_enable_default_console() to return error/success Petr Mladek
2026-06-04 10:14 ` [PATCH v4 11/11] printk: Try enable preferred consoles only when there are any Petr Mladek
2026-07-14  8:42 ` [PATCH v4 00/11] printk: Clean up preferred console handling Petr Mladek

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox