* [RFC 1/5] arm64: move acpi/dt decision earlier in boot process
2015-09-08 12:43 [RFC 0/5] console/acpi: add DBG2 and SPCR console configuration Leif Lindholm
@ 2015-09-08 12:43 ` Leif Lindholm
2015-09-08 12:43 ` [RFC 2/5] of/serial: move earlycon early_param handling to serial Leif Lindholm
` (5 subsequent siblings)
6 siblings, 0 replies; 22+ messages in thread
From: Leif Lindholm @ 2015-09-08 12:43 UTC (permalink / raw)
To: linux-arm-kernel, devicetree, linux-serial, linux-acpi,
linux-kernel
Cc: mark.rutland, al.stone, torez, jcm, graeme.gregory, linaro-acpi,
lv.zheng
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.
--
2.1.4
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [RFC 2/5] of/serial: move earlycon early_param handling to serial
2015-09-08 12:43 [RFC 0/5] console/acpi: add DBG2 and SPCR console configuration Leif Lindholm
2015-09-08 12:43 ` [RFC 1/5] arm64: move acpi/dt decision earlier in boot process Leif Lindholm
@ 2015-09-08 12:43 ` Leif Lindholm
[not found] ` <1441716217-23786-3-git-send-email-leif.lindholm-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
2015-09-08 12:43 ` [RFC 3/5] acpi/serial: add DBG2 earlycon support Leif Lindholm
` (4 subsequent siblings)
6 siblings, 1 reply; 22+ messages in thread
From: Leif Lindholm @ 2015-09-08 12:43 UTC (permalink / raw)
To: linux-arm-kernel, devicetree, linux-serial, linux-acpi,
linux-kernel
Cc: mark.rutland, al.stone, torez, jcm, graeme.gregory, linaro-acpi,
lv.zheng
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);
--
2.1.4
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [RFC 3/5] acpi/serial: add DBG2 earlycon support
2015-09-08 12:43 [RFC 0/5] console/acpi: add DBG2 and SPCR console configuration Leif Lindholm
2015-09-08 12:43 ` [RFC 1/5] arm64: move acpi/dt decision earlier in boot process Leif Lindholm
2015-09-08 12:43 ` [RFC 2/5] of/serial: move earlycon early_param handling to serial Leif Lindholm
@ 2015-09-08 12:43 ` Leif Lindholm
2015-09-08 13:09 ` Mark Rutland
2015-09-08 16:38 ` [Linaro-acpi] " Mark Salter
2015-09-08 12:43 ` [RFC 4/5] tty/console: use SPCR table to define console Leif Lindholm
` (3 subsequent siblings)
6 siblings, 2 replies; 22+ messages in thread
From: Leif Lindholm @ 2015-09-08 12:43 UTC (permalink / raw)
To: linux-arm-kernel, devicetree, linux-serial, linux-acpi,
linux-kernel
Cc: mark.rutland, al.stone, torez, jcm, graeme.gregory, linaro-acpi,
lv.zheng
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 \
--
2.1.4
^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [RFC 3/5] acpi/serial: add DBG2 earlycon support
2015-09-08 12:43 ` [RFC 3/5] acpi/serial: add DBG2 earlycon support Leif Lindholm
@ 2015-09-08 13:09 ` Mark Rutland
2015-09-08 17:03 ` Leif Lindholm
2015-09-08 16:38 ` [Linaro-acpi] " Mark Salter
1 sibling, 1 reply; 22+ messages in thread
From: Mark Rutland @ 2015-09-08 13:09 UTC (permalink / raw)
To: Leif Lindholm
Cc: linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org,
linux-serial@vger.kernel.org, linux-acpi@vger.kernel.org,
linux-kernel@vger.kernel.org, al.stone@linaro.org,
torez@redhat.com, jcm@redhat.com, graeme.gregory@linaro.org,
linaro-acpi@lists.linaro.org, lv.zheng@intel.com
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.
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [RFC 3/5] acpi/serial: add DBG2 earlycon support
2015-09-08 13:09 ` Mark Rutland
@ 2015-09-08 17:03 ` Leif Lindholm
2015-09-09 9:36 ` Mark Rutland
0 siblings, 1 reply; 22+ messages in thread
From: Leif Lindholm @ 2015-09-08 17:03 UTC (permalink / raw)
To: Mark Rutland
Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-serial-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-acpi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
al.stone-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org,
torez-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org,
jcm-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org,
graeme.gregory-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org,
linaro-acpi-cunTk1MwBs8s++Sfvej+rw@public.gmane.org,
lv.zheng-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org
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-QSEj5FYQhm4dnm+yROfE0A@public.gmane.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
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [RFC 3/5] acpi/serial: add DBG2 earlycon support
2015-09-08 17:03 ` Leif Lindholm
@ 2015-09-09 9:36 ` Mark Rutland
0 siblings, 0 replies; 22+ messages in thread
From: Mark Rutland @ 2015-09-09 9:36 UTC (permalink / raw)
To: Leif Lindholm
Cc: linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org,
linux-serial@vger.kernel.org, linux-acpi@vger.kernel.org,
linux-kernel@vger.kernel.org, al.stone@linaro.org,
torez@redhat.com, jcm@redhat.com, graeme.gregory@linaro.org,
linaro-acpi@lists.linaro.org, lv.zheng@intel.com
> > > +#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.
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Linaro-acpi] [RFC 3/5] acpi/serial: add DBG2 earlycon support
2015-09-08 12:43 ` [RFC 3/5] acpi/serial: add DBG2 earlycon support Leif Lindholm
2015-09-08 13:09 ` Mark Rutland
@ 2015-09-08 16:38 ` Mark Salter
2015-09-08 17:17 ` Leif Lindholm
1 sibling, 1 reply; 22+ messages in thread
From: Mark Salter @ 2015-09-08 16:38 UTC (permalink / raw)
To: Leif Lindholm, linux-arm-kernel, devicetree, linux-serial,
linux-acpi, linux-kernel
Cc: linaro-acpi, jcm, torez, lv.zheng
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);
> 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)
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 \
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Linaro-acpi] [RFC 3/5] acpi/serial: add DBG2 earlycon support
2015-09-08 16:38 ` [Linaro-acpi] " Mark Salter
@ 2015-09-08 17:17 ` Leif Lindholm
[not found] ` <20150908171744.GQ10728-t77nlHhSwNqAroYi2ySoxKxOck334EZe@public.gmane.org>
2015-09-08 17:53 ` Mark Salter
0 siblings, 2 replies; 22+ messages in thread
From: Leif Lindholm @ 2015-09-08 17:17 UTC (permalink / raw)
To: Mark Salter
Cc: linux-arm-kernel, devicetree, linux-serial, linux-acpi,
linux-kernel, linaro-acpi, jcm, torez, lv.zheng
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);
> > 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)
>
> 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 \
>
^ permalink raw reply [flat|nested] 22+ messages in thread
[parent not found: <20150908171744.GQ10728-t77nlHhSwNqAroYi2ySoxKxOck334EZe@public.gmane.org>]
* Re: [Linaro-acpi] [RFC 3/5] acpi/serial: add DBG2 earlycon support
[not found] ` <20150908171744.GQ10728-t77nlHhSwNqAroYi2ySoxKxOck334EZe@public.gmane.org>
@ 2015-09-08 17:40 ` Mark Salter
0 siblings, 0 replies; 22+ messages in thread
From: Mark Salter @ 2015-09-08 17:40 UTC (permalink / raw)
To: Leif Lindholm
Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-serial-u79uwXL29TY76Z2rM5mHXA,
linux-acpi-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linaro-acpi-cunTk1MwBs8s++Sfvej+rw, jcm-H+wXaHxf7aLQT0dZR+AlfA,
torez-H+wXaHxf7aLQT0dZR+AlfA, lv.zheng-ral2JQCrhuEAvxtiuMwx3w
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-QSEj5FYQhm4dnm+yROfE0A@public.gmane.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);
> > > 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)
> >
> > 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 \
> >
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [Linaro-acpi] [RFC 3/5] acpi/serial: add DBG2 earlycon support
2015-09-08 17:17 ` Leif Lindholm
[not found] ` <20150908171744.GQ10728-t77nlHhSwNqAroYi2ySoxKxOck334EZe@public.gmane.org>
@ 2015-09-08 17:53 ` Mark Salter
1 sibling, 0 replies; 22+ messages in thread
From: Mark Salter @ 2015-09-08 17:53 UTC (permalink / raw)
To: Leif Lindholm
Cc: linux-arm-kernel, devicetree, linux-serial, linux-acpi,
linux-kernel, linaro-acpi, jcm, torez, lv.zheng
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).
^ permalink raw reply [flat|nested] 22+ messages in thread
* [RFC 4/5] tty/console: use SPCR table to define console
2015-09-08 12:43 [RFC 0/5] console/acpi: add DBG2 and SPCR console configuration Leif Lindholm
` (2 preceding siblings ...)
2015-09-08 12:43 ` [RFC 3/5] acpi/serial: add DBG2 earlycon support Leif Lindholm
@ 2015-09-08 12:43 ` Leif Lindholm
2015-09-10 15:28 ` [Linaro-acpi] " Mark Salter
[not found] ` <1441716217-23786-1-git-send-email-leif.lindholm-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
` (2 subsequent siblings)
6 siblings, 1 reply; 22+ messages in thread
From: Leif Lindholm @ 2015-09-08 12:43 UTC (permalink / raw)
To: linux-arm-kernel, devicetree, linux-serial, linux-acpi,
linux-kernel
Cc: mark.rutland, al.stone, torez, jcm, graeme.gregory, linaro-acpi,
lv.zheng
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*/
--
2.1.4
^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [Linaro-acpi] [RFC 4/5] tty/console: use SPCR table to define console
2015-09-08 12:43 ` [RFC 4/5] tty/console: use SPCR table to define console Leif Lindholm
@ 2015-09-10 15:28 ` Mark Salter
0 siblings, 0 replies; 22+ messages in thread
From: Mark Salter @ 2015-09-10 15:28 UTC (permalink / raw)
To: Leif Lindholm, linux-arm-kernel, devicetree, linux-serial,
linux-acpi, linux-kernel
Cc: linaro-acpi, jcm, torez, lv.zheng
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*/
^ permalink raw reply [flat|nested] 22+ messages in thread
[parent not found: <1441716217-23786-1-git-send-email-leif.lindholm-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>]
* [RFC 5/5] HACK: serial: move pl011 initcall to device_initcall
[not found] ` <1441716217-23786-1-git-send-email-leif.lindholm-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
@ 2015-09-08 12:43 ` Leif Lindholm
2015-09-08 12:56 ` Mark Rutland
0 siblings, 1 reply; 22+ messages in thread
From: Leif Lindholm @ 2015-09-08 12:43 UTC (permalink / raw)
To: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-serial-u79uwXL29TY76Z2rM5mHXA,
linux-acpi-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
Cc: mark.rutland-5wv7dgnIgG8, al.stone-QSEj5FYQhm4dnm+yROfE0A,
torez-H+wXaHxf7aLQT0dZR+AlfA, jcm-H+wXaHxf7aLQT0dZR+AlfA,
graeme.gregory-QSEj5FYQhm4dnm+yROfE0A,
linaro-acpi-cunTk1MwBs8s++Sfvej+rw,
lv.zheng-ral2JQCrhuEAvxtiuMwx3w
---
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");
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [RFC 5/5] HACK: serial: move pl011 initcall to device_initcall
2015-09-08 12:43 ` [RFC 5/5] HACK: serial: move pl011 initcall to device_initcall Leif Lindholm
@ 2015-09-08 12:56 ` Mark Rutland
2015-09-08 16:55 ` Leif Lindholm
0 siblings, 1 reply; 22+ messages in thread
From: Mark Rutland @ 2015-09-08 12:56 UTC (permalink / raw)
To: Leif Lindholm
Cc: linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org,
linux-serial@vger.kernel.org, linux-acpi@vger.kernel.org,
linux-kernel@vger.kernel.org, al.stone@linaro.org,
torez@redhat.com, jcm@redhat.com, graeme.gregory@linaro.org,
linaro-acpi@lists.linaro.org, lv.zheng@intel.com
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
>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [RFC 5/5] HACK: serial: move pl011 initcall to device_initcall
2015-09-08 12:56 ` Mark Rutland
@ 2015-09-08 16:55 ` Leif Lindholm
0 siblings, 0 replies; 22+ messages in thread
From: Leif Lindholm @ 2015-09-08 16:55 UTC (permalink / raw)
To: Mark Rutland
Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-serial-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-acpi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
al.stone-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org,
torez-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org,
jcm-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org,
graeme.gregory-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org,
linaro-acpi-cunTk1MwBs8s++Sfvej+rw@public.gmane.org,
lv.zheng-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org
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
> >
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [RFC 0/5] console/acpi: add DBG2 and SPCR console configuration
2015-09-08 12:43 [RFC 0/5] console/acpi: add DBG2 and SPCR console configuration Leif Lindholm
` (4 preceding siblings ...)
[not found] ` <1441716217-23786-1-git-send-email-leif.lindholm-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
@ 2015-09-23 7:38 ` Ming Lei
2015-09-23 15:45 ` G Gregory
2016-01-12 15:14 ` Christopher Covington
6 siblings, 1 reply; 22+ messages in thread
From: Ming Lei @ 2015-09-23 7:38 UTC (permalink / raw)
To: Leif Lindholm
Cc: linux-arm-kernel, devicetree@vger.kernel.org, linux-serial,
Linux ACPI, Linux Kernel Mailing List, Mark Rutland, al.stone,
torez, Jon Masters, G Gregory, linaro-acpi, Lv Zheng
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).aspx
> [2] https://msdn.microsoft.com/en-us/library/windows/hardware/dn639132(v=vs.85).aspx
>
> 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,
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [RFC 0/5] console/acpi: add DBG2 and SPCR console configuration
2015-09-23 7:38 ` [RFC 0/5] console/acpi: add DBG2 and SPCR console configuration Ming Lei
@ 2015-09-23 15:45 ` G Gregory
0 siblings, 0 replies; 22+ messages in thread
From: G Gregory @ 2015-09-23 15:45 UTC (permalink / raw)
To: Ming Lei
Cc: Leif Lindholm, linux-arm-kernel, devicetree@vger.kernel.org,
linux-serial, Linux ACPI, Linux Kernel Mailing List, Mark Rutland,
al.stone, torez, Jon Masters, linaro-acpi, Lv Zheng
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).aspx
> > [2] https://msdn.microsoft.com/en-us/library/windows/hardware/dn639132(v=vs.85).aspx
> >
> > 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
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [RFC 0/5] console/acpi: add DBG2 and SPCR console configuration
2015-09-08 12:43 [RFC 0/5] console/acpi: add DBG2 and SPCR console configuration Leif Lindholm
` (5 preceding siblings ...)
2015-09-23 7:38 ` [RFC 0/5] console/acpi: add DBG2 and SPCR console configuration Ming Lei
@ 2016-01-12 15:14 ` Christopher Covington
2016-01-13 0:07 ` Al Stone
6 siblings, 1 reply; 22+ messages in thread
From: Christopher Covington @ 2016-01-12 15:14 UTC (permalink / raw)
To: Leif Lindholm, linux-arm-kernel, devicetree, linux-serial,
linux-acpi, linux-kernel, graeme.gregory
Cc: mark.rutland, al.stone, linaro-acpi, jcm, torez, lv.zheng,
Timur Tabi, linux-arm-msm@vger.kernel.org
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).aspx
> [2] https://msdn.microsoft.com/en-us/library/windows/hardware/dn639132(v=vs.85).aspx
>
> 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
--
Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [RFC 0/5] console/acpi: add DBG2 and SPCR console configuration
2016-01-12 15:14 ` Christopher Covington
@ 2016-01-13 0:07 ` Al Stone
0 siblings, 0 replies; 22+ messages in thread
From: Al Stone @ 2016-01-13 0:07 UTC (permalink / raw)
To: Christopher Covington, Leif Lindholm, linux-arm-kernel,
devicetree, linux-serial, linux-acpi, linux-kernel,
graeme.gregory
Cc: mark.rutland, linaro-acpi, jcm, torez, lv.zheng, Timur Tabi,
linux-arm-msm@vger.kernel.org
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).aspx
>> [2] https://msdn.microsoft.com/en-us/library/windows/hardware/dn639132(v=vs.85).aspx
>>
>> 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.
--
ciao,
al
-----------------------------------
Al Stone
Software Engineer
Linaro Enterprise Group
al.stone@linaro.org
-----------------------------------
^ permalink raw reply [flat|nested] 22+ messages in thread