[PATCH v8 0/4] ACPI: parse the SPCR table

[PATCH v8 0/4] ACPI: parse the SPCR table

[Aleksey Makarov]

'ARM Server Base Boot Requirements' [1] mentions SPCR (Serial Port Console
Redirection Table) [2] as a mandatory ACPI table that specifies the
configuration of serial console.

Move "earlycon" early_param handling to earlycon.c to parse this option once

Parse SPCR table, setup earlycon and register specified console.

Enable parsing this table on ARM64.  Earlycon should be set up as early as
possible.  ACPI boot tables are mapped in
arch/arm64/kernel/acpi.c:acpi_boot_table_init() called from setup_arch() and
that's where we parse spcr.  So it has to be opted-in per-arch. When
ACPI_SPCR_TABLE is defined initialization of DT earlycon is deferred until the
DT/ACPI decision is done.

Implement console_match() for pl011.

Based on the work by Leif Lindholm [3]
Thanks to Peter Hurley for explaining how this should work.

Should be applied to next-20160520
Tested on QEMU and ThunderX.
SPCR support is included in QEMU's ARM64 mach-virt since 2.4 release.

v8:
- rebase to next-20160520
- remove the patch "ACPICA: Headers: Add new constants for the DBG2 ACPI table"
  as it have got to linux-next
- add Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>, Reviewed-by:
  Peter Hurley <peter@hurleysoftware.com> (but see below)
- fix the patch "serial: pl011: add console matching function".  The patch by
  Christopher Covington [4] specifies that SBSA uart does 32-bit access to
  registers and this breaks the match function.  In this series the function
  was changed to match when SPCR specifies both mmio32 and mmio access.
  I removed Acked-by: Greg from this patch because of these changes.

v7:
https://lkml.kernel.org/g/1459431629-27934-1-git-send-email-aleksey.makarov@linaro.org
- add Acked-by: Rob Herring for "of/serial: move earlycon early_param handling
  to serial"
- call DT earlycon initialization from the arch ACPI code, not from parse_spcr()
  (Rafael J. Wysocki)
- fix a few minor issues (Rafael J. Wysocki)

v6:
https://lkml.kernel.org/g/1458823925-19560-1-git-send-email-aleksey.makarov@linaro.org
- add documentation for parse_spcr() functioin (Yury Norov)
- don't initialize err variable (Yury Norov)
- add __initdata for the earlycon_init_is_deferred flag variable
- rename the function exported in "of/serial: move earlycon early_param handling
  to serial" to avoid clash with the function from arch/microblaze/kernel/prom.c
- defer initialization of DT earlycon until DT/ACPI decision is made
  (Rob Herring, Peter Hurley)
- use snprintf instead of sprintf (Andy Shevchenko)
- drop patch that adds EARLYCON_DECLARE for pl011 as EARLYCON_DECLARE is
  equivalent to OF_EARLYCON_DECLARE for 4.6+ (Peter Hurley).  This means that
  SPCR earlycon will not work on the kernels before 4.6

v5:
https://lkml.kernel.org/g/1458643595-14719-1-git-send-email-aleksey.makarov@linaro.org
- drop patch "serial: pl011: use ACPI SPCR to setup 32-bit access" because
  it is ugly. Also because Christopher Covington came with a better solution [4]
- remove error message when the table is not provided by ACPI (Andy Shevchenko)
- rewrite spcr.c following the suggestions by Peter Hurley
- add console_match() for pl011 in a separate patch
- add EARLYCON_DECLARE for pl011 in a separate patch
- add patch "of/serial: move earlycon early_param handling to serial" from
  the GDB2 series

v4:
https://lkml.kernel.org/g/1456747355-15692-1-git-send-email-aleksey.makarov@linaro.org
- drop patch "ACPI: change __init to __ref for early_acpi_os_unmap_memory()"
  ACPI developers work on a new API and asked not to do that.
  Instead, use acpi_get_table_with_size()/early_acpi_os_unmap_memory() once
  and cache the result. (Lv Zheng)
- fix some style issues (Yury Norov)

v3:
https://lkml.kernel.org/g/1455559532-8305-1-git-send-email-aleksey.makarov@linaro.org

Greg Kroah-Hartman did not like v2 so I have rewritten this patchset:

- drop acpi_match() member of struct console
- drop implementations of this member for pl011 and 8250
- drop the patch that renames some vars in printk.c as it is not needed anymore
- drop patch that introduces system wide acpi_table_parse2().
  Instead introduce a custom acpi_table_parse_spcr() in spcr.c

Instead of introducing a new match_acpi() member of struct console,
this patchset introduces a new function acpi_console_check().
This function is called when a new uart is registered at serial_core.c
the same way OF code checks for console.  If the registered uart is the
console specified by SPCR table, this function calls add_preferred_console()

The restrictions of this approach are:

- only serial consoles can be set up
- only consoles specified by the memory/io address can be set up
  (SPCR can specify devices by PCI id/PCI address)

v2:
https://lkml.kernel.org/g/1455299022-11641-1-git-send-email-aleksey.makarov@linaro.org
- don't use SPCR if user specified console in command line
- fix initialization order of newcon->index = 0
- rename some variables at printk.c (Joe Perches, Peter Hurley)
- enable ACPI_SPCR_TABLE in a separate patch (Andy Shevchenko)
- remove the retry loop for console registering (Peter Hurley).
  Instead, obtain SPCR with acpi_get_table().  That works after
  call to acpi_early_init() i. e. in any *_initcall()
- describe design decision behind introducing acpi_match() (Peter Hurley)
- fix compilation for x86 + ACPI (Graeme Gregory)
- introduce DBG2 constants in a separate patch (Andy Shevchenko)
- fix a typo in DBG2 constants (Andy Shevchenko)
- add ACPI_DBG2_ARM_SBSA_32BIT constant (Christopher Covington)
- add support for ACPI_DBG2_ARM_SBSA_* consoles (Christopher Covington)
- add documentation for functions
- add a patch that uses SPCR to find if SBSA serial driver should use 32-bit
  accessor functions (Christopher Covington)
- change __init to __ref for early_acpi_os_unmap_memory() in a separate patch
- introduce acpi_table_parse2() in a separate patch
- fix fetching the SPCR table early (Mark Salter)
- add a patch from Mark Salter that introduces support for matching 8250-based
  consoles

v1:
https://lkml.kernel.org/g/1453722324-22407-1-git-send-email-aleksey.makarov@linaro.org

[1] http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.den0044a/index.html
[2] https://msdn.microsoft.com/en-us/library/windows/hardware/dn639132(v=vs.85).aspx
[3] https://lkml.kernel.org/g/1441716217-23786-1-git-send-email-leif.lindholm@linaro.org
[4] https://lkml.kernel.org/g/1457415800-8799-1-git-send-email-cov@codeaurora.org

Aleksey Makarov (3):
  ACPI: parse SPCR and enable matching console
  ARM64: ACPI: enable ACPI_SPCR_TABLE
  serial: pl011: add console matching function

Leif Lindholm (1):
  of/serial: move earlycon early_param handling to serial

 arch/arm64/Kconfig              |   1 +
 arch/arm64/kernel/acpi.c        |  11 +++-
 drivers/acpi/Kconfig            |   3 ++
 drivers/acpi/Makefile           |   1 +
 drivers/acpi/spcr.c             | 111 ++++++++++++++++++++++++++++++++++++++++
 drivers/of/fdt.c                |  11 +---
 drivers/tty/serial/amba-pl011.c |  56 ++++++++++++++++++++
 drivers/tty/serial/earlycon.c   |  19 ++++++-
 include/linux/acpi.h            |   6 +++
 include/linux/of_fdt.h          |   2 +
 include/linux/serial_core.h     |   6 +++
 11 files changed, 214 insertions(+), 13 deletions(-)
 create mode 100644 drivers/acpi/spcr.c

-- 
2.8.2

[PATCH v8 1/4] of/serial: move earlycon early_param handling to serial

[Aleksey Makarov]

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

We have multiple "earlycon" early_param handlers - merge the DT one into
the main earlycon one.  It's a cleanup that also will be useful
to defer setting up DT console until ACPI/DT decision is made.

Rename the exported function to avoid clashing with the function from
arch/microblaze/kernel/prom.c

Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org>
Signed-off-by: Aleksey Makarov <aleksey.makarov@linaro.org>
Acked-by: Rob Herring <robh@kernel.org>
Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Reviewed-by: Peter Hurley <peter@hurleysoftware.com>
Tested-by: Kefeng Wang <wangkefeng.wang@huawei.com>
---
 drivers/of/fdt.c              | 11 +----------
 drivers/tty/serial/earlycon.c |  2 +-
 include/linux/of_fdt.h        |  2 ++
 3 files changed, 4 insertions(+), 11 deletions(-)

diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
index e7a8caa..56da2a9 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -906,7 +906,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_stdout(void)
 {
 	int offset;
 	const char *p, *q, *options = NULL;
@@ -950,15 +950,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..7aae655 100644
--- a/drivers/tty/serial/earlycon.c
+++ b/drivers/tty/serial/earlycon.c
@@ -209,7 +209,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_stdout();
 
 	err = setup_earlycon(buf);
 	if (err == -ENOENT || err == -EALREADY)
diff --git a/include/linux/of_fdt.h b/include/linux/of_fdt.h
index 26c3302..2f69d4c 100644
--- a/include/linux/of_fdt.h
+++ b/include/linux/of_fdt.h
@@ -66,6 +66,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_stdout(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);
@@ -94,6 +95,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_stdout(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.8.2

[PATCH v8 2/4] ACPI: parse SPCR and enable matching console

[Aleksey Makarov]

'ARM Server Base Boot Requiremets' [1] mentions SPCR (Serial Port
Console Redirection Table) [2] as a mandatory ACPI table that
specifies the configuration of serial console.

Defer initialization of DT earlycon until ACPI/DT decision is made.

Parse the ACPI SPCR table, setup earlycon if required,
enable specified console.

Thanks to Peter Hurley for explaining how this should work.

[1] http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.den0044a/index.html
[2] https://msdn.microsoft.com/en-us/library/windows/hardware/dn639132(v=vs.85).aspx

Signed-off-by: Aleksey Makarov <aleksey.makarov@linaro.org>
Reviewed-by: Peter Hurley <peter@hurleysoftware.com>
Tested-by: Kefeng Wang <wangkefeng.wang@huawei.com>
---
 drivers/acpi/Kconfig          |   3 ++
 drivers/acpi/Makefile         |   1 +
 drivers/acpi/spcr.c           | 111 ++++++++++++++++++++++++++++++++++++++++++
 drivers/tty/serial/earlycon.c |  19 +++++++-
 include/linux/acpi.h          |   6 +++
 include/linux/serial_core.h   |   6 +++
 6 files changed, 144 insertions(+), 2 deletions(-)
 create mode 100644 drivers/acpi/spcr.c

diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
index c204344..c8c02f1 100644
--- a/drivers/acpi/Kconfig
+++ b/drivers/acpi/Kconfig
@@ -77,6 +77,9 @@ config ACPI_DEBUGGER_USER
 
 endif
 
+config ACPI_SPCR_TABLE
+	bool
+
 config ACPI_SLEEP
 	bool
 	depends on SUSPEND || HIBERNATION
diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
index 8b08e20..d0dc940 100644
--- a/drivers/acpi/Makefile
+++ b/drivers/acpi/Makefile
@@ -82,6 +82,7 @@ obj-$(CONFIG_ACPI_EC_DEBUGFS)	+= ec_sys.o
 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_SPCR_TABLE)	+= spcr.o
 obj-$(CONFIG_ACPI_DEBUGGER_USER) += acpi_dbg.o
 
 # processor has its own "processor." module_param namespace
diff --git a/drivers/acpi/spcr.c b/drivers/acpi/spcr.c
new file mode 100644
index 0000000..f4cfc5c
--- /dev/null
+++ b/drivers/acpi/spcr.c
@@ -0,0 +1,111 @@
+/*
+ * Copyright (c) 2012, Intel Corporation
+ * Copyright (c) 2015, Red Hat, Inc.
+ * 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: SPCR: " fmt
+
+#include <linux/acpi.h>
+#include <linux/console.h>
+#include <linux/kernel.h>
+#include <linux/serial_core.h>
+
+/**
+ * parse_spcr() - parse ACPI SPCR table and add preferred console
+ *
+ * @earlycon: set up earlycon for the console specified by the table
+ *
+ * For the architectures with support for ACPI, CONFIG_ACPI_SPCR_TABLE may be
+ * defined to parse ACPI SPCR table.  As a result of the parsing preferred
+ * console is registered and if @earlycon is true, earlycon is set up.
+ *
+ * When CONFIG_ACPI_SPCR_TABLE is defined, this function should be called
+ * from arch inintialization code as soon as the DT/ACPI decision is made.
+ *
+ */
+int __init parse_spcr(bool earlycon)
+{
+	static char opts[64];
+	struct acpi_table_spcr *table;
+	acpi_size table_size;
+	acpi_status status;
+	char *uart;
+	char *iotype;
+	int baud_rate;
+	int err;
+
+	if (acpi_disabled)
+		return -ENODEV;
+
+	status = acpi_get_table_with_size(ACPI_SIG_SPCR, 0,
+					  (struct acpi_table_header **)&table,
+					  &table_size);
+
+	if (ACPI_FAILURE(status))
+		return -ENOENT;
+
+	if (table->header.revision < 2) {
+		err = -ENOENT;
+		pr_err("wrong table version\n");
+		goto done;
+	}
+
+	iotype = table->serial_port.space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY ?
+			"mmio" : "io";
+
+	switch (table->interface_type) {
+	case ACPI_DBG2_ARM_SBSA_32BIT:
+		iotype = "mmio32";
+		/* fall through */
+	case ACPI_DBG2_ARM_PL011:
+	case ACPI_DBG2_ARM_SBSA_GENERIC:
+	case ACPI_DBG2_BCM2835:
+		uart = "pl011";
+		break;
+	case ACPI_DBG2_16550_COMPATIBLE:
+	case ACPI_DBG2_16550_SUBSET:
+		uart = "uart";
+		break;
+	default:
+		err = -ENOENT;
+		goto done;
+	}
+
+	switch (table->baud_rate) {
+	case 3:
+		baud_rate = 9600;
+		break;
+	case 4:
+		baud_rate = 19200;
+		break;
+	case 6:
+		baud_rate = 57600;
+		break;
+	case 7:
+		baud_rate = 115200;
+		break;
+	default:
+		err = -ENOENT;
+		goto done;
+	}
+
+	snprintf(opts, sizeof(opts), "%s,%s,0x%llx,%d", uart, iotype,
+		 table->serial_port.address, baud_rate);
+
+	pr_info("console: %s", opts);
+
+	if (earlycon)
+		setup_earlycon(opts);
+
+	err = add_preferred_console(uart, 0, opts + strlen(uart) + 1);
+
+done:
+	early_acpi_os_unmap_memory((void __iomem *)table, table_size);
+	return err;
+}
diff --git a/drivers/tty/serial/earlycon.c b/drivers/tty/serial/earlycon.c
index 7aae655..ea00b9f 100644
--- a/drivers/tty/serial/earlycon.c
+++ b/drivers/tty/serial/earlycon.c
@@ -21,6 +21,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>
@@ -199,6 +200,14 @@ int __init setup_earlycon(char *buf)
 	return -ENOENT;
 }
 
