[PATCH 6/8] ACPI: serial: implement earlycon on ACPI DBG2 port

[Aleksey Makarov]

Add ACPI_DBG2_EARLYCON_DECLARE() macros that declares
an earlycon on the serial port specified in the DBG2 ACPI table.

Pass the string "earlycon=acpi_dbg2" to the kernel to activate it.

Callbacks for EARLYCON_DECLARE() and OF_EARLYCON_DECLARE()
can also be used for this macros.

Signed-off-by: Aleksey Makarov <aleksey...@linaro.org>
---
Documentation/kernel-parameters.txt | 3 ++
drivers/tty/serial/earlycon.c | 56 +++++++++++++++++++++++++++++++++++++
include/linux/acpi_dbg2.h | 20 +++++++++++++
3 files changed, 79 insertions(+)

diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index 9a53c92..4949ebb 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -1058,6 +1058,9 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
A valid base address must be provided, and the serial
port must already be setup and configured.

+ acpi_dbg2
+ Use serial port specified by the DBG2 ACPI table.
+
earlyprintk= [X86,SH,BLACKFIN,ARM,M68k]
earlyprintk=vga
earlyprintk=efi
diff --git a/drivers/tty/serial/earlycon.c b/drivers/tty/serial/earlycon.c
index 05f9e4b..3d11248 100644
--- a/drivers/tty/serial/earlycon.c
+++ b/drivers/tty/serial/earlycon.c
@@ -21,6 +21,7 @@
#include <linux/serial_core.h>
#include <linux/sizes.h>
#include <linux/mod_devicetable.h>
+#include <linux/acpi.h>

#ifdef CONFIG_FIX_EARLYCON_MEM
#include <asm/fixmap.h>
@@ -186,6 +187,8 @@ int __init setup_earlycon(char *buf)
return -ENOENT;
}

+static bool setup_dbg2_earlycon;
+
/* early_param wrapper for setup_earlycon() */
static int __init param_setup_earlycon(char *buf)
{
@@ -198,6 +201,11 @@ static int __init param_setup_earlycon(char *buf)
if (!buf || !buf[0])
return early_init_dt_scan_chosen_serial();

+ if (!strcmp(buf, "acpi_dbg2")) {
+ setup_dbg2_earlycon = true;
+ return 0;
+ }
+
err = setup_earlycon(buf);
if (err == -ENOENT || err == -EALREADY)
return 0;
@@ -228,3 +236,51 @@ int __init of_setup_earlycon(unsigned long addr,
register_console(early_console_dev.con);
return 0;
}
+
+int __init acpi_setup_earlycon(struct acpi_dbg2_device *device, void *d)
+{
+ int err;
+ struct uart_port *port = &early_console_dev.port;
+ int (*setup)(struct earlycon_device *, const char *) = d;
+ struct acpi_generic_address *reg;
+
+ if (!setup_dbg2_earlycon)
+ return -ENODEV;
+
+ if (device->register_count < 1)
+ return -ENODEV;
+
+ if (device->base_address_offset >= device->length)
+ return -EINVAL;
+
+ reg = (void *)device + device->base_address_offset;
+
+ if (reg->space_id != ACPI_ADR_SPACE_SYSTEM_MEMORY &&
+ reg->space_id != ACPI_ADR_SPACE_SYSTEM_IO)
+ return -EINVAL;
+
+ spin_lock_init(&port->lock);
+ port->uartclk = BASE_BAUD * 16;
+
+ if (reg->space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY) {
+ if (device->port_type == ACPI_DBG2_ARM_SBSA_32BIT)
+ port->iotype = UPIO_MEM32;
+ else
+ port->iotype = UPIO_MEM;
+ port->mapbase = reg->address;
+ port->membase = earlycon_map(reg->address, SZ_4K);
+ } else {
+ port->iotype = UPIO_PORT;
+ port->iobase = reg->address;
+ }
+
+ early_console_dev.con->data = &early_console_dev;
+ err = setup(&early_console_dev, NULL);
+ if (err < 0)
+ return err;
+ if (!early_console_dev.con->write)
+ return -ENODEV;
+
+ register_console(early_console_dev.con);
+ return 0;
+}
diff --git a/include/linux/acpi_dbg2.h b/include/linux/acpi_dbg2.h
index 465afcb..9b96de2 100644
--- a/include/linux/acpi_dbg2.h
+++ b/include/linux/acpi_dbg2.h
@@ -37,12 +37,32 @@ int acpi_dbg2_setup(struct acpi_table_header *header, const void *data);
ACPI_DECLARE_PROBE_ENTRY(dbg2, name, ACPI_SIG_DBG2, \
acpi_dbg2_setup, &__acpi_dbg2_data_##name)

+int __init acpi_setup_earlycon(struct acpi_dbg2_device *device, void *d);
+
+/*
+ * ACPI_DBG2_EARLYCON_DECLARE() - Define handler for ACPI GDB2 serial port
+ * @name: Identifier to compose name of table data
+ * @subtype: Subtype of the port
+ * @console_setup: Function to be called to setup the port
+ *
+ * Type of the console_setup() callback is
+ * int (*setup)(struct earlycon_device *, const char *)
+ * It's the type of callback of of_setup_earlycon().
+ */
+#define ACPI_DBG2_EARLYCON_DECLARE(name, subtype, console_setup) \
+ ACPI_DBG2_DECLARE(name, ACPI_DBG2_SERIAL_PORT, subtype, \
+ acpi_setup_earlycon, console_setup)
+
#else

#define ACPI_DBG2_DECLARE(name, type, subtype, setup_fn, data_ptr) \
static const void *__acpi_dbg_data_##name[] \
__used __initdata = { (void *)setup_fn, (void *)data_ptr }

+#define ACPI_DBG2_EARLYCON_DECLARE(name, subtype, console_setup) \
+ static const void *__acpi_dbg_data_serial_##name[] \
+ __used __initdata = { (void *)console_setup }
+
#endif

#endif
--
2.7.1

[Aleksey Makarov]

Add a handler for ACPI DBG2 serial port of type ACPI_DBG2_ARM_PL011
that sets up an earlycon on it.

Signed-off-by: Aleksey Makarov <aleksey...@linaro.org>
---

drivers/tty/serial/amba-pl011.c | 3 +++
1 file changed, 3 insertions(+)

diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
index c0da0cc..5633741 100644
--- a/drivers/tty/serial/amba-pl011.c
+++ b/drivers/tty/serial/amba-pl011.c
@@ -59,6 +59,7 @@
#include <linux/sizes.h>
#include <linux/io.h>
#include <linux/acpi.h>
+#include <linux/acpi_dbg2.h>

#include "amba-pl011.h"

@@ -2329,6 +2330,8 @@ static int __init pl011_early_console_setup(struct earlycon_device *device,
}
EARLYCON_DECLARE(pl011, pl011_early_console_setup);
OF_EARLYCON_DECLARE(pl011, "arm,pl011", pl011_early_console_setup);
+ACPI_DBG2_EARLYCON_DECLARE(pl011, ACPI_DBG2_ARM_PL011,
+ pl011_early_console_setup);

#else
#define AMBA_CONSOLE NULL
--
2.7.1

[Aleksey Makarov]

'ARM Server Base Boot Requiremets' [1] mentions DBG2 (Microsoft Debug
Port Table 2) [2] as a mandatory ACPI table that specifies debug ports.

- Implement macros

ACPI_DBG2_DECLARE(name, type, subtype, setup_fn, data_ptr)

that defines a handler for the port of given type and subtype.

- For each declared port that is also described in the ACPI DBG2 table
call the provided callback.

[1] http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.den0044a/index.html
[2] http://go.microsoft.com/fwlink/p/?LinkId=234837

Signed-off-by: Aleksey Makarov <aleksey...@linaro.org>
---

drivers/acpi/Kconfig | 3 ++
drivers/acpi/Makefile | 1 +
drivers/acpi/dbg2.c | 85 +++++++++++++++++++++++++++++++++++++++
include/asm-generic/vmlinux.lds.h | 1 +
include/linux/acpi_dbg2.h | 48 ++++++++++++++++++++++
5 files changed, 138 insertions(+)
create mode 100644 drivers/acpi/dbg2.c
create mode 100644 include/linux/acpi_dbg2.h

diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
index 65fb483..660666e 100644
--- a/drivers/acpi/Kconfig
+++ b/drivers/acpi/Kconfig
@@ -57,6 +57,9 @@ config ACPI_SYSTEM_POWER_STATES_SUPPORT
config ACPI_CCA_REQUIRED
bool

+config ACPI_DBG2_TABLE
+ bool
+
config ACPI_DEBUGGER
bool "AML debugger interface"
select ACPI_DEBUG
diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
index 346101c..ff5e4f0 100644
--- a/drivers/acpi/Makefile
+++ b/drivers/acpi/Makefile
@@ -82,6 +82,7 @@ obj-$(CONFIG_ACPI_CUSTOM_METHOD)+= custom_method.o
obj-$(CONFIG_ACPI_BGRT) += bgrt.o
obj-$(CONFIG_ACPI_CPPC_LIB) += cppc_acpi.o
obj-$(CONFIG_ACPI_DEBUGGER_USER) += acpi_dbg.o
+obj-$(CONFIG_ACPI_DBG2_TABLE) += dbg2.o

# processor has its own "processor." module_param namespace
processor-y := processor_driver.o
diff --git a/drivers/acpi/dbg2.c b/drivers/acpi/dbg2.c
new file mode 100644
index 0000000..8a79117
--- /dev/null
+++ b/drivers/acpi/dbg2.c
@@ -0,0 +1,85 @@
+/*
+ * Copyright (c) 2012, Intel Corporation
+ * Copyright (c) 2015, 2016 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 pr_fmt(fmt) "ACPI: DBG2: " fmt
+
+#include <linux/acpi_dbg2.h>
+#include <linux/acpi.h>
+#include <linux/kernel.h>
+
+static const char * __init type2string(u16 type)
+{
+ switch (type) {
+ case ACPI_DBG2_SERIAL_PORT:
+ return "SERIAL";
+ case ACPI_DBG2_1394_PORT:
+ return "1394";
+ case ACPI_DBG2_USB_PORT:
+ return "USB";
+ case ACPI_DBG2_NET_PORT:
+ return "NET";
+ default:
+ return "?";
+ }
+}
+
+static const char * __init subtype2string(u16 subtype)
+{
+ switch (subtype) {
+ case ACPI_DBG2_16550_COMPATIBLE:
+ return "16550_COMPATIBLE";
+ case ACPI_DBG2_16550_SUBSET:
+ return "16550_SUBSET";
+ case ACPI_DBG2_ARM_PL011:
+ return "ARM_PL011";
+ case ACPI_DBG2_ARM_SBSA_32BIT:
+ return "ARM_SBSA_32BIT";
+ case ACPI_DBG2_ARM_SBSA_GENERIC:
+ return "ARM_SBSA_GENERIC";
+ case ACPI_DBG2_ARM_DCC:
+ return "ARM_DCC";
+ case ACPI_DBG2_BCM2835:
+ return "BCM2835";
+ default:
+ return "?";
+ }
+}
+
+int __init acpi_dbg2_setup(struct acpi_table_header *table, const void *data)
+{
+ struct acpi_table_dbg2 *dbg2 = (struct acpi_table_dbg2 *)table;
+ struct acpi_dbg2_data *dbg2_data = (struct acpi_dbg2_data *)data;
+ struct acpi_dbg2_device *dbg2_device, *dbg2_end;
+ int i;
+
+ dbg2_device = ACPI_ADD_PTR(struct acpi_dbg2_device, dbg2,
+ dbg2->info_offset);
+ dbg2_end = ACPI_ADD_PTR(struct acpi_dbg2_device, dbg2, table->length);
+
+ for (i = 0; i < dbg2->info_count; i++) {
+ if (dbg2_device + 1 > dbg2_end) {
+ pr_err("device pointer overflows, bad table\n");

+ return 0;
+ }
+

+ if (dbg2_device->port_type == dbg2_data->port_type &&
+ dbg2_device->port_subtype == dbg2_data->port_subtype) {
+ pr_info("debug port type: %s subtype: %s\n",
+ type2string(dbg2_device->port_type),
+ subtype2string(dbg2_device->port_subtype));
+ dbg2_data->setup(dbg2_device, dbg2_data->data);
+ }
+
+ dbg2_device = ACPI_ADD_PTR(struct acpi_dbg2_device, dbg2_device,
+ dbg2_device->length);
+ }
+
+ return 0;
+}
diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index c4bd0e2..ad9752c 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -526,6 +526,7 @@
IRQCHIP_OF_MATCH_TABLE() \
ACPI_PROBE_TABLE(irqchip) \
ACPI_PROBE_TABLE(clksrc) \
+ ACPI_PROBE_TABLE(dbg2) \
EARLYCON_TABLE() \
EARLYCON_OF_TABLES()

diff --git a/include/linux/acpi_dbg2.h b/include/linux/acpi_dbg2.h
new file mode 100644
index 0000000..465afcb
--- /dev/null
+++ b/include/linux/acpi_dbg2.h
@@ -0,0 +1,48 @@
+#ifndef _ACPI_DBG2_H_
+#define _ACPI_DBG2_H_
+
+#ifdef CONFIG_ACPI_DBG2_TABLE
+
+#include <linux/kernel.h>
+
+struct acpi_dbg2_device;
+struct acpi_table_header;
+
+struct acpi_dbg2_data {
+ u16 port_type;
+ u16 port_subtype;
+ int (*setup)(struct acpi_dbg2_device *, void *);
+ void *data;
+};
+
+int acpi_dbg2_setup(struct acpi_table_header *header, const void *data);
+
+/*
+ * ACPI_DBG2_DECLARE() - Define handler for ACPI DBG2 port

+ * @name: Identifier to compose name of table data

+ * @type: Type of the port

+ * @subtype: Subtype of the port

+ * @setup_fn: Function to be called to setup the port
+ * (of type int (*)(struct acpi_dbg2_device *, void *);)
+ * @data_ptr: Sideband data provided back to the driver
+ */
+#define ACPI_DBG2_DECLARE(name, type, subtype, setup_fn, data_ptr) \
+ static const struct acpi_dbg2_data \
+ __acpi_dbg2_data_##name __used = { \
+ .port_type = type, \
+ .port_subtype = subtype, \
+ .setup = setup_fn, \
+ .data = data_ptr, \
+ }; \
+ ACPI_DECLARE_PROBE_ENTRY(dbg2, name, ACPI_SIG_DBG2, \
+ acpi_dbg2_setup, &__acpi_dbg2_data_##name)
+
+#else
+
+#define ACPI_DBG2_DECLARE(name, type, subtype, setup_fn, data_ptr) \
+ static const void *__acpi_dbg_data_##name[] \
+ __used __initdata = { (void *)setup_fn, (void *)data_ptr }
+
+#endif
+
+#endif
--
2.7.1

[Aleksey Makarov]

From: Leif Lindholm <leif.l...@linaro.org>

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.l...@linaro.org>
---
arch/arm64/kernel/acpi.c | 54 ++++++++++++++++++++++++++----------------------
1 file changed, 29 insertions(+), 25 deletions(-)

diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c
index d1ce8e2..7a944f7 100644
--- a/arch/arm64/kernel/acpi.c
+++ b/arch/arm64/kernel/acpi.c
@@ -44,6 +44,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)
@@ -57,23 +70,27 @@ 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()
* or early_memremap() should be called here to for ACPI table mapping.
@@ -181,23 +198,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.7.1

[Aleksey Makarov]

ARM Server Base Boot Requiremets' [1] mentions DBG2 (Microsoft Debug
Port Table 2) [2] as a mandatory ACPI table that specifies debug ports.

