This moves the loop that tries to match a newly registered console
with the command line or add_preferred_console list into a separate
helper, in order to be able to call it multiple times in subsequent
patches.
Signed-off-by: Benjamin Herrenschmidt <[email protected]>
---
Sorry for the delay, I got ... distracted :-)
I ended up going for the int return codes instead of the enum, there
aren't enough interesting cases anyways. I *hope* the logic doesn't
change from the eixsting code. There might be subtle differences
if a console is marked as brailled and fails to initialize but I doubt
it matters.
kernel/printk/printk.c | 107 +++++++++++++++++++++++++----------------
1 file changed, 66 insertions(+), 41 deletions(-)
diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index 1ef6f75d92f1..17602d7b7ffc 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -280,6 +280,7 @@ static struct console *exclusive_console;
static struct console_cmdline console_cmdline[MAX_CMDLINECONSOLES];
static int preferred_console = -1;
+static bool has_preferred_console;
int console_set_on_cmdline;
EXPORT_SYMBOL(console_set_on_cmdline);
@@ -2626,6 +2627,60 @@ static int __init keep_bootcon_setup(char *str)
early_param("keep_bootcon", keep_bootcon_setup);
+/*
+ * This is called by register_console() to try to match
+ * 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_new_console(struct console *newcon)
+{
+ struct console_cmdline *c;
+ int i;
+
+ for (i = 0, c = console_cmdline;
+ i < MAX_CMDLINECONSOLES && c->name[0];
+ i++, c++) {
+ if (!newcon->match ||
+ newcon->match(newcon, c->name, c->index, c->options) != 0) {
+ /* default matching */
+ BUILD_BUG_ON(sizeof(c->name) != sizeof(newcon->name));
+ if (strcmp(c->name, newcon->name) != 0)
+ continue;
+ if (newcon->index >= 0 &&
+ newcon->index != c->index)
+ continue;
+ if (newcon->index < 0)
+ newcon->index = c->index;
+
+ if (_braille_register_console(newcon, c))
+ return 0;
+
+ if (newcon->setup &&
+ newcon->setup(newcon, c->options) != 0)
+ return -EIO;
+ }
+ newcon->flags |= CON_ENABLED;
+ if (i == preferred_console) {
+ newcon->flags |= CON_CONSDEV;
+ has_preferred_console = true;
+ }
+ return 0;
+ }
+
+ /*
+ * Some consoles, such as pstore and netconsole, can be enabled even
+ * without matching.
+ */
+ if (newcon->flags & CON_ENABLED)
+ return 0;
+
+ return -ENOENT;
+}
+
/*
* The console driver calls this routine during kernel initialization
* to register the console printing procedure with printk() and to
@@ -2647,11 +2702,9 @@ early_param("keep_bootcon", keep_bootcon_setup);
*/
void register_console(struct console *newcon)
{
- int i;
unsigned long flags;
struct console *bcon = NULL;
- struct console_cmdline *c;
- static bool has_preferred;
+ int err;
if (console_drivers)
for_each_console(bcon)
@@ -2678,15 +2731,15 @@ void register_console(struct console *newcon)
if (console_drivers && console_drivers->flags & CON_BOOT)
bcon = console_drivers;
- if (!has_preferred || bcon || !console_drivers)
- has_preferred = preferred_console >= 0;
+ if (!has_preferred_console || bcon || !console_drivers)
+ has_preferred_console = preferred_console >= 0;
/*
* See if we want to use this console driver. If we
* didn't select a console we take the first one
* that registers here.
*/
- if (!has_preferred) {
+ if (!has_preferred_console) {
if (newcon->index < 0)
newcon->index = 0;
if (newcon->setup == NULL ||
@@ -2694,48 +2747,20 @@ void register_console(struct console *newcon)
newcon->flags |= CON_ENABLED;
if (newcon->device) {
newcon->flags |= CON_CONSDEV;
- has_preferred = true;
+ has_preferred_console = true;
}
}
}
/*
- * See if this console matches one we selected on
- * the command line.
+ * See if this console matches one we selected on
+ * the command line or if it was statically enabled
*/
- for (i = 0, c = console_cmdline;
- i < MAX_CMDLINECONSOLES && c->name[0];
- i++, c++) {
- if (!newcon->match ||
- newcon->match(newcon, c->name, c->index, c->options) != 0) {
- /* default matching */
- BUILD_BUG_ON(sizeof(c->name) != sizeof(newcon->name));
- if (strcmp(c->name, newcon->name) != 0)
- continue;
- if (newcon->index >= 0 &&
- newcon->index != c->index)
- continue;
- if (newcon->index < 0)
- newcon->index = c->index;
-
- if (_braille_register_console(newcon, c))
- return;
-
- if (newcon->setup &&
- newcon->setup(newcon, c->options) != 0)
- break;
- }
+ err = try_enable_new_console(newcon);
- newcon->flags |= CON_ENABLED;
- if (i == preferred_console) {
- newcon->flags |= CON_CONSDEV;
- has_preferred = true;
- }
- break;
- }
-
- if (!(newcon->flags & CON_ENABLED))
- return;
+ /* printk() messages are not printed to the Braille console. */
+ if (err || newcon->flags & CON_BRL)
+ return;
/*
* If we have a bootconsole, and are switching to a real console,
On Thu 2020-02-06 15:02:18, Benjamin Herrenschmidt wrote:
> This moves the loop that tries to match a newly registered console
> with the command line or add_preferred_console list into a separate
> helper, in order to be able to call it multiple times in subsequent
> patches.
>
> Signed-off-by: Benjamin Herrenschmidt <[email protected]>
Looks fine to me.
Reviewed-by: Petr Mladek <[email protected]>
Just few nits below.
> @@ -2626,6 +2627,60 @@ static int __init keep_bootcon_setup(char *str)
> + /*
> + * Some consoles, such as pstore and netconsole, can be enabled even
> + * without matching.
> + */
There are few lines in the patchset where the indentation is done
by spaces instead of tabs. The above 3 lines are just one example.
I'll fix this when pushing. But please, be more careful next time ;-)
I suggest to use some more clever editor that helps with code
formatting.
Also I suggest to run ./scripts/checkpatch.pl on the patches
before sending.
Also the three patches were not send in a single thread so that
it was harder to find all the pieces. I personally use:
git format-patch --cover-letter origin/master -o some-dir
./scripts/checkpatch.pl some-dir/*
$> edit some-dir/0000-*.patch
git send-email --smtp-server=... --to= --cc= ... some-dir/*
Best Regards,
Petr
On Tue, 2020-02-11 at 15:29 +0100, Petr Mladek wrote:
> > @@ -2626,6 +2627,60 @@ static int __init keep_bootcon_setup(char *str)
> > + /*
> > + * Some consoles, such as pstore and netconsole, can be enabled even
> > + * without matching.
> > + */
>
> There are few lines in the patchset where the indentation is done
> by spaces instead of tabs. The above 3 lines are just one example.
Odd. Something must have gone wrong with my emacs config when I
switched laptops, I'll have a look thanks.
> I'll fix this when pushing. But please, be more careful next time ;-)
> I suggest to use some more clever editor that helps with code
> formatting.
I normally do :)
> Also I suggest to run ./scripts/checkpatch.pl on the patches
> before sending.
Yeah I never quite got that knack, I still keep forgetting ... the doom
of old habits, I didn't even when I was maintainer of arch/powerpc ...
oops :)
But yeah, I'll try to remember next time.
> Also the three patches were not send in a single thread so that
> it was harder to find all the pieces. I personally use:
>
> git format-patch --cover-letter origin/master -o some-dir
> ./scripts/checkpatch.pl some-dir/*
> $> edit some-dir/0000-*.patch
> git send-email --smtp-server=... --to= --cc= ... some-dir/*
I usually do when it's more than a couple of patches but yeah, again,
bad old habits. Sorry about that.
Cheers,
Ben.
On (20/02/06 15:02), Benjamin Herrenschmidt wrote:
[..]
> +
> + /*
> + * Some consoles, such as pstore and netconsole, can be enabled even
> + * without matching.
> + */
> + if (newcon->flags & CON_ENABLED)
> + return 0;
> +
> + return -ENOENT;
> +}
Looks good to me // modulo checkpatch warnings //
Reviewed-by: Sergey Senozhatsky <[email protected]>
-ss