* [PATCH] printk: Set console_set_on_cmdline=1 when using console="" or console=null @ 2022-01-27 23:00 Andre Kalb 2022-02-08 15:55 ` Petr Mladek 0 siblings, 1 reply; 9+ messages in thread From: Andre Kalb @ 2022-01-27 23:00 UTC (permalink / raw) To: Petr Mladek, Sergey Senozhatsky, Steven Rostedt, John Ogness, linux-kernel 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 <andre.kalb@sma.de> --- 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 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] printk: Set console_set_on_cmdline=1 when using console="" or console=null 2022-01-27 23:00 [PATCH] printk: Set console_set_on_cmdline=1 when using console="" or console=null Andre Kalb @ 2022-02-08 15:55 ` Petr Mladek 2022-02-14 13:21 ` [PATCH v2] printk: Set console_set_on_cmdline=1 when __add_preferred_console() is called with user_specified == true Andre Kalb 0 siblings, 1 reply; 9+ messages in thread From: Petr Mladek @ 2022-02-08 15:55 UTC (permalink / raw) To: Andre Kalb; +Cc: Sergey Senozhatsky, Steven Rostedt, John Ogness, linux-kernel 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 <andre.kalb@sma.de> > --- > 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 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v2] printk: Set console_set_on_cmdline=1 when __add_preferred_console() is called with user_specified == true 2022-02-08 15:55 ` Petr Mladek @ 2022-02-14 13:21 ` Andre Kalb 2022-02-15 3:15 ` Sergey Senozhatsky 2022-02-15 18:10 ` Petr Mladek 0 siblings, 2 replies; 9+ messages in thread From: Andre Kalb @ 2022-02-14 13:21 UTC (permalink / raw) To: pmladek; +Cc: andre.kalb, john.ogness, linux-kernel, rostedt, senozhatsky From: Andre Kalb <andre.kalb@sma.de> 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 <pmladek@suse.com> Signed-off-by: Andre Kalb <andre.kalb@sma.de> --- 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 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v2] printk: Set console_set_on_cmdline=1 when __add_preferred_console() is called with user_specified == true 2022-02-14 13:21 ` [PATCH v2] printk: Set console_set_on_cmdline=1 when __add_preferred_console() is called with user_specified == true Andre Kalb @ 2022-02-15 3:15 ` Sergey Senozhatsky 2022-02-15 17:58 ` Petr Mladek 2022-02-15 18:10 ` Petr Mladek 1 sibling, 1 reply; 9+ messages in thread From: Sergey Senozhatsky @ 2022-02-15 3:15 UTC (permalink / raw) To: Andre Kalb; +Cc: pmladek, john.ogness, linux-kernel, rostedt, senozhatsky 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; } ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] printk: Set console_set_on_cmdline=1 when __add_preferred_console() is called with user_specified == true 2022-02-15 3:15 ` Sergey Senozhatsky @ 2022-02-15 17:58 ` Petr Mladek 0 siblings, 0 replies; 9+ messages in thread From: Petr Mladek @ 2022-02-15 17:58 UTC (permalink / raw) To: Sergey Senozhatsky; +Cc: Andre Kalb, john.ogness, linux-kernel, rostedt 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] printk: Set console_set_on_cmdline=1 when __add_preferred_console() is called with user_specified == true 2022-02-14 13:21 ` [PATCH v2] printk: Set console_set_on_cmdline=1 when __add_preferred_console() is called with user_specified == true Andre Kalb 2022-02-15 3:15 ` Sergey Senozhatsky @ 2022-02-15 18:10 ` Petr Mladek 2022-02-16 3:18 ` Sergey Senozhatsky 1 sibling, 1 reply; 9+ messages in thread From: Petr Mladek @ 2022-02-15 18:10 UTC (permalink / raw) To: Andre Kalb; +Cc: john.ogness, linux-kernel, rostedt, senozhatsky On Mon 2022-02-14 14:21:29, Andre Kalb wrote: > From: Andre Kalb <andre.kalb@sma.de> > > 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 <pmladek@suse.com> > Signed-off-by: Andre Kalb <andre.kalb@sma.de> > --- > 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 <pmladek@suse.com> Sergey, is it enough from your POV, please? Best Regards, Petr ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] printk: Set console_set_on_cmdline=1 when __add_preferred_console() is called with user_specified == true 2022-02-15 18:10 ` Petr Mladek @ 2022-02-16 3:18 ` Sergey Senozhatsky 2022-02-16 10:41 ` [PATCH v3] " Andre Kalb 0 siblings, 1 reply; 9+ messages in thread From: Sergey Senozhatsky @ 2022-02-16 3:18 UTC (permalink / raw) To: Petr Mladek; +Cc: Andre Kalb, john.ogness, linux-kernel, rostedt, senozhatsky 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 <pmladek@suse.com> > > Sergey, is it enough from your POV, please? Yes, Petr. Thank you. Reviewed-by: Sergey Senozhatsky <senozhatsky@chromium.org> ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v3] printk: Set console_set_on_cmdline=1 when __add_preferred_console() is called with user_specified == true 2022-02-16 3:18 ` Sergey Senozhatsky @ 2022-02-16 10:41 ` Andre Kalb 2022-02-21 15:49 ` Petr Mladek 0 siblings, 1 reply; 9+ messages in thread From: Andre Kalb @ 2022-02-16 10:41 UTC (permalink / raw) To: pmladek, senozhatsky; +Cc: andre.kalb, john.ogness, linux-kernel, rostedt From: Andre Kalb <andre.kalb@sma.de> 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 <pmladek@suse.com> Signed-off-by: Andre Kalb <andre.kalb@sma.de> Reviewed-by: Petr Mladek <pmladek@suse.com> Reviewed-by: Sergey Senozhatsky <senozhatsky@chromium.org> --- 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 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v3] printk: Set console_set_on_cmdline=1 when __add_preferred_console() is called with user_specified == true 2022-02-16 10:41 ` [PATCH v3] " Andre Kalb @ 2022-02-21 15:49 ` Petr Mladek 0 siblings, 0 replies; 9+ messages in thread From: Petr Mladek @ 2022-02-21 15:49 UTC (permalink / raw) To: Andre Kalb; +Cc: senozhatsky, john.ogness, linux-kernel, rostedt On Wed 2022-02-16 11:41:38, Andre Kalb wrote: > From: Andre Kalb <andre.kalb@sma.de> > > 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 <pmladek@suse.com> > Signed-off-by: Andre Kalb <andre.kalb@sma.de> > Reviewed-by: Petr Mladek <pmladek@suse.com> > Reviewed-by: Sergey Senozhatsky <senozhatsky@chromium.org> The patch is comitted in printk/linux.git, branch for-5.18. Best Regards, Petr ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2022-02-21 15:49 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-01-27 23:00 [PATCH] printk: Set console_set_on_cmdline=1 when using console="" or console=null Andre Kalb 2022-02-08 15:55 ` Petr Mladek 2022-02-14 13:21 ` [PATCH v2] printk: Set console_set_on_cmdline=1 when __add_preferred_console() is called with user_specified == true Andre Kalb 2022-02-15 3:15 ` Sergey Senozhatsky 2022-02-15 17:58 ` Petr Mladek 2022-02-15 18:10 ` Petr Mladek 2022-02-16 3:18 ` Sergey Senozhatsky 2022-02-16 10:41 ` [PATCH v3] " Andre Kalb 2022-02-21 15:49 ` Petr Mladek
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.