- Move acpi/dt decision earlier in boot process on ARM64
- Move earlycon early_param handling to serial to parse earlycon option once
- Add definition of DBG2 subtypes. It's the same patch as in the SPCR series [3]
- Refactor ACPI linker tables code to enable iterating over subtables other than
subtables having acpi_subtype header (such as DBG2)

- Implement macros

ACPI_DBG2_DECLARE(name, type, subtype, setup_fn, data_ptr)

that defines a handler for the port of the given type and subtype.
- For each port defined by that macros that is also described in the ACPI DBG2
table call provided callback.
- Implement a helper macros that can be used to define early serial console
- Enable DBG2 on ARM64
- Define early console for pl011 serial

[1] http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.den0044a/index.html
[2] http://go.microsoft.com/fwlink/p/?LinkId=234837

[3] https://lkml.kernel.org/g/455559532-8305-1-git-se...@linaro.org

Aleksey Makarov (6):
ACPI: add definitions of DBG2 subtypes
ACPI: genaralize iterating over subtables in ACPI_PROBE_TABLE()
ACPI: parse DBG2 table
ACPI: serial: implement earlycon on ACPI DBG2 port
ACPI: enable ACPI_DBG2_TABLE on ARM64
serial: pl011: add ACPI DBG2 serial port

Leif Lindholm (2):
arm64: move acpi/dt decision earlier in boot process
of/serial: move earlycon early_param handling to serial

Documentation/kernel-parameters.txt | 3 +
arch/arm64/Kconfig | 1 +
arch/arm64/kernel/acpi.c | 56 ++++++++++---------
drivers/acpi/Kconfig | 3 +

drivers/acpi/Makefile | 1 +
drivers/acpi/dbg2.c | 85 ++++++++++++++++++++++++++++

drivers/acpi/scan.c | 44 ++++++++++-----
drivers/clocksource/arm_arch_timer.c | 3 +-
drivers/irqchip/irq-gic.c | 4 +-
drivers/of/fdt.c | 11 +---
drivers/tty/serial/amba-pl011.c | 3 +
drivers/tty/serial/earlycon.c | 57 +++++++++++++++++++
include/acpi/actbl2.h | 5 ++
include/asm-generic/vmlinux.lds.h | 1 +
include/linux/acpi.h | 104 ++++++++++++++++++++++++-----------
include/linux/acpi_dbg2.h | 68 +++++++++++++++++++++++
include/linux/clocksource.h | 2 +-
include/linux/irqchip.h | 5 +-
include/linux/of_fdt.h | 2 +
19 files changed, 370 insertions(+), 88 deletions(-)

create mode 100644 drivers/acpi/dbg2.c
create mode 100644 include/linux/acpi_dbg2.h

--
2.7.1

[Matthias Brugger]

So we only enable ACPI if we pass acpi=force as kernel parameter?
I'm not sure if this is what you wanted to do.

Regards,
Matthias

[Graeme Gregory]

The current preference from ARM64 maintainers was that is both ACPI
tables and a DT were presented then DT should take precedence.

With no DT provided the code should use ACPI.

Graeme

[Matthias Brugger]

If argument of parse_acpi is neither "off" nor "force" we return with
-EINVAL here. Actually parse_acpi will be only called if we pass "acpi="
as kernel parameter. Therefor we can get rid of "acpi=off" as this is
the _new_ standard. IMHO we should introduce "acpi=on" if we really want
to change the standard behavior.

>>>
>>> - 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;

Or param_acpi_off is true or param_acpi_force is true, the depth of the
DT has no influence.

>>>
>>> -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();
>>> +
>>
>> So we only enable ACPI if we pass acpi=force as kernel parameter?
>> I'm not sure if this is what you wanted to do.
>>
>
> The current preference from ARM64 maintainers was that is both ACPI
> tables and a DT were presented then DT should take precedence.
>
> With no DT provided the code should use ACPI.

From my understanding in this patch that can never happen.

On which version is this set based on?
I'm looking on v4.5-rc5 ATM.

Regards,
Matthias

[Aleksey Makarov]

Hi Matthias,

Thank you for review. The bug is fixed in the next version of the patchset.

Aleksey Makarov

> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majo...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html

[Aleksey Makarov]

From: Leif Lindholm <leif.l...@linaro.org>

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.l...@linaro.org>

Signed-off-by: Aleksey Makarov <aleksey...@linaro.org>
---

arch/arm64/kernel/acpi.c | 62 +++++++++++++++++++++++-------------------------
1 file changed, 30 insertions(+), 32 deletions(-)

diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c
index d1ce8e2..3faa323 100644
--- a/arch/arm64/kernel/acpi.c
+++ b/arch/arm64/kernel/acpi.c
@@ -41,26 +41,8 @@ EXPORT_SYMBOL(acpi_disabled);
int acpi_pci_disabled = 1; /* skip ACPI PCI scan and IRQ initialization */
EXPORT_SYMBOL(acpi_pci_disabled);

-static bool param_acpi_off __initdata;
static bool param_acpi_force __initdata;

-static int __init parse_acpi(char *arg)
-{
- if (!arg)
- return -EINVAL;
-
- /* "acpi=off" disables both ACPI table parsing and interpreter */
- if (strcmp(arg, "off") == 0)
- param_acpi_off = true;
- else if (strcmp(arg, "force") == 0) /* force ACPI to be enabled */
- param_acpi_force = true;
- else
- return -EINVAL; /* Core will print when we return error */
-

- return 0;
-}
-early_param("acpi", parse_acpi);

-

static int __init dt_scan_depth1_nodes(unsigned long node,

const char *uname, int depth,

void *data)
@@ -74,6 +56,35 @@ static int __init dt_scan_depth1_nodes(unsigned long node,
return 0;
}

