[PATCH 0/3] ACPI: parse the SPCR table

[PATCH 0/3] ACPI: parse the SPCR table

[Aleksey Makarov]

This patchset is based on the patchset by Leif Lindholm [1]

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

Licensing concerns have prevented implementing it in the past, but as of
10 August 2015, these tables have both been released also under 
OWF 1.0 [4].

SPCR support is included in QEMU's ARM mach-virt since 2.4 release.

Parse the SPCR table and check if any registered console match
the description.  If it does, enable that console.

To implement that, introduce a new member
int (*acpi_match)(struct console *, struct acpi_table_spcr *);
of struct console.  It allows drivers to check if they provide
a matching console device.

Also add an implementation of this member function to the pl011 driver
and fix a minor issue in kernel/printk/printk.c

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

Aleksey Makarov (3):
  printk: make preferred_console local static bool
  ACPI: parse SPCR and enable matching console
  serial: pl011: add acpi_match for amba-pl011.c

 arch/arm64/Kconfig              |  1 +
 drivers/acpi/Kconfig            |  3 ++
 drivers/acpi/Makefile           |  1 +
 drivers/acpi/spcr.c             | 85 +++++++++++++++++++++++++++++++++++++
 drivers/tty/serial/amba-pl011.c | 14 ++++++
 include/acpi/actbl2.h           |  4 ++
 include/linux/console.h         | 12 ++++++
 kernel/printk/printk.c          | 94 +++++++++++++++++++++++++++++++----------
 8 files changed, 191 insertions(+), 23 deletions(-)
 create mode 100644 drivers/acpi/spcr.c

-- 
2.7.0

[PATCH 0/3] ACPI: parse the SPCR table

[Aleksey Makarov]

This patchset is based on the patchset by Leif Lindholm [1]

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

Licensing concerns have prevented implementing it in the past, but as of
10 August 2015, these tables have both been released also under 
OWF 1.0 [4].

SPCR support is included in QEMU's ARM mach-virt since 2.4 release.

Parse the SPCR table and check if any registered console match
the description.  If it does, enable that console.

To implement that, introduce a new member
int (*acpi_match)(struct console *, struct acpi_table_spcr *);
of struct console.  It allows drivers to check if they provide
a matching console device.

Also add an implementation of this member function to the pl011 driver
and fix a minor issue in kernel/printk/printk.c

[1] https://lkml.kernel.org/g/1441716217-23786-1-git-send-email-leif.lindholm at linaro.org
[2] http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.den0044a/index.html
[3] https://msdn.microsoft.com/en-us/library/windows/hardware/dn639132(v=vs.85).aspx
[4] http://www.openwebfoundation.org/legal/the-owf-1-0-agreements/owfa-1-0

Aleksey Makarov (3):
  printk: make preferred_console local static bool
  ACPI: parse SPCR and enable matching console
  serial: pl011: add acpi_match for amba-pl011.c

 arch/arm64/Kconfig              |  1 +
 drivers/acpi/Kconfig            |  3 ++
 drivers/acpi/Makefile           |  1 +
 drivers/acpi/spcr.c             | 85 +++++++++++++++++++++++++++++++++++++
 drivers/tty/serial/amba-pl011.c | 14 ++++++
 include/acpi/actbl2.h           |  4 ++
 include/linux/console.h         | 12 ++++++
 kernel/printk/printk.c          | 94 +++++++++++++++++++++++++++++++----------
 8 files changed, 191 insertions(+), 23 deletions(-)
 create mode 100644 drivers/acpi/spcr.c

-- 
2.7.0

[PATCH 1/3] printk: make preferred_console local static bool

[Aleksey Makarov]

The variable preferred_console is used only inside register_console()
and it's semantics is boolean.  Make it clear.

Signed-off-by: Aleksey Makarov <aleksey.makarov@linaro.org>
---
 kernel/printk/printk.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index 2ce8826..37e531f 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -143,7 +143,6 @@ static struct console *exclusive_console;
 static struct console_cmdline console_cmdline[MAX_CMDLINECONSOLES];
 
 static int selected_console = -1;
-static int preferred_console = -1;
 int console_set_on_cmdline;
 EXPORT_SYMBOL(console_set_on_cmdline);
 
@@ -2456,6 +2455,7 @@ void register_console(struct console *newcon)
 	unsigned long flags;
 	struct console *bcon = NULL;
 	struct console_cmdline *c;
+	static bool preferred_console;
 
 	if (console_drivers)
 		for_each_console(bcon)
@@ -2482,15 +2482,15 @@ void register_console(struct console *newcon)
 	if (console_drivers && console_drivers->flags & CON_BOOT)
 		bcon = console_drivers;
 
-	if (preferred_console < 0 || bcon || !console_drivers)
-		preferred_console = selected_console;
+	if (!preferred_console || bcon || !console_drivers)
+		preferred_console = selected_console >= 0;
 
 	/*
 	 *	See if we want to use this console driver. If we
 	 *	didn't select a console we take the first one
 	 *	that registers here.
 	 */
-	if (preferred_console < 0) {
+	if (!preferred_console) {
 		if (newcon->index < 0)
 			newcon->index = 0;
 		if (newcon->setup == NULL ||
@@ -2498,7 +2498,7 @@ void register_console(struct console *newcon)
 			newcon->flags |= CON_ENABLED;
 			if (newcon->device) {
 				newcon->flags |= CON_CONSDEV;
-				preferred_console = 0;
+				preferred_console = true;
 			}
 		}
 	}
@@ -2533,7 +2533,7 @@ void register_console(struct console *newcon)
 		newcon->flags |= CON_ENABLED;
 		if (i == selected_console) {
 			newcon->flags |= CON_CONSDEV;
-			preferred_console = selected_console;
+			preferred_console = true;
 		}
 		break;
 	}
-- 
2.7.0

[PATCH 1/3] printk: make preferred_console local static bool

[Aleksey Makarov]

The variable preferred_console is used only inside register_console()
and it's semantics is boolean.  Make it clear.

Signed-off-by: Aleksey Makarov <aleksey.makarov@linaro.org>
---
 kernel/printk/printk.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index 2ce8826..37e531f 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -143,7 +143,6 @@ static struct console *exclusive_console;
 static struct console_cmdline console_cmdline[MAX_CMDLINECONSOLES];
 
 static int selected_console = -1;
-static int preferred_console = -1;
 int console_set_on_cmdline;
 EXPORT_SYMBOL(console_set_on_cmdline);
 
@@ -2456,6 +2455,7 @@ void register_console(struct console *newcon)
 	unsigned long flags;
 	struct console *bcon = NULL;
 	struct console_cmdline *c;
+	static bool preferred_console;
 
 	if (console_drivers)
 		for_each_console(bcon)
@@ -2482,15 +2482,15 @@ void register_console(struct console *newcon)
 	if (console_drivers && console_drivers->flags & CON_BOOT)
 		bcon = console_drivers;
 
-	if (preferred_console < 0 || bcon || !console_drivers)
-		preferred_console = selected_console;
+	if (!preferred_console || bcon || !console_drivers)
+		preferred_console = selected_console >= 0;
 
 	/*
 	 *	See if we want to use this console driver. If we
 	 *	didn't select a console we take the first one
 	 *	that registers here.
 	 */
-	if (preferred_console < 0) {
+	if (!preferred_console) {
 		if (newcon->index < 0)
 			newcon->index = 0;
 		if (newcon->setup == NULL ||
@@ -2498,7 +2498,7 @@ void register_console(struct console *newcon)
 			newcon->flags |= CON_ENABLED;
 			if (newcon->device) {
 				newcon->flags |= CON_CONSDEV;
-				preferred_console = 0;
+				preferred_console = true;
 			}
 		}
 	}
@@ -2533,7 +2533,7 @@ void register_console(struct console *newcon)
 		newcon->flags |= CON_ENABLED;
 		if (i == selected_console) {
 			newcon->flags |= CON_CONSDEV;
-			preferred_console = selected_console;
+			preferred_console = true;
 		}
 		break;
 	}
-- 
2.7.0

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

[Aleksey Makarov]

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

Parse this table and check if any registered console match
the description.  If it does, enable that console.

To implement that, introduce a new member
int (*acpi_match)(struct console *, struct acpi_table_spcr *)
of struct console.  It allows drivers to check if they provide
a matching console device.

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

Signed-off-by: Aleksey Makarov <aleksey.makarov@linaro.org>
---
 arch/arm64/Kconfig      |  1 +
 drivers/acpi/Kconfig    |  3 ++
 drivers/acpi/Makefile   |  1 +
 drivers/acpi/spcr.c     | 85 +++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/console.h | 12 +++++++
 kernel/printk/printk.c  | 82 +++++++++++++++++++++++++++++++++++++----------
 6 files changed, 167 insertions(+), 17 deletions(-)
 create mode 100644 drivers/acpi/spcr.c

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 573bebc..bf31e3c 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_ATOMIC64_DEC_IF_POSITIVE
 	select ARCH_HAS_ELF_RANDOMIZE
 	select ARCH_HAS_GCOV_PROFILE_ALL
diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
index e315061..142a338 100644
--- a/drivers/acpi/Kconfig
+++ b/drivers/acpi/Kconfig
@@ -60,6 +60,9 @@ config ACPI_CCA_REQUIRED
 config IORT_TABLE
 	bool
 
+config ACPI_SPCR_TABLE
+	bool
+
 config ACPI_DEBUGGER
 	bool "AML debugger interface (EXPERIMENTAL)"
 	select ACPI_DEBUG
diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
index 265eb90..8316859 100644
--- a/drivers/acpi/Makefile
+++ b/drivers/acpi/Makefile
@@ -81,6 +81,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_IORT_TABLE) 	+= iort.o
+obj-$(CONFIG_ACPI_SPCR_TABLE)	+= spcr.o
 
 # processor has its own "processor." module_param namespace
 processor-y			:= processor_driver.o
diff --git a/drivers/acpi/spcr.c b/drivers/acpi/spcr.c
new file mode 100644
index 0000000..ccb19a0
--- /dev/null
+++ b/drivers/acpi/spcr.c
@@ -0,0 +1,85 @@
+/*
+ * 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>
+
+static struct acpi_table_spcr *spcr_table;
+
+int console_acpi_match(struct console *c, char **options)
+{
+	int err;
+
+	if (!c->acpi_match)
+		return -ENODEV;
+
+	if (!spcr_table)
+		return -EAGAIN;
+
+	err = c->acpi_match(c, spcr_table);
+	if (err < 0)
+		return err;
+
+	if (options) {
+		switch (spcr_table->baud_rate) {
+		case 3:
+			*options = "9600";
+			break;
+		case 4:
+			*options = "19200";
+			break;
+		case 6:
+			*options = "57600";
+			break;
+		case 7:
+			*options = "115200";
+			break;
+		default:
+			*options = "";
+			break;
+		}
+	}
+
+	return err;
+}
+
+static int __init spcr_table_detect(void)
+{
+	struct acpi_table_header *table;
+	acpi_status status;
+
+	if (acpi_disabled)
+		return -ENODEV;
+
+	status = acpi_get_table(ACPI_SIG_SPCR, 0, &table);
+	if (ACPI_FAILURE(status)) {
+		const char *msg = acpi_format_exception(status);
+
+		pr_err("Failed to get table, %s\n", msg);
+		return -EINVAL;
+	}
+
+	if (table->revision < 2)
+		return -EOPNOTSUPP;
+
+	spcr_table = (struct acpi_table_spcr *)table;
+
+	pr_info("Console at 0x%016llx\n", spcr_table->serial_port.address);
+
+	acpi_register_consoles_try_again();
+
+	return 0;
+}
+
+arch_initcall(spcr_table_detect);
diff --git a/include/linux/console.h b/include/linux/console.h
index bd19434..94d0bd8 100644
--- a/include/linux/console.h
+++ b/include/linux/console.h
@@ -117,6 +117,7 @@ static inline int con_debug_leave(void)
 #define CON_BRL		(32) /* Used for a braille device */
 #define CON_EXTENDED	(64) /* Use the extended output format a la /dev/kmsg */
 