+/*
+ * When CONFIG_ACPI_SPCR_TABLE is defined, "earlycon" without parameters in
+ * command line does not start DT earlycon immediately, instead it defers
+ * starting it until DT/ACPI decision is made.  At that time if ACPI is enabled
+ * call parse_spcr(), else call early_init_dt_scan_chosen_stdout()
+ */
+bool earlycon_init_is_deferred __initdata;
+
 /* early_param wrapper for setup_earlycon() */
 static int __init param_setup_earlycon(char *buf)
 {
@@ -208,8 +217,14 @@ static int __init param_setup_earlycon(char *buf)
 	 * Just 'earlycon' is a valid param for devicetree earlycons;
 	 * don't generate a warning from parse_early_params() in that case
 	 */
-	if (!buf || !buf[0])
-		return early_init_dt_scan_chosen_stdout();
+	if (!buf || !buf[0]) {
+		if (IS_ENABLED(CONFIG_ACPI_SPCR_TABLE)) {
+			earlycon_init_is_deferred = true;
+			return 0;
+		} else {
+			return early_init_dt_scan_chosen_stdout();
+		}
+	}
 
 	err = setup_earlycon(buf);
 	if (err == -ENOENT || err == -EALREADY)
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index 288fac5..1de43be 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -997,4 +997,10 @@ static inline struct fwnode_handle *acpi_get_next_subnode(struct device *dev,
 #define acpi_probe_device_table(t)	({ int __r = 0; __r;})
 #endif
 
+#ifdef CONFIG_ACPI_SPCR_TABLE
+int parse_spcr(bool earlycon);
+#else
+static inline int parse_spcr(bool earlycon) { return 0; }
+#endif
+
 #endif	/*_LINUX_ACPI_H*/
diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index a3d7c0d..60b6143 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -366,6 +366,12 @@ extern int of_setup_earlycon(const struct earlycon_id *match,
 			     unsigned long node,
 			     const char *options);
 
+#ifdef CONFIG_SERIAL_EARLYCON
+extern bool earlycon_init_is_deferred __initdata;
+#else
+static const bool earlycon_init_is_deferred;
+#endif
+
 struct uart_port *uart_get_console(struct uart_port *ports, int nr,
 				   struct console *c);
 int uart_parse_earlycon(char *p, unsigned char *iotype, unsigned long *addr,
-- 
2.8.2

[PATCH v8 3/4] ARM64: ACPI: enable ACPI_SPCR_TABLE

[Aleksey Makarov]

SBBR mentions SPCR as a mandatory ACPI table.  So enable it for ARM64

Earlycon should be set up as early as possible.  ACPI boot tables are
mapped in arch/arm64/kernel/acpi.c:acpi_boot_table_init() that
is called from setup_arch() and that's where we parse SPCR.
So it has to be opted-in per-arch.

When ACPI_SPCR_TABLE is defined initialization of DT earlycon is
deferred until the DT/ACPI decision is done.  Initialize DT earlycon
if ACPI is disabled.

Signed-off-by: Aleksey Makarov <aleksey.makarov@linaro.org>
Tested-by: Kefeng Wang <wangkefeng.wang@huawei.com>
---
 arch/arm64/Kconfig       |  1 +
 arch/arm64/kernel/acpi.c | 11 ++++++++++-
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 5d30abf..80c0816 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -4,6 +4,7 @@ config ARM64
 	select ACPI_GENERIC_GSI if ACPI
 	select ACPI_PCI_HOST_GENERIC if ACPI
 	select ACPI_REDUCED_HARDWARE_ONLY if ACPI
+	select ACPI_SPCR_TABLE if ACPI
 	select ARCH_HAS_DEVMEM_IS_ALLOWED
 	select ARCH_HAS_ATOMIC64_DEC_IF_POSITIVE
 	select ARCH_HAS_ELF_RANDOMIZE
diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c
index 3e4f1a4..252a6d9 100644
--- a/arch/arm64/kernel/acpi.c
+++ b/arch/arm64/kernel/acpi.c
@@ -24,6 +24,7 @@
 #include <linux/memblock.h>
 #include <linux/of_fdt.h>
 #include <linux/smp.h>
+#include <linux/serial_core.h>
 
 #include <asm/cputype.h>
 #include <asm/cpu_ops.h>
@@ -206,7 +207,7 @@ void __init acpi_boot_table_init(void)
 	if (param_acpi_off ||
 	    (!param_acpi_on && !param_acpi_force &&
 	     of_scan_flat_dt(dt_scan_depth1_nodes, NULL)))
-		return;
+		goto done;
 
 	/*
 	 * ACPI is disabled at this point. Enable it in order to parse
@@ -226,6 +227,14 @@ void __init acpi_boot_table_init(void)
 		if (!param_acpi_force)
 			disable_acpi();
 	}
+
+done:
+	if (acpi_disabled) {
+		if (earlycon_init_is_deferred)
+			early_init_dt_scan_chosen_stdout();
+	} else {
+		parse_spcr(earlycon_init_is_deferred);
+	}
 }
 
 #ifdef CONFIG_ACPI_APEI
-- 
2.8.2

[PATCH v8 4/4] serial: pl011: add console matching function

[Aleksey Makarov]

This patch adds function pl011_console_match() that implements
method match of struct console.  It allows to match consoles against
data specified in a string, for example taken from command line or
compiled by ACPI SPCR table handler.

Signed-off-by: Aleksey Makarov <aleksey.makarov@linaro.org>
Reviewed-by: Peter Hurley <peter@hurleysoftware.com>
---
 drivers/tty/serial/amba-pl011.c | 56 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 56 insertions(+)

diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
index a2aa655..388edc8 100644
--- a/drivers/tty/serial/amba-pl011.c
+++ b/drivers/tty/serial/amba-pl011.c
@@ -2288,12 +2288,68 @@ static int __init pl011_console_setup(struct console *co, char *options)
 	return uart_set_options(&uap->port, co, baud, parity, bits, flow);
 }
 
+/**
+ *	pl011_console_match - non-standard console matching
+ *	@co:	  registering console
+ *	@name:	  name from console command line
+ *	@idx:	  index from console command line
+ *	@options: ptr to option string from console command line
+ *
+ *	Only attempts to match console command lines of the form:
+ *	    console=pl011,mmio|mmio32,<addr>[,<options>]
+ *	    console=pl011,0x<addr>[,<options>]
+ *	This form is used to register an initial earlycon boot console and
+ *	replace it with the amba_console at pl011 driver init.
+ *
+ *	Performs console setup for a match (as required by interface)
+ *	If no <options> are specified, then assume the h/w is already setup.
+ *
+ *	Returns 0 if console matches; otherwise non-zero to use default matching
+ */
+static int __init pl011_console_match(struct console *co, char *name, int idx,
+				      char *options)
+{
+	char match[] = "pl011";	/* pl011-specific earlycon name */
+	unsigned char iotype;
+	unsigned long addr;
+	int i;
+
+	if (strncmp(name, match, 5) != 0)
+		return -ENODEV;
+
+	if (uart_parse_earlycon(options, &iotype, &addr, &options))
+		return -ENODEV;
+
+	/* try to match the port specified on the command line */
+	for (i = 0; i < ARRAY_SIZE(amba_ports); i++) {
+		struct uart_port *port;
+
+		if (!amba_ports[i])
+			continue;
+
+		port = &amba_ports[i]->port;
+
+		if (iotype != UPIO_MEM && iotype != UPIO_MEM32)
+			continue;
+
+		if (port->mapbase != addr)
+			continue;
+
+		co->index = i;
+		port->cons = co;
+		return pl011_console_setup(co, options);
+	}
+
+	return -ENODEV;
+}
+
 static struct uart_driver amba_reg;
 static struct console amba_console = {
 	.name		= "ttyAMA",
 	.write		= pl011_console_write,
 	.device		= uart_console_device,
 	.setup		= pl011_console_setup,
+	.match		= pl011_console_match,
 	.flags		= CON_PRINTBUFFER,
 	.index		= -1,
 	.data		= &amba_reg,
-- 
2.8.2

Re: [PATCH v8 2/4] ACPI: parse SPCR and enable matching console

[Rafael J. Wysocki]

On Fri, May 20, 2016 at 3:03 PM, Aleksey Makarov
<aleksey.makarov@linaro.org> wrote:
> 'ARM Server Base Boot Requiremets' [1] mentions SPCR (Serial Port
> Console Redirection Table) [2] as a mandatory ACPI table that
> specifies the configuration of serial console.
>
> Defer initialization of DT earlycon until ACPI/DT decision is made.
>
> Parse the ACPI SPCR table, setup earlycon if required,
> enable specified console.
>
> Thanks to Peter Hurley for explaining how this should work.
>
> [1] http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.den0044a/index.html
> [2] https://msdn.microsoft.com/en-us/library/windows/hardware/dn639132(v=vs.85).aspx
>
> Signed-off-by: Aleksey Makarov <aleksey.makarov@linaro.org>
> Reviewed-by: Peter Hurley <peter@hurleysoftware.com>
> Tested-by: Kefeng Wang <wangkefeng.wang@huawei.com>

Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

for the ACPI part.

> ---
>  drivers/acpi/Kconfig          |   3 ++
>  drivers/acpi/Makefile         |   1 +
>  drivers/acpi/spcr.c           | 111 ++++++++++++++++++++++++++++++++++++++++++
>  drivers/tty/serial/earlycon.c |  19 +++++++-
>  include/linux/acpi.h          |   6 +++
>  include/linux/serial_core.h   |   6 +++
>  6 files changed, 144 insertions(+), 2 deletions(-)
>  create mode 100644 drivers/acpi/spcr.c
>
> diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
> index c204344..c8c02f1 100644
> --- a/drivers/acpi/Kconfig
> +++ b/drivers/acpi/Kconfig
> @@ -77,6 +77,9 @@ config ACPI_DEBUGGER_USER
>
>  endif
>
> +config ACPI_SPCR_TABLE
> +       bool
> +
>  config ACPI_SLEEP
>         bool
>         depends on SUSPEND || HIBERNATION
> diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
> index 8b08e20..d0dc940 100644
> --- a/drivers/acpi/Makefile
> +++ b/drivers/acpi/Makefile
> @@ -82,6 +82,7 @@ obj-$(CONFIG_ACPI_EC_DEBUGFS) += ec_sys.o
>  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_SPCR_TABLE)  += spcr.o
>  obj-$(CONFIG_ACPI_DEBUGGER_USER) += acpi_dbg.o
>
>  # processor has its own "processor." module_param namespace
> diff --git a/drivers/acpi/spcr.c b/drivers/acpi/spcr.c
> new file mode 100644
> index 0000000..f4cfc5c
> --- /dev/null
> +++ b/drivers/acpi/spcr.c
> @@ -0,0 +1,111 @@
> +/*
> + * Copyright (c) 2012, Intel Corporation
> + * Copyright (c) 2015, Red Hat, Inc.
> + * 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: SPCR: " fmt
> +
> +#include <linux/acpi.h>
> +#include <linux/console.h>
> +#include <linux/kernel.h>
> +#include <linux/serial_core.h>
> +
> +/**
> + * parse_spcr() - parse ACPI SPCR table and add preferred console
> + *
> + * @earlycon: set up earlycon for the console specified by the table
> + *
> + * For the architectures with support for ACPI, CONFIG_ACPI_SPCR_TABLE may be
> + * defined to parse ACPI SPCR table.  As a result of the parsing preferred
> + * console is registered and if @earlycon is true, earlycon is set up.
> + *
> + * When CONFIG_ACPI_SPCR_TABLE is defined, this function should be called
> + * from arch inintialization code as soon as the DT/ACPI decision is made.
> + *
> + */
> +int __init parse_spcr(bool earlycon)
> +{
> +       static char opts[64];
> +       struct acpi_table_spcr *table;
> +       acpi_size table_size;
> +       acpi_status status;
> +       char *uart;
> +       char *iotype;
> +       int baud_rate;
> +       int err;
> +
> +       if (acpi_disabled)
> +               return -ENODEV;
> +
> +       status = acpi_get_table_with_size(ACPI_SIG_SPCR, 0,
> +                                         (struct acpi_table_header **)&table,
> +                                         &table_size);
> +
> +       if (ACPI_FAILURE(status))
> +               return -ENOENT;
> +
> +       if (table->header.revision < 2) {
> +               err = -ENOENT;
> +               pr_err("wrong table version\n");
> +               goto done;
> +       }
> +
> +       iotype = table->serial_port.space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY ?
> +                       "mmio" : "io";
> +
> +       switch (table->interface_type) {
> +       case ACPI_DBG2_ARM_SBSA_32BIT:
> +               iotype = "mmio32";
> +               /* fall through */
> +       case ACPI_DBG2_ARM_PL011:
> +       case ACPI_DBG2_ARM_SBSA_GENERIC:
> +       case ACPI_DBG2_BCM2835:
> +               uart = "pl011";
> +               break;
> +       case ACPI_DBG2_16550_COMPATIBLE:
> +       case ACPI_DBG2_16550_SUBSET:
> +               uart = "uart";
> +               break;
> +       default:
> +               err = -ENOENT;
> +               goto done;
> +       }
> +
> +       switch (table->baud_rate) {
> +       case 3:
> +               baud_rate = 9600;
> +               break;
> +       case 4:
> +               baud_rate = 19200;
> +               break;
> +       case 6:
> +               baud_rate = 57600;
> +               break;
> +       case 7:
> +               baud_rate = 115200;
> +               break;
> +       default:
> +               err = -ENOENT;
> +               goto done;
> +       }
> +
> +       snprintf(opts, sizeof(opts), "%s,%s,0x%llx,%d", uart, iotype,
> +                table->serial_port.address, baud_rate);
> +
> +       pr_info("console: %s", opts);
> +
> +       if (earlycon)
> +               setup_earlycon(opts);
> +
> +       err = add_preferred_console(uart, 0, opts + strlen(uart) + 1);
> +
> +done:
> +       early_acpi_os_unmap_memory((void __iomem *)table, table_size);
> +       return err;
> +}
> diff --git a/drivers/tty/serial/earlycon.c b/drivers/tty/serial/earlycon.c
> index 7aae655..ea00b9f 100644
> --- a/drivers/tty/serial/earlycon.c
> +++ b/drivers/tty/serial/earlycon.c
> @@ -21,6 +21,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>
> @@ -199,6 +200,14 @@ int __init setup_earlycon(char *buf)
>         return -ENOENT;
>  }
>
> +/*
> + * When CONFIG_ACPI_SPCR_TABLE is defined, "earlycon" without parameters in
> + * command line does not start DT earlycon immediately, instead it defers
> + * starting it until DT/ACPI decision is made.  At that time if ACPI is enabled
> + * call parse_spcr(), else call early_init_dt_scan_chosen_stdout()
> + */
> +bool earlycon_init_is_deferred __initdata;
> +
>  /* early_param wrapper for setup_earlycon() */
>  static int __init param_setup_earlycon(char *buf)
>  {
> @@ -208,8 +217,14 @@ static int __init param_setup_earlycon(char *buf)
>          * Just 'earlycon' is a valid param for devicetree earlycons;
>          * don't generate a warning from parse_early_params() in that case
>          */
> -       if (!buf || !buf[0])
> -               return early_init_dt_scan_chosen_stdout();
> +       if (!buf || !buf[0]) {
> +               if (IS_ENABLED(CONFIG_ACPI_SPCR_TABLE)) {
> +                       earlycon_init_is_deferred = true;
> +                       return 0;
> +               } else {
> +                       return early_init_dt_scan_chosen_stdout();
> +               }
> +       }
>
>         err = setup_earlycon(buf);
>         if (err == -ENOENT || err == -EALREADY)
> diff --git a/include/linux/acpi.h b/include/linux/acpi.h
> index 288fac5..1de43be 100644
> --- a/include/linux/acpi.h
> +++ b/include/linux/acpi.h
> @@ -997,4 +997,10 @@ static inline struct fwnode_handle *acpi_get_next_subnode(struct device *dev,
>  #define acpi_probe_device_table(t)     ({ int __r = 0; __r;})
>  #endif
>
> +#ifdef CONFIG_ACPI_SPCR_TABLE
> +int parse_spcr(bool earlycon);
> +#else
> +static inline int parse_spcr(bool earlycon) { return 0; }
> +#endif
> +
>  #endif /*_LINUX_ACPI_H*/
> diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
> index a3d7c0d..60b6143 100644
> --- a/include/linux/serial_core.h
> +++ b/include/linux/serial_core.h
> @@ -366,6 +366,12 @@ extern int of_setup_earlycon(const struct earlycon_id *match,
>                              unsigned long node,
>                              const char *options);
>
> +#ifdef CONFIG_SERIAL_EARLYCON
> +extern bool earlycon_init_is_deferred __initdata;
> +#else
> +static const bool earlycon_init_is_deferred;
> +#endif
> +
>  struct uart_port *uart_get_console(struct uart_port *ports, int nr,
>                                    struct console *c);
>  int uart_parse_earlycon(char *p, unsigned char *iotype, unsigned long *addr,
> --

Re: [PATCH v8 0/4] ACPI: parse the SPCR table

[Aleksey Makarov]

On 05/20/2016 04:03 PM, Aleksey Makarov wrote:
> 'ARM Server Base Boot Requirements' [1] mentions SPCR (Serial Port Console
> Redirection Table) [2] as a mandatory ACPI table that specifies the
> configuration of serial console.

Hi Russell,

Can you review these patches and consider ACKing the pl011 part [4/4]
please?  It had an ACK from Greg Kroah-Hartman in v7 but since then I
changed it a bit.

Who can I ask to accept the patch set for the upstream tree?

Thank you
Aleksey Makarov

> Move "earlycon" early_param handling to earlycon.c to parse this option once
> 
> Parse SPCR table, setup earlycon and register specified console.
> 
> Enable parsing this table on ARM64.  Earlycon should be set up as early as
> possible.  ACPI boot tables are mapped in
> arch/arm64/kernel/acpi.c:acpi_boot_table_init() called from setup_arch() and
> that's where we parse spcr.  So it has to be opted-in per-arch. When
> ACPI_SPCR_TABLE is defined initialization of DT earlycon is deferred until the
> DT/ACPI decision is done.
> 
> Implement console_match() for pl011.
> 
> Based on the work by Leif Lindholm [3]
> Thanks to Peter Hurley for explaining how this should work.
> 
> Should be applied to next-20160520
> Tested on QEMU and ThunderX.
> SPCR support is included in QEMU's ARM64 mach-virt since 2.4 release.
> 
> v8:
> - rebase to next-20160520
> - remove the patch "ACPICA: Headers: Add new constants for the DBG2 ACPI table"
>   as it have got to linux-next
> - add Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>, Reviewed-by:
>   Peter Hurley <peter@hurleysoftware.com> (but see below)
> - fix the patch "serial: pl011: add console matching function".  The patch by
>   Christopher Covington [4] specifies that SBSA uart does 32-bit access to
>   registers and this breaks the match function.  In this series the function
>   was changed to match when SPCR specifies both mmio32 and mmio access.
>   I removed Acked-by: Greg from this patch because of these changes.
> 
> v7:
> https://lkml.kernel.org/g/1459431629-27934-1-git-send-email-aleksey.makarov@linaro.org
> - add Acked-by: Rob Herring for "of/serial: move earlycon early_param handling
>   to serial"
> - call DT earlycon initialization from the arch ACPI code, not from parse_spcr()
>   (Rafael J. Wysocki)
> - fix a few minor issues (Rafael J. Wysocki)
> 
> v6:
> https://lkml.kernel.org/g/1458823925-19560-1-git-send-email-aleksey.makarov@linaro.org
> - add documentation for parse_spcr() functioin (Yury Norov)
> - don't initialize err variable (Yury Norov)
> - add __initdata for the earlycon_init_is_deferred flag variable
> - rename the function exported in "of/serial: move earlycon early_param handling
>   to serial" to avoid clash with the function from arch/microblaze/kernel/prom.c
> - defer initialization of DT earlycon until DT/ACPI decision is made
>   (Rob Herring, Peter Hurley)
> - use snprintf instead of sprintf (Andy Shevchenko)
> - drop patch that adds EARLYCON_DECLARE for pl011 as EARLYCON_DECLARE is
>   equivalent to OF_EARLYCON_DECLARE for 4.6+ (Peter Hurley).  This means that
>   SPCR earlycon will not work on the kernels before 4.6
> 
> v5:
> https://lkml.kernel.org/g/1458643595-14719-1-git-send-email-aleksey.makarov@linaro.org
> - drop patch "serial: pl011: use ACPI SPCR to setup 32-bit access" because
>   it is ugly. Also because Christopher Covington came with a better solution [4]
> - remove error message when the table is not provided by ACPI (Andy Shevchenko)
> - rewrite spcr.c following the suggestions by Peter Hurley
> - add console_match() for pl011 in a separate patch
> - add EARLYCON_DECLARE for pl011 in a separate patch
> - add patch "of/serial: move earlycon early_param handling to serial" from
>   the GDB2 series
> 
> v4:
> https://lkml.kernel.org/g/1456747355-15692-1-git-send-email-aleksey.makarov@linaro.org
> - drop patch "ACPI: change __init to __ref for early_acpi_os_unmap_memory()"
>   ACPI developers work on a new API and asked not to do that.
>   Instead, use acpi_get_table_with_size()/early_acpi_os_unmap_memory() once
>   and cache the result. (Lv Zheng)
> - fix some style issues (Yury Norov)
> 
> v3:
> https://lkml.kernel.org/g/1455559532-8305-1-git-send-email-aleksey.makarov@linaro.org
> 
> Greg Kroah-Hartman did not like v2 so I have rewritten this patchset:
> 
> - drop acpi_match() member of struct console
> - drop implementations of this member for pl011 and 8250
> - drop the patch that renames some vars in printk.c as it is not needed anymore
> - drop patch that introduces system wide acpi_table_parse2().
>   Instead introduce a custom acpi_table_parse_spcr() in spcr.c
> 
> Instead of introducing a new match_acpi() member of struct console,
> this patchset introduces a new function acpi_console_check().
> This function is called when a new uart is registered at serial_core.c
> the same way OF code checks for console.  If the registered uart is the
> console specified by SPCR table, this function calls add_preferred_console()
> 
> The restrictions of this approach are:
> 
> - only serial consoles can be set up
> - only consoles specified by the memory/io address can be set up
>   (SPCR can specify devices by PCI id/PCI address)
> 
> v2:
> https://lkml.kernel.org/g/1455299022-11641-1-git-send-email-aleksey.makarov@linaro.org
> - don't use SPCR if user specified console in command line
> - fix initialization order of newcon->index = 0
> - rename some variables at printk.c (Joe Perches, Peter Hurley)
> - enable ACPI_SPCR_TABLE in a separate patch (Andy Shevchenko)
> - remove the retry loop for console registering (Peter Hurley).
>   Instead, obtain SPCR with acpi_get_table().  That works after
>   call to acpi_early_init() i. e. in any *_initcall()
> - describe design decision behind introducing acpi_match() (Peter Hurley)
> - fix compilation for x86 + ACPI (Graeme Gregory)
> - introduce DBG2 constants in a separate patch (Andy Shevchenko)
> - fix a typo in DBG2 constants (Andy Shevchenko)
> - add ACPI_DBG2_ARM_SBSA_32BIT constant (Christopher Covington)
> - add support for ACPI_DBG2_ARM_SBSA_* consoles (Christopher Covington)
> - add documentation for functions
> - add a patch that uses SPCR to find if SBSA serial driver should use 32-bit
>   accessor functions (Christopher Covington)
> - change __init to __ref for early_acpi_os_unmap_memory() in a separate patch
> - introduce acpi_table_parse2() in a separate patch
> - fix fetching the SPCR table early (Mark Salter)
> - add a patch from Mark Salter that introduces support for matching 8250-based
>   consoles
> 
> v1:
> https://lkml.kernel.org/g/1453722324-22407-1-git-send-email-aleksey.makarov@linaro.org
> 
> [1] http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.den0044a/index.html
> [2] https://msdn.microsoft.com/en-us/library/windows/hardware/dn639132(v=vs.85).aspx
> [3] https://lkml.kernel.org/g/1441716217-23786-1-git-send-email-leif.lindholm@linaro.org
> [4] https://lkml.kernel.org/g/1457415800-8799-1-git-send-email-cov@codeaurora.org
> 
> Aleksey Makarov (3):
>   ACPI: parse SPCR and enable matching console
>   ARM64: ACPI: enable ACPI_SPCR_TABLE
>   serial: pl011: add console matching function
> 
> Leif Lindholm (1):
>   of/serial: move earlycon early_param handling to serial
> 
>  arch/arm64/Kconfig              |   1 +
>  arch/arm64/kernel/acpi.c        |  11 +++-
>  drivers/acpi/Kconfig            |   3 ++
>  drivers/acpi/Makefile           |   1 +
>  drivers/acpi/spcr.c             | 111 ++++++++++++++++++++++++++++++++++++++++
>  drivers/of/fdt.c                |  11 +---
>  drivers/tty/serial/amba-pl011.c |  56 ++++++++++++++++++++
>  drivers/tty/serial/earlycon.c   |  19 ++++++-
>  include/linux/acpi.h            |   6 +++
>  include/linux/of_fdt.h          |   2 +
>  include/linux/serial_core.h     |   6 +++
>  11 files changed, 214 insertions(+), 13 deletions(-)
>  create mode 100644 drivers/acpi/spcr.c
> 

Re: [PATCH v8 4/4] serial: pl011: add console matching function

[Aleksey Makarov]

On 05/20/2016 04:03 PM, Aleksey Makarov wrote:
> This patch adds function pl011_console_match() that implements
> method match of struct console.  It allows to match consoles against
> data specified in a string, for example taken from command line or
> compiled by ACPI SPCR table handler.
> 
> Signed-off-by: Aleksey Makarov <aleksey.makarov@linaro.org>
> Reviewed-by: Peter Hurley <peter@hurleysoftware.com>

Hi Greg, Russell,

Can you review this patch and consider ACKing it please?
It had an ACK from Greg in v7 but since then I changed it a bit.

The patch by Christopher Covington [1] specifies that SBSA uart 
does 32-bit access to registers and this breaks the match function.  
In this series the function was changed to match when SPCR specifies
both mmio32 and mmio access.  I removed Acked-by: Greg from this
patch because of these changes.

The difference between v7 and v8:

diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
index 4139b64..388edc8 100644
--- a/drivers/tty/serial/amba-pl011.c
+++ b/drivers/tty/serial/amba-pl011.c
@@ -2328,10 +2329,10 @@ static int __init pl011_console_match(struct console *co, char *name, int idx,
 
                port = &amba_ports[i]->port;
 
-               if (port->iotype != iotype)
+               if (iotype != UPIO_MEM && iotype != UPIO_MEM32)
                        continue;
-               if ((iotype == UPIO_MEM || iotype == UPIO_MEM32) &&
-                   (port->mapbase != addr))
+
+               if (port->mapbase != addr)
                        continue;
 
                co->index = i;

[1] https://lkml.kernel.org/g/1457415800-8799-1-git-send-email-cov@codeaurora.org

Thank you
Aleksey Makarov

> ---
>  drivers/tty/serial/amba-pl011.c | 56 +++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 56 insertions(+)
> 
> diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
> index a2aa655..388edc8 100644
> --- a/drivers/tty/serial/amba-pl011.c
> +++ b/drivers/tty/serial/amba-pl011.c
> @@ -2288,12 +2288,68 @@ static int __init pl011_console_setup(struct console *co, char *options)
>  	return uart_set_options(&uap->port, co, baud, parity, bits, flow);
>  }
>  
> +/**
> + *	pl011_console_match - non-standard console matching
> + *	@co:	  registering console
> + *	@name:	  name from console command line
> + *	@idx:	  index from console command line
> + *	@options: ptr to option string from console command line
> + *
> + *	Only attempts to match console command lines of the form:
> + *	    console=pl011,mmio|mmio32,<addr>[,<options>]
> + *	    console=pl011,0x<addr>[,<options>]
> + *	This form is used to register an initial earlycon boot console and
> + *	replace it with the amba_console at pl011 driver init.
> + *
> + *	Performs console setup for a match (as required by interface)
> + *	If no <options> are specified, then assume the h/w is already setup.
> + *
> + *	Returns 0 if console matches; otherwise non-zero to use default matching
> + */
> +static int __init pl011_console_match(struct console *co, char *name, int idx,
> +				      char *options)
> +{
> +	char match[] = "pl011";	/* pl011-specific earlycon name */
> +	unsigned char iotype;
> +	unsigned long addr;
> +	int i;
> +
> +	if (strncmp(name, match, 5) != 0)
> +		return -ENODEV;
> +
> +	if (uart_parse_earlycon(options, &iotype, &addr, &options))
> +		return -ENODEV;
> +
> +	/* try to match the port specified on the command line */
> +	for (i = 0; i < ARRAY_SIZE(amba_ports); i++) {
> +		struct uart_port *port;
> +
> +		if (!amba_ports[i])
> +			continue;
> +
> +		port = &amba_ports[i]->port;
> +
> +		if (iotype != UPIO_MEM && iotype != UPIO_MEM32)
> +			continue;
> +
> +		if (port->mapbase != addr)
> +			continue;
> +
> +		co->index = i;
> +		port->cons = co;
> +		return pl011_console_setup(co, options);
> +	}
> +
> +	return -ENODEV;
> +}
> +
>  static struct uart_driver amba_reg;
>  static struct console amba_console = {
>  	.name		= "ttyAMA",
>  	.write		= pl011_console_write,
>  	.device		= uart_console_device,
>  	.setup		= pl011_console_setup,
> +	.match		= pl011_console_match,
>  	.flags		= CON_PRINTBUFFER,
>  	.index		= -1,
>  	.data		= &amba_reg,
> 

Re: [PATCH v8 0/4] ACPI: parse the SPCR table

[Timur Tabi]

On Fri, May 20, 2016 at 8:03 AM, Aleksey Makarov
<aleksey.makarov@linaro.org> wrote:
> 'ARM Server Base Boot Requirements' [1] mentions SPCR (Serial Port Console
> Redirection Table) [2] as a mandatory ACPI table that specifies the
> configuration of serial console.
>
> Move "earlycon" early_param handling to earlycon.c to parse this option once
>
> Parse SPCR table, setup earlycon and register specified console.
>
> Enable parsing this table on ARM64.  Earlycon should be set up as early as
> possible.  ACPI boot tables are mapped in
> arch/arm64/kernel/acpi.c:acpi_boot_table_init() called from setup_arch() and
> that's where we parse spcr.  So it has to be opted-in per-arch. When
> ACPI_SPCR_TABLE is defined initialization of DT earlycon is deferred until the
> DT/ACPI decision is done.
>
> Implement console_match() for pl011.
>
> Based on the work by Leif Lindholm [3]
> Thanks to Peter Hurley for explaining how this should work.
>
> Should be applied to next-20160520
> Tested on QEMU and ThunderX.
> SPCR support is included in QEMU's ARM64 mach-virt since 2.4 release.
>
> v8:
> - rebase to next-20160520
> - remove the patch "ACPICA: Headers: Add new constants for the DBG2 ACPI table"
>   as it have got to linux-next
> - add Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>, Reviewed-by:
>   Peter Hurley <peter@hurleysoftware.com> (but see below)
> - fix the patch "serial: pl011: add console matching function".  The patch by
>   Christopher Covington [4] specifies that SBSA uart does 32-bit access to
>   registers and this breaks the match function.  In this series the function
>   was changed to match when SPCR specifies both mmio32 and mmio access.
>   I removed Acked-by: Greg from this patch because of these changes.

All four patches:

Tested-by: Timur Tabi <timur@codeaurora.org>

-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project.

Re: [PATCH v8 4/4] serial: pl011: add console matching function

[Aleksey Makarov]

On 06/02/2016 09:02 PM, Aleksey Makarov wrote:
> 
> On 05/20/2016 04:03 PM, Aleksey Makarov wrote:

Hi Russell,

Can you review this patch please?

Thank you
Aleksey Makarov

>> This patch adds function pl011_console_match() that implements
>> method match of struct console.  It allows to match consoles against
>> data specified in a string, for example taken from command line or
>> compiled by ACPI SPCR table handler.
>>
>> Signed-off-by: Aleksey Makarov <aleksey.makarov@linaro.org>
>> Reviewed-by: Peter Hurley <peter@hurleysoftware.com>
> 
> Hi Greg, Russell,
> 
> Can you review this patch and consider ACKing it please?
> It had an ACK from Greg in v7 but since then I changed it a bit.
> 
> The patch by Christopher Covington [1] specifies that SBSA uart 
> does 32-bit access to registers and this breaks the match function.  
> In this series the function was changed to match when SPCR specifies
> both mmio32 and mmio access.  I removed Acked-by: Greg from this
> patch because of these changes.
> 
> The difference between v7 and v8:
> 
> diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
> index 4139b64..388edc8 100644
> --- a/drivers/tty/serial/amba-pl011.c
> +++ b/drivers/tty/serial/amba-pl011.c
> @@ -2328,10 +2329,10 @@ static int __init pl011_console_match(struct console *co, char *name, int idx,
>  
>                 port = &amba_ports[i]->port;
>  
> -               if (port->iotype != iotype)
> +               if (iotype != UPIO_MEM && iotype != UPIO_MEM32)
>                         continue;
> -               if ((iotype == UPIO_MEM || iotype == UPIO_MEM32) &&
> -                   (port->mapbase != addr))
> +
> +               if (port->mapbase != addr)
>                         continue;
>  
>                 co->index = i;
> 
> [1] https://lkml.kernel.org/g/1457415800-8799-1-git-send-email-cov@codeaurora.org
> 
> Thank you
> Aleksey Makarov
> 
>> ---
>>  drivers/tty/serial/amba-pl011.c | 56 +++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 56 insertions(+)
>>
>> diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
>> index a2aa655..388edc8 100644
>> --- a/drivers/tty/serial/amba-pl011.c
>> +++ b/drivers/tty/serial/amba-pl011.c
>> @@ -2288,12 +2288,68 @@ static int __init pl011_console_setup(struct console *co, char *options)
>>  	return uart_set_options(&uap->port, co, baud, parity, bits, flow);
>>  }
>>  
>> +/**
>> + *	pl011_console_match - non-standard console matching
>> + *	@co:	  registering console
>> + *	@name:	  name from console command line
>> + *	@idx:	  index from console command line
>> + *	@options: ptr to option string from console command line
>> + *
>> + *	Only attempts to match console command lines of the form:
>> + *	    console=pl011,mmio|mmio32,<addr>[,<options>]
>> + *	    console=pl011,0x<addr>[,<options>]
>> + *	This form is used to register an initial earlycon boot console and
>> + *	replace it with the amba_console at pl011 driver init.
>> + *
>> + *	Performs console setup for a match (as required by interface)
>> + *	If no <options> are specified, then assume the h/w is already setup.
>> + *
>> + *	Returns 0 if console matches; otherwise non-zero to use default matching
>> + */
>> +static int __init pl011_console_match(struct console *co, char *name, int idx,
>> +				      char *options)
>> +{
>> +	char match[] = "pl011";	/* pl011-specific earlycon name */
>> +	unsigned char iotype;
>> +	unsigned long addr;
>> +	int i;
>> +
>> +	if (strncmp(name, match, 5) != 0)
>> +		return -ENODEV;
>> +
>> +	if (uart_parse_earlycon(options, &iotype, &addr, &options))
>> +		return -ENODEV;
>> +
>> +	/* try to match the port specified on the command line */
>> +	for (i = 0; i < ARRAY_SIZE(amba_ports); i++) {
>> +		struct uart_port *port;
>> +
>> +		if (!amba_ports[i])
>> +			continue;
>> +
>> +		port = &amba_ports[i]->port;
>> +
>> +		if (iotype != UPIO_MEM && iotype != UPIO_MEM32)
>> +			continue;
>> +
>> +		if (port->mapbase != addr)
>> +			continue;
>> +
>> +		co->index = i;
>> +		port->cons = co;
>> +		return pl011_console_setup(co, options);
>> +	}
>> +
>> +	return -ENODEV;
>> +}
>> +
>>  static struct uart_driver amba_reg;
>>  static struct console amba_console = {
>>  	.name		= "ttyAMA",
>>  	.write		= pl011_console_write,
>>  	.device		= uart_console_device,
>>  	.setup		= pl011_console_setup,
>> +	.match		= pl011_console_match,
>>  	.flags		= CON_PRINTBUFFER,
>>  	.index		= -1,
>>  	.data		= &amba_reg,
>>

Re: [PATCH v8 4/4] serial: pl011: add console matching function

[Russell King - ARM Linux]

On Mon, Jun 20, 2016 at 03:26:39PM +0300, Aleksey Makarov wrote:
> 
> On 06/02/2016 09:02 PM, Aleksey Makarov wrote:
> > 
> > On 05/20/2016 04:03 PM, Aleksey Makarov wrote:
> 
> Hi Russell,
> 
> Can you review this patch please?
> 
> Thank you
> Aleksey Makarov
> 
> >> This patch adds function pl011_console_match() that implements
> >> method match of struct console.  It allows to match consoles against
> >> data specified in a string, for example taken from command line or
> >> compiled by ACPI SPCR table handler.
> >>
> >> Signed-off-by: Aleksey Makarov <aleksey.makarov@linaro.org>
> >> Reviewed-by: Peter Hurley <peter@hurleysoftware.com>
> > 
> > Hi Greg, Russell,
> > 
> > Can you review this patch and consider ACKing it please?
> > It had an ACK from Greg in v7 but since then I changed it a bit.
> > 
> > The patch by Christopher Covington [1] specifies that SBSA uart 
> > does 32-bit access to registers and this breaks the match function.  
> > In this series the function was changed to match when SPCR specifies
> > both mmio32 and mmio access.  I removed Acked-by: Greg from this
> > patch because of these changes.
> > 
> > The difference between v7 and v8:
> > 
> > diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
> > index 4139b64..388edc8 100644
> > --- a/drivers/tty/serial/amba-pl011.c
> > +++ b/drivers/tty/serial/amba-pl011.c
> > @@ -2328,10 +2329,10 @@ static int __init pl011_console_match(struct console *co, char *name, int idx,
> >  
> >                 port = &amba_ports[i]->port;
> >  
> > -               if (port->iotype != iotype)
> > +               if (iotype != UPIO_MEM && iotype != UPIO_MEM32)
> >                         continue;
> > -               if ((iotype == UPIO_MEM || iotype == UPIO_MEM32) &&
> > -                   (port->mapbase != addr))
> > +
> > +               if (port->mapbase != addr)
> >                         continue;
> >  
> >                 co->index = i;

I'm looking at this change, and I don't know what effect it ultimately
has.  This is kind of a problem, because I've zero knowledge of ACPI,
and so I can't say whether this change is appropriate or not.  I have
no ARM server platforms (yet), nor any access to them, so I can't even
try it out.

In short, I've no idea.  I guess it's up to those who have come up
with SBSA to decide whether this is an appropriate change or not.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

Re: [PATCH v8 0/4] ACPI: parse the SPCR table

[Aleksey Makarov]

On 05/20/2016 04:03 PM, Aleksey Makarov wrote:

Hi Rafael,

Can you consider this patch set for queuing up please?

You approved ACPI part. I believe it's 2/4 and 3/4.

	https://lkml.kernel.org/g/CAJZ5v0hdoLTfjrD8+WxSoxM48dqbZK2KwY_h+63kHKHKgO=JFA@mail.gmail.com

1/4 is ACKed by Rob Herring and Greg Kroah-Hartman

As for 4/4 it had an ACK from Greg but since then I had to make a small
change.  For details look at

	https://lkml.kernel.org/g/c37d62c7-1e1b-0b10-a083-c5207fbbb820@gmail.com

Thank you
Aleksey Makarov

> 'ARM Server Base Boot Requirements' [1] mentions SPCR (Serial Port Console
> Redirection Table) [2] as a mandatory ACPI table that specifies the
> configuration of serial console.
> 
> Move "earlycon" early_param handling to earlycon.c to parse this option once
> 
> Parse SPCR table, setup earlycon and register specified console.
> 
> Enable parsing this table on ARM64.  Earlycon should be set up as early as
> possible.  ACPI boot tables are mapped in
> arch/arm64/kernel/acpi.c:acpi_boot_table_init() called from setup_arch() and
> that's where we parse spcr.  So it has to be opted-in per-arch. When
> ACPI_SPCR_TABLE is defined initialization of DT earlycon is deferred until the
> DT/ACPI decision is done.
> 
> Implement console_match() for pl011.
> 
> Based on the work by Leif Lindholm [3]
> Thanks to Peter Hurley for explaining how this should work.
> 
> Should be applied to next-20160520
> Tested on QEMU and ThunderX.
> SPCR support is included in QEMU's ARM64 mach-virt since 2.4 release.
> 
> v8:
> - rebase to next-20160520
> - remove the patch "ACPICA: Headers: Add new constants for the DBG2 ACPI table"
>   as it have got to linux-next
> - add Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>, Reviewed-by:
>   Peter Hurley <peter@hurleysoftware.com> (but see below)
> - fix the patch "serial: pl011: add console matching function".  The patch by
>   Christopher Covington [4] specifies that SBSA uart does 32-bit access to
>   registers and this breaks the match function.  In this series the function
>   was changed to match when SPCR specifies both mmio32 and mmio access.
>   I removed Acked-by: Greg from this patch because of these changes.
> 
> v7:
> https://lkml.kernel.org/g/1459431629-27934-1-git-send-email-aleksey.makarov@linaro.org
> - add Acked-by: Rob Herring for "of/serial: move earlycon early_param handling
>   to serial"
> - call DT earlycon initialization from the arch ACPI code, not from parse_spcr()
>   (Rafael J. Wysocki)
> - fix a few minor issues (Rafael J. Wysocki)
> 
> v6:
> https://lkml.kernel.org/g/1458823925-19560-1-git-send-email-aleksey.makarov@linaro.org
> - add documentation for parse_spcr() functioin (Yury Norov)
> - don't initialize err variable (Yury Norov)
> - add __initdata for the earlycon_init_is_deferred flag variable
> - rename the function exported in "of/serial: move earlycon early_param handling
>   to serial" to avoid clash with the function from arch/microblaze/kernel/prom.c
> - defer initialization of DT earlycon until DT/ACPI decision is made
>   (Rob Herring, Peter Hurley)
> - use snprintf instead of sprintf (Andy Shevchenko)
> - drop patch that adds EARLYCON_DECLARE for pl011 as EARLYCON_DECLARE is
>   equivalent to OF_EARLYCON_DECLARE for 4.6+ (Peter Hurley).  This means that
>   SPCR earlycon will not work on the kernels before 4.6
> 
> v5:
> https://lkml.kernel.org/g/1458643595-14719-1-git-send-email-aleksey.makarov@linaro.org
> - drop patch "serial: pl011: use ACPI SPCR to setup 32-bit access" because
>   it is ugly. Also because Christopher Covington came with a better solution [4]
> - remove error message when the table is not provided by ACPI (Andy Shevchenko)
> - rewrite spcr.c following the suggestions by Peter Hurley
> - add console_match() for pl011 in a separate patch
> - add EARLYCON_DECLARE for pl011 in a separate patch
> - add patch "of/serial: move earlycon early_param handling to serial" from
>   the GDB2 series
> 
> v4:
> https://lkml.kernel.org/g/1456747355-15692-1-git-send-email-aleksey.makarov@linaro.org
> - drop patch "ACPI: change __init to __ref for early_acpi_os_unmap_memory()"
>   ACPI developers work on a new API and asked not to do that.
>   Instead, use acpi_get_table_with_size()/early_acpi_os_unmap_memory() once
>   and cache the result. (Lv Zheng)
> - fix some style issues (Yury Norov)
> 
> v3:
> https://lkml.kernel.org/g/1455559532-8305-1-git-send-email-aleksey.makarov@linaro.org
> 
> Greg Kroah-Hartman did not like v2 so I have rewritten this patchset:
> 
> - drop acpi_match() member of struct console
> - drop implementations of this member for pl011 and 8250
> - drop the patch that renames some vars in printk.c as it is not needed anymore
> - drop patch that introduces system wide acpi_table_parse2().
>   Instead introduce a custom acpi_table_parse_spcr() in spcr.c
> 
> Instead of introducing a new match_acpi() member of struct console,
> this patchset introduces a new function acpi_console_check().
> This function is called when a new uart is registered at serial_core.c
> the same way OF code checks for console.  If the registered uart is the
> console specified by SPCR table, this function calls add_preferred_console()
> 
> The restrictions of this approach are:
> 
> - only serial consoles can be set up
> - only consoles specified by the memory/io address can be set up
>   (SPCR can specify devices by PCI id/PCI address)
> 
> v2:
> https://lkml.kernel.org/g/1455299022-11641-1-git-send-email-aleksey.makarov@linaro.org
> - don't use SPCR if user specified console in command line
> - fix initialization order of newcon->index = 0
> - rename some variables at printk.c (Joe Perches, Peter Hurley)
> - enable ACPI_SPCR_TABLE in a separate patch (Andy Shevchenko)
> - remove the retry loop for console registering (Peter Hurley).
>   Instead, obtain SPCR with acpi_get_table().  That works after
>   call to acpi_early_init() i. e. in any *_initcall()
> - describe design decision behind introducing acpi_match() (Peter Hurley)
> - fix compilation for x86 + ACPI (Graeme Gregory)
> - introduce DBG2 constants in a separate patch (Andy Shevchenko)
> - fix a typo in DBG2 constants (Andy Shevchenko)
> - add ACPI_DBG2_ARM_SBSA_32BIT constant (Christopher Covington)
> - add support for ACPI_DBG2_ARM_SBSA_* consoles (Christopher Covington)
> - add documentation for functions
> - add a patch that uses SPCR to find if SBSA serial driver should use 32-bit
>   accessor functions (Christopher Covington)
> - change __init to __ref for early_acpi_os_unmap_memory() in a separate patch
> - introduce acpi_table_parse2() in a separate patch
> - fix fetching the SPCR table early (Mark Salter)
> - add a patch from Mark Salter that introduces support for matching 8250-based
>   consoles
> 
> v1:
> https://lkml.kernel.org/g/1453722324-22407-1-git-send-email-aleksey.makarov@linaro.org
> 
> [1] http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.den0044a/index.html
> [2] https://msdn.microsoft.com/en-us/library/windows/hardware/dn639132(v=vs.85).aspx
> [3] https://lkml.kernel.org/g/1441716217-23786-1-git-send-email-leif.lindholm@linaro.org
> [4] https://lkml.kernel.org/g/1457415800-8799-1-git-send-email-cov@codeaurora.org
> 
> Aleksey Makarov (3):
>   ACPI: parse SPCR and enable matching console
>   ARM64: ACPI: enable ACPI_SPCR_TABLE
>   serial: pl011: add console matching function
> 
> Leif Lindholm (1):
>   of/serial: move earlycon early_param handling to serial
> 
>  arch/arm64/Kconfig              |   1 +
>  arch/arm64/kernel/acpi.c        |  11 +++-
>  drivers/acpi/Kconfig            |   3 ++
>  drivers/acpi/Makefile           |   1 +
>  drivers/acpi/spcr.c             | 111 ++++++++++++++++++++++++++++++++++++++++
>  drivers/of/fdt.c                |  11 +---
>  drivers/tty/serial/amba-pl011.c |  56 ++++++++++++++++++++
>  drivers/tty/serial/earlycon.c   |  19 ++++++-
>  include/linux/acpi.h            |   6 +++
>  include/linux/of_fdt.h          |   2 +
>  include/linux/serial_core.h     |   6 +++
>  11 files changed, 214 insertions(+), 13 deletions(-)
>  create mode 100644 drivers/acpi/spcr.c
> 

Re: [PATCH v8 4/4] serial: pl011: add console matching function

[Aleksey Makarov]

On 05/20/2016 04:03 PM, Aleksey Makarov wrote:
> This patch adds function pl011_console_match() that implements
> method match of struct console.  It allows to match consoles against
> data specified in a string, for example taken from command line or
> compiled by ACPI SPCR table handler.

Hi Greg,

Can you review this new version of the patch and consider ACKing it please?

It had your ACK before but I had to make some changes:

The patch by Christopher Covington [1] specifies that SBSA uart
does 32-bit access to registers and this breaks the match function.
In this series the function was changed to match with both mmio32 and
mmio access. I removed Acked-by: Greg from this patch because of these
changes.

The difference between v7 and v8:

diff --git a/drivers/tty/serial/amba-pl011.c
b/drivers/tty/serial/amba-pl011.c
index 4139b64..388edc8 100644
--- a/drivers/tty/serial/amba-pl011.c
+++ b/drivers/tty/serial/amba-pl011.c
@@ -2328,10 +2329,10 @@ static int __init pl011_console_match(struct
console *co, char *name, int idx,

                port = &amba_ports[i]->port;

-               if (port->iotype != iotype)
+               if (iotype != UPIO_MEM && iotype != UPIO_MEM32)
                        continue;
-               if ((iotype == UPIO_MEM || iotype == UPIO_MEM32) &&
-                   (port->mapbase != addr))
+
+               if (port->mapbase != addr)
                        continue;

                co->index = i;

[1]
https://lkml.kernel.org/g/1457415800-8799-1-git-send-email-cov@codeaurora.org

Thank you
Aleksey Makarov

> 
> Signed-off-by: Aleksey Makarov <aleksey.makarov@linaro.org>
> Reviewed-by: Peter Hurley <peter@hurleysoftware.com>
> ---
>  drivers/tty/serial/amba-pl011.c | 56 +++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 56 insertions(+)
> 
> diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
> index a2aa655..388edc8 100644
> --- a/drivers/tty/serial/amba-pl011.c
> +++ b/drivers/tty/serial/amba-pl011.c
> @@ -2288,12 +2288,68 @@ static int __init pl011_console_setup(struct console *co, char *options)
>  	return uart_set_options(&uap->port, co, baud, parity, bits, flow);
>  }
>  
> +/**
> + *	pl011_console_match - non-standard console matching
> + *	@co:	  registering console
> + *	@name:	  name from console command line
> + *	@idx:	  index from console command line
> + *	@options: ptr to option string from console command line
> + *
> + *	Only attempts to match console command lines of the form:
> + *	    console=pl011,mmio|mmio32,<addr>[,<options>]
> + *	    console=pl011,0x<addr>[,<options>]
> + *	This form is used to register an initial earlycon boot console and
> + *	replace it with the amba_console at pl011 driver init.
> + *
> + *	Performs console setup for a match (as required by interface)
> + *	If no <options> are specified, then assume the h/w is already setup.
> + *
> + *	Returns 0 if console matches; otherwise non-zero to use default matching
> + */
> +static int __init pl011_console_match(struct console *co, char *name, int idx,
> +				      char *options)
> +{
> +	char match[] = "pl011";	/* pl011-specific earlycon name */
> +	unsigned char iotype;
> +	unsigned long addr;
> +	int i;
> +
> +	if (strncmp(name, match, 5) != 0)
> +		return -ENODEV;
> +
> +	if (uart_parse_earlycon(options, &iotype, &addr, &options))
> +		return -ENODEV;
> +
> +	/* try to match the port specified on the command line */
> +	for (i = 0; i < ARRAY_SIZE(amba_ports); i++) {
> +		struct uart_port *port;
> +
> +		if (!amba_ports[i])
> +			continue;
> +
> +		port = &amba_ports[i]->port;
> +
> +		if (iotype != UPIO_MEM && iotype != UPIO_MEM32)
> +			continue;
> +
> +		if (port->mapbase != addr)
> +			continue;
> +
> +		co->index = i;
> +		port->cons = co;
> +		return pl011_console_setup(co, options);
> +	}
> +
> +	return -ENODEV;
> +}
> +
>  static struct uart_driver amba_reg;
>  static struct console amba_console = {
>  	.name		= "ttyAMA",
>  	.write		= pl011_console_write,
>  	.device		= uart_console_device,
>  	.setup		= pl011_console_setup,
> +	.match		= pl011_console_match,
>  	.flags		= CON_PRINTBUFFER,
>  	.index		= -1,
>  	.data		= &amba_reg,
> 

Re: [PATCH v8 4/4] serial: pl011: add console matching function

[Yury Norov]

On Fri, May 20, 2016 at 04:03:23PM +0300, Aleksey Makarov wrote:
> This patch adds function pl011_console_match() that implements
> method match of struct console.  It allows to match consoles against
> data specified in a string, for example taken from command line or
> compiled by ACPI SPCR table handler.
> 
> Signed-off-by: Aleksey Makarov <aleksey.makarov@linaro.org>
> Reviewed-by: Peter Hurley <peter@hurleysoftware.com>
> ---
>  drivers/tty/serial/amba-pl011.c | 56 +++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 56 insertions(+)
> 
> diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
> index a2aa655..388edc8 100644
> --- a/drivers/tty/serial/amba-pl011.c
> +++ b/drivers/tty/serial/amba-pl011.c
> @@ -2288,12 +2288,68 @@ static int __init pl011_console_setup(struct console *co, char *options)
>  	return uart_set_options(&uap->port, co, baud, parity, bits, flow);
>  }
>  
> +/**
> + *	pl011_console_match - non-standard console matching
> + *	@co:	  registering console
> + *	@name:	  name from console command line
> + *	@idx:	  index from console command line
> + *	@options: ptr to option string from console command line
> + *
> + *	Only attempts to match console command lines of the form:
> + *	    console=pl011,mmio|mmio32,<addr>[,<options>]
> + *	    console=pl011,0x<addr>[,<options>]
> + *	This form is used to register an initial earlycon boot console and
> + *	replace it with the amba_console at pl011 driver init.
> + *
> + *	Performs console setup for a match (as required by interface)
> + *	If no <options> are specified, then assume the h/w is already setup.
> + *
> + *	Returns 0 if console matches; otherwise non-zero to use default matching
> + */
> +static int __init pl011_console_match(struct console *co, char *name, int idx,
> +				      char *options)
> +{
> +	char match[] = "pl011";	/* pl011-specific earlycon name */
> +	unsigned char iotype;
> +	unsigned long addr;
> +	int i;
> +
> +	if (strncmp(name, match, 5) != 0)
> +		return -ENODEV;
> +
> +	if (uart_parse_earlycon(options, &iotype, &addr, &options))
> +		return -ENODEV;
> +
> +	/* try to match the port specified on the command line */
> +	for (i = 0; i < ARRAY_SIZE(amba_ports); i++) {
> +		struct uart_port *port;
> +
> +		if (!amba_ports[i])
> +			continue;
> +
> +		port = &amba_ports[i]->port;
> +
> +		if (iotype != UPIO_MEM && iotype != UPIO_MEM32)
> +			continue;

So it looks like iotype is constant inside the loop, and UPIO_MEM
and UPIO_MEM32 too, of course. It means you can move this check out of
cycle and avoid ports traversing at all in specific case. Am I wrong?

> +
> +		if (port->mapbase != addr)
> +			continue;
> +
> +		co->index = i;
> +		port->cons = co;
> +		return pl011_console_setup(co, options);
> +	}
> +
> +	return -ENODEV;
> +}
> +
>  static struct uart_driver amba_reg;
>  static struct console amba_console = {
>  	.name		= "ttyAMA",
>  	.write		= pl011_console_write,
>  	.device		= uart_console_device,
>  	.setup		= pl011_console_setup,
> +	.match		= pl011_console_match,
>  	.flags		= CON_PRINTBUFFER,
>  	.index		= -1,
>  	.data		= &amba_reg,
> -- 
> 2.8.2

Re: [PATCH v8 4/4] serial: pl011: add console matching function

[Peter Hurley]

> On Jun 22, 2016, at 5:18 AM, Yury Norov <ynorov@caviumnetworks.com> wrote:
> 
>> On Fri, May 20, 2016 at 04:03:23PM +0300, Aleksey Makarov wrote:
>> This patch adds function pl011_console_match() that implements
>> method match of struct console.  It allows to match consoles against
>> data specified in a string, for example taken from command line or
>> compiled by ACPI SPCR table handler.
>> 
>> Signed-off-by: Aleksey Makarov <aleksey.makarov@linaro.org>
>> Reviewed-by: Peter Hurley <peter@hurleysoftware.com>
>> ---
>> drivers/tty/serial/amba-pl011.c | 56 +++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 56 insertions(+)
>> 
>> diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
>> index a2aa655..388edc8 100644
>> --- a/drivers/tty/serial/amba-pl011.c
>> +++ b/drivers/tty/serial/amba-pl011.c
>> @@ -2288,12 +2288,68 @@ static int __init pl011_console_setup(struct console *co, char *options)
>>    return uart_set_options(&uap->port, co, baud, parity, bits, flow);
>> }
>> 
>> +/**
>> + *    pl011_console_match - non-standard console matching
>> + *    @co:      registering console
>> + *    @name:      name from console command line
>> + *    @idx:      index from console command line
>> + *    @options: ptr to option string from console command line
>> + *
>> + *    Only attempts to match console command lines of the form:
>> + *        console=pl011,mmio|mmio32,<addr>[,<options>]
>> + *        console=pl011,0x<addr>[,<options>]
>> + *    This form is used to register an initial earlycon boot console and
>> + *    replace it with the amba_console at pl011 driver init.
>> + *
>> + *    Performs console setup for a match (as required by interface)
>> + *    If no <options> are specified, then assume the h/w is already setup.
>> + *
>> + *    Returns 0 if console matches; otherwise non-zero to use default matching
>> + */
>> +static int __init pl011_console_match(struct console *co, char *name, int idx,
>> +                      char *options)
>> +{
>> +    char match[] = "pl011";    /* pl011-specific earlycon name */
>> +    unsigned char iotype;
>> +    unsigned long addr;
>> +    int i;
>> +
>> +    if (strncmp(name, match, 5) != 0)
>> +        return -ENODEV;
>> +
>> +    if (uart_parse_earlycon(options, &iotype, &addr, &options))
>> +        return -ENODEV;
>> +
>> +    /* try to match the port specified on the command line */
>> +    for (i = 0; i < ARRAY_SIZE(amba_ports); i++) {
>> +        struct uart_port *port;
>> +
>> +        if (!amba_ports[i])
>> +            continue;
>> +
>> +        port = &amba_ports[i]->port;
>> +
>> +        if (iotype != UPIO_MEM && iotype != UPIO_MEM32)
>> +            continue;
> 
> So it looks like iotype is constant inside the loop, and UPIO_MEM
> and UPIO_MEM32 too, of course. It means you can move this check out of
> cycle and avoid ports traversing at all in specific case. Am I wrong?
> 

No you're not wrong but I'd prefer if we don't use assumptions like that in port enumeration.


>> +
>> +        if (port->mapbase != addr)
>> +            continue;
>> +
>> +        co->index = i;
>> +        port->cons = co;
>> +        return pl011_console_setup(co, options);
>> +    }
>> +
>> +    return -ENODEV;
>> +}
>> +
>> static struct uart_driver amba_reg;
>> static struct console amba_console = {
>>    .name        = "ttyAMA",
>>    .write        = pl011_console_write,
>>    .device        = uart_console_device,
>>    .setup        = pl011_console_setup,
>> +    .match        = pl011_console_match,
>>    .flags        = CON_PRINTBUFFER,
>>    .index        = -1,
>>    .data        = &amba_reg,
>> -- 
>> 2.8.2

Re: [PATCH v8 4/4] serial: pl011: add console matching function

[Yury Norov]

Hi Peter, 

Nice to meet you.

On Wed, Jun 22, 2016 at 07:08:33AM -0700, Peter Hurley wrote:
> 
> 
> > On Jun 22, 2016, at 5:18 AM, Yury Norov <ynorov@caviumnetworks.com> wrote:
> > 
> >> On Fri, May 20, 2016 at 04:03:23PM +0300, Aleksey Makarov wrote:
> >> This patch adds function pl011_console_match() that implements
> >> method match of struct console.  It allows to match consoles against
> >> data specified in a string, for example taken from command line or
> >> compiled by ACPI SPCR table handler.
> >> 
> >> Signed-off-by: Aleksey Makarov <aleksey.makarov@linaro.org>
> >> Reviewed-by: Peter Hurley <peter@hurleysoftware.com>
> >> ---
> >> drivers/tty/serial/amba-pl011.c | 56 +++++++++++++++++++++++++++++++++++++++++
> >> 1 file changed, 56 insertions(+)
> >> 
> >> diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
> >> index a2aa655..388edc8 100644
> >> --- a/drivers/tty/serial/amba-pl011.c
> >> +++ b/drivers/tty/serial/amba-pl011.c
> >> @@ -2288,12 +2288,68 @@ static int __init pl011_console_setup(struct console *co, char *options)
> >>    return uart_set_options(&uap->port, co, baud, parity, bits, flow);
> >> }
> >> 
> >> +/**
> >> + *    pl011_console_match - non-standard console matching
> >> + *    @co:      registering console
> >> + *    @name:      name from console command line
> >> + *    @idx:      index from console command line
> >> + *    @options: ptr to option string from console command line
> >> + *
> >> + *    Only attempts to match console command lines of the form:
> >> + *        console=pl011,mmio|mmio32,<addr>[,<options>]
> >> + *        console=pl011,0x<addr>[,<options>]
> >> + *    This form is used to register an initial earlycon boot console and
> >> + *    replace it with the amba_console at pl011 driver init.
> >> + *
> >> + *    Performs console setup for a match (as required by interface)
> >> + *    If no <options> are specified, then assume the h/w is already setup.
> >> + *
> >> + *    Returns 0 if console matches; otherwise non-zero to use default matching
> >> + */
> >> +static int __init pl011_console_match(struct console *co, char *name, int idx,
> >> +                      char *options)
> >> +{
> >> +    char match[] = "pl011";    /* pl011-specific earlycon name */
> >> +    unsigned char iotype;
> >> +    unsigned long addr;
> >> +    int i;
> >> +
> >> +    if (strncmp(name, match, 5) != 0)
> >> +        return -ENODEV;
> >> +
> >> +    if (uart_parse_earlycon(options, &iotype, &addr, &options))
> >> +        return -ENODEV;
> >> +
> >> +    /* try to match the port specified on the command line */
> >> +    for (i = 0; i < ARRAY_SIZE(amba_ports); i++) {
> >> +        struct uart_port *port;
> >> +
> >> +        if (!amba_ports[i])
> >> +            continue;
> >> +
> >> +        port = &amba_ports[i]->port;
> >> +
> >> +        if (iotype != UPIO_MEM && iotype != UPIO_MEM32)
> >> +            continue;
> > 
> > So it looks like iotype is constant inside the loop, and UPIO_MEM
> > and UPIO_MEM32 too, of course. It means you can move this check out of
> > cycle and avoid ports traversing at all in specific case. Am I wrong?
> > 
> 
> No you're not wrong but I'd prefer if we don't use assumptions like that in port enumeration.

I don't think this is an assumption. I think this is solid fact.
There's no a single chance for stack-allocated variable to be
shared with other thread of execution, or be unexpectedly changed
somehow else. So this code not only decreases performance (I don't
think it's really hot path), but also confuses reader, and makes him
spend more time on reading this than it deserves.

If you still insist on current version, I'd ask you or Alexey to add
clear description why we check the same condition again and again
inside the loop.

Yury.

> >> +
> >> +        if (port->mapbase != addr)
> >> +            continue;
> >> +
> >> +        co->index = i;
> >> +        port->cons = co;
> >> +        return pl011_console_setup(co, options);
> >> +    }
> >> +
> >> +    return -ENODEV;
> >> +}
> >> +
> >> static struct uart_driver amba_reg;
> >> static struct console amba_console = {
> >>    .name        = "ttyAMA",
> >>    .write        = pl011_console_write,
> >>    .device        = uart_console_device,
> >>    .setup        = pl011_console_setup,
> >> +    .match        = pl011_console_match,
> >>    .flags        = CON_PRINTBUFFER,
> >>    .index        = -1,
> >>    .data        = &amba_reg,
> >> -- 
> >> 2.8.2

Re: [PATCH v8 4/4] serial: pl011: add console matching function

[Matthias Brugger]

On 22/06/16 22:45, Yury Norov wrote:
> Hi Peter,
>
> Nice to meet you.
>
> On Wed, Jun 22, 2016 at 07:08:33AM -0700, Peter Hurley wrote:
>>
>>
>>> On Jun 22, 2016, at 5:18 AM, Yury Norov <ynorov@caviumnetworks.com> wrote:
>>>
>>>> On Fri, May 20, 2016 at 04:03:23PM +0300, Aleksey Makarov wrote:
>>>> This patch adds function pl011_console_match() that implements
>>>> method match of struct console.  It allows to match consoles against
>>>> data specified in a string, for example taken from command line or
>>>> compiled by ACPI SPCR table handler.
>>>>
>>>> Signed-off-by: Aleksey Makarov <aleksey.makarov@linaro.org>
>>>> Reviewed-by: Peter Hurley <peter@hurleysoftware.com>
>>>> ---
>>>> drivers/tty/serial/amba-pl011.c | 56 +++++++++++++++++++++++++++++++++++++++++
>>>> 1 file changed, 56 insertions(+)
>>>>
>>>> diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
>>>> index a2aa655..388edc8 100644
>>>> --- a/drivers/tty/serial/amba-pl011.c
>>>> +++ b/drivers/tty/serial/amba-pl011.c
>>>> @@ -2288,12 +2288,68 @@ static int __init pl011_console_setup(struct console *co, char *options)
>>>>     return uart_set_options(&uap->port, co, baud, parity, bits, flow);
>>>> }
>>>>
>>>> +/**
>>>> + *    pl011_console_match - non-standard console matching
>>>> + *    @co:      registering console
>>>> + *    @name:      name from console command line
>>>> + *    @idx:      index from console command line
>>>> + *    @options: ptr to option string from console command line
>>>> + *
>>>> + *    Only attempts to match console command lines of the form:
>>>> + *        console=pl011,mmio|mmio32,<addr>[,<options>]
>>>> + *        console=pl011,0x<addr>[,<options>]
>>>> + *    This form is used to register an initial earlycon boot console and
>>>> + *    replace it with the amba_console at pl011 driver init.
>>>> + *
>>>> + *    Performs console setup for a match (as required by interface)
>>>> + *    If no <options> are specified, then assume the h/w is already setup.
>>>> + *
>>>> + *    Returns 0 if console matches; otherwise non-zero to use default matching
>>>> + */
>>>> +static int __init pl011_console_match(struct console *co, char *name, int idx,
>>>> +                      char *options)
>>>> +{
>>>> +    char match[] = "pl011";    /* pl011-specific earlycon name */
>>>> +    unsigned char iotype;
>>>> +    unsigned long addr;
>>>> +    int i;
>>>> +
>>>> +    if (strncmp(name, match, 5) != 0)
>>>> +        return -ENODEV;
>>>> +
>>>> +    if (uart_parse_earlycon(options, &iotype, &addr, &options))
>>>> +        return -ENODEV;
>>>> +
>>>> +    /* try to match the port specified on the command line */
>>>> +    for (i = 0; i < ARRAY_SIZE(amba_ports); i++) {
>>>> +        struct uart_port *port;
>>>> +
>>>> +        if (!amba_ports[i])
>>>> +            continue;
>>>> +
>>>> +        port = &amba_ports[i]->port;
>>>> +
>>>> +        if (iotype != UPIO_MEM && iotype != UPIO_MEM32)
>>>> +            continue;
>>>
>>> So it looks like iotype is constant inside the loop, and UPIO_MEM
>>> and UPIO_MEM32 too, of course. It means you can move this check out of
>>> cycle and avoid ports traversing at all in specific case. Am I wrong?
>>>
>>
>> No you're not wrong but I'd prefer if we don't use assumptions like that in port enumeration.
>
> I don't think this is an assumption. I think this is solid fact.
> There's no a single chance for stack-allocated variable to be
> shared with other thread of execution, or be unexpectedly changed
> somehow else. So this code not only decreases performance (I don't
> think it's really hot path), but also confuses reader, and makes him
> spend more time on reading this than it deserves.

I agree with Yury on this. From my point of view, it makes no sense to 
check the iotype in every loop iteration.

I tried to apply these patches against linux-next. They needed some 
changes to apply, which is quite normal after such a long time. Apart 
from that the DBG2 subtype patch didn't end up in mainline, so this 
patches do not compile.

I followed up on that on the corresponding thread. Please don't forget 
to pin-point in the introduction mail to any other series which is 
needed for your patches to compile/work.

Thanks a lot,
Matthias


> If you still insist on current version, I'd ask you or Alexey to add
> clear description why we check the same condition again and again
> inside the loop.
>
> Yury.
>
>>>> +
>>>> +        if (port->mapbase != addr)
>>>> +            continue;
>>>> +
>>>> +        co->index = i;
>>>> +        port->cons = co;
>>>> +        return pl011_console_setup(co, options);
>>>> +    }
>>>> +
>>>> +    return -ENODEV;
>>>> +}
>>>> +
>>>> static struct uart_driver amba_reg;
>>>> static struct console amba_console = {
>>>>     .name        = "ttyAMA",
>>>>     .write        = pl011_console_write,
>>>>     .device        = uart_console_device,
>>>>     .setup        = pl011_console_setup,
>>>> +    .match        = pl011_console_match,
>>>>     .flags        = CON_PRINTBUFFER,
>>>>     .index        = -1,
>>>>     .data        = &amba_reg,
>>>> --
>>>> 2.8.2
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>

Re: [PATCH v8 4/4] serial: pl011: add console matching function

[Matthias Brugger]

On 11/07/16 16:40, Matthias Brugger wrote:

>
> I tried to apply these patches against linux-next. They needed some
> changes to apply, which is quite normal after such a long time. Apart
> from that the DBG2 subtype patch didn't end up in mainline, so this
> patches do not compile.
>
> I followed up on that on the corresponding thread. Please don't forget
> to pin-point in the introduction mail to any other series which is
> needed for your patches to compile/work.
>

Ok, I sorted out, that the DBG2 subtypes actually are part of mainline 
and that I screwed up my branches.

Sorry for the noise.
Matthias

Re: [PATCH v8 4/4] serial: pl011: add console matching function

[Graeme Gregory]

On Mon, Jun 20, 2016 at 04:19:14PM +0100, Russell King - ARM Linux wrote:
> On Mon, Jun 20, 2016 at 03:26:39PM +0300, Aleksey Makarov wrote:
> > 
> > On 06/02/2016 09:02 PM, Aleksey Makarov wrote:
> > > 
> > > On 05/20/2016 04:03 PM, Aleksey Makarov wrote:
> > 
> > Hi Russell,
> > 
> > Can you review this patch please?
> > 
> > Thank you
> > Aleksey Makarov
> > 
> > >> This patch adds function pl011_console_match() that implements
> > >> method match of struct console.  It allows to match consoles against
> > >> data specified in a string, for example taken from command line or
> > >> compiled by ACPI SPCR table handler.
> > >>
> > >> Signed-off-by: Aleksey Makarov <aleksey.makarov@linaro.org>
> > >> Reviewed-by: Peter Hurley <peter@hurleysoftware.com>
> > > 
> > > Hi Greg, Russell,
> > > 
> > > Can you review this patch and consider ACKing it please?
> > > It had an ACK from Greg in v7 but since then I changed it a bit.
> > > 
> > > The patch by Christopher Covington [1] specifies that SBSA uart 
> > > does 32-bit access to registers and this breaks the match function.  
> > > In this series the function was changed to match when SPCR specifies
> > > both mmio32 and mmio access.  I removed Acked-by: Greg from this
> > > patch because of these changes.
> > > 
> > > The difference between v7 and v8:
> > > 
> > > diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
> > > index 4139b64..388edc8 100644
> > > --- a/drivers/tty/serial/amba-pl011.c
> > > +++ b/drivers/tty/serial/amba-pl011.c
> > > @@ -2328,10 +2329,10 @@ static int __init pl011_console_match(struct console *co, char *name, int idx,
> > >  
> > >                 port = &amba_ports[i]->port;
> > >  
> > > -               if (port->iotype != iotype)
> > > +               if (iotype != UPIO_MEM && iotype != UPIO_MEM32)
> > >                         continue;
> > > -               if ((iotype == UPIO_MEM || iotype == UPIO_MEM32) &&
> > > -                   (port->mapbase != addr))
> > > +
> > > +               if (port->mapbase != addr)
> > >                         continue;
> > >  
> > >                 co->index = i;
> 
> I'm looking at this change, and I don't know what effect it ultimately
> has.  This is kind of a problem, because I've zero knowledge of ACPI,
> and so I can't say whether this change is appropriate or not.  I have
> no ARM server platforms (yet), nor any access to them, so I can't even
> try it out.
> 
> In short, I've no idea.  I guess it's up to those who have come up
> with SBSA to decide whether this is an appropriate change or not.
> 

Lorenzo, Sudeep, as ARM ACPI maintainers and keepers of SBSA do you have
any comment on this?

Graeme