+static int __init parse_acpi(char *arg)
+{
+ if (!arg)
+ return -EINVAL;
+
+ /*

+ * 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 (strcmp(arg, "off") == 0)
+ return 0;
+ else if (strcmp(arg, "force") == 0)
+ param_acpi_force = true;
+ else if (of_scan_flat_dt(dt_scan_depth1_nodes, NULL))
+ return 0;
+
+ /*

+ * ACPI is disabled at this point. Enable it in order to parse
+ * the ACPI tables and carry out sanity checks

+ */
+ enable_acpi();
+

+ return 0;
+}
+

+early_param("acpi", parse_acpi);
+
/*

* __acpi_map_table() will be called before page_init(), so early_ioremap()
* or early_memremap() should be called here to for ACPI table mapping.

@@ -181,23 +192,10 @@ out:

[Aleksey Makarov]

Add a handler for ACPI DBG2 serial port of type ACPI_DBG2_ARM_PL011
that sets up an earlycon on it.

Signed-off-by: Aleksey Makarov <aleksey...@linaro.org>
---

drivers/tty/serial/amba-pl011.c | 3 +++
1 file changed, 3 insertions(+)

diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c

index 500232a..d38da86 100644

--- a/drivers/tty/serial/amba-pl011.c
+++ b/drivers/tty/serial/amba-pl011.c
@@ -59,6 +59,7 @@
#include <linux/sizes.h>
#include <linux/io.h>
#include <linux/acpi.h>
+#include <linux/acpi_dbg2.h>

#include "amba-pl011.h"

@@ -2327,6 +2328,8 @@ static int __init pl011_early_console_setup(struct earlycon_device *device,
return 0;

[Aleksey Makarov]

Add ACPI_DBG2_EARLYCON_DECLARE() macros that declares
an earlycon on the serial port specified in the DBG2 ACPI table.

Pass the string "earlycon=acpi_dbg2" to the kernel to activate it.

Callbacks for EARLYCON_DECLARE() and OF_EARLYCON_DECLARE()
can also be used for this macros.

Signed-off-by: Aleksey Makarov <aleksey...@linaro.org>
---

Documentation/kernel-parameters.txt | 3 ++
drivers/tty/serial/earlycon.c | 60 +++++++++++++++++++++++++++++++++++++
include/linux/acpi_dbg2.h | 20 +++++++++++++
3 files changed, 83 insertions(+)

diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index 1fee970..22600e0 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -1069,6 +1069,9 @@ bytes respectively. Such letter suffixes can also be entirely omitted.

A valid base address must be provided, and the serial
port must already be setup and configured.

+ acpi_dbg2
+ Use serial port specified by the DBG2 ACPI table.
+
earlyprintk= [X86,SH,BLACKFIN,ARM,M68k]
earlyprintk=vga
earlyprintk=efi
diff --git a/drivers/tty/serial/earlycon.c b/drivers/tty/serial/earlycon.c

index d217366..9ba3a04 100644
--- a/drivers/tty/serial/earlycon.c
+++ b/drivers/tty/serial/earlycon.c
@@ -22,6 +22,7 @@
#include <linux/sizes.h>
#include <linux/of.h>
#include <linux/of_fdt.h>

+#include <linux/acpi.h>

#ifdef CONFIG_FIX_EARLYCON_MEM
#include <asm/fixmap.h>

@@ -200,6 +201,8 @@ int __init setup_earlycon(char *buf)

return -ENOENT;
}

+static bool setup_dbg2_earlycon;
+
/* early_param wrapper for setup_earlycon() */
static int __init param_setup_earlycon(char *buf)
{

@@ -212,6 +215,11 @@ static int __init param_setup_earlycon(char *buf)

if (!buf || !buf[0])
return early_init_dt_scan_chosen_serial();

+ if (!strcmp(buf, "acpi_dbg2")) {
+ setup_dbg2_earlycon = true;

+ return 0;
+ }
+

err = setup_earlycon(buf);
if (err == -ENOENT || err == -EALREADY)
return 0;

@@ -286,3 +294,55 @@ int __init of_setup_earlycon(const struct earlycon_id *match,
}

#endif /* CONFIG_OF_EARLY_FLATTREE */
+
+#ifdef CONFIG_ACPI_DBG2_TABLE

+
+int __init acpi_setup_earlycon(struct acpi_dbg2_device *device, void *d)
+{
+ int err;
+ struct uart_port *port = &early_console_dev.port;
+ int (*setup)(struct earlycon_device *, const char *) = d;
+ struct acpi_generic_address *reg;
+
+ if (!setup_dbg2_earlycon)
+ return -ENODEV;
+
+ if (device->register_count < 1)
+ return -ENODEV;
+
+ if (device->base_address_offset >= device->length)

+ return -EINVAL;
+

+ reg = (void *)device + device->base_address_offset;
+
+ if (reg->space_id != ACPI_ADR_SPACE_SYSTEM_MEMORY &&
+ reg->space_id != ACPI_ADR_SPACE_SYSTEM_IO)

+ return -EINVAL;
+

+ spin_lock_init(&port->lock);
+ port->uartclk = BASE_BAUD * 16;
+
+ if (reg->space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY) {
+ if (device->port_type == ACPI_DBG2_ARM_SBSA_32BIT)
+ port->iotype = UPIO_MEM32;
+ else
+ port->iotype = UPIO_MEM;
+ port->mapbase = reg->address;
+ port->membase = earlycon_map(reg->address, SZ_4K);
+ } else {
+ port->iotype = UPIO_PORT;
+ port->iobase = reg->address;
+ }
+
+ early_console_dev.con->data = &early_console_dev;
+ err = setup(&early_console_dev, NULL);
+ if (err < 0)
+ return err;
+ if (!early_console_dev.con->write)
+ return -ENODEV;
+
+ register_console(early_console_dev.con);

+ return 0;
+}
+

+#endif /* CONFIG_ACPI_DBG2_TABLE */
diff --git a/include/linux/acpi_dbg2.h b/include/linux/acpi_dbg2.h
index 125ae7e..b653752 100644

--- a/include/linux/acpi_dbg2.h
+++ b/include/linux/acpi_dbg2.h
@@ -37,12 +37,32 @@ int acpi_dbg2_setup(struct acpi_table_header *header, const void *data);
ACPI_DECLARE_PROBE_ENTRY(dbg2, name, ACPI_SIG_DBG2, \
acpi_dbg2_setup, &__acpi_dbg2_data_##name)

+int acpi_setup_earlycon(struct acpi_dbg2_device *device, void *d);
+
+/**
+ * ACPI_DBG2_EARLYCON_DECLARE() - Define handler for ACPI GDB2 serial port

+ * @name: Identifier to compose name of table data

+ * @subtype: Subtype of the port

+ * @console_setup: Function to be called to setup the port
+ *

+ * Type of the console_setup() callback is
+ * int (*setup)(struct earlycon_device *, const char *)
+ * It's the type of callback of of_setup_earlycon().
+ */
+#define ACPI_DBG2_EARLYCON_DECLARE(name, subtype, console_setup) \
+ ACPI_DBG2_DECLARE(name, ACPI_DBG2_SERIAL_PORT, subtype, \
+ acpi_setup_earlycon, console_setup)
+
#else

#define ACPI_DBG2_DECLARE(name, type, subtype, setup_fn, data_ptr) \
static const void *__acpi_dbg_data_##name[] \

__used __initdata = { (void *)setup_fn, (void *)data_ptr }

[Aleksey Makarov]

'ARM Server Base Boot Requiremets' [1] mentions DBG2 (Microsoft Debug

Port Table 2) [2] as a mandatory ACPI table that specifies debug ports.

- Implement macros

ACPI_DBG2_DECLARE(name, type, subtype, setup_fn, data_ptr)

that defines a handler for the port of given type and subtype.

- For each declared port that is also described in the ACPI DBG2 table
call the provided callback.

[1] http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.den0044a/index.html
[2] http://go.microsoft.com/fwlink/p/?LinkId=234837

Signed-off-by: Aleksey Makarov <aleksey...@linaro.org>
---

drivers/acpi/Kconfig | 3 ++
drivers/acpi/Makefile | 1 +

drivers/acpi/dbg2.c | 85 +++++++++++++++++++++++++++++++++++++++
include/asm-generic/vmlinux.lds.h | 1 +
include/linux/acpi_dbg2.h | 48 ++++++++++++++++++++++
5 files changed, 138 insertions(+)

create mode 100644 drivers/acpi/dbg2.c
create mode 100644 include/linux/acpi_dbg2.h

diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
index 65fb483..660666e 100644
--- a/drivers/acpi/Kconfig
+++ b/drivers/acpi/Kconfig
@@ -57,6 +57,9 @@ config ACPI_SYSTEM_POWER_STATES_SUPPORT
config ACPI_CCA_REQUIRED
bool

+config ACPI_DBG2_TABLE
+ bool
+
config ACPI_DEBUGGER
bool "AML debugger interface"
select ACPI_DEBUG
diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile

index 7395928..3b5f1ea 100644
--- a/drivers/acpi/Makefile
+++ b/drivers/acpi/Makefile
@@ -83,6 +83,7 @@ obj-$(CONFIG_ACPI_CUSTOM_METHOD)+= custom_method.o

+ return 0;
+ }
+

+ if (dbg2_device->port_type == dbg2_data->port_type &&
+ dbg2_device->port_subtype == dbg2_data->port_subtype) {
+ pr_info("debug port type: %s subtype: %s\n",
+ type2string(dbg2_device->port_type),
+ subtype2string(dbg2_device->port_subtype));
+ dbg2_data->setup(dbg2_device, dbg2_data->data);
+ }
+
+ dbg2_device = ACPI_ADD_PTR(struct acpi_dbg2_device, dbg2_device,
+ dbg2_device->length);
+ }

+
+ return 0;
+}

diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index e9a81a6..ab6a727 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -525,6 +525,7 @@

IRQCHIP_OF_MATCH_TABLE() \
ACPI_PROBE_TABLE(irqchip) \
ACPI_PROBE_TABLE(clksrc) \
+ ACPI_PROBE_TABLE(dbg2) \
EARLYCON_TABLE()

#define INIT_TEXT \

diff --git a/include/linux/acpi_dbg2.h b/include/linux/acpi_dbg2.h
new file mode 100644

index 0000000..125ae7e

--- /dev/null
+++ b/include/linux/acpi_dbg2.h
@@ -0,0 +1,48 @@
+#ifndef _ACPI_DBG2_H_
+#define _ACPI_DBG2_H_
+
+#ifdef CONFIG_ACPI_DBG2_TABLE
+
+#include <linux/kernel.h>
+
+struct acpi_dbg2_device;
+struct acpi_table_header;
+
+struct acpi_dbg2_data {
+ u16 port_type;
+ u16 port_subtype;
+ int (*setup)(struct acpi_dbg2_device *, void *);
+ void *data;
+};
+

