Support for configuring bootconsole and console via the ACPI tables DBG2 (Debug Port Table 2) [1] and SPCR (Serial Port Console Redirection Table) [2], defined by Microsoft, has been discussed on and off over the years.
[1] https://msdn.microsoft.com/en-us/library/windows/hardware/dn639131(v=vs.85).... [2] https://msdn.microsoft.com/en-us/library/windows/hardware/dn639132(v=vs.85)....
Licensing concerns have prevented this happening in the past, but as of 10 August 2015, these tables have both been released also under OWF 1.0 (http://www.openwebfoundation.org/legal/the-owf-1-0-agreements/owfa-1-0) which is think is noncontroversially GPL-compatible?
This set is a first attempt at implementing this.
Submitting as an RFC since the SPCR handling currently depends on the console driver being initialized after subsystem initcalls. Workaround to enable testing surrounding infrastructure in 5/5, _really_ not intended to be merged. (Suggestions for acceptable ways of working around this appreciated.)
For testing the DBG2 stuff with pl011, you would need: - A patch to unbreak pl011 earlycon, like http://permalink.gmane.org/gmane.linux.ports.arm.kernel/433219 - A QEMU that generates DBG2 tables, like current HEAD with the addition of http://lists.nongnu.org/archive/html/qemu-devel/2015-09/msg01719.html
SPCR support is included in QEMU's ARM mach-virt since 2.4 release.
DBG2 support has an Intel copyright notice added to it since my starting point was Lv Zheng's 2012 DBGP/DBG2 set (although not much of the original remains - this is quite a cut-down version).
Leif Lindholm (4): arm64: move acpi/dt decision earlier in boot process of/serial: move earlycon early_param handling to serial acpi/serial: add DBG2 earlycon support HACK: serial: move pl011 initcall to device_initcall
Torez Smith (1): tty/console: use SPCR table to define console
arch/arm64/kernel/acpi.c | 55 +++++---- drivers/acpi/Makefile | 1 + drivers/acpi/console.c | 260 +++++++++++++++++++++++++++++++++++++++ drivers/of/fdt.c | 13 +- drivers/tty/serial/amba-pl011.c | 2 +- drivers/tty/serial/earlycon.c | 18 ++- drivers/tty/serial/serial_core.c | 14 ++- include/linux/acpi.h | 13 ++ include/linux/of_fdt.h | 1 + include/linux/serial_core.h | 9 +- 10 files changed, 337 insertions(+), 49 deletions(-) create mode 100644 drivers/acpi/console.c
In order to support selecting earlycon via either ACPI or DT, move the decision on whether to attempt ACPI configuration into the early_param handling. Then make acpi_boot_table_init() bail out if acpi_disabled.
Signed-off-by: Leif Lindholm leif.lindholm@linaro.org --- arch/arm64/kernel/acpi.c | 53 +++++++++++++++++++++++++----------------------- 1 file changed, 28 insertions(+), 25 deletions(-)
diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c index 19de753..b9a5623 100644 --- a/arch/arm64/kernel/acpi.c +++ b/arch/arm64/kernel/acpi.c @@ -39,6 +39,19 @@ EXPORT_SYMBOL(acpi_pci_disabled); static bool param_acpi_off __initdata; static bool param_acpi_force __initdata;
+static int __init dt_scan_depth1_nodes(unsigned long node, + const char *uname, int depth, + void *data) +{ + /* + * Return 1 as soon as we encounter a node at depth 1 that is + * not the /chosen node. + */ + if (depth == 1 && (strcmp(uname, "chosen") != 0)) + return 1; + return 0; +} + static int __init parse_acpi(char *arg) { if (!arg) @@ -52,22 +65,25 @@ static int __init parse_acpi(char *arg) else return -EINVAL; /* Core will print when we return error */
- return 0; -} -early_param("acpi", parse_acpi); + /* + * Enable ACPI instead of device tree unless + * - ACPI has been disabled explicitly (acpi=off), or + * - the device tree is not empty (it has more than just a /chosen node) + * and ACPI has not been force enabled (acpi=force) + */ + if (param_acpi_off || + (!param_acpi_force && of_scan_flat_dt(dt_scan_depth1_nodes, NULL))) + return 0;
-static int __init dt_scan_depth1_nodes(unsigned long node, - const char *uname, int depth, - void *data) -{ /* - * Return 1 as soon as we encounter a node at depth 1 that is - * not the /chosen node. + * ACPI is disabled at this point. Enable it in order to parse + * the ACPI tables and carry out sanity checks */ - if (depth == 1 && (strcmp(uname, "chosen") != 0)) - return 1; + enable_acpi(); + return 0; } +early_param("acpi", parse_acpi);
/* * __acpi_map_table() will be called before page_init(), so early_ioremap() @@ -176,23 +192,10 @@ out: */ void __init acpi_boot_table_init(void) { - /* - * Enable ACPI instead of device tree unless - * - ACPI has been disabled explicitly (acpi=off), or - * - the device tree is not empty (it has more than just a /chosen node) - * and ACPI has not been force enabled (acpi=force) - */ - if (param_acpi_off || - (!param_acpi_force && of_scan_flat_dt(dt_scan_depth1_nodes, NULL))) + if (acpi_disabled) return;
/* - * ACPI is disabled at this point. Enable it in order to parse - * the ACPI tables and carry out sanity checks - */ - enable_acpi(); - - /* * If ACPI tables are initialized and FADT sanity checks passed, * leave ACPI enabled and carry on booting; otherwise disable ACPI * on initialization error.
We have multiple "earlycon" early_param handlers - merge the DT one into the main earlycon one. This means the earlycon early_param handler does not just return success if no options are specified.
Signed-off-by: Leif Lindholm leif.lindholm@linaro.org --- drivers/of/fdt.c | 11 +---------- drivers/tty/serial/earlycon.c | 4 +++- include/linux/of_fdt.h | 1 + 3 files changed, 5 insertions(+), 11 deletions(-)
diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c index 6e82bc42..fcfc4c7 100644 --- a/drivers/of/fdt.c +++ b/drivers/of/fdt.c @@ -793,7 +793,7 @@ static inline void early_init_dt_check_for_initrd(unsigned long node) #ifdef CONFIG_SERIAL_EARLYCON extern struct of_device_id __earlycon_of_table[];
-static int __init early_init_dt_scan_chosen_serial(void) +int __init early_init_dt_scan_chosen_serial(void) { int offset; const char *p; @@ -834,15 +834,6 @@ static int __init early_init_dt_scan_chosen_serial(void) } return -ENODEV; } - -static int __init setup_of_earlycon(char *buf) -{ - if (buf) - return 0; - - return early_init_dt_scan_chosen_serial(); -} -early_param("earlycon", setup_of_earlycon); #endif
/** diff --git a/drivers/tty/serial/earlycon.c b/drivers/tty/serial/earlycon.c index f096360..2bda09a 100644 --- a/drivers/tty/serial/earlycon.c +++ b/drivers/tty/serial/earlycon.c @@ -17,6 +17,7 @@ #include <linux/kernel.h> #include <linux/init.h> #include <linux/io.h> +#include <linux/of_fdt.h> #include <linux/serial_core.h> #include <linux/sizes.h> #include <linux/mod_devicetable.h> @@ -187,7 +188,8 @@ static int __init param_setup_earlycon(char *buf) * don't generate a warning from parse_early_params() in that case */ if (!buf || !buf[0]) - return 0; + if (IS_ENABLED(CONFIG_OF_FLATTREE)) + return early_init_dt_scan_chosen_serial();
err = setup_earlycon(buf); if (err == -ENOENT || err == -EALREADY) diff --git a/include/linux/of_fdt.h b/include/linux/of_fdt.h index df9ef38..772c47c 100644 --- a/include/linux/of_fdt.h +++ b/include/linux/of_fdt.h @@ -63,6 +63,7 @@ extern int early_init_dt_scan_chosen(unsigned long node, const char *uname, int depth, void *data); extern int early_init_dt_scan_memory(unsigned long node, const char *uname, int depth, void *data); +extern int early_init_dt_scan_chosen_serial(void); extern void early_init_fdt_scan_reserved_mem(void); extern void early_init_fdt_reserve_self(void); extern void early_init_dt_add_memory_arch(u64 base, u64 size);
On Tue, Sep 08, 2015 at 01:43:34PM +0100, Leif Lindholm wrote:
We have multiple "earlycon" early_param handlers - merge the DT one into the main earlycon one. This means the earlycon early_param handler does not just return success if no options are specified.
Signed-off-by: Leif Lindholm leif.lindholm@linaro.org
drivers/of/fdt.c | 11 +---------- drivers/tty/serial/earlycon.c | 4 +++- include/linux/of_fdt.h | 1 + 3 files changed, 5 insertions(+), 11 deletions(-)
diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c index 6e82bc42..fcfc4c7 100644 --- a/drivers/of/fdt.c +++ b/drivers/of/fdt.c @@ -793,7 +793,7 @@ static inline void early_init_dt_check_for_initrd(unsigned long node) #ifdef CONFIG_SERIAL_EARLYCON extern struct of_device_id __earlycon_of_table[]; -static int __init early_init_dt_scan_chosen_serial(void) +int __init early_init_dt_scan_chosen_serial(void) { int offset; const char *p; @@ -834,15 +834,6 @@ static int __init early_init_dt_scan_chosen_serial(void) } return -ENODEV; }
-static int __init setup_of_earlycon(char *buf) -{
- if (buf)
return 0;- return early_init_dt_scan_chosen_serial();
-} -early_param("earlycon", setup_of_earlycon); #endif /** diff --git a/drivers/tty/serial/earlycon.c b/drivers/tty/serial/earlycon.c index f096360..2bda09a 100644 --- a/drivers/tty/serial/earlycon.c +++ b/drivers/tty/serial/earlycon.c @@ -17,6 +17,7 @@ #include <linux/kernel.h> #include <linux/init.h> #include <linux/io.h> +#include <linux/of_fdt.h> #include <linux/serial_core.h> #include <linux/sizes.h> #include <linux/mod_devicetable.h> @@ -187,7 +188,8 @@ static int __init param_setup_earlycon(char *buf) * don't generate a warning from parse_early_params() in that case */ if (!buf || !buf[0])
return 0;
if (IS_ENABLED(CONFIG_OF_FLATTREE))return early_init_dt_scan_chosen_serial();err = setup_earlycon(buf); if (err == -ENOENT || err == -EALREADY) diff --git a/include/linux/of_fdt.h b/include/linux/of_fdt.h index df9ef38..772c47c 100644 --- a/include/linux/of_fdt.h +++ b/include/linux/of_fdt.h @@ -63,6 +63,7 @@ extern int early_init_dt_scan_chosen(unsigned long node, const char *uname, int depth, void *data); extern int early_init_dt_scan_memory(unsigned long node, const char *uname, int depth, void *data); +extern int early_init_dt_scan_chosen_serial(void);
I think you need a static inline stub to prevent link errors in !CONFIG_OF_FLATTREE kernels, as we do for early_init_fdt_scan_reserved_mem and friends.
Mark.
extern void early_init_fdt_scan_reserved_mem(void); extern void early_init_fdt_reserve_self(void); extern void early_init_dt_add_memory_arch(u64 base, u64 size); -- 2.1.4
On Tue, Sep 08, 2015 at 01:52:45PM +0100, Mark Rutland wrote:
On Tue, Sep 08, 2015 at 01:43:34PM +0100, Leif Lindholm wrote:
We have multiple "earlycon" early_param handlers - merge the DT one into the main earlycon one. This means the earlycon early_param handler does not just return success if no options are specified.
Signed-off-by: Leif Lindholm leif.lindholm@linaro.org
drivers/of/fdt.c | 11 +---------- drivers/tty/serial/earlycon.c | 4 +++- include/linux/of_fdt.h | 1 + 3 files changed, 5 insertions(+), 11 deletions(-)
diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c index 6e82bc42..fcfc4c7 100644 --- a/drivers/of/fdt.c +++ b/drivers/of/fdt.c @@ -793,7 +793,7 @@ static inline void early_init_dt_check_for_initrd(unsigned long node) #ifdef CONFIG_SERIAL_EARLYCON extern struct of_device_id __earlycon_of_table[]; -static int __init early_init_dt_scan_chosen_serial(void) +int __init early_init_dt_scan_chosen_serial(void) { int offset; const char *p; @@ -834,15 +834,6 @@ static int __init early_init_dt_scan_chosen_serial(void) } return -ENODEV; }
-static int __init setup_of_earlycon(char *buf) -{
- if (buf)
return 0;- return early_init_dt_scan_chosen_serial();
-} -early_param("earlycon", setup_of_earlycon); #endif /** diff --git a/drivers/tty/serial/earlycon.c b/drivers/tty/serial/earlycon.c index f096360..2bda09a 100644 --- a/drivers/tty/serial/earlycon.c +++ b/drivers/tty/serial/earlycon.c @@ -17,6 +17,7 @@ #include <linux/kernel.h> #include <linux/init.h> #include <linux/io.h> +#include <linux/of_fdt.h> #include <linux/serial_core.h> #include <linux/sizes.h> #include <linux/mod_devicetable.h> @@ -187,7 +188,8 @@ static int __init param_setup_earlycon(char *buf) * don't generate a warning from parse_early_params() in that case */ if (!buf || !buf[0])
return 0;
if (IS_ENABLED(CONFIG_OF_FLATTREE))return early_init_dt_scan_chosen_serial();err = setup_earlycon(buf); if (err == -ENOENT || err == -EALREADY) diff --git a/include/linux/of_fdt.h b/include/linux/of_fdt.h index df9ef38..772c47c 100644 --- a/include/linux/of_fdt.h +++ b/include/linux/of_fdt.h @@ -63,6 +63,7 @@ extern int early_init_dt_scan_chosen(unsigned long node, const char *uname, int depth, void *data); extern int early_init_dt_scan_memory(unsigned long node, const char *uname, int depth, void *data); +extern int early_init_dt_scan_chosen_serial(void);
I think you need a static inline stub to prevent link errors in !CONFIG_OF_FLATTREE kernels, as we do for early_init_fdt_scan_reserved_mem and friends.
Will add, thanks.
Mark.
extern void early_init_fdt_scan_reserved_mem(void); extern void early_init_fdt_reserve_self(void); extern void early_init_dt_add_memory_arch(u64 base, u64 size); -- 2.1.4
The ACPI DBG2 table defines a debug console. Add support for parsing it and using it to select earlycon destination when no arguments provided.
Signed-off-by: Leif Lindholm leif.lindholm@linaro.org --- arch/arm64/kernel/acpi.c | 2 + drivers/acpi/Makefile | 1 + drivers/acpi/console.c | 103 ++++++++++++++++++++++++++++++++++++++++++ drivers/of/fdt.c | 2 +- drivers/tty/serial/earlycon.c | 16 ++++--- include/linux/acpi.h | 4 ++ include/linux/serial_core.h | 9 ++-- 7 files changed, 126 insertions(+), 11 deletions(-) create mode 100644 drivers/acpi/console.c
diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c index b9a5623..be7600a 100644 --- a/arch/arm64/kernel/acpi.c +++ b/arch/arm64/kernel/acpi.c @@ -207,6 +207,8 @@ void __init acpi_boot_table_init(void) if (!param_acpi_force) disable_acpi(); } + + acpi_early_console_probe(); }
void __init acpi_gic_init(void) diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile index b5e7cd8..a89587d 100644 --- a/drivers/acpi/Makefile +++ b/drivers/acpi/Makefile @@ -10,6 +10,7 @@ ccflags-$(CONFIG_ACPI_DEBUG) += -DACPI_DEBUG_OUTPUT # obj-y += tables.o obj-$(CONFIG_X86) += blacklist.o +obj-y += console.o
# # ACPI Core Subsystem (Interpreter) diff --git a/drivers/acpi/console.c b/drivers/acpi/console.c new file mode 100644 index 0000000..a985890 --- /dev/null +++ b/drivers/acpi/console.c @@ -0,0 +1,103 @@ +/* + * Copyright (c) 2012, Intel Corporation + * Copyright (c) 2015, Linaro Ltd. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + * + */ + +#define DEBUG +#define pr_fmt(fmt) "ACPI: " KBUILD_MODNAME ": " fmt + +#include <linux/acpi.h> +#include <linux/kernel.h> +#include <linux/serial_core.h> + +#define NUM_ELEMS(x) (sizeof(x) / sizeof(*x)) + +#ifdef CONFIG_SERIAL_EARLYCON +static int use_earlycon __initdata; +static int __init setup_acpi_earlycon(char *buf) +{ + if (!buf) + use_earlycon = 1; + + return 0; +} +early_param("earlycon", setup_acpi_earlycon); + +extern struct earlycon_id __earlycon_table[]; + +static __initdata struct { + int id; + const char *name; +} subtypes[] = { + {0, "uart8250"}, + {1, "uart8250"}, + {2, NULL}, + {3, "pl011"}, +}; + +static int __init acpi_setup_earlycon(unsigned long addr, const char *driver) +{ + const struct earlycon_id *match; + + for (match = __earlycon_table; match->name[0]; match++) + if (strcmp(driver, match->name) == 0) + return setup_earlycon_driver(addr, match->setup); + + return -ENODEV; +} + +static int __init acpi_parse_dbg2(struct acpi_table_header *table) +{ + struct acpi_table_dbg2 *dbg2; + struct acpi_dbg2_device *entry; + void *tbl_end; + + dbg2 = (struct acpi_table_dbg2 *)table; + if (!dbg2) { + pr_debug("DBG2 not present.\n"); + return -ENODEV; + } + + tbl_end = (void *)table + table->length; + + entry = (struct acpi_dbg2_device *)((void *)dbg2 + dbg2->info_offset); + + while (((void *)entry) + sizeof(struct acpi_dbg2_device) < tbl_end) { + struct acpi_generic_address *addr; + + if (entry->revision != 0) { + pr_debug("DBG2 revision %d not supported.\n", + entry->revision); + return -ENODEV; + } + + addr = (void *)entry + entry->base_address_offset; + + pr_debug("DBG2 PROBE - console (%04x:%04x).\n", + entry->port_type, entry->port_subtype); + + if (use_earlycon && + (entry->port_type == ACPI_DBG2_SERIAL_PORT) && + (entry->port_subtype < NUM_ELEMS(subtypes))) + acpi_setup_earlycon(addr->address, + subtypes[entry->port_subtype].name); + + entry = (struct acpi_dbg2_device *) + ((void *)entry + entry->length); + } + + return 0; +} + +int __init acpi_early_console_probe(void) +{ + acpi_table_parse(ACPI_SIG_DBG2, acpi_parse_dbg2); + + return 0; +} +#endif /* CONFIG_SERIAL_EARLYCON */ diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c index fcfc4c7..a96209f 100644 --- a/drivers/of/fdt.c +++ b/drivers/of/fdt.c @@ -829,7 +829,7 @@ int __init early_init_dt_scan_chosen_serial(void) if (!addr) return -ENXIO;
- of_setup_earlycon(addr, match->data); + setup_earlycon_driver(addr, match->data); return 0; } return -ENODEV; diff --git a/drivers/tty/serial/earlycon.c b/drivers/tty/serial/earlycon.c index 2bda09a..c063cbb 100644 --- a/drivers/tty/serial/earlycon.c +++ b/drivers/tty/serial/earlycon.c @@ -13,6 +13,7 @@
#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+#include <linux/acpi.h> #include <linux/console.h> #include <linux/kernel.h> #include <linux/init.h> @@ -184,12 +185,16 @@ static int __init param_setup_earlycon(char *buf) int err;
/* - * Just 'earlycon' is a valid param for devicetree earlycons; - * don't generate a warning from parse_early_params() in that case + * Just 'earlycon' is a valid param for devicetree or ACPI earlycons; + * ACPI cannot be parsed yet, so return without action if enabled. + * Otherwise, attempt initialization using DT. */ - if (!buf || !buf[0]) - if (IS_ENABLED(CONFIG_OF_FLATTREE)) + if (!buf || !buf[0]) { + if (!acpi_disabled) + return 0; + else if (IS_ENABLED(CONFIG_OF_FLATTREE)) return early_init_dt_scan_chosen_serial(); + }
err = setup_earlycon(buf); if (err == -ENOENT || err == -EALREADY) @@ -198,8 +203,7 @@ static int __init param_setup_earlycon(char *buf) } early_param("earlycon", param_setup_earlycon);
-int __init of_setup_earlycon(unsigned long addr, - int (*setup)(struct earlycon_device *, const char *)) +int __init setup_earlycon_driver(unsigned long addr, earlycon_initfunc_t setup) { int err; struct uart_port *port = &early_console_dev.port; diff --git a/include/linux/acpi.h b/include/linux/acpi.h index 7235c48..88cb9c1 100644 --- a/include/linux/acpi.h +++ b/include/linux/acpi.h @@ -811,4 +811,8 @@ static inline struct acpi_device *acpi_get_next_child(struct device *dev,
#endif
+#if defined(CONFIG_ACPI) && defined(CONFIG_SERIAL_EARLYCON) +int __init acpi_early_console_probe(void); +#endif + #endif /*_LINUX_ACPI_H*/ diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h index 297d4fa..39e99b0 100644 --- a/include/linux/serial_core.h +++ b/include/linux/serial_core.h @@ -339,14 +339,15 @@ struct earlycon_device { unsigned int baud; };
+typedef int (*earlycon_initfunc_t)(struct earlycon_device *, const char *); + struct earlycon_id { - char name[16]; - int (*setup)(struct earlycon_device *, const char *options); + char name[16]; + earlycon_initfunc_t setup; } __aligned(32);
extern int setup_earlycon(char *buf); -extern int of_setup_earlycon(unsigned long addr, - int (*setup)(struct earlycon_device *, const char *)); +extern int setup_earlycon_driver(unsigned long addr, earlycon_initfunc_t setup);
#define EARLYCON_DECLARE(_name, func) \ static const struct earlycon_id __earlycon_##_name \
On Tue, Sep 08, 2015 at 01:43:35PM +0100, Leif Lindholm wrote:
The ACPI DBG2 table defines a debug console. Add support for parsing it and using it to select earlycon destination when no arguments provided.
Signed-off-by: Leif Lindholm leif.lindholm@linaro.org
[...]
diff --git a/drivers/acpi/console.c b/drivers/acpi/console.c new file mode 100644 index 0000000..a985890 --- /dev/null +++ b/drivers/acpi/console.c @@ -0,0 +1,103 @@ +/*
- Copyright (c) 2012, Intel Corporation
- Copyright (c) 2015, Linaro Ltd.
- This program is free software; you can redistribute it and/or modify
- it under the terms of the GNU General Public License version 2 as
- published by the Free Software Foundation.
- */
+#define DEBUG
Why?
+#define pr_fmt(fmt) "ACPI: " KBUILD_MODNAME ": " fmt
+#include <linux/acpi.h> +#include <linux/kernel.h> +#include <linux/serial_core.h>
+#define NUM_ELEMS(x) (sizeof(x) / sizeof(*x))
Use ARRAY_SIZE (from kernel.h).
+#ifdef CONFIG_SERIAL_EARLYCON +static int use_earlycon __initdata; +static int __init setup_acpi_earlycon(char *buf) +{
- if (!buf)
use_earlycon = 1;- return 0;
+} +early_param("earlycon", setup_acpi_earlycon);
It seems a shame to add this after folding the OF case into the earlycon code. What necessitates this being a separate early_param? Why is it too early to parse DBG2?
[...]
diff --git a/drivers/tty/serial/earlycon.c b/drivers/tty/serial/earlycon.c index 2bda09a..c063cbb 100644 --- a/drivers/tty/serial/earlycon.c +++ b/drivers/tty/serial/earlycon.c @@ -13,6 +13,7 @@ #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt +#include <linux/acpi.h> #include <linux/console.h> #include <linux/kernel.h> #include <linux/init.h> @@ -184,12 +185,16 @@ static int __init param_setup_earlycon(char *buf) int err; /*
* Just 'earlycon' is a valid param for devicetree earlycons;* don't generate a warning from parse_early_params() in that case
* Just 'earlycon' is a valid param for devicetree or ACPI earlycons;* ACPI cannot be parsed yet, so return without action if enabled.* Otherwise, attempt initialization using DT.
- if (!buf || !buf[0])
if (IS_ENABLED(CONFIG_OF_FLATTREE))
- if (!buf || !buf[0]) {
if (!acpi_disabled)return 0;else if (IS_ENABLED(CONFIG_OF_FLATTREE)) return early_init_dt_scan_chosen_serial();- }
It would be much nicer if we could handle the ACPI earlycon parsing in the same place. As above, I assume I'm missing something that prevents that.
Mark.
On Tue, Sep 08, 2015 at 02:09:51PM +0100, Mark Rutland wrote:
On Tue, Sep 08, 2015 at 01:43:35PM +0100, Leif Lindholm wrote:
The ACPI DBG2 table defines a debug console. Add support for parsing it and using it to select earlycon destination when no arguments provided.
Signed-off-by: Leif Lindholm leif.lindholm@linaro.org
[...]
diff --git a/drivers/acpi/console.c b/drivers/acpi/console.c new file mode 100644 index 0000000..a985890 --- /dev/null +++ b/drivers/acpi/console.c @@ -0,0 +1,103 @@ +/*
- Copyright (c) 2012, Intel Corporation
- Copyright (c) 2015, Linaro Ltd.
- This program is free software; you can redistribute it and/or modify
- it under the terms of the GNU General Public License version 2 as
- published by the Free Software Foundation.
- */
+#define DEBUG
Why?
Kept around from Lv Zheng's 2012 set. Will drop.
+#define pr_fmt(fmt) "ACPI: " KBUILD_MODNAME ": " fmt
+#include <linux/acpi.h> +#include <linux/kernel.h> +#include <linux/serial_core.h>
+#define NUM_ELEMS(x) (sizeof(x) / sizeof(*x))
Use ARRAY_SIZE (from kernel.h).
I was sure there was something like that, but my grep-fu failed me. Will fix.
+#ifdef CONFIG_SERIAL_EARLYCON +static int use_earlycon __initdata; +static int __init setup_acpi_earlycon(char *buf) +{
- if (!buf)
use_earlycon = 1;- return 0;
+} +early_param("earlycon", setup_acpi_earlycon);
It seems a shame to add this after folding the OF case into the earlycon code. What necessitates this being a separate early_param? Why is it too early to parse DBG2?
Currently, we don't even know where our ACPI tables are at this point (efi_init() is called two functions after parse_early_param() in setup_arch). More specifically, because acpi_boot_table_init() is called even later than that.
If we moved both of those earlier, we could drop the extra earlycon param handling for ACPI. That would of course reduce the ability to have dynamically configurable debug messages for both of these.
[...]
diff --git a/drivers/tty/serial/earlycon.c b/drivers/tty/serial/earlycon.c index 2bda09a..c063cbb 100644 --- a/drivers/tty/serial/earlycon.c +++ b/drivers/tty/serial/earlycon.c @@ -13,6 +13,7 @@ #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt +#include <linux/acpi.h> #include <linux/console.h> #include <linux/kernel.h> #include <linux/init.h> @@ -184,12 +185,16 @@ static int __init param_setup_earlycon(char *buf) int err; /*
* Just 'earlycon' is a valid param for devicetree earlycons;* don't generate a warning from parse_early_params() in that case
* Just 'earlycon' is a valid param for devicetree or ACPI earlycons;* ACPI cannot be parsed yet, so return without action if enabled.* Otherwise, attempt initialization using DT.
- if (!buf || !buf[0])
if (IS_ENABLED(CONFIG_OF_FLATTREE))
- if (!buf || !buf[0]) {
if (!acpi_disabled)return 0;else if (IS_ENABLED(CONFIG_OF_FLATTREE)) return early_init_dt_scan_chosen_serial();- }
It would be much nicer if we could handle the ACPI earlycon parsing in the same place. As above, I assume I'm missing something that prevents that.
I don't disagree.
/ Leif
+#ifdef CONFIG_SERIAL_EARLYCON +static int use_earlycon __initdata; +static int __init setup_acpi_earlycon(char *buf) +{
- if (!buf)
use_earlycon = 1;- return 0;
+} +early_param("earlycon", setup_acpi_earlycon);
It seems a shame to add this after folding the OF case into the earlycon code. What necessitates this being a separate early_param? Why is it too early to parse DBG2?
Currently, we don't even know where our ACPI tables are at this point (efi_init() is called two functions after parse_early_param() in setup_arch). More specifically, because acpi_boot_table_init() is called even later than that.
If we moved both of those earlier, we could drop the extra earlycon param handling for ACPI. That would of course reduce the ability to have dynamically configurable debug messages for both of these.
Ok. Would you be able to put something in the commit message regarding the above, to make it clear why we need this multi-step dance (and why it's preferable to the alternative)?
Mark.
On Tue, 2015-09-08 at 13:43 +0100, Leif Lindholm wrote:
The ACPI DBG2 table defines a debug console. Add support for parsing it and using it to select earlycon destination when no arguments provided.
Signed-off-by: Leif Lindholm leif.lindholm@linaro.org
arch/arm64/kernel/acpi.c | 2 + drivers/acpi/Makefile | 1 + drivers/acpi/console.c | 103 ++++++++++++++++++++++++++++++++++++++++++ drivers/of/fdt.c | 2 +- drivers/tty/serial/earlycon.c | 16 ++++--- include/linux/acpi.h | 4 ++ include/linux/serial_core.h | 9 ++-- 7 files changed, 126 insertions(+), 11 deletions(-) create mode 100644 drivers/acpi/console.c
diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c index b9a5623..be7600a 100644 --- a/arch/arm64/kernel/acpi.c +++ b/arch/arm64/kernel/acpi.c @@ -207,6 +207,8 @@ void __init acpi_boot_table_init(void) if (!param_acpi_force) disable_acpi(); }
- acpi_early_console_probe();
} void __init acpi_gic_init(void) diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile index b5e7cd8..a89587d 100644 --- a/drivers/acpi/Makefile +++ b/drivers/acpi/Makefile @@ -10,6 +10,7 @@ ccflags-$(CONFIG_ACPI_DEBUG) += -DACPI_DEBUG_OUTPUT # obj-y += tables.o obj-$(CONFIG_X86) += blacklist.o +obj-y += console.o
obj-$(CONFIG_SERIAL_EARLYCON) += console.o
to eliminate whole-file #ifdef
# # ACPI Core Subsystem (Interpreter) diff --git a/drivers/acpi/console.c b/drivers/acpi/console.c new file mode 100644 index 0000000..a985890 --- /dev/null +++ b/drivers/acpi/console.c @@ -0,0 +1,103 @@ +/*
- Copyright (c) 2012, Intel Corporation
- Copyright (c) 2015, Linaro Ltd.
- This program is free software; you can redistribute it and/or modify
- it under the terms of the GNU General Public License version 2 as
- published by the Free Software Foundation.
- */
+#define DEBUG +#define pr_fmt(fmt) "ACPI: " KBUILD_MODNAME ": " fmt
+#include <linux/acpi.h> +#include <linux/kernel.h> +#include <linux/serial_core.h>
+#define NUM_ELEMS(x) (sizeof(x) / sizeof(*x))
+#ifdef CONFIG_SERIAL_EARLYCON +static int use_earlycon __initdata; +static int __init setup_acpi_earlycon(char *buf) +{
- if (!buf)
use_earlycon = 1;- return 0;
+} +early_param("earlycon", setup_acpi_earlycon);
+extern struct earlycon_id __earlycon_table[];
+static __initdata struct {
- int id;
- const char *name;
+} subtypes[] = {
- {0, "uart8250"},
- {1, "uart8250"},
- {2, NULL},
- {3, "pl011"},
+};
Instead of having a table here, why not have an ACPI_EARLYCON_DECLARE() where individual drivers can provide an id similar to OF_EARLYCON_DECLARE() providing compatible strings?
+static int __init acpi_setup_earlycon(unsigned long addr, const char *driver) +{
- const struct earlycon_id *match;
- for (match = __earlycon_table; match->name[0]; match++)
if (strcmp(driver, match->name) == 0)return setup_earlycon_driver(addr, match->setup);- return -ENODEV;
+}
+static int __init acpi_parse_dbg2(struct acpi_table_header *table) +{
- struct acpi_table_dbg2 *dbg2;
- struct acpi_dbg2_device *entry;
- void *tbl_end;
- dbg2 = (struct acpi_table_dbg2 *)table;
- if (!dbg2) {
pr_debug("DBG2 not present.\n");return -ENODEV;- }
- tbl_end = (void *)table + table->length;
- entry = (struct acpi_dbg2_device *)((void *)dbg2 + dbg2->info_offset);
- while (((void *)entry) + sizeof(struct acpi_dbg2_device) < tbl_end) {
struct acpi_generic_address *addr;if (entry->revision != 0) {pr_debug("DBG2 revision %d not supported.\n",entry->revision);return -ENODEV;}addr = (void *)entry + entry->base_address_offset;pr_debug("DBG2 PROBE - console (%04x:%04x).\n",entry->port_type, entry->port_subtype);if (use_earlycon &&(entry->port_type == ACPI_DBG2_SERIAL_PORT) &&(entry->port_subtype < NUM_ELEMS(subtypes)))acpi_setup_earlycon(addr->address,subtypes[entry->port_subtype].name);
Don't we need to handle space_id (and bit width) as well as address?
entry = (struct acpi_dbg2_device *)((void *)entry + entry->length);- }
- return 0;
+}
+int __init acpi_early_console_probe(void) +{
- acpi_table_parse(ACPI_SIG_DBG2, acpi_parse_dbg2);
- return 0;
+} +#endif /* CONFIG_SERIAL_EARLYCON */ diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c index fcfc4c7..a96209f 100644 --- a/drivers/of/fdt.c +++ b/drivers/of/fdt.c @@ -829,7 +829,7 @@ int __init early_init_dt_scan_chosen_serial(void) if (!addr) return -ENXIO;
of_setup_earlycon(addr, match->data);
setup_earlycon_driver(addr, match->data);diff --git a/drivers/tty/serial/earlycon.c b/drivers/tty/serial/earlycon.c index 2bda09a..c063cbb 100644 --- a/drivers/tty/serial/earlycon.c +++ b/drivers/tty/serial/earlycon.c @@ -13,6 +13,7 @@ #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt +#include <linux/acpi.h> #include <linux/console.h> #include <linux/kernel.h> #include <linux/init.h> @@ -184,12 +185,16 @@ static int __init param_setup_earlycon(char *buf) int err; /*
* Just 'earlycon' is a valid param for devicetree earlycons;* don't generate a warning from parse_early_params() in that case
* Just 'earlycon' is a valid param for devicetree or ACPI earlycons;* ACPI cannot be parsed yet, so return without action if enabled.* Otherwise, attempt initialization using DT.
- if (!buf || !buf[0])
if (IS_ENABLED(CONFIG_OF_FLATTREE))
- if (!buf || !buf[0]) {
if (!acpi_disabled)
How do we know for sure that "acpi" has been parsed before "earlycon"?
return 0;else if (IS_ENABLED(CONFIG_OF_FLATTREE)) return early_init_dt_scan_chosen_serial();- }
err = setup_earlycon(buf); if (err == -ENOENT || err == -EALREADY) @@ -198,8 +203,7 @@ static int __init param_setup_earlycon(char *buf) } early_param("earlycon", param_setup_earlycon); -int __init of_setup_earlycon(unsigned long addr,
int (*setup)(struct earlycon_device *, const char *))+int __init setup_earlycon_driver(unsigned long addr, earlycon_initfunc_t setup) { int err; struct uart_port *port = &early_console_dev.port; diff --git a/include/linux/acpi.h b/include/linux/acpi.h index 7235c48..88cb9c1 100644 --- a/include/linux/acpi.h +++ b/include/linux/acpi.h @@ -811,4 +811,8 @@ static inline struct acpi_device *acpi_get_next_child(struct device *dev, #endif +#if defined(CONFIG_ACPI) && defined(CONFIG_SERIAL_EARLYCON) +int __init acpi_early_console_probe(void); +#endif
#endif /*_LINUX_ACPI_H*/ diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h index 297d4fa..39e99b0 100644 --- a/include/linux/serial_core.h +++ b/include/linux/serial_core.h @@ -339,14 +339,15 @@ struct earlycon_device { unsigned int baud; }; +typedef int (*earlycon_initfunc_t)(struct earlycon_device *, const char *);
struct earlycon_id {
- char name[16];
- int (*setup)(struct earlycon_device *, const char *options);
- char name[16];
- earlycon_initfunc_t setup;
} __aligned(32); extern int setup_earlycon(char *buf); -extern int of_setup_earlycon(unsigned long addr,
int (*setup)(struct earlycon_device *, const char *));+extern int setup_earlycon_driver(unsigned long addr, earlycon_initfunc_t setup); #define EARLYCON_DECLARE(_name, func) \ static const struct earlycon_id __earlycon_##_name \
On Tue, Sep 08, 2015 at 12:38:59PM -0400, Mark Salter wrote:
On Tue, 2015-09-08 at 13:43 +0100, Leif Lindholm wrote:
The ACPI DBG2 table defines a debug console. Add support for parsing it and using it to select earlycon destination when no arguments provided.
Signed-off-by: Leif Lindholm leif.lindholm@linaro.org
arch/arm64/kernel/acpi.c | 2 + drivers/acpi/Makefile | 1 + drivers/acpi/console.c | 103 ++++++++++++++++++++++++++++++++++++++++++ drivers/of/fdt.c | 2 +- drivers/tty/serial/earlycon.c | 16 ++++--- include/linux/acpi.h | 4 ++ include/linux/serial_core.h | 9 ++-- 7 files changed, 126 insertions(+), 11 deletions(-) create mode 100644 drivers/acpi/console.c
diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c index b9a5623..be7600a 100644 --- a/arch/arm64/kernel/acpi.c +++ b/arch/arm64/kernel/acpi.c @@ -207,6 +207,8 @@ void __init acpi_boot_table_init(void) if (!param_acpi_force) disable_acpi(); }
- acpi_early_console_probe();
} void __init acpi_gic_init(void) diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile index b5e7cd8..a89587d 100644 --- a/drivers/acpi/Makefile +++ b/drivers/acpi/Makefile @@ -10,6 +10,7 @@ ccflags-$(CONFIG_ACPI_DEBUG) += -DACPI_DEBUG_OUTPUT # obj-y += tables.o obj-$(CONFIG_X86) += blacklist.o +obj-y += console.o
obj-$(CONFIG_SERIAL_EARLYCON) += console.o
to eliminate whole-file #ifdef
Yes, that makes more sense for this patch standalone, but I felt it would be a bit weird to add the conditionality here only to delete it in the subsequent patch. I don't feel strongly about it.
# # ACPI Core Subsystem (Interpreter) diff --git a/drivers/acpi/console.c b/drivers/acpi/console.c new file mode 100644 index 0000000..a985890 --- /dev/null +++ b/drivers/acpi/console.c @@ -0,0 +1,103 @@ +/*
- Copyright (c) 2012, Intel Corporation
- Copyright (c) 2015, Linaro Ltd.
- This program is free software; you can redistribute it and/or modify
- it under the terms of the GNU General Public License version 2 as
- published by the Free Software Foundation.
- */
+#define DEBUG +#define pr_fmt(fmt) "ACPI: " KBUILD_MODNAME ": " fmt
+#include <linux/acpi.h> +#include <linux/kernel.h> +#include <linux/serial_core.h>
+#define NUM_ELEMS(x) (sizeof(x) / sizeof(*x))
+#ifdef CONFIG_SERIAL_EARLYCON +static int use_earlycon __initdata; +static int __init setup_acpi_earlycon(char *buf) +{
- if (!buf)
use_earlycon = 1;- return 0;
+} +early_param("earlycon", setup_acpi_earlycon);
+extern struct earlycon_id __earlycon_table[];
+static __initdata struct {
- int id;
- const char *name;
+} subtypes[] = {
- {0, "uart8250"},
- {1, "uart8250"},
- {2, NULL},
- {3, "pl011"},
+};
Instead of having a table here, why not have an ACPI_EARLYCON_DECLARE() where individual drivers can provide an id similar to OF_EARLYCON_DECLARE() providing compatible strings?
The IDs are defined by the DBG2 specification, so it felt more natural to encapsulate it here. However, a comment to that effect would be useful. Or would you still prefer ACPI_EARLYCON_DECLARE(0, uart8250) ACPI_EARLYCON_DECLARE(1, uart8250) ... ?
+static int __init acpi_setup_earlycon(unsigned long addr, const char *driver) +{
- const struct earlycon_id *match;
- for (match = __earlycon_table; match->name[0]; match++)
if (strcmp(driver, match->name) == 0)return setup_earlycon_driver(addr, match->setup);- return -ENODEV;
+}
+static int __init acpi_parse_dbg2(struct acpi_table_header *table) +{
- struct acpi_table_dbg2 *dbg2;
- struct acpi_dbg2_device *entry;
- void *tbl_end;
- dbg2 = (struct acpi_table_dbg2 *)table;
- if (!dbg2) {
pr_debug("DBG2 not present.\n");return -ENODEV;- }
- tbl_end = (void *)table + table->length;
- entry = (struct acpi_dbg2_device *)((void *)dbg2 + dbg2->info_offset);
- while (((void *)entry) + sizeof(struct acpi_dbg2_device) < tbl_end) {
struct acpi_generic_address *addr;if (entry->revision != 0) {pr_debug("DBG2 revision %d not supported.\n",entry->revision);return -ENODEV;}addr = (void *)entry + entry->base_address_offset;pr_debug("DBG2 PROBE - console (%04x:%04x).\n",entry->port_type, entry->port_subtype);if (use_earlycon &&(entry->port_type == ACPI_DBG2_SERIAL_PORT) &&(entry->port_subtype < NUM_ELEMS(subtypes)))acpi_setup_earlycon(addr->address,subtypes[entry->port_subtype].name);Don't we need to handle space_id (and bit width) as well as address?
space_id? bit width? DBG2 is for quite primitive debug ports, already initialised by formware (or other pre-kernel agent).
entry = (struct acpi_dbg2_device *)((void *)entry + entry->length);- }
- return 0;
+}
+int __init acpi_early_console_probe(void) +{
- acpi_table_parse(ACPI_SIG_DBG2, acpi_parse_dbg2);
- return 0;
+} +#endif /* CONFIG_SERIAL_EARLYCON */ diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c index fcfc4c7..a96209f 100644 --- a/drivers/of/fdt.c +++ b/drivers/of/fdt.c @@ -829,7 +829,7 @@ int __init early_init_dt_scan_chosen_serial(void) if (!addr) return -ENXIO;
of_setup_earlycon(addr, match->data);
setup_earlycon_driver(addr, match->data);diff --git a/drivers/tty/serial/earlycon.c b/drivers/tty/serial/earlycon.c index 2bda09a..c063cbb 100644 --- a/drivers/tty/serial/earlycon.c +++ b/drivers/tty/serial/earlycon.c @@ -13,6 +13,7 @@ #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt +#include <linux/acpi.h> #include <linux/console.h> #include <linux/kernel.h> #include <linux/init.h> @@ -184,12 +185,16 @@ static int __init param_setup_earlycon(char *buf) int err; /*
* Just 'earlycon' is a valid param for devicetree earlycons;* don't generate a warning from parse_early_params() in that case
* Just 'earlycon' is a valid param for devicetree or ACPI earlycons;* ACPI cannot be parsed yet, so return without action if enabled.* Otherwise, attempt initialization using DT.
- if (!buf || !buf[0])
if (IS_ENABLED(CONFIG_OF_FLATTREE))
- if (!buf || !buf[0]) {
if (!acpi_disabled)How do we know for sure that "acpi" has been parsed before "earlycon"?
Because "arch" comes before "drivers" in kernel image link order. *twitch* Yes, not the best argument ever.
return 0;else if (IS_ENABLED(CONFIG_OF_FLATTREE)) return early_init_dt_scan_chosen_serial();- }
err = setup_earlycon(buf); if (err == -ENOENT || err == -EALREADY) @@ -198,8 +203,7 @@ static int __init param_setup_earlycon(char *buf) } early_param("earlycon", param_setup_earlycon); -int __init of_setup_earlycon(unsigned long addr,
int (*setup)(struct earlycon_device *, const char *))+int __init setup_earlycon_driver(unsigned long addr, earlycon_initfunc_t setup) { int err; struct uart_port *port = &early_console_dev.port; diff --git a/include/linux/acpi.h b/include/linux/acpi.h index 7235c48..88cb9c1 100644 --- a/include/linux/acpi.h +++ b/include/linux/acpi.h @@ -811,4 +811,8 @@ static inline struct acpi_device *acpi_get_next_child(struct device *dev, #endif +#if defined(CONFIG_ACPI) && defined(CONFIG_SERIAL_EARLYCON) +int __init acpi_early_console_probe(void); +#endif
#endif /*_LINUX_ACPI_H*/ diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h index 297d4fa..39e99b0 100644 --- a/include/linux/serial_core.h +++ b/include/linux/serial_core.h @@ -339,14 +339,15 @@ struct earlycon_device { unsigned int baud; }; +typedef int (*earlycon_initfunc_t)(struct earlycon_device *, const char *);
struct earlycon_id {
- char name[16];
- int (*setup)(struct earlycon_device *, const char *options);
- char name[16];
- earlycon_initfunc_t setup;
} __aligned(32); extern int setup_earlycon(char *buf); -extern int of_setup_earlycon(unsigned long addr,
int (*setup)(struct earlycon_device *, const char *));+extern int setup_earlycon_driver(unsigned long addr, earlycon_initfunc_t setup); #define EARLYCON_DECLARE(_name, func) \ static const struct earlycon_id __earlycon_##_name \
On Tue, 2015-09-08 at 18:17 +0100, Leif Lindholm wrote:
On Tue, Sep 08, 2015 at 12:38:59PM -0400, Mark Salter wrote:
On Tue, 2015-09-08 at 13:43 +0100, Leif Lindholm wrote:
The ACPI DBG2 table defines a debug console. Add support for parsing it and using it to select earlycon destination when no arguments provided.
Signed-off-by: Leif Lindholm leif.lindholm@linaro.org
arch/arm64/kernel/acpi.c | 2 + drivers/acpi/Makefile | 1 + drivers/acpi/console.c | 103 ++++++++++++++++++++++++++++++++++++++++++ drivers/of/fdt.c | 2 +- drivers/tty/serial/earlycon.c | 16 ++++--- include/linux/acpi.h | 4 ++ include/linux/serial_core.h | 9 ++-- 7 files changed, 126 insertions(+), 11 deletions(-) create mode 100644 drivers/acpi/console.c
diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c index b9a5623..be7600a 100644 --- a/arch/arm64/kernel/acpi.c +++ b/arch/arm64/kernel/acpi.c @@ -207,6 +207,8 @@ void __init acpi_boot_table_init(void) if (!param_acpi_force) disable_acpi(); }
- acpi_early_console_probe();
} void __init acpi_gic_init(void) diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile index b5e7cd8..a89587d 100644 --- a/drivers/acpi/Makefile +++ b/drivers/acpi/Makefile @@ -10,6 +10,7 @@ ccflags-$(CONFIG_ACPI_DEBUG) += -DACPI_DEBUG_OUTPUT # obj-y += tables.o obj-$(CONFIG_X86) += blacklist.o +obj-y += console.o
obj-$(CONFIG_SERIAL_EARLYCON) += console.o
to eliminate whole-file #ifdef
Yes, that makes more sense for this patch standalone, but I felt it would be a bit weird to add the conditionality here only to delete it in the subsequent patch. I don't feel strongly about it.
OIC. I didn't read ahead far enough.
# # ACPI Core Subsystem (Interpreter) diff --git a/drivers/acpi/console.c b/drivers/acpi/console.c new file mode 100644 index 0000000..a985890 --- /dev/null +++ b/drivers/acpi/console.c @@ -0,0 +1,103 @@ +/*
- Copyright (c) 2012, Intel Corporation
- Copyright (c) 2015, Linaro Ltd.
- This program is free software; you can redistribute it and/or modify
- it under the terms of the GNU General Public License version 2 as
- published by the Free Software Foundation.
- */
+#define DEBUG +#define pr_fmt(fmt) "ACPI: " KBUILD_MODNAME ": " fmt
+#include <linux/acpi.h> +#include <linux/kernel.h> +#include <linux/serial_core.h>
+#define NUM_ELEMS(x) (sizeof(x) / sizeof(*x))
+#ifdef CONFIG_SERIAL_EARLYCON +static int use_earlycon __initdata; +static int __init setup_acpi_earlycon(char *buf) +{
- if (!buf)
use_earlycon = 1;- return 0;
+} +early_param("earlycon", setup_acpi_earlycon);
+extern struct earlycon_id __earlycon_table[];
+static __initdata struct {
- int id;
- const char *name;
+} subtypes[] = {
- {0, "uart8250"},
- {1, "uart8250"},
- {2, NULL},
- {3, "pl011"},
+};
Instead of having a table here, why not have an ACPI_EARLYCON_DECLARE() where individual drivers can provide an id similar to OF_EARLYCON_DECLARE() providing compatible strings?
The IDs are defined by the DBG2 specification, so it felt more natural to encapsulate it here. However, a comment to that effect would be useful. Or would you still prefer ACPI_EARLYCON_DECLARE(0, uart8250) ACPI_EARLYCON_DECLARE(1, uart8250) ... ?
The idea is that the driver itself is only place that needs to handle a new uart type being supported rather than two places.
+static int __init acpi_setup_earlycon(unsigned long addr, const char *driver) +{
- const struct earlycon_id *match;
- for (match = __earlycon_table; match->name[0]; match++)
if (strcmp(driver, match->name) == 0)return setup_earlycon_driver(addr, match->setup);- return -ENODEV;
+}
+static int __init acpi_parse_dbg2(struct acpi_table_header *table) +{
- struct acpi_table_dbg2 *dbg2;
- struct acpi_dbg2_device *entry;
- void *tbl_end;
- dbg2 = (struct acpi_table_dbg2 *)table;
- if (!dbg2) {
pr_debug("DBG2 not present.\n");return -ENODEV;- }
- tbl_end = (void *)table + table->length;
- entry = (struct acpi_dbg2_device *)((void *)dbg2 + dbg2->info_offset);
- while (((void *)entry) + sizeof(struct acpi_dbg2_device) < tbl_end) {
struct acpi_generic_address *addr;if (entry->revision != 0) {pr_debug("DBG2 revision %d not supported.\n",entry->revision);return -ENODEV;}addr = (void *)entry + entry->base_address_offset;pr_debug("DBG2 PROBE - console (%04x:%04x).\n",entry->port_type, entry->port_subtype);if (use_earlycon &&(entry->port_type == ACPI_DBG2_SERIAL_PORT) &&(entry->port_subtype < NUM_ELEMS(subtypes)))acpi_setup_earlycon(addr->address,subtypes[entry->port_subtype].name);Don't we need to handle space_id (and bit width) as well as address?
space_id? bit width? DBG2 is for quite primitive debug ports, already initialised by formware (or other pre-kernel agent).
How else would it work for x86 where uart may be mmio or ioport? Even with firmware initialization, kernel still needs to poke at registers.
entry = (struct acpi_dbg2_device *)((void *)entry + entry->length);- }
- return 0;
+}
+int __init acpi_early_console_probe(void) +{
- acpi_table_parse(ACPI_SIG_DBG2, acpi_parse_dbg2);
- return 0;
+} +#endif /* CONFIG_SERIAL_EARLYCON */ diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c index fcfc4c7..a96209f 100644 --- a/drivers/of/fdt.c +++ b/drivers/of/fdt.c @@ -829,7 +829,7 @@ int __init early_init_dt_scan_chosen_serial(void) if (!addr) return -ENXIO;
of_setup_earlycon(addr, match->data);
setup_earlycon_driver(addr, match->data);diff --git a/drivers/tty/serial/earlycon.c b/drivers/tty/serial/earlycon.c index 2bda09a..c063cbb 100644 --- a/drivers/tty/serial/earlycon.c +++ b/drivers/tty/serial/earlycon.c @@ -13,6 +13,7 @@ #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt +#include <linux/acpi.h> #include <linux/console.h> #include <linux/kernel.h> #include <linux/init.h> @@ -184,12 +185,16 @@ static int __init param_setup_earlycon(char *buf) int err; /*
* Just 'earlycon' is a valid param for devicetree earlycons;* don't generate a warning from parse_early_params() in that case
* Just 'earlycon' is a valid param for devicetree or ACPI earlycons;* ACPI cannot be parsed yet, so return without action if enabled.* Otherwise, attempt initialization using DT.
- if (!buf || !buf[0])
if (IS_ENABLED(CONFIG_OF_FLATTREE))
- if (!buf || !buf[0]) {
if (!acpi_disabled)How do we know for sure that "acpi" has been parsed before "earlycon"?
Because "arch" comes before "drivers" in kernel image link order. *twitch* Yes, not the best argument ever.
return 0;else if (IS_ENABLED(CONFIG_OF_FLATTREE)) return early_init_dt_scan_chosen_serial();- }
err = setup_earlycon(buf); if (err == -ENOENT || err == -EALREADY) @@ -198,8 +203,7 @@ static int __init param_setup_earlycon(char *buf) } early_param("earlycon", param_setup_earlycon); -int __init of_setup_earlycon(unsigned long addr,
int (*setup)(struct earlycon_device *, const char *))+int __init setup_earlycon_driver(unsigned long addr, earlycon_initfunc_t setup) { int err; struct uart_port *port = &early_console_dev.port; diff --git a/include/linux/acpi.h b/include/linux/acpi.h index 7235c48..88cb9c1 100644 --- a/include/linux/acpi.h +++ b/include/linux/acpi.h @@ -811,4 +811,8 @@ static inline struct acpi_device *acpi_get_next_child(struct device *dev, #endif +#if defined(CONFIG_ACPI) && defined(CONFIG_SERIAL_EARLYCON) +int __init acpi_early_console_probe(void); +#endif
#endif /*_LINUX_ACPI_H*/ diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h index 297d4fa..39e99b0 100644 --- a/include/linux/serial_core.h +++ b/include/linux/serial_core.h @@ -339,14 +339,15 @@ struct earlycon_device { unsigned int baud; }; +typedef int (*earlycon_initfunc_t)(struct earlycon_device *, const char *);
struct earlycon_id {
- char name[16];
- int (*setup)(struct earlycon_device *, const char *options);
- char name[16];
- earlycon_initfunc_t setup;
} __aligned(32); extern int setup_earlycon(char *buf); -extern int of_setup_earlycon(unsigned long addr,
int (*setup)(struct earlycon_device *, const char *));+extern int setup_earlycon_driver(unsigned long addr, earlycon_initfunc_t setup); #define EARLYCON_DECLARE(_name, func) \ static const struct earlycon_id __earlycon_##_name \
On Tue, 2015-09-08 at 18:17 +0100, Leif Lindholm wrote:
*/
- if (!buf || !buf[0])
if (IS_ENABLED(CONFIG_OF_FLATTREE))
- if (!buf || !buf[0]) {
if (!acpi_disabled)How do we know for sure that "acpi" has been parsed before "earlycon"?
Because "arch" comes before "drivers" in kernel image link order. *twitch* Yes, not the best argument ever.
I don't think that matters. Things are parsed in order they are found on cmdline (except for early vs late distinction).
From: Torez Smith torez@redhat.com
If console= is not added to the kernel command line, the console is not registered until much further into the booting process. This patch adds support to parse the SPCR ACPI table to pull console support out, then use the appropriate drivers to set up console support earlier in the boot process.
Signed-off-by: Jon Masters jcm@redhat.com [rebased and cleaned up] Signed-off-by: Torez Smith torez@redhat.com [reworked to use _CRS, moved to drivers/acpi] Signed-off-by: Leif Lindholm leif.lindholm@linaro.org --- drivers/acpi/console.c | 157 +++++++++++++++++++++++++++++++++++++++ drivers/tty/serial/serial_core.c | 14 +++- include/linux/acpi.h | 11 ++- 3 files changed, 179 insertions(+), 3 deletions(-)
diff --git a/drivers/acpi/console.c b/drivers/acpi/console.c index a985890..02883a1 100644 --- a/drivers/acpi/console.c +++ b/drivers/acpi/console.c @@ -1,5 +1,6 @@ /* * Copyright (c) 2012, Intel Corporation + * Copyright (c) 2015, Red Hat, Inc. * Copyright (c) 2015, Linaro Ltd. * * This program is free software; you can redistribute it and/or modify @@ -12,11 +13,17 @@ #define pr_fmt(fmt) "ACPI: " KBUILD_MODNAME ": " fmt
#include <linux/acpi.h> +#include <linux/console.h> #include <linux/kernel.h> #include <linux/serial_core.h> +#include <linux/tty.h>
#define NUM_ELEMS(x) (sizeof(x) / sizeof(*x))
+static u64 acpi_serial_addr; +static struct acpi_device *acpi_serial_device; +static char *acpi_serial_options; + #ifdef CONFIG_SERIAL_EARLYCON static int use_earlycon __initdata; static int __init setup_acpi_earlycon(char *buf) @@ -101,3 +108,153 @@ int __init acpi_early_console_probe(void) return 0; } #endif /* CONFIG_SERIAL_EARLYCON */ + +/* + * Parse the SPCR table. If we are not working with version 2 or + * higher, bail. + * Otherwise, pull out the baud rate and address to the console device. + */ +static int __init acpi_parse_spcr(struct acpi_table_header *table) +{ + struct acpi_table_spcr *spcr = (struct acpi_table_spcr *)table; + + if (table->revision < 2) + return -EOPNOTSUPP; + + /* Handle possible alignment issues */ + memcpy(&acpi_serial_addr, + &spcr->serial_port.address, sizeof(acpi_serial_addr)); + + /* + * The baud rate the BIOS used for redirection. Valid values are.... + * 3 = 9600 + * 4 = 19200 + * 6 = 57600 + * 7 = 115200 + * 0-2, 5, 8 - 255 = reserved + */ + switch (spcr->baud_rate) { + case 3: + acpi_serial_options = "9600"; + break; + case 4: + acpi_serial_options = "19200"; + break; + case 6: + acpi_serial_options = "57600"; + break; + case 7: + acpi_serial_options = "115200"; + break; + default: + acpi_serial_options = ""; + break; + } + + pr_info("SPCR serial device: 0x%llx (options: %s)\n", + acpi_serial_addr, acpi_serial_options); + + return 0; +} + +/* + * Parse an ACPI "Device" to determine if it represents the + * data found in the SPCR table. If the associated Device has + * and Address entry, and, that Address matches the one found + * in our SPCR table, it's the entry we are interested in. + * + */ +static acpi_status acpi_spcr_device_scan(acpi_handle handle, + u32 level, void *context, void **retv) +{ + unsigned long long addr = 0; + struct acpi_buffer name_buffer = { ACPI_ALLOCATE_BUFFER, NULL }; + acpi_status status = AE_OK; + struct acpi_device *adev; + struct list_head resource_list; + struct resource_entry *rentry; + + status = acpi_get_name(handle, ACPI_FULL_PATHNAME, &name_buffer); + if (ACPI_FAILURE(status)) + return status; + + adev = acpi_bus_get_acpi_device(handle); + if (!adev) { + pr_err("Err locating SPCR device from ACPI handle\n"); + return AE_OK; /* skip this one */ + } + + /* + * Read device address from _CRS. + */ + INIT_LIST_HEAD(&resource_list); + if (acpi_dev_get_resources(adev, &resource_list, NULL, NULL) <= 0) + return AE_OK; + + list_for_each_entry(rentry, &resource_list, node) { + if (resource_type(rentry->res) == IORESOURCE_MEM) + addr = rentry->res->start; + } + acpi_dev_free_resource_list(&resource_list); + + if (addr == acpi_serial_addr) { + acpi_serial_device = adev; + + pr_info("SPCR serial console: %s (0x%llx)\n", + (char *)(name_buffer.pointer), addr); + + return AE_OK; /* harmless to continue */ + } + + /* continue */ + return AE_OK; /* continue */ +} + +static int __init acpi_setup_spcr(void) +{ + if (0 != acpi_table_parse(ACPI_SIG_SPCR, acpi_parse_spcr)) { + pr_warn("SPCR table not found - auto console disabled\n"); + return -ENODEV; + } + + acpi_walk_namespace(ACPI_TYPE_DEVICE, ACPI_ROOT_OBJECT, + ACPI_UINT32_MAX, acpi_spcr_device_scan, + NULL, NULL, NULL); + + return 0; +} + +static int __init acpi_spcr_setup(void) +{ + /* + * If ACPI is enabled, scan the tables for + * automatic console configuration + */ + if (!acpi_disabled) + acpi_setup_spcr(); + + return 0; +} +subsys_initcall_sync(acpi_spcr_setup); + +/** + * acpi_console_check() - Check for and configure console from ACPI information + * @adev - Pointer to device + * @name - Name to use for preferred console without index. ex. "ttyS" + * @index - Index to use for preferred console. + * + * Check if the given device matches the information provided in the SPCR table + * If it does then register it as the preferred console and return TRUE. + * Otherwise return FALSE. + */ +bool acpi_console_check(struct acpi_device *adev, char *name, int index) +{ + if (acpi_disabled || !adev || adev != acpi_serial_device + || console_set_on_cmdline) + return false; + + pr_info("adding preferred console [%s]\n", name); + + return !add_preferred_console(name, index, + kstrdup(acpi_serial_options, GFP_KERNEL)); +} diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c index 603d2cc..4b20bc6 100644 --- a/drivers/tty/serial/serial_core.c +++ b/drivers/tty/serial/serial_core.c @@ -34,6 +34,7 @@ #include <linux/serial_core.h> #include <linux/delay.h> #include <linux/mutex.h> +#include <linux/acpi.h>
#include <asm/irq.h> #include <asm/uaccess.h> @@ -2696,9 +2697,18 @@ int uart_add_one_port(struct uart_driver *drv, struct uart_port *uport) spin_lock_init(&uport->lock); lockdep_set_class(&uport->lock, &port_lock_key); } - if (uport->cons && uport->dev) - of_console_check(uport->dev->of_node, uport->cons->name, uport->line);
+ /* + * Support both open FW and ACPI access to console definitions. + * Both of_console_check() and acpi_console_check() will call + * add_preferred_console() if a console definition is found. + */ + if (uport->cons && uport->dev) { + if (!acpi_console_check(ACPI_COMPANION(uport->dev), + uport->cons->name, uport->line)) + of_console_check(uport->dev->of_node, + uport->cons->name, uport->line); + } uart_configure_port(drv, state, uport);
num_groups = 2; diff --git a/include/linux/acpi.h b/include/linux/acpi.h index 88cb9c1..f1b9a64 100644 --- a/include/linux/acpi.h +++ b/include/linux/acpi.h @@ -811,8 +811,17 @@ static inline struct acpi_device *acpi_get_next_child(struct device *dev,
#endif
-#if defined(CONFIG_ACPI) && defined(CONFIG_SERIAL_EARLYCON) +#if defined(CONFIG_ACPI) +# if defined(CONFIG_SERIAL_EARLYCON) int __init acpi_early_console_probe(void); +# endif +bool acpi_console_check(struct acpi_device *adev, char *name, int index); +#else +static inline bool acpi_console_check(struct acpi_device *adev, char *name, + int index) +{ + return FALSE; +} #endif
#endif /*_LINUX_ACPI_H*/
On Tue, 2015-09-08 at 13:43 +0100, Leif Lindholm wrote:
From: Torez Smith torez@redhat.com
If console= is not added to the kernel command line, the console is not registered until much further into the booting process. This patch adds support to parse the SPCR ACPI table to pull console support out, then use the appropriate drivers to set up console support earlier in the boot process.
Signed-off-by: Jon Masters jcm@redhat.com [rebased and cleaned up] Signed-off-by: Torez Smith torez@redhat.com [reworked to use _CRS, moved to drivers/acpi] Signed-off-by: Leif Lindholm leif.lindholm@linaro.org
drivers/acpi/console.c | 157 +++++++++++++++++++++++++++++++++++++++ drivers/tty/serial/serial_core.c | 14 +++- include/linux/acpi.h | 11 ++- 3 files changed, 179 insertions(+), 3 deletions(-)
diff --git a/drivers/acpi/console.c b/drivers/acpi/console.c index a985890..02883a1 100644 --- a/drivers/acpi/console.c +++ b/drivers/acpi/console.c @@ -1,5 +1,6 @@ /*
- Copyright (c) 2012, Intel Corporation
- Copyright (c) 2015, Red Hat, Inc.
- Copyright (c) 2015, Linaro Ltd.
- This program is free software; you can redistribute it and/or modify
@@ -12,11 +13,17 @@ #define pr_fmt(fmt) "ACPI: " KBUILD_MODNAME ": " fmt #include <linux/acpi.h> +#include <linux/console.h> #include <linux/kernel.h> #include <linux/serial_core.h> +#include <linux/tty.h> #define NUM_ELEMS(x) (sizeof(x) / sizeof(*x)) +static u64 acpi_serial_addr; +static struct acpi_device *acpi_serial_device; +static char *acpi_serial_options;
#ifdef CONFIG_SERIAL_EARLYCON static int use_earlycon __initdata; static int __init setup_acpi_earlycon(char *buf) @@ -101,3 +108,153 @@ int __init acpi_early_console_probe(void) return 0; } #endif /* CONFIG_SERIAL_EARLYCON */
+/*
- Parse the SPCR table. If we are not working with version 2 or
- higher, bail.
- Otherwise, pull out the baud rate and address to the console device.
- */
+static int __init acpi_parse_spcr(struct acpi_table_header *table) +{
- struct acpi_table_spcr *spcr = (struct acpi_table_spcr *)table;
- if (table->revision < 2)
return -EOPNOTSUPP;- /* Handle possible alignment issues */
- memcpy(&acpi_serial_addr,
&spcr->serial_port.address, sizeof(acpi_serial_addr));- /*
* The baud rate the BIOS used for redirection. Valid values are....* 3 = 9600* 4 = 19200* 6 = 57600* 7 = 115200* 0-2, 5, 8 - 255 = reserved- */
- switch (spcr->baud_rate) {
- case 3:
acpi_serial_options = "9600";break;- case 4:
acpi_serial_options = "19200";break;- case 6:
acpi_serial_options = "57600";break;- case 7:
acpi_serial_options = "115200";break;- default:
acpi_serial_options = "";break;- }
- pr_info("SPCR serial device: 0x%llx (options: %s)\n",
acpi_serial_addr, acpi_serial_options);- return 0;
+}
+/*
- Parse an ACPI "Device" to determine if it represents the
- data found in the SPCR table. If the associated Device has
- and Address entry, and, that Address matches the one found
- in our SPCR table, it's the entry we are interested in.
- */
+static acpi_status acpi_spcr_device_scan(acpi_handle handle,
u32 level, void *context, void **retv)+{
- unsigned long long addr = 0;
- struct acpi_buffer name_buffer = { ACPI_ALLOCATE_BUFFER, NULL };
- acpi_status status = AE_OK;
- struct acpi_device *adev;
- struct list_head resource_list;
- struct resource_entry *rentry;
- status = acpi_get_name(handle, ACPI_FULL_PATHNAME, &name_buffer);
- if (ACPI_FAILURE(status))
return status;
acpi_get_name() is asked to allocate a buffer for the name, but that buffer doesn't get freed below. The name is only used in the pr_info call, so the acpi_get_name() call should probably be moved inside that if block and a kfree(name_buffer.pointer) added.
- adev = acpi_bus_get_acpi_device(handle);
- if (!adev) {
pr_err("Err locating SPCR device from ACPI handle\n");return AE_OK; /* skip this one */- }
- /*
* Read device address from _CRS.*/- INIT_LIST_HEAD(&resource_list);
- if (acpi_dev_get_resources(adev, &resource_list, NULL, NULL) <= 0)
return AE_OK;- list_for_each_entry(rentry, &resource_list, node) {
if (resource_type(rentry->res) == IORESOURCE_MEM)addr = rentry->res->start;- }
- acpi_dev_free_resource_list(&resource_list);
- if (addr == acpi_serial_addr) {
acpi_serial_device = adev;pr_info("SPCR serial console: %s (0x%llx)\n",(char *)(name_buffer.pointer), addr);return AE_OK; /* harmless to continue */- }
- /* continue */
- return AE_OK; /* continue */
+}
+static int __init acpi_setup_spcr(void) +{
- if (0 != acpi_table_parse(ACPI_SIG_SPCR, acpi_parse_spcr)) {
pr_warn("SPCR table not found - auto console disabled\n");return -ENODEV;- }
- acpi_walk_namespace(ACPI_TYPE_DEVICE, ACPI_ROOT_OBJECT,
ACPI_UINT32_MAX, acpi_spcr_device_scan,NULL, NULL, NULL);- return 0;
+}
+static int __init acpi_spcr_setup(void) +{
- /*
* If ACPI is enabled, scan the tables for* automatic console configuration*/- if (!acpi_disabled)
acpi_setup_spcr();- return 0;
+} +subsys_initcall_sync(acpi_spcr_setup);
+/**
- acpi_console_check() - Check for and configure console from ACPI information
- @adev - Pointer to device
- @name - Name to use for preferred console without index. ex. "ttyS"
- @index - Index to use for preferred console.
- Check if the given device matches the information provided in the SPCR table
- If it does then register it as the preferred console and return TRUE.
- Otherwise return FALSE.
- */
+bool acpi_console_check(struct acpi_device *adev, char *name, int index) +{
- if (acpi_disabled || !adev || adev != acpi_serial_device
|| console_set_on_cmdline)return false;- pr_info("adding preferred console [%s]\n", name);
- return !add_preferred_console(name, index,
kstrdup(acpi_serial_options, GFP_KERNEL));+} diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c index 603d2cc..4b20bc6 100644 --- a/drivers/tty/serial/serial_core.c +++ b/drivers/tty/serial/serial_core.c @@ -34,6 +34,7 @@ #include <linux/serial_core.h> #include <linux/delay.h> #include <linux/mutex.h> +#include <linux/acpi.h> #include <asm/irq.h> #include <asm/uaccess.h> @@ -2696,9 +2697,18 @@ int uart_add_one_port(struct uart_driver *drv, struct uart_port *uport) spin_lock_init(&uport->lock); lockdep_set_class(&uport->lock, &port_lock_key); }
- if (uport->cons && uport->dev)
of_console_check(uport->dev->of_node, uport->cons->name, uport->line);
- /*
* Support both open FW and ACPI access to console definitions.* Both of_console_check() and acpi_console_check() will call* add_preferred_console() if a console definition is found.*/- if (uport->cons && uport->dev) {
if (!acpi_console_check(ACPI_COMPANION(uport->dev),uport->cons->name, uport->line))of_console_check(uport->dev->of_node,uport->cons->name, uport->line);- } uart_configure_port(drv, state, uport);
num_groups = 2; diff --git a/include/linux/acpi.h b/include/linux/acpi.h index 88cb9c1..f1b9a64 100644 --- a/include/linux/acpi.h +++ b/include/linux/acpi.h @@ -811,8 +811,17 @@ static inline struct acpi_device *acpi_get_next_child(struct device *dev, #endif -#if defined(CONFIG_ACPI) && defined(CONFIG_SERIAL_EARLYCON) +#if defined(CONFIG_ACPI) +# if defined(CONFIG_SERIAL_EARLYCON) int __init acpi_early_console_probe(void); +# endif +bool acpi_console_check(struct acpi_device *adev, char *name, int index); +#else +static inline bool acpi_console_check(struct acpi_device *adev, char *name,
int index)+{
- return FALSE;
+} #endif #endif /*_LINUX_ACPI_H*/
--- drivers/tty/serial/amba-pl011.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c index 452dbba..31cf985 100644 --- a/drivers/tty/serial/amba-pl011.c +++ b/drivers/tty/serial/amba-pl011.c @@ -2806,7 +2806,7 @@ static void __exit pl011_exit(void) * While this can be a module, if builtin it's most likely the console * So let's leave module_exit but move module_init to an earlier place */ -arch_initcall(pl011_init); +device_initcall(pl011_init); module_exit(pl011_exit);
MODULE_AUTHOR("ARM Ltd/Deep Blue Solutions Ltd");
On Tue, Sep 08, 2015 at 01:43:37PM +0100, Leif Lindholm wrote:
drivers/tty/serial/amba-pl011.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c index 452dbba..31cf985 100644 --- a/drivers/tty/serial/amba-pl011.c +++ b/drivers/tty/serial/amba-pl011.c @@ -2806,7 +2806,7 @@ static void __exit pl011_exit(void)
- While this can be a module, if builtin it's most likely the console
- So let's leave module_exit but move module_init to an earlier place
*/ -arch_initcall(pl011_init); +device_initcall(pl011_init); module_exit(pl011_exit);
What's the ordering constraint that you're trying to solve with this?
The cover didn't mention anything, and there's no commit message.
Mark.
MODULE_AUTHOR("ARM Ltd/Deep Blue Solutions Ltd"); -- 2.1.4
On Tue, Sep 08, 2015 at 01:56:08PM +0100, Mark Rutland wrote:
On Tue, Sep 08, 2015 at 01:43:37PM +0100, Leif Lindholm wrote:
drivers/tty/serial/amba-pl011.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c index 452dbba..31cf985 100644 --- a/drivers/tty/serial/amba-pl011.c +++ b/drivers/tty/serial/amba-pl011.c @@ -2806,7 +2806,7 @@ static void __exit pl011_exit(void)
- While this can be a module, if builtin it's most likely the console
- So let's leave module_exit but move module_init to an earlier place
*/ -arch_initcall(pl011_init); +device_initcall(pl011_init); module_exit(pl011_exit);
What's the ordering constraint that you're trying to solve with this?
Basically that _CRS is not available until the ACPI subsystem is up - so when uart_add_one_port() calls acpi_console_check(), that function looks for a match against variables that have not been set yet.
The cover didn't mention anything, and there's no commit message.
That's because I really don't want it committed :)
Mark.
MODULE_AUTHOR("ARM Ltd/Deep Blue Solutions Ltd"); -- 2.1.4
Hi Leif,
On Tue, Sep 8, 2015 at 8:43 PM, Leif Lindholm leif.lindholm@linaro.org wrote:
Support for configuring bootconsole and console via the ACPI tables DBG2 (Debug Port Table 2) [1] and SPCR (Serial Port Console Redirection Table) [2], defined by Microsoft, has been discussed on and off over the years.
[1] https://msdn.microsoft.com/en-us/library/windows/hardware/dn639131(v=vs.85).... [2] https://msdn.microsoft.com/en-us/library/windows/hardware/dn639132(v=vs.85)....
Licensing concerns have prevented this happening in the past, but as of 10 August 2015, these tables have both been released also under OWF 1.0 (http://www.openwebfoundation.org/legal/the-owf-1-0-agreements/owfa-1-0) which is think is noncontroversially GPL-compatible?
This set is a first attempt at implementing this.
From distribution view, the patchset looks very useful.
I have one simple question, are there other candidates for conveying this kind of information like what DBG2/SPCR does? Or are DBG2 and SPCR the offical speficiations to do such things in ARM server industry?
Thanks,
On Wed, Sep 23, 2015 at 03:38:20PM +0800, Ming Lei wrote:
Hi Leif,
On Tue, Sep 8, 2015 at 8:43 PM, Leif Lindholm leif.lindholm@linaro.org wrote:
Support for configuring bootconsole and console via the ACPI tables DBG2 (Debug Port Table 2) [1] and SPCR (Serial Port Console Redirection Table) [2], defined by Microsoft, has been discussed on and off over the years.
[1] https://msdn.microsoft.com/en-us/library/windows/hardware/dn639131(v=vs.85).... [2] https://msdn.microsoft.com/en-us/library/windows/hardware/dn639132(v=vs.85)....
Licensing concerns have prevented this happening in the past, but as of 10 August 2015, these tables have both been released also under OWF 1.0 (http://www.openwebfoundation.org/legal/the-owf-1-0-agreements/owfa-1-0) which is think is noncontroversially GPL-compatible?
This set is a first attempt at implementing this.
From distribution view, the patchset looks very useful.
I have one simple question, are there other candidates for conveying this kind of information like what DBG2/SPCR does? Or are DBG2 and SPCR the offical speficiations to do such things in ARM server industry?
They are mandated in the SBBR spec
Graeme
Hi,
On 09/08/2015 08:43 AM, Leif Lindholm wrote:
Support for configuring bootconsole and console via the ACPI tables DBG2 (Debug Port Table 2) [1] and SPCR (Serial Port Console Redirection Table) [2], defined by Microsoft, has been discussed on and off over the years.
[1] https://msdn.microsoft.com/en-us/library/windows/hardware/dn639131(v=vs.85).... [2] https://msdn.microsoft.com/en-us/library/windows/hardware/dn639132(v=vs.85)....
Licensing concerns have prevented this happening in the past, but as of 10 August 2015, these tables have both been released also under OWF 1.0 (http://www.openwebfoundation.org/legal/the-owf-1-0-agreements/owfa-1-0) which is think is noncontroversially GPL-compatible?
This set is a first attempt at implementing this.
Submitting as an RFC since the SPCR handling currently depends on the console driver being initialized after subsystem initcalls. Workaround to enable testing surrounding infrastructure in 5/5, _really_ not intended to be merged. (Suggestions for acceptable ways of working around this appreciated.)
What's required for this to graduate from RFC status? Is anyone working on it?
Thanks, Christopher Covington
On 01/12/2016 08:14 AM, Christopher Covington wrote:
Hi,
On 09/08/2015 08:43 AM, Leif Lindholm wrote:
Support for configuring bootconsole and console via the ACPI tables DBG2 (Debug Port Table 2) [1] and SPCR (Serial Port Console Redirection Table) [2], defined by Microsoft, has been discussed on and off over the years.
[1] https://msdn.microsoft.com/en-us/library/windows/hardware/dn639131(v=vs.85).... [2] https://msdn.microsoft.com/en-us/library/windows/hardware/dn639132(v=vs.85)....
Licensing concerns have prevented this happening in the past, but as of 10 August 2015, these tables have both been released also under OWF 1.0 (http://www.openwebfoundation.org/legal/the-owf-1-0-agreements/owfa-1-0) which is think is noncontroversially GPL-compatible?
This set is a first attempt at implementing this.
Submitting as an RFC since the SPCR handling currently depends on the console driver being initialized after subsystem initcalls. Workaround to enable testing surrounding infrastructure in 5/5, _really_ not intended to be merged. (Suggestions for acceptable ways of working around this appreciated.)
What's required for this to graduate from RFC status? Is anyone working on it?
Thanks, Christopher Covington
Well, we have asked Aleksey Makarov aleksy.makarov@linaro.org to pick this task up from Leif within Linaro. Perhaps drop him a line and see if the two of you can collaborate on this somehow? I think it just needs nudging and a bit of TLC to get it past being an RFC.
-
Al Stone -
Christopher Covington -
G Gregory -
Leif Lindholm -
Mark Rutland -
Mark Salter -
Ming Lei