In case of using console="" or console=null set console_set_on_cmdline=1
to disable chosen{ "stdout-path" } node from devicetree.
To jump out from drivers/of/base.c, line 2087 of_console_check function
with false.
Signed-off-by: Andre Kalb <[email protected]>
---
kernel/printk/printk.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index 82abfaf3c2aa..df5ab35b8af2 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -2385,6 +2385,7 @@ static int __init console_setup(char *str)
*/
if (str[0] == 0 || strcmp(str, "null") == 0) {
__add_preferred_console("ttynull", 0, NULL, NULL, true);
+ console_set_on_cmdline = 1;
return 1;
}
--
2.31.1
On Fri 2022-01-28 00:00:52, Andre Kalb wrote:
> In case of using console="" or console=null set console_set_on_cmdline=1
> to disable chosen{ "stdout-path" } node from devicetree.
>
> To jump out from drivers/of/base.c, line 2087 of_console_check function
> with false.
It makes sense. I would just fix it slightly different way, see below.
> Signed-off-by: Andre Kalb <[email protected]>
> ---
> kernel/printk/printk.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> index 82abfaf3c2aa..df5ab35b8af2 100644
> --- a/kernel/printk/printk.c
> +++ b/kernel/printk/printk.c
> @@ -2385,6 +2385,7 @@ static int __init console_setup(char *str)
> */
> if (str[0] == 0 || strcmp(str, "null") == 0) {
> __add_preferred_console("ttynull", 0, NULL, NULL,
> true);
> + console_set_on_cmdline = 1;
We basically always need to set it when __add_preferred_console() is
called with user_specified == true.
Therefore, we should move the assigment into
__add_preferred_console(). We should do something like:
diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index 82abfaf3c2aa..3654695ca5d2 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -2324,6 +2324,15 @@ asmlinkage __visible void early_printk(const char *fmt, ...)
}
#endif
+static void set_user_specified(struct console_cmdline *c, bool user_specified)
+{
+ if (!user_specified)
+ return;
+
+ c->user_specified = true;
+ console_set_on_cmdline = 1;
+}
+
static int __add_preferred_console(char *name, int idx, char *options,
char *brl_options, bool user_specified)
{
@@ -2340,8 +2349,7 @@ static int __add_preferred_console(char *name, int idx, char *options,
if (strcmp(c->name, name) == 0 && c->index == idx) {
if (!brl_options)
preferred_console = i;
- if (user_specified)
- c->user_specified = true;
+ set_user_specified(c, user_specified);
return 0;
}
}
@@ -2351,7 +2359,7 @@ static int __add_preferred_console(char *name, int idx, char *options,
preferred_console = i;
strlcpy(c->name, name, sizeof(c->name));
c->options = options;
- c->user_specified = user_specified;
+ set_user_specified(c, user_specified);
braille_set_options(c, brl_options);
c->index = idx;
@@ -2417,7 +2425,6 @@ static int __init console_setup(char *str)
*s = 0;
__add_preferred_console(buf, idx, options, brl_options, true);
- console_set_on_cmdline = 1;
return 1;
}
__setup("console=", console_setup);
Best Regards,
Petr
From: Andre Kalb <[email protected]>
In case of using console="" or console=null
set console_set_on_cmdline=1 to disable "stdout-path" node from DT.
We basically need to set it every time when __add_preferred_console()
is called with parameter 'user_specified' set.
Therefore we can move setting it into a helper function that is
called from __add_preferred_console().
Suggested-by: Petr Mladek <[email protected]>
Signed-off-by: Andre Kalb <[email protected]>
---
Changelog v1 to v2:
Move console_set_on_cmdling into separate function set_user_specified(), which is called from
__add_preferred_console().
The old patch v1 could be used to backport to stable 5.4 and lower.
---
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 82abfaf3c2aa..3654695ca5d2 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -2324,6 +2324,15 @@ asmlinkage __visible void early_printk(const char *fmt, ...)
}
#endif
+static void set_user_specified(struct console_cmdline *c, bool user_specified)
+{
+ if (!user_specified)
+ return;
+
+ c->user_specified = true;
+ console_set_on_cmdline = 1;
+}
+
static int __add_preferred_console(char *name, int idx, char *options,
char *brl_options, bool user_specified)
{
@@ -2340,8 +2349,7 @@ static int __add_preferred_console(char *name, int idx, char *options,
if (strcmp(c->name, name) == 0 && c->index == idx) {
if (!brl_options)
preferred_console = i;
- if (user_specified)
- c->user_specified = true;
+ set_user_specified(c, user_specified);
return 0;
}
}
@@ -2351,7 +2359,7 @@ static int __add_preferred_console(char *name, int idx, char *options,
preferred_console = i;
strlcpy(c->name, name, sizeof(c->name));
c->options = options;
- c->user_specified = user_specified;
+ set_user_specified(c, user_specified);
braille_set_options(c, brl_options);
c->index = idx;
@@ -2417,7 +2425,6 @@ static int __init console_setup(char *str)
*s = 0;
__add_preferred_console(buf, idx, options, brl_options, true);
- console_set_on_cmdline = 1;
return 1;
}
__setup("console=", console_setup);
--
2.31.1
On (22/02/14 14:21), Andre Kalb wrote:
> +static void set_user_specified(struct console_cmdline *c, bool user_specified)
> +{
> + if (!user_specified)
> + return;
> +
> + c->user_specified = true;
> + console_set_on_cmdline = 1;
> +}
In original code we always set c->user_specified. Is it guaranteed that
->user_specified is properly initialized to 0? Maybe can do something like:
static void set_user_specified(struct console_cmdline *c, bool user_specified)
{
c->user_specified = user_specified;
if (!user_specified)
return;
console_set_on_cmdline = 1;
}
On Tue 2022-02-15 12:15:58, Sergey Senozhatsky wrote:
> On (22/02/14 14:21), Andre Kalb wrote:
> > +static void set_user_specified(struct console_cmdline *c, bool user_specified)
> > +{
> > + if (!user_specified)
> > + return;
> > +
> > + c->user_specified = true;
> > + console_set_on_cmdline = 1;
> > +}
>
> In original code we always set c->user_specified. Is it guaranteed that
> ->user_specified is properly initialized to 0? Maybe can do something like:
It is guaranteed. console_cmdline is a static array initialized with
zeroes. The 2nd set_user_specified() call is done for a not-yet-used
slot in the array, so it must be zero.
> static void set_user_specified(struct console_cmdline *c, bool user_specified)
> {
> c->user_specified = user_specified;
This will change the behavior for the 1st set_user_specified() call.
It happens when the same console is added more times by device tree,
SPCR, and/or command line. c->user_specified must stay "true" when
at least one __add_preferred_console() call added it from the command line.
> if (!user_specified)
> return;
>
> console_set_on_cmdline = 1;
> }
I agree that it is not obvious. It would make sense to add a comment
into the code. I am going to propose something in a reply to the
original post.
Best Regards,
Petr
On Mon 2022-02-14 14:21:29, Andre Kalb wrote:
> From: Andre Kalb <[email protected]>
>
> In case of using console="" or console=null
> set console_set_on_cmdline=1 to disable "stdout-path" node from DT.
>
> We basically need to set it every time when __add_preferred_console()
> is called with parameter 'user_specified' set.
> Therefore we can move setting it into a helper function that is
> called from __add_preferred_console().
>
> Suggested-by: Petr Mladek <[email protected]>
> Signed-off-by: Andre Kalb <[email protected]>
> ---
> Changelog v1 to v2:
> Move console_set_on_cmdling into separate function set_user_specified(), which is called from
> __add_preferred_console().
>
> The old patch v1 could be used to backport to stable 5.4 and lower.
> ---
> 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 82abfaf3c2aa..3654695ca5d2 100644
> --- a/kernel/printk/printk.c
> +++ b/kernel/printk/printk.c
> @@ -2324,6 +2324,15 @@ asmlinkage __visible void early_printk(const char *fmt, ...)
> }
> #endif
>
We should add a comment explaining the less obvious behavior as
discussed in the thread. Something like:
> +static void set_user_specified(struct console_cmdline *c, bool user_specified)
> +{
> + if (!user_specified)
> + return;
> +
/*
* @c 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;
/* At least one console defined by the user on the command line. */
> + console_set_on_cmdline = 1;
> +}
> +
> static int __add_preferred_console(char *name, int idx, char *options,
> char *brl_options, bool user_specified)
> {
With the above comments:
Reviewed-by: Petr Mladek <[email protected]>
Sergey, is it enough from your POV, please?
Best Regards,
Petr
On (22/02/15 19:10), Petr Mladek wrote:
[..]
> We should add a comment explaining the less obvious behavior as
> discussed in the thread. Something like:
>
> > +static void set_user_specified(struct console_cmdline *c, bool user_specified)
> > +{
> > + if (!user_specified)
> > + return;
> > +
> /*
> * @c 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;
> /* At least one console defined by the user on the command line. */
> > + console_set_on_cmdline = 1;
> > +}
> > +
> > static int __add_preferred_console(char *name, int idx, char *options,
> > char *brl_options, bool user_specified)
> > {
>
> With the above comments:
>
> Reviewed-by: Petr Mladek <[email protected]>
>
> Sergey, is it enough from your POV, please?
Yes, Petr. Thank you.
Reviewed-by: Sergey Senozhatsky <[email protected]>
From: Andre Kalb <[email protected]>
In case of using console="" or console=null
set console_set_on_cmdline=1 to disable "stdout-path" node from DT.
We basically need to set it every time when __add_preferred_console()
is called with parameter 'user_specified' set.
Therefore we can move setting it into a helper function that is
called from __add_preferred_console().
Suggested-by: Petr Mladek <[email protected]>
Signed-off-by: Andre Kalb <[email protected]>
Reviewed-by: Petr Mladek <[email protected]>
Reviewed-by: Sergey Senozhatsky <[email protected]>
---
Changelog v1 to v2:
Move console_set_on_cmdling into separate function set_user_specified(), which is called from
__add_preferred_console().
The old patch v1 could be used to backport to stable 5.4 and lower.
Changelog v2 to v3:
Add comment to explain the less obvious behaviour.
---
kernel/printk/printk.c | 20 ++++++++++++++++----
1 file changed, 16 insertions(+), 4 deletions(-)
diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index 82abfaf3c2aa..e4e1a5266108 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -2324,6 +2324,20 @@ asmlinkage __visible void early_printk(const char *fmt, ...)
}
#endif
+static void set_user_specified(struct console_cmdline *c, bool user_specified)
+{
+ if (!user_specified)
+ return;
+
+ /*
+ * @c 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;
+ /* At least one console defined by the user on the command line. */
+ console_set_on_cmdline = 1;
+}
+
static int __add_preferred_console(char *name, int idx, char *options,
char *brl_options, bool user_specified)
{
@@ -2340,8 +2354,7 @@ static int __add_preferred_console(char *name, int idx, char *options,
if (strcmp(c->name, name) == 0 && c->index == idx) {
if (!brl_options)
preferred_console = i;
- if (user_specified)
- c->user_specified = true;
+ set_user_specified(c, user_specified);
return 0;
}
}
@@ -2351,7 +2364,7 @@ static int __add_preferred_console(char *name, int idx, char *options,
preferred_console = i;
strlcpy(c->name, name, sizeof(c->name));
c->options = options;
- c->user_specified = user_specified;
+ set_user_specified(c, user_specified);
braille_set_options(c, brl_options);
c->index = idx;
@@ -2417,7 +2430,6 @@ static int __init console_setup(char *str)
*s = 0;
__add_preferred_console(buf, idx, options, brl_options, true);
- console_set_on_cmdline = 1;
return 1;
}
__setup("console=", console_setup);
--
2.31.1
On Wed 2022-02-16 11:41:38, Andre Kalb wrote:
> From: Andre Kalb <[email protected]>
>
> In case of using console="" or console=null
> set console_set_on_cmdline=1 to disable "stdout-path" node from DT.
>
> We basically need to set it every time when __add_preferred_console()
> is called with parameter 'user_specified' set.
> Therefore we can move setting it into a helper function that is
> called from __add_preferred_console().
>
> Suggested-by: Petr Mladek <[email protected]>
> Signed-off-by: Andre Kalb <[email protected]>
> Reviewed-by: Petr Mladek <[email protected]>
> Reviewed-by: Sergey Senozhatsky <[email protected]>
The patch is comitted in printk/linux.git, branch for-5.18.
Best Regards,
Petr