+int acpi_dbg2_setup(struct acpi_table_header *header, const void *data);
+
+/**
+ * ACPI_DBG2_DECLARE() - Define handler for ACPI DBG2 port

+ * @name: Identifier to compose name of table data

+ * @type: Type of the port

+ * @subtype: Subtype of the port

[Aleksey Makarov]

"ARM Server Base Boot Requiremets" [1] mentions DBG2 (Microsoft Debug
Port Table 2) [2] as a mandatory ACPI table that specifies debug ports.

- Move acpi/dt decision earlier in boot process on ARM64
- Move earlycon early_param handling to serial to parse earlycon option once
- Add definition of DBG2 subtypes. It's the same patch as in the SPCR series [3]
- Refactor ACPI linker tables code to enable iterating over subtables other than
subtables having acpi_subtype header (such as DBG2)

- Implement macros

ACPI_DBG2_DECLARE(name, type, subtype, setup_fn, data_ptr)

that defines a handler for the port of the given type and subtype.
- For each port defined by that macros that is also described in the ACPI DBG2
table call provided callback.
- Implement a helper macros that can be used to define early serial console
- Enable DBG2 on ARM64
- Define early console for pl011 serial

Based on the original work by Leif Lindholm <leif.l...@linaro.org> [4]

Should be applied to next-20160224.

Tested on last QEMU with patches that introduce DBG2 and fix a QEMU bug:

https://git.linaro.org/people/aleksey.makarov/qemu.git amakarov/gdb2.03

v2:
- Fix parse_acpi() (Matthias Brugger)
- Add a reference to QEMU DBG2 patches and to original work
- Rebase to the last linux-next

v1:
https://lkml.kernel.org/g/1456148818-26257-1-git-s...@linaro.org

[1] http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.den0044a/index.html
[2] http://go.microsoft.com/fwlink/p/?LinkId=234837

[3] https://lkml.kernel.org/g/1455559532-8305-1-git-se...@linaro.org
[4] https://lkml.kernel.org/g/1441716217-23786-1-git-...@linaro.org

Aleksey Makarov (6):
ACPI: add definitions of DBG2 subtypes
ACPI: genaralize iterating over subtables in ACPI_PROBE_TABLE()
ACPI: parse DBG2 table
ACPI: serial: implement earlycon on ACPI DBG2 port
ACPI: enable ACPI_DBG2_TABLE on ARM64
serial: pl011: add ACPI DBG2 serial port

Leif Lindholm (2):
arm64: move acpi/dt decision earlier in boot process
of/serial: move earlycon early_param handling to serial

Documentation/kernel-parameters.txt | 3 +
arch/arm64/Kconfig | 1 +

arch/arm64/kernel/acpi.c | 64 ++++++++++-----------
drivers/acpi/Kconfig | 3 +
drivers/acpi/Makefile | 1 +

drivers/acpi/dbg2.c | 85 ++++++++++++++++++++++++++++
drivers/acpi/scan.c | 44 ++++++++++-----
drivers/clocksource/arm_arch_timer.c | 3 +-
drivers/irqchip/irq-gic.c | 4 +-
drivers/of/fdt.c | 11 +---
drivers/tty/serial/amba-pl011.c | 3 +

drivers/tty/serial/earlycon.c | 61 ++++++++++++++++++++

include/acpi/actbl2.h | 5 ++
include/asm-generic/vmlinux.lds.h | 1 +
include/linux/acpi.h | 104 ++++++++++++++++++++++++-----------
include/linux/acpi_dbg2.h | 68 +++++++++++++++++++++++
include/linux/clocksource.h | 2 +-
include/linux/irqchip.h | 5 +-
include/linux/of_fdt.h | 2 +

19 files changed, 375 insertions(+), 95 deletions(-)

create mode 100644 drivers/acpi/dbg2.c
create mode 100644 include/linux/acpi_dbg2.h

--
2.7.1

[Aleksey Makarov]

Refactor ACPI_PROBE_TABLE() so that it is possible to iterate
over subtables that do not have acpi_subtype header (for example
over subtables of DBG2 table)

To do so:

- Add void * data pointer argument for callback. It will allow to
parse the table and iterate over subtables calling the subtable
callbacks that are provided via this data pointer.
- Save only ACPI table id, callback and callback data in the linker
tables. This makes tables more terse and simplifies the semantics
of the fields of it's elements.
The additional data that required for subtable parsing are saved
in static namespace without increasing the size of linker tables.
- Introduce two macros with clear meaning of arguments, instead one.
The old ACPI_DECLARE_PROBE_ENTRY() macros has arguments that have
different meaning if the entry is for table or for subtable. The
arguments of the new macroses ACPI_DECLARE_PROBE_ENTRY() and
ACPI_DECLARE_PROBE_SUBTYPE_ENTRY() have unambiguous meaning.
- Document arguments of those macroses instead of the fields of linker
table data.
- Fix the issue that prevents using this macros for parsing subtables
other than MADT. In the original code, MADT-specific function
was used to iterate over subtables, while the name was common.
Introduce a new parameter to macors that specifies the size of
the table to do it in common way.
- Don't expose the internals of the tables to the drivers passing
a pointer to struct acpi_probe_entry to the subtable callback validator.
Instead, pass the sideband data specified by the driver.
- Fix the driver's callback signatures: a. Add an unused pointer to void
for table matchers (currently only clocksource callback) b. Change
the type of sideband data to kernel_ulong_t.

Signed-off-by: Aleksey Makarov <aleksey...@linaro.org>
---

drivers/acpi/scan.c | 44 ++++++++++-----
drivers/clocksource/arm_arch_timer.c | 3 +-
drivers/irqchip/irq-gic.c | 4 +-

include/linux/acpi.h | 104 ++++++++++++++++++++++++-----------

include/linux/clocksource.h | 2 +-
include/linux/irqchip.h | 5 +-

6 files changed, 109 insertions(+), 53 deletions(-)

diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index 5f28cf7..09b5f63 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -1969,16 +1969,38 @@ static struct acpi_probe_entry *ape;
static int acpi_probe_count;
static DEFINE_SPINLOCK(acpi_probe_lock);

-static int __init acpi_match_madt(struct acpi_subtable_header *header,
- const unsigned long end)
+static int __init acpi_match_and_setup(struct acpi_subtable_header *header,
+ const unsigned long end)
{
- if (!ape->subtable_valid || ape->subtable_valid(header, ape))
- if (!ape->probe_subtbl(header, end))
+ const struct acpi_probe_subtype_data *data = ape->data;
+
+ if (data->valid(header, data->data))
+ if (!data->setup(header, end))
acpi_probe_count++;

return 0;
}

+int __init acpi_probe_subtype_setup(struct acpi_table_header *table,
+ const void *data)
+{
+ const struct acpi_probe_subtype_data *data_subtype = data;
+
+ return acpi_parse_entries(ape->id, data_subtype->size,
+ acpi_match_and_setup, table,
+ data_subtype->subtype, 0);
+}
+
+static int __init acpi_probe_table_handler(struct acpi_table_header *table)
+{
+ int err = ape->setup(table, ape->data);
+
+ if (ape->setup != acpi_probe_subtype_setup && err >= 0)
+ acpi_probe_count++;
+
+ return err;
+}
+
int __init __acpi_probe_device_table(struct acpi_probe_entry *ap_head, int nr)
{
int count = 0;
@@ -1988,16 +2010,10 @@ int __init __acpi_probe_device_table(struct acpi_probe_entry *ap_head, int nr)

spin_lock(&acpi_probe_lock);
for (ape = ap_head; nr; ape++, nr--) {
- if (ACPI_COMPARE_NAME(ACPI_SIG_MADT, ape->id)) {
- acpi_probe_count = 0;
- acpi_table_parse_madt(ape->type, acpi_match_madt, 0);
- count += acpi_probe_count;
- } else {
- int res;
- res = acpi_table_parse(ape->id, ape->probe_table);
- if (!res)
- count++;
- }
+ acpi_probe_count = 0;
+ if (acpi_table_parse(ape->id, acpi_probe_table_handler) < 0)
+ continue;
+ count += acpi_probe_count;
}
spin_unlock(&acpi_probe_lock);

diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
index f492ced..c0df7cb 100644
--- a/drivers/clocksource/arm_arch_timer.c
+++ b/drivers/clocksource/arm_arch_timer.c
@@ -868,7 +868,8 @@ static int __init map_generic_timer_interrupt(u32 interrupt, u32 flags)
}

/* Initialize per-processor generic timer */
-static int __init arch_timer_acpi_init(struct acpi_table_header *table)
+static int __init arch_timer_acpi_init(struct acpi_table_header *table,
+ const void *data)
{
struct acpi_table_gtdt *gtdt;

diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
index 282344b..13d8323 100644
--- a/drivers/irqchip/irq-gic.c
+++ b/drivers/irqchip/irq-gic.c
@@ -1287,12 +1287,12 @@ static bool __init acpi_gic_redist_is_present(void)
}

