| Message ID | 20200428162227.687978-1-daniel.thompson@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | serial: earlycon: Allow earlier DT scan is acpi=off | expand |
Hi, On Tue, Apr 28, 2020 at 9:22 AM Daniel Thompson <daniel.thompson@linaro.org> wrote: > > Currently if the kernel has support for ACPI SPCR parsing then earlycon > without arguments is processed later than the full earlycon=... > alternative. > > If ACPI has been explicitly disabled on the kernel command line then > there is not need to defer since the ACPI code (both x86 and arm64) > will never actually run. > > Or, put another way it allows lazy people to throw "acpi=off earlycon" > onto the command line of a DT systems and be confident the console will > start as early as possible without them having to lookup the driver > and address needed for a specific platform. > > Signed-off-by: Daniel Thompson <daniel.thompson@linaro.org> > --- > drivers/tty/serial/earlycon.c | 28 +++++++++++++++++++++++++++- > 1 file changed, 27 insertions(+), 1 deletion(-) I wonder if a cleaner option is to just add a special "earlycon" value like "earlycon=not_acpi". This wouldn't require any special peeking and would just be a sentinel that just says "you should autodetect the earlycon, but don't worry about waiting for ACPI". ...that in itself is a bit of a hack, but at least it's more self contained in the earlycon driver and maybe more discoverable when someone is figuring out how to setup earlycon? -Doug > > diff --git a/drivers/tty/serial/earlycon.c b/drivers/tty/serial/earlycon.c > index 2ae9190b64bb..ebb648aacb47 100644 > --- a/drivers/tty/serial/earlycon.c > +++ b/drivers/tty/serial/earlycon.c > @@ -215,6 +215,31 @@ int __init setup_earlycon(char *buf) > */ > bool earlycon_acpi_spcr_enable __initdata; > > +/* > + * This takes a sneaky peek at other boot arguments (which may not have > + * been parsed at this point in the boot) to check whether ACPI has > + * been explicitly disabled. If it is explicitly disabled then there is > + * no reason to defer initialization of the early console. > + */ > +static bool earlycon_check_for_acpi_off(void) > +{ > + static const char token[] = "acpi=off"; > + const char *arg; > + char before, after; > + > + arg = strstr(boot_command_line, token); > + while (arg) { > + before = arg == boot_command_line ? ' ' : arg[-1]; > + after = arg[sizeof(token)-1]; > + if (isspace(before) && (isspace(after) || after == '\0')) > + return true; > + > + arg = strstr(arg+1, token); > + } > + > + return false; > +} > + > /* early_param wrapper for setup_earlycon() */ > static int __init param_setup_earlycon(char *buf) > { > @@ -222,7 +247,8 @@ static int __init param_setup_earlycon(char *buf) > > /* Just 'earlycon' is a valid param for devicetree and ACPI SPCR. */ > if (!buf || !buf[0]) { > - if (IS_ENABLED(CONFIG_ACPI_SPCR_TABLE)) { > + if (IS_ENABLED(CONFIG_ACPI_SPCR_TABLE) && > + !earlycon_check_for_acpi_off()) { > earlycon_acpi_spcr_enable = true; > return 0; > } else if (!buf) { > -- > 2.25.1 >
On Wed, Apr 29, 2020 at 05:40:10PM -0700, Doug Anderson wrote: > Hi, > > On Tue, Apr 28, 2020 at 9:22 AM Daniel Thompson > <daniel.thompson@linaro.org> wrote: > > > > Currently if the kernel has support for ACPI SPCR parsing then earlycon > > without arguments is processed later than the full earlycon=... > > alternative. > > > > If ACPI has been explicitly disabled on the kernel command line then > > there is not need to defer since the ACPI code (both x86 and arm64) > > will never actually run. > > > > Or, put another way it allows lazy people to throw "acpi=off earlycon" > > onto the command line of a DT systems and be confident the console will > > start as early as possible without them having to lookup the driver > > and address needed for a specific platform. > > > > Signed-off-by: Daniel Thompson <daniel.thompson@linaro.org> > > --- > > drivers/tty/serial/earlycon.c | 28 +++++++++++++++++++++++++++- > > 1 file changed, 27 insertions(+), 1 deletion(-) > > I wonder if a cleaner option is to just add a special "earlycon" value > like "earlycon=not_acpi". This wouldn't require any special peeking > and would just be a sentinel that just says "you should autodetect the > earlycon, but don't worry about waiting for ACPI". ...that in itself > is a bit of a hack, but at least it's more self contained in the > earlycon driver and maybe more discoverable when someone is figuring > out how to setup earlycon? Taking this idea further I wonder if we could even make this earlycon=acpi. In other words if the loader provided a DT and earlycon has no arguments when we use the DT to setup the earlycon regardless of any later decision to adopt ACPI. I think the only time this would do the wrong thing is if the DT and ACPI tables are both passed to the kernel and have different early consoles. This would be a very weird thing for the firmware to do by, just in case, we could offer earlycon=acpi to accommodate it. Naturally I do have to code and test it ;-). Daniel.
diff --git a/drivers/tty/serial/earlycon.c b/drivers/tty/serial/earlycon.c index 2ae9190b64bb..ebb648aacb47 100644 --- a/drivers/tty/serial/earlycon.c +++ b/drivers/tty/serial/earlycon.c @@ -215,6 +215,31 @@ int __init setup_earlycon(char *buf) */ bool earlycon_acpi_spcr_enable __initdata; +/* + * This takes a sneaky peek at other boot arguments (which may not have + * been parsed at this point in the boot) to check whether ACPI has + * been explicitly disabled. If it is explicitly disabled then there is + * no reason to defer initialization of the early console. + */ +static bool earlycon_check_for_acpi_off(void) +{ + static const char token[] = "acpi=off"; + const char *arg; + char before, after; + + arg = strstr(boot_command_line, token); + while (arg) { + before = arg == boot_command_line ? ' ' : arg[-1]; + after = arg[sizeof(token)-1]; + if (isspace(before) && (isspace(after) || after == '\0')) + return true; + + arg = strstr(arg+1, token); + } + + return false; +} + /* early_param wrapper for setup_earlycon() */ static int __init param_setup_earlycon(char *buf) { @@ -222,7 +247,8 @@ static int __init param_setup_earlycon(char *buf) /* Just 'earlycon' is a valid param for devicetree and ACPI SPCR. */ if (!buf || !buf[0]) { - if (IS_ENABLED(CONFIG_ACPI_SPCR_TABLE)) { + if (IS_ENABLED(CONFIG_ACPI_SPCR_TABLE) && + !earlycon_check_for_acpi_off()) { earlycon_acpi_spcr_enable = true; return 0; } else if (!buf) {
Currently if the kernel has support for ACPI SPCR parsing then earlycon without arguments is processed later than the full earlycon=... alternative. If ACPI has been explicitly disabled on the kernel command line then there is not need to defer since the ACPI code (both x86 and arm64) will never actually run. Or, put another way it allows lazy people to throw "acpi=off earlycon" onto the command line of a DT systems and be confident the console will start as early as possible without them having to lookup the driver and address needed for a specific platform. Signed-off-by: Daniel Thompson <daniel.thompson@linaro.org> --- drivers/tty/serial/earlycon.c | 28 +++++++++++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) -- 2.25.1