+struct acpi_table_spcr;
 struct console {
 	char	name[16];
 	void	(*write)(struct console *, const char *, unsigned);
@@ -125,6 +126,7 @@ struct console {
 	void	(*unblank)(void);
 	int	(*setup)(struct console *, char *);
 	int	(*match)(struct console *, char *name, int idx, char *options);
+	int	(*acpi_match)(struct console *, struct acpi_table_spcr *);
 	short	flags;
 	short	index;
 	int	cflag;
@@ -132,6 +134,16 @@ struct console {
 	struct	 console *next;
 };
 
+#ifdef CONFIG_ACPI
+int console_acpi_match(struct console *c, char **options);
+#else
+static inline int console_acpi_match(struct console *c, char **options)
+{
+	return -ENODEV;
+}
+#endif
+void acpi_register_consoles_try_again(void);
+
 /*
  * for_each_console() allows you to iterate on each console
  */
diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index 37e531f..3cf8cba 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -2430,6 +2430,25 @@ static int __init keep_bootcon_setup(char *str)
 
 early_param("keep_bootcon", keep_bootcon_setup);
 
+static DEFINE_MUTEX(acpi_consoles_delayed_mutex);
+static struct console *acpi_consoles_delayed;
+
+void acpi_register_consoles_try_again(void)
+{
+	mutex_lock(&acpi_consoles_delayed_mutex);
+	while (acpi_consoles_delayed) {
+
+		struct console *c = acpi_consoles_delayed;
+
+		acpi_consoles_delayed = acpi_consoles_delayed->next;
+
+		mutex_unlock(&acpi_consoles_delayed_mutex);
+		register_console(c);
+		mutex_lock(&acpi_consoles_delayed_mutex);
+	}
+	mutex_unlock(&acpi_consoles_delayed_mutex);
+}
+
 /*
  * The console driver calls this routine during kernel initialization
  * to register the console printing procedure with printk() and to
@@ -2538,8 +2557,30 @@ void register_console(struct console *newcon)
 		break;
 	}
 
-	if (!(newcon->flags & CON_ENABLED))
-		return;
+	if (!(newcon->flags & CON_ENABLED)) {
+		char *opts;
+		int err;
+
+		if (newcon->index < 0)
+			newcon->index = 0;
+
+		err = console_acpi_match(newcon, &opts);
+
+		if (err == -EAGAIN) {
+			mutex_lock(&acpi_consoles_delayed_mutex);
+			newcon->next = acpi_consoles_delayed;
+			acpi_consoles_delayed = newcon;
+			mutex_unlock(&acpi_consoles_delayed_mutex);
+			return;
+		} else if (err < 0) {
+			return;
+		} else {
+			if (newcon->setup && newcon->setup(newcon, opts) != 0)
+				return;
+			newcon->flags |= CON_ENABLED | CON_CONSDEV;
+			preferred_console = true;
+		}
+	}
 
 	/*
 	 * If we have a bootconsole, and are switching to a real console,
@@ -2612,34 +2653,41 @@ void register_console(struct console *newcon)
 }
 EXPORT_SYMBOL(register_console);
 
+static int delete_from_console_list(struct console **list, struct console *c)
+{
+	while (*list) {
+		struct console *cur = *list;
+
+		if (cur == c) {
+			*list = cur->next;
+			return 0;
+		}
+		list = &cur->next;
+	}
+	return 1;
+}
+
 int unregister_console(struct console *console)
 {
-        struct console *a, *b;
 	int res;
 
 	pr_info("%sconsole [%s%d] disabled\n",
 		(console->flags & CON_BOOT) ? "boot" : "" ,
 		console->name, console->index);
 
+	mutex_lock(&acpi_consoles_delayed_mutex);
+	res = delete_from_console_list(&acpi_consoles_delayed, console);
+	mutex_unlock(&acpi_consoles_delayed_mutex);
+	if (res == 0)
+		return res;
+
 	res = _braille_unregister_console(console);
 	if (res)
 		return res;
 
-	res = 1;
 	console_lock();
-	if (console_drivers == console) {
-		console_drivers=console->next;
-		res = 0;
-	} else if (console_drivers) {
-		for (a=console_drivers->next, b=console_drivers ;
-		     a; b=a, a=b->next) {
-			if (a == console) {
-				b->next = a->next;
-				res = 0;
-				break;
-			}
-		}
-	}
+
+	res = delete_from_console_list(&console_drivers, console);
 
 	if (!res && (console->flags & CON_EXTENDED))
 		nr_ext_console_drivers--;
-- 
2.7.0

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

[Aleksey Makarov]

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

Parse this table and check if any registered console match
the description.  If it does, enable that console.

To implement that, introduce a new member
int (*acpi_match)(struct console *, struct acpi_table_spcr *)
of struct console.  It allows drivers to check if they provide
a matching console device.

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

Signed-off-by: Aleksey Makarov <aleksey.makarov@linaro.org>
---
 arch/arm64/Kconfig      |  1 +
 drivers/acpi/Kconfig    |  3 ++
 drivers/acpi/Makefile   |  1 +
 drivers/acpi/spcr.c     | 85 +++++++++++++++++++++++++++++++++++++++++++++++++
 include/linux/console.h | 12 +++++++
 kernel/printk/printk.c  | 82 +++++++++++++++++++++++++++++++++++++----------
 6 files changed, 167 insertions(+), 17 deletions(-)
 create mode 100644 drivers/acpi/spcr.c

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 573bebc..bf31e3c 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_ATOMIC64_DEC_IF_POSITIVE
 	select ARCH_HAS_ELF_RANDOMIZE
 	select ARCH_HAS_GCOV_PROFILE_ALL
diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
index e315061..142a338 100644
--- a/drivers/acpi/Kconfig
+++ b/drivers/acpi/Kconfig
@@ -60,6 +60,9 @@ config ACPI_CCA_REQUIRED
 config IORT_TABLE
 	bool
 
+config ACPI_SPCR_TABLE
+	bool
+
 config ACPI_DEBUGGER
 	bool "AML debugger interface (EXPERIMENTAL)"
 	select ACPI_DEBUG
diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
index 265eb90..8316859 100644
--- a/drivers/acpi/Makefile
+++ b/drivers/acpi/Makefile
@@ -81,6 +81,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_IORT_TABLE) 	+= iort.o
+obj-$(CONFIG_ACPI_SPCR_TABLE)	+= spcr.o
 
 # processor has its own "processor." module_param namespace
 processor-y			:= processor_driver.o
diff --git a/drivers/acpi/spcr.c b/drivers/acpi/spcr.c
new file mode 100644
index 0000000..ccb19a0
--- /dev/null
+++ b/drivers/acpi/spcr.c
@@ -0,0 +1,85 @@
+/*
+ * 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>
+
+static struct acpi_table_spcr *spcr_table;
+
+int console_acpi_match(struct console *c, char **options)
+{
+	int err;
+
+	if (!c->acpi_match)
+		return -ENODEV;
+
+	if (!spcr_table)
+		return -EAGAIN;
+
+	err = c->acpi_match(c, spcr_table);
+	if (err < 0)
+		return err;
+
+	if (options) {
+		switch (spcr_table->baud_rate) {
+		case 3:
+			*options = "9600";
+			break;
+		case 4:
+			*options = "19200";
+			break;
+		case 6:
+			*options = "57600";
+			break;
+		case 7:
+			*options = "115200";
+			break;
+		default:
+			*options = "";
+			break;
+		}
+	}
+
+	return err;
+}
+
+static int __init spcr_table_detect(void)
+{
+	struct acpi_table_header *table;
+	acpi_status status;
+
+	if (acpi_disabled)
+		return -ENODEV;
+
+	status = acpi_get_table(ACPI_SIG_SPCR, 0, &table);
+	if (ACPI_FAILURE(status)) {
+		const char *msg = acpi_format_exception(status);
+
+		pr_err("Failed to get table, %s\n", msg);
+		return -EINVAL;
+	}
+
+	if (table->revision < 2)
+		return -EOPNOTSUPP;
+
+	spcr_table = (struct acpi_table_spcr *)table;
+
+	pr_info("Console@0x%016llx\n", spcr_table->serial_port.address);
+
+	acpi_register_consoles_try_again();
+
+	return 0;
+}
+
+arch_initcall(spcr_table_detect);
diff --git a/include/linux/console.h b/include/linux/console.h
index bd19434..94d0bd8 100644
--- a/include/linux/console.h
+++ b/include/linux/console.h
@@ -117,6 +117,7 @@ static inline int con_debug_leave(void)
 #define CON_BRL		(32) /* Used for a braille device */
 #define CON_EXTENDED	(64) /* Use the extended output format a la /dev/kmsg */
 
+struct acpi_table_spcr;
 struct console {
 	char	name[16];
 	void	(*write)(struct console *, const char *, unsigned);
@@ -125,6 +126,7 @@ struct console {
 	void	(*unblank)(void);
 	int	(*setup)(struct console *, char *);
 	int	(*match)(struct console *, char *name, int idx, char *options);
+	int	(*acpi_match)(struct console *, struct acpi_table_spcr *);
 	short	flags;
 	short	index;
 	int	cflag;
@@ -132,6 +134,16 @@ struct console {
 	struct	 console *next;
 };
 
+#ifdef CONFIG_ACPI
+int console_acpi_match(struct console *c, char **options);
+#else
+static inline int console_acpi_match(struct console *c, char **options)
+{
+	return -ENODEV;
+}
+#endif
+void acpi_register_consoles_try_again(void);
+
 /*
  * for_each_console() allows you to iterate on each console
  */
diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index 37e531f..3cf8cba 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -2430,6 +2430,25 @@ static int __init keep_bootcon_setup(char *str)
 
 early_param("keep_bootcon", keep_bootcon_setup);
 
+static DEFINE_MUTEX(acpi_consoles_delayed_mutex);
+static struct console *acpi_consoles_delayed;
+
+void acpi_register_consoles_try_again(void)
+{
+	mutex_lock(&acpi_consoles_delayed_mutex);
+	while (acpi_consoles_delayed) {
+
+		struct console *c = acpi_consoles_delayed;
+
+		acpi_consoles_delayed = acpi_consoles_delayed->next;
+
+		mutex_unlock(&acpi_consoles_delayed_mutex);
+		register_console(c);
+		mutex_lock(&acpi_consoles_delayed_mutex);
+	}
+	mutex_unlock(&acpi_consoles_delayed_mutex);
+}
+
 /*
  * The console driver calls this routine during kernel initialization
  * to register the console printing procedure with printk() and to
@@ -2538,8 +2557,30 @@ void register_console(struct console *newcon)
 		break;
 	}
 
-	if (!(newcon->flags & CON_ENABLED))
-		return;
+	if (!(newcon->flags & CON_ENABLED)) {
+		char *opts;
+		int err;
+
+		if (newcon->index < 0)
+			newcon->index = 0;
+
+		err = console_acpi_match(newcon, &opts);
+
+		if (err == -EAGAIN) {
+			mutex_lock(&acpi_consoles_delayed_mutex);
+			newcon->next = acpi_consoles_delayed;
+			acpi_consoles_delayed = newcon;
+			mutex_unlock(&acpi_consoles_delayed_mutex);
+			return;
+		} else if (err < 0) {
+			return;
+		} else {
+			if (newcon->setup && newcon->setup(newcon, opts) != 0)
+				return;
+			newcon->flags |= CON_ENABLED | CON_CONSDEV;
+			preferred_console = true;
+		}
+	}
 
 	/*
 	 * If we have a bootconsole, and are switching to a real console,
@@ -2612,34 +2653,41 @@ void register_console(struct console *newcon)
 }
 EXPORT_SYMBOL(register_console);
 
+static int delete_from_console_list(struct console **list, struct console *c)
+{
+	while (*list) {
+		struct console *cur = *list;
+
+		if (cur == c) {
+			*list = cur->next;
+			return 0;
+		}
+		list = &cur->next;
+	}
+	return 1;
+}
+
 int unregister_console(struct console *console)
 {
-        struct console *a, *b;
 	int res;
 
 	pr_info("%sconsole [%s%d] disabled\n",
 		(console->flags & CON_BOOT) ? "boot" : "" ,
 		console->name, console->index);
 
+	mutex_lock(&acpi_consoles_delayed_mutex);
+	res = delete_from_console_list(&acpi_consoles_delayed, console);
+	mutex_unlock(&acpi_consoles_delayed_mutex);
+	if (res == 0)
+		return res;
+
 	res = _braille_unregister_console(console);
 	if (res)
 		return res;
 
-	res = 1;
 	console_lock();
-	if (console_drivers == console) {
-		console_drivers=console->next;
-		res = 0;
-	} else if (console_drivers) {
-		for (a=console_drivers->next, b=console_drivers ;
-		     a; b=a, a=b->next) {
-			if (a == console) {
-				b->next = a->next;
-				res = 0;
-				break;
-			}
-		}
-	}
+
+	res = delete_from_console_list(&console_drivers, console);
 
 	if (!res && (console->flags & CON_EXTENDED))
 		nr_ext_console_drivers--;
-- 
2.7.0

[PATCH 3/3] serial: pl011: add acpi_match for amba-pl011.c

[Aleksey Makarov]

Add an implementation of acpi_match() to the pl011 driver.
It allows to check if the console matches one specified with
ACPI SPCR table.

Signed-off-by: Aleksey Makarov <aleksey.makarov@linaro.org>
---
 drivers/tty/serial/amba-pl011.c | 14 ++++++++++++++
 include/acpi/actbl2.h           |  4 ++++
 2 files changed, 18 insertions(+)

diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
index 899a771..3f4aa1b 100644
--- a/drivers/tty/serial/amba-pl011.c
+++ b/drivers/tty/serial/amba-pl011.c
@@ -2189,12 +2189,26 @@ static int __init pl011_console_setup(struct console *co, char *options)
 	return uart_set_options(&uap->port, co, baud, parity, bits, flow);
 }
 
+static int __init pl011_console_acpi_match(struct console *co,
+					   struct acpi_table_spcr *spcr)
+{
+	struct uart_amba_port *uap = amba_ports[co->index];
+
+	if (spcr->interface_type == ACPI_DBG2_ARM_PL011 &&
+	    spcr->serial_port.space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY &&
+	    spcr->serial_port.address == (u64)uap->port.mapbase)
+		return 0;
+
+	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,
+	.acpi_match	= pl011_console_acpi_match,
 	.flags		= CON_PRINTBUFFER,
 	.index		= -1,
 	.data		= &amba_reg,
diff --git a/include/acpi/actbl2.h b/include/acpi/actbl2.h
index 6e28f54..ce4cb37 100644
--- a/include/acpi/actbl2.h
+++ b/include/acpi/actbl2.h
@@ -371,6 +371,10 @@ 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_GENERIC  0x000e
+#define ACPI_DBG2_ARM_DCC           0x000f
+#define ACPI_DBG2_DCM2835           0x0010
 
 #define ACPI_DBG2_1394_STANDARD     0x0000
 
-- 
2.7.0

[PATCH 3/3] serial: pl011: add acpi_match for amba-pl011.c

[Aleksey Makarov]

Add an implementation of acpi_match() to the pl011 driver.
It allows to check if the console matches one specified with
ACPI SPCR table.

Signed-off-by: Aleksey Makarov <aleksey.makarov@linaro.org>
---
 drivers/tty/serial/amba-pl011.c | 14 ++++++++++++++
 include/acpi/actbl2.h           |  4 ++++
 2 files changed, 18 insertions(+)

diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
index 899a771..3f4aa1b 100644
--- a/drivers/tty/serial/amba-pl011.c
+++ b/drivers/tty/serial/amba-pl011.c
@@ -2189,12 +2189,26 @@ static int __init pl011_console_setup(struct console *co, char *options)
 	return uart_set_options(&uap->port, co, baud, parity, bits, flow);
 }
 
+static int __init pl011_console_acpi_match(struct console *co,
+					   struct acpi_table_spcr *spcr)
+{
+	struct uart_amba_port *uap = amba_ports[co->index];
+
+	if (spcr->interface_type == ACPI_DBG2_ARM_PL011 &&
+	    spcr->serial_port.space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY &&
+	    spcr->serial_port.address == (u64)uap->port.mapbase)
+		return 0;
+
+	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,
+	.acpi_match	= pl011_console_acpi_match,
 	.flags		= CON_PRINTBUFFER,
 	.index		= -1,
 	.data		= &amba_reg,
diff --git a/include/acpi/actbl2.h b/include/acpi/actbl2.h
index 6e28f54..ce4cb37 100644
--- a/include/acpi/actbl2.h
+++ b/include/acpi/actbl2.h
@@ -371,6 +371,10 @@ 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_GENERIC  0x000e
+#define ACPI_DBG2_ARM_DCC           0x000f
+#define ACPI_DBG2_DCM2835           0x0010
 
 #define ACPI_DBG2_1394_STANDARD     0x0000
 
-- 
2.7.0

Re: [PATCH 1/3] printk: make preferred_console local static bool

[Joe Perches]

On Mon, 2016-01-25 at 17:45 +0600, Aleksey Makarov wrote:
> The variable preferred_console is used only inside register_console()
> and it's semantics is boolean.  Make it clear.

This loses the index of the preferred console.
I'm not sure this is better.

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

Re: [PATCH 1/3] printk: make preferred_console local static bool

[Joe Perches]

On Mon, 2016-01-25 at 17:45 +0600, Aleksey Makarov wrote:
> The variable preferred_console is used only inside register_console()
> and it's semantics is boolean.  Make it clear.

This loses the index of the preferred console.
I'm not sure this is better.

[PATCH 1/3] printk: make preferred_console local static bool

[Joe Perches]

On Mon, 2016-01-25 at 17:45 +0600, Aleksey Makarov wrote:
> The variable preferred_console is used only inside register_console()
> and it's semantics is boolean.??Make it clear.

This loses the index of the preferred console.
I'm not sure this is better.

Re: [PATCH 1/3] printk: make preferred_console local static bool

[Aleksey Makarov]

On 25.01.2016 18:45, Joe Perches wrote:
> On Mon, 2016-01-25 at 17:45 +0600, Aleksey Makarov wrote:
>> The variable preferred_console is used only inside register_console()
>> and it's semantics is boolean.  Make it clear.
>
> This loses the index of the preferred console.
> I'm not sure this is better.

That index is not used anywhere.  I believe the patch makes things clear.

Also, it indexes the array console_cmdline.  With introduction of the 
ACPI-selected console it does not have any sense and I would have to 
change it anyway.

Thank you
Aleksey Makarov

[PATCH 1/3] printk: make preferred_console local static bool

[Aleksey Makarov]

On 25.01.2016 18:45, Joe Perches wrote:
> On Mon, 2016-01-25 at 17:45 +0600, Aleksey Makarov wrote:
>> The variable preferred_console is used only inside register_console()
>> and it's semantics is boolean.  Make it clear.
>
> This loses the index of the preferred console.
> I'm not sure this is better.

That index is not used anywhere.  I believe the patch makes things clear.

Also, it indexes the array console_cmdline.  With introduction of the 
ACPI-selected console it does not have any sense and I would have to 
change it anyway.

Thank you
Aleksey Makarov

Re: [PATCH 1/3] printk: make preferred_console local static bool

[Joe Perches]

On Mon, 2016-01-25 at 18:51 +0600, Aleksey Makarov wrote:
> 
> On 25.01.2016 18:45, Joe Perches wrote:
> > On Mon, 2016-01-25 at 17:45 +0600, Aleksey Makarov wrote:
> > > The variable preferred_console is used only inside register_console()
> > > and it's semantics is boolean.  Make it clear.
> > 
> > This loses the index of the preferred console.
> > I'm not sure this is better.
> 
> That index is not used anywhere.  I believe the patch makes things clear.

Well, with this change the name and its use
is a bit misleading.  Maybe changing it to
something like use_selected_console is better.

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

Re: [PATCH 1/3] printk: make preferred_console local static bool

[Joe Perches]

On Mon, 2016-01-25 at 18:51 +0600, Aleksey Makarov wrote:
> 
> On 25.01.2016 18:45, Joe Perches wrote:
> > On Mon, 2016-01-25 at 17:45 +0600, Aleksey Makarov wrote:
> > > The variable preferred_console is used only inside register_console()
> > > and it's semantics is boolean.  Make it clear.
> > 
> > This loses the index of the preferred console.
> > I'm not sure this is better.
> 
> That index is not used anywhere.  I believe the patch makes things clear.

Well, with this change the name and its use
is a bit misleading.  Maybe changing it to
something like use_selected_console is better.

[PATCH 1/3] printk: make preferred_console local static bool

[Joe Perches]

On Mon, 2016-01-25 at 18:51 +0600, Aleksey Makarov wrote:
> 
> On 25.01.2016 18:45, Joe Perches wrote:
> > On Mon, 2016-01-25 at 17:45 +0600, Aleksey Makarov wrote:
> > > The variable preferred_console is used only inside register_console()
> > > and it's semantics is boolean.??Make it clear.
> > 
> > This loses the index of the preferred console.
> > I'm not sure this is better.
> 
> That index is not used anywhere.??I believe the patch makes things clear.

Well, with this change the name and its use
is a bit misleading. ?Maybe changing it to
something like use_selected_console is better.

Re: [PATCH 1/3] printk: make preferred_console local static bool

[Aleksey Makarov]

On 25.01.2016 19:23, Joe Perches wrote:
> On Mon, 2016-01-25 at 18:51 +0600, Aleksey Makarov wrote:
>>
>> On 25.01.2016 18:45, Joe Perches wrote:
>>> On Mon, 2016-01-25 at 17:45 +0600, Aleksey Makarov wrote:
>>>> The variable preferred_console is used only inside register_console()
>>>> and it's semantics is boolean.  Make it clear.
>>>
>>> This loses the index of the preferred console.
>>> I'm not sure this is better.
>>
>> That index is not used anywhere.  I believe the patch makes things clear.
>
> Well, with this change the name and its use
> is a bit misleading.  Maybe changing it to
> something like use_selected_console is better.

Thank you. I will fix this in the next version.

[PATCH 1/3] printk: make preferred_console local static bool

[Aleksey Makarov]

On 25.01.2016 19:23, Joe Perches wrote:
> On Mon, 2016-01-25 at 18:51 +0600, Aleksey Makarov wrote:
>>
>> On 25.01.2016 18:45, Joe Perches wrote:
>>> On Mon, 2016-01-25 at 17:45 +0600, Aleksey Makarov wrote:
>>>> The variable preferred_console is used only inside register_console()
>>>> and it's semantics is boolean.  Make it clear.
>>>
>>> This loses the index of the preferred console.
>>> I'm not sure this is better.
>>
>> That index is not used anywhere.  I believe the patch makes things clear.
>
> Well, with this change the name and its use
> is a bit misleading.  Maybe changing it to
> something like use_selected_console is better.

Thank you. I will fix this in the next version.

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

[Andy Shevchenko]

On Mon, Jan 25, 2016 at 1:45 PM, Aleksey Makarov
<aleksey.makarov@linaro.org> wrote:
> 'ARM Server Base Boot Requiremets' [1] mention SPCR
> (Serial Port Console Redirection Table) [2] as a mandatory ACPI table
> that specifies the configuration of serial console.
>
> Parse this table and check if any registered console match
> the description.  If it does, enable that console.
>
> To implement that, introduce a new member
> int (*acpi_match)(struct console *, struct acpi_table_spcr *)
> of struct console.  It allows drivers to check if they provide
> a matching console device.
>
> [1] http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.den0044a/index.html
> [2] http://msdn.microsoft.com/en-us/library/windows/hardware/dn639131(v=vs.85).aspx

Can you split this to several patches?
I see preparatory patch in console code, i.e.
delete_from_console_list(), adding SPCR support to ACPI, enabling it.

>
> Signed-off-by: Aleksey Makarov <aleksey.makarov@linaro.org>
> ---
>  arch/arm64/Kconfig      |  1 +
>  drivers/acpi/Kconfig    |  3 ++
>  drivers/acpi/Makefile   |  1 +
>  drivers/acpi/spcr.c     | 85 +++++++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/console.h | 12 +++++++
>  kernel/printk/printk.c  | 82 +++++++++++++++++++++++++++++++++++++----------
>  6 files changed, 167 insertions(+), 17 deletions(-)
>  create mode 100644 drivers/acpi/spcr.c
>
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 573bebc..bf31e3c 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_ATOMIC64_DEC_IF_POSITIVE
>         select ARCH_HAS_ELF_RANDOMIZE
>         select ARCH_HAS_GCOV_PROFILE_ALL
> diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
> index e315061..142a338 100644
> --- a/drivers/acpi/Kconfig
> +++ b/drivers/acpi/Kconfig
> @@ -60,6 +60,9 @@ config ACPI_CCA_REQUIRED
>  config IORT_TABLE
>         bool
>
> +config ACPI_SPCR_TABLE
> +       bool
> +
>  config ACPI_DEBUGGER
>         bool "AML debugger interface (EXPERIMENTAL)"
>         select ACPI_DEBUG
> diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
> index 265eb90..8316859 100644
> --- a/drivers/acpi/Makefile
> +++ b/drivers/acpi/Makefile
> @@ -81,6 +81,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_IORT_TABLE)       += iort.o
> +obj-$(CONFIG_ACPI_SPCR_TABLE)  += spcr.o
>
>  # processor has its own "processor." module_param namespace
>  processor-y                    := processor_driver.o
> diff --git a/drivers/acpi/spcr.c b/drivers/acpi/spcr.c
> new file mode 100644
> index 0000000..ccb19a0
> --- /dev/null
> +++ b/drivers/acpi/spcr.c
> @@ -0,0 +1,85 @@
> +/*
> + * 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>
> +
> +static struct acpi_table_spcr *spcr_table;
> +
> +int console_acpi_match(struct console *c, char **options)
> +{
> +       int err;
> +
> +       if (!c->acpi_match)
> +               return -ENODEV;
> +
> +       if (!spcr_table)
> +               return -EAGAIN;
> +
> +       err = c->acpi_match(c, spcr_table);
> +       if (err < 0)
> +               return err;
> +
> +       if (options) {
> +               switch (spcr_table->baud_rate) {
> +               case 3:
> +                       *options = "9600";
> +                       break;
> +               case 4:
> +                       *options = "19200";
> +                       break;
> +               case 6:
> +                       *options = "57600";
> +                       break;
> +               case 7:
> +                       *options = "115200";
> +                       break;
> +               default:
> +                       *options = "";
> +                       break;
> +               }
> +       }
> +
> +       return err;
> +}
> +
> +static int __init spcr_table_detect(void)
> +{
> +       struct acpi_table_header *table;
> +       acpi_status status;
> +
> +       if (acpi_disabled)
> +               return -ENODEV;
> +
> +       status = acpi_get_table(ACPI_SIG_SPCR, 0, &table);
> +       if (ACPI_FAILURE(status)) {
> +               const char *msg = acpi_format_exception(status);
> +
> +               pr_err("Failed to get table, %s\n", msg);
> +               return -EINVAL;
> +       }
> +
> +       if (table->revision < 2)
> +               return -EOPNOTSUPP;
> +
> +       spcr_table = (struct acpi_table_spcr *)table;
> +
> +       pr_info("Console at 0x%016llx\n", spcr_table->serial_port.address);
> +
> +       acpi_register_consoles_try_again();
> +
> +       return 0;
> +}
> +
> +arch_initcall(spcr_table_detect);
> diff --git a/include/linux/console.h b/include/linux/console.h
> index bd19434..94d0bd8 100644
> --- a/include/linux/console.h
> +++ b/include/linux/console.h
> @@ -117,6 +117,7 @@ static inline int con_debug_leave(void)
>  #define CON_BRL                (32) /* Used for a braille device */
>  #define CON_EXTENDED   (64) /* Use the extended output format a la /dev/kmsg */
>
> +struct acpi_table_spcr;
>  struct console {
>         char    name[16];
>         void    (*write)(struct console *, const char *, unsigned);
> @@ -125,6 +126,7 @@ struct console {
>         void    (*unblank)(void);
>         int     (*setup)(struct console *, char *);
>         int     (*match)(struct console *, char *name, int idx, char *options);
> +       int     (*acpi_match)(struct console *, struct acpi_table_spcr *);
>         short   flags;
>         short   index;
>         int     cflag;
> @@ -132,6 +134,16 @@ struct console {
>         struct   console *next;
>  };
>
> +#ifdef CONFIG_ACPI
> +int console_acpi_match(struct console *c, char **options);
> +#else
> +static inline int console_acpi_match(struct console *c, char **options)
> +{
> +       return -ENODEV;
> +}
> +#endif
> +void acpi_register_consoles_try_again(void);
> +
>  /*
>   * for_each_console() allows you to iterate on each console
>   */
> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> index 37e531f..3cf8cba 100644
> --- a/kernel/printk/printk.c
> +++ b/kernel/printk/printk.c
> @@ -2430,6 +2430,25 @@ static int __init keep_bootcon_setup(char *str)
>
>  early_param("keep_bootcon", keep_bootcon_setup);
>
> +static DEFINE_MUTEX(acpi_consoles_delayed_mutex);
> +static struct console *acpi_consoles_delayed;
> +
> +void acpi_register_consoles_try_again(void)
> +{
> +       mutex_lock(&acpi_consoles_delayed_mutex);
> +       while (acpi_consoles_delayed) {
> +
> +               struct console *c = acpi_consoles_delayed;
> +
> +               acpi_consoles_delayed = acpi_consoles_delayed->next;
> +
> +               mutex_unlock(&acpi_consoles_delayed_mutex);
> +               register_console(c);
> +               mutex_lock(&acpi_consoles_delayed_mutex);
> +       }
> +       mutex_unlock(&acpi_consoles_delayed_mutex);
> +}
> +
>  /*
>   * The console driver calls this routine during kernel initialization
>   * to register the console printing procedure with printk() and to
> @@ -2538,8 +2557,30 @@ void register_console(struct console *newcon)
>                 break;
>         }
>
> -       if (!(newcon->flags & CON_ENABLED))
> -               return;
> +       if (!(newcon->flags & CON_ENABLED)) {
> +               char *opts;
> +               int err;
> +
> +               if (newcon->index < 0)
> +                       newcon->index = 0;
> +
> +               err = console_acpi_match(newcon, &opts);
> +
> +               if (err == -EAGAIN) {
> +                       mutex_lock(&acpi_consoles_delayed_mutex);
> +                       newcon->next = acpi_consoles_delayed;
> +                       acpi_consoles_delayed = newcon;
> +                       mutex_unlock(&acpi_consoles_delayed_mutex);
> +                       return;
> +               } else if (err < 0) {
> +                       return;
> +               } else {
> +                       if (newcon->setup && newcon->setup(newcon, opts) != 0)
> +                               return;
> +                       newcon->flags |= CON_ENABLED | CON_CONSDEV;
> +                       preferred_console = true;
> +               }
> +       }
>
>         /*
>          * If we have a bootconsole, and are switching to a real console,
> @@ -2612,34 +2653,41 @@ void register_console(struct console *newcon)
>  }
>  EXPORT_SYMBOL(register_console);
>
> +static int delete_from_console_list(struct console **list, struct console *c)
> +{
> +       while (*list) {
> +               struct console *cur = *list;
> +
> +               if (cur == c) {
> +                       *list = cur->next;
> +                       return 0;
> +               }
> +               list = &cur->next;
> +       }
> +       return 1;
> +}
> +
>  int unregister_console(struct console *console)
>  {
> -        struct console *a, *b;
>         int res;
>
>         pr_info("%sconsole [%s%d] disabled\n",
>                 (console->flags & CON_BOOT) ? "boot" : "" ,
>                 console->name, console->index);
>
> +       mutex_lock(&acpi_consoles_delayed_mutex);
> +       res = delete_from_console_list(&acpi_consoles_delayed, console);
> +       mutex_unlock(&acpi_consoles_delayed_mutex);
> +       if (res == 0)
> +               return res;
> +
>         res = _braille_unregister_console(console);
>         if (res)
>                 return res;
>
> -       res = 1;
>         console_lock();
> -       if (console_drivers == console) {
> -               console_drivers=console->next;
> -               res = 0;
> -       } else if (console_drivers) {
> -               for (a=console_drivers->next, b=console_drivers ;
> -                    a; b=a, a=b->next) {
> -                       if (a == console) {
> -                               b->next = a->next;
> -                               res = 0;
> -                               break;
> -                       }
> -               }
> -       }
> +
> +       res = delete_from_console_list(&console_drivers, console);
>
>         if (!res && (console->flags & CON_EXTENDED))
>                 nr_ext_console_drivers--;
> --
> 2.7.0
>



-- 
With Best Regards,
Andy Shevchenko

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

[Andy Shevchenko]

On Mon, Jan 25, 2016 at 1:45 PM, Aleksey Makarov
<aleksey.makarov@linaro.org> wrote:
> 'ARM Server Base Boot Requiremets' [1] mention SPCR
> (Serial Port Console Redirection Table) [2] as a mandatory ACPI table
> that specifies the configuration of serial console.
>
> Parse this table and check if any registered console match
> the description.  If it does, enable that console.
>
> To implement that, introduce a new member
> int (*acpi_match)(struct console *, struct acpi_table_spcr *)
> of struct console.  It allows drivers to check if they provide
> a matching console device.
>
> [1] http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.den0044a/index.html
> [2] http://msdn.microsoft.com/en-us/library/windows/hardware/dn639131(v=vs.85).aspx

Can you split this to several patches?
I see preparatory patch in console code, i.e.
delete_from_console_list(), adding SPCR support to ACPI, enabling it.

>
> Signed-off-by: Aleksey Makarov <aleksey.makarov@linaro.org>
> ---
>  arch/arm64/Kconfig      |  1 +
>  drivers/acpi/Kconfig    |  3 ++
>  drivers/acpi/Makefile   |  1 +
>  drivers/acpi/spcr.c     | 85 +++++++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/console.h | 12 +++++++
>  kernel/printk/printk.c  | 82 +++++++++++++++++++++++++++++++++++++----------
>  6 files changed, 167 insertions(+), 17 deletions(-)
>  create mode 100644 drivers/acpi/spcr.c
>
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 573bebc..bf31e3c 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_ATOMIC64_DEC_IF_POSITIVE
>         select ARCH_HAS_ELF_RANDOMIZE
>         select ARCH_HAS_GCOV_PROFILE_ALL
> diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
> index e315061..142a338 100644
> --- a/drivers/acpi/Kconfig
> +++ b/drivers/acpi/Kconfig
> @@ -60,6 +60,9 @@ config ACPI_CCA_REQUIRED
>  config IORT_TABLE
>         bool
>
> +config ACPI_SPCR_TABLE
> +       bool
> +
>  config ACPI_DEBUGGER
>         bool "AML debugger interface (EXPERIMENTAL)"
>         select ACPI_DEBUG
> diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
> index 265eb90..8316859 100644
> --- a/drivers/acpi/Makefile
> +++ b/drivers/acpi/Makefile
> @@ -81,6 +81,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_IORT_TABLE)       += iort.o
> +obj-$(CONFIG_ACPI_SPCR_TABLE)  += spcr.o
>
>  # processor has its own "processor." module_param namespace
>  processor-y                    := processor_driver.o
> diff --git a/drivers/acpi/spcr.c b/drivers/acpi/spcr.c
> new file mode 100644
> index 0000000..ccb19a0
> --- /dev/null
> +++ b/drivers/acpi/spcr.c
> @@ -0,0 +1,85 @@
> +/*
> + * 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>
> +
> +static struct acpi_table_spcr *spcr_table;
> +
> +int console_acpi_match(struct console *c, char **options)
> +{
> +       int err;
> +
> +       if (!c->acpi_match)
> +               return -ENODEV;
> +
> +       if (!spcr_table)
> +               return -EAGAIN;
> +
> +       err = c->acpi_match(c, spcr_table);
> +       if (err < 0)
> +               return err;
> +
> +       if (options) {
> +               switch (spcr_table->baud_rate) {
> +               case 3:
> +                       *options = "9600";
> +                       break;
> +               case 4:
> +                       *options = "19200";
> +                       break;
> +               case 6:
> +                       *options = "57600";
> +                       break;
> +               case 7:
> +                       *options = "115200";
> +                       break;
> +               default:
> +                       *options = "";
> +                       break;
> +               }
> +       }
> +
> +       return err;
> +}
> +
> +static int __init spcr_table_detect(void)
> +{
> +       struct acpi_table_header *table;
> +       acpi_status status;
> +
> +       if (acpi_disabled)
> +               return -ENODEV;
> +
> +       status = acpi_get_table(ACPI_SIG_SPCR, 0, &table);
> +       if (ACPI_FAILURE(status)) {
> +               const char *msg = acpi_format_exception(status);
> +
> +               pr_err("Failed to get table, %s\n", msg);
> +               return -EINVAL;
> +       }
> +
> +       if (table->revision < 2)
> +               return -EOPNOTSUPP;
> +
> +       spcr_table = (struct acpi_table_spcr *)table;
> +
> +       pr_info("Console at 0x%016llx\n", spcr_table->serial_port.address);
> +
> +       acpi_register_consoles_try_again();
> +
> +       return 0;
> +}
> +
> +arch_initcall(spcr_table_detect);
> diff --git a/include/linux/console.h b/include/linux/console.h
> index bd19434..94d0bd8 100644
> --- a/include/linux/console.h
> +++ b/include/linux/console.h
> @@ -117,6 +117,7 @@ static inline int con_debug_leave(void)
>  #define CON_BRL                (32) /* Used for a braille device */
>  #define CON_EXTENDED   (64) /* Use the extended output format a la /dev/kmsg */
>
> +struct acpi_table_spcr;
>  struct console {
>         char    name[16];
>         void    (*write)(struct console *, const char *, unsigned);
> @@ -125,6 +126,7 @@ struct console {
>         void    (*unblank)(void);
>         int     (*setup)(struct console *, char *);
>         int     (*match)(struct console *, char *name, int idx, char *options);
> +       int     (*acpi_match)(struct console *, struct acpi_table_spcr *);
>         short   flags;
>         short   index;
>         int     cflag;
> @@ -132,6 +134,16 @@ struct console {
>         struct   console *next;
>  };
>
> +#ifdef CONFIG_ACPI
> +int console_acpi_match(struct console *c, char **options);
> +#else
> +static inline int console_acpi_match(struct console *c, char **options)
> +{
> +       return -ENODEV;
> +}
> +#endif
> +void acpi_register_consoles_try_again(void);
> +
>  /*
>   * for_each_console() allows you to iterate on each console
>   */
> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> index 37e531f..3cf8cba 100644
> --- a/kernel/printk/printk.c
> +++ b/kernel/printk/printk.c
> @@ -2430,6 +2430,25 @@ static int __init keep_bootcon_setup(char *str)
>
>  early_param("keep_bootcon", keep_bootcon_setup);
>
> +static DEFINE_MUTEX(acpi_consoles_delayed_mutex);
> +static struct console *acpi_consoles_delayed;
> +
> +void acpi_register_consoles_try_again(void)
> +{
> +       mutex_lock(&acpi_consoles_delayed_mutex);
> +       while (acpi_consoles_delayed) {
> +
> +               struct console *c = acpi_consoles_delayed;
> +
> +               acpi_consoles_delayed = acpi_consoles_delayed->next;
> +
> +               mutex_unlock(&acpi_consoles_delayed_mutex);
> +               register_console(c);
> +               mutex_lock(&acpi_consoles_delayed_mutex);
> +       }
> +       mutex_unlock(&acpi_consoles_delayed_mutex);
> +}
> +
>  /*
>   * The console driver calls this routine during kernel initialization
>   * to register the console printing procedure with printk() and to
> @@ -2538,8 +2557,30 @@ void register_console(struct console *newcon)
>                 break;
>         }
>
> -       if (!(newcon->flags & CON_ENABLED))
> -               return;
> +       if (!(newcon->flags & CON_ENABLED)) {
> +               char *opts;
> +               int err;
> +
> +               if (newcon->index < 0)
> +                       newcon->index = 0;
> +
> +               err = console_acpi_match(newcon, &opts);
> +
> +               if (err == -EAGAIN) {
> +                       mutex_lock(&acpi_consoles_delayed_mutex);
> +                       newcon->next = acpi_consoles_delayed;
> +                       acpi_consoles_delayed = newcon;
> +                       mutex_unlock(&acpi_consoles_delayed_mutex);
> +                       return;
> +               } else if (err < 0) {
> +                       return;
> +               } else {
> +                       if (newcon->setup && newcon->setup(newcon, opts) != 0)
> +                               return;
> +                       newcon->flags |= CON_ENABLED | CON_CONSDEV;
> +                       preferred_console = true;
> +               }
> +       }
>
>         /*
>          * If we have a bootconsole, and are switching to a real console,
> @@ -2612,34 +2653,41 @@ void register_console(struct console *newcon)
>  }
>  EXPORT_SYMBOL(register_console);
>
> +static int delete_from_console_list(struct console **list, struct console *c)
> +{
> +       while (*list) {
> +               struct console *cur = *list;
> +
> +               if (cur == c) {
> +                       *list = cur->next;
> +                       return 0;
> +               }
> +               list = &cur->next;
> +       }
> +       return 1;
> +}
> +
>  int unregister_console(struct console *console)
>  {
> -        struct console *a, *b;
>         int res;
>
>         pr_info("%sconsole [%s%d] disabled\n",
>                 (console->flags & CON_BOOT) ? "boot" : "" ,
>                 console->name, console->index);
>
> +       mutex_lock(&acpi_consoles_delayed_mutex);
> +       res = delete_from_console_list(&acpi_consoles_delayed, console);
> +       mutex_unlock(&acpi_consoles_delayed_mutex);
> +       if (res == 0)
> +               return res;
> +
>         res = _braille_unregister_console(console);
>         if (res)
>                 return res;
>
> -       res = 1;
>         console_lock();
> -       if (console_drivers == console) {
> -               console_drivers=console->next;
> -               res = 0;
> -       } else if (console_drivers) {
> -               for (a=console_drivers->next, b=console_drivers ;
> -                    a; b=a, a=b->next) {
> -                       if (a == console) {
> -                               b->next = a->next;
> -                               res = 0;
> -                               break;
> -                       }
> -               }
> -       }
> +
> +       res = delete_from_console_list(&console_drivers, console);
>
>         if (!res && (console->flags & CON_EXTENDED))
>                 nr_ext_console_drivers--;
> --
> 2.7.0
>



-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 3/3] serial: pl011: add acpi_match for amba-pl011.c

[Andy Shevchenko]

On Mon, Jan 25, 2016 at 1:45 PM, Aleksey Makarov
<aleksey.makarov@linaro.org> wrote:
> Add an implementation of acpi_match() to the pl011 driver.
> It allows to check if the console matches one specified with
> ACPI SPCR table.

I don't know Rafael's opinion on this, but I would split it to extend
ACPI header first with reference to newest revision of Microsoft
document.

>
> Signed-off-by: Aleksey Makarov <aleksey.makarov@linaro.org>
> ---
>  drivers/tty/serial/amba-pl011.c | 14 ++++++++++++++
>  include/acpi/actbl2.h           |  4 ++++
>  2 files changed, 18 insertions(+)
>
> diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
> index 899a771..3f4aa1b 100644
> --- a/drivers/tty/serial/amba-pl011.c
> +++ b/drivers/tty/serial/amba-pl011.c
> @@ -2189,12 +2189,26 @@ static int __init pl011_console_setup(struct console *co, char *options)
>         return uart_set_options(&uap->port, co, baud, parity, bits, flow);
>  }
>
> +static int __init pl011_console_acpi_match(struct console *co,
> +                                          struct acpi_table_spcr *spcr)
> +{
> +       struct uart_amba_port *uap = amba_ports[co->index];
> +
> +       if (spcr->interface_type == ACPI_DBG2_ARM_PL011 &&
> +           spcr->serial_port.space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY &&
> +           spcr->serial_port.address == (u64)uap->port.mapbase)
> +               return 0;
> +
> +       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,
> +       .acpi_match     = pl011_console_acpi_match,
>         .flags          = CON_PRINTBUFFER,
>         .index          = -1,
>         .data           = &amba_reg,
> diff --git a/include/acpi/actbl2.h b/include/acpi/actbl2.h
> index 6e28f54..ce4cb37 100644
> --- a/include/acpi/actbl2.h
> +++ b/include/acpi/actbl2.h
> @@ -371,6 +371,10 @@ 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_GENERIC  0x000e
> +#define ACPI_DBG2_ARM_DCC           0x000f
> +#define ACPI_DBG2_DCM2835           0x0010
>
>  #define ACPI_DBG2_1394_STANDARD     0x0000
>
> --
> 2.7.0
>



-- 
With Best Regards,
Andy Shevchenko

[PATCH 3/3] serial: pl011: add acpi_match for amba-pl011.c

[Andy Shevchenko]

On Mon, Jan 25, 2016 at 1:45 PM, Aleksey Makarov
<aleksey.makarov@linaro.org> wrote:
> Add an implementation of acpi_match() to the pl011 driver.
> It allows to check if the console matches one specified with
> ACPI SPCR table.

I don't know Rafael's opinion on this, but I would split it to extend
ACPI header first with reference to newest revision of Microsoft
document.

>
> Signed-off-by: Aleksey Makarov <aleksey.makarov@linaro.org>
> ---
>  drivers/tty/serial/amba-pl011.c | 14 ++++++++++++++
>  include/acpi/actbl2.h           |  4 ++++
>  2 files changed, 18 insertions(+)
>
> diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
> index 899a771..3f4aa1b 100644
> --- a/drivers/tty/serial/amba-pl011.c
> +++ b/drivers/tty/serial/amba-pl011.c
> @@ -2189,12 +2189,26 @@ static int __init pl011_console_setup(struct console *co, char *options)
>         return uart_set_options(&uap->port, co, baud, parity, bits, flow);
>  }
>
> +static int __init pl011_console_acpi_match(struct console *co,
> +                                          struct acpi_table_spcr *spcr)
> +{
> +       struct uart_amba_port *uap = amba_ports[co->index];
> +
> +       if (spcr->interface_type == ACPI_DBG2_ARM_PL011 &&
> +           spcr->serial_port.space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY &&
> +           spcr->serial_port.address == (u64)uap->port.mapbase)
> +               return 0;
> +
> +       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,
> +       .acpi_match     = pl011_console_acpi_match,
>         .flags          = CON_PRINTBUFFER,
>         .index          = -1,
>         .data           = &amba_reg,
> diff --git a/include/acpi/actbl2.h b/include/acpi/actbl2.h
> index 6e28f54..ce4cb37 100644
> --- a/include/acpi/actbl2.h
> +++ b/include/acpi/actbl2.h
> @@ -371,6 +371,10 @@ 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_GENERIC  0x000e
> +#define ACPI_DBG2_ARM_DCC           0x000f
> +#define ACPI_DBG2_DCM2835           0x0010
>
>  #define ACPI_DBG2_1394_STANDARD     0x0000
>
> --
> 2.7.0
>



-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 3/3] serial: pl011: add acpi_match for amba-pl011.c

[Andy Shevchenko]

On Mon, Jan 25, 2016 at 4:21 PM, Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Mon, Jan 25, 2016 at 1:45 PM, Aleksey Makarov
> <aleksey.makarov@linaro.org> wrote:
>> Add an implementation of acpi_match() to the pl011 driver.
>> It allows to check if the console matches one specified with
>> ACPI SPCR table.
>
> I don't know Rafael's opinion on this, but I would split it to extend
> ACPI header first with reference to newest revision of Microsoft
> document.


>> --- a/include/acpi/actbl2.h
>> +++ b/include/acpi/actbl2.h
>> @@ -371,6 +371,10 @@ 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_GENERIC  0x000e
>> +#define ACPI_DBG2_ARM_DCC           0x000f
>> +#define ACPI_DBG2_DCM2835           0x0010

^^^ Exactly because of such typos. Should be BCM


-- 
With Best Regards,
Andy Shevchenko

[PATCH 3/3] serial: pl011: add acpi_match for amba-pl011.c

[Andy Shevchenko]

On Mon, Jan 25, 2016 at 4:21 PM, Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Mon, Jan 25, 2016 at 1:45 PM, Aleksey Makarov
> <aleksey.makarov@linaro.org> wrote:
>> Add an implementation of acpi_match() to the pl011 driver.
>> It allows to check if the console matches one specified with
>> ACPI SPCR table.
>
> I don't know Rafael's opinion on this, but I would split it to extend
> ACPI header first with reference to newest revision of Microsoft
> document.


>> --- a/include/acpi/actbl2.h
>> +++ b/include/acpi/actbl2.h
>> @@ -371,6 +371,10 @@ 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_GENERIC  0x000e
>> +#define ACPI_DBG2_ARM_DCC           0x000f
>> +#define ACPI_DBG2_DCM2835           0x0010

^^^ Exactly because of such typos. Should be BCM


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 1/3] printk: make preferred_console local static bool

[Andy Shevchenko]

On Mon, Jan 25, 2016 at 1:45 PM, Aleksey Makarov
<aleksey.makarov@linaro.org> wrote:
> The variable preferred_console is used only inside register_console()
> and it's semantics is boolean.  Make it clear.

However the patch looks okay it brings imbalance to understanding how
exactly the preferred console is chosen.
Even in case of restricted usage I would leave things as is for now.

>
> Signed-off-by: Aleksey Makarov <aleksey.makarov@linaro.org>
> ---
>  kernel/printk/printk.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> index 2ce8826..37e531f 100644
> --- a/kernel/printk/printk.c
> +++ b/kernel/printk/printk.c
> @@ -143,7 +143,6 @@ static struct console *exclusive_console;
>  static struct console_cmdline console_cmdline[MAX_CMDLINECONSOLES];
>
>  static int selected_console = -1;
> -static int preferred_console = -1;
>  int console_set_on_cmdline;
>  EXPORT_SYMBOL(console_set_on_cmdline);
>
> @@ -2456,6 +2455,7 @@ void register_console(struct console *newcon)
>         unsigned long flags;
>         struct console *bcon = NULL;
>         struct console_cmdline *c;
> +       static bool preferred_console;
>
>         if (console_drivers)
>                 for_each_console(bcon)
> @@ -2482,15 +2482,15 @@ void register_console(struct console *newcon)
>         if (console_drivers && console_drivers->flags & CON_BOOT)
>                 bcon = console_drivers;
>
> -       if (preferred_console < 0 || bcon || !console_drivers)
> -               preferred_console = selected_console;
> +       if (!preferred_console || bcon || !console_drivers)
> +               preferred_console = selected_console >= 0;
>
>         /*
>          *      See if we want to use this console driver. If we
>          *      didn't select a console we take the first one
>          *      that registers here.
>          */
> -       if (preferred_console < 0) {
> +       if (!preferred_console) {
>                 if (newcon->index < 0)
>                         newcon->index = 0;
>                 if (newcon->setup == NULL ||
> @@ -2498,7 +2498,7 @@ void register_console(struct console *newcon)
>                         newcon->flags |= CON_ENABLED;
>                         if (newcon->device) {
>                                 newcon->flags |= CON_CONSDEV;
> -                               preferred_console = 0;
> +                               preferred_console = true;
>                         }
>                 }
>         }
> @@ -2533,7 +2533,7 @@ void register_console(struct console *newcon)
>                 newcon->flags |= CON_ENABLED;
>                 if (i == selected_console) {
>                         newcon->flags |= CON_CONSDEV;
> -                       preferred_console = selected_console;
> +                       preferred_console = true;
>                 }
>                 break;
>         }
> --
> 2.7.0
>



-- 
With Best Regards,
Andy Shevchenko

[PATCH 1/3] printk: make preferred_console local static bool

[Andy Shevchenko]

On Mon, Jan 25, 2016 at 1:45 PM, Aleksey Makarov
<aleksey.makarov@linaro.org> wrote:
> The variable preferred_console is used only inside register_console()
> and it's semantics is boolean.  Make it clear.

However the patch looks okay it brings imbalance to understanding how
exactly the preferred console is chosen.
Even in case of restricted usage I would leave things as is for now.

>
> Signed-off-by: Aleksey Makarov <aleksey.makarov@linaro.org>
> ---
>  kernel/printk/printk.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> index 2ce8826..37e531f 100644
> --- a/kernel/printk/printk.c
> +++ b/kernel/printk/printk.c
> @@ -143,7 +143,6 @@ static struct console *exclusive_console;
>  static struct console_cmdline console_cmdline[MAX_CMDLINECONSOLES];
>
>  static int selected_console = -1;
> -static int preferred_console = -1;
>  int console_set_on_cmdline;
>  EXPORT_SYMBOL(console_set_on_cmdline);
>
> @@ -2456,6 +2455,7 @@ void register_console(struct console *newcon)
>         unsigned long flags;
>         struct console *bcon = NULL;
>         struct console_cmdline *c;
> +       static bool preferred_console;
>
>         if (console_drivers)
>                 for_each_console(bcon)
> @@ -2482,15 +2482,15 @@ void register_console(struct console *newcon)
>         if (console_drivers && console_drivers->flags & CON_BOOT)
>                 bcon = console_drivers;
>
> -       if (preferred_console < 0 || bcon || !console_drivers)
> -               preferred_console = selected_console;
> +       if (!preferred_console || bcon || !console_drivers)
> +               preferred_console = selected_console >= 0;
>
>         /*
>          *      See if we want to use this console driver. If we
>          *      didn't select a console we take the first one
>          *      that registers here.
>          */
> -       if (preferred_console < 0) {
> +       if (!preferred_console) {
>                 if (newcon->index < 0)
>                         newcon->index = 0;
>                 if (newcon->setup == NULL ||
> @@ -2498,7 +2498,7 @@ void register_console(struct console *newcon)
>                         newcon->flags |= CON_ENABLED;
>                         if (newcon->device) {
>                                 newcon->flags |= CON_CONSDEV;
> -                               preferred_console = 0;
> +                               preferred_console = true;
>                         }
>                 }
>         }
> @@ -2533,7 +2533,7 @@ void register_console(struct console *newcon)
>                 newcon->flags |= CON_ENABLED;
>                 if (i == selected_console) {
>                         newcon->flags |= CON_CONSDEV;
> -                       preferred_console = selected_console;
> +                       preferred_console = true;
>                 }
>                 break;
>         }
> --
> 2.7.0
>



-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 1/3] printk: make preferred_console local static bool

[Aleksey Makarov]

On 25.01.2016 20:24, Andy Shevchenko wrote:
> On Mon, Jan 25, 2016 at 1:45 PM, Aleksey Makarov
> <aleksey.makarov@linaro.org> wrote:
>> The variable preferred_console is used only inside register_console()
>> and it's semantics is boolean.  Make it clear.
>
> However the patch looks okay it brings imbalance to understanding how
> exactly the preferred console is chosen.

On the contrary, I would say it makes things more clear.

Also please consider this patch preparatory for the further changes.
See my replies to Joe Perches.

[PATCH 1/3] printk: make preferred_console local static bool

[Aleksey Makarov]

On 25.01.2016 20:24, Andy Shevchenko wrote:
> On Mon, Jan 25, 2016 at 1:45 PM, Aleksey Makarov
> <aleksey.makarov@linaro.org> wrote:
>> The variable preferred_console is used only inside register_console()
>> and it's semantics is boolean.  Make it clear.
>
> However the patch looks okay it brings imbalance to understanding how
> exactly the preferred console is chosen.

On the contrary, I would say it makes things more clear.

Also please consider this patch preparatory for the further changes.
See my replies to Joe Perches.

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

[Aleksey Makarov]

On 25.01.2016 20:14, Andy Shevchenko wrote:
> On Mon, Jan 25, 2016 at 1:45 PM, Aleksey Makarov
> <aleksey.makarov@linaro.org> wrote:
>> 'ARM Server Base Boot Requiremets' [1] mention SPCR
>> (Serial Port Console Redirection Table) [2] as a mandatory ACPI table
>> that specifies the configuration of serial console.
>>
>> Parse this table and check if any registered console match
>> the description.  If it does, enable that console.
>>
>> To implement that, introduce a new member
>> int (*acpi_match)(struct console *, struct acpi_table_spcr *)
>> of struct console.  It allows drivers to check if they provide
>> a matching console device.
>>
>> [1] http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.den0044a/index.html
>> [2] http://msdn.microsoft.com/en-us/library/windows/hardware/dn639131(v=vs.85).aspx
>
> Can you split this to several patches?

I am not sure I should.

> I see preparatory patch in console code, i.e.
> delete_from_console_list(), adding SPCR support to ACPI, enabling it.

It would be difficult to justify delete_from_console_list() in a 
separate patches before the rest of the changes. And enabling SPCR in a 
separate patch also looks oddly for me.

It would be great to have other comments on this before I fix this in 
the next version.

Thank you

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

[Aleksey Makarov]

On 25.01.2016 20:14, Andy Shevchenko wrote:
> On Mon, Jan 25, 2016 at 1:45 PM, Aleksey Makarov
> <aleksey.makarov@linaro.org> wrote:
>> 'ARM Server Base Boot Requiremets' [1] mention SPCR
>> (Serial Port Console Redirection Table) [2] as a mandatory ACPI table
>> that specifies the configuration of serial console.
>>
>> Parse this table and check if any registered console match
>> the description.  If it does, enable that console.
>>
>> To implement that, introduce a new member
>> int (*acpi_match)(struct console *, struct acpi_table_spcr *)
>> of struct console.  It allows drivers to check if they provide
>> a matching console device.
>>
>> [1] http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.den0044a/index.html
>> [2] http://msdn.microsoft.com/en-us/library/windows/hardware/dn639131(v=vs.85).aspx
>
> Can you split this to several patches?

I am not sure I should.

> I see preparatory patch in console code, i.e.
> delete_from_console_list(), adding SPCR support to ACPI, enabling it.

It would be difficult to justify delete_from_console_list() in a 
separate patches before the rest of the changes. And enabling SPCR in a 
separate patch also looks oddly for me.

It would be great to have other comments on this before I fix this in 
the next version.

Thank you

Re: [PATCH 3/3] serial: pl011: add acpi_match for amba-pl011.c

[Aleksey Makarov]

On 25.01.2016 20:22, Andy Shevchenko wrote:
> On Mon, Jan 25, 2016 at 4:21 PM, Andy Shevchenko
> <andy.shevchenko@gmail.com> wrote:
>> On Mon, Jan 25, 2016 at 1:45 PM, Aleksey Makarov
>> <aleksey.makarov@linaro.org> wrote:
>>> Add an implementation of acpi_match() to the pl011 driver.
>>> It allows to check if the console matches one specified with
>>> ACPI SPCR table.
>>
>> I don't know Rafael's opinion on this, but I would split it to extend
>> ACPI header first with reference to newest revision of Microsoft
>> document.

This makes sense.

>>> --- a/include/acpi/actbl2.h
>>> +++ b/include/acpi/actbl2.h
>>> @@ -371,6 +371,10 @@ 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_GENERIC  0x000e
>>> +#define ACPI_DBG2_ARM_DCC           0x000f
>>> +#define ACPI_DBG2_DCM2835           0x0010
>
> ^^^ Exactly because of such typos. Should be BCM

I will fix this, thank you.

[PATCH 3/3] serial: pl011: add acpi_match for amba-pl011.c

[Aleksey Makarov]

On 25.01.2016 20:22, Andy Shevchenko wrote:
> On Mon, Jan 25, 2016 at 4:21 PM, Andy Shevchenko
> <andy.shevchenko@gmail.com> wrote:
>> On Mon, Jan 25, 2016 at 1:45 PM, Aleksey Makarov
>> <aleksey.makarov@linaro.org> wrote:
>>> Add an implementation of acpi_match() to the pl011 driver.
>>> It allows to check if the console matches one specified with
>>> ACPI SPCR table.
>>
>> I don't know Rafael's opinion on this, but I would split it to extend
>> ACPI header first with reference to newest revision of Microsoft
>> document.

This makes sense.

>>> --- a/include/acpi/actbl2.h
>>> +++ b/include/acpi/actbl2.h
>>> @@ -371,6 +371,10 @@ 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_GENERIC  0x000e
>>> +#define ACPI_DBG2_ARM_DCC           0x000f
>>> +#define ACPI_DBG2_DCM2835           0x0010
>
> ^^^ Exactly because of such typos. Should be BCM

I will fix this, thank you.

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

[Peter Hurley]

On 01/25/2016 03:45 AM, Aleksey Makarov wrote:
> This patchset is based on the patchset by Leif Lindholm [1]
> 
> 'ARM Server Base Boot Requirements' [2] mention SPCR 
> (Serial Port Console Redirection Table) [3] as a mandatory
> ACPI table that specifies the configuration of serial console.
> 
> Licensing concerns have prevented implementing it in the past, but as of
> 10 August 2015, these tables have both been released also under 
> OWF 1.0 [4].

This license has a patent retaliation provision, which makes it
incompatible with GPLv2.

*If the license applies to this code*, then this patch set does not
meet the criteria for submission.

Regards,
Peter Hurley


> SPCR support is included in QEMU's ARM mach-virt since 2.4 release.
> 
> Parse the SPCR table and check if any registered console match
> the description.  If it does, enable that console.
> 
> To implement that, introduce a new member
> int (*acpi_match)(struct console *, struct acpi_table_spcr *);
> of struct console.  It allows drivers to check if they provide
> a matching console device.
> 
> Also add an implementation of this member function to the pl011 driver
> and fix a minor issue in kernel/printk/printk.c
> 
> [1] https://lkml.kernel.org/g/1441716217-23786-1-git-send-email-leif.lindholm@linaro.org
> [2] http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.den0044a/index.html
> [3] https://msdn.microsoft.com/en-us/library/windows/hardware/dn639132(v=vs.85).aspx
> [4] http://www.openwebfoundation.org/legal/the-owf-1-0-agreements/owfa-1-0
> 
> Aleksey Makarov (3):
>   printk: make preferred_console local static bool
>   ACPI: parse SPCR and enable matching console
>   serial: pl011: add acpi_match for amba-pl011.c
> 
>  arch/arm64/Kconfig              |  1 +
>  drivers/acpi/Kconfig            |  3 ++
>  drivers/acpi/Makefile           |  1 +
>  drivers/acpi/spcr.c             | 85 +++++++++++++++++++++++++++++++++++++
>  drivers/tty/serial/amba-pl011.c | 14 ++++++
>  include/acpi/actbl2.h           |  4 ++
>  include/linux/console.h         | 12 ++++++
>  kernel/printk/printk.c          | 94 +++++++++++++++++++++++++++++++----------
>  8 files changed, 191 insertions(+), 23 deletions(-)
>  create mode 100644 drivers/acpi/spcr.c
> 

[PATCH 0/3] ACPI: parse the SPCR table

[Peter Hurley]

On 01/25/2016 03:45 AM, Aleksey Makarov wrote:
> This patchset is based on the patchset by Leif Lindholm [1]
> 
> 'ARM Server Base Boot Requirements' [2] mention SPCR 
> (Serial Port Console Redirection Table) [3] as a mandatory
> ACPI table that specifies the configuration of serial console.
> 
> Licensing concerns have prevented implementing it in the past, but as of
> 10 August 2015, these tables have both been released also under 
> OWF 1.0 [4].

This license has a patent retaliation provision, which makes it
incompatible with GPLv2.

*If the license applies to this code*, then this patch set does not
meet the criteria for submission.

Regards,
Peter Hurley


> SPCR support is included in QEMU's ARM mach-virt since 2.4 release.
> 
> Parse the SPCR table and check if any registered console match
> the description.  If it does, enable that console.
> 
> To implement that, introduce a new member
> int (*acpi_match)(struct console *, struct acpi_table_spcr *);
> of struct console.  It allows drivers to check if they provide
> a matching console device.
> 
> Also add an implementation of this member function to the pl011 driver
> and fix a minor issue in kernel/printk/printk.c
> 
> [1] https://lkml.kernel.org/g/1441716217-23786-1-git-send-email-leif.lindholm at linaro.org
> [2] http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.den0044a/index.html
> [3] https://msdn.microsoft.com/en-us/library/windows/hardware/dn639132(v=vs.85).aspx
> [4] http://www.openwebfoundation.org/legal/the-owf-1-0-agreements/owfa-1-0
> 
> Aleksey Makarov (3):
>   printk: make preferred_console local static bool
>   ACPI: parse SPCR and enable matching console
>   serial: pl011: add acpi_match for amba-pl011.c
> 
>  arch/arm64/Kconfig              |  1 +
>  drivers/acpi/Kconfig            |  3 ++
>  drivers/acpi/Makefile           |  1 +
>  drivers/acpi/spcr.c             | 85 +++++++++++++++++++++++++++++++++++++
>  drivers/tty/serial/amba-pl011.c | 14 ++++++
>  include/acpi/actbl2.h           |  4 ++
>  include/linux/console.h         | 12 ++++++
>  kernel/printk/printk.c          | 94 +++++++++++++++++++++++++++++++----------
>  8 files changed, 191 insertions(+), 23 deletions(-)
>  create mode 100644 drivers/acpi/spcr.c
> 

Re: [PATCH 1/3] printk: make preferred_console local static bool

[Peter Hurley]

On 01/25/2016 05:28 AM, Aleksey Makarov wrote:
> 
> 
> On 25.01.2016 19:23, Joe Perches wrote:
>> On Mon, 2016-01-25 at 18:51 +0600, Aleksey Makarov wrote:
>>>
>>> On 25.01.2016 18:45, Joe Perches wrote:
>>>> On Mon, 2016-01-25 at 17:45 +0600, Aleksey Makarov wrote:
>>>>> The variable preferred_console is used only inside register_console()
>>>>> and it's semantics is boolean.  Make it clear.
>>>>
>>>> This loses the index of the preferred console.
>>>> I'm not sure this is better.
>>>
>>> That index is not used anywhere.  I believe the patch makes things clear.
>>
>> Well, with this change the name and its use
>> is a bit misleading.  Maybe changing it to
>> something like use_selected_console is better.
> 
> Thank you. I will fix this in the next version.

Ideally, the transform should be

 preferred_console => has_preferred
 selected_console => preferred_console

This would match the actual use of add_preferred_console()

Regards,
Peter Hurley

[PATCH 1/3] printk: make preferred_console local static bool

[Peter Hurley]

On 01/25/2016 05:28 AM, Aleksey Makarov wrote:
> 
> 
> On 25.01.2016 19:23, Joe Perches wrote:
>> On Mon, 2016-01-25 at 18:51 +0600, Aleksey Makarov wrote:
>>>
>>> On 25.01.2016 18:45, Joe Perches wrote:
>>>> On Mon, 2016-01-25 at 17:45 +0600, Aleksey Makarov wrote:
>>>>> The variable preferred_console is used only inside register_console()
>>>>> and it's semantics is boolean.  Make it clear.
>>>>
>>>> This loses the index of the preferred console.
>>>> I'm not sure this is better.
>>>
>>> That index is not used anywhere.  I believe the patch makes things clear.
>>
>> Well, with this change the name and its use
>> is a bit misleading.  Maybe changing it to
>> something like use_selected_console is better.
> 
> Thank you. I will fix this in the next version.

Ideally, the transform should be

 preferred_console => has_preferred
 selected_console => preferred_console

This would match the actual use of add_preferred_console()

Regards,
Peter Hurley

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

[Peter Hurley]

On 01/25/2016 03:45 AM, Aleksey Makarov wrote:
> 'ARM Server Base Boot Requiremets' [1] mention SPCR
> (Serial Port Console Redirection Table) [2] as a mandatory ACPI table
> that specifies the configuration of serial console.
> 
> Parse this table and check if any registered console match
> the description.  If it does, enable that console.
> 
> To implement that, introduce a new member
> int (*acpi_match)(struct console *, struct acpi_table_spcr *)
> of struct console.  It allows drivers to check if they provide
> a matching console device.

Many, many platform proms with all sorts of binary table layout are already
supported by the existing console infrastructure. Why is ACPI different, that
requires extensive (and messy) changes to console initialization?


How is this going to work with earlycon?


This commit log is missing the reasoning behind adding locks, refactoring
into delete_from_console_list(), and retry loops.


> [1] http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.den0044a/index.html
> [2] http://msdn.microsoft.com/en-us/library/windows/hardware/dn639131(v=vs.85).aspx
> 
> Signed-off-by: Aleksey Makarov <aleksey.makarov@linaro.org>
> ---
>  arch/arm64/Kconfig      |  1 +
>  drivers/acpi/Kconfig    |  3 ++
>  drivers/acpi/Makefile   |  1 +
>  drivers/acpi/spcr.c     | 85 +++++++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/console.h | 12 +++++++
>  kernel/printk/printk.c  | 82 +++++++++++++++++++++++++++++++++++++----------
>  6 files changed, 167 insertions(+), 17 deletions(-)
>  create mode 100644 drivers/acpi/spcr.c
> 
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 573bebc..bf31e3c 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_ATOMIC64_DEC_IF_POSITIVE
>  	select ARCH_HAS_ELF_RANDOMIZE
>  	select ARCH_HAS_GCOV_PROFILE_ALL
> diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
> index e315061..142a338 100644
> --- a/drivers/acpi/Kconfig
> +++ b/drivers/acpi/Kconfig
> @@ -60,6 +60,9 @@ config ACPI_CCA_REQUIRED
>  config IORT_TABLE
>  	bool
>  
> +config ACPI_SPCR_TABLE
> +	bool
> +
>  config ACPI_DEBUGGER
>  	bool "AML debugger interface (EXPERIMENTAL)"
>  	select ACPI_DEBUG
> diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
> index 265eb90..8316859 100644
> --- a/drivers/acpi/Makefile
> +++ b/drivers/acpi/Makefile
> @@ -81,6 +81,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_IORT_TABLE) 	+= iort.o
> +obj-$(CONFIG_ACPI_SPCR_TABLE)	+= spcr.o
>  
>  # processor has its own "processor." module_param namespace
>  processor-y			:= processor_driver.o
> diff --git a/drivers/acpi/spcr.c b/drivers/acpi/spcr.c
> new file mode 100644
> index 0000000..ccb19a0
> --- /dev/null
> +++ b/drivers/acpi/spcr.c
> @@ -0,0 +1,85 @@
> +/*
> + * 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>
> +
> +static struct acpi_table_spcr *spcr_table;
> +
> +int console_acpi_match(struct console *c, char **options)
> +{
> +	int err;
> +
> +	if (!c->acpi_match)
> +		return -ENODEV;
> +
> +	if (!spcr_table)
> +		return -EAGAIN;
> +
> +	err = c->acpi_match(c, spcr_table);
> +	if (err < 0)
> +		return err;
> +
> +	if (options) {
> +		switch (spcr_table->baud_rate) {
> +		case 3:
> +			*options = "9600";
> +			break;
> +		case 4:
> +			*options = "19200";
> +			break;
> +		case 6:
> +			*options = "57600";
> +			break;
> +		case 7:
> +			*options = "115200";
> +			break;
> +		default:
> +			*options = "";
> +			break;
> +		}
> +	}
> +
> +	return err;
> +}
> +
> +static int __init spcr_table_detect(void)
> +{
> +	struct acpi_table_header *table;
> +	acpi_status status;
> +
> +	if (acpi_disabled)
> +		return -ENODEV;
> +
> +	status = acpi_get_table(ACPI_SIG_SPCR, 0, &table);
> +	if (ACPI_FAILURE(status)) {
> +		const char *msg = acpi_format_exception(status);
> +
> +		pr_err("Failed to get table, %s\n", msg);
> +		return -EINVAL;
> +	}
> +
> +	if (table->revision < 2)
> +		return -EOPNOTSUPP;
> +
> +	spcr_table = (struct acpi_table_spcr *)table;
> +
> +	pr_info("Console at 0x%016llx\n", spcr_table->serial_port.address);
> +
> +	acpi_register_consoles_try_again();
> +
> +	return 0;
> +}
> +
> +arch_initcall(spcr_table_detect);
> diff --git a/include/linux/console.h b/include/linux/console.h
> index bd19434..94d0bd8 100644
> --- a/include/linux/console.h
> +++ b/include/linux/console.h
> @@ -117,6 +117,7 @@ static inline int con_debug_leave(void)
>  #define CON_BRL		(32) /* Used for a braille device */
>  #define CON_EXTENDED	(64) /* Use the extended output format a la /dev/kmsg */
>  
> +struct acpi_table_spcr;
>  struct console {
>  	char	name[16];
>  	void	(*write)(struct console *, const char *, unsigned);
> @@ -125,6 +126,7 @@ struct console {
>  	void	(*unblank)(void);
>  	int	(*setup)(struct console *, char *);
>  	int	(*match)(struct console *, char *name, int idx, char *options);
> +	int	(*acpi_match)(struct console *, struct acpi_table_spcr *);
>  	short	flags;
>  	short	index;
>  	int	cflag;
> @@ -132,6 +134,16 @@ struct console {
>  	struct	 console *next;
>  };
>  
> +#ifdef CONFIG_ACPI
> +int console_acpi_match(struct console *c, char **options);
> +#else
> +static inline int console_acpi_match(struct console *c, char **options)
> +{
> +	return -ENODEV;
> +}
> +#endif
> +void acpi_register_consoles_try_again(void);
> +
>  /*
>   * for_each_console() allows you to iterate on each console
>   */
> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> index 37e531f..3cf8cba 100644
> --- a/kernel/printk/printk.c
> +++ b/kernel/printk/printk.c
> @@ -2430,6 +2430,25 @@ static int __init keep_bootcon_setup(char *str)
>  
>  early_param("keep_bootcon", keep_bootcon_setup);
>  
> +static DEFINE_MUTEX(acpi_consoles_delayed_mutex);
> +static struct console *acpi_consoles_delayed;
> +
> +void acpi_register_consoles_try_again(void)
> +{
> +	mutex_lock(&acpi_consoles_delayed_mutex);
> +	while (acpi_consoles_delayed) {
> +
> +		struct console *c = acpi_consoles_delayed;
> +
> +		acpi_consoles_delayed = acpi_consoles_delayed->next;
> +
> +		mutex_unlock(&acpi_consoles_delayed_mutex);
> +		register_console(c);
> +		mutex_lock(&acpi_consoles_delayed_mutex);
> +	}
> +	mutex_unlock(&acpi_consoles_delayed_mutex);
> +}

Why is this necessary? There is no mention of this hack in the
commit log.


> +
>  /*
>   * The console driver calls this routine during kernel initialization
>   * to register the console printing procedure with printk() and to
> @@ -2538,8 +2557,30 @@ void register_console(struct console *newcon)
>  		break;
>  	}
>  
> -	if (!(newcon->flags & CON_ENABLED))
> -		return;
> +	if (!(newcon->flags & CON_ENABLED)) {
> +		char *opts;
> +		int err;
> +
> +		if (newcon->index < 0)
> +			newcon->index = 0;
> +
> +		err = console_acpi_match(newcon, &opts);
> +
> +		if (err == -EAGAIN) {
> +			mutex_lock(&acpi_consoles_delayed_mutex);
> +			newcon->next = acpi_consoles_delayed;
> +			acpi_consoles_delayed = newcon;
> +			mutex_unlock(&acpi_consoles_delayed_mutex);
> +			return;
> +		} else if (err < 0) {
> +			return;
> +		} else {
> +			if (newcon->setup && newcon->setup(newcon, opts) != 0)
> +				return;
> +			newcon->flags |= CON_ENABLED | CON_CONSDEV;
> +			preferred_console = true;
> +		}
> +	}
>  
>  	/*
>  	 * If we have a bootconsole, and are switching to a real console,
> @@ -2612,34 +2653,41 @@ void register_console(struct console *newcon)
>  }
>  EXPORT_SYMBOL(register_console);
>  
> +static int delete_from_console_list(struct console **list, struct console *c)
> +{
> +	while (*list) {
> +		struct console *cur = *list;
> +
> +		if (cur == c) {
> +			*list = cur->next;
> +			return 0;
> +		}
> +		list = &cur->next;
> +	}
> +	return 1;
> +}
> +
>  int unregister_console(struct console *console)
>  {
> -        struct console *a, *b;
>  	int res;
>  
>  	pr_info("%sconsole [%s%d] disabled\n",
>  		(console->flags & CON_BOOT) ? "boot" : "" ,
>  		console->name, console->index);
>  
> +	mutex_lock(&acpi_consoles_delayed_mutex);
> +	res = delete_from_console_list(&acpi_consoles_delayed, console);
> +	mutex_unlock(&acpi_consoles_delayed_mutex);
> +	if (res == 0)
> +		return res;
> +
>  	res = _braille_unregister_console(console);
>  	if (res)
>  		return res;
>  
> -	res = 1;
>  	console_lock();
> -	if (console_drivers == console) {
> -		console_drivers=console->next;
> -		res = 0;
> -	} else if (console_drivers) {
> -		for (a=console_drivers->next, b=console_drivers ;
> -		     a; b=a, a=b->next) {
> -			if (a == console) {
> -				b->next = a->next;
> -				res = 0;
> -				break;
> -			}
> -		}
> -	}
> +
> +	res = delete_from_console_list(&console_drivers, console);
>  
>  	if (!res && (console->flags & CON_EXTENDED))
>  		nr_ext_console_drivers--;
> 

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

[Peter Hurley]

On 01/25/2016 03:45 AM, Aleksey Makarov wrote:
> 'ARM Server Base Boot Requiremets' [1] mention SPCR
> (Serial Port Console Redirection Table) [2] as a mandatory ACPI table
> that specifies the configuration of serial console.
> 
> Parse this table and check if any registered console match
> the description.  If it does, enable that console.
> 
> To implement that, introduce a new member
> int (*acpi_match)(struct console *, struct acpi_table_spcr *)
> of struct console.  It allows drivers to check if they provide
> a matching console device.

Many, many platform proms with all sorts of binary table layout are already
supported by the existing console infrastructure. Why is ACPI different, that
requires extensive (and messy) changes to console initialization?


How is this going to work with earlycon?


This commit log is missing the reasoning behind adding locks, refactoring
into delete_from_console_list(), and retry loops.


> [1] http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.den0044a/index.html
> [2] http://msdn.microsoft.com/en-us/library/windows/hardware/dn639131(v=vs.85).aspx
> 
> Signed-off-by: Aleksey Makarov <aleksey.makarov@linaro.org>
> ---
>  arch/arm64/Kconfig      |  1 +
>  drivers/acpi/Kconfig    |  3 ++
>  drivers/acpi/Makefile   |  1 +
>  drivers/acpi/spcr.c     | 85 +++++++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/console.h | 12 +++++++
>  kernel/printk/printk.c  | 82 +++++++++++++++++++++++++++++++++++++----------
>  6 files changed, 167 insertions(+), 17 deletions(-)
>  create mode 100644 drivers/acpi/spcr.c
> 
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 573bebc..bf31e3c 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_ATOMIC64_DEC_IF_POSITIVE
>  	select ARCH_HAS_ELF_RANDOMIZE
>  	select ARCH_HAS_GCOV_PROFILE_ALL
> diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
> index e315061..142a338 100644
> --- a/drivers/acpi/Kconfig
> +++ b/drivers/acpi/Kconfig
> @@ -60,6 +60,9 @@ config ACPI_CCA_REQUIRED
>  config IORT_TABLE
>  	bool
>  
> +config ACPI_SPCR_TABLE
> +	bool
> +
>  config ACPI_DEBUGGER
>  	bool "AML debugger interface (EXPERIMENTAL)"
>  	select ACPI_DEBUG
> diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
> index 265eb90..8316859 100644
> --- a/drivers/acpi/Makefile
> +++ b/drivers/acpi/Makefile
> @@ -81,6 +81,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_IORT_TABLE) 	+= iort.o
> +obj-$(CONFIG_ACPI_SPCR_TABLE)	+= spcr.o
>  
>  # processor has its own "processor." module_param namespace
>  processor-y			:= processor_driver.o
> diff --git a/drivers/acpi/spcr.c b/drivers/acpi/spcr.c
> new file mode 100644
> index 0000000..ccb19a0
> --- /dev/null
> +++ b/drivers/acpi/spcr.c
> @@ -0,0 +1,85 @@
> +/*
> + * 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>
> +
> +static struct acpi_table_spcr *spcr_table;
> +
> +int console_acpi_match(struct console *c, char **options)
> +{
> +	int err;
> +
> +	if (!c->acpi_match)
> +		return -ENODEV;
> +
> +	if (!spcr_table)
> +		return -EAGAIN;
> +
> +	err = c->acpi_match(c, spcr_table);
> +	if (err < 0)
> +		return err;
> +
> +	if (options) {
> +		switch (spcr_table->baud_rate) {
> +		case 3:
> +			*options = "9600";
> +			break;
> +		case 4:
> +			*options = "19200";
> +			break;
> +		case 6:
> +			*options = "57600";
> +			break;
> +		case 7:
> +			*options = "115200";
> +			break;
> +		default:
> +			*options = "";
> +			break;
> +		}
> +	}
> +
> +	return err;
> +}
> +
> +static int __init spcr_table_detect(void)
> +{
> +	struct acpi_table_header *table;
> +	acpi_status status;
> +
> +	if (acpi_disabled)
> +		return -ENODEV;
> +
> +	status = acpi_get_table(ACPI_SIG_SPCR, 0, &table);
> +	if (ACPI_FAILURE(status)) {
> +		const char *msg = acpi_format_exception(status);
> +
> +		pr_err("Failed to get table, %s\n", msg);
> +		return -EINVAL;
> +	}
> +
> +	if (table->revision < 2)
> +		return -EOPNOTSUPP;
> +
> +	spcr_table = (struct acpi_table_spcr *)table;
> +
> +	pr_info("Console at 0x%016llx\n", spcr_table->serial_port.address);
> +
> +	acpi_register_consoles_try_again();
> +
> +	return 0;
> +}
> +
> +arch_initcall(spcr_table_detect);
> diff --git a/include/linux/console.h b/include/linux/console.h
> index bd19434..94d0bd8 100644
> --- a/include/linux/console.h
> +++ b/include/linux/console.h
> @@ -117,6 +117,7 @@ static inline int con_debug_leave(void)
>  #define CON_BRL		(32) /* Used for a braille device */
>  #define CON_EXTENDED	(64) /* Use the extended output format a la /dev/kmsg */
>  
> +struct acpi_table_spcr;
>  struct console {
>  	char	name[16];
>  	void	(*write)(struct console *, const char *, unsigned);
> @@ -125,6 +126,7 @@ struct console {
>  	void	(*unblank)(void);
>  	int	(*setup)(struct console *, char *);
>  	int	(*match)(struct console *, char *name, int idx, char *options);
> +	int	(*acpi_match)(struct console *, struct acpi_table_spcr *);
>  	short	flags;
>  	short	index;
>  	int	cflag;
> @@ -132,6 +134,16 @@ struct console {
>  	struct	 console *next;
>  };
>  
> +#ifdef CONFIG_ACPI
> +int console_acpi_match(struct console *c, char **options);
> +#else
> +static inline int console_acpi_match(struct console *c, char **options)
> +{
> +	return -ENODEV;
> +}
> +#endif
> +void acpi_register_consoles_try_again(void);
> +
>  /*
>   * for_each_console() allows you to iterate on each console
>   */
> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> index 37e531f..3cf8cba 100644
> --- a/kernel/printk/printk.c
> +++ b/kernel/printk/printk.c
> @@ -2430,6 +2430,25 @@ static int __init keep_bootcon_setup(char *str)
>  
>  early_param("keep_bootcon", keep_bootcon_setup);
>  
> +static DEFINE_MUTEX(acpi_consoles_delayed_mutex);
> +static struct console *acpi_consoles_delayed;
> +
> +void acpi_register_consoles_try_again(void)
> +{
> +	mutex_lock(&acpi_consoles_delayed_mutex);
> +	while (acpi_consoles_delayed) {
> +
> +		struct console *c = acpi_consoles_delayed;
> +
> +		acpi_consoles_delayed = acpi_consoles_delayed->next;
> +
> +		mutex_unlock(&acpi_consoles_delayed_mutex);
> +		register_console(c);
> +		mutex_lock(&acpi_consoles_delayed_mutex);
> +	}
> +	mutex_unlock(&acpi_consoles_delayed_mutex);
> +}

Why is this necessary? There is no mention of this hack in the
commit log.


> +
>  /*
>   * The console driver calls this routine during kernel initialization
>   * to register the console printing procedure with printk() and to
> @@ -2538,8 +2557,30 @@ void register_console(struct console *newcon)
>  		break;
>  	}
>  
> -	if (!(newcon->flags & CON_ENABLED))
> -		return;
> +	if (!(newcon->flags & CON_ENABLED)) {
> +		char *opts;
> +		int err;
> +
> +		if (newcon->index < 0)
> +			newcon->index = 0;
> +
> +		err = console_acpi_match(newcon, &opts);
> +
> +		if (err == -EAGAIN) {
> +			mutex_lock(&acpi_consoles_delayed_mutex);
> +			newcon->next = acpi_consoles_delayed;
> +			acpi_consoles_delayed = newcon;
> +			mutex_unlock(&acpi_consoles_delayed_mutex);
> +			return;
> +		} else if (err < 0) {
> +			return;
> +		} else {
> +			if (newcon->setup && newcon->setup(newcon, opts) != 0)
> +				return;
> +			newcon->flags |= CON_ENABLED | CON_CONSDEV;
> +			preferred_console = true;
> +		}
> +	}
>  
>  	/*
>  	 * If we have a bootconsole, and are switching to a real console,
> @@ -2612,34 +2653,41 @@ void register_console(struct console *newcon)
>  }
>  EXPORT_SYMBOL(register_console);
>  
> +static int delete_from_console_list(struct console **list, struct console *c)
> +{
> +	while (*list) {
> +		struct console *cur = *list;
> +
> +		if (cur == c) {
> +			*list = cur->next;
> +			return 0;
> +		}
> +		list = &cur->next;
> +	}
> +	return 1;
> +}
> +
>  int unregister_console(struct console *console)
>  {
> -        struct console *a, *b;
>  	int res;
>  
>  	pr_info("%sconsole [%s%d] disabled\n",
>  		(console->flags & CON_BOOT) ? "boot" : "" ,
>  		console->name, console->index);
>  
> +	mutex_lock(&acpi_consoles_delayed_mutex);
> +	res = delete_from_console_list(&acpi_consoles_delayed, console);
> +	mutex_unlock(&acpi_consoles_delayed_mutex);
> +	if (res == 0)
> +		return res;
> +
>  	res = _braille_unregister_console(console);
>  	if (res)
>  		return res;
>  
> -	res = 1;
>  	console_lock();
> -	if (console_drivers == console) {
> -		console_drivers=console->next;
> -		res = 0;
> -	} else if (console_drivers) {
> -		for (a=console_drivers->next, b=console_drivers ;
> -		     a; b=a, a=b->next) {
> -			if (a == console) {
> -				b->next = a->next;
> -				res = 0;
> -				break;
> -			}
> -		}
> -	}
> +
> +	res = delete_from_console_list(&console_drivers, console);
>  
>  	if (!res && (console->flags & CON_EXTENDED))
>  		nr_ext_console_drivers--;
> 

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

[Aleksey Makarov]

On 01/25/2016 07:11 PM, Peter Hurley wrote:
> On 01/25/2016 03:45 AM, Aleksey Makarov wrote:
>> This patchset is based on the patchset by Leif Lindholm [1]
>>
>> 'ARM Server Base Boot Requirements' [2] mention SPCR 
>> (Serial Port Console Redirection Table) [3] as a mandatory
>> ACPI table that specifies the configuration of serial console.
>>
>> Licensing concerns have prevented implementing it in the past, but as of
>> 10 August 2015, these tables have both been released also under 
>> OWF 1.0 [4].
> 
> This license has a patent retaliation provision, which makes it
> incompatible with GPLv2.
> 
> *If the license applies to this code*, then this patch set does not
> meet the criteria for submission.

The license applies not to this code but to the document describing the tables.

Here is an excerpt from it:

  Patent Notice:
  Microsoft is making certain patent rights available for implementations of this specification under two options:
  1)  Microsoft’s Community Promise, available at
  http://www.microsoft.com/openspecifications/en/us/programs/community-promise/default.aspx; or
  2)  The Open Web Foundation Final Specification Agreement Version 1.0 ("OWF 1.0")
  as of October 1, 2012, available at http://www.openwebfoundation.org/legal/the-owf-1-0-agreements/owfa-1-0. 

I believe that it means that the patch set meets the criteria for submission.  Am I right?

Thank you
Aleksey Makarov

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

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

[Aleksey Makarov]

On 01/25/2016 07:11 PM, Peter Hurley wrote:
> On 01/25/2016 03:45 AM, Aleksey Makarov wrote:
>> This patchset is based on the patchset by Leif Lindholm [1]
>>
>> 'ARM Server Base Boot Requirements' [2] mention SPCR 
>> (Serial Port Console Redirection Table) [3] as a mandatory
>> ACPI table that specifies the configuration of serial console.
>>
>> Licensing concerns have prevented implementing it in the past, but as of
>> 10 August 2015, these tables have both been released also under 
>> OWF 1.0 [4].
> 
> This license has a patent retaliation provision, which makes it
> incompatible with GPLv2.
> 
> *If the license applies to this code*, then this patch set does not
> meet the criteria for submission.

The license applies not to this code but to the document describing the tables.

Here is an excerpt from it:

  Patent Notice:
  Microsoft is making certain patent rights available for implementations of this specification under two options:
  1)  Microsoft’s Community Promise, available at
  http://www.microsoft.com/openspecifications/en/us/programs/community-promise/default.aspx; or
  2)  The Open Web Foundation Final Specification Agreement Version 1.0 ("OWF 1.0")
  as of October 1, 2012, available at http://www.openwebfoundation.org/legal/the-owf-1-0-agreements/owfa-1-0. 

I believe that it means that the patch set meets the criteria for submission.  Am I right?

Thank you
Aleksey Makarov

[PATCH 0/3] ACPI: parse the SPCR table

[Aleksey Makarov]

On 01/25/2016 07:11 PM, Peter Hurley wrote:
> On 01/25/2016 03:45 AM, Aleksey Makarov wrote:
>> This patchset is based on the patchset by Leif Lindholm [1]
>>
>> 'ARM Server Base Boot Requirements' [2] mention SPCR 
>> (Serial Port Console Redirection Table) [3] as a mandatory
>> ACPI table that specifies the configuration of serial console.
>>
>> Licensing concerns have prevented implementing it in the past, but as of
>> 10 August 2015, these tables have both been released also under 
>> OWF 1.0 [4].
> 
> This license has a patent retaliation provision, which makes it
> incompatible with GPLv2.
> 
> *If the license applies to this code*, then this patch set does not
> meet the criteria for submission.

The license applies not to this code but to the document describing the tables.

Here is an excerpt from it:

  Patent Notice:
  Microsoft is making certain patent rights available for implementations of this specification under two options:
  1)  Microsoft?s Community Promise, available at
  http://www.microsoft.com/openspecifications/en/us/programs/community-promise/default.aspx; or
  2)  The Open Web Foundation Final Specification Agreement Version 1.0 ("OWF 1.0")
  as of October 1, 2012, available at http://www.openwebfoundation.org/legal/the-owf-1-0-agreements/owfa-1-0. 

I believe that it means that the patch set meets the criteria for submission.  Am I right?

Thank you
Aleksey Makarov

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

[One Thousand Gnomes]

On Wed, 27 Jan 2016 15:17:52 +0300
Aleksey Makarov <aleksey.makarov@linaro.org> wrote:

> On 01/25/2016 07:11 PM, Peter Hurley wrote:
> > On 01/25/2016 03:45 AM, Aleksey Makarov wrote:  
> >> This patchset is based on the patchset by Leif Lindholm [1]
> >>
> >> 'ARM Server Base Boot Requirements' [2] mention SPCR 
> >> (Serial Port Console Redirection Table) [3] as a mandatory
> >> ACPI table that specifies the configuration of serial console.
> >>
> >> Licensing concerns have prevented implementing it in the past, but as of
> >> 10 August 2015, these tables have both been released also under 
> >> OWF 1.0 [4].  
> > 
> > This license has a patent retaliation provision, which makes it
> > incompatible with GPLv2.
> > 
> > *If the license applies to this code*, then this patch set does not
> > meet the criteria for submission.  
> 
> The license applies not to this code but to the document describing the tables.
> 
> Here is an excerpt from it:
> 
>   Patent Notice:
>   Microsoft is making certain patent rights available for implementations of this specification under two options:
>   1)  Microsoft’s Community Promise, available at
>   http://www.microsoft.com/openspecifications/en/us/programs/community-promise/default.aspx; or
>   2)  The Open Web Foundation Final Specification Agreement Version 1.0 ("OWF 1.0")
>   as of October 1, 2012, available at http://www.openwebfoundation.org/legal/the-owf-1-0-agreements/owfa-1-0. 
> 
> I believe that it means that the patch set meets the criteria for submission.  Am I right?

This is not a forum for legal advice. I would suggest that Linaro
discusses it privately with the Linux Foundation and Linus and does so
under attorney-client privilege. The Linux Foundation does have some
reasons to exist.

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

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

[One Thousand Gnomes]

On Wed, 27 Jan 2016 15:17:52 +0300
Aleksey Makarov <aleksey.makarov@linaro.org> wrote:

> On 01/25/2016 07:11 PM, Peter Hurley wrote:
> > On 01/25/2016 03:45 AM, Aleksey Makarov wrote:  
> >> This patchset is based on the patchset by Leif Lindholm [1]
> >>
> >> 'ARM Server Base Boot Requirements' [2] mention SPCR 
> >> (Serial Port Console Redirection Table) [3] as a mandatory
> >> ACPI table that specifies the configuration of serial console.
> >>
> >> Licensing concerns have prevented implementing it in the past, but as of
> >> 10 August 2015, these tables have both been released also under 
> >> OWF 1.0 [4].  
> > 
> > This license has a patent retaliation provision, which makes it
> > incompatible with GPLv2.
> > 
> > *If the license applies to this code*, then this patch set does not
> > meet the criteria for submission.  
> 
> The license applies not to this code but to the document describing the tables.
> 
> Here is an excerpt from it:
> 
>   Patent Notice:
>   Microsoft is making certain patent rights available for implementations of this specification under two options:
>   1)  Microsoft’s Community Promise, available at
>   http://www.microsoft.com/openspecifications/en/us/programs/community-promise/default.aspx; or
>   2)  The Open Web Foundation Final Specification Agreement Version 1.0 ("OWF 1.0")
>   as of October 1, 2012, available at http://www.openwebfoundation.org/legal/the-owf-1-0-agreements/owfa-1-0. 
> 
> I believe that it means that the patch set meets the criteria for submission.  Am I right?

This is not a forum for legal advice. I would suggest that Linaro
discusses it privately with the Linux Foundation and Linus and does so
under attorney-client privilege. The Linux Foundation does have some
reasons to exist.

Alan

[PATCH 0/3] ACPI: parse the SPCR table

[One Thousand Gnomes]

On Wed, 27 Jan 2016 15:17:52 +0300
Aleksey Makarov <aleksey.makarov@linaro.org> wrote:

> On 01/25/2016 07:11 PM, Peter Hurley wrote:
> > On 01/25/2016 03:45 AM, Aleksey Makarov wrote:  
> >> This patchset is based on the patchset by Leif Lindholm [1]
> >>
> >> 'ARM Server Base Boot Requirements' [2] mention SPCR 
> >> (Serial Port Console Redirection Table) [3] as a mandatory
> >> ACPI table that specifies the configuration of serial console.
> >>
> >> Licensing concerns have prevented implementing it in the past, but as of
> >> 10 August 2015, these tables have both been released also under 
> >> OWF 1.0 [4].  
> > 
> > This license has a patent retaliation provision, which makes it
> > incompatible with GPLv2.
> > 
> > *If the license applies to this code*, then this patch set does not
> > meet the criteria for submission.  
> 
> The license applies not to this code but to the document describing the tables.
> 
> Here is an excerpt from it:
> 
>   Patent Notice:
>   Microsoft is making certain patent rights available for implementations of this specification under two options:
>   1)  Microsoft?s Community Promise, available at
>   http://www.microsoft.com/openspecifications/en/us/programs/community-promise/default.aspx; or
>   2)  The Open Web Foundation Final Specification Agreement Version 1.0 ("OWF 1.0")
>   as of October 1, 2012, available at http://www.openwebfoundation.org/legal/the-owf-1-0-agreements/owfa-1-0. 
> 
> I believe that it means that the patch set meets the criteria for submission.  Am I right?

This is not a forum for legal advice. I would suggest that Linaro
discusses it privately with the Linux Foundation and Linus and does so
under attorney-client privilege. The Linux Foundation does have some
reasons to exist.

Alan

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

[Aleksey Makarov]

On 01/25/2016 07:32 PM, Peter Hurley wrote:
> On 01/25/2016 03:45 AM, Aleksey Makarov wrote:
>> 'ARM Server Base Boot Requiremets' [1] mention SPCR (Serial Port
>> Console Redirection Table) [2] as a mandatory ACPI table that
>> specifies the configuration of serial console.
>> 
>> Parse this table and check if any registered console match the
>> description.  If it does, enable that console.
>> 
>> To implement that, introduce a new member int (*acpi_match)(struct
>> console *, struct acpi_table_spcr *) of struct console.  It allows
>> drivers to check if they provide a matching console device.
> 
> Many, many platform proms with all sorts of binary table layout are
> already supported by the existing console infrastructure. Why is ACPI
> different, that requires extensive (and messy) changes to console
> initialization?

Without this patch, when linux calls register_console(), that function
checks if any console has been enabled so far. 1) If not, it enables the
console being registered. 2) If there exists any enabled console, it
looks at the console_cmdline array. That array holds a list of
consoles that user wishes to enable. There are two ways to append
an item to that list: first is to pass "console=..." option in command
line and second is to call add_preferred_console(char *name, int idx,
char *options). As it is clear from the signature, the function
requires the name of the driver (like "ttyS") and the line id. On the
other hand, the SPCR ACPI table describes console by specifying the
address of it's registers or PCI Device ID / PCI Vendor ID or PCI Bus
Number / PCI Device Number. So to use this function we would need to
have a method to translate this info to the name of terminal and line
index. I could not figure out any way to do that.

In the initial version of the patch after getting the reference to the
SPCR ACPI table the full tree of ACPI devices was searched to find any
device with the same address.  When uart_add_one_port() was called
to register a new serial port, the ACPI companion of this port was
compared to the found device. If it was the same device, the code
called add_preferred_console() (the terminal name and line index are
known in uart_add_one_port()).

This original approach had two problems: 

    1) It works with the SPCR tables that describe consoles only by
the address of the registers. I do not think that consoles that are
described by PCI info will appear in the near future, but decided to
implement this in a generic way. I would like to discuss if this
decision was good.

    2) Wrong order of initialization.  Many console drivers have already
been registered by the time uart_add_one_port() adds an item to the
console_cmdline array.  There is a similar problem with my
implementation,  but having a dedicated acpi_match() callback I
believe made it simpler to circumwent.

That's why I believe we need to add a new funcion pointer to struct
console. On the other hand, I do not understand which existing
structure you are referring.

> How is this going to work with earlycon?

If an earlycon that matches SPCR is being registered, the code will enable it.
While it is harmless. Even so I will check for earlycon in the next version
of the patch set, thank you.

> This commit log is missing the reasoning behind adding locks,
> refactoring into delete_from_console_list(), and retry loops.

I will add this to the next verion of the series.

Thank you
Aleksey Makarov


>> [1]
>> http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.den0044a/index.html
>>
>> [2] http://msdn.microsoft.com/en-us/library/windows/hardware/dn639131(v=vs.85).aspx
>> 
>> Signed-off-by: Aleksey Makarov <aleksey.makarov@linaro.org>

[ .. ]

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

[Aleksey Makarov]

On 01/25/2016 07:32 PM, Peter Hurley wrote:
> On 01/25/2016 03:45 AM, Aleksey Makarov wrote:
>> 'ARM Server Base Boot Requiremets' [1] mention SPCR (Serial Port
>> Console Redirection Table) [2] as a mandatory ACPI table that
>> specifies the configuration of serial console.
>> 
>> Parse this table and check if any registered console match the
>> description.  If it does, enable that console.
>> 
>> To implement that, introduce a new member int (*acpi_match)(struct
>> console *, struct acpi_table_spcr *) of struct console.  It allows
>> drivers to check if they provide a matching console device.
> 
> Many, many platform proms with all sorts of binary table layout are
> already supported by the existing console infrastructure. Why is ACPI
> different, that requires extensive (and messy) changes to console
> initialization?

Without this patch, when linux calls register_console(), that function
checks if any console has been enabled so far. 1) If not, it enables the
console being registered. 2) If there exists any enabled console, it
looks at the console_cmdline array. That array holds a list of
consoles that user wishes to enable. There are two ways to append
an item to that list: first is to pass "console=..." option in command
line and second is to call add_preferred_console(char *name, int idx,
char *options). As it is clear from the signature, the function
requires the name of the driver (like "ttyS") and the line id. On the
other hand, the SPCR ACPI table describes console by specifying the
address of it's registers or PCI Device ID / PCI Vendor ID or PCI Bus
Number / PCI Device Number. So to use this function we would need to
have a method to translate this info to the name of terminal and line
index. I could not figure out any way to do that.

In the initial version of the patch after getting the reference to the
SPCR ACPI table the full tree of ACPI devices was searched to find any
device with the same address.  When uart_add_one_port() was called
to register a new serial port, the ACPI companion of this port was
compared to the found device. If it was the same device, the code
called add_preferred_console() (the terminal name and line index are
known in uart_add_one_port()).

This original approach had two problems: 

    1) It works with the SPCR tables that describe consoles only by
the address of the registers. I do not think that consoles that are
described by PCI info will appear in the near future, but decided to
implement this in a generic way. I would like to discuss if this
decision was good.

    2) Wrong order of initialization.  Many console drivers have already
been registered by the time uart_add_one_port() adds an item to the
console_cmdline array.  There is a similar problem with my
implementation,  but having a dedicated acpi_match() callback I
believe made it simpler to circumwent.

That's why I believe we need to add a new funcion pointer to struct
console. On the other hand, I do not understand which existing
structure you are referring.

> How is this going to work with earlycon?

If an earlycon that matches SPCR is being registered, the code will enable it.
While it is harmless. Even so I will check for earlycon in the next version
of the patch set, thank you.

> This commit log is missing the reasoning behind adding locks,
> refactoring into delete_from_console_list(), and retry loops.

I will add this to the next verion of the series.

Thank you
Aleksey Makarov


>> [1]
>> http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.den0044a/index.html
>>
>> [2] http://msdn.microsoft.com/en-us/library/windows/hardware/dn639131(v=vs.85).aspx
>> 
>> Signed-off-by: Aleksey Makarov <aleksey.makarov@linaro.org>

[ .. ]

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

[Peter Hurley]

On 01/27/2016 05:57 AM, Aleksey Makarov wrote:
> 
> 
> On 01/25/2016 07:32 PM, Peter Hurley wrote:
>> On 01/25/2016 03:45 AM, Aleksey Makarov wrote:
>>> 'ARM Server Base Boot Requiremets' [1] mention SPCR (Serial Port
>>> Console Redirection Table) [2] as a mandatory ACPI table that
>>> specifies the configuration of serial console.
>>>
>>> Parse this table and check if any registered console match the
>>> description.  If it does, enable that console.
>>>
>>> To implement that, introduce a new member int (*acpi_match)(struct
>>> console *, struct acpi_table_spcr *) of struct console.  It allows
>>> drivers to check if they provide a matching console device.
>>
>> Many, many platform proms with all sorts of binary table layout are
>> already supported by the existing console infrastructure. Why is ACPI
>> different, that requires extensive (and messy) changes to console
>> initialization?
> 
> Without this patch, when linux calls register_console(), that function
> checks if any console has been enabled so far. 1) If not, it enables the
> console being registered. 2) If there exists any enabled console, it
> looks at the console_cmdline array. That array holds a list of
> consoles that user wishes to enable. There are two ways to append
> an item to that list: first is to pass "console=..." option in command
> line and second is to call add_preferred_console(char *name, int idx,
> char *options). As it is clear from the signature, the function
> requires the name of the driver (like "ttyS") and the line id. On the
> other hand, the SPCR ACPI table describes console by specifying the
> address of it's registers or PCI Device ID / PCI Vendor ID or PCI Bus
> Number / PCI Device Number. So to use this function we would need to
> have a method to translate this info to the name of terminal and line
> index. I could not figure out any way to do that.

I'm not sure how this answers my question.

Which existing drivers/arch setup have you studied to conclude that
the existing console mechanisms don't work? Have you actually looked
at the in-tree callers of add_preferred_console()?


> In the initial version of the patch after getting the reference to the
> SPCR ACPI table the full tree of ACPI devices was searched to find any
> device with the same address.  When uart_add_one_port() was called
> to register a new serial port, the ACPI companion of this port was
> compared to the found device. If it was the same device, the code
> called add_preferred_console() (the terminal name and line index are
> known in uart_add_one_port()).

Yeah, I wasn't a fan of that.

But I think it was a bad choice to pick SPCR as table format, in the
first place. At least DBG2 has the actual ACPI device identifier :/


> This original approach had two problems: 
> 
>     1) It works with the SPCR tables that describe consoles only by
> the address of the registers. I do not think that consoles that are
> described by PCI info will appear in the near future, but decided to
> implement this in a generic way. I would like to discuss if this
> decision was good.
> 
>     2) Wrong order of initialization.  Many console drivers have already
> been registered by the time uart_add_one_port() adds an item to the
> console_cmdline array.  There is a similar problem with my
> implementation,  but having a dedicated acpi_match() callback I
> believe made it simpler to circumwent.

I don't see how the "wrong order of initialization" and the need for
acpi_match() correlate. What do you mean by "wrong order"? What is the
"right order"?


> That's why I believe we need to add a new funcion pointer to struct
> console. On the other hand, I do not understand which existing
> structure you are referring.
> 
>> How is this going to work with earlycon?
> 
> If an earlycon that matches SPCR is being registered, the code will enable it.

I think you should review how and when an earlycon is specified, initialized
and registered before you conclude that this will magically work.


> While it is harmless. Even so I will check for earlycon in the next version
> of the patch set, thank you.
> 
>> This commit log is missing the reasoning behind adding locks,
>> refactoring into delete_from_console_list(), and retry loops.
> 
> I will add this to the next verion of the series.
> 
> Thank you
> Aleksey Makarov
> 
> 
>>> [1]
>>> http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.den0044a/index.html
>>>
>>> [2] http://msdn.microsoft.com/en-us/library/windows/hardware/dn639131(v=vs.85).aspx
>>>
>>> Signed-off-by: Aleksey Makarov <aleksey.makarov@linaro.org>
> 
> [ .. ]
> 

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

[Peter Hurley]

On 01/27/2016 05:57 AM, Aleksey Makarov wrote:
> 
> 
> On 01/25/2016 07:32 PM, Peter Hurley wrote:
>> On 01/25/2016 03:45 AM, Aleksey Makarov wrote:
>>> 'ARM Server Base Boot Requiremets' [1] mention SPCR (Serial Port
>>> Console Redirection Table) [2] as a mandatory ACPI table that
>>> specifies the configuration of serial console.
>>>
>>> Parse this table and check if any registered console match the
>>> description.  If it does, enable that console.
>>>
>>> To implement that, introduce a new member int (*acpi_match)(struct
>>> console *, struct acpi_table_spcr *) of struct console.  It allows
>>> drivers to check if they provide a matching console device.
>>
>> Many, many platform proms with all sorts of binary table layout are
>> already supported by the existing console infrastructure. Why is ACPI
>> different, that requires extensive (and messy) changes to console
>> initialization?
> 
> Without this patch, when linux calls register_console(), that function
> checks if any console has been enabled so far. 1) If not, it enables the
> console being registered. 2) If there exists any enabled console, it
> looks at the console_cmdline array. That array holds a list of
> consoles that user wishes to enable. There are two ways to append
> an item to that list: first is to pass "console=..." option in command
> line and second is to call add_preferred_console(char *name, int idx,
> char *options). As it is clear from the signature, the function
> requires the name of the driver (like "ttyS") and the line id. On the
> other hand, the SPCR ACPI table describes console by specifying the
> address of it's registers or PCI Device ID / PCI Vendor ID or PCI Bus
> Number / PCI Device Number. So to use this function we would need to
> have a method to translate this info to the name of terminal and line
> index. I could not figure out any way to do that.

I'm not sure how this answers my question.

Which existing drivers/arch setup have you studied to conclude that
the existing console mechanisms don't work? Have you actually looked
at the in-tree callers of add_preferred_console()?


> In the initial version of the patch after getting the reference to the
> SPCR ACPI table the full tree of ACPI devices was searched to find any
> device with the same address.  When uart_add_one_port() was called
> to register a new serial port, the ACPI companion of this port was
> compared to the found device. If it was the same device, the code
> called add_preferred_console() (the terminal name and line index are
> known in uart_add_one_port()).

Yeah, I wasn't a fan of that.

But I think it was a bad choice to pick SPCR as table format, in the
first place. At least DBG2 has the actual ACPI device identifier :/


> This original approach had two problems: 
> 
>     1) It works with the SPCR tables that describe consoles only by
> the address of the registers. I do not think that consoles that are
> described by PCI info will appear in the near future, but decided to
> implement this in a generic way. I would like to discuss if this
> decision was good.
> 
>     2) Wrong order of initialization.  Many console drivers have already
> been registered by the time uart_add_one_port() adds an item to the
> console_cmdline array.  There is a similar problem with my
> implementation,  but having a dedicated acpi_match() callback I
> believe made it simpler to circumwent.

I don't see how the "wrong order of initialization" and the need for
acpi_match() correlate. What do you mean by "wrong order"? What is the
"right order"?


> That's why I believe we need to add a new funcion pointer to struct
> console. On the other hand, I do not understand which existing
> structure you are referring.
> 
>> How is this going to work with earlycon?
> 
> If an earlycon that matches SPCR is being registered, the code will enable it.

I think you should review how and when an earlycon is specified, initialized
and registered before you conclude that this will magically work.


> While it is harmless. Even so I will check for earlycon in the next version
> of the patch set, thank you.
> 
>> This commit log is missing the reasoning behind adding locks,
>> refactoring into delete_from_console_list(), and retry loops.
> 
> I will add this to the next verion of the series.
> 
> Thank you
> Aleksey Makarov
> 
> 
>>> [1]
>>> http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.den0044a/index.html
>>>
>>> [2] http://msdn.microsoft.com/en-us/library/windows/hardware/dn639131(v=vs.85).aspx
>>>
>>> Signed-off-by: Aleksey Makarov <aleksey.makarov@linaro.org>
> 
> [ .. ]
> 

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

[Aleksey Makarov]

On 01/28/2016 03:45 AM, Peter Hurley wrote:
> On 01/27/2016 05:57 AM, Aleksey Makarov wrote:
>>
>>
>> On 01/25/2016 07:32 PM, Peter Hurley wrote:
>>> On 01/25/2016 03:45 AM, Aleksey Makarov wrote:
>>>> 'ARM Server Base Boot Requiremets' [1] mention SPCR (Serial Port
>>>> Console Redirection Table) [2] as a mandatory ACPI table that
>>>> specifies the configuration of serial console.
>>>>
>>>> Parse this table and check if any registered console match the
>>>> description.  If it does, enable that console.
>>>>
>>>> To implement that, introduce a new member int (*acpi_match)(struct
>>>> console *, struct acpi_table_spcr *) of struct console.  It allows
>>>> drivers to check if they provide a matching console device.
>>>
>>> Many, many platform proms with all sorts of binary table layout are
>>> already supported by the existing console infrastructure. Why is ACPI
>>> different, that requires extensive (and messy) changes to console
>>> initialization?
>>
>> Without this patch, when linux calls register_console(), that function
>> checks if any console has been enabled so far. 1) If not, it enables the
>> console being registered. 2) If there exists any enabled console, it
>> looks at the console_cmdline array. That array holds a list of
>> consoles that user wishes to enable. There are two ways to append
>> an item to that list: first is to pass "console=..." option in command
>> line and second is to call add_preferred_console(char *name, int idx,
>> char *options). As it is clear from the signature, the function
>> requires the name of the driver (like "ttyS") and the line id. On the
>> other hand, the SPCR ACPI table describes console by specifying the
>> address of it's registers or PCI Device ID / PCI Vendor ID or PCI Bus
>> Number / PCI Device Number. So to use this function we would need to
>> have a method to translate this info to the name of terminal and line
>> index. I could not figure out any way to do that.
> 
> I'm not sure how this answers my question.

It does not.  For the actual answer please scroll down.

> Which existing drivers/arch setup have you studied to conclude that
> the existing console mechanisms don't work? 

I studied how add_preferred_console() is used in drivers/of/base.c,
grepped the tree for it and studied each instance.
Almost everything I see is dumb calls to this 
function with hardcoded tty name and line index.  I don't see 
how this can help.

> Have you actually looked
> at the in-tree callers of add_preferred_console()?

Yes I have. It looks like you are implying that there is some drivers/arch
setup that would help to implement this correctly. Could you please
give me a reference to it?

>> In the initial version of the patch after getting the reference to the
>> SPCR ACPI table the full tree of ACPI devices was searched to find any
>> device with the same address.  When uart_add_one_port() was called
>> to register a new serial port, the ACPI companion of this port was
>> compared to the found device. If it was the same device, the code
>> called add_preferred_console() (the terminal name and line index are
>> known in uart_add_one_port()).
> 
> Yeah, I wasn't a fan of that.
> 
> But I think it was a bad choice to pick SPCR as table format, in the
> first place. At least DBG2 has the actual ACPI device identifier :/

I am working on parsing DBG2 to implement earlycon based on 
info from it.  As I understand,
- SPCR specifies which existing console should be enabled (= made preferred).
- DBG2 specifies how kernel can add a new earlycon to the system.
These are two differend subsystems and both make sense.

Also I don't understand how having (optional) ACPI device identifier
in DBG2 table can help to implement earlycon introduction.

>> This original approach had two problems: 
>>
>>     1) It works with the SPCR tables that describe consoles only by
>> the address of the registers. I do not think that consoles that are
>> described by PCI info will appear in the near future, but decided to
>> implement this in a generic way. I would like to discuss if this
>> decision was good.
>>
>>     2) Wrong order of initialization.  Many console drivers have already
>> been registered by the time uart_add_one_port() adds an item to the
>> console_cmdline array.  There is a similar problem with my
>> implementation,  but having a dedicated acpi_match() callback I
>> believe made it simpler to circumwent.
> 
> I don't see how the "wrong order of initialization" and the need for
> acpi_match() correlate. What do you mean by "wrong order"? What is the
> "right order"?

There is a race between initialization/registration of consoles and 
parsing SPCR table.  By the time the console is registered we should 
have SPCR table parsed and add_preferred_console() called.

In the original submission add_preferred_console() was called from
uart registration code, that requires to parse ACPI table very early
(and nobody can say how early, because uarts can be registered very
early.  pl011 is initialized with arch_initcall() for example).

I decided to fix this with retrying registration of consoles after
ACPI is parsed.  I do not like it either, but it is the simplest 
solution I could suggest.  But it's quite difficult do implement 
such deferring code in the uart registration and, more important,
it is wrong as there may be non-uart consoles.

So acpi_match() is a part of registration retrying mechanism and
the right place for it is at console registration.

>> That's why I believe we need to add a new funcion pointer to struct
>> console. On the other hand, I do not understand which existing
>> structure you are referring.
>>
>>> How is this going to work with earlycon?
>>
>> If an earlycon that matches SPCR is being registered, the code will enable it.
> 
> I think you should review how and when an earlycon is specified, initialized
> and registered before you conclude that this will magically work.

I reviewed that again and the only issue that I can imagine is that
ACPI can not make earlycon preferred.  Is it that you are afraid of?
I believe it should not.  What problems do you see?

Thank you
Aleksey Makarov

>> While it is harmless. Even so I will check for earlycon in the next version
>> of the patch set, thank you.
>>
>>> This commit log is missing the reasoning behind adding locks,
>>> refactoring into delete_from_console_list(), and retry loops.
>>
>> I will add this to the next verion of the series.
>>
>> Thank you
>> Aleksey Makarov
>>
>>
>>>> [1]
>>>> http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.den0044a/index.html
>>>>
>>>> [2] http://msdn.microsoft.com/en-us/library/windows/hardware/dn639131(v=vs.85).aspx
>>>>
>>>> Signed-off-by: Aleksey Makarov <aleksey.makarov@linaro.org>
>>
>> [ .. ]
>>
> 

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

[Aleksey Makarov]

On 01/28/2016 03:45 AM, Peter Hurley wrote:
> On 01/27/2016 05:57 AM, Aleksey Makarov wrote:
>>
>>
>> On 01/25/2016 07:32 PM, Peter Hurley wrote:
>>> On 01/25/2016 03:45 AM, Aleksey Makarov wrote:
>>>> 'ARM Server Base Boot Requiremets' [1] mention SPCR (Serial Port
>>>> Console Redirection Table) [2] as a mandatory ACPI table that
>>>> specifies the configuration of serial console.
>>>>
>>>> Parse this table and check if any registered console match the
>>>> description.  If it does, enable that console.
>>>>
>>>> To implement that, introduce a new member int (*acpi_match)(struct
>>>> console *, struct acpi_table_spcr *) of struct console.  It allows
>>>> drivers to check if they provide a matching console device.
>>>
>>> Many, many platform proms with all sorts of binary table layout are
>>> already supported by the existing console infrastructure. Why is ACPI
>>> different, that requires extensive (and messy) changes to console
>>> initialization?
>>
>> Without this patch, when linux calls register_console(), that function
>> checks if any console has been enabled so far. 1) If not, it enables the
>> console being registered. 2) If there exists any enabled console, it
>> looks at the console_cmdline array. That array holds a list of
>> consoles that user wishes to enable. There are two ways to append
>> an item to that list: first is to pass "console=..." option in command
>> line and second is to call add_preferred_console(char *name, int idx,
>> char *options). As it is clear from the signature, the function
>> requires the name of the driver (like "ttyS") and the line id. On the
>> other hand, the SPCR ACPI table describes console by specifying the
>> address of it's registers or PCI Device ID / PCI Vendor ID or PCI Bus
>> Number / PCI Device Number. So to use this function we would need to
>> have a method to translate this info to the name of terminal and line
>> index. I could not figure out any way to do that.
> 
> I'm not sure how this answers my question.

It does not.  For the actual answer please scroll down.

> Which existing drivers/arch setup have you studied to conclude that
> the existing console mechanisms don't work? 

I studied how add_preferred_console() is used in drivers/of/base.c,
grepped the tree for it and studied each instance.
Almost everything I see is dumb calls to this 
function with hardcoded tty name and line index.  I don't see 
how this can help.

> Have you actually looked
> at the in-tree callers of add_preferred_console()?

Yes I have. It looks like you are implying that there is some drivers/arch
setup that would help to implement this correctly. Could you please
give me a reference to it?

>> In the initial version of the patch after getting the reference to the
>> SPCR ACPI table the full tree of ACPI devices was searched to find any
>> device with the same address.  When uart_add_one_port() was called
>> to register a new serial port, the ACPI companion of this port was
>> compared to the found device. If it was the same device, the code
>> called add_preferred_console() (the terminal name and line index are
>> known in uart_add_one_port()).
> 
> Yeah, I wasn't a fan of that.
> 
> But I think it was a bad choice to pick SPCR as table format, in the
> first place. At least DBG2 has the actual ACPI device identifier :/

I am working on parsing DBG2 to implement earlycon based on 
info from it.  As I understand,
- SPCR specifies which existing console should be enabled (= made preferred).
- DBG2 specifies how kernel can add a new earlycon to the system.
These are two differend subsystems and both make sense.

Also I don't understand how having (optional) ACPI device identifier
in DBG2 table can help to implement earlycon introduction.

>> This original approach had two problems: 
>>
>>     1) It works with the SPCR tables that describe consoles only by
>> the address of the registers. I do not think that consoles that are
>> described by PCI info will appear in the near future, but decided to
>> implement this in a generic way. I would like to discuss if this
>> decision was good.
>>
>>     2) Wrong order of initialization.  Many console drivers have already
>> been registered by the time uart_add_one_port() adds an item to the
>> console_cmdline array.  There is a similar problem with my
>> implementation,  but having a dedicated acpi_match() callback I
>> believe made it simpler to circumwent.
> 
> I don't see how the "wrong order of initialization" and the need for
> acpi_match() correlate. What do you mean by "wrong order"? What is the
> "right order"?

There is a race between initialization/registration of consoles and 
parsing SPCR table.  By the time the console is registered we should 
have SPCR table parsed and add_preferred_console() called.

In the original submission add_preferred_console() was called from
uart registration code, that requires to parse ACPI table very early
(and nobody can say how early, because uarts can be registered very
early.  pl011 is initialized with arch_initcall() for example).

I decided to fix this with retrying registration of consoles after
ACPI is parsed.  I do not like it either, but it is the simplest 
solution I could suggest.  But it's quite difficult do implement 
such deferring code in the uart registration and, more important,
it is wrong as there may be non-uart consoles.

So acpi_match() is a part of registration retrying mechanism and
the right place for it is at console registration.

>> That's why I believe we need to add a new funcion pointer to struct
>> console. On the other hand, I do not understand which existing
>> structure you are referring.
>>
>>> How is this going to work with earlycon?
>>
>> If an earlycon that matches SPCR is being registered, the code will enable it.
> 
> I think you should review how and when an earlycon is specified, initialized
> and registered before you conclude that this will magically work.

I reviewed that again and the only issue that I can imagine is that
ACPI can not make earlycon preferred.  Is it that you are afraid of?
I believe it should not.  What problems do you see?

Thank you
Aleksey Makarov

>> While it is harmless. Even so I will check for earlycon in the next version
>> of the patch set, thank you.
>>
>>> This commit log is missing the reasoning behind adding locks,
>>> refactoring into delete_from_console_list(), and retry loops.
>>
>> I will add this to the next verion of the series.
>>
>> Thank you
>> Aleksey Makarov
>>
>>
>>>> [1]
>>>> http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.den0044a/index.html
>>>>
>>>> [2] http://msdn.microsoft.com/en-us/library/windows/hardware/dn639131(v=vs.85).aspx
>>>>
>>>> Signed-off-by: Aleksey Makarov <aleksey.makarov@linaro.org>
>>
>> [ .. ]
>>
> 

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

[Peter Hurley]

On 01/28/2016 05:23 AM, Aleksey Makarov wrote:
> 
> 
> On 01/28/2016 03:45 AM, Peter Hurley wrote:
>> On 01/27/2016 05:57 AM, Aleksey Makarov wrote:
>>>
>>>
>>> On 01/25/2016 07:32 PM, Peter Hurley wrote:
>>>> On 01/25/2016 03:45 AM, Aleksey Makarov wrote:
>>>>> 'ARM Server Base Boot Requiremets' [1] mention SPCR (Serial Port
>>>>> Console Redirection Table) [2] as a mandatory ACPI table that
>>>>> specifies the configuration of serial console.
>>>>>
>>>>> Parse this table and check if any registered console match the
>>>>> description.  If it does, enable that console.
>>>>>
>>>>> To implement that, introduce a new member int (*acpi_match)(struct
>>>>> console *, struct acpi_table_spcr *) of struct console.  It allows
>>>>> drivers to check if they provide a matching console device.
>>>>
>>>> Many, many platform proms with all sorts of binary table layout are
>>>> already supported by the existing console infrastructure. Why is ACPI
>>>> different, that requires extensive (and messy) changes to console
>>>> initialization?
>>>
>>> Without this patch, when linux calls register_console(), that function
>>> checks if any console has been enabled so far. 1) If not, it enables the
>>> console being registered. 2) If there exists any enabled console, it
>>> looks at the console_cmdline array. That array holds a list of
>>> consoles that user wishes to enable. There are two ways to append
>>> an item to that list: first is to pass "console=..." option in command
>>> line and second is to call add_preferred_console(char *name, int idx,
>>> char *options). As it is clear from the signature, the function
>>> requires the name of the driver (like "ttyS") and the line id. On the
>>> other hand, the SPCR ACPI table describes console by specifying the
>>> address of it's registers or PCI Device ID / PCI Vendor ID or PCI Bus
>>> Number / PCI Device Number. So to use this function we would need to
>>> have a method to translate this info to the name of terminal and line
>>> index. I could not figure out any way to do that.
>>
>> I'm not sure how this answers my question.
> 
> It does not.  For the actual answer please scroll down.

Where?
All I see is random descriptions of _what_ this patchset does, not _why_.

For example, below you say the first implementation had the wrong order
of initialization. I ask what the "wrong order" is, what the "right order"
should be and how adding "match_acpi()" fixes it.

Your reply:
> There is a race between initialization/registration of consoles and 
> parsing SPCR table.  By the time the console is registered we should 
> have SPCR table parsed and add_preferred_console() called.

1. There's no race because early kernel init is single-threaded.
2. If by "race", you mean static-but-wrong order-of-execution, then I'm
   left to assume you mean that the "right order" is
     a. parse the SPCR table
     b. add_preferred_console
     c. register_console

I agree that this is the correct order-of-initialization.
However, this is not what this patch does.

You continue:
> In the original submission add_preferred_console() was called from
> uart registration code, that requires to parse ACPI table very early
> (and nobody can say how early, because uarts can be registered very
> early.  pl011 is initialized with arch_initcall() for example).

You fail to describe why this is wrong.

You continue:
> I decided to fix this with retrying registration of consoles after
> ACPI is parsed.

Fix what? That your patchset doesn't mandate fixed order-of-initialization
between parsing ACPI for console information and initializing the uart
driver (or any console driver)?

Why not? What is *better* about randomizing the order-of-initialization?


*The reason I brought up earlycon* is because you won't be able to do
this same crazy deferred initialization with earlycon, so you'll be forced
to parse ACPI early, thus rendering the whole retry-console-initialization
pointless.



>> Which existing drivers/arch setup have you studied to conclude that
>> the existing console mechanisms don't work? 
> 
> I studied how add_preferred_console() is used in drivers/of/base.c,
> grepped the tree for it and studied each instance.
> Almost everything I see is dumb calls to this 
> function with hardcoded tty name and line index.  I don't see 
> how this can help.

But did you notice that _none_ of them needed retry loops?
   

>> Have you actually looked
>> at the in-tree callers of add_preferred_console()?
> 
> Yes I have. It looks like you are implying that there is some drivers/arch
> setup that would help to implement this correctly. Could you please
> give me a reference to it?

What does this do?

	add_preferred_console("uart", 0, "io,0x3f8,115200n8");


>>> In the initial version of the patch after getting the reference to the
>>> SPCR ACPI table the full tree of ACPI devices was searched to find any
>>> device with the same address.  When uart_add_one_port() was called
>>> to register a new serial port, the ACPI companion of this port was
>>> compared to the found device. If it was the same device, the code
>>> called add_preferred_console() (the terminal name and line index are
>>> known in uart_add_one_port()).
>>
>> Yeah, I wasn't a fan of that.
>>
>> But I think it was a bad choice to pick SPCR as table format, in the
>> first place. At least DBG2 has the actual ACPI device identifier :/
> 
> I am working on parsing DBG2 to implement earlycon based on 
> info from it.  As I understand,
> - SPCR specifies which existing console should be enabled (= made preferred).
> - DBG2 specifies how kernel can add a new earlycon to the system.
> These are two differend subsystems and both make sense.

This seems an arbitrary distinction.
Many setups just start an earlycon as an option of the normal console.
This is what devicetree does for "stdout-path".


> Also I don't understand how having (optional) ACPI device identifier
> in DBG2 table can help to implement earlycon introduction.

I didn't say it would. But it would help for _console_ because the
device association is clear (as opposed to searching by register address).
Earlycon doesn't need or use devices (in the device model sense).


>>> This original approach had two problems: 
>>>
>>>     1) It works with the SPCR tables that describe consoles only by
>>> the address of the registers. I do not think that consoles that are
>>> described by PCI info will appear in the near future, but decided to
>>> implement this in a generic way. I would like to discuss if this
>>> decision was good.
>>>
>>>     2) Wrong order of initialization.  Many console drivers have already
>>> been registered by the time uart_add_one_port() adds an item to the
>>> console_cmdline array.  There is a similar problem with my
>>> implementation,  but having a dedicated acpi_match() callback I
>>> believe made it simpler to circumwent.
>>
>> I don't see how the "wrong order of initialization" and the need for
>> acpi_match() correlate. What do you mean by "wrong order"? What is the
>> "right order"?
> 
> There is a race between initialization/registration of consoles and 
> parsing SPCR table.  By the time the console is registered we should 
> have SPCR table parsed and add_preferred_console() called.
> 
> In the original submission add_preferred_console() was called from
> uart registration code, that requires to parse ACPI table very early
> (and nobody can say how early, because uarts can be registered very
> early.  pl011 is initialized with arch_initcall() for example).
> 
> I decided to fix this with retrying registration of consoles after
> ACPI is parsed.  I do not like it either, but it is the simplest 
> solution I could suggest.  But it's quite difficult do implement 
> such deferring code in the uart registration and, more important,
> it is wrong as there may be non-uart consoles.
> 
> So acpi_match() is a part of registration retrying mechanism and
> the right place for it is at console registration.

Once you get DBG2 parsed so that you can start an earlycon from
that information, all this "retry" mechanism won't be necessary
because you'll be parsing ACPI early enough to start earlycon
which will be early enough to be before any console registrations too.
You can parse SPCR at the same time.

The way I see it you're taking the existing ACPI code as a given,
and trying to make the rest of the kernel workaround that.
But your approach really needs to be the other way around.

devicetree had to deal with many of these same issues, which is why
there is special-case devicetree code for parsing the flat devicetree
for earlycon startup and other stuff.

And devicetree manages to parse the "stdout-path" before device
initialization as well, which means there's no race wrt console
initialization.

Unfortunately, the easiest way is not always the best way.


>>> That's why I believe we need to add a new funcion pointer to struct
>>> console. On the other hand, I do not understand which existing
>>> structure you are referring.
>>>
>>>> How is this going to work with earlycon?
>>>
>>> If an earlycon that matches SPCR is being registered, the code will enable it.
>>
>> I think you should review how and when an earlycon is specified, initialized
>> and registered before you conclude that this will magically work.
> 
> I reviewed that again and the only issue that I can imagine is that
> ACPI can not make earlycon preferred.  Is it that you are afraid of?
> I believe it should not.  What problems do you see?

I was asking how this would _start_ an earlycon but I see now from
your answers elsewhere that you don't intend to start an earlycon
from the information in SPCR, ever.

I disagree on this distinction. If anything DBG2 should _also_ start a console.
And both should be capable of (optionally) starting an earlycon.

Regards,
Peter Hurley


>>> While it is harmless. Even so I will check for earlycon in the next version
>>> of the patch set, thank you.
>>>
>>>> This commit log is missing the reasoning behind adding locks,
>>>> refactoring into delete_from_console_list(), and retry loops.
>>>
>>> I will add this to the next verion of the series.
>>>
>>> Thank you
>>> Aleksey Makarov
>>>
>>>
>>>>> [1]
>>>>> http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.den0044a/index.html
>>>>>
>>>>> [2] http://msdn.microsoft.com/en-us/library/windows/hardware/dn639131(v=vs.85).aspx
>>>>>
>>>>> Signed-off-by: Aleksey Makarov <aleksey.makarov@linaro.org>
>>>
>>> [ .. ]
>>>
>>

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

[Peter Hurley]

On 01/28/2016 05:23 AM, Aleksey Makarov wrote:
> 
> 
> On 01/28/2016 03:45 AM, Peter Hurley wrote:
>> On 01/27/2016 05:57 AM, Aleksey Makarov wrote:
>>>
>>>
>>> On 01/25/2016 07:32 PM, Peter Hurley wrote:
>>>> On 01/25/2016 03:45 AM, Aleksey Makarov wrote:
>>>>> 'ARM Server Base Boot Requiremets' [1] mention SPCR (Serial Port
>>>>> Console Redirection Table) [2] as a mandatory ACPI table that
>>>>> specifies the configuration of serial console.
>>>>>
>>>>> Parse this table and check if any registered console match the
>>>>> description.  If it does, enable that console.
>>>>>
>>>>> To implement that, introduce a new member int (*acpi_match)(struct
>>>>> console *, struct acpi_table_spcr *) of struct console.  It allows
>>>>> drivers to check if they provide a matching console device.
>>>>
>>>> Many, many platform proms with all sorts of binary table layout are
>>>> already supported by the existing console infrastructure. Why is ACPI
>>>> different, that requires extensive (and messy) changes to console
>>>> initialization?
>>>
>>> Without this patch, when linux calls register_console(), that function
>>> checks if any console has been enabled so far. 1) If not, it enables the
>>> console being registered. 2) If there exists any enabled console, it
>>> looks at the console_cmdline array. That array holds a list of
>>> consoles that user wishes to enable. There are two ways to append
>>> an item to that list: first is to pass "console=..." option in command
>>> line and second is to call add_preferred_console(char *name, int idx,
>>> char *options). As it is clear from the signature, the function
>>> requires the name of the driver (like "ttyS") and the line id. On the
>>> other hand, the SPCR ACPI table describes console by specifying the
>>> address of it's registers or PCI Device ID / PCI Vendor ID or PCI Bus
>>> Number / PCI Device Number. So to use this function we would need to
>>> have a method to translate this info to the name of terminal and line
>>> index. I could not figure out any way to do that.
>>
>> I'm not sure how this answers my question.
> 
> It does not.  For the actual answer please scroll down.

Where?
All I see is random descriptions of _what_ this patchset does, not _why_.

For example, below you say the first implementation had the wrong order
of initialization. I ask what the "wrong order" is, what the "right order"
should be and how adding "match_acpi()" fixes it.

Your reply:
> There is a race between initialization/registration of consoles and 
> parsing SPCR table.  By the time the console is registered we should 
> have SPCR table parsed and add_preferred_console() called.

1. There's no race because early kernel init is single-threaded.
2. If by "race", you mean static-but-wrong order-of-execution, then I'm
   left to assume you mean that the "right order" is
     a. parse the SPCR table
     b. add_preferred_console
     c. register_console

I agree that this is the correct order-of-initialization.
However, this is not what this patch does.

You continue:
> In the original submission add_preferred_console() was called from
> uart registration code, that requires to parse ACPI table very early
> (and nobody can say how early, because uarts can be registered very
> early.  pl011 is initialized with arch_initcall() for example).

You fail to describe why this is wrong.

You continue:
> I decided to fix this with retrying registration of consoles after
> ACPI is parsed.

Fix what? That your patchset doesn't mandate fixed order-of-initialization
between parsing ACPI for console information and initializing the uart
driver (or any console driver)?

Why not? What is *better* about randomizing the order-of-initialization?


*The reason I brought up earlycon* is because you won't be able to do
this same crazy deferred initialization with earlycon, so you'll be forced
to parse ACPI early, thus rendering the whole retry-console-initialization
pointless.



>> Which existing drivers/arch setup have you studied to conclude that
>> the existing console mechanisms don't work? 
> 
> I studied how add_preferred_console() is used in drivers/of/base.c,
> grepped the tree for it and studied each instance.
> Almost everything I see is dumb calls to this 
> function with hardcoded tty name and line index.  I don't see 
> how this can help.

But did you notice that _none_ of them needed retry loops?
   

>> Have you actually looked
>> at the in-tree callers of add_preferred_console()?
> 
> Yes I have. It looks like you are implying that there is some drivers/arch
> setup that would help to implement this correctly. Could you please
> give me a reference to it?

What does this do?

	add_preferred_console("uart", 0, "io,0x3f8,115200n8");


>>> In the initial version of the patch after getting the reference to the
>>> SPCR ACPI table the full tree of ACPI devices was searched to find any
>>> device with the same address.  When uart_add_one_port() was called
>>> to register a new serial port, the ACPI companion of this port was
>>> compared to the found device. If it was the same device, the code
>>> called add_preferred_console() (the terminal name and line index are
>>> known in uart_add_one_port()).
>>
>> Yeah, I wasn't a fan of that.
>>
>> But I think it was a bad choice to pick SPCR as table format, in the
>> first place. At least DBG2 has the actual ACPI device identifier :/
> 
> I am working on parsing DBG2 to implement earlycon based on 
> info from it.  As I understand,
> - SPCR specifies which existing console should be enabled (= made preferred).
> - DBG2 specifies how kernel can add a new earlycon to the system.
> These are two differend subsystems and both make sense.

This seems an arbitrary distinction.
Many setups just start an earlycon as an option of the normal console.
This is what devicetree does for "stdout-path".


> Also I don't understand how having (optional) ACPI device identifier
> in DBG2 table can help to implement earlycon introduction.

I didn't say it would. But it would help for _console_ because the
device association is clear (as opposed to searching by register address).
Earlycon doesn't need or use devices (in the device model sense).


>>> This original approach had two problems: 
>>>
>>>     1) It works with the SPCR tables that describe consoles only by
>>> the address of the registers. I do not think that consoles that are
>>> described by PCI info will appear in the near future, but decided to
>>> implement this in a generic way. I would like to discuss if this
>>> decision was good.
>>>
>>>     2) Wrong order of initialization.  Many console drivers have already
>>> been registered by the time uart_add_one_port() adds an item to the
>>> console_cmdline array.  There is a similar problem with my
>>> implementation,  but having a dedicated acpi_match() callback I
>>> believe made it simpler to circumwent.
>>
>> I don't see how the "wrong order of initialization" and the need for
>> acpi_match() correlate. What do you mean by "wrong order"? What is the
>> "right order"?
> 
> There is a race between initialization/registration of consoles and 
> parsing SPCR table.  By the time the console is registered we should 
> have SPCR table parsed and add_preferred_console() called.
> 
> In the original submission add_preferred_console() was called from
> uart registration code, that requires to parse ACPI table very early
> (and nobody can say how early, because uarts can be registered very
> early.  pl011 is initialized with arch_initcall() for example).
> 
> I decided to fix this with retrying registration of consoles after
> ACPI is parsed.  I do not like it either, but it is the simplest 
> solution I could suggest.  But it's quite difficult do implement 
> such deferring code in the uart registration and, more important,
> it is wrong as there may be non-uart consoles.
> 
> So acpi_match() is a part of registration retrying mechanism and
> the right place for it is at console registration.

Once you get DBG2 parsed so that you can start an earlycon from
that information, all this "retry" mechanism won't be necessary
because you'll be parsing ACPI early enough to start earlycon
which will be early enough to be before any console registrations too.
You can parse SPCR at the same time.

The way I see it you're taking the existing ACPI code as a given,
and trying to make the rest of the kernel workaround that.
But your approach really needs to be the other way around.

devicetree had to deal with many of these same issues, which is why
there is special-case devicetree code for parsing the flat devicetree
for earlycon startup and other stuff.

And devicetree manages to parse the "stdout-path" before device
initialization as well, which means there's no race wrt console
initialization.

Unfortunately, the easiest way is not always the best way.


>>> That's why I believe we need to add a new funcion pointer to struct
>>> console. On the other hand, I do not understand which existing
>>> structure you are referring.
>>>
>>>> How is this going to work with earlycon?
>>>
>>> If an earlycon that matches SPCR is being registered, the code will enable it.
>>
>> I think you should review how and when an earlycon is specified, initialized
>> and registered before you conclude that this will magically work.
> 
> I reviewed that again and the only issue that I can imagine is that
> ACPI can not make earlycon preferred.  Is it that you are afraid of?
> I believe it should not.  What problems do you see?

I was asking how this would _start_ an earlycon but I see now from
your answers elsewhere that you don't intend to start an earlycon
from the information in SPCR, ever.

I disagree on this distinction. If anything DBG2 should _also_ start a console.
And both should be capable of (optionally) starting an earlycon.

Regards,
Peter Hurley


>>> While it is harmless. Even so I will check for earlycon in the next version
>>> of the patch set, thank you.
>>>
>>>> This commit log is missing the reasoning behind adding locks,
>>>> refactoring into delete_from_console_list(), and retry loops.
>>>
>>> I will add this to the next verion of the series.
>>>
>>> Thank you
>>> Aleksey Makarov
>>>
>>>
>>>>> [1]
>>>>> http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.den0044a/index.html
>>>>>
>>>>> [2] http://msdn.microsoft.com/en-us/library/windows/hardware/dn639131(v=vs.85).aspx
>>>>>
>>>>> Signed-off-by: Aleksey Makarov <aleksey.makarov@linaro.org>
>>>
>>> [ .. ]
>>>
>>

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

[Jon Masters]

On 01/27/2016 08:45 AM, One Thousand Gnomes wrote:
> On Wed, 27 Jan 2016 15:17:52 +0300
> Aleksey Makarov <aleksey.makarov@linaro.org> wrote:
> 
>> On 01/25/2016 07:11 PM, Peter Hurley wrote:
>>> On 01/25/2016 03:45 AM, Aleksey Makarov wrote:  
>>>> This patchset is based on the patchset by Leif Lindholm [1]
>>>>
>>>> 'ARM Server Base Boot Requirements' [2] mention SPCR 
>>>> (Serial Port Console Redirection Table) [3] as a mandatory
>>>> ACPI table that specifies the configuration of serial console.
>>>>
>>>> Licensing concerns have prevented implementing it in the past, but as of
>>>> 10 August 2015, these tables have both been released also under 
>>>> OWF 1.0 [4].  
>>>
>>> This license has a patent retaliation provision, which makes it
>>> incompatible with GPLv2.
>>>
>>> *If the license applies to this code*, then this patch set does not
>>> meet the criteria for submission.  
>>
>> The license applies not to this code but to the document describing the tables.
>>
>> Here is an excerpt from it:
>>
>>   Patent Notice:
>>   Microsoft is making certain patent rights available for implementations of this specification under two options:
>>   1)  Microsoft’s Community Promise, available at
>>   http://www.microsoft.com/openspecifications/en/us/programs/community-promise/default.aspx; or
>>   2)  The Open Web Foundation Final Specification Agreement Version 1.0 ("OWF 1.0")
>>   as of October 1, 2012, available at http://www.openwebfoundation.org/legal/the-owf-1-0-agreements/owfa-1-0. 
>>
>> I believe that it means that the patch set meets the criteria for submission.  Am I right?
> 
> This is not a forum for legal advice. I would suggest that Linaro
> discusses it privately with the Linux Foundation and Linus and does so
> under attorney-client privilege. The Linux Foundation does have some
> reasons to exist.

Thanks for the reply, Alan. Can folks discuss and let me know off list
if another license change is subsequently requested for this table? I
spoke with Microsoft about this and they graciously made the previous
license change. I am willing to connect additional dots off-list.

Jon.

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

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

[Jon Masters]

On 01/27/2016 08:45 AM, One Thousand Gnomes wrote:
> On Wed, 27 Jan 2016 15:17:52 +0300
> Aleksey Makarov <aleksey.makarov@linaro.org> wrote:
> 
>> On 01/25/2016 07:11 PM, Peter Hurley wrote:
>>> On 01/25/2016 03:45 AM, Aleksey Makarov wrote:  
>>>> This patchset is based on the patchset by Leif Lindholm [1]
>>>>
>>>> 'ARM Server Base Boot Requirements' [2] mention SPCR 
>>>> (Serial Port Console Redirection Table) [3] as a mandatory
>>>> ACPI table that specifies the configuration of serial console.
>>>>
>>>> Licensing concerns have prevented implementing it in the past, but as of
>>>> 10 August 2015, these tables have both been released also under 
>>>> OWF 1.0 [4].  
>>>
>>> This license has a patent retaliation provision, which makes it
>>> incompatible with GPLv2.
>>>
>>> *If the license applies to this code*, then this patch set does not
>>> meet the criteria for submission.  
>>
>> The license applies not to this code but to the document describing the tables.
>>
>> Here is an excerpt from it:
>>
>>   Patent Notice:
>>   Microsoft is making certain patent rights available for implementations of this specification under two options:
>>   1)  Microsoft’s Community Promise, available at
>>   http://www.microsoft.com/openspecifications/en/us/programs/community-promise/default.aspx; or
>>   2)  The Open Web Foundation Final Specification Agreement Version 1.0 ("OWF 1.0")
>>   as of October 1, 2012, available at http://www.openwebfoundation.org/legal/the-owf-1-0-agreements/owfa-1-0. 
>>
>> I believe that it means that the patch set meets the criteria for submission.  Am I right?
> 
> This is not a forum for legal advice. I would suggest that Linaro
> discusses it privately with the Linux Foundation and Linus and does so
> under attorney-client privilege. The Linux Foundation does have some
> reasons to exist.

Thanks for the reply, Alan. Can folks discuss and let me know off list
if another license change is subsequently requested for this table? I
spoke with Microsoft about this and they graciously made the previous
license change. I am willing to connect additional dots off-list.

Jon.

[PATCH 0/3] ACPI: parse the SPCR table

[Jon Masters]

On 01/27/2016 08:45 AM, One Thousand Gnomes wrote:
> On Wed, 27 Jan 2016 15:17:52 +0300
> Aleksey Makarov <aleksey.makarov@linaro.org> wrote:
> 
>> On 01/25/2016 07:11 PM, Peter Hurley wrote:
>>> On 01/25/2016 03:45 AM, Aleksey Makarov wrote:  
>>>> This patchset is based on the patchset by Leif Lindholm [1]
>>>>
>>>> 'ARM Server Base Boot Requirements' [2] mention SPCR 
>>>> (Serial Port Console Redirection Table) [3] as a mandatory
>>>> ACPI table that specifies the configuration of serial console.
>>>>
>>>> Licensing concerns have prevented implementing it in the past, but as of
>>>> 10 August 2015, these tables have both been released also under 
>>>> OWF 1.0 [4].  
>>>
>>> This license has a patent retaliation provision, which makes it
>>> incompatible with GPLv2.
>>>
>>> *If the license applies to this code*, then this patch set does not
>>> meet the criteria for submission.  
>>
>> The license applies not to this code but to the document describing the tables.
>>
>> Here is an excerpt from it:
>>
>>   Patent Notice:
>>   Microsoft is making certain patent rights available for implementations of this specification under two options:
>>   1)  Microsoft?s Community Promise, available at
>>   http://www.microsoft.com/openspecifications/en/us/programs/community-promise/default.aspx; or
>>   2)  The Open Web Foundation Final Specification Agreement Version 1.0 ("OWF 1.0")
>>   as of October 1, 2012, available at http://www.openwebfoundation.org/legal/the-owf-1-0-agreements/owfa-1-0. 
>>
>> I believe that it means that the patch set meets the criteria for submission.  Am I right?
> 
> This is not a forum for legal advice. I would suggest that Linaro
> discusses it privately with the Linux Foundation and Linus and does so
> under attorney-client privilege. The Linux Foundation does have some
> reasons to exist.

Thanks for the reply, Alan. Can folks discuss and let me know off list
if another license change is subsequently requested for this table? I
spoke with Microsoft about this and they graciously made the previous
license change. I am willing to connect additional dots off-list.

Jon.

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

[Graeme Gregory]

On Mon, Jan 25, 2016 at 05:45:22PM +0600, Aleksey Makarov wrote:
> 'ARM Server Base Boot Requiremets' [1] mention SPCR
> (Serial Port Console Redirection Table) [2] as a mandatory ACPI table
> that specifies the configuration of serial console.
> 
> Parse this table and check if any registered console match
> the description.  If it does, enable that console.
> 
> To implement that, introduce a new member
> int (*acpi_match)(struct console *, struct acpi_table_spcr *)
> of struct console.  It allows drivers to check if they provide
> a matching console device.
> 

Fails to compile on x86

kernel/built-in.o: In function `register_console':
(.text+0x48cba): undefined reference to `console_acpi_match'
Makefile:929: recipe for target 'vmlinux' failed
make: *** [vmlinux] Error 1

Because CONFIG_ACPI_SPCR_TABLE is never set.

Graeme


> [1] http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.den0044a/index.html
> [2] http://msdn.microsoft.com/en-us/library/windows/hardware/dn639131(v=vs.85).aspx
> 
> Signed-off-by: Aleksey Makarov <aleksey.makarov@linaro.org>
> ---
>  arch/arm64/Kconfig      |  1 +
>  drivers/acpi/Kconfig    |  3 ++
>  drivers/acpi/Makefile   |  1 +
>  drivers/acpi/spcr.c     | 85 +++++++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/console.h | 12 +++++++
>  kernel/printk/printk.c  | 82 +++++++++++++++++++++++++++++++++++++----------
>  6 files changed, 167 insertions(+), 17 deletions(-)
>  create mode 100644 drivers/acpi/spcr.c
> 
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 573bebc..bf31e3c 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_ATOMIC64_DEC_IF_POSITIVE
>  	select ARCH_HAS_ELF_RANDOMIZE
>  	select ARCH_HAS_GCOV_PROFILE_ALL
> diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
> index e315061..142a338 100644
> --- a/drivers/acpi/Kconfig
> +++ b/drivers/acpi/Kconfig
> @@ -60,6 +60,9 @@ config ACPI_CCA_REQUIRED
>  config IORT_TABLE
>  	bool
>  
> +config ACPI_SPCR_TABLE
> +	bool
> +
>  config ACPI_DEBUGGER
>  	bool "AML debugger interface (EXPERIMENTAL)"
>  	select ACPI_DEBUG
> diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
> index 265eb90..8316859 100644
> --- a/drivers/acpi/Makefile
> +++ b/drivers/acpi/Makefile
> @@ -81,6 +81,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_IORT_TABLE) 	+= iort.o
> +obj-$(CONFIG_ACPI_SPCR_TABLE)	+= spcr.o
>  
>  # processor has its own "processor." module_param namespace
>  processor-y			:= processor_driver.o
> diff --git a/drivers/acpi/spcr.c b/drivers/acpi/spcr.c
> new file mode 100644
> index 0000000..ccb19a0
> --- /dev/null
> +++ b/drivers/acpi/spcr.c
> @@ -0,0 +1,85 @@
> +/*
> + * 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>
> +
> +static struct acpi_table_spcr *spcr_table;
> +
> +int console_acpi_match(struct console *c, char **options)
> +{
> +	int err;
> +
> +	if (!c->acpi_match)
> +		return -ENODEV;
> +
> +	if (!spcr_table)
> +		return -EAGAIN;
> +
> +	err = c->acpi_match(c, spcr_table);
> +	if (err < 0)
> +		return err;
> +
> +	if (options) {
> +		switch (spcr_table->baud_rate) {
> +		case 3:
> +			*options = "9600";
> +			break;
> +		case 4:
> +			*options = "19200";
> +			break;
> +		case 6:
> +			*options = "57600";
> +			break;
> +		case 7:
> +			*options = "115200";
> +			break;
> +		default:
> +			*options = "";
> +			break;
> +		}
> +	}
> +
> +	return err;
> +}
> +
> +static int __init spcr_table_detect(void)
> +{
> +	struct acpi_table_header *table;
> +	acpi_status status;
> +
> +	if (acpi_disabled)
> +		return -ENODEV;
> +
> +	status = acpi_get_table(ACPI_SIG_SPCR, 0, &table);
> +	if (ACPI_FAILURE(status)) {
> +		const char *msg = acpi_format_exception(status);
> +
> +		pr_err("Failed to get table, %s\n", msg);
> +		return -EINVAL;
> +	}
> +
> +	if (table->revision < 2)
> +		return -EOPNOTSUPP;
> +
> +	spcr_table = (struct acpi_table_spcr *)table;
> +
> +	pr_info("Console at 0x%016llx\n", spcr_table->serial_port.address);
> +
> +	acpi_register_consoles_try_again();
> +
> +	return 0;
> +}
> +
> +arch_initcall(spcr_table_detect);
> diff --git a/include/linux/console.h b/include/linux/console.h
> index bd19434..94d0bd8 100644
> --- a/include/linux/console.h
> +++ b/include/linux/console.h
> @@ -117,6 +117,7 @@ static inline int con_debug_leave(void)
>  #define CON_BRL		(32) /* Used for a braille device */
>  #define CON_EXTENDED	(64) /* Use the extended output format a la /dev/kmsg */
>  
> +struct acpi_table_spcr;
>  struct console {
>  	char	name[16];
>  	void	(*write)(struct console *, const char *, unsigned);
> @@ -125,6 +126,7 @@ struct console {
>  	void	(*unblank)(void);
>  	int	(*setup)(struct console *, char *);
>  	int	(*match)(struct console *, char *name, int idx, char *options);
> +	int	(*acpi_match)(struct console *, struct acpi_table_spcr *);
>  	short	flags;
>  	short	index;
>  	int	cflag;
> @@ -132,6 +134,16 @@ struct console {
>  	struct	 console *next;
>  };
>  
> +#ifdef CONFIG_ACPI
> +int console_acpi_match(struct console *c, char **options);
> +#else
> +static inline int console_acpi_match(struct console *c, char **options)
> +{
> +	return -ENODEV;
> +}
> +#endif
> +void acpi_register_consoles_try_again(void);
> +
>  /*
>   * for_each_console() allows you to iterate on each console
>   */
> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> index 37e531f..3cf8cba 100644
> --- a/kernel/printk/printk.c
> +++ b/kernel/printk/printk.c
> @@ -2430,6 +2430,25 @@ static int __init keep_bootcon_setup(char *str)
>  
>  early_param("keep_bootcon", keep_bootcon_setup);
>  
> +static DEFINE_MUTEX(acpi_consoles_delayed_mutex);
> +static struct console *acpi_consoles_delayed;
> +
> +void acpi_register_consoles_try_again(void)
> +{
> +	mutex_lock(&acpi_consoles_delayed_mutex);
> +	while (acpi_consoles_delayed) {
> +
> +		struct console *c = acpi_consoles_delayed;
> +
> +		acpi_consoles_delayed = acpi_consoles_delayed->next;
> +
> +		mutex_unlock(&acpi_consoles_delayed_mutex);
> +		register_console(c);
> +		mutex_lock(&acpi_consoles_delayed_mutex);
> +	}
> +	mutex_unlock(&acpi_consoles_delayed_mutex);
> +}
> +
>  /*
>   * The console driver calls this routine during kernel initialization
>   * to register the console printing procedure with printk() and to
> @@ -2538,8 +2557,30 @@ void register_console(struct console *newcon)
>  		break;
>  	}
>  
> -	if (!(newcon->flags & CON_ENABLED))
> -		return;
> +	if (!(newcon->flags & CON_ENABLED)) {
> +		char *opts;
> +		int err;
> +
> +		if (newcon->index < 0)
> +			newcon->index = 0;
> +
> +		err = console_acpi_match(newcon, &opts);
> +
> +		if (err == -EAGAIN) {
> +			mutex_lock(&acpi_consoles_delayed_mutex);
> +			newcon->next = acpi_consoles_delayed;
> +			acpi_consoles_delayed = newcon;
> +			mutex_unlock(&acpi_consoles_delayed_mutex);
> +			return;
> +		} else if (err < 0) {
> +			return;
> +		} else {
> +			if (newcon->setup && newcon->setup(newcon, opts) != 0)
> +				return;
> +			newcon->flags |= CON_ENABLED | CON_CONSDEV;
> +			preferred_console = true;
> +		}
> +	}
>  
>  	/*
>  	 * If we have a bootconsole, and are switching to a real console,
> @@ -2612,34 +2653,41 @@ void register_console(struct console *newcon)
>  }
>  EXPORT_SYMBOL(register_console);
>  
> +static int delete_from_console_list(struct console **list, struct console *c)
> +{
> +	while (*list) {
> +		struct console *cur = *list;
> +
> +		if (cur == c) {
> +			*list = cur->next;
> +			return 0;
> +		}
> +		list = &cur->next;
> +	}
> +	return 1;
> +}
> +
>  int unregister_console(struct console *console)
>  {
> -        struct console *a, *b;
>  	int res;
>  
>  	pr_info("%sconsole [%s%d] disabled\n",
>  		(console->flags & CON_BOOT) ? "boot" : "" ,
>  		console->name, console->index);
>  
> +	mutex_lock(&acpi_consoles_delayed_mutex);
> +	res = delete_from_console_list(&acpi_consoles_delayed, console);
> +	mutex_unlock(&acpi_consoles_delayed_mutex);
> +	if (res == 0)
> +		return res;
> +
>  	res = _braille_unregister_console(console);
>  	if (res)
>  		return res;
>  
> -	res = 1;
>  	console_lock();
> -	if (console_drivers == console) {
> -		console_drivers=console->next;
> -		res = 0;
> -	} else if (console_drivers) {
> -		for (a=console_drivers->next, b=console_drivers ;
> -		     a; b=a, a=b->next) {
> -			if (a == console) {
> -				b->next = a->next;
> -				res = 0;
> -				break;
> -			}
> -		}
> -	}
> +
> +	res = delete_from_console_list(&console_drivers, console);
>  
>  	if (!res && (console->flags & CON_EXTENDED))
>  		nr_ext_console_drivers--;
> -- 
> 2.7.0
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Graeme Gregory]

On Mon, Jan 25, 2016 at 05:45:22PM +0600, Aleksey Makarov wrote:
> 'ARM Server Base Boot Requiremets' [1] mention SPCR
> (Serial Port Console Redirection Table) [2] as a mandatory ACPI table
> that specifies the configuration of serial console.
> 
> Parse this table and check if any registered console match
> the description.  If it does, enable that console.
> 
> To implement that, introduce a new member
> int (*acpi_match)(struct console *, struct acpi_table_spcr *)
> of struct console.  It allows drivers to check if they provide
> a matching console device.
> 

Fails to compile on x86

kernel/built-in.o: In function `register_console':
(.text+0x48cba): undefined reference to `console_acpi_match'
Makefile:929: recipe for target 'vmlinux' failed
make: *** [vmlinux] Error 1

Because CONFIG_ACPI_SPCR_TABLE is never set.

Graeme


> [1] http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.den0044a/index.html
> [2] http://msdn.microsoft.com/en-us/library/windows/hardware/dn639131(v=vs.85).aspx
> 
> Signed-off-by: Aleksey Makarov <aleksey.makarov@linaro.org>
> ---
>  arch/arm64/Kconfig      |  1 +
>  drivers/acpi/Kconfig    |  3 ++
>  drivers/acpi/Makefile   |  1 +
>  drivers/acpi/spcr.c     | 85 +++++++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/console.h | 12 +++++++
>  kernel/printk/printk.c  | 82 +++++++++++++++++++++++++++++++++++++----------
>  6 files changed, 167 insertions(+), 17 deletions(-)
>  create mode 100644 drivers/acpi/spcr.c
> 
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 573bebc..bf31e3c 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_ATOMIC64_DEC_IF_POSITIVE
>  	select ARCH_HAS_ELF_RANDOMIZE
>  	select ARCH_HAS_GCOV_PROFILE_ALL
> diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
> index e315061..142a338 100644
> --- a/drivers/acpi/Kconfig
> +++ b/drivers/acpi/Kconfig
> @@ -60,6 +60,9 @@ config ACPI_CCA_REQUIRED
>  config IORT_TABLE
>  	bool
>  
> +config ACPI_SPCR_TABLE
> +	bool
> +
>  config ACPI_DEBUGGER
>  	bool "AML debugger interface (EXPERIMENTAL)"
>  	select ACPI_DEBUG
> diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
> index 265eb90..8316859 100644
> --- a/drivers/acpi/Makefile
> +++ b/drivers/acpi/Makefile
> @@ -81,6 +81,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_IORT_TABLE) 	+= iort.o
> +obj-$(CONFIG_ACPI_SPCR_TABLE)	+= spcr.o
>  
>  # processor has its own "processor." module_param namespace
>  processor-y			:= processor_driver.o
> diff --git a/drivers/acpi/spcr.c b/drivers/acpi/spcr.c
> new file mode 100644
> index 0000000..ccb19a0
> --- /dev/null
> +++ b/drivers/acpi/spcr.c
> @@ -0,0 +1,85 @@
> +/*
> + * 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>
> +
> +static struct acpi_table_spcr *spcr_table;
> +
> +int console_acpi_match(struct console *c, char **options)
> +{
> +	int err;
> +
> +	if (!c->acpi_match)
> +		return -ENODEV;
> +
> +	if (!spcr_table)
> +		return -EAGAIN;
> +
> +	err = c->acpi_match(c, spcr_table);
> +	if (err < 0)
> +		return err;
> +
> +	if (options) {
> +		switch (spcr_table->baud_rate) {
> +		case 3:
> +			*options = "9600";
> +			break;
> +		case 4:
> +			*options = "19200";
> +			break;
> +		case 6:
> +			*options = "57600";
> +			break;
> +		case 7:
> +			*options = "115200";
> +			break;
> +		default:
> +			*options = "";
> +			break;
> +		}
> +	}
> +
> +	return err;
> +}
> +
> +static int __init spcr_table_detect(void)
> +{
> +	struct acpi_table_header *table;
> +	acpi_status status;
> +
> +	if (acpi_disabled)
> +		return -ENODEV;
> +
> +	status = acpi_get_table(ACPI_SIG_SPCR, 0, &table);
> +	if (ACPI_FAILURE(status)) {
> +		const char *msg = acpi_format_exception(status);
> +
> +		pr_err("Failed to get table, %s\n", msg);
> +		return -EINVAL;
> +	}
> +
> +	if (table->revision < 2)
> +		return -EOPNOTSUPP;
> +
> +	spcr_table = (struct acpi_table_spcr *)table;
> +
> +	pr_info("Console at 0x%016llx\n", spcr_table->serial_port.address);
> +
> +	acpi_register_consoles_try_again();
> +
> +	return 0;
> +}
> +
> +arch_initcall(spcr_table_detect);
> diff --git a/include/linux/console.h b/include/linux/console.h
> index bd19434..94d0bd8 100644
> --- a/include/linux/console.h
> +++ b/include/linux/console.h
> @@ -117,6 +117,7 @@ static inline int con_debug_leave(void)
>  #define CON_BRL		(32) /* Used for a braille device */
>  #define CON_EXTENDED	(64) /* Use the extended output format a la /dev/kmsg */
>  
> +struct acpi_table_spcr;
>  struct console {
>  	char	name[16];
>  	void	(*write)(struct console *, const char *, unsigned);
> @@ -125,6 +126,7 @@ struct console {
>  	void	(*unblank)(void);
>  	int	(*setup)(struct console *, char *);
>  	int	(*match)(struct console *, char *name, int idx, char *options);
> +	int	(*acpi_match)(struct console *, struct acpi_table_spcr *);
>  	short	flags;
>  	short	index;
>  	int	cflag;
> @@ -132,6 +134,16 @@ struct console {
>  	struct	 console *next;
>  };
>  
> +#ifdef CONFIG_ACPI
> +int console_acpi_match(struct console *c, char **options);
> +#else
> +static inline int console_acpi_match(struct console *c, char **options)
> +{
> +	return -ENODEV;
> +}
> +#endif
> +void acpi_register_consoles_try_again(void);
> +
>  /*
>   * for_each_console() allows you to iterate on each console
>   */
> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> index 37e531f..3cf8cba 100644
> --- a/kernel/printk/printk.c
> +++ b/kernel/printk/printk.c
> @@ -2430,6 +2430,25 @@ static int __init keep_bootcon_setup(char *str)
>  
>  early_param("keep_bootcon", keep_bootcon_setup);
>  
> +static DEFINE_MUTEX(acpi_consoles_delayed_mutex);
> +static struct console *acpi_consoles_delayed;
> +
> +void acpi_register_consoles_try_again(void)
> +{
> +	mutex_lock(&acpi_consoles_delayed_mutex);
> +	while (acpi_consoles_delayed) {
> +
> +		struct console *c = acpi_consoles_delayed;
> +
> +		acpi_consoles_delayed = acpi_consoles_delayed->next;
> +
> +		mutex_unlock(&acpi_consoles_delayed_mutex);
> +		register_console(c);
> +		mutex_lock(&acpi_consoles_delayed_mutex);
> +	}
> +	mutex_unlock(&acpi_consoles_delayed_mutex);
> +}
> +
>  /*
>   * The console driver calls this routine during kernel initialization
>   * to register the console printing procedure with printk() and to
> @@ -2538,8 +2557,30 @@ void register_console(struct console *newcon)
>  		break;
>  	}
>  
> -	if (!(newcon->flags & CON_ENABLED))
> -		return;
> +	if (!(newcon->flags & CON_ENABLED)) {
> +		char *opts;
> +		int err;
> +
> +		if (newcon->index < 0)
> +			newcon->index = 0;
> +
> +		err = console_acpi_match(newcon, &opts);
> +
> +		if (err == -EAGAIN) {
> +			mutex_lock(&acpi_consoles_delayed_mutex);
> +			newcon->next = acpi_consoles_delayed;
> +			acpi_consoles_delayed = newcon;
> +			mutex_unlock(&acpi_consoles_delayed_mutex);
> +			return;
> +		} else if (err < 0) {
> +			return;
> +		} else {
> +			if (newcon->setup && newcon->setup(newcon, opts) != 0)
> +				return;
> +			newcon->flags |= CON_ENABLED | CON_CONSDEV;
> +			preferred_console = true;
> +		}
> +	}
>  
>  	/*
>  	 * If we have a bootconsole, and are switching to a real console,
> @@ -2612,34 +2653,41 @@ void register_console(struct console *newcon)
>  }
>  EXPORT_SYMBOL(register_console);
>  
> +static int delete_from_console_list(struct console **list, struct console *c)
> +{
> +	while (*list) {
> +		struct console *cur = *list;
> +
> +		if (cur == c) {
> +			*list = cur->next;
> +			return 0;
> +		}
> +		list = &cur->next;
> +	}
> +	return 1;
> +}
> +
>  int unregister_console(struct console *console)
>  {
> -        struct console *a, *b;
>  	int res;
>  
>  	pr_info("%sconsole [%s%d] disabled\n",
>  		(console->flags & CON_BOOT) ? "boot" : "" ,
>  		console->name, console->index);
>  
> +	mutex_lock(&acpi_consoles_delayed_mutex);
> +	res = delete_from_console_list(&acpi_consoles_delayed, console);
> +	mutex_unlock(&acpi_consoles_delayed_mutex);
> +	if (res == 0)
> +		return res;
> +
>  	res = _braille_unregister_console(console);
>  	if (res)
>  		return res;
>  
> -	res = 1;
>  	console_lock();
> -	if (console_drivers == console) {
> -		console_drivers=console->next;
> -		res = 0;
> -	} else if (console_drivers) {
> -		for (a=console_drivers->next, b=console_drivers ;
> -		     a; b=a, a=b->next) {
> -			if (a == console) {
> -				b->next = a->next;
> -				res = 0;
> -				break;
> -			}
> -		}
> -	}
> +
> +	res = delete_from_console_list(&console_drivers, console);
>  
>  	if (!res && (console->flags & CON_EXTENDED))
>  		nr_ext_console_drivers--;
> -- 
> 2.7.0
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Graeme Gregory]

On Mon, Feb 01, 2016 at 09:01:26AM +0000, Graeme Gregory wrote:
> On Mon, Jan 25, 2016 at 05:45:22PM +0600, Aleksey Makarov wrote:
> > 'ARM Server Base Boot Requiremets' [1] mention SPCR
> > (Serial Port Console Redirection Table) [2] as a mandatory ACPI table
> > that specifies the configuration of serial console.
> > 
> > Parse this table and check if any registered console match
> > the description.  If it does, enable that console.
> > 
> > To implement that, introduce a new member
> > int (*acpi_match)(struct console *, struct acpi_table_spcr *)
> > of struct console.  It allows drivers to check if they provide
> > a matching console device.
> > 
> 
> Fails to compile on x86
> 
> kernel/built-in.o: In function `register_console':
> (.text+0x48cba): undefined reference to `console_acpi_match'
> Makefile:929: recipe for target 'vmlinux' failed
> make: *** [vmlinux] Error 1
> 
> Because CONFIG_ACPI_SPCR_TABLE is never set.
> 
> Graeme
> 
> 
> > [1] http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.den0044a/index.html
> > [2] http://msdn.microsoft.com/en-us/library/windows/hardware/dn639131(v=vs.85).aspx
> > 
> > Signed-off-by: Aleksey Makarov <aleksey.makarov@linaro.org>
> > ---
> >  arch/arm64/Kconfig      |  1 +
> >  drivers/acpi/Kconfig    |  3 ++
> >  drivers/acpi/Makefile   |  1 +
> >  drivers/acpi/spcr.c     | 85 +++++++++++++++++++++++++++++++++++++++++++++++++
> >  include/linux/console.h | 12 +++++++
> >  kernel/printk/printk.c  | 82 +++++++++++++++++++++++++++++++++++++----------
> >  6 files changed, 167 insertions(+), 17 deletions(-)
> >  create mode 100644 drivers/acpi/spcr.c
> > 
> > diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> > index 573bebc..bf31e3c 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_ATOMIC64_DEC_IF_POSITIVE
> >  	select ARCH_HAS_ELF_RANDOMIZE
> >  	select ARCH_HAS_GCOV_PROFILE_ALL
> > diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
> > index e315061..142a338 100644
> > --- a/drivers/acpi/Kconfig
> > +++ b/drivers/acpi/Kconfig
> > @@ -60,6 +60,9 @@ config ACPI_CCA_REQUIRED
> >  config IORT_TABLE
> >  	bool
> >  
> > +config ACPI_SPCR_TABLE
> > +	bool
> > +
> >  config ACPI_DEBUGGER
> >  	bool "AML debugger interface (EXPERIMENTAL)"
> >  	select ACPI_DEBUG
> > diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
> > index 265eb90..8316859 100644
> > --- a/drivers/acpi/Makefile
> > +++ b/drivers/acpi/Makefile
> > @@ -81,6 +81,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_IORT_TABLE) 	+= iort.o
> > +obj-$(CONFIG_ACPI_SPCR_TABLE)	+= spcr.o
> >  
> >  # processor has its own "processor." module_param namespace
> >  processor-y			:= processor_driver.o
> > diff --git a/drivers/acpi/spcr.c b/drivers/acpi/spcr.c
> > new file mode 100644
> > index 0000000..ccb19a0
> > --- /dev/null
> > +++ b/drivers/acpi/spcr.c
> > @@ -0,0 +1,85 @@
> > +/*
> > + * 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>
> > +
> > +static struct acpi_table_spcr *spcr_table;
> > +
> > +int console_acpi_match(struct console *c, char **options)
> > +{
> > +	int err;
> > +
> > +	if (!c->acpi_match)
> > +		return -ENODEV;
> > +
> > +	if (!spcr_table)
> > +		return -EAGAIN;
> > +
> > +	err = c->acpi_match(c, spcr_table);
> > +	if (err < 0)
> > +		return err;
> > +
> > +	if (options) {
> > +		switch (spcr_table->baud_rate) {
> > +		case 3:
> > +			*options = "9600";
> > +			break;
> > +		case 4:
> > +			*options = "19200";
> > +			break;
> > +		case 6:
> > +			*options = "57600";
> > +			break;
> > +		case 7:
> > +			*options = "115200";
> > +			break;
> > +		default:
> > +			*options = "";
> > +			break;
> > +		}
> > +	}
> > +
> > +	return err;
> > +}
> > +
> > +static int __init spcr_table_detect(void)
> > +{
> > +	struct acpi_table_header *table;
> > +	acpi_status status;
> > +
> > +	if (acpi_disabled)
> > +		return -ENODEV;
> > +
> > +	status = acpi_get_table(ACPI_SIG_SPCR, 0, &table);
> > +	if (ACPI_FAILURE(status)) {
> > +		const char *msg = acpi_format_exception(status);
> > +
> > +		pr_err("Failed to get table, %s\n", msg);
> > +		return -EINVAL;
> > +	}
> > +
> > +	if (table->revision < 2)
> > +		return -EOPNOTSUPP;
> > +
> > +	spcr_table = (struct acpi_table_spcr *)table;
> > +
> > +	pr_info("Console at 0x%016llx\n", spcr_table->serial_port.address);
> > +
> > +	acpi_register_consoles_try_again();
> > +
> > +	return 0;
> > +}
> > +
> > +arch_initcall(spcr_table_detect);
> > diff --git a/include/linux/console.h b/include/linux/console.h
> > index bd19434..94d0bd8 100644
> > --- a/include/linux/console.h
> > +++ b/include/linux/console.h
> > @@ -117,6 +117,7 @@ static inline int con_debug_leave(void)
> >  #define CON_BRL		(32) /* Used for a braille device */
> >  #define CON_EXTENDED	(64) /* Use the extended output format a la /dev/kmsg */
> >  
> > +struct acpi_table_spcr;
> >  struct console {
> >  	char	name[16];
> >  	void	(*write)(struct console *, const char *, unsigned);
> > @@ -125,6 +126,7 @@ struct console {
> >  	void	(*unblank)(void);
> >  	int	(*setup)(struct console *, char *);
> >  	int	(*match)(struct console *, char *name, int idx, char *options);
> > +	int	(*acpi_match)(struct console *, struct acpi_table_spcr *);
> >  	short	flags;
> >  	short	index;
> >  	int	cflag;
> > @@ -132,6 +134,16 @@ struct console {
> >  	struct	 console *next;
> >  };
> >  
> > +#ifdef CONFIG_ACPI

This should be #ifdef CONFIG_ACPI_SPCR_TABLE

> > +int console_acpi_match(struct console *c, char **options);
> > +#else
> > +static inline int console_acpi_match(struct console *c, char **options)
> > +{
> > +	return -ENODEV;

This requires a #include <linux/errno.h> at the top of the file to work.

Graeme

> > +}
> > +#endif
> > +void acpi_register_consoles_try_again(void);
> > +
> >  /*
> >   * for_each_console() allows you to iterate on each console
> >   */
> > diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> > index 37e531f..3cf8cba 100644
> > --- a/kernel/printk/printk.c
> > +++ b/kernel/printk/printk.c
> > @@ -2430,6 +2430,25 @@ static int __init keep_bootcon_setup(char *str)
> >  
> >  early_param("keep_bootcon", keep_bootcon_setup);
> >  
> > +static DEFINE_MUTEX(acpi_consoles_delayed_mutex);
> > +static struct console *acpi_consoles_delayed;
> > +
> > +void acpi_register_consoles_try_again(void)
> > +{
> > +	mutex_lock(&acpi_consoles_delayed_mutex);
> > +	while (acpi_consoles_delayed) {
> > +
> > +		struct console *c = acpi_consoles_delayed;
> > +
> > +		acpi_consoles_delayed = acpi_consoles_delayed->next;
> > +
> > +		mutex_unlock(&acpi_consoles_delayed_mutex);
> > +		register_console(c);
> > +		mutex_lock(&acpi_consoles_delayed_mutex);
> > +	}
> > +	mutex_unlock(&acpi_consoles_delayed_mutex);
> > +}
> > +
> >  /*
> >   * The console driver calls this routine during kernel initialization
> >   * to register the console printing procedure with printk() and to
> > @@ -2538,8 +2557,30 @@ void register_console(struct console *newcon)
> >  		break;
> >  	}
> >  
> > -	if (!(newcon->flags & CON_ENABLED))
> > -		return;
> > +	if (!(newcon->flags & CON_ENABLED)) {
> > +		char *opts;
> > +		int err;
> > +
> > +		if (newcon->index < 0)
> > +			newcon->index = 0;
> > +
> > +		err = console_acpi_match(newcon, &opts);
> > +
> > +		if (err == -EAGAIN) {
> > +			mutex_lock(&acpi_consoles_delayed_mutex);
> > +			newcon->next = acpi_consoles_delayed;
> > +			acpi_consoles_delayed = newcon;
> > +			mutex_unlock(&acpi_consoles_delayed_mutex);
> > +			return;
> > +		} else if (err < 0) {
> > +			return;
> > +		} else {
> > +			if (newcon->setup && newcon->setup(newcon, opts) != 0)
> > +				return;
> > +			newcon->flags |= CON_ENABLED | CON_CONSDEV;
> > +			preferred_console = true;
> > +		}
> > +	}
> >  
> >  	/*
> >  	 * If we have a bootconsole, and are switching to a real console,
> > @@ -2612,34 +2653,41 @@ void register_console(struct console *newcon)
> >  }
> >  EXPORT_SYMBOL(register_console);
> >  
> > +static int delete_from_console_list(struct console **list, struct console *c)
> > +{
> > +	while (*list) {
> > +		struct console *cur = *list;
> > +
> > +		if (cur == c) {
> > +			*list = cur->next;
> > +			return 0;
> > +		}
> > +		list = &cur->next;
> > +	}
> > +	return 1;
> > +}
> > +
> >  int unregister_console(struct console *console)
> >  {
> > -        struct console *a, *b;
> >  	int res;
> >  
> >  	pr_info("%sconsole [%s%d] disabled\n",
> >  		(console->flags & CON_BOOT) ? "boot" : "" ,
> >  		console->name, console->index);
> >  
> > +	mutex_lock(&acpi_consoles_delayed_mutex);
> > +	res = delete_from_console_list(&acpi_consoles_delayed, console);
> > +	mutex_unlock(&acpi_consoles_delayed_mutex);
> > +	if (res == 0)
> > +		return res;
> > +
> >  	res = _braille_unregister_console(console);
> >  	if (res)
> >  		return res;
> >  
> > -	res = 1;
> >  	console_lock();
> > -	if (console_drivers == console) {
> > -		console_drivers=console->next;
> > -		res = 0;
> > -	} else if (console_drivers) {
> > -		for (a=console_drivers->next, b=console_drivers ;
> > -		     a; b=a, a=b->next) {
> > -			if (a == console) {
> > -				b->next = a->next;
> > -				res = 0;
> > -				break;
> > -			}
> > -		}
> > -	}
> > +
> > +	res = delete_from_console_list(&console_drivers, console);
> >  
> >  	if (!res && (console->flags & CON_EXTENDED))
> >  		nr_ext_console_drivers--;
> > -- 
> > 2.7.0
> > 
> > 
> > _______________________________________________
> > linux-arm-kernel mailing list
> > linux-arm-kernel@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Graeme Gregory]

On Mon, Feb 01, 2016 at 09:01:26AM +0000, Graeme Gregory wrote:
> On Mon, Jan 25, 2016 at 05:45:22PM +0600, Aleksey Makarov wrote:
> > 'ARM Server Base Boot Requiremets' [1] mention SPCR
> > (Serial Port Console Redirection Table) [2] as a mandatory ACPI table
> > that specifies the configuration of serial console.
> > 
> > Parse this table and check if any registered console match
> > the description.  If it does, enable that console.
> > 
> > To implement that, introduce a new member
> > int (*acpi_match)(struct console *, struct acpi_table_spcr *)
> > of struct console.  It allows drivers to check if they provide
> > a matching console device.
> > 
> 
> Fails to compile on x86
> 
> kernel/built-in.o: In function `register_console':
> (.text+0x48cba): undefined reference to `console_acpi_match'
> Makefile:929: recipe for target 'vmlinux' failed
> make: *** [vmlinux] Error 1
> 
> Because CONFIG_ACPI_SPCR_TABLE is never set.
> 
> Graeme
> 
> 
> > [1] http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.den0044a/index.html
> > [2] http://msdn.microsoft.com/en-us/library/windows/hardware/dn639131(v=vs.85).aspx
> > 
> > Signed-off-by: Aleksey Makarov <aleksey.makarov@linaro.org>
> > ---
> >  arch/arm64/Kconfig      |  1 +
> >  drivers/acpi/Kconfig    |  3 ++
> >  drivers/acpi/Makefile   |  1 +
> >  drivers/acpi/spcr.c     | 85 +++++++++++++++++++++++++++++++++++++++++++++++++
> >  include/linux/console.h | 12 +++++++
> >  kernel/printk/printk.c  | 82 +++++++++++++++++++++++++++++++++++++----------
> >  6 files changed, 167 insertions(+), 17 deletions(-)
> >  create mode 100644 drivers/acpi/spcr.c
> > 
> > diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> > index 573bebc..bf31e3c 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_ATOMIC64_DEC_IF_POSITIVE
> >  	select ARCH_HAS_ELF_RANDOMIZE
> >  	select ARCH_HAS_GCOV_PROFILE_ALL
> > diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
> > index e315061..142a338 100644
> > --- a/drivers/acpi/Kconfig
> > +++ b/drivers/acpi/Kconfig
> > @@ -60,6 +60,9 @@ config ACPI_CCA_REQUIRED
> >  config IORT_TABLE
> >  	bool
> >  
> > +config ACPI_SPCR_TABLE
> > +	bool
> > +
> >  config ACPI_DEBUGGER
> >  	bool "AML debugger interface (EXPERIMENTAL)"
> >  	select ACPI_DEBUG
> > diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
> > index 265eb90..8316859 100644
> > --- a/drivers/acpi/Makefile
> > +++ b/drivers/acpi/Makefile
> > @@ -81,6 +81,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_IORT_TABLE) 	+= iort.o
> > +obj-$(CONFIG_ACPI_SPCR_TABLE)	+= spcr.o
> >  
> >  # processor has its own "processor." module_param namespace
> >  processor-y			:= processor_driver.o
> > diff --git a/drivers/acpi/spcr.c b/drivers/acpi/spcr.c
> > new file mode 100644
> > index 0000000..ccb19a0
> > --- /dev/null
> > +++ b/drivers/acpi/spcr.c
> > @@ -0,0 +1,85 @@
> > +/*
> > + * 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>
> > +
> > +static struct acpi_table_spcr *spcr_table;
> > +
> > +int console_acpi_match(struct console *c, char **options)
> > +{
> > +	int err;
> > +
> > +	if (!c->acpi_match)
> > +		return -ENODEV;
> > +
> > +	if (!spcr_table)
> > +		return -EAGAIN;
> > +
> > +	err = c->acpi_match(c, spcr_table);
> > +	if (err < 0)
> > +		return err;
> > +
> > +	if (options) {
> > +		switch (spcr_table->baud_rate) {
> > +		case 3:
> > +			*options = "9600";
> > +			break;
> > +		case 4:
> > +			*options = "19200";
> > +			break;
> > +		case 6:
> > +			*options = "57600";
> > +			break;
> > +		case 7:
> > +			*options = "115200";
> > +			break;
> > +		default:
> > +			*options = "";
> > +			break;
> > +		}
> > +	}
> > +
> > +	return err;
> > +}
> > +
> > +static int __init spcr_table_detect(void)
> > +{
> > +	struct acpi_table_header *table;
> > +	acpi_status status;
> > +
> > +	if (acpi_disabled)
> > +		return -ENODEV;
> > +
> > +	status = acpi_get_table(ACPI_SIG_SPCR, 0, &table);
> > +	if (ACPI_FAILURE(status)) {
> > +		const char *msg = acpi_format_exception(status);
> > +
> > +		pr_err("Failed to get table, %s\n", msg);
> > +		return -EINVAL;
> > +	}
> > +
> > +	if (table->revision < 2)
> > +		return -EOPNOTSUPP;
> > +
> > +	spcr_table = (struct acpi_table_spcr *)table;
> > +
> > +	pr_info("Console at 0x%016llx\n", spcr_table->serial_port.address);
> > +
> > +	acpi_register_consoles_try_again();
> > +
> > +	return 0;
> > +}
> > +
> > +arch_initcall(spcr_table_detect);
> > diff --git a/include/linux/console.h b/include/linux/console.h
> > index bd19434..94d0bd8 100644
> > --- a/include/linux/console.h
> > +++ b/include/linux/console.h
> > @@ -117,6 +117,7 @@ static inline int con_debug_leave(void)
> >  #define CON_BRL		(32) /* Used for a braille device */
> >  #define CON_EXTENDED	(64) /* Use the extended output format a la /dev/kmsg */
> >  
> > +struct acpi_table_spcr;
> >  struct console {
> >  	char	name[16];
> >  	void	(*write)(struct console *, const char *, unsigned);
> > @@ -125,6 +126,7 @@ struct console {
> >  	void	(*unblank)(void);
> >  	int	(*setup)(struct console *, char *);
> >  	int	(*match)(struct console *, char *name, int idx, char *options);
> > +	int	(*acpi_match)(struct console *, struct acpi_table_spcr *);
> >  	short	flags;
> >  	short	index;
> >  	int	cflag;
> > @@ -132,6 +134,16 @@ struct console {
> >  	struct	 console *next;
> >  };
> >  
> > +#ifdef CONFIG_ACPI

This should be #ifdef CONFIG_ACPI_SPCR_TABLE

> > +int console_acpi_match(struct console *c, char **options);
> > +#else
> > +static inline int console_acpi_match(struct console *c, char **options)
> > +{
> > +	return -ENODEV;

This requires a #include <linux/errno.h> at the top of the file to work.

Graeme

> > +}
> > +#endif
> > +void acpi_register_consoles_try_again(void);
> > +
> >  /*
> >   * for_each_console() allows you to iterate on each console
> >   */
> > diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> > index 37e531f..3cf8cba 100644
> > --- a/kernel/printk/printk.c
> > +++ b/kernel/printk/printk.c
> > @@ -2430,6 +2430,25 @@ static int __init keep_bootcon_setup(char *str)
> >  
> >  early_param("keep_bootcon", keep_bootcon_setup);
> >  
> > +static DEFINE_MUTEX(acpi_consoles_delayed_mutex);
> > +static struct console *acpi_consoles_delayed;
> > +
> > +void acpi_register_consoles_try_again(void)
> > +{
> > +	mutex_lock(&acpi_consoles_delayed_mutex);
> > +	while (acpi_consoles_delayed) {
> > +
> > +		struct console *c = acpi_consoles_delayed;
> > +
> > +		acpi_consoles_delayed = acpi_consoles_delayed->next;
> > +
> > +		mutex_unlock(&acpi_consoles_delayed_mutex);
> > +		register_console(c);
> > +		mutex_lock(&acpi_consoles_delayed_mutex);
> > +	}
> > +	mutex_unlock(&acpi_consoles_delayed_mutex);
> > +}
> > +
> >  /*
> >   * The console driver calls this routine during kernel initialization
> >   * to register the console printing procedure with printk() and to
> > @@ -2538,8 +2557,30 @@ void register_console(struct console *newcon)
> >  		break;
> >  	}
> >  
> > -	if (!(newcon->flags & CON_ENABLED))
> > -		return;
> > +	if (!(newcon->flags & CON_ENABLED)) {
> > +		char *opts;
> > +		int err;
> > +
> > +		if (newcon->index < 0)
> > +			newcon->index = 0;
> > +
> > +		err = console_acpi_match(newcon, &opts);
> > +
> > +		if (err == -EAGAIN) {
> > +			mutex_lock(&acpi_consoles_delayed_mutex);
> > +			newcon->next = acpi_consoles_delayed;
> > +			acpi_consoles_delayed = newcon;
> > +			mutex_unlock(&acpi_consoles_delayed_mutex);
> > +			return;
> > +		} else if (err < 0) {
> > +			return;
> > +		} else {
> > +			if (newcon->setup && newcon->setup(newcon, opts) != 0)
> > +				return;
> > +			newcon->flags |= CON_ENABLED | CON_CONSDEV;
> > +			preferred_console = true;
> > +		}
> > +	}
> >  
> >  	/*
> >  	 * If we have a bootconsole, and are switching to a real console,
> > @@ -2612,34 +2653,41 @@ void register_console(struct console *newcon)
> >  }
> >  EXPORT_SYMBOL(register_console);
> >  
> > +static int delete_from_console_list(struct console **list, struct console *c)
> > +{
> > +	while (*list) {
> > +		struct console *cur = *list;
> > +
> > +		if (cur == c) {
> > +			*list = cur->next;
> > +			return 0;
> > +		}
> > +		list = &cur->next;
> > +	}
> > +	return 1;
> > +}
> > +
> >  int unregister_console(struct console *console)
> >  {
> > -        struct console *a, *b;
> >  	int res;
> >  
> >  	pr_info("%sconsole [%s%d] disabled\n",
> >  		(console->flags & CON_BOOT) ? "boot" : "" ,
> >  		console->name, console->index);
> >  
> > +	mutex_lock(&acpi_consoles_delayed_mutex);
> > +	res = delete_from_console_list(&acpi_consoles_delayed, console);
> > +	mutex_unlock(&acpi_consoles_delayed_mutex);
> > +	if (res == 0)
> > +		return res;
> > +
> >  	res = _braille_unregister_console(console);
> >  	if (res)
> >  		return res;
> >  
> > -	res = 1;
> >  	console_lock();
> > -	if (console_drivers == console) {
> > -		console_drivers=console->next;
> > -		res = 0;
> > -	} else if (console_drivers) {
> > -		for (a=console_drivers->next, b=console_drivers ;
> > -		     a; b=a, a=b->next) {
> > -			if (a == console) {
> > -				b->next = a->next;
> > -				res = 0;
> > -				break;
> > -			}
> > -		}
> > -	}
> > +
> > +	res = delete_from_console_list(&console_drivers, console);
> >  
> >  	if (!res && (console->flags & CON_EXTENDED))
> >  		nr_ext_console_drivers--;
> > -- 
> > 2.7.0
> > 
> > 
> > _______________________________________________
> > linux-arm-kernel mailing list
> > linux-arm-kernel at lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Al Stone]

On 01/27/2016 05:17 AM, Aleksey Makarov wrote:
> 
> 
> On 01/25/2016 07:11 PM, Peter Hurley wrote:
>> On 01/25/2016 03:45 AM, Aleksey Makarov wrote:
>>> This patchset is based on the patchset by Leif Lindholm [1]
>>>
>>> 'ARM Server Base Boot Requirements' [2] mention SPCR 
>>> (Serial Port Console Redirection Table) [3] as a mandatory
>>> ACPI table that specifies the configuration of serial console.
>>>
>>> Licensing concerns have prevented implementing it in the past, but as of
>>> 10 August 2015, these tables have both been released also under 
>>> OWF 1.0 [4].
>>
>> This license has a patent retaliation provision, which makes it
>> incompatible with GPLv2.
>>
>> *If the license applies to this code*, then this patch set does not
>> meet the criteria for submission.
> 
> The license applies not to this code but to the document describing the tables.

Just for the record, the SPCR table struct definition has been part
of the Linux kernel since at least commit b24aad44 on 2009-07-24
(line 1112 of include/acpi/actbl2.h) -- or so git blame tells me.

-- 
ciao,
al
-----------------------------------
Al Stone
Software Engineer
Red Hat, Inc.
ahs3@redhat.com
-----------------------------------

[PATCH 0/3] ACPI: parse the SPCR table

[Al Stone]

On 01/27/2016 05:17 AM, Aleksey Makarov wrote:
> 
> 
> On 01/25/2016 07:11 PM, Peter Hurley wrote:
>> On 01/25/2016 03:45 AM, Aleksey Makarov wrote:
>>> This patchset is based on the patchset by Leif Lindholm [1]
>>>
>>> 'ARM Server Base Boot Requirements' [2] mention SPCR 
>>> (Serial Port Console Redirection Table) [3] as a mandatory
>>> ACPI table that specifies the configuration of serial console.
>>>
>>> Licensing concerns have prevented implementing it in the past, but as of
>>> 10 August 2015, these tables have both been released also under 
>>> OWF 1.0 [4].
>>
>> This license has a patent retaliation provision, which makes it
>> incompatible with GPLv2.
>>
>> *If the license applies to this code*, then this patch set does not
>> meet the criteria for submission.
> 
> The license applies not to this code but to the document describing the tables.

Just for the record, the SPCR table struct definition has been part
of the Linux kernel since at least commit b24aad44 on 2009-07-24
(line 1112 of include/acpi/actbl2.h) -- or so git blame tells me.

-- 
ciao,
al
-----------------------------------
Al Stone
Software Engineer
Red Hat, Inc.
ahs3 at redhat.com
-----------------------------------

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

[Peter Hurley]

Hi Al,

Somehow your email was filtered. Apologies for that.

On 02/10/2016 03:39 PM, Al Stone wrote:
> On 01/27/2016 05:17 AM, Aleksey Makarov wrote:
>>
>>
>> On 01/25/2016 07:11 PM, Peter Hurley wrote:
>>> On 01/25/2016 03:45 AM, Aleksey Makarov wrote:
>>>> This patchset is based on the patchset by Leif Lindholm [1]
>>>>
>>>> 'ARM Server Base Boot Requirements' [2] mention SPCR 
>>>> (Serial Port Console Redirection Table) [3] as a mandatory
>>>> ACPI table that specifies the configuration of serial console.
>>>>
>>>> Licensing concerns have prevented implementing it in the past, but as of
>>>> 10 August 2015, these tables have both been released also under 
>>>> OWF 1.0 [4].
>>>
>>> This license has a patent retaliation provision, which makes it
>>> incompatible with GPLv2.
>>>
>>> *If the license applies to this code*, then this patch set does not
>>> meet the criteria for submission.
>>
>> The license applies not to this code but to the document describing the tables.
> 
> Just for the record, the SPCR table struct definition has been part
> of the Linux kernel since at least commit b24aad44 on 2009-07-24
> (line 1112 of include/acpi/actbl2.h) -- or so git blame tells me.

Just to be clear here:

The Microsoft specification, which defines the SPCR table struct and which
this patch series relies on, notes that patents apply. Specifically, it
says:

Patent Notice:
Microsoft is making certain patent rights available for implementations of this specification under two options:
1)  Microsoft’s Community Promise, available at http://www.microsoft.com/openspecifications/en/us/programs/community-promise/default.aspx; or
2)  The Open Web Foundation Final Specification Agreement Version 1.0 ("OWF 1.0") as of October 1, 2012, available at http://www.openwebfoundation.org/legal/the-owf-1-0-agreements/owfa-1-0. 
Version 1.03 — August 10, 2015

I don't believe either of those patent licenses are GPL compatible.

Unless you're saying Red Hat is signing off on this?

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

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

[Peter Hurley]

Hi Al,

Somehow your email was filtered. Apologies for that.

On 02/10/2016 03:39 PM, Al Stone wrote:
> On 01/27/2016 05:17 AM, Aleksey Makarov wrote:
>>
>>
>> On 01/25/2016 07:11 PM, Peter Hurley wrote:
>>> On 01/25/2016 03:45 AM, Aleksey Makarov wrote:
>>>> This patchset is based on the patchset by Leif Lindholm [1]
>>>>
>>>> 'ARM Server Base Boot Requirements' [2] mention SPCR 
>>>> (Serial Port Console Redirection Table) [3] as a mandatory
>>>> ACPI table that specifies the configuration of serial console.
>>>>
>>>> Licensing concerns have prevented implementing it in the past, but as of
>>>> 10 August 2015, these tables have both been released also under 
>>>> OWF 1.0 [4].
>>>
>>> This license has a patent retaliation provision, which makes it
>>> incompatible with GPLv2.
>>>
>>> *If the license applies to this code*, then this patch set does not
>>> meet the criteria for submission.
>>
>> The license applies not to this code but to the document describing the tables.
> 
> Just for the record, the SPCR table struct definition has been part
> of the Linux kernel since at least commit b24aad44 on 2009-07-24
> (line 1112 of include/acpi/actbl2.h) -- or so git blame tells me.

Just to be clear here:

The Microsoft specification, which defines the SPCR table struct and which
this patch series relies on, notes that patents apply. Specifically, it
says:

Patent Notice:
Microsoft is making certain patent rights available for implementations of this specification under two options:
1)  Microsoft’s Community Promise, available at http://www.microsoft.com/openspecifications/en/us/programs/community-promise/default.aspx; or
2)  The Open Web Foundation Final Specification Agreement Version 1.0 ("OWF 1.0") as of October 1, 2012, available at http://www.openwebfoundation.org/legal/the-owf-1-0-agreements/owfa-1-0. 
Version 1.03 — August 10, 2015

I don't believe either of those patent licenses are GPL compatible.

Unless you're saying Red Hat is signing off on this?

Regards,
Peter Hurley

[PATCH 0/3] ACPI: parse the SPCR table

[Peter Hurley]

Hi Al,

Somehow your email was filtered. Apologies for that.

On 02/10/2016 03:39 PM, Al Stone wrote:
> On 01/27/2016 05:17 AM, Aleksey Makarov wrote:
>>
>>
>> On 01/25/2016 07:11 PM, Peter Hurley wrote:
>>> On 01/25/2016 03:45 AM, Aleksey Makarov wrote:
>>>> This patchset is based on the patchset by Leif Lindholm [1]
>>>>
>>>> 'ARM Server Base Boot Requirements' [2] mention SPCR 
>>>> (Serial Port Console Redirection Table) [3] as a mandatory
>>>> ACPI table that specifies the configuration of serial console.
>>>>
>>>> Licensing concerns have prevented implementing it in the past, but as of
>>>> 10 August 2015, these tables have both been released also under 
>>>> OWF 1.0 [4].
>>>
>>> This license has a patent retaliation provision, which makes it
>>> incompatible with GPLv2.
>>>
>>> *If the license applies to this code*, then this patch set does not
>>> meet the criteria for submission.
>>
>> The license applies not to this code but to the document describing the tables.
> 
> Just for the record, the SPCR table struct definition has been part
> of the Linux kernel since at least commit b24aad44 on 2009-07-24
> (line 1112 of include/acpi/actbl2.h) -- or so git blame tells me.

Just to be clear here:

The Microsoft specification, which defines the SPCR table struct and which
this patch series relies on, notes that patents apply. Specifically, it
says:

Patent Notice:
Microsoft is making certain patent rights available for implementations of this specification under two options:
1)  Microsoft?s Community Promise, available at http://www.microsoft.com/openspecifications/en/us/programs/community-promise/default.aspx; or
2)  The Open Web Foundation Final Specification Agreement Version 1.0 ("OWF 1.0") as of October 1, 2012, available at http://www.openwebfoundation.org/legal/the-owf-1-0-agreements/owfa-1-0. 
Version 1.03 ? August 10, 2015

I don't believe either of those patent licenses are GPL compatible.

Unless you're saying Red Hat is signing off on this?

Regards,
Peter Hurley

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

[Peter Hurley]

Hi Jon,

[ cc'd everyone back in on the assumption private mail was accidental ]


On 03/04/2016 12:21 AM, Jon Masters wrote:
> Top posting - in transit currently
> 
> Peter,
> 
> I would like to understand what concrete action you feel is needed in
> order to be comfortable with adding SPCR support to the Linux
> kernel?
>
> To clarify for others - this thread pertains to perceived issues with
> the license of a document describing the format of the SPCR, a
> document whose license was explicitly changed in response to previous
> concerns raised by yourself, not the code that actually implements
> SPCR support.

Not exactly, because there is no such thing as patenting a document.

The specification purports that patents apply to implementations of
SPCR, and grants rights to those patents under two optional licenses.

What I want is what is expected of any submitter to the Linux Kernel:
that the submitter has the legal right to do so as spelled out
in the Developer's Certificate of Origin (the text of which is in
Documentation/SubmittingPatches).

To have the legal right to submit material to which patents apply
requires a license.

So either
A) the submission does not infringe, and no patent license is required, or
B) the submission requires a patent license because otherwise it would
infringe.

If A, what I would like to see is the due diligence as to why the
submission does not infringe.

If B, FSF maintains an online list of GPL compatible licenses.
Neither of the two licenses offered are on the FSF online list.

The FSF also maintains a compliance lab and will render opinions on
the GPL compatibility of a license. They can be reached at
licensing@fsf.org

Alternatively, a statement from Linux Foundation that this
has been discussed and agreed upon privately is also fine.

Alternatively, any sign-off from ARM, Linaro or Red Hat legal that either
a) one of those licenses is GPL compatible, or
b) the submission does not infringe on the purported patents (and why)


> However you retain the opinion that the license under which the table
> is published "is not GPL compatible". So under what license would you
> like to see this table specification released?

Assuming none of the options above is acceptable, any license in the
"GPL-Compatible Free Software Licenses" listed here:

http://www.gnu.org/licenses/license-list.en.html#GPLCompatibleLicenses



> Jon.
> 
> -- Computer Architect | Sent from my 64-bit #ARM Powered phone
>> > On Mar 4, 2016, at 05:08, Peter Hurley <peter@hurleysoftware.com> wrote:
>> > 
>> > Hi Al,
>> > 
>> > Somehow your email was filtered. Apologies for that.
>> > 
>>> >> On 02/10/2016 03:39 PM, Al Stone wrote:
>>>> >>> On 01/27/2016 05:17 AM, Aleksey Makarov wrote:
>>>> >>> 
>>>> >>> 
>>>>> >>>> On 01/25/2016 07:11 PM, Peter Hurley wrote:
>>>>>> >>>>> On 01/25/2016 03:45 AM, Aleksey Makarov wrote:
>>>>>> >>>>> This patchset is based on the patchset by Leif Lindholm [1]
>>>>>> >>>>> 
>>>>>> >>>>> 'ARM Server Base Boot Requirements' [2] mention SPCR 
>>>>>> >>>>> (Serial Port Console Redirection Table) [3] as a mandatory
>>>>>> >>>>> ACPI table that specifies the configuration of serial console.
>>>>>> >>>>> 
>>>>>> >>>>> Licensing concerns have prevented implementing it in the past, but as of
>>>>>> >>>>> 10 August 2015, these tables have both been released also under 
>>>>>> >>>>> OWF 1.0 [4].
>>>>> >>>> 
>>>>> >>>> This license has a patent retaliation provision, which makes it
>>>>> >>>> incompatible with GPLv2.
>>>>> >>>> 
>>>>> >>>> *If the license applies to this code*, then this patch set does not
>>>>> >>>> meet the criteria for submission.
>>>> >>> 
>>>> >>> The license applies not to this code but to the document describing the tables.
>>> >> 
>>> >> Just for the record, the SPCR table struct definition has been part
>>> >> of the Linux kernel since at least commit b24aad44 on 2009-07-24
>>> >> (line 1112 of include/acpi/actbl2.h) -- or so git blame tells me.
>> > 
>> > Just to be clear here:
>> > 
>> > The Microsoft specification, which defines the SPCR table struct and which
>> > this patch series relies on, notes that patents apply. Specifically, it
>> > says:
>> > 
>> > Patent Notice:
>> > Microsoft is making certain patent rights available for implementations of this specification under two options:
>> > 1)  Microsoft’s Community Promise, available at http://www.microsoft.com/openspecifications/en/us/programs/community-promise/default.aspx; or
>> > 2)  The Open Web Foundation Final Specification Agreement Version 1.0 ("OWF 1.0") as of October 1, 2012, available at http://www.openwebfoundation.org/legal/the-owf-1-0-agreements/owfa-1-0. 
>> > Version 1.03 — August 10, 2015
>> > 
>> > I don't believe either of those patent licenses are GPL compatible.
>> > 
>> > Unless you're saying Red Hat is signing off on this?
>> > 
>> > Regards,
>> > Peter Hurley

[PATCH 0/3] ACPI: parse the SPCR table

[Peter Hurley]

Hi Jon,

[ cc'd everyone back in on the assumption private mail was accidental ]


On 03/04/2016 12:21 AM, Jon Masters wrote:
> Top posting - in transit currently
> 
> Peter,
> 
> I would like to understand what concrete action you feel is needed in
> order to be comfortable with adding SPCR support to the Linux
> kernel?
>
> To clarify for others - this thread pertains to perceived issues with
> the license of a document describing the format of the SPCR, a
> document whose license was explicitly changed in response to previous
> concerns raised by yourself, not the code that actually implements
> SPCR support.

Not exactly, because there is no such thing as patenting a document.

The specification purports that patents apply to implementations of
SPCR, and grants rights to those patents under two optional licenses.

What I want is what is expected of any submitter to the Linux Kernel:
that the submitter has the legal right to do so as spelled out
in the Developer's Certificate of Origin (the text of which is in
Documentation/SubmittingPatches).

To have the legal right to submit material to which patents apply
requires a license.

So either
A) the submission does not infringe, and no patent license is required, or
B) the submission requires a patent license because otherwise it would
infringe.

If A, what I would like to see is the due diligence as to why the
submission does not infringe.

If B, FSF maintains an online list of GPL compatible licenses.
Neither of the two licenses offered are on the FSF online list.

The FSF also maintains a compliance lab and will render opinions on
the GPL compatibility of a license. They can be reached at
licensing at fsf.org

Alternatively, a statement from Linux Foundation that this
has been discussed and agreed upon privately is also fine.

Alternatively, any sign-off from ARM, Linaro or Red Hat legal that either
a) one of those licenses is GPL compatible, or
b) the submission does not infringe on the purported patents (and why)


> However you retain the opinion that the license under which the table
> is published "is not GPL compatible". So under what license would you
> like to see this table specification released?

Assuming none of the options above is acceptable, any license in the
"GPL-Compatible Free Software Licenses" listed here:

http://www.gnu.org/licenses/license-list.en.html#GPLCompatibleLicenses



> Jon.
> 
> -- Computer Architect | Sent from my 64-bit #ARM Powered phone
>> > On Mar 4, 2016, at 05:08, Peter Hurley <peter@hurleysoftware.com> wrote:
>> > 
>> > Hi Al,
>> > 
>> > Somehow your email was filtered. Apologies for that.
>> > 
>>> >> On 02/10/2016 03:39 PM, Al Stone wrote:
>>>> >>> On 01/27/2016 05:17 AM, Aleksey Makarov wrote:
>>>> >>> 
>>>> >>> 
>>>>> >>>> On 01/25/2016 07:11 PM, Peter Hurley wrote:
>>>>>> >>>>> On 01/25/2016 03:45 AM, Aleksey Makarov wrote:
>>>>>> >>>>> This patchset is based on the patchset by Leif Lindholm [1]
>>>>>> >>>>> 
>>>>>> >>>>> 'ARM Server Base Boot Requirements' [2] mention SPCR 
>>>>>> >>>>> (Serial Port Console Redirection Table) [3] as a mandatory
>>>>>> >>>>> ACPI table that specifies the configuration of serial console.
>>>>>> >>>>> 
>>>>>> >>>>> Licensing concerns have prevented implementing it in the past, but as of
>>>>>> >>>>> 10 August 2015, these tables have both been released also under 
>>>>>> >>>>> OWF 1.0 [4].
>>>>> >>>> 
>>>>> >>>> This license has a patent retaliation provision, which makes it
>>>>> >>>> incompatible with GPLv2.
>>>>> >>>> 
>>>>> >>>> *If the license applies to this code*, then this patch set does not
>>>>> >>>> meet the criteria for submission.
>>>> >>> 
>>>> >>> The license applies not to this code but to the document describing the tables.
>>> >> 
>>> >> Just for the record, the SPCR table struct definition has been part
>>> >> of the Linux kernel since at least commit b24aad44 on 2009-07-24
>>> >> (line 1112 of include/acpi/actbl2.h) -- or so git blame tells me.
>> > 
>> > Just to be clear here:
>> > 
>> > The Microsoft specification, which defines the SPCR table struct and which
>> > this patch series relies on, notes that patents apply. Specifically, it
>> > says:
>> > 
>> > Patent Notice:
>> > Microsoft is making certain patent rights available for implementations of this specification under two options:
>> > 1)  Microsoft?s Community Promise, available at http://www.microsoft.com/openspecifications/en/us/programs/community-promise/default.aspx; or
>> > 2)  The Open Web Foundation Final Specification Agreement Version 1.0 ("OWF 1.0") as of October 1, 2012, available at http://www.openwebfoundation.org/legal/the-owf-1-0-agreements/owfa-1-0. 
>> > Version 1.03 ? August 10, 2015
>> > 
>> > I don't believe either of those patent licenses are GPL compatible.
>> > 
>> > Unless you're saying Red Hat is signing off on this?
>> > 
>> > Regards,
>> > Peter Hurley

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

[Peter Hurley]

On 03/04/2016 11:34 AM, Peter Hurley wrote:
> Hi Jon,
> 
> [ cc'd everyone back in on the assumption private mail was accidental ]
> 
> 
> On 03/04/2016 12:21 AM, Jon Masters wrote:
>> Top posting - in transit currently
>>
>> Peter,
>>
>> I would like to understand what concrete action you feel is needed in
>> order to be comfortable with adding SPCR support to the Linux
>> kernel?
>>
>> To clarify for others - this thread pertains to perceived issues with
>> the license of a document describing the format of the SPCR, a
>> document whose license was explicitly changed in response to previous
>> concerns raised by yourself, not the code that actually implements
>> SPCR support.
> 
> Not exactly, because there is no such thing as patenting a document.
> 
> The specification purports that patents apply to implementations of
> SPCR, and grants rights to those patents under two optional licenses.
> 
> What I want is what is expected of any submitter to the Linux Kernel:
> that the submitter has the legal right to do so as spelled out
> in the Developer's Certificate of Origin (the text of which is in
> Documentation/SubmittingPatches).
> 
> To have the legal right to submit material to which patents apply
> requires a license.
> 
> So either
> A) the submission does not infringe, and no patent license is required, or
> B) the submission requires a patent license because otherwise it would
> infringe.
> 
> If A, what I would like to see is the due diligence as to why the
> submission does not infringe.
> 
> If B, FSF maintains an online list of GPL compatible licenses.
> Neither of the two licenses offered are on the FSF online list.
> 
> The FSF also maintains a compliance lab and will render opinions on
> the GPL compatibility of a license. They can be reached at
> licensing@fsf.org
> 
> Alternatively, a statement from Linux Foundation that this
> has been discussed and agreed upon privately is also fine.
> 
> Alternatively, any sign-off from ARM, Linaro or Red Hat legal that either
> a) one of those licenses is GPL compatible, or
> b) the submission does not infringe on the purported patents (and why)
> 
> 
>> However you retain the opinion that the license under which the table
>> is published "is not GPL compatible". So under what license would you
>> like to see this table specification released?
> 
> Assuming none of the options above is acceptable, any license in the
> "GPL-Compatible Free Software Licenses" listed here:
> 
> http://www.gnu.org/licenses/license-list.en.html#GPLCompatibleLicenses

FWIW, my specific concern with both licenses offered by Microsoft
is that both contain a patent retaliation provision.

For example, in the Open Web Foundation OWFa 1.0, Patents and Copyright
Grants:

3.1.2.  Termination.

3.1.2.1.  As a Result of Claims by You.  All rights, grants, and promises made by me to you under this Agreement are terminated if you file, maintain, or voluntarily participate in a lawsuit against me or any person or entity asserting that its Permitted Uses infringe any  Granted Claims you would have had the right to enforce had you signed this Agreement, unless that suit was in response to a corresponding suit first brought against you.


There is no such provision in the GPL.

Note also that Aleksey only refers to the GPL in the submission, and
neither of these other licenses which actually grant the
patent license(s). That's also not ok; file header must include
the necessary licenses for which the submission was made.



>> Jon.
>>
>> -- Computer Architect | Sent from my 64-bit #ARM Powered phone
>>>> On Mar 4, 2016, at 05:08, Peter Hurley <peter@hurleysoftware.com> wrote:
>>>>
>>>> Hi Al,
>>>>
>>>> Somehow your email was filtered. Apologies for that.
>>>>
>>>>>> On 02/10/2016 03:39 PM, Al Stone wrote:
>>>>>>>> On 01/27/2016 05:17 AM, Aleksey Makarov wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>>>> On 01/25/2016 07:11 PM, Peter Hurley wrote:
>>>>>>>>>>>> On 01/25/2016 03:45 AM, Aleksey Makarov wrote:
>>>>>>>>>>>> This patchset is based on the patchset by Leif Lindholm [1]
>>>>>>>>>>>>
>>>>>>>>>>>> 'ARM Server Base Boot Requirements' [2] mention SPCR 
>>>>>>>>>>>> (Serial Port Console Redirection Table) [3] as a mandatory
>>>>>>>>>>>> ACPI table that specifies the configuration of serial console.
>>>>>>>>>>>>
>>>>>>>>>>>> Licensing concerns have prevented implementing it in the past, but as of
>>>>>>>>>>>> 10 August 2015, these tables have both been released also under 
>>>>>>>>>>>> OWF 1.0 [4].
>>>>>>>>>>
>>>>>>>>>> This license has a patent retaliation provision, which makes it
>>>>>>>>>> incompatible with GPLv2.
>>>>>>>>>>
>>>>>>>>>> *If the license applies to this code*, then this patch set does not
>>>>>>>>>> meet the criteria for submission.
>>>>>>>>
>>>>>>>> The license applies not to this code but to the document describing the tables.
>>>>>>
>>>>>> Just for the record, the SPCR table struct definition has been part
>>>>>> of the Linux kernel since at least commit b24aad44 on 2009-07-24
>>>>>> (line 1112 of include/acpi/actbl2.h) -- or so git blame tells me.
>>>>
>>>> Just to be clear here:
>>>>
>>>> The Microsoft specification, which defines the SPCR table struct and which
>>>> this patch series relies on, notes that patents apply. Specifically, it
>>>> says:
>>>>
>>>> Patent Notice:
>>>> Microsoft is making certain patent rights available for implementations of this specification under two options:
>>>> 1)  Microsoft’s Community Promise, available at http://www.microsoft.com/openspecifications/en/us/programs/community-promise/default.aspx; or
>>>> 2)  The Open Web Foundation Final Specification Agreement Version 1.0 ("OWF 1.0") as of October 1, 2012, available at http://www.openwebfoundation.org/legal/the-owf-1-0-agreements/owfa-1-0. 
>>>> Version 1.03 — August 10, 2015
>>>>
>>>> I don't believe either of those patent licenses are GPL compatible.
>>>>
>>>> Unless you're saying Red Hat is signing off on this?
>>>>
>>>> Regards,
>>>> Peter Hurley
> 

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

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

[Peter Hurley]

On 03/04/2016 11:34 AM, Peter Hurley wrote:
> Hi Jon,
> 
> [ cc'd everyone back in on the assumption private mail was accidental ]
> 
> 
> On 03/04/2016 12:21 AM, Jon Masters wrote:
>> Top posting - in transit currently
>>
>> Peter,
>>
>> I would like to understand what concrete action you feel is needed in
>> order to be comfortable with adding SPCR support to the Linux
>> kernel?
>>
>> To clarify for others - this thread pertains to perceived issues with
>> the license of a document describing the format of the SPCR, a
>> document whose license was explicitly changed in response to previous
>> concerns raised by yourself, not the code that actually implements
>> SPCR support.
> 
> Not exactly, because there is no such thing as patenting a document.
> 
> The specification purports that patents apply to implementations of
> SPCR, and grants rights to those patents under two optional licenses.
> 
> What I want is what is expected of any submitter to the Linux Kernel:
> that the submitter has the legal right to do so as spelled out
> in the Developer's Certificate of Origin (the text of which is in
> Documentation/SubmittingPatches).
> 
> To have the legal right to submit material to which patents apply
> requires a license.
> 
> So either
> A) the submission does not infringe, and no patent license is required, or
> B) the submission requires a patent license because otherwise it would
> infringe.
> 
> If A, what I would like to see is the due diligence as to why the
> submission does not infringe.
> 
> If B, FSF maintains an online list of GPL compatible licenses.
> Neither of the two licenses offered are on the FSF online list.
> 
> The FSF also maintains a compliance lab and will render opinions on
> the GPL compatibility of a license. They can be reached at
> licensing@fsf.org
> 
> Alternatively, a statement from Linux Foundation that this
> has been discussed and agreed upon privately is also fine.
> 
> Alternatively, any sign-off from ARM, Linaro or Red Hat legal that either
> a) one of those licenses is GPL compatible, or
> b) the submission does not infringe on the purported patents (and why)
> 
> 
>> However you retain the opinion that the license under which the table
>> is published "is not GPL compatible". So under what license would you
>> like to see this table specification released?
> 
> Assuming none of the options above is acceptable, any license in the
> "GPL-Compatible Free Software Licenses" listed here:
> 
> http://www.gnu.org/licenses/license-list.en.html#GPLCompatibleLicenses

FWIW, my specific concern with both licenses offered by Microsoft
is that both contain a patent retaliation provision.

For example, in the Open Web Foundation OWFa 1.0, Patents and Copyright
Grants:

3.1.2.  Termination.

3.1.2.1.  As a Result of Claims by You.  All rights, grants, and promises made by me to you under this Agreement are terminated if you file, maintain, or voluntarily participate in a lawsuit against me or any person or entity asserting that its Permitted Uses infringe any  Granted Claims you would have had the right to enforce had you signed this Agreement, unless that suit was in response to a corresponding suit first brought against you.


There is no such provision in the GPL.

Note also that Aleksey only refers to the GPL in the submission, and
neither of these other licenses which actually grant the
patent license(s). That's also not ok; file header must include
the necessary licenses for which the submission was made.



>> Jon.
>>
>> -- Computer Architect | Sent from my 64-bit #ARM Powered phone
>>>> On Mar 4, 2016, at 05:08, Peter Hurley <peter@hurleysoftware.com> wrote:
>>>>
>>>> Hi Al,
>>>>
>>>> Somehow your email was filtered. Apologies for that.
>>>>
>>>>>> On 02/10/2016 03:39 PM, Al Stone wrote:
>>>>>>>> On 01/27/2016 05:17 AM, Aleksey Makarov wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>>>> On 01/25/2016 07:11 PM, Peter Hurley wrote:
>>>>>>>>>>>> On 01/25/2016 03:45 AM, Aleksey Makarov wrote:
>>>>>>>>>>>> This patchset is based on the patchset by Leif Lindholm [1]
>>>>>>>>>>>>
>>>>>>>>>>>> 'ARM Server Base Boot Requirements' [2] mention SPCR 
>>>>>>>>>>>> (Serial Port Console Redirection Table) [3] as a mandatory
>>>>>>>>>>>> ACPI table that specifies the configuration of serial console.
>>>>>>>>>>>>
>>>>>>>>>>>> Licensing concerns have prevented implementing it in the past, but as of
>>>>>>>>>>>> 10 August 2015, these tables have both been released also under 
>>>>>>>>>>>> OWF 1.0 [4].
>>>>>>>>>>
>>>>>>>>>> This license has a patent retaliation provision, which makes it
>>>>>>>>>> incompatible with GPLv2.
>>>>>>>>>>
>>>>>>>>>> *If the license applies to this code*, then this patch set does not
>>>>>>>>>> meet the criteria for submission.
>>>>>>>>
>>>>>>>> The license applies not to this code but to the document describing the tables.
>>>>>>
>>>>>> Just for the record, the SPCR table struct definition has been part
>>>>>> of the Linux kernel since at least commit b24aad44 on 2009-07-24
>>>>>> (line 1112 of include/acpi/actbl2.h) -- or so git blame tells me.
>>>>
>>>> Just to be clear here:
>>>>
>>>> The Microsoft specification, which defines the SPCR table struct and which
>>>> this patch series relies on, notes that patents apply. Specifically, it
>>>> says:
>>>>
>>>> Patent Notice:
>>>> Microsoft is making certain patent rights available for implementations of this specification under two options:
>>>> 1)  Microsoft’s Community Promise, available at http://www.microsoft.com/openspecifications/en/us/programs/community-promise/default.aspx; or
>>>> 2)  The Open Web Foundation Final Specification Agreement Version 1.0 ("OWF 1.0") as of October 1, 2012, available at http://www.openwebfoundation.org/legal/the-owf-1-0-agreements/owfa-1-0. 
>>>> Version 1.03 — August 10, 2015
>>>>
>>>> I don't believe either of those patent licenses are GPL compatible.
>>>>
>>>> Unless you're saying Red Hat is signing off on this?
>>>>
>>>> Regards,
>>>> Peter Hurley
> 

[PATCH 0/3] ACPI: parse the SPCR table

[Peter Hurley]

On 03/04/2016 11:34 AM, Peter Hurley wrote:
> Hi Jon,
> 
> [ cc'd everyone back in on the assumption private mail was accidental ]
> 
> 
> On 03/04/2016 12:21 AM, Jon Masters wrote:
>> Top posting - in transit currently
>>
>> Peter,
>>
>> I would like to understand what concrete action you feel is needed in
>> order to be comfortable with adding SPCR support to the Linux
>> kernel?
>>
>> To clarify for others - this thread pertains to perceived issues with
>> the license of a document describing the format of the SPCR, a
>> document whose license was explicitly changed in response to previous
>> concerns raised by yourself, not the code that actually implements
>> SPCR support.
> 
> Not exactly, because there is no such thing as patenting a document.
> 
> The specification purports that patents apply to implementations of
> SPCR, and grants rights to those patents under two optional licenses.
> 
> What I want is what is expected of any submitter to the Linux Kernel:
> that the submitter has the legal right to do so as spelled out
> in the Developer's Certificate of Origin (the text of which is in
> Documentation/SubmittingPatches).
> 
> To have the legal right to submit material to which patents apply
> requires a license.
> 
> So either
> A) the submission does not infringe, and no patent license is required, or
> B) the submission requires a patent license because otherwise it would
> infringe.
> 
> If A, what I would like to see is the due diligence as to why the
> submission does not infringe.
> 
> If B, FSF maintains an online list of GPL compatible licenses.
> Neither of the two licenses offered are on the FSF online list.
> 
> The FSF also maintains a compliance lab and will render opinions on
> the GPL compatibility of a license. They can be reached at
> licensing at fsf.org
> 
> Alternatively, a statement from Linux Foundation that this
> has been discussed and agreed upon privately is also fine.
> 
> Alternatively, any sign-off from ARM, Linaro or Red Hat legal that either
> a) one of those licenses is GPL compatible, or
> b) the submission does not infringe on the purported patents (and why)
> 
> 
>> However you retain the opinion that the license under which the table
>> is published "is not GPL compatible". So under what license would you
>> like to see this table specification released?
> 
> Assuming none of the options above is acceptable, any license in the
> "GPL-Compatible Free Software Licenses" listed here:
> 
> http://www.gnu.org/licenses/license-list.en.html#GPLCompatibleLicenses

FWIW, my specific concern with both licenses offered by Microsoft
is that both contain a patent retaliation provision.

For example, in the Open Web Foundation OWFa 1.0, Patents and Copyright
Grants:

3.1.2.  Termination.

3.1.2.1.  As a Result of Claims by You.  All rights, grants, and promises made by me to you under this Agreement are terminated if you file, maintain, or voluntarily participate in a lawsuit against me or any person or entity asserting that its Permitted Uses infringe any  Granted Claims you would have had the right to enforce had you signed this Agreement, unless that suit was in response to a corresponding suit first brought against you.


There is no such provision in the GPL.

Note also that Aleksey only refers to the GPL in the submission, and
neither of these other licenses which actually grant the
patent license(s). That's also not ok; file header must include
the necessary licenses for which the submission was made.



>> Jon.
>>
>> -- Computer Architect | Sent from my 64-bit #ARM Powered phone
>>>> On Mar 4, 2016, at 05:08, Peter Hurley <peter@hurleysoftware.com> wrote:
>>>>
>>>> Hi Al,
>>>>
>>>> Somehow your email was filtered. Apologies for that.
>>>>
>>>>>> On 02/10/2016 03:39 PM, Al Stone wrote:
>>>>>>>> On 01/27/2016 05:17 AM, Aleksey Makarov wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>>>> On 01/25/2016 07:11 PM, Peter Hurley wrote:
>>>>>>>>>>>> On 01/25/2016 03:45 AM, Aleksey Makarov wrote:
>>>>>>>>>>>> This patchset is based on the patchset by Leif Lindholm [1]
>>>>>>>>>>>>
>>>>>>>>>>>> 'ARM Server Base Boot Requirements' [2] mention SPCR 
>>>>>>>>>>>> (Serial Port Console Redirection Table) [3] as a mandatory
>>>>>>>>>>>> ACPI table that specifies the configuration of serial console.
>>>>>>>>>>>>
>>>>>>>>>>>> Licensing concerns have prevented implementing it in the past, but as of
>>>>>>>>>>>> 10 August 2015, these tables have both been released also under 
>>>>>>>>>>>> OWF 1.0 [4].
>>>>>>>>>>
>>>>>>>>>> This license has a patent retaliation provision, which makes it
>>>>>>>>>> incompatible with GPLv2.
>>>>>>>>>>
>>>>>>>>>> *If the license applies to this code*, then this patch set does not
>>>>>>>>>> meet the criteria for submission.
>>>>>>>>
>>>>>>>> The license applies not to this code but to the document describing the tables.
>>>>>>
>>>>>> Just for the record, the SPCR table struct definition has been part
>>>>>> of the Linux kernel since at least commit b24aad44 on 2009-07-24
>>>>>> (line 1112 of include/acpi/actbl2.h) -- or so git blame tells me.
>>>>
>>>> Just to be clear here:
>>>>
>>>> The Microsoft specification, which defines the SPCR table struct and which
>>>> this patch series relies on, notes that patents apply. Specifically, it
>>>> says:
>>>>
>>>> Patent Notice:
>>>> Microsoft is making certain patent rights available for implementations of this specification under two options:
>>>> 1)  Microsoft?s Community Promise, available at http://www.microsoft.com/openspecifications/en/us/programs/community-promise/default.aspx; or
>>>> 2)  The Open Web Foundation Final Specification Agreement Version 1.0 ("OWF 1.0") as of October 1, 2012, available at http://www.openwebfoundation.org/legal/the-owf-1-0-agreements/owfa-1-0. 
>>>> Version 1.03 ? August 10, 2015
>>>>
>>>> I don't believe either of those patent licenses are GPL compatible.
>>>>
>>>> Unless you're saying Red Hat is signing off on this?
>>>>
>>>> Regards,
>>>> Peter Hurley
>