static bool __init gic_validate_dist(struct acpi_subtable_header *header,
- struct acpi_probe_entry *ape)
+ kernel_ulong_t driver_data)
{
struct acpi_madt_generic_distributor *dist;
dist = (struct acpi_madt_generic_distributor *)header;

- return (dist->version == ape->driver_data &&
+ return (dist->version == driver_data &&
(dist->version != ACPI_MADT_GIC_VERSION_NONE ||
!acpi_gic_redist_is_present()));
}
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index 06ed7e5..9636c32 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -879,47 +879,76 @@ int acpi_dev_prop_read(struct acpi_device *adev, const char *propname,
struct fwnode_handle *acpi_get_next_subnode(struct device *dev,
struct fwnode_handle *subnode);

-struct acpi_probe_entry;
typedef bool (*acpi_probe_entry_validate_subtbl)(struct acpi_subtable_header *,
- struct acpi_probe_entry *);
+ kernel_ulong_t);
+
+typedef int (*acpi_probe_entry_setup)(struct acpi_table_header *, const void *);

#define ACPI_TABLE_ID_LEN 5

-/**
- * struct acpi_probe_entry - boot-time probing entry
- * @id: ACPI table name
- * @type: Optional subtable type to match
- * (if @id contains subtables)
- * @subtable_valid: Optional callback to check the validity of
- * the subtable
- * @probe_table: Callback to the driver being probed when table
- * match is successful
- * @probe_subtbl: Callback to the driver being probed when table and
- * subtable match (and optional callback is successful)
- * @driver_data: Sideband data provided back to the driver
- */
struct acpi_probe_entry {
__u8 id[ACPI_TABLE_ID_LEN];
- __u8 type;
- acpi_probe_entry_validate_subtbl subtable_valid;
- union {
- acpi_tbl_table_handler probe_table;
- acpi_tbl_entry_handler probe_subtbl;
- };
- kernel_ulong_t driver_data;
+ acpi_probe_entry_setup setup;
+ const void *data;
};

-#define ACPI_DECLARE_PROBE_ENTRY(table, name, table_id, subtable, valid, data, fn) \
+/**
+ * ACPI_DECLARE_PROBE_ENTRY() - declare boot-time probing entry
+ * @table: Name of subsystem to match

+ * @name: Identifier to compose name of table data

+ * @table_id: ACPI table name
+ * @setup_fn: Callback to the driver being probed when table
+ * matches (of type acpi_probe_entry_setup)

+ * @data_ptr: Sideband data provided back to the driver
+ */

+#define ACPI_DECLARE_PROBE_ENTRY(table, name, table_id, setup_fn, data_ptr) \
static const struct acpi_probe_entry __acpi_probe_##name \
__used __section(__##table##_acpi_probe_table) \
= { \
.id = table_id, \
- .type = subtable, \
- .subtable_valid = valid, \
- .probe_table = (acpi_tbl_table_handler)fn, \
- .driver_data = data, \

+ .setup = setup_fn, \
+ .data = data_ptr, \
}

+struct acpi_probe_subtype_data {
+ u8 subtype;
+ unsigned long size;
+ acpi_probe_entry_validate_subtbl valid;
+ acpi_tbl_entry_handler setup;
+ kernel_ulong_t data;
+};
+
+int acpi_probe_subtype_setup(struct acpi_table_header *table, const void *data);
+
+/**
+ * ACPI_DECLARE_PROBE_SUBTYPE_ENTRY() - declare boot-time probing entry that
+ * matches subtables
+ * @table: Name of subsystem to match

+ * @name: Identifier to compose name of table data

+ * @table_id: ACPI table name
+ * @table_size: Size of the table (without subtables) of type table_id
+ * @subtable: Subtable type to match
+ * @valid_fn: Callback to check the validity of the subtable
+ * (of type acpi_probe_entry_validate_subtbl)
+ * @data_int: Sideband data provided back to the driver
+ * (of type kernel_ulong_t)
+ * @setup_fn: Callback to the driver being probed when table and
+ * subtable match and valid_fn callback is successful
+ * (of type acpi_tbl_entry_handler)
+ */
+#define ACPI_DECLARE_PROBE_SUBTYPE_ENTRY(table, name, table_id, table_size, \
+ subtable, valid_fn, data_int, setup_fn) \
+ static const struct acpi_probe_subtype_data \
+ __acpi_probe_subtype_##name __used = { \
+ .subtype = subtable, \
+ .size = table_size, \
+ .valid = valid_fn, \
+ .setup = setup_fn, \
+ .data = data_int, \
+ }; \
+ ACPI_DECLARE_PROBE_ENTRY(table, name, table_id, \
+ acpi_probe_subtype_setup, &__acpi_probe_subtype_##name)
+
#define ACPI_PROBE_TABLE(name) __##name##_acpi_probe_table
#define ACPI_PROBE_TABLE_END(name) __##name##_acpi_probe_table_end

@@ -992,14 +1021,23 @@ static inline struct fwnode_handle *acpi_get_next_subnode(struct device *dev,
return NULL;
}

-#define ACPI_DECLARE_PROBE_ENTRY(table, name, table_id, subtable, validate, data, fn) \
+#define ACPI_DECLARE_PROBE_ENTRY(table, name, table_id, match, data) \
static const void * __acpi_table_##name[] \
__attribute__((unused)) \
- = { (void *) table_id, \
- (void *) subtable, \
- (void *) valid, \
- (void *) fn, \
- (void *) data }
+ = { (void *)table_id, \
+ (void *)match, \
+ (void *)data }
+
+#define ACPI_DECLARE_PROBE_SUBTYPE_ENTRY(table, name, table_id, table_size, \
+ subtable, valid_fn, data_int, setup_fn) \
+ static const void *__acpi_table_##name[] \
+ __attribute__((unused)) \
+ = { (void *)table_id, \
+ (void *)table_size, \
+ (void *)subtable, \
+ (void *)valid_fn, \
+ (void *)data_int, \
+ (void *)setup_fn }

#define acpi_probe_device_table(t) ({ int __r = 0; __r;})
#endif
diff --git a/include/linux/clocksource.h b/include/linux/clocksource.h
index 6013021..2bb5d51 100644
--- a/include/linux/clocksource.h
+++ b/include/linux/clocksource.h
@@ -259,6 +259,6 @@ static inline void clocksource_probe(void) {}
#endif

#define CLOCKSOURCE_ACPI_DECLARE(name, table_id, fn) \
- ACPI_DECLARE_PROBE_ENTRY(clksrc, name, table_id, 0, NULL, 0, fn)
+ ACPI_DECLARE_PROBE_ENTRY(clksrc, name, table_id, fn, NULL)

#endif /* _LINUX_CLOCKSOURCE_H */
diff --git a/include/linux/irqchip.h b/include/linux/irqchip.h
index 89c34b2..cc4c455 100644
--- a/include/linux/irqchip.h
+++ b/include/linux/irqchip.h
@@ -39,8 +39,9 @@
* @fn: initialization function
*/
#define IRQCHIP_ACPI_DECLARE(name, subtable, validate, data, fn) \
- ACPI_DECLARE_PROBE_ENTRY(irqchip, name, ACPI_SIG_MADT, \
- subtable, validate, data, fn)
+ ACPI_DECLARE_PROBE_SUBTYPE_ENTRY(irqchip, name, ACPI_SIG_MADT, \
+ sizeof(struct acpi_table_madt),\
+ subtable, validate, data, fn)

#ifdef CONFIG_IRQCHIP
void irqchip_init(void);
--
2.7.1

[Aleksey Makarov]

The recent version of Microsoft Debug Port Table 2 (DBG2) [1]
specifies additional serial debug port subtypes. These constants
are also referred by Serial Port Console Redirection Table (SPCR) [2]

Add these constants.

[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

Signed-off-by: Aleksey Makarov <aleksey...@linaro.org>
---

include/acpi/actbl2.h | 5 +++++
1 file changed, 5 insertions(+)

diff --git a/include/acpi/actbl2.h b/include/acpi/actbl2.h
index a4ef625..652f747 100644
--- a/include/acpi/actbl2.h
+++ b/include/acpi/actbl2.h
@@ -371,6 +371,11 @@ struct acpi_dbg2_device {

#define ACPI_DBG2_16550_COMPATIBLE 0x0000
#define ACPI_DBG2_16550_SUBSET 0x0001
+#define ACPI_DBG2_ARM_PL011 0x0003
+#define ACPI_DBG2_ARM_SBSA_32BIT 0x000D
+#define ACPI_DBG2_ARM_SBSA_GENERIC 0x000E
+#define ACPI_DBG2_ARM_DCC 0x000F
+#define ACPI_DBG2_BCM2835 0x0010

#define ACPI_DBG2_1394_STANDARD 0x0000

--
2.7.1

[Matthias Brugger]

I think we should strcmp for "on" here and return an error on other
values. IMHO an update to Documentation/kernel-parameters.txt would be
convenient.

I still wonder if we really want to change the default to ACPI disabled.
But that's a decision the maintainers have to take.

Regards,
Matthias

[Aleksey Makarov]

Hi Matthias,

Actually this patch is not correct at all.

on x86:
- if default is off then "acpi=force" enables it.
- if default is on then "acpi=off" disables it.

on ARM64 by default OF is preferred to ACPI, but ACPI is used if OF does not provide any nontrivial data.
So we need both "force" and "off"
- "acpi=force" forces ACPI,
- "acpi=off" disables ACPI

The patch makes incorrect code transformation, it changes the default behaviour.

This patch should just be dropped. The rest of the patchset does not depend on it.

> I still wonder if we really want to change the default to ACPI disabled.
> But that's a decision the maintainers have to take.

I did not realize that the patch changes default.

Thank you
Aleksey Makarov

[Aleksey Makarov]

Add ACPI_DBG2_EARLYCON_DECLARE() macros that declares
an earlycon on the serial port specified in the DBG2 ACPI table.

Pass the string "earlycon=acpi_dbg2" to the kernel to activate it.

Callbacks for EARLYCON_DECLARE() and OF_EARLYCON_DECLARE()
can also be used for this macros.

Signed-off-by: Aleksey Makarov <aleksey...@linaro.org>
---

Documentation/kernel-parameters.txt | 3 ++
drivers/tty/serial/earlycon.c | 60 +++++++++++++++++++++++++++++++++++++
include/linux/acpi_dbg2.h | 20 +++++++++++++
3 files changed, 83 insertions(+)

diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt

index e0a21e4..19b947b 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -1072,6 +1072,9 @@ bytes respectively. Such letter suffixes can also be entirely omitted.

+ return 0;
+ }
+

err = setup_earlycon(buf);
if (err == -ENOENT || err == -EALREADY)
return 0;
@@ -286,3 +294,55 @@ int __init of_setup_earlycon(const struct earlycon_id *match,
}

#endif /* CONFIG_OF_EARLY_FLATTREE */
+
+#ifdef CONFIG_ACPI_DBG2_TABLE
+
+int __init acpi_setup_earlycon(struct acpi_dbg2_device *device, void *d)
+{
+ int err;
+ struct uart_port *port = &early_console_dev.port;
+ int (*setup)(struct earlycon_device *, const char *) = d;
+ struct acpi_generic_address *reg;
+
+ if (!setup_dbg2_earlycon)
+ return -ENODEV;
+
+ if (device->register_count < 1)
+ return -ENODEV;
+
+ if (device->base_address_offset >= device->length)

+ return -EINVAL;
+

+ reg = (void *)device + device->base_address_offset;
+
+ if (reg->space_id != ACPI_ADR_SPACE_SYSTEM_MEMORY &&
+ reg->space_id != ACPI_ADR_SPACE_SYSTEM_IO)

+ return -EINVAL;
+

+ spin_lock_init(&port->lock);
+ port->uartclk = BASE_BAUD * 16;
+
+ if (reg->space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY) {
+ if (device->port_type == ACPI_DBG2_ARM_SBSA_32BIT)
+ port->iotype = UPIO_MEM32;
+ else
+ port->iotype = UPIO_MEM;
+ port->mapbase = reg->address;
+ port->membase = earlycon_map(reg->address, SZ_4K);
+ } else {
+ port->iotype = UPIO_PORT;
+ port->iobase = reg->address;
+ }
+
+ early_console_dev.con->data = &early_console_dev;
+ err = setup(&early_console_dev, NULL);
+ if (err < 0)
+ return err;
+ if (!early_console_dev.con->write)
+ return -ENODEV;
+
+ register_console(early_console_dev.con);

+ return 0;
+}
+

+#endif /* CONFIG_ACPI_DBG2_TABLE */
diff --git a/include/linux/acpi_dbg2.h b/include/linux/acpi_dbg2.h
index 125ae7e..b653752 100644
--- a/include/linux/acpi_dbg2.h
+++ b/include/linux/acpi_dbg2.h
@@ -37,12 +37,32 @@ int acpi_dbg2_setup(struct acpi_table_header *header, const void *data);
ACPI_DECLARE_PROBE_ENTRY(dbg2, name, ACPI_SIG_DBG2, \
acpi_dbg2_setup, &__acpi_dbg2_data_##name)

+int acpi_setup_earlycon(struct acpi_dbg2_device *device, void *d);
+
+/**

+ * ACPI_DBG2_EARLYCON_DECLARE() - Define handler for ACPI GDB2 serial port

+ * @name: Identifier to compose name of table data

+ * @subtype: Subtype of the port

+ * @console_setup: Function to be called to setup the port
+ *

+ * Type of the console_setup() callback is
+ * int (*setup)(struct earlycon_device *, const char *)
+ * It's the type of callback of of_setup_earlycon().
+ */
+#define ACPI_DBG2_EARLYCON_DECLARE(name, subtype, console_setup) \
+ ACPI_DBG2_DECLARE(name, ACPI_DBG2_SERIAL_PORT, subtype, \
+ acpi_setup_earlycon, console_setup)
+
#else

#define ACPI_DBG2_DECLARE(name, type, subtype, setup_fn, data_ptr) \
static const void *__acpi_dbg_data_##name[] \

__used __initdata = { (void *)setup_fn, (void *)data_ptr }

[Aleksey Makarov]

Add a handler for ACPI DBG2 serial port of type ACPI_DBG2_ARM_PL011
that sets up an earlycon on it.

Signed-off-by: Aleksey Makarov <aleksey...@linaro.org>
---

[Aleksey Makarov]

Signed-off-by: Aleksey Makarov <aleksey...@linaro.org>
---

+ * @name: Identifier to compose name of table data

+ * @name: Identifier to compose name of table data

[Aleksey Makarov]

'ARM Server Base Boot Requirements' [1] mentions DBG2 (Microsoft Debug

Port Table 2) [2] as a mandatory ACPI table that specifies debug ports.

- Implement macros

ACPI_DBG2_DECLARE(name, type, subtype, setup_fn, data_ptr)

that defines a handler for the port of given type and subtype.

- For each declared port that is also described in the ACPI DBG2 table
call the provided callback.

[1] http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.den0044a/index.html
[2] http://go.microsoft.com/fwlink/p/?LinkId=234837

Signed-off-by: Aleksey Makarov <aleksey...@linaro.org>
---

drivers/acpi/Kconfig | 3 ++
drivers/acpi/Makefile | 1 +

drivers/acpi/dbg2.c | 88 +++++++++++++++++++++++++++++++++++++++

include/asm-generic/vmlinux.lds.h | 1 +
include/linux/acpi_dbg2.h | 48 +++++++++++++++++++++

5 files changed, 141 insertions(+)

create mode 100644 drivers/acpi/dbg2.c
create mode 100644 include/linux/acpi_dbg2.h

index 0000000..0f0f6ca
--- /dev/null
+++ b/drivers/acpi/dbg2.c
@@ -0,0 +1,88 @@

+ return 0;
+ }
+

+ if (dbg2_device->port_type == dbg2_data->port_type &&
+ dbg2_device->port_subtype == dbg2_data->port_subtype) {

+ if (dbg2_device->port_type == ACPI_DBG2_SERIAL_PORT)
+ pr_info("debug port: SERIAL; subtype: %s\n",
+ subtype2string(dbg2_device->port_subtype));
+ else
+ pr_info("debug port: %s\n",
+ type2string(dbg2_device->port_type));

+ dbg2_data->setup(dbg2_device, dbg2_data->data);
+ }
+
+ dbg2_device = ACPI_ADD_PTR(struct acpi_dbg2_device, dbg2_device,
+ dbg2_device->length);
+ }

+
+ return 0;
+}

diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index 8f5a12a..8cc49ba 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -526,6 +526,7 @@

+int acpi_dbg2_setup(struct acpi_table_header *header, const void *data);
+
+/**
+ * ACPI_DBG2_DECLARE() - Define handler for ACPI DBG2 port

+ * @name: Identifier to compose name of table data

+ * @type: Type of the port

+ * @subtype: Subtype of the port

+ * @setup_fn: Function to be called to setup the port
+ * (of type int (*)(struct acpi_dbg2_device *, void *);)

+ * @data_ptr: Sideband data provided back to the driver
+ */

+#define ACPI_DBG2_DECLARE(name, type, subtype, setup_fn, data_ptr) \
+ static const struct acpi_dbg2_data \
+ __acpi_dbg2_data_##name __used = { \
+ .port_type = type, \
+ .port_subtype = subtype, \

+ .setup = setup_fn, \
+ .data = data_ptr, \

[Aleksey Makarov]

"ARM Server Base Boot Requirements" [1] mentions DBG2 (Microsoft Debug
Port Table 2) [2] as a mandatory ACPI table that specifies debug ports.

- Move earlycon early_param handling to serial to parse earlycon option once
- Add definition of DBG2 subtypes. It's the same patch as in the SPCR series [3]
- Refactor ACPI linker tables code to enable iterating over subtables other than
subtables having acpi_subtype header (such as DBG2)

- Implement macros

ACPI_DBG2_DECLARE(name, type, subtype, setup_fn, data_ptr)

that defines a handler for the port of the given type and subtype.
- For each port defined by that macros that is also described in the ACPI DBG2
table call provided callback.
- Implement a helper macros that can be used to define early serial console
- Enable DBG2 on ARM64
- Define early console for pl011 serial

Based on the work by Leif Lindholm <leif.l...@linaro.org> [4]

Should be applied to next-20160229.

Tested on last QEMU with patches that introduce DBG2 and fix a QEMU bug:

https://git.linaro.org/people/aleksey.makarov/qemu.git amakarov/gdb2.03

v3:
- Drop "arm64: move acpi/dt decision earlier in boot process". It is incorrect.
(Matthias Brugger)
- Fix the message at port discovery.

v2:
https://lkml.kernel.org/g/1456333819-13482-1-git-s...@linaro.org

- Fix parse_acpi() (Matthias Brugger)
- Add a reference to QEMU DBG2 patches and to original work
- Rebase to the last linux-next

v1:
https://lkml.kernel.org/g/1456148818-26257-1-git-s...@linaro.org

[1] http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.den0044a/index.html
[2] http://go.microsoft.com/fwlink/p/?LinkId=234837

[3] https://lkml.kernel.org/g/1455559532-8305-1-git-se...@linaro.org
[4] https://lkml.kernel.org/g/1441716217-23786-1-git-...@linaro.org

Aleksey Makarov (6):
ACPI: add definitions of DBG2 subtypes
ACPI: genaralize iterating over subtables in ACPI_PROBE_TABLE()
ACPI: parse DBG2 table
ACPI: serial: implement earlycon on ACPI DBG2 port
ACPI: enable ACPI_DBG2_TABLE on ARM64
serial: pl011: add ACPI DBG2 serial port

Leif Lindholm (1):

of/serial: move earlycon early_param handling to serial

Documentation/kernel-parameters.txt | 3 +
arch/arm64/Kconfig | 1 +

arch/arm64/kernel/acpi.c | 2 +
drivers/acpi/Kconfig | 3 +

drivers/acpi/Makefile | 1 +
drivers/acpi/dbg2.c | 88 +++++++++++++++++++++++++++++

drivers/acpi/scan.c | 44 ++++++++++-----
drivers/clocksource/arm_arch_timer.c | 3 +-
drivers/irqchip/irq-gic.c | 4 +-

drivers/of/fdt.c | 11 +---
drivers/tty/serial/amba-pl011.c | 3 +
drivers/tty/serial/earlycon.c | 61 ++++++++++++++++++++
include/acpi/actbl2.h | 5 ++
include/asm-generic/vmlinux.lds.h | 1 +
include/linux/acpi.h | 104 ++++++++++++++++++++++++-----------
include/linux/acpi_dbg2.h | 68 +++++++++++++++++++++++

include/linux/clocksource.h | 2 +-
include/linux/irqchip.h | 5 +-

include/linux/of_fdt.h | 2 +
19 files changed, 348 insertions(+), 63 deletions(-)

create mode 100644 drivers/acpi/dbg2.c
create mode 100644 include/linux/acpi_dbg2.h

--
2.7.1

[Aleksey Makarov]

From: Leif Lindholm <leif.l...@linaro.org>

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.l...@linaro.org>

Signed-off-by: Aleksey Makarov <aleksey...@linaro.org>
---

drivers/of/fdt.c | 11 +----------
drivers/tty/serial/earlycon.c | 3 ++-
include/linux/of_fdt.h | 2 ++
3 files changed, 5 insertions(+), 11 deletions(-)

diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
index 3349d2a..0547256 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -805,7 +805,7 @@ static inline void early_init_dt_check_for_initrd(unsigned long node)

#ifdef CONFIG_SERIAL_EARLYCON

-static int __init early_init_dt_scan_chosen_serial(void)
+int __init early_init_dt_scan_chosen_serial(void)
{
int offset;
const char *p, *q, *options = NULL;
@@ -849,15 +849,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 067783f..d217366 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/of.h>
@@ -209,7 +210,7 @@ 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;
+ 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 2fbe868..56b2a43 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);
@@ -91,6 +92,7 @@ extern void early_get_first_memblock_info(void *, phys_addr_t *);
extern u64 of_flat_dt_translate_address(unsigned long node);
extern void of_fdt_limit_memory(int limit);
#else /* CONFIG_OF_FLATTREE */
+static inline int early_init_dt_scan_chosen_serial(void) { return -ENODEV; }
static inline void early_init_fdt_scan_reserved_mem(void) {}
static inline void early_init_fdt_reserve_self(void) {}
static inline const char *of_flat_dt_get_machine_name(void) { return NULL; }
--
2.7.1

[Peter Hurley]

On 02/29/2016 04:42 AM, Aleksey Makarov wrote:
> 'ARM Server Base Boot Requirements' [1] mentions DBG2 (Microsoft Debug
> Port Table 2) [2] as a mandatory ACPI table that specifies debug ports.
>
> - Implement macros
>
> ACPI_DBG2_DECLARE(name, type, subtype, setup_fn, data_ptr)
>
> that defines a handler for the port of given type and subtype.
>
> - For each declared port that is also described in the ACPI DBG2 table
> call the provided callback.

On 02/22/2016 06:43 AM, Aleksey Makarov wrote:
> On 02/19/2016 08:20 PM, Christopher Covington wrote:
>> Can the device specified in DBG2 be used for both earlycon and KGDB? If it can only be used for one, let's make sure the choice of earlycon vs KGDB is intentional rather than accidental.
>
> I just sent the DBG2 series. It enables an earlycon on DBG2 port with
> an "earlycon=acpi_dbg2" option (we can discuss particular name).
> If you need KGDB on that port just support it for that port in the kernel
> (i. e. add a new instance of ACPI_DBG2_DECLARE() macros for that port, see the patches)
> and change the command line options.
> I hope that is OK. We could continue this discussion in the DBG2 thread.

This method will not work for kgdb, since kgdb doesn't actually
implement the i/o but rather runs on top of a console.

[Peter Hurley]

On 02/29/2016 04:41 AM, Aleksey Makarov wrote:
> From: Leif Lindholm <leif.l...@linaro.org>
>
> 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.

Why is this patch necessary?

Is this cleanup or required for some reason in other parts of this
series?

[Peter Hurley]

On 02/29/2016 04:42 AM, Aleksey Makarov wrote:

So this series doesn't start an ACPI earlycon at early_param time?
That doesn't seem very useful.

When does the ACPI earlycon actually start?
And don't say "when the DBG2 table is probed"; that much is obvious.

[Aleksey Makarov]

On 03/01/2016 05:49 PM, Peter Hurley wrote:
> On 02/29/2016 04:42 AM, Aleksey Makarov wrote:
>> 'ARM Server Base Boot Requirements' [1] mentions DBG2 (Microsoft Debug
>> Port Table 2) [2] as a mandatory ACPI table that specifies debug ports.
>>
>> - Implement macros
>>
>> ACPI_DBG2_DECLARE(name, type, subtype, setup_fn, data_ptr)
>>
>> that defines a handler for the port of given type and subtype.
>>
>> - For each declared port that is also described in the ACPI DBG2 table
>> call the provided callback.
>
> On 02/22/2016 06:43 AM, Aleksey Makarov wrote:
>> On 02/19/2016 08:20 PM, Christopher Covington wrote:
>>> Can the device specified in DBG2 be used for both earlycon and KGDB? If it can only be used for one, let's make sure the choice of earlycon vs KGDB is intentional rather than accidental.
>>
>> I just sent the DBG2 series. It enables an earlycon on DBG2 port with
>> an "earlycon=acpi_dbg2" option (we can discuss particular name).
>> If you need KGDB on that port just support it for that port in the kernel
>> (i. e. add a new instance of ACPI_DBG2_DECLARE() macros for that port, see the patches)
>> and change the command line options.
>> I hope that is OK. We could continue this discussion in the DBG2 thread.
>
> This method will not work for kgdb, since kgdb doesn't actually
> implement the i/o but rather runs on top of a console.

I see. Thank you for pointing this out.

I don't have requirements to implement running kgdb over the serial port
specified with DBG2. This feature should be supported separately.

Thank you
Aleksey Makarov

[Aleksey Makarov]

On 03/01/2016 05:50 PM, Peter Hurley wrote:
> On 02/29/2016 04:41 AM, Aleksey Makarov wrote:
>> From: Leif Lindholm <leif.l...@linaro.org>
>>
>> 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.
>
> Why is this patch necessary?
>
> Is this cleanup or required for some reason in other parts of this
> series?

It's a cleanup.

Also, without this, the series has to introduce a new (third) handler for "earlycon=".

[Aleksey Makarov]

ACPI earlycon starts as soon as ACPI tables become accessible (setup_arch()).
I think that is still quite early.

[Peter Hurley]

On 03/01/2016 08:31 AM, Aleksey Makarov wrote:
>
>
> On 03/01/2016 05:50 PM, Peter Hurley wrote:
>> On 02/29/2016 04:41 AM, Aleksey Makarov wrote:
>>> From: Leif Lindholm <leif.l...@linaro.org>
>>>
>>> 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.
>>
>> Why is this patch necessary?
>>
>> Is this cleanup or required for some reason in other parts of this
>> series?
>
> It's a cleanup.
>
> Also, without this, the series has to introduce a new (third) handler for "earlycon=".

Not if it were opt-in like OF is via the same blank
"earlycon" parameter.

[Peter Hurley]

On 03/01/2016 08:24 AM, Aleksey Makarov wrote:
>
>
> On 03/01/2016 05:49 PM, Peter Hurley wrote:
>> On 02/29/2016 04:42 AM, Aleksey Makarov wrote:
>>> 'ARM Server Base Boot Requirements' [1] mentions DBG2 (Microsoft Debug
>>> Port Table 2) [2] as a mandatory ACPI table that specifies debug ports.
>>>
>>> - Implement macros
>>>
>>> ACPI_DBG2_DECLARE(name, type, subtype, setup_fn, data_ptr)
>>>
>>> that defines a handler for the port of given type and subtype.
>>>
>>> - For each declared port that is also described in the ACPI DBG2 table
>>> call the provided callback.
>>
>> On 02/22/2016 06:43 AM, Aleksey Makarov wrote:
>>> On 02/19/2016 08:20 PM, Christopher Covington wrote:
>>>> Can the device specified in DBG2 be used for both earlycon and KGDB? If it can only be used for one, let's make sure the choice of earlycon vs KGDB is intentional rather than accidental.
>>>
>>> I just sent the DBG2 series. It enables an earlycon on DBG2 port with
>>> an "earlycon=acpi_dbg2" option (we can discuss particular name).
>>> If you need KGDB on that port just support it for that port in the kernel
>>> (i. e. add a new instance of ACPI_DBG2_DECLARE() macros for that port, see the patches)
>>> and change the command line options.
>>> I hope that is OK. We could continue this discussion in the DBG2 thread.
>>
>> This method will not work for kgdb, since kgdb doesn't actually
>> implement the i/o but rather runs on top of a console.
>
> I see. Thank you for pointing this out.
>
> I don't have requirements to implement running kgdb over the serial port
> specified with DBG2. This feature should be supported separately.

And this takes us back full-circle to my initial point regarding
supporting earlycon via ACPI: which is that my view is earlycon should
be opt-in for any ACPI-specified console, rather than console via
SPCR and earlycon via DBG2.

[Aleksey Makarov]

On 03/01/2016 08:24 PM, Peter Hurley wrote:
> On 03/01/2016 08:31 AM, Aleksey Makarov wrote:
>>
>>
>> On 03/01/2016 05:50 PM, Peter Hurley wrote:
>>> On 02/29/2016 04:41 AM, Aleksey Makarov wrote:
>>>> From: Leif Lindholm <leif.l...@linaro.org>
>>>>
>>>> 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.
>>>
>>> Why is this patch necessary?
>>>
>>> Is this cleanup or required for some reason in other parts of this
>>> series?
>>
>> It's a cleanup.
>>
>> Also, without this, the series has to introduce a new (third) handler for "earlycon=".
>
> Not if it were opt-in like OF is via the same blank
> "earlycon" parameter.

So you are suggesting to enable ACPI earlycon with just "earlyon" without
any arguments to this paramener, right? That's good idea as OF and ACPI
can not be used simultaneously to specify earlycon. Actually that is
how it worked in the original submussion. I will change this in the next version.

Still I think that having this parameter parsed in one place is a good idea.
Or would you prefer to drop this patch?

[Peter Hurley]

On 03/01/2016 09:52 AM, Aleksey Makarov wrote:
>
>
> On 03/01/2016 08:24 PM, Peter Hurley wrote:
>> On 03/01/2016 08:31 AM, Aleksey Makarov wrote:
>>>
>>>
>>> On 03/01/2016 05:50 PM, Peter Hurley wrote:
>>>> On 02/29/2016 04:41 AM, Aleksey Makarov wrote:
>>>>> From: Leif Lindholm <leif.l...@linaro.org>
>>>>>
>>>>> 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.
>>>>
>>>> Why is this patch necessary?
>>>>
>>>> Is this cleanup or required for some reason in other parts of this
>>>> series?
>>>
>>> It's a cleanup.
>>>
>>> Also, without this, the series has to introduce a new (third) handler for "earlycon=".
>>
>> Not if it were opt-in like OF is via the same blank
>> "earlycon" parameter.
>
> So you are suggesting to enable ACPI earlycon with just "earlyon" without
> any arguments to this paramener, right? That's good idea as OF and ACPI
> can not be used simultaneously to specify earlycon. Actually that is
> how it worked in the original submussion. I will change this in the next version.
>
> Still I think that having this parameter parsed in one place is a good idea.
> Or would you prefer to drop this patch?

No need to drop it, imo; the cleanup is fine with me.

But it really helps if either the commit message or the cover letter
describes the purpose and not just the effect (ie., the why and not just
the what).

Regards,
Peter Hurley

[Aleksey Makarov]

This is the main point on which we do not agree:
should SPCR start a new earlycon or match existing full-featured console.

But I do not see how this kgdb/DBG2 feature makes your point of view
more founded. Can you please elaborate?

On the contrary, if SPCR console is earlycon, we will not be able to
run kgdb on it.

[Peter Hurley]

My point of view is not that SPCR should *only* start an earlycon
but rather *optionally also* start an earlycon.

This is the existing behavior of every other firmware-specified console.

> But I do not see how this kgdb/DBG2 feature makes your point of view
> more founded. Can you please elaborate?

Well, my main concern is that other configurations that you have not
provided for will not be supportable at all because of the design choices
you're making here.

For example, your current design does not allow for earlycon+console
on the SPCR port and debugger on DBG2 port. I think that's a problem.

Another concern is that, since you haven't accounted for options which
we'll want to implement in the near future, that a rewrite will be
necessary to implement those.

This framework is fairly heavyweight to already not have properly
considered how to start kgdb via DBG2.

> On the contrary, if SPCR console is earlycon, we will not be able to
> run kgdb on it.

I'm not sure what you mean by "run kgdb on it". What is "it" and why
would that be a problem?

[Peter Hurley]

I see now; the probe is in patch 6/7.

setup_arch()
acpi_boot_table_init()
acpi_probe_device_table()
...
acpi_dbg2_setup()
->setup()
acpi_setup_earlycon()

console_setup is a terrible macro argument name; console_setup() is an
actual kernel function (although file-scope).
Please change it to something short and generic.

Honestly, I'd just prefer you skip all this apparatus that makes
ACPI earlycon appear to be like OF earlycon code-wise, but without any of
the real underpinning or flexibility.

This would be trivial to parse the ACPI table and invoke
setup_earlycon() with a string specifier instead.

For example,

int __init acpi_earlycon_setup(struct acpi_dbg2_device *dbg2)
{
char opts[64];
struct acpi_generic_addr *addr = (void*)dbg2 + dbg2->base_address_offset;
int mmio = addr->space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY;

if (dbg2->port_type != ACPI_DBG2_SERIAL_PORT)
return 0;

switch (dbg2->port_subtype) {
case ACPI_DBG2_ARM_PL011:
case ACPI_DBG2_ARM_SBSA_GENERIC:
case ACPI_DBG2_BCM2835:
sprintf(opts, "pl011,%s,0x%llx", mmio, addr->address);
break;
case ACPI_DBG2_ARM_SBSA_32BIT:
sprintf(opts, "pl011,mmio32,0x%llx", addr->address);
break;
case ACPI_DBG2_16550_COMPATIBLE:
case ACPI_DBG2_16550_SUBSET:
sprintf(opts, "uart,%s,0x%llx", mmio, addr->address);
break;
default:
return 0;
}

return setup_earlycon(opts);
}

This supports every earlycon ACPI DBG2 declares, not just the ARM_PL011
subtype of your series.



>>> +
>>> #endif
>>>
>>> #endif
>>>
>>

[Peter Hurley]

This should be
char *mmio = (addr->space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY) ? "mmio" : "io";

[Aleksey Makarov]

Again, we have DBG2 to specify earlycon.
SPCR specifies full-featured console. DBG2 specifies earlycon.

> This is the existing behavior of every other firmware-specified console.

It SPCR + DBG2 works exactly as every other firmware-specified console
(well, after I change "earlycon=acpi_dbg2" to just "earlycon" back)
plus it allows to specify console and earlycon separately.

>> But I do not see how this kgdb/DBG2 feature makes your point of view
>> more founded. Can you please elaborate?
>
> Well, my main concern is that other configurations that you have not
> provided for will not be supportable at all because of the design choices
> you're making here.
>
> For example, your current design does not allow for earlycon+console
> on the SPCR port and debugger on DBG2 port. I think that's a problem.

It can not run earlycon+console on the SPCR port because earlycon
is specified separately by DBG2. And that is not a problem, just specify it.

As for debugger, it's not the object of these patchets.
But I am sure it will not be hard to run it on DBG2, it's just not
what these patches do.

> Another concern is that, since you haven't accounted for options which
> we'll want to implement in the near future, that a rewrite will be
> necessary to implement those.

Correct. I can not forecast any future design decision. Nobody can.

> This framework is fairly heavyweight to already not have properly
> considered how to start kgdb via DBG2.

There is no framework. It's just a fix for ACPI tables and one simple
callback function. No design decisions were made to restrict kdbg via DBG2.

>> On the contrary, if SPCR console is earlycon, we will not be able to
>> run kgdb on it.
>
> I'm not sure what you mean by "run kgdb on it". What is "it" and why
> would that be a problem?

kgdb can not run over earlycon. If SPCR runs earlycon we can not run kgdb over it.

[Aleksey Makarov]

Is 'setup_fn' ok?

> Honestly, I'd just prefer you skip all this apparatus that makes
> ACPI earlycon appear to be like OF earlycon code-wise, but without any of
> the real underpinning or flexibility.

Actually it was Mark Salter who asked to introduce such macros.

https://lkml.kernel.org/g/1441730339....@redhat.com

I think reusing the OF functions is a good decision.

Your "but without any of the real underpinning or flexibility" is unfounded.

- Note that this decision forces setting earlycon on GDB2 debug port.
DBG2 does not specify that it should be exactly earlycon.

- You missed ACPI_DBG2_ARM_DCC. And actually I think the list of
debug ports is open. You will have to make up names like "uart" "pl011"
each time a new port is introduced into the specs.

- Most important thing, this way you disclose the internals of serial ports
to the generic earlycon.c Such info as access mode should stay
in the respective drivers.

- I would not like printing address and then parsing it back.

> This supports every earlycon ACPI DBG2 declares, not just the ARM_PL011
> subtype of your series.

To support earlycon on other types of debug port just add literally one
string of code (as in pl011).

>
>
>
>>>> +
>>>> #endif
>>>>
>>>> #endif
>>>>
>>>
>

[Peter Hurley]

1. Lack of real underpinning.

Can't start an earlycon at early_param time to debug arch issues.
As a result, everyone will continue using command line for earlycon which
makes this series useless.


2. Lack of flexibility.

OF earlycon supports any new hardware simply by string matching w/o
requiring any approvals. I can add earlycon support for anything in
10 minutes.

ACPI earlycon for any new hardware requires approvals and spec changes.
2-3 months.



Founded.

Obviously my example was simplified to point out how easy it is
to start an earlycon with a string specifier.

I assumed you would understand that

if (!setup_dbg2_earlycon)
return 0;

was omitted for clarity (or handled at a higher level).


> - You missed ACPI_DBG2_ARM_DCC.

What is this?

Your series _only_ adds ACPI_DBG2_ARM_PL011 and none of the others.
If you add ACPI_DBG2_ARM_DCC support to your series, I'll add it
to my example.

> And actually I think the list of
> debug ports is open. You will have to make up names like "uart" "pl011"
> each time a new port is introduced into the specs.

5 new ports have been added in 1 decade. I think we can keep up with
that rate of change.

And you keep going on about "make up names". _For the last time_,
these "make up names" are the documented and defined console names for
earlycons. They will *never* change, because these same string
specifiers are used on the command line.

> - Most important thing, this way you disclose the internals of serial ports
> to the generic earlycon.c Such info as access mode should stay
> in the respective drivers.

??

My example function above doesn't go in "generic earlycon.c"
You leave it in ACPI where it belongs. setup_earlycon() is already global
scope, because like I've said repeatedly, it's already used by firmware to
start earlycons.

And I don't know what you mean by "access mode".

If you're referring to ["io","mmio","mmio32"], this is how earlycon is
specified and has been since the first patch. Besides your own patch decodes
and sets the iotype (UPIO_IO, UPIO_MEM, UPIO_MEM32) so I don't get what
you're objecting to here.

And if anything, the DBG2 table is under-specified.
Such things as endianness, register stride and i/o width are missing.

Not to mention line settings like baud rate, parity, stop bits, and flow
control.

> - I would not like printing address and then parsing it back.

The "parsing it back" is already implemented: that's how command line
earlycon works. That will never change.

And this method is already used by firmware other than OF.

>> This supports every earlycon ACPI DBG2 declares, not just the ARM_PL011
>> subtype of your series.
>
> To support earlycon on other types of debug port just add literally one
> string of code (as in pl011).

And as I've already shown, so does my way.
In 1/2 as much code, without macros or all the ACPI linker table changes.

>>>>> +
>>>>> #endif
>>>>>
>>>>> #endif
>>>>>
>>>>
>>

[Peter Hurley]

On 03/04/2016 05:03 AM, Aleksey Makarov wrote:

> Actually it was Mark Salter who asked to introduce such macros.
>
> https://lkml.kernel.org/g/1441730339....@redhat.com

I wasn't copied on that series, sorry.

> I think reusing the OF functions is a good decision.

But you're not reusing the OF functions; you're duplicating them.

[Peter Hurley]

No it doesn't.

For OF, /chosen/stdout-path specifies the "full-featured console".
Adding "earlycon" to the command line starts an earlycon on the same
console, without firmware changes.

For PCDP, the firmware-specified console starts an earlycon and
registers a full-featured console for the same device.

Same for MIPS Malta.

In fact, there's not a single in-tree example of firmware that
uses one table/setting for earlycon and a different table/setting for
full-featured console.

> (well, after I change "earlycon=acpi_dbg2" to just "earlycon" back)
> plus it allows to specify console and earlycon separately.
>
>>> But I do not see how this kgdb/DBG2 feature makes your point of view
>>> more founded. Can you please elaborate?
>>
>> Well, my main concern is that other configurations that you have not
>> provided for will not be supportable at all because of the design choices
>> you're making here.
>>
>> For example, your current design does not allow for earlycon+console
>> on the SPCR port and debugger on DBG2 port. I think that's a problem.
>
> It can not run earlycon+console on the SPCR port because earlycon
> is specified separately by DBG2. And that is not a problem, just specify it.
>
> As for debugger, it's not the object of these patchets.
> But I am sure it will not be hard to run it on DBG2, it's just not
> what these patches do.

No offense, but 2 emails ago you were sure it was as trivial
as a macro invocation, until I pointed out that wouldn't work.

>> Another concern is that, since you haven't accounted for options which
>> we'll want to implement in the near future, that a rewrite will be
>> necessary to implement those.
>
> Correct. I can not forecast any future design decision. Nobody can.

Actually Chris already pointed out kgdb needs to run from DBG2.
I understand you don't want to implement that. However, it is a design
decision that has already been forecasted.

So no one's asking you to either forecast or implement it, only to
account for it in the design.

I think console + optional earlycon over the SPCR-specified device
and kgdb over the DBG2-specified device makes more sense,
but I'm also willing to accept a more flexible configuration of
options.

For example,
1: SPCR: console
2: SPCR + earlycon command line: console + earlycon
3: SPCR: console, DBG2 + earlycon command line: earlycon
4: SPCR: console, DBG2 + kgdb command line: console + kgdboc
5: DBG2 + kgdb command line: console + kgdb
6: DBG2 + earlycon + kgdb command line: earlycon + console + kgdboc

etc..

>> This framework is fairly heavyweight to already not have properly
>> considered how to start kgdb via DBG2.
>
> There is no framework. It's just a fix for ACPI tables and one simple
> callback function.

Documentation/kernel-parameters.txt | 3 +
arch/arm64/Kconfig | 1 +
arch/arm64/kernel/acpi.c | 2 +
drivers/acpi/Kconfig | 3 +

drivers/acpi/Makefile | 1 +

drivers/acpi/dbg2.c | 88 +++++++++++++++++++++++++++++
drivers/acpi/scan.c | 44 ++++++++++-----
drivers/clocksource/arm_arch_timer.c | 3 +-
drivers/irqchip/irq-gic.c | 4 +-
drivers/of/fdt.c | 11 +---
drivers/tty/serial/amba-pl011.c | 3 +
drivers/tty/serial/earlycon.c | 61 ++++++++++++++++++++
include/acpi/actbl2.h | 5 ++
include/asm-generic/vmlinux.lds.h | 1 +
include/linux/acpi.h | 104 ++++++++++++++++++++++++-----------
include/linux/acpi_dbg2.h | 68 +++++++++++++++++++++++
include/linux/clocksource.h | 2 +-
include/linux/irqchip.h | 5 +-
include/linux/of_fdt.h | 2 +
19 files changed, 348 insertions(+), 63 deletions(-)

create mode 100644 drivers/acpi/dbg2.c
create mode 100644 include/linux/acpi_dbg2.h

> No design decisions were made to restrict kdbg via DBG2.

A plausible outline for what is required for implementation on top
of what you have submitted would be sufficient; an assertion that
"it will not be hard" is not.

>>> On the contrary, if SPCR console is earlycon, we will not be able to
>>> run kgdb on it.
>>
>> I'm not sure what you mean by "run kgdb on it". What is "it" and why
>> would that be a problem?
>
> kgdb can not run over earlycon.

Right. I'm the one that told you that.

> If SPCR runs earlycon we can not run kgdb over it.

??

Right now I can run an earlycon, a console and kgdb on that console
all on the same device with OF.

Why is this a problem for ACPI?

If SPCR defines the console to start and the user also opts for earlycon
on the same device, kgdb will still be able to later start on the console.

As I explained, when a real console starts, the earlycon is disabled.