[PATCH v2 0/2] acpi, x86: Add SPCR table support

[PATCH v2 0/2] acpi, x86: Add SPCR table support

[Prarit Bhargava]

The SPCR (Serial Port Console Redirection) Table provides information
about the configuration of the serial port.  This information can be used to
configure the early console.

SPCR support was added for ARM64 and is made available across all arches
in this patchset.  The first patch adds a weak per-arch configuration function
and moves the SPCR code into ACPI.  The second patch adds support to x86.

The existing behaviour on ARM64 is maintained.  If the SPCR exists the
earlycon and console are automatically configured.

The existing default behaviour on x86 is also maintained.  If no console or
earlycon parameter is defined and the SPCR exists, the serial port is not
configured.  If the earlycon parameter is used both the early console
and the console are configured using the data from the SPCR.

Additional testing (requested by Timur): On Gigabyte (no quirks), Qualcomm
Amberwing systems to test Qualcomm quirks, HPE Mantis system to test xgene
quirks.  Tested several x86 systems with and without a SPCR to confirm
no changes in default behaviour.

[v2]: See 1/2 for code changes.

Signed-off-by: Prarit Bhargava <prarit@redhat.com>
Cc: linux-doc@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-pm@vger.kernel.org
Cc: linux-acpi@vger.kernel.org
Cc: linux-serial@vger.kernel.org
Cc: Bhupesh Sharma <bhsharma@redhat.com>
Cc: Lv Zheng <lv.zheng@intel.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: x86@kernel.org
Cc: Jonathan Corbet <corbet@lwn.net>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
Cc: Timur Tabi <timur@codeaurora.org>

Prarit Bhargava (2):
  acpi, spcr: Make SPCR avialable to other architectures
  acpi, x86: Use SPCR table for earlycon on x86

 Documentation/admin-guide/kernel-parameters.txt |   9 +-
 arch/arm64/kernel/acpi.c                        | 128 ++++++++++++++++-
 arch/x86/kernel/acpi/boot.c                     |   4 +
 drivers/acpi/Kconfig                            |   7 +-
 drivers/acpi/spcr.c                             | 180 ++++++------------------
 drivers/tty/serial/earlycon.c                   |  15 +-
 include/linux/acpi.h                            |  14 +-
 include/linux/serial_core.h                     |   2 -
 8 files changed, 198 insertions(+), 161 deletions(-)

-- 
2.15.0.rc0.39.g2f0e14e64

[PATCH v2 0/2] acpi, x86: Add SPCR table support

[Prarit Bhargava]

The SPCR (Serial Port Console Redirection) Table provides information
about the configuration of the serial port.  This information can be used to
configure the early console.

SPCR support was added for ARM64 and is made available across all arches
in this patchset.  The first patch adds a weak per-arch configuration function
and moves the SPCR code into ACPI.  The second patch adds support to x86.

The existing behaviour on ARM64 is maintained.  If the SPCR exists the
earlycon and console are automatically configured.

The existing default behaviour on x86 is also maintained.  If no console or
earlycon parameter is defined and the SPCR exists, the serial port is not
configured.  If the earlycon parameter is used both the early console
and the console are configured using the data from the SPCR.

Additional testing (requested by Timur): On Gigabyte (no quirks), Qualcomm
Amberwing systems to test Qualcomm quirks, HPE Mantis system to test xgene
quirks.  Tested several x86 systems with and without a SPCR to confirm
no changes in default behaviour.

[v2]: See 1/2 for code changes.

Signed-off-by: Prarit Bhargava <prarit@redhat.com>
Cc: linux-doc at vger.kernel.org
Cc: linux-kernel at vger.kernel.org
Cc: linux-arm-kernel at lists.infradead.org
Cc: linux-pm at vger.kernel.org
Cc: linux-acpi at vger.kernel.org
Cc: linux-serial at vger.kernel.org
Cc: Bhupesh Sharma <bhsharma@redhat.com>
Cc: Lv Zheng <lv.zheng@intel.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: x86 at kernel.org
Cc: Jonathan Corbet <corbet@lwn.net>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
Cc: Timur Tabi <timur@codeaurora.org>

Prarit Bhargava (2):
  acpi, spcr: Make SPCR avialable to other architectures
  acpi, x86: Use SPCR table for earlycon on x86

 Documentation/admin-guide/kernel-parameters.txt |   9 +-
 arch/arm64/kernel/acpi.c                        | 128 ++++++++++++++++-
 arch/x86/kernel/acpi/boot.c                     |   4 +
 drivers/acpi/Kconfig                            |   7 +-
 drivers/acpi/spcr.c                             | 180 ++++++------------------
 drivers/tty/serial/earlycon.c                   |  15 +-
 include/linux/acpi.h                            |  14 +-
 include/linux/serial_core.h                     |   2 -
 8 files changed, 198 insertions(+), 161 deletions(-)

-- 
2.15.0.rc0.39.g2f0e14e64

[PATCH v2 1/2] acpi, spcr: Make SPCR avialable to other architectures

[Prarit Bhargava]

Other architectures can use SPCR to setup an early console or console
but the current code is ARM64 specific.

Change the name of parse_spcr() to acpi_parse_spcr().  Add a weak
function acpi_arch_setup_console() that can be used for arch-specific
setup.  Move flags into ACPI code.  Update the Documention on the use of
the SPCR.

[v2]: Don't return an error in the baud_rate check of acpi_parse_spcr().
Keep ACPI_SPCR_TABLE selected for ARM64.  Fix 8-bit port access width
mmio value.  Move baud rate check earlier.

Signed-off-by: Prarit Bhargava <prarit@redhat.com>
Cc: linux-doc@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-pm@vger.kernel.org
Cc: linux-acpi@vger.kernel.org
Cc: linux-serial@vger.kernel.org
Cc: Bhupesh Sharma <bhsharma@redhat.com>
Cc: Lv Zheng <lv.zheng@intel.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: x86@kernel.org
Cc: Jonathan Corbet <corbet@lwn.net>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
Cc: Timur Tabi <timur@codeaurora.org>
---
 Documentation/admin-guide/kernel-parameters.txt |   6 +-
 arch/arm64/kernel/acpi.c                        | 128 ++++++++++++++++-
 drivers/acpi/Kconfig                            |   7 +-
 drivers/acpi/spcr.c                             | 175 ++++++------------------
 drivers/tty/serial/earlycon.c                   |  15 +-
 include/linux/acpi.h                            |  11 +-
 include/linux/serial_core.h                     |   2 -
 7 files changed, 184 insertions(+), 160 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 6571fbfdb2a1..0d173289c67e 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -914,9 +914,9 @@
 
 	earlycon=	[KNL] Output early console device and options.
 
-			When used with no options, the early console is
-			determined by the stdout-path property in device
-			tree's chosen node.
+			[ARM64] The early console is determined by the
+			stdout-path property in device tree's chosen node,
+			or determined by the ACPI SPCR table.
 
 		cdns,<addr>[,options]
 			Start an early, polled-mode console on a Cadence
diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c
index b3162715ed78..b3e33bbdf3b7 100644
--- a/arch/arm64/kernel/acpi.c
+++ b/arch/arm64/kernel/acpi.c
@@ -25,7 +25,6 @@
 #include <linux/memblock.h>
 #include <linux/of_fdt.h>
 #include <linux/smp.h>
-#include <linux/serial_core.h>
 
 #include <asm/cputype.h>
 #include <asm/cpu_ops.h>
@@ -177,6 +176,128 @@ static int __init acpi_fadt_sanity_check(void)
 	return ret;
 }
 
+/*
+ * Erratum 44 for QDF2432v1 and QDF2400v1 SoCs describes the BUSY bit as
+ * occasionally getting stuck as 1. To avoid the potential for a hang, check
+ * TXFE == 0 instead of BUSY == 1. This may not be suitable for all UART
+ * implementations, so only do so if an affected platform is detected in
+ * acpi_parse_spcr().
+ */
+bool qdf2400_e44_present;
+EXPORT_SYMBOL(qdf2400_e44_present);
+
+/*
+ * Some Qualcomm Datacenter Technologies SoCs have a defective UART BUSY bit.
+ * Detect them by examining the OEM fields in the SPCR header, similar to PCI
+ * quirk detection in pci_mcfg.c.
+ */
+static bool qdf2400_erratum_44_present(struct acpi_table_header *h)
+{
+	if (memcmp(h->oem_id, "QCOM  ", ACPI_OEM_ID_SIZE))
+		return false;
+
+	if (!memcmp(h->oem_table_id, "QDF2432 ", ACPI_OEM_TABLE_ID_SIZE))
+		return true;
+
+	if (!memcmp(h->oem_table_id, "QDF2400 ", ACPI_OEM_TABLE_ID_SIZE) &&
+			h->oem_revision == 1)
+		return true;
+
+	return false;
+}
+
+/*
+ * APM X-Gene v1 and v2 UART hardware is an 16550 like device but has its
+ * register aligned to 32-bit. In addition, the BIOS also encoded the
+ * access width to be 8 bits. This function detects this errata condition.
+ */
+static bool xgene_8250_erratum_present(struct acpi_table_spcr *tb)
+{
+	bool xgene_8250 = false;
+
+	if (tb->interface_type != ACPI_DBG2_16550_COMPATIBLE)
+		return false;
+
+	if (memcmp(tb->header.oem_id, "APMC0D", ACPI_OEM_ID_SIZE) &&
+	    memcmp(tb->header.oem_id, "HPE   ", ACPI_OEM_ID_SIZE))
+		return false;
+
+	if (!memcmp(tb->header.oem_table_id, "XGENESPC",
+	    ACPI_OEM_TABLE_ID_SIZE) && tb->header.oem_revision == 0)
+		xgene_8250 = true;
+
+	if (!memcmp(tb->header.oem_table_id, "ProLiant",
+	    ACPI_OEM_TABLE_ID_SIZE) && tb->header.oem_revision == 1)
+		xgene_8250 = true;
+
+	return xgene_8250;
+}
+
+int acpi_arch_setup_console(struct acpi_table_spcr *table,
+			    char *opts, char *uart, char *iotype,
+			    int baud_rate, bool earlycon)
+{
+	if (table->header.revision < 2) {
+		pr_err("wrong table version\n");
+		return -ENOENT;
+	}
+
+	switch (table->interface_type) {
+	case ACPI_DBG2_ARM_SBSA_32BIT:
+		snprintf(iotype, ACPI_SPCR_BUF_SIZE, "mmio32");
+		/* fall through */
+	case ACPI_DBG2_ARM_PL011:
+	case ACPI_DBG2_ARM_SBSA_GENERIC:
+	case ACPI_DBG2_BCM2835:
+		snprintf(uart, ACPI_SPCR_BUF_SIZE, "pl011");
+		break;
+	default:
+		if (strlen(uart) == 0)
+			return -ENOENT;
+	}
+
+	/*
+	 * If the E44 erratum is required, then we need to tell the pl011
+	 * driver to implement the work-around.
+	 *
+	 * The global variable is used by the probe function when it
+	 * creates the UARTs, whether or not they're used as a console.
+	 *
+	 * If the user specifies "traditional" earlycon, the qdf2400_e44
+	 * console name matches the EARLYCON_DECLARE() statement, and
+	 * SPCR is not used.  Parameter "earlycon" is false.
+	 *
+	 * If the user specifies "SPCR" earlycon, then we need to update
+	 * the console name so that it also says "qdf2400_e44".  Parameter
+	 * "earlycon" is true.
+	 *
+	 * For consistency, if we change the console name, then we do it
+	 * for everyone, not just earlycon.
+	 */
+	if (qdf2400_erratum_44_present(&table->header)) {
+		qdf2400_e44_present = true;
+		if (earlycon)
+			snprintf(uart, ACPI_SPCR_BUF_SIZE, "qdf2400_e44");
+	}
+
+	if (xgene_8250_erratum_present(table)) {
+		snprintf(iotype, ACPI_SPCR_BUF_SIZE, "mmio32");
+
+		/* for xgene v1 and v2 we don't know the clock rate of the
+		 * UART so don't attempt to change to the baud rate state
+		 * in the table because driver cannot calculate the dividers
+		 */
+		snprintf(opts, ACPI_SPCR_OPTS_SIZE, "%s,%s,0x%llx", uart,
+			 iotype, table->serial_port.address);
+	} else {
+		snprintf(opts, ACPI_SPCR_OPTS_SIZE, "%s,%s,0x%llx,%d", uart,
+			 iotype, table->serial_port.address, baud_rate);
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL(acpi_arch_setup_console);
+
 /*
  * acpi_boot_table_init() called from setup_arch(), always.
  *	1. find RSDP and get its address, and then find XSDT
@@ -230,10 +351,11 @@ void __init acpi_boot_table_init(void)
 
 done:
 	if (acpi_disabled) {
-		if (earlycon_init_is_deferred)
+		if (console_acpi_spcr_enable)
 			early_init_dt_scan_chosen_stdout();
 	} else {
-		parse_spcr(earlycon_init_is_deferred);
+		/* Always enable the ACPI SPCR console */
+		acpi_parse_spcr(console_acpi_spcr_enable);
 		if (IS_ENABLED(CONFIG_ACPI_BGRT))
 			acpi_table_parse(ACPI_SIG_BGRT, acpi_parse_bgrt);
 	}
diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
index 46505396869e..9ae98eeada76 100644
--- a/drivers/acpi/Kconfig
+++ b/drivers/acpi/Kconfig
@@ -79,7 +79,12 @@ config ACPI_DEBUGGER_USER
 endif
 
 config ACPI_SPCR_TABLE
-	bool
+	bool "ACPI Serial Port Console Redirection Support"
+	default y if ARM64
+	help
+	  Enable support for Serial Port Console Redirection (SPCR) Table.
+	  This table provides information about the configuration of the
+	  earlycon console.
 
 config ACPI_LPIT
 	bool
diff --git a/drivers/acpi/spcr.c b/drivers/acpi/spcr.c
index 324b35bfe781..f4bb8110e404 100644
--- a/drivers/acpi/spcr.c
+++ b/drivers/acpi/spcr.c
@@ -16,65 +16,18 @@
 #include <linux/kernel.h>
 #include <linux/serial_core.h>
 
-/*
- * Erratum 44 for QDF2432v1 and QDF2400v1 SoCs describes the BUSY bit as
- * occasionally getting stuck as 1. To avoid the potential for a hang, check
- * TXFE == 0 instead of BUSY == 1. This may not be suitable for all UART
- * implementations, so only do so if an affected platform is detected in
- * parse_spcr().
- */
-bool qdf2400_e44_present;
-EXPORT_SYMBOL(qdf2400_e44_present);
-
-/*
- * Some Qualcomm Datacenter Technologies SoCs have a defective UART BUSY bit.
- * Detect them by examining the OEM fields in the SPCR header, similiar to PCI
- * quirk detection in pci_mcfg.c.
- */
-static bool qdf2400_erratum_44_present(struct acpi_table_header *h)
-{
-	if (memcmp(h->oem_id, "QCOM  ", ACPI_OEM_ID_SIZE))
-		return false;
-
-	if (!memcmp(h->oem_table_id, "QDF2432 ", ACPI_OEM_TABLE_ID_SIZE))
-		return true;
-
-	if (!memcmp(h->oem_table_id, "QDF2400 ", ACPI_OEM_TABLE_ID_SIZE) &&
-			h->oem_revision == 1)
-		return true;
-
-	return false;
-}
-
-/*
- * APM X-Gene v1 and v2 UART hardware is an 16550 like device but has its
- * register aligned to 32-bit. In addition, the BIOS also encoded the
- * access width to be 8 bits. This function detects this errata condition.
- */
-static bool xgene_8250_erratum_present(struct acpi_table_spcr *tb)
+int __weak acpi_arch_setup_console(struct acpi_table_spcr *table,
+				   char *opts, char *uart, char *iotype,
+				   int baud_rate, bool earlycon)
 {
-	bool xgene_8250 = false;
-
-	if (tb->interface_type != ACPI_DBG2_16550_COMPATIBLE)
-		return false;
-
-	if (memcmp(tb->header.oem_id, "APMC0D", ACPI_OEM_ID_SIZE) &&
-	    memcmp(tb->header.oem_id, "HPE   ", ACPI_OEM_ID_SIZE))
-		return false;
-
-	if (!memcmp(tb->header.oem_table_id, "XGENESPC",
-	    ACPI_OEM_TABLE_ID_SIZE) && tb->header.oem_revision == 0)
-		xgene_8250 = true;
-
-	if (!memcmp(tb->header.oem_table_id, "ProLiant",
-	    ACPI_OEM_TABLE_ID_SIZE) && tb->header.oem_revision == 1)
-		xgene_8250 = true;
-
-	return xgene_8250;
+	snprintf(opts, ACPI_SPCR_OPTS_SIZE, "%s,%s,0x%llx,%d", uart, iotype,
+		 table->serial_port.address, baud_rate);
+	return 0;
 }
 
+bool console_acpi_spcr_enable __initdata;
 /**
- * parse_spcr() - parse ACPI SPCR table and add preferred console
+ * acpi_parse_spcr() - parse ACPI SPCR table and add preferred console
  *
  * @earlycon: set up earlycon for the console specified by the table
  *
@@ -86,13 +39,13 @@ static bool xgene_8250_erratum_present(struct acpi_table_spcr *tb)
  * from arch initialization code as soon as the DT/ACPI decision is made.
  *
  */
-int __init parse_spcr(bool earlycon)
+int __init acpi_parse_spcr(bool earlycon)
 {
-	static char opts[64];
+	static char opts[ACPI_SPCR_OPTS_SIZE];
+	static char uart[ACPI_SPCR_BUF_SIZE];
+	static char iotype[ACPI_SPCR_BUF_SIZE];
 	struct acpi_table_spcr *table;
 	acpi_status status;
-	char *uart;
-	char *iotype;
 	int baud_rate;
 	int err;
 
@@ -105,48 +58,6 @@ int __init parse_spcr(bool earlycon)
 	if (ACPI_FAILURE(status))
 		return -ENOENT;
 
-	if (table->header.revision < 2) {
-		err = -ENOENT;
-		pr_err("wrong table version\n");
-		goto done;
-	}
-
-	if (table->serial_port.space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY) {
-		switch (ACPI_ACCESS_BIT_WIDTH((
-			table->serial_port.access_width))) {
-		default:
-			pr_err("Unexpected SPCR Access Width.  Defaulting to byte size\n");
-		case 8:
-			iotype = "mmio";
-			break;
-		case 16:
-			iotype = "mmio16";
-			break;
-		case 32:
-			iotype = "mmio32";
-			break;
-		}
-	} else
-		iotype = "io";
-
-	switch (table->interface_type) {
-	case ACPI_DBG2_ARM_SBSA_32BIT:
-		iotype = "mmio32";
-		/* fall through */
-	case ACPI_DBG2_ARM_PL011:
-	case ACPI_DBG2_ARM_SBSA_GENERIC:
-	case ACPI_DBG2_BCM2835:
-		uart = "pl011";
-		break;
-	case ACPI_DBG2_16550_COMPATIBLE:
-	case ACPI_DBG2_16550_SUBSET:
-		uart = "uart";
-		break;
-	default:
-		err = -ENOENT;
-		goto done;
-	}
-
 	switch (table->baud_rate) {
 	case 3:
 		baud_rate = 9600;
@@ -165,43 +76,36 @@ int __init parse_spcr(bool earlycon)
 		goto done;
 	}
 
-	/*
-	 * If the E44 erratum is required, then we need to tell the pl011
-	 * driver to implement the work-around.
-	 *
-	 * The global variable is used by the probe function when it
-	 * creates the UARTs, whether or not they're used as a console.
-	 *
-	 * If the user specifies "traditional" earlycon, the qdf2400_e44
-	 * console name matches the EARLYCON_DECLARE() statement, and
-	 * SPCR is not used.  Parameter "earlycon" is false.
-	 *
-	 * If the user specifies "SPCR" earlycon, then we need to update
-	 * the console name so that it also says "qdf2400_e44".  Parameter
-	 * "earlycon" is true.
-	 *
-	 * For consistency, if we change the console name, then we do it
-	 * for everyone, not just earlycon.
-	 */
-	if (qdf2400_erratum_44_present(&table->header)) {
-		qdf2400_e44_present = true;
-		if (earlycon)
-			uart = "qdf2400_e44";
+	switch (table->interface_type) {
+	case ACPI_DBG2_16550_COMPATIBLE:
+	case ACPI_DBG2_16550_SUBSET:
+		snprintf(uart, ACPI_SPCR_BUF_SIZE, "uart");
+		break;
+	default:
+		break;
 	}
 
-	if (xgene_8250_erratum_present(table)) {
-		iotype = "mmio32";
+	if (table->serial_port.space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY) {
+		u8 width = ACPI_ACCESS_BIT_WIDTH((
+					table->serial_port.access_width));
+		switch (width) {
+		default:
+			pr_err("Unexpected SPCR Access Width.  Defaulting to byte size\n");
+		case 8:
+			snprintf(iotype, ACPI_SPCR_BUF_SIZE, "mmio");
+			break;
+		case 16:
+		case 32:
+			snprintf(iotype, ACPI_SPCR_BUF_SIZE, "mmio%d", width);
+			break;
+		}
+	} else
+		snprintf(iotype, ACPI_SPCR_BUF_SIZE, "io");
 
-		/* for xgene v1 and v2 we don't know the clock rate of the
-		 * UART so don't attempt to change to the baud rate state
-		 * in the table because driver cannot calculate the dividers
-		 */
-		snprintf(opts, sizeof(opts), "%s,%s,0x%llx", uart, iotype,
-			 table->serial_port.address);
-	} else {
-		snprintf(opts, sizeof(opts), "%s,%s,0x%llx,%d", uart, iotype,
-			 table->serial_port.address, baud_rate);
-	}
+	err = acpi_arch_setup_console(table, opts, uart, iotype, baud_rate,
+				      earlycon);
+	if (err)
+		goto done;
 
 	pr_info("console: %s\n", opts);
 
@@ -209,7 +113,6 @@ int __init parse_spcr(bool earlycon)
 		setup_earlycon(opts);
 
 	err = add_preferred_console(uart, 0, opts + strlen(uart) + 1);
-
 done:
 	acpi_put_table((struct acpi_table_header *)table);
 	return err;
diff --git a/drivers/tty/serial/earlycon.c b/drivers/tty/serial/earlycon.c
index 4c8b80f1c688..b22afb62c7a3 100644
--- a/drivers/tty/serial/earlycon.c
+++ b/drivers/tty/serial/earlycon.c
@@ -196,26 +196,15 @@ int __init setup_earlycon(char *buf)
 	return -ENOENT;
 }
 
-/*
- * When CONFIG_ACPI_SPCR_TABLE is defined, "earlycon" without parameters in
- * command line does not start DT earlycon immediately, instead it defers
- * starting it until DT/ACPI decision is made.  At that time if ACPI is enabled
- * call parse_spcr(), else call early_init_dt_scan_chosen_stdout()
- */
-bool earlycon_init_is_deferred __initdata;
-
 /* early_param wrapper for setup_earlycon() */
 static int __init param_setup_earlycon(char *buf)
 {
 	int err;
 
-	/*
-	 * Just 'earlycon' is a valid param for devicetree earlycons;
-	 * don't generate a warning from parse_early_params() in that case
-	 */
+	/* Just 'earlycon' is a valid param for devicetree and ACPI SPCR. */
 	if (!buf || !buf[0]) {
 		if (IS_ENABLED(CONFIG_ACPI_SPCR_TABLE)) {
-			earlycon_init_is_deferred = true;
+			console_acpi_spcr_enable = true;
 			return 0;
 		} else if (!buf) {
 			return early_init_dt_scan_chosen_stdout();
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index dc1ebfeeb5ec..875d7327d91c 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -1241,10 +1241,17 @@ static inline bool acpi_has_watchdog(void) { return false; }
 #endif
 
 #ifdef CONFIG_ACPI_SPCR_TABLE
+#define ACPI_SPCR_OPTS_SIZE 64
+#define ACPI_SPCR_BUF_SIZE 32
 extern bool qdf2400_e44_present;
-int parse_spcr(bool earlycon);
+extern bool console_acpi_spcr_enable __initdata;
+extern int acpi_arch_setup_console(struct acpi_table_spcr *table,
+				   char *opts, char *uart, char *iotype,
+				   int baud_rate, bool earlycon);
+int acpi_parse_spcr(bool earlycon);
 #else
-static inline int parse_spcr(bool earlycon) { return 0; }
+static const bool console_acpi_spcr_enable;
+static inline int acpi_parse_spcr(bool earlycon) { return 0; }
 #endif
 
 #if IS_ENABLED(CONFIG_ACPI_GENERIC_GSI)
diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index 37b044e78333..abfffb4b1c37 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -376,10 +376,8 @@ extern int of_setup_earlycon(const struct earlycon_id *match,
 			     const char *options);
 
 #ifdef CONFIG_SERIAL_EARLYCON
-extern bool earlycon_init_is_deferred __initdata;
 int setup_earlycon(char *buf);
 #else
-static const bool earlycon_init_is_deferred;
 static inline int setup_earlycon(char *buf) { return 0; }
 #endif
 
-- 
2.15.0.rc0.39.g2f0e14e64

[PATCH v2 1/2] acpi, spcr: Make SPCR avialable to other architectures

[Prarit Bhargava]

Other architectures can use SPCR to setup an early console or console
but the current code is ARM64 specific.

Change the name of parse_spcr() to acpi_parse_spcr().  Add a weak
function acpi_arch_setup_console() that can be used for arch-specific
setup.  Move flags into ACPI code.  Update the Documention on the use of
the SPCR.

[v2]: Don't return an error in the baud_rate check of acpi_parse_spcr().
Keep ACPI_SPCR_TABLE selected for ARM64.  Fix 8-bit port access width
mmio value.  Move baud rate check earlier.

Signed-off-by: Prarit Bhargava <prarit@redhat.com>
Cc: linux-doc at vger.kernel.org
Cc: linux-kernel at vger.kernel.org
Cc: linux-arm-kernel at lists.infradead.org
Cc: linux-pm at vger.kernel.org
Cc: linux-acpi at vger.kernel.org
Cc: linux-serial at vger.kernel.org
Cc: Bhupesh Sharma <bhsharma@redhat.com>
Cc: Lv Zheng <lv.zheng@intel.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: x86 at kernel.org
Cc: Jonathan Corbet <corbet@lwn.net>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
Cc: Timur Tabi <timur@codeaurora.org>
---
 Documentation/admin-guide/kernel-parameters.txt |   6 +-
 arch/arm64/kernel/acpi.c                        | 128 ++++++++++++++++-
 drivers/acpi/Kconfig                            |   7 +-
 drivers/acpi/spcr.c                             | 175 ++++++------------------
 drivers/tty/serial/earlycon.c                   |  15 +-
 include/linux/acpi.h                            |  11 +-
 include/linux/serial_core.h                     |   2 -
 7 files changed, 184 insertions(+), 160 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 6571fbfdb2a1..0d173289c67e 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -914,9 +914,9 @@
 
 	earlycon=	[KNL] Output early console device and options.
 
-			When used with no options, the early console is
-			determined by the stdout-path property in device
-			tree's chosen node.
+			[ARM64] The early console is determined by the
+			stdout-path property in device tree's chosen node,
+			or determined by the ACPI SPCR table.
 
 		cdns,<addr>[,options]
 			Start an early, polled-mode console on a Cadence
diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c
index b3162715ed78..b3e33bbdf3b7 100644
--- a/arch/arm64/kernel/acpi.c
+++ b/arch/arm64/kernel/acpi.c
@@ -25,7 +25,6 @@
 #include <linux/memblock.h>
 #include <linux/of_fdt.h>
 #include <linux/smp.h>
-#include <linux/serial_core.h>
 
 #include <asm/cputype.h>
 #include <asm/cpu_ops.h>
@@ -177,6 +176,128 @@ static int __init acpi_fadt_sanity_check(void)
 	return ret;
 }
 
+/*
+ * Erratum 44 for QDF2432v1 and QDF2400v1 SoCs describes the BUSY bit as
+ * occasionally getting stuck as 1. To avoid the potential for a hang, check
+ * TXFE == 0 instead of BUSY == 1. This may not be suitable for all UART
+ * implementations, so only do so if an affected platform is detected in
+ * acpi_parse_spcr().
+ */
+bool qdf2400_e44_present;
+EXPORT_SYMBOL(qdf2400_e44_present);
+
+/*
+ * Some Qualcomm Datacenter Technologies SoCs have a defective UART BUSY bit.
+ * Detect them by examining the OEM fields in the SPCR header, similar to PCI
+ * quirk detection in pci_mcfg.c.
+ */
+static bool qdf2400_erratum_44_present(struct acpi_table_header *h)
+{
+	if (memcmp(h->oem_id, "QCOM  ", ACPI_OEM_ID_SIZE))
+		return false;
+
+	if (!memcmp(h->oem_table_id, "QDF2432 ", ACPI_OEM_TABLE_ID_SIZE))
+		return true;
+
+	if (!memcmp(h->oem_table_id, "QDF2400 ", ACPI_OEM_TABLE_ID_SIZE) &&
+			h->oem_revision == 1)
+		return true;
+
+	return false;
+}
+
+/*
+ * APM X-Gene v1 and v2 UART hardware is an 16550 like device but has its
+ * register aligned to 32-bit. In addition, the BIOS also encoded the
+ * access width to be 8 bits. This function detects this errata condition.
+ */
+static bool xgene_8250_erratum_present(struct acpi_table_spcr *tb)
+{
+	bool xgene_8250 = false;
+
+	if (tb->interface_type != ACPI_DBG2_16550_COMPATIBLE)
+		return false;
+
+	if (memcmp(tb->header.oem_id, "APMC0D", ACPI_OEM_ID_SIZE) &&
+	    memcmp(tb->header.oem_id, "HPE   ", ACPI_OEM_ID_SIZE))
+		return false;
+
+	if (!memcmp(tb->header.oem_table_id, "XGENESPC",
+	    ACPI_OEM_TABLE_ID_SIZE) && tb->header.oem_revision == 0)
+		xgene_8250 = true;
+
+	if (!memcmp(tb->header.oem_table_id, "ProLiant",
+	    ACPI_OEM_TABLE_ID_SIZE) && tb->header.oem_revision == 1)
+		xgene_8250 = true;
+
+	return xgene_8250;
+}
+
+int acpi_arch_setup_console(struct acpi_table_spcr *table,
+			    char *opts, char *uart, char *iotype,
+			    int baud_rate, bool earlycon)
+{
+	if (table->header.revision < 2) {
+		pr_err("wrong table version\n");
+		return -ENOENT;
+	}
+
+	switch (table->interface_type) {
+	case ACPI_DBG2_ARM_SBSA_32BIT:
+		snprintf(iotype, ACPI_SPCR_BUF_SIZE, "mmio32");
+		/* fall through */
+	case ACPI_DBG2_ARM_PL011:
+	case ACPI_DBG2_ARM_SBSA_GENERIC:
+	case ACPI_DBG2_BCM2835:
+		snprintf(uart, ACPI_SPCR_BUF_SIZE, "pl011");
+		break;
+	default:
+		if (strlen(uart) == 0)
+			return -ENOENT;
+	}
+
+	/*
+	 * If the E44 erratum is required, then we need to tell the pl011
+	 * driver to implement the work-around.
+	 *
+	 * The global variable is used by the probe function when it
+	 * creates the UARTs, whether or not they're used as a console.
+	 *
+	 * If the user specifies "traditional" earlycon, the qdf2400_e44
+	 * console name matches the EARLYCON_DECLARE() statement, and
+	 * SPCR is not used.  Parameter "earlycon" is false.
+	 *
+	 * If the user specifies "SPCR" earlycon, then we need to update
+	 * the console name so that it also says "qdf2400_e44".  Parameter
+	 * "earlycon" is true.
+	 *
+	 * For consistency, if we change the console name, then we do it
+	 * for everyone, not just earlycon.
+	 */
+	if (qdf2400_erratum_44_present(&table->header)) {
+		qdf2400_e44_present = true;
+		if (earlycon)
+			snprintf(uart, ACPI_SPCR_BUF_SIZE, "qdf2400_e44");
+	}
+
+	if (xgene_8250_erratum_present(table)) {
+		snprintf(iotype, ACPI_SPCR_BUF_SIZE, "mmio32");
+
+		/* for xgene v1 and v2 we don't know the clock rate of the
+		 * UART so don't attempt to change to the baud rate state
+		 * in the table because driver cannot calculate the dividers
+		 */
+		snprintf(opts, ACPI_SPCR_OPTS_SIZE, "%s,%s,0x%llx", uart,
+			 iotype, table->serial_port.address);
+	} else {
+		snprintf(opts, ACPI_SPCR_OPTS_SIZE, "%s,%s,0x%llx,%d", uart,
+			 iotype, table->serial_port.address, baud_rate);
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL(acpi_arch_setup_console);
+
 /*
  * acpi_boot_table_init() called from setup_arch(), always.
  *	1. find RSDP and get its address, and then find XSDT
@@ -230,10 +351,11 @@ void __init acpi_boot_table_init(void)
 
 done:
 	if (acpi_disabled) {
-		if (earlycon_init_is_deferred)
+		if (console_acpi_spcr_enable)
 			early_init_dt_scan_chosen_stdout();
 	} else {
-		parse_spcr(earlycon_init_is_deferred);
+		/* Always enable the ACPI SPCR console */
+		acpi_parse_spcr(console_acpi_spcr_enable);
 		if (IS_ENABLED(CONFIG_ACPI_BGRT))
 			acpi_table_parse(ACPI_SIG_BGRT, acpi_parse_bgrt);
 	}
diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
index 46505396869e..9ae98eeada76 100644
--- a/drivers/acpi/Kconfig
+++ b/drivers/acpi/Kconfig
@@ -79,7 +79,12 @@ config ACPI_DEBUGGER_USER
 endif
 
 config ACPI_SPCR_TABLE
-	bool
+	bool "ACPI Serial Port Console Redirection Support"
+	default y if ARM64
+	help
+	  Enable support for Serial Port Console Redirection (SPCR) Table.
+	  This table provides information about the configuration of the
+	  earlycon console.
 
 config ACPI_LPIT
 	bool
diff --git a/drivers/acpi/spcr.c b/drivers/acpi/spcr.c
index 324b35bfe781..f4bb8110e404 100644
--- a/drivers/acpi/spcr.c
+++ b/drivers/acpi/spcr.c
@@ -16,65 +16,18 @@
 #include <linux/kernel.h>
 #include <linux/serial_core.h>
 
-/*
- * Erratum 44 for QDF2432v1 and QDF2400v1 SoCs describes the BUSY bit as
- * occasionally getting stuck as 1. To avoid the potential for a hang, check
- * TXFE == 0 instead of BUSY == 1. This may not be suitable for all UART
- * implementations, so only do so if an affected platform is detected in
- * parse_spcr().
- */
-bool qdf2400_e44_present;
-EXPORT_SYMBOL(qdf2400_e44_present);
-
-/*
- * Some Qualcomm Datacenter Technologies SoCs have a defective UART BUSY bit.
- * Detect them by examining the OEM fields in the SPCR header, similiar to PCI
- * quirk detection in pci_mcfg.c.
- */
-static bool qdf2400_erratum_44_present(struct acpi_table_header *h)
-{
-	if (memcmp(h->oem_id, "QCOM  ", ACPI_OEM_ID_SIZE))
-		return false;
-
-	if (!memcmp(h->oem_table_id, "QDF2432 ", ACPI_OEM_TABLE_ID_SIZE))
-		return true;
-
-	if (!memcmp(h->oem_table_id, "QDF2400 ", ACPI_OEM_TABLE_ID_SIZE) &&
-			h->oem_revision == 1)
-		return true;
-
-	return false;
-}
-
-/*
- * APM X-Gene v1 and v2 UART hardware is an 16550 like device but has its
- * register aligned to 32-bit. In addition, the BIOS also encoded the
- * access width to be 8 bits. This function detects this errata condition.
- */
-static bool xgene_8250_erratum_present(struct acpi_table_spcr *tb)
+int __weak acpi_arch_setup_console(struct acpi_table_spcr *table,
+				   char *opts, char *uart, char *iotype,
+				   int baud_rate, bool earlycon)
 {
-	bool xgene_8250 = false;
-
-	if (tb->interface_type != ACPI_DBG2_16550_COMPATIBLE)
-		return false;
-
-	if (memcmp(tb->header.oem_id, "APMC0D", ACPI_OEM_ID_SIZE) &&
-	    memcmp(tb->header.oem_id, "HPE   ", ACPI_OEM_ID_SIZE))
-		return false;
-
-	if (!memcmp(tb->header.oem_table_id, "XGENESPC",
-	    ACPI_OEM_TABLE_ID_SIZE) && tb->header.oem_revision == 0)
-		xgene_8250 = true;
-
-	if (!memcmp(tb->header.oem_table_id, "ProLiant",
-	    ACPI_OEM_TABLE_ID_SIZE) && tb->header.oem_revision == 1)
-		xgene_8250 = true;
-
-	return xgene_8250;
+	snprintf(opts, ACPI_SPCR_OPTS_SIZE, "%s,%s,0x%llx,%d", uart, iotype,
+		 table->serial_port.address, baud_rate);
+	return 0;
 }
 
+bool console_acpi_spcr_enable __initdata;
 /**
- * parse_spcr() - parse ACPI SPCR table and add preferred console
+ * acpi_parse_spcr() - parse ACPI SPCR table and add preferred console
  *
  * @earlycon: set up earlycon for the console specified by the table
  *
@@ -86,13 +39,13 @@ static bool xgene_8250_erratum_present(struct acpi_table_spcr *tb)
  * from arch initialization code as soon as the DT/ACPI decision is made.
  *
  */
-int __init parse_spcr(bool earlycon)
+int __init acpi_parse_spcr(bool earlycon)
 {
-	static char opts[64];
+	static char opts[ACPI_SPCR_OPTS_SIZE];
+	static char uart[ACPI_SPCR_BUF_SIZE];
+	static char iotype[ACPI_SPCR_BUF_SIZE];
 	struct acpi_table_spcr *table;
 	acpi_status status;
-	char *uart;
-	char *iotype;
 	int baud_rate;
 	int err;
 
@@ -105,48 +58,6 @@ int __init parse_spcr(bool earlycon)
 	if (ACPI_FAILURE(status))
 		return -ENOENT;
 
-	if (table->header.revision < 2) {
-		err = -ENOENT;
-		pr_err("wrong table version\n");
-		goto done;
-	}
-
-	if (table->serial_port.space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY) {
-		switch (ACPI_ACCESS_BIT_WIDTH((
-			table->serial_port.access_width))) {
-		default:
-			pr_err("Unexpected SPCR Access Width.  Defaulting to byte size\n");
-		case 8:
-			iotype = "mmio";
-			break;
-		case 16:
-			iotype = "mmio16";
-			break;
-		case 32:
-			iotype = "mmio32";
-			break;
-		}
-	} else
-		iotype = "io";
-
-	switch (table->interface_type) {
-	case ACPI_DBG2_ARM_SBSA_32BIT:
-		iotype = "mmio32";
-		/* fall through */
-	case ACPI_DBG2_ARM_PL011:
-	case ACPI_DBG2_ARM_SBSA_GENERIC:
-	case ACPI_DBG2_BCM2835:
-		uart = "pl011";
-		break;
-	case ACPI_DBG2_16550_COMPATIBLE:
-	case ACPI_DBG2_16550_SUBSET:
-		uart = "uart";
-		break;
-	default:
-		err = -ENOENT;
-		goto done;
-	}
-
 	switch (table->baud_rate) {
 	case 3:
 		baud_rate = 9600;
@@ -165,43 +76,36 @@ int __init parse_spcr(bool earlycon)
 		goto done;
 	}
 
-	/*
-	 * If the E44 erratum is required, then we need to tell the pl011
-	 * driver to implement the work-around.
-	 *
-	 * The global variable is used by the probe function when it
-	 * creates the UARTs, whether or not they're used as a console.
-	 *
-	 * If the user specifies "traditional" earlycon, the qdf2400_e44
-	 * console name matches the EARLYCON_DECLARE() statement, and
-	 * SPCR is not used.  Parameter "earlycon" is false.
-	 *
-	 * If the user specifies "SPCR" earlycon, then we need to update
-	 * the console name so that it also says "qdf2400_e44".  Parameter
-	 * "earlycon" is true.
-	 *
-	 * For consistency, if we change the console name, then we do it
-	 * for everyone, not just earlycon.
-	 */
-	if (qdf2400_erratum_44_present(&table->header)) {
-		qdf2400_e44_present = true;
-		if (earlycon)
-			uart = "qdf2400_e44";
+	switch (table->interface_type) {
+	case ACPI_DBG2_16550_COMPATIBLE:
+	case ACPI_DBG2_16550_SUBSET:
+		snprintf(uart, ACPI_SPCR_BUF_SIZE, "uart");
+		break;
+	default:
+		break;
 	}
 
-	if (xgene_8250_erratum_present(table)) {
-		iotype = "mmio32";
+	if (table->serial_port.space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY) {
+		u8 width = ACPI_ACCESS_BIT_WIDTH((
+					table->serial_port.access_width));
+		switch (width) {
+		default:
+			pr_err("Unexpected SPCR Access Width.  Defaulting to byte size\n");
+		case 8:
+			snprintf(iotype, ACPI_SPCR_BUF_SIZE, "mmio");
+			break;
+		case 16:
+		case 32:
+			snprintf(iotype, ACPI_SPCR_BUF_SIZE, "mmio%d", width);
+			break;
+		}
+	} else
+		snprintf(iotype, ACPI_SPCR_BUF_SIZE, "io");
 
-		/* for xgene v1 and v2 we don't know the clock rate of the
-		 * UART so don't attempt to change to the baud rate state
-		 * in the table because driver cannot calculate the dividers
-		 */
-		snprintf(opts, sizeof(opts), "%s,%s,0x%llx", uart, iotype,
-			 table->serial_port.address);
-	} else {
-		snprintf(opts, sizeof(opts), "%s,%s,0x%llx,%d", uart, iotype,
-			 table->serial_port.address, baud_rate);
-	}
+	err = acpi_arch_setup_console(table, opts, uart, iotype, baud_rate,
+				      earlycon);
+	if (err)
+		goto done;
 
 	pr_info("console: %s\n", opts);
 
@@ -209,7 +113,6 @@ int __init parse_spcr(bool earlycon)
 		setup_earlycon(opts);
 
 	err = add_preferred_console(uart, 0, opts + strlen(uart) + 1);
-
 done:
 	acpi_put_table((struct acpi_table_header *)table);
 	return err;
diff --git a/drivers/tty/serial/earlycon.c b/drivers/tty/serial/earlycon.c
index 4c8b80f1c688..b22afb62c7a3 100644
--- a/drivers/tty/serial/earlycon.c
+++ b/drivers/tty/serial/earlycon.c
@@ -196,26 +196,15 @@ int __init setup_earlycon(char *buf)
 	return -ENOENT;
 }
 
-/*
- * When CONFIG_ACPI_SPCR_TABLE is defined, "earlycon" without parameters in
- * command line does not start DT earlycon immediately, instead it defers
- * starting it until DT/ACPI decision is made.  At that time if ACPI is enabled
- * call parse_spcr(), else call early_init_dt_scan_chosen_stdout()
- */
-bool earlycon_init_is_deferred __initdata;
-
 /* early_param wrapper for setup_earlycon() */
 static int __init param_setup_earlycon(char *buf)
 {
 	int err;
 
-	/*
-	 * Just 'earlycon' is a valid param for devicetree earlycons;
-	 * don't generate a warning from parse_early_params() in that case
-	 */
+	/* Just 'earlycon' is a valid param for devicetree and ACPI SPCR. */
 	if (!buf || !buf[0]) {
 		if (IS_ENABLED(CONFIG_ACPI_SPCR_TABLE)) {
-			earlycon_init_is_deferred = true;
+			console_acpi_spcr_enable = true;
 			return 0;
 		} else if (!buf) {
 			return early_init_dt_scan_chosen_stdout();
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index dc1ebfeeb5ec..875d7327d91c 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -1241,10 +1241,17 @@ static inline bool acpi_has_watchdog(void) { return false; }
 #endif
 
 #ifdef CONFIG_ACPI_SPCR_TABLE
+#define ACPI_SPCR_OPTS_SIZE 64
+#define ACPI_SPCR_BUF_SIZE 32
 extern bool qdf2400_e44_present;
-int parse_spcr(bool earlycon);
+extern bool console_acpi_spcr_enable __initdata;
+extern int acpi_arch_setup_console(struct acpi_table_spcr *table,
+				   char *opts, char *uart, char *iotype,
+				   int baud_rate, bool earlycon);
+int acpi_parse_spcr(bool earlycon);
 #else
-static inline int parse_spcr(bool earlycon) { return 0; }
+static const bool console_acpi_spcr_enable;
+static inline int acpi_parse_spcr(bool earlycon) { return 0; }
 #endif
 
 #if IS_ENABLED(CONFIG_ACPI_GENERIC_GSI)
diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index 37b044e78333..abfffb4b1c37 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -376,10 +376,8 @@ extern int of_setup_earlycon(const struct earlycon_id *match,
 			     const char *options);
 
 #ifdef CONFIG_SERIAL_EARLYCON
-extern bool earlycon_init_is_deferred __initdata;
 int setup_earlycon(char *buf);
 #else
-static const bool earlycon_init_is_deferred;
 static inline int setup_earlycon(char *buf) { return 0; }
 #endif
 
-- 
2.15.0.rc0.39.g2f0e14e64

[PATCH v2 2/2] acpi, x86: Use SPCR table for earlycon on x86

[Prarit Bhargava]

The ACPI SPCR code has been used to define an earlycon console for ARM64
and can be used for x86.

Modify the ACPI SPCR parsing code to account for console behaviour
differences between ARM64 and x86.  Initialize the SPCR code from
x86 ACPI initialization code.

Signed-off-by: Prarit Bhargava <prarit@redhat.com>
Cc: linux-doc@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-pm@vger.kernel.org
Cc: linux-acpi@vger.kernel.org
Cc: linux-serial@vger.kernel.org
Cc: Bhupesh Sharma <bhsharma@redhat.com>
Cc: Lv Zheng <lv.zheng@intel.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: x86@kernel.org
Cc: Jonathan Corbet <corbet@lwn.net>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
Cc: Timur Tabi <timur@codeaurora.org>
---
 Documentation/admin-guide/kernel-parameters.txt | 3 +++
 arch/arm64/kernel/acpi.c                        | 2 +-
 arch/x86/kernel/acpi/boot.c                     | 4 ++++
 drivers/acpi/Kconfig                            | 2 +-
 drivers/acpi/spcr.c                             | 7 +++++--
 include/linux/acpi.h                            | 7 +++++--
 6 files changed, 19 insertions(+), 6 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 0d173289c67e..c7cc890a0e81 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -918,6 +918,9 @@
 			stdout-path property in device tree's chosen node,
 			or determined by the ACPI SPCR table.
 
+			[X86] When used with no options the early console is
+			determined by the ACPI SPCR table.
+
 		cdns,<addr>[,options]
 			Start an early, polled-mode console on a Cadence
 			(xuartps) serial port at the specified address. Only
diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c
index b3e33bbdf3b7..aaee4864b63e 100644
--- a/arch/arm64/kernel/acpi.c
+++ b/arch/arm64/kernel/acpi.c
@@ -355,7 +355,7 @@ void __init acpi_boot_table_init(void)
 			early_init_dt_scan_chosen_stdout();
 	} else {
 		/* Always enable the ACPI SPCR console */
-		acpi_parse_spcr(console_acpi_spcr_enable);
+		acpi_parse_spcr(console_acpi_spcr_enable, true);
 		if (IS_ENABLED(CONFIG_ACPI_BGRT))
 			acpi_table_parse(ACPI_SIG_BGRT, acpi_parse_bgrt);
 	}
diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c
index f4c463df8b08..f01a03643cff 100644
--- a/arch/x86/kernel/acpi/boot.c
+++ b/arch/x86/kernel/acpi/boot.c
@@ -36,6 +36,7 @@
 #include <linux/ioport.h>
 #include <linux/pci.h>
 #include <linux/efi-bgrt.h>
+#include <linux/serial_core.h>
 
 #include <asm/e820/api.h>
 #include <asm/irqdomain.h>
@@ -1626,6 +1627,9 @@ int __init acpi_boot_init(void)
 	if (!acpi_noirq)
 		x86_init.pci.init = pci_acpi_init;
 
+	/* Do not enable ACPI SPCR console by default */
+	acpi_parse_spcr(console_acpi_spcr_enable, console_acpi_spcr_enable);
+
 	return 0;
 }
 
diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
index 9ae98eeada76..2d4e841e0682 100644
--- a/drivers/acpi/Kconfig
+++ b/drivers/acpi/Kconfig
@@ -80,7 +80,7 @@ endif
 
 config ACPI_SPCR_TABLE
 	bool "ACPI Serial Port Console Redirection Support"
-	default y if ARM64
+	default y if (X86 || ARM64)
 	help
 	  Enable support for Serial Port Console Redirection (SPCR) Table.
 	  This table provides information about the configuration of the
diff --git a/drivers/acpi/spcr.c b/drivers/acpi/spcr.c
index f4bb8110e404..c9469b488527 100644
--- a/drivers/acpi/spcr.c
+++ b/drivers/acpi/spcr.c
@@ -39,7 +39,7 @@ bool console_acpi_spcr_enable __initdata;
  * from arch initialization code as soon as the DT/ACPI decision is made.
  *
  */
-int __init acpi_parse_spcr(bool earlycon)
+int __init acpi_parse_spcr(bool earlycon, bool enable_console)
 {
 	static char opts[ACPI_SPCR_OPTS_SIZE];
 	static char uart[ACPI_SPCR_BUF_SIZE];
@@ -112,7 +112,10 @@ int __init acpi_parse_spcr(bool earlycon)
 	if (earlycon)
 		setup_earlycon(opts);
 
-	err = add_preferred_console(uart, 0, opts + strlen(uart) + 1);
+	if (enable_console)
+		err = add_preferred_console(uart, 0, opts + strlen(uart) + 1);
+	else
+		err = 0;
 done:
 	acpi_put_table((struct acpi_table_header *)table);
 	return err;
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index 875d7327d91c..4c2d449bab9b 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -1248,10 +1248,13 @@ extern bool console_acpi_spcr_enable __initdata;
 extern int acpi_arch_setup_console(struct acpi_table_spcr *table,
 				   char *opts, char *uart, char *iotype,
 				   int baud_rate, bool earlycon);
-int acpi_parse_spcr(bool earlycon);
+int acpi_parse_spcr(bool earlycon, bool enable_console);
 #else
 static const bool console_acpi_spcr_enable;
-static inline int acpi_parse_spcr(bool earlycon) { return 0; }
+static inline int acpi_parse_spcr(bool earlycon, bool enable_console)
+{
+	return 0;
+}
 #endif
 
 #if IS_ENABLED(CONFIG_ACPI_GENERIC_GSI)
-- 
2.15.0.rc0.39.g2f0e14e64

[PATCH v2 2/2] acpi, x86: Use SPCR table for earlycon on x86

[Prarit Bhargava]

The ACPI SPCR code has been used to define an earlycon console for ARM64
and can be used for x86.

Modify the ACPI SPCR parsing code to account for console behaviour
differences between ARM64 and x86.  Initialize the SPCR code from
x86 ACPI initialization code.

Signed-off-by: Prarit Bhargava <prarit@redhat.com>
Cc: linux-doc@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-pm@vger.kernel.org
Cc: linux-acpi@vger.kernel.org
Cc: linux-serial@vger.kernel.org
Cc: Bhupesh Sharma <bhsharma@redhat.com>
Cc: Lv Zheng <lv.zheng@intel.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: x86@kernel.org
Cc: Jonathan Corbet <corbet@lwn.net>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
Cc: Timur Tabi <timur@codeaurora.org>
---
 Documentation/admin-guide/kernel-parameters.txt | 3 +++
 arch/arm64/kernel/acpi.c                        | 2 +-
 arch/x86/kernel/acpi/boot.c                     | 4 ++++
 drivers/acpi/Kconfig                            | 2 +-
 drivers/acpi/spcr.c                             | 7 +++++--
 include/linux/acpi.h                            | 7 +++++--
 6 files changed, 19 insertions(+), 6 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 0d173289c67e..c7cc890a0e81 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -918,6 +918,9 @@
 			stdout-path property in device tree's chosen node,
 			or determined by the ACPI SPCR table.
 
+			[X86] When used with no options the early console is
+			determined by the ACPI SPCR table.
+
 		cdns,<addr>[,options]
 			Start an early, polled-mode console on a Cadence
 			(xuartps) serial port at the specified address. Only
diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c
index b3e33bbdf3b7..aaee4864b63e 100644
--- a/arch/arm64/kernel/acpi.c
+++ b/arch/arm64/kernel/acpi.c
@@ -355,7 +355,7 @@ void __init acpi_boot_table_init(void)
 			early_init_dt_scan_chosen_stdout();
 	} else {
 		/* Always enable the ACPI SPCR console */
-		acpi_parse_spcr(console_acpi_spcr_enable);
+		acpi_parse_spcr(console_acpi_spcr_enable, true);
 		if (IS_ENABLED(CONFIG_ACPI_BGRT))
 			acpi_table_parse(ACPI_SIG_BGRT, acpi_parse_bgrt);
 	}
diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c
index f4c463df8b08..f01a03643cff 100644
--- a/arch/x86/kernel/acpi/boot.c
+++ b/arch/x86/kernel/acpi/boot.c
@@ -36,6 +36,7 @@
 #include <linux/ioport.h>
 #include <linux/pci.h>
 #include <linux/efi-bgrt.h>
+#include <linux/serial_core.h>
 
 #include <asm/e820/api.h>
 #include <asm/irqdomain.h>
@@ -1626,6 +1627,9 @@ int __init acpi_boot_init(void)
 	if (!acpi_noirq)
 		x86_init.pci.init = pci_acpi_init;
 
+	/* Do not enable ACPI SPCR console by default */
+	acpi_parse_spcr(console_acpi_spcr_enable, console_acpi_spcr_enable);
+
 	return 0;
 }
 
diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
index 9ae98eeada76..2d4e841e0682 100644
--- a/drivers/acpi/Kconfig
+++ b/drivers/acpi/Kconfig
@@ -80,7 +80,7 @@ endif
 
 config ACPI_SPCR_TABLE
 	bool "ACPI Serial Port Console Redirection Support"
-	default y if ARM64
+	default y if (X86 || ARM64)
 	help
 	  Enable support for Serial Port Console Redirection (SPCR) Table.
 	  This table provides information about the configuration of the
diff --git a/drivers/acpi/spcr.c b/drivers/acpi/spcr.c
index f4bb8110e404..c9469b488527 100644
--- a/drivers/acpi/spcr.c
+++ b/drivers/acpi/spcr.c
@@ -39,7 +39,7 @@ bool console_acpi_spcr_enable __initdata;
  * from arch initialization code as soon as the DT/ACPI decision is made.
  *
  */
-int __init acpi_parse_spcr(bool earlycon)
+int __init acpi_parse_spcr(bool earlycon, bool enable_console)
 {
 	static char opts[ACPI_SPCR_OPTS_SIZE];
 	static char uart[ACPI_SPCR_BUF_SIZE];
@@ -112,7 +112,10 @@ int __init acpi_parse_spcr(bool earlycon)
 	if (earlycon)
 		setup_earlycon(opts);
 
-	err = add_preferred_console(uart, 0, opts + strlen(uart) + 1);
+	if (enable_console)
+		err = add_preferred_console(uart, 0, opts + strlen(uart) + 1);
+	else
+		err = 0;
 done:
 	acpi_put_table((struct acpi_table_header *)table);
 	return err;
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index 875d7327d91c..4c2d449bab9b 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -1248,10 +1248,13 @@ extern bool console_acpi_spcr_enable __initdata;
 extern int acpi_arch_setup_console(struct acpi_table_spcr *table,
 				   char *opts, char *uart, char *iotype,
 				   int baud_rate, bool earlycon);
-int acpi_parse_spcr(bool earlycon);
+int acpi_parse_spcr(bool earlycon, bool enable_console);
 #else
 static const bool console_acpi_spcr_enable;
-static inline int acpi_parse_spcr(bool earlycon) { return 0; }
+static inline int acpi_parse_spcr(bool earlycon, bool enable_console)
+{
+	return 0;
+}
 #endif
 
 #if IS_ENABLED(CONFIG_ACPI_GENERIC_GSI)
-- 
2.15.0.rc0.39.g2f0e14e64

[PATCH v2 2/2] acpi, x86: Use SPCR table for earlycon on x86

[Prarit Bhargava]

The ACPI SPCR code has been used to define an earlycon console for ARM64
and can be used for x86.

Modify the ACPI SPCR parsing code to account for console behaviour
differences between ARM64 and x86.  Initialize the SPCR code from
x86 ACPI initialization code.

Signed-off-by: Prarit Bhargava <prarit@redhat.com>
Cc: linux-doc at vger.kernel.org
Cc: linux-kernel at vger.kernel.org
Cc: linux-arm-kernel at lists.infradead.org
Cc: linux-pm at vger.kernel.org
Cc: linux-acpi at vger.kernel.org
Cc: linux-serial at vger.kernel.org
Cc: Bhupesh Sharma <bhsharma@redhat.com>
Cc: Lv Zheng <lv.zheng@intel.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: x86 at kernel.org
Cc: Jonathan Corbet <corbet@lwn.net>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
Cc: Timur Tabi <timur@codeaurora.org>
---
 Documentation/admin-guide/kernel-parameters.txt | 3 +++
 arch/arm64/kernel/acpi.c                        | 2 +-
 arch/x86/kernel/acpi/boot.c                     | 4 ++++
 drivers/acpi/Kconfig                            | 2 +-
 drivers/acpi/spcr.c                             | 7 +++++--
 include/linux/acpi.h                            | 7 +++++--
 6 files changed, 19 insertions(+), 6 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 0d173289c67e..c7cc890a0e81 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -918,6 +918,9 @@
 			stdout-path property in device tree's chosen node,
 			or determined by the ACPI SPCR table.
 
+			[X86] When used with no options the early console is
+			determined by the ACPI SPCR table.
+
 		cdns,<addr>[,options]
 			Start an early, polled-mode console on a Cadence
 			(xuartps) serial port at the specified address. Only
diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c
index b3e33bbdf3b7..aaee4864b63e 100644
--- a/arch/arm64/kernel/acpi.c
+++ b/arch/arm64/kernel/acpi.c
@@ -355,7 +355,7 @@ void __init acpi_boot_table_init(void)
 			early_init_dt_scan_chosen_stdout();
 	} else {
 		/* Always enable the ACPI SPCR console */
-		acpi_parse_spcr(console_acpi_spcr_enable);
+		acpi_parse_spcr(console_acpi_spcr_enable, true);
 		if (IS_ENABLED(CONFIG_ACPI_BGRT))
 			acpi_table_parse(ACPI_SIG_BGRT, acpi_parse_bgrt);
 	}
diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c
index f4c463df8b08..f01a03643cff 100644
--- a/arch/x86/kernel/acpi/boot.c
+++ b/arch/x86/kernel/acpi/boot.c
@@ -36,6 +36,7 @@
 #include <linux/ioport.h>
 #include <linux/pci.h>
 #include <linux/efi-bgrt.h>
+#include <linux/serial_core.h>
 
 #include <asm/e820/api.h>
 #include <asm/irqdomain.h>
@@ -1626,6 +1627,9 @@ int __init acpi_boot_init(void)
 	if (!acpi_noirq)
 		x86_init.pci.init = pci_acpi_init;
 
+	/* Do not enable ACPI SPCR console by default */
+	acpi_parse_spcr(console_acpi_spcr_enable, console_acpi_spcr_enable);
+
 	return 0;
 }
 
diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
index 9ae98eeada76..2d4e841e0682 100644
--- a/drivers/acpi/Kconfig
+++ b/drivers/acpi/Kconfig
@@ -80,7 +80,7 @@ endif
 
 config ACPI_SPCR_TABLE
 	bool "ACPI Serial Port Console Redirection Support"
-	default y if ARM64
+	default y if (X86 || ARM64)
 	help
 	  Enable support for Serial Port Console Redirection (SPCR) Table.
 	  This table provides information about the configuration of the
diff --git a/drivers/acpi/spcr.c b/drivers/acpi/spcr.c
index f4bb8110e404..c9469b488527 100644
--- a/drivers/acpi/spcr.c
+++ b/drivers/acpi/spcr.c
@@ -39,7 +39,7 @@ bool console_acpi_spcr_enable __initdata;
  * from arch initialization code as soon as the DT/ACPI decision is made.
  *
  */
-int __init acpi_parse_spcr(bool earlycon)
+int __init acpi_parse_spcr(bool earlycon, bool enable_console)
 {
 	static char opts[ACPI_SPCR_OPTS_SIZE];
 	static char uart[ACPI_SPCR_BUF_SIZE];
@@ -112,7 +112,10 @@ int __init acpi_parse_spcr(bool earlycon)
 	if (earlycon)
 		setup_earlycon(opts);
 
-	err = add_preferred_console(uart, 0, opts + strlen(uart) + 1);
+	if (enable_console)
+		err = add_preferred_console(uart, 0, opts + strlen(uart) + 1);
+	else
+		err = 0;
 done:
 	acpi_put_table((struct acpi_table_header *)table);
 	return err;
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index 875d7327d91c..4c2d449bab9b 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -1248,10 +1248,13 @@ extern bool console_acpi_spcr_enable __initdata;
 extern int acpi_arch_setup_console(struct acpi_table_spcr *table,
 				   char *opts, char *uart, char *iotype,
 				   int baud_rate, bool earlycon);
-int acpi_parse_spcr(bool earlycon);
+int acpi_parse_spcr(bool earlycon, bool enable_console);
 #else
 static const bool console_acpi_spcr_enable;
-static inline int acpi_parse_spcr(bool earlycon) { return 0; }
+static inline int acpi_parse_spcr(bool earlycon, bool enable_console)
+{
+	return 0;
+}
 #endif
 
 #if IS_ENABLED(CONFIG_ACPI_GENERIC_GSI)
-- 
2.15.0.rc0.39.g2f0e14e64

Re: [PATCH v2 2/2] acpi, x86: Use SPCR table for earlycon on x86

[Ingo Molnar]

* Prarit Bhargava <prarit@redhat.com> wrote:

> The ACPI SPCR code has been used to define an earlycon console for ARM64
> and can be used for x86.
> 
> Modify the ACPI SPCR parsing code to account for console behaviour
> differences between ARM64 and x86.  Initialize the SPCR code from
> x86 ACPI initialization code.
> 
> Signed-off-by: Prarit Bhargava <prarit@redhat.com>
> Cc: linux-doc@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-pm@vger.kernel.org
> Cc: linux-acpi@vger.kernel.org
> Cc: linux-serial@vger.kernel.org
> Cc: Bhupesh Sharma <bhsharma@redhat.com>
> Cc: Lv Zheng <lv.zheng@intel.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: "H. Peter Anvin" <hpa@zytor.com>
> Cc: x86@kernel.org
> Cc: Jonathan Corbet <corbet@lwn.net>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> Cc: Timur Tabi <timur@codeaurora.org>
> ---
>  Documentation/admin-guide/kernel-parameters.txt | 3 +++
>  arch/arm64/kernel/acpi.c                        | 2 +-
>  arch/x86/kernel/acpi/boot.c                     | 4 ++++
>  drivers/acpi/Kconfig                            | 2 +-
>  drivers/acpi/spcr.c                             | 7 +++++--
>  include/linux/acpi.h                            | 7 +++++--
>  6 files changed, 19 insertions(+), 6 deletions(-)

LGTM from an x86 perspective:

  Acked-by: Ingo Molnar <mingo@kernel.org>

(assuming it causes no regressions in linux-next.)

Since patch #1 affects ARM64 significantly, once that patch is acceptable to the 
arm64 folks feel free to upstream this second patch via the ARM64 tree as well.

Thanks,

	Ingo

[PATCH v2 2/2] acpi, x86: Use SPCR table for earlycon on x86

[Ingo Molnar]

* Prarit Bhargava <prarit@redhat.com> wrote:

> The ACPI SPCR code has been used to define an earlycon console for ARM64
> and can be used for x86.
> 
> Modify the ACPI SPCR parsing code to account for console behaviour
> differences between ARM64 and x86.  Initialize the SPCR code from
> x86 ACPI initialization code.
> 
> Signed-off-by: Prarit Bhargava <prarit@redhat.com>
> Cc: linux-doc at vger.kernel.org
> Cc: linux-kernel at vger.kernel.org
> Cc: linux-arm-kernel at lists.infradead.org
> Cc: linux-pm at vger.kernel.org
> Cc: linux-acpi at vger.kernel.org
> Cc: linux-serial at vger.kernel.org
> Cc: Bhupesh Sharma <bhsharma@redhat.com>
> Cc: Lv Zheng <lv.zheng@intel.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: "H. Peter Anvin" <hpa@zytor.com>
> Cc: x86 at kernel.org
> Cc: Jonathan Corbet <corbet@lwn.net>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> Cc: Timur Tabi <timur@codeaurora.org>
> ---
>  Documentation/admin-guide/kernel-parameters.txt | 3 +++
>  arch/arm64/kernel/acpi.c                        | 2 +-
>  arch/x86/kernel/acpi/boot.c                     | 4 ++++
>  drivers/acpi/Kconfig                            | 2 +-
>  drivers/acpi/spcr.c                             | 7 +++++--
>  include/linux/acpi.h                            | 7 +++++--
>  6 files changed, 19 insertions(+), 6 deletions(-)

LGTM from an x86 perspective:

  Acked-by: Ingo Molnar <mingo@kernel.org>

(assuming it causes no regressions in linux-next.)

Since patch #1 affects ARM64 significantly, once that patch is acceptable to the 
arm64 folks feel free to upstream this second patch via the ARM64 tree as well.

Thanks,

	Ingo

Re: [PATCH v2 1/2] acpi, spcr: Make SPCR avialable to other architectures

[Rafael J. Wysocki]

On Monday, December 11, 2017 4:50:58 PM CET Prarit Bhargava wrote:
> Other architectures can use SPCR to setup an early console or console
> but the current code is ARM64 specific.
> 
> Change the name of parse_spcr() to acpi_parse_spcr().  Add a weak
> function acpi_arch_setup_console() that can be used for arch-specific
> setup.  Move flags into ACPI code.  Update the Documention on the use of
> the SPCR.
> 
> [v2]: Don't return an error in the baud_rate check of acpi_parse_spcr().
> Keep ACPI_SPCR_TABLE selected for ARM64.  Fix 8-bit port access width
> mmio value.  Move baud rate check earlier.
> 
> Signed-off-by: Prarit Bhargava <prarit@redhat.com>

This mostly affects ARM64, so ACKs from that side are requisite for it.

> Cc: linux-doc@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-pm@vger.kernel.org
> Cc: linux-acpi@vger.kernel.org
> Cc: linux-serial@vger.kernel.org
> Cc: Bhupesh Sharma <bhsharma@redhat.com>
> Cc: Lv Zheng <lv.zheng@intel.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: "H. Peter Anvin" <hpa@zytor.com>
> Cc: x86@kernel.org
> Cc: Jonathan Corbet <corbet@lwn.net>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> Cc: Timur Tabi <timur@codeaurora.org>
> ---
>  Documentation/admin-guide/kernel-parameters.txt |   6 +-
>  arch/arm64/kernel/acpi.c                        | 128 ++++++++++++++++-
>  drivers/acpi/Kconfig                            |   7 +-
>  drivers/acpi/spcr.c                             | 175 ++++++------------------
>  drivers/tty/serial/earlycon.c                   |  15 +-
>  include/linux/acpi.h                            |  11 +-
>  include/linux/serial_core.h                     |   2 -
>  7 files changed, 184 insertions(+), 160 deletions(-)
> 
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index 6571fbfdb2a1..0d173289c67e 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -914,9 +914,9 @@
>  
>  	earlycon=	[KNL] Output early console device and options.
>  
> -			When used with no options, the early console is
> -			determined by the stdout-path property in device
> -			tree's chosen node.
> +			[ARM64] The early console is determined by the
> +			stdout-path property in device tree's chosen node,
> +			or determined by the ACPI SPCR table.
>  
>  		cdns,<addr>[,options]
>  			Start an early, polled-mode console on a Cadence
> diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c
> index b3162715ed78..b3e33bbdf3b7 100644
> --- a/arch/arm64/kernel/acpi.c
> +++ b/arch/arm64/kernel/acpi.c
> @@ -25,7 +25,6 @@
>  #include <linux/memblock.h>
>  #include <linux/of_fdt.h>
>  #include <linux/smp.h>
> -#include <linux/serial_core.h>
>  
>  #include <asm/cputype.h>
>  #include <asm/cpu_ops.h>
> @@ -177,6 +176,128 @@ static int __init acpi_fadt_sanity_check(void)
>  	return ret;
>  }
>  
> +/*
> + * Erratum 44 for QDF2432v1 and QDF2400v1 SoCs describes the BUSY bit as
> + * occasionally getting stuck as 1. To avoid the potential for a hang, check
> + * TXFE == 0 instead of BUSY == 1. This may not be suitable for all UART
> + * implementations, so only do so if an affected platform is detected in
> + * acpi_parse_spcr().
> + */
> +bool qdf2400_e44_present;
> +EXPORT_SYMBOL(qdf2400_e44_present);
> +
> +/*
> + * Some Qualcomm Datacenter Technologies SoCs have a defective UART BUSY bit.
> + * Detect them by examining the OEM fields in the SPCR header, similar to PCI
> + * quirk detection in pci_mcfg.c.
> + */
> +static bool qdf2400_erratum_44_present(struct acpi_table_header *h)
> +{
> +	if (memcmp(h->oem_id, "QCOM  ", ACPI_OEM_ID_SIZE))
> +		return false;
> +
> +	if (!memcmp(h->oem_table_id, "QDF2432 ", ACPI_OEM_TABLE_ID_SIZE))
> +		return true;
> +
> +	if (!memcmp(h->oem_table_id, "QDF2400 ", ACPI_OEM_TABLE_ID_SIZE) &&
> +			h->oem_revision == 1)
> +		return true;
> +
> +	return false;
> +}
> +
> +/*
> + * APM X-Gene v1 and v2 UART hardware is an 16550 like device but has its
> + * register aligned to 32-bit. In addition, the BIOS also encoded the
> + * access width to be 8 bits. This function detects this errata condition.
> + */
> +static bool xgene_8250_erratum_present(struct acpi_table_spcr *tb)
> +{
> +	bool xgene_8250 = false;
> +
> +	if (tb->interface_type != ACPI_DBG2_16550_COMPATIBLE)
> +		return false;
> +
> +	if (memcmp(tb->header.oem_id, "APMC0D", ACPI_OEM_ID_SIZE) &&
> +	    memcmp(tb->header.oem_id, "HPE   ", ACPI_OEM_ID_SIZE))
> +		return false;
> +
> +	if (!memcmp(tb->header.oem_table_id, "XGENESPC",
> +	    ACPI_OEM_TABLE_ID_SIZE) && tb->header.oem_revision == 0)
> +		xgene_8250 = true;
> +
> +	if (!memcmp(tb->header.oem_table_id, "ProLiant",
> +	    ACPI_OEM_TABLE_ID_SIZE) && tb->header.oem_revision == 1)
> +		xgene_8250 = true;
> +
> +	return xgene_8250;
> +}
> +
> +int acpi_arch_setup_console(struct acpi_table_spcr *table,
> +			    char *opts, char *uart, char *iotype,
> +			    int baud_rate, bool earlycon)
> +{
> +	if (table->header.revision < 2) {
> +		pr_err("wrong table version\n");
> +		return -ENOENT;
> +	}
> +
> +	switch (table->interface_type) {
> +	case ACPI_DBG2_ARM_SBSA_32BIT:
> +		snprintf(iotype, ACPI_SPCR_BUF_SIZE, "mmio32");
> +		/* fall through */
> +	case ACPI_DBG2_ARM_PL011:
> +	case ACPI_DBG2_ARM_SBSA_GENERIC:
> +	case ACPI_DBG2_BCM2835:
> +		snprintf(uart, ACPI_SPCR_BUF_SIZE, "pl011");
> +		break;
> +	default:
> +		if (strlen(uart) == 0)
> +			return -ENOENT;
> +	}
> +
> +	/*
> +	 * If the E44 erratum is required, then we need to tell the pl011
> +	 * driver to implement the work-around.
> +	 *
> +	 * The global variable is used by the probe function when it
> +	 * creates the UARTs, whether or not they're used as a console.
> +	 *
> +	 * If the user specifies "traditional" earlycon, the qdf2400_e44
> +	 * console name matches the EARLYCON_DECLARE() statement, and
> +	 * SPCR is not used.  Parameter "earlycon" is false.
> +	 *
> +	 * If the user specifies "SPCR" earlycon, then we need to update
> +	 * the console name so that it also says "qdf2400_e44".  Parameter
> +	 * "earlycon" is true.
> +	 *
> +	 * For consistency, if we change the console name, then we do it
> +	 * for everyone, not just earlycon.
> +	 */
> +	if (qdf2400_erratum_44_present(&table->header)) {
> +		qdf2400_e44_present = true;
> +		if (earlycon)
> +			snprintf(uart, ACPI_SPCR_BUF_SIZE, "qdf2400_e44");
> +	}
> +
> +	if (xgene_8250_erratum_present(table)) {
> +		snprintf(iotype, ACPI_SPCR_BUF_SIZE, "mmio32");
> +
> +		/* for xgene v1 and v2 we don't know the clock rate of the
> +		 * UART so don't attempt to change to the baud rate state
> +		 * in the table because driver cannot calculate the dividers
> +		 */
> +		snprintf(opts, ACPI_SPCR_OPTS_SIZE, "%s,%s,0x%llx", uart,
> +			 iotype, table->serial_port.address);
> +	} else {
> +		snprintf(opts, ACPI_SPCR_OPTS_SIZE, "%s,%s,0x%llx,%d", uart,
> +			 iotype, table->serial_port.address, baud_rate);
> +	}
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(acpi_arch_setup_console);
> +
>  /*
>   * acpi_boot_table_init() called from setup_arch(), always.
>   *	1. find RSDP and get its address, and then find XSDT
> @@ -230,10 +351,11 @@ void __init acpi_boot_table_init(void)
>  
>  done:
>  	if (acpi_disabled) {
> -		if (earlycon_init_is_deferred)
> +		if (console_acpi_spcr_enable)
>  			early_init_dt_scan_chosen_stdout();
>  	} else {
> -		parse_spcr(earlycon_init_is_deferred);
> +		/* Always enable the ACPI SPCR console */
> +		acpi_parse_spcr(console_acpi_spcr_enable);
>  		if (IS_ENABLED(CONFIG_ACPI_BGRT))
>  			acpi_table_parse(ACPI_SIG_BGRT, acpi_parse_bgrt);
>  	}
> diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
> index 46505396869e..9ae98eeada76 100644
> --- a/drivers/acpi/Kconfig
> +++ b/drivers/acpi/Kconfig
> @@ -79,7 +79,12 @@ config ACPI_DEBUGGER_USER
>  endif
>  
>  config ACPI_SPCR_TABLE
> -	bool
> +	bool "ACPI Serial Port Console Redirection Support"
> +	default y if ARM64
> +	help
> +	  Enable support for Serial Port Console Redirection (SPCR) Table.
> +	  This table provides information about the configuration of the
> +	  earlycon console.
>  
>  config ACPI_LPIT
>  	bool
> diff --git a/drivers/acpi/spcr.c b/drivers/acpi/spcr.c
> index 324b35bfe781..f4bb8110e404 100644
> --- a/drivers/acpi/spcr.c
> +++ b/drivers/acpi/spcr.c
> @@ -16,65 +16,18 @@
>  #include <linux/kernel.h>
>  #include <linux/serial_core.h>
>  
> -/*
> - * Erratum 44 for QDF2432v1 and QDF2400v1 SoCs describes the BUSY bit as
> - * occasionally getting stuck as 1. To avoid the potential for a hang, check
> - * TXFE == 0 instead of BUSY == 1. This may not be suitable for all UART
> - * implementations, so only do so if an affected platform is detected in
> - * parse_spcr().
> - */
> -bool qdf2400_e44_present;
> -EXPORT_SYMBOL(qdf2400_e44_present);
> -
> -/*
> - * Some Qualcomm Datacenter Technologies SoCs have a defective UART BUSY bit.
> - * Detect them by examining the OEM fields in the SPCR header, similiar to PCI
> - * quirk detection in pci_mcfg.c.
> - */
> -static bool qdf2400_erratum_44_present(struct acpi_table_header *h)
> -{
> -	if (memcmp(h->oem_id, "QCOM  ", ACPI_OEM_ID_SIZE))
> -		return false;
> -
> -	if (!memcmp(h->oem_table_id, "QDF2432 ", ACPI_OEM_TABLE_ID_SIZE))
> -		return true;
> -
> -	if (!memcmp(h->oem_table_id, "QDF2400 ", ACPI_OEM_TABLE_ID_SIZE) &&
> -			h->oem_revision == 1)
> -		return true;
> -
> -	return false;
> -}
> -
> -/*
> - * APM X-Gene v1 and v2 UART hardware is an 16550 like device but has its
> - * register aligned to 32-bit. In addition, the BIOS also encoded the
> - * access width to be 8 bits. This function detects this errata condition.
> - */
> -static bool xgene_8250_erratum_present(struct acpi_table_spcr *tb)
> +int __weak acpi_arch_setup_console(struct acpi_table_spcr *table,
> +				   char *opts, char *uart, char *iotype,
> +				   int baud_rate, bool earlycon)
>  {
> -	bool xgene_8250 = false;
> -
> -	if (tb->interface_type != ACPI_DBG2_16550_COMPATIBLE)
> -		return false;
> -
> -	if (memcmp(tb->header.oem_id, "APMC0D", ACPI_OEM_ID_SIZE) &&
> -	    memcmp(tb->header.oem_id, "HPE   ", ACPI_OEM_ID_SIZE))
> -		return false;
> -
> -	if (!memcmp(tb->header.oem_table_id, "XGENESPC",
> -	    ACPI_OEM_TABLE_ID_SIZE) && tb->header.oem_revision == 0)
> -		xgene_8250 = true;
> -
> -	if (!memcmp(tb->header.oem_table_id, "ProLiant",
> -	    ACPI_OEM_TABLE_ID_SIZE) && tb->header.oem_revision == 1)
> -		xgene_8250 = true;
> -
> -	return xgene_8250;
> +	snprintf(opts, ACPI_SPCR_OPTS_SIZE, "%s,%s,0x%llx,%d", uart, iotype,
> +		 table->serial_port.address, baud_rate);
> +	return 0;
>  }
>  
> +bool console_acpi_spcr_enable __initdata;
>  /**
> - * parse_spcr() - parse ACPI SPCR table and add preferred console
> + * acpi_parse_spcr() - parse ACPI SPCR table and add preferred console
>   *
>   * @earlycon: set up earlycon for the console specified by the table
>   *
> @@ -86,13 +39,13 @@ static bool xgene_8250_erratum_present(struct acpi_table_spcr *tb)
>   * from arch initialization code as soon as the DT/ACPI decision is made.
>   *
>   */
> -int __init parse_spcr(bool earlycon)
> +int __init acpi_parse_spcr(bool earlycon)
>  {
> -	static char opts[64];
> +	static char opts[ACPI_SPCR_OPTS_SIZE];
> +	static char uart[ACPI_SPCR_BUF_SIZE];
> +	static char iotype[ACPI_SPCR_BUF_SIZE];
>  	struct acpi_table_spcr *table;
>  	acpi_status status;
> -	char *uart;
> -	char *iotype;
>  	int baud_rate;
>  	int err;
>  
> @@ -105,48 +58,6 @@ int __init parse_spcr(bool earlycon)
>  	if (ACPI_FAILURE(status))
>  		return -ENOENT;
>  
> -	if (table->header.revision < 2) {
> -		err = -ENOENT;
> -		pr_err("wrong table version\n");
> -		goto done;
> -	}
> -
> -	if (table->serial_port.space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY) {
> -		switch (ACPI_ACCESS_BIT_WIDTH((
> -			table->serial_port.access_width))) {
> -		default:
> -			pr_err("Unexpected SPCR Access Width.  Defaulting to byte size\n");
> -		case 8:
> -			iotype = "mmio";
> -			break;
> -		case 16:
> -			iotype = "mmio16";
> -			break;
> -		case 32:
> -			iotype = "mmio32";
> -			break;
> -		}
> -	} else
> -		iotype = "io";
> -
> -	switch (table->interface_type) {
> -	case ACPI_DBG2_ARM_SBSA_32BIT:
> -		iotype = "mmio32";
> -		/* fall through */
> -	case ACPI_DBG2_ARM_PL011:
> -	case ACPI_DBG2_ARM_SBSA_GENERIC:
> -	case ACPI_DBG2_BCM2835:
> -		uart = "pl011";
> -		break;
> -	case ACPI_DBG2_16550_COMPATIBLE:
> -	case ACPI_DBG2_16550_SUBSET:
> -		uart = "uart";
> -		break;
> -	default:
> -		err = -ENOENT;
> -		goto done;
> -	}
> -
>  	switch (table->baud_rate) {
>  	case 3:
>  		baud_rate = 9600;
> @@ -165,43 +76,36 @@ int __init parse_spcr(bool earlycon)
>  		goto done;
>  	}
>  
> -	/*
> -	 * If the E44 erratum is required, then we need to tell the pl011
> -	 * driver to implement the work-around.
> -	 *
> -	 * The global variable is used by the probe function when it
> -	 * creates the UARTs, whether or not they're used as a console.
> -	 *
> -	 * If the user specifies "traditional" earlycon, the qdf2400_e44
> -	 * console name matches the EARLYCON_DECLARE() statement, and
> -	 * SPCR is not used.  Parameter "earlycon" is false.
> -	 *
> -	 * If the user specifies "SPCR" earlycon, then we need to update
> -	 * the console name so that it also says "qdf2400_e44".  Parameter
> -	 * "earlycon" is true.
> -	 *
> -	 * For consistency, if we change the console name, then we do it
> -	 * for everyone, not just earlycon.
> -	 */
> -	if (qdf2400_erratum_44_present(&table->header)) {
> -		qdf2400_e44_present = true;
> -		if (earlycon)
> -			uart = "qdf2400_e44";
> +	switch (table->interface_type) {
> +	case ACPI_DBG2_16550_COMPATIBLE:
> +	case ACPI_DBG2_16550_SUBSET:
> +		snprintf(uart, ACPI_SPCR_BUF_SIZE, "uart");
> +		break;
> +	default:
> +		break;
>  	}
>  
> -	if (xgene_8250_erratum_present(table)) {
> -		iotype = "mmio32";
> +	if (table->serial_port.space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY) {
> +		u8 width = ACPI_ACCESS_BIT_WIDTH((
> +					table->serial_port.access_width));
> +		switch (width) {
> +		default:
> +			pr_err("Unexpected SPCR Access Width.  Defaulting to byte size\n");
> +		case 8:
> +			snprintf(iotype, ACPI_SPCR_BUF_SIZE, "mmio");
> +			break;
> +		case 16:
> +		case 32:
> +			snprintf(iotype, ACPI_SPCR_BUF_SIZE, "mmio%d", width);
> +			break;
> +		}
> +	} else
> +		snprintf(iotype, ACPI_SPCR_BUF_SIZE, "io");
>  
> -		/* for xgene v1 and v2 we don't know the clock rate of the
> -		 * UART so don't attempt to change to the baud rate state
> -		 * in the table because driver cannot calculate the dividers
> -		 */
> -		snprintf(opts, sizeof(opts), "%s,%s,0x%llx", uart, iotype,
> -			 table->serial_port.address);
> -	} else {
> -		snprintf(opts, sizeof(opts), "%s,%s,0x%llx,%d", uart, iotype,
> -			 table->serial_port.address, baud_rate);
> -	}
> +	err = acpi_arch_setup_console(table, opts, uart, iotype, baud_rate,
> +				      earlycon);
> +	if (err)
> +		goto done;
>  
>  	pr_info("console: %s\n", opts);
>  
> @@ -209,7 +113,6 @@ int __init parse_spcr(bool earlycon)
>  		setup_earlycon(opts);
>  
>  	err = add_preferred_console(uart, 0, opts + strlen(uart) + 1);
> -
>  done:
>  	acpi_put_table((struct acpi_table_header *)table);
>  	return err;
> diff --git a/drivers/tty/serial/earlycon.c b/drivers/tty/serial/earlycon.c
> index 4c8b80f1c688..b22afb62c7a3 100644
> --- a/drivers/tty/serial/earlycon.c
> +++ b/drivers/tty/serial/earlycon.c
> @@ -196,26 +196,15 @@ int __init setup_earlycon(char *buf)
>  	return -ENOENT;
>  }
>  
> -/*
> - * When CONFIG_ACPI_SPCR_TABLE is defined, "earlycon" without parameters in
> - * command line does not start DT earlycon immediately, instead it defers
> - * starting it until DT/ACPI decision is made.  At that time if ACPI is enabled
> - * call parse_spcr(), else call early_init_dt_scan_chosen_stdout()
> - */
> -bool earlycon_init_is_deferred __initdata;
> -
>  /* early_param wrapper for setup_earlycon() */
>  static int __init param_setup_earlycon(char *buf)
>  {
>  	int err;
>  
> -	/*
> -	 * Just 'earlycon' is a valid param for devicetree earlycons;
> -	 * don't generate a warning from parse_early_params() in that case
> -	 */
> +	/* Just 'earlycon' is a valid param for devicetree and ACPI SPCR. */
>  	if (!buf || !buf[0]) {
>  		if (IS_ENABLED(CONFIG_ACPI_SPCR_TABLE)) {
> -			earlycon_init_is_deferred = true;
> +			console_acpi_spcr_enable = true;
>  			return 0;
>  		} else if (!buf) {
>  			return early_init_dt_scan_chosen_stdout();
> diff --git a/include/linux/acpi.h b/include/linux/acpi.h
> index dc1ebfeeb5ec..875d7327d91c 100644
> --- a/include/linux/acpi.h
> +++ b/include/linux/acpi.h
> @@ -1241,10 +1241,17 @@ static inline bool acpi_has_watchdog(void) { return false; }
>  #endif
>  
>  #ifdef CONFIG_ACPI_SPCR_TABLE
> +#define ACPI_SPCR_OPTS_SIZE 64
> +#define ACPI_SPCR_BUF_SIZE 32
>  extern bool qdf2400_e44_present;
> -int parse_spcr(bool earlycon);
> +extern bool console_acpi_spcr_enable __initdata;
> +extern int acpi_arch_setup_console(struct acpi_table_spcr *table,
> +				   char *opts, char *uart, char *iotype,
> +				   int baud_rate, bool earlycon);
> +int acpi_parse_spcr(bool earlycon);
>  #else
> -static inline int parse_spcr(bool earlycon) { return 0; }
> +static const bool console_acpi_spcr_enable;
> +static inline int acpi_parse_spcr(bool earlycon) { return 0; }
>  #endif
>  
>  #if IS_ENABLED(CONFIG_ACPI_GENERIC_GSI)
> diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
> index 37b044e78333..abfffb4b1c37 100644
> --- a/include/linux/serial_core.h
> +++ b/include/linux/serial_core.h
> @@ -376,10 +376,8 @@ extern int of_setup_earlycon(const struct earlycon_id *match,
>  			     const char *options);
>  
>  #ifdef CONFIG_SERIAL_EARLYCON
> -extern bool earlycon_init_is_deferred __initdata;
>  int setup_earlycon(char *buf);
>  #else
> -static const bool earlycon_init_is_deferred;
>  static inline int setup_earlycon(char *buf) { return 0; }
>  #endif
>  
> 

[PATCH v2 1/2] acpi, spcr: Make SPCR avialable to other architectures

[Rafael J. Wysocki]

On Monday, December 11, 2017 4:50:58 PM CET Prarit Bhargava wrote:
> Other architectures can use SPCR to setup an early console or console
> but the current code is ARM64 specific.
> 
> Change the name of parse_spcr() to acpi_parse_spcr().  Add a weak
> function acpi_arch_setup_console() that can be used for arch-specific
> setup.  Move flags into ACPI code.  Update the Documention on the use of
> the SPCR.
> 
> [v2]: Don't return an error in the baud_rate check of acpi_parse_spcr().
> Keep ACPI_SPCR_TABLE selected for ARM64.  Fix 8-bit port access width
> mmio value.  Move baud rate check earlier.
> 
> Signed-off-by: Prarit Bhargava <prarit@redhat.com>

This mostly affects ARM64, so ACKs from that side are requisite for it.

> Cc: linux-doc at vger.kernel.org
> Cc: linux-kernel at vger.kernel.org
> Cc: linux-arm-kernel at lists.infradead.org
> Cc: linux-pm at vger.kernel.org
> Cc: linux-acpi at vger.kernel.org
> Cc: linux-serial at vger.kernel.org
> Cc: Bhupesh Sharma <bhsharma@redhat.com>
> Cc: Lv Zheng <lv.zheng@intel.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: "H. Peter Anvin" <hpa@zytor.com>
> Cc: x86 at kernel.org
> Cc: Jonathan Corbet <corbet@lwn.net>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> Cc: Timur Tabi <timur@codeaurora.org>
> ---
>  Documentation/admin-guide/kernel-parameters.txt |   6 +-
>  arch/arm64/kernel/acpi.c                        | 128 ++++++++++++++++-
>  drivers/acpi/Kconfig                            |   7 +-
>  drivers/acpi/spcr.c                             | 175 ++++++------------------
>  drivers/tty/serial/earlycon.c                   |  15 +-
>  include/linux/acpi.h                            |  11 +-
>  include/linux/serial_core.h                     |   2 -
>  7 files changed, 184 insertions(+), 160 deletions(-)
> 
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index 6571fbfdb2a1..0d173289c67e 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -914,9 +914,9 @@
>  
>  	earlycon=	[KNL] Output early console device and options.
>  
> -			When used with no options, the early console is
> -			determined by the stdout-path property in device
> -			tree's chosen node.
> +			[ARM64] The early console is determined by the
> +			stdout-path property in device tree's chosen node,
> +			or determined by the ACPI SPCR table.
>  
>  		cdns,<addr>[,options]
>  			Start an early, polled-mode console on a Cadence
> diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c
> index b3162715ed78..b3e33bbdf3b7 100644
> --- a/arch/arm64/kernel/acpi.c
> +++ b/arch/arm64/kernel/acpi.c
> @@ -25,7 +25,6 @@
>  #include <linux/memblock.h>
>  #include <linux/of_fdt.h>
>  #include <linux/smp.h>
> -#include <linux/serial_core.h>
>  
>  #include <asm/cputype.h>
>  #include <asm/cpu_ops.h>
> @@ -177,6 +176,128 @@ static int __init acpi_fadt_sanity_check(void)
>  	return ret;
>  }
>  
> +/*
> + * Erratum 44 for QDF2432v1 and QDF2400v1 SoCs describes the BUSY bit as
> + * occasionally getting stuck as 1. To avoid the potential for a hang, check
> + * TXFE == 0 instead of BUSY == 1. This may not be suitable for all UART
> + * implementations, so only do so if an affected platform is detected in
> + * acpi_parse_spcr().
> + */
> +bool qdf2400_e44_present;
> +EXPORT_SYMBOL(qdf2400_e44_present);
> +
> +/*
> + * Some Qualcomm Datacenter Technologies SoCs have a defective UART BUSY bit.
> + * Detect them by examining the OEM fields in the SPCR header, similar to PCI
> + * quirk detection in pci_mcfg.c.
> + */
> +static bool qdf2400_erratum_44_present(struct acpi_table_header *h)
> +{
> +	if (memcmp(h->oem_id, "QCOM  ", ACPI_OEM_ID_SIZE))
> +		return false;
> +
> +	if (!memcmp(h->oem_table_id, "QDF2432 ", ACPI_OEM_TABLE_ID_SIZE))
> +		return true;
> +
> +	if (!memcmp(h->oem_table_id, "QDF2400 ", ACPI_OEM_TABLE_ID_SIZE) &&
> +			h->oem_revision == 1)
> +		return true;
> +
> +	return false;
> +}
> +
> +/*
> + * APM X-Gene v1 and v2 UART hardware is an 16550 like device but has its
> + * register aligned to 32-bit. In addition, the BIOS also encoded the
> + * access width to be 8 bits. This function detects this errata condition.
> + */
> +static bool xgene_8250_erratum_present(struct acpi_table_spcr *tb)
> +{
> +	bool xgene_8250 = false;
> +
> +	if (tb->interface_type != ACPI_DBG2_16550_COMPATIBLE)
> +		return false;
> +
> +	if (memcmp(tb->header.oem_id, "APMC0D", ACPI_OEM_ID_SIZE) &&
> +	    memcmp(tb->header.oem_id, "HPE   ", ACPI_OEM_ID_SIZE))
> +		return false;
> +
> +	if (!memcmp(tb->header.oem_table_id, "XGENESPC",
> +	    ACPI_OEM_TABLE_ID_SIZE) && tb->header.oem_revision == 0)
> +		xgene_8250 = true;
> +
> +	if (!memcmp(tb->header.oem_table_id, "ProLiant",
> +	    ACPI_OEM_TABLE_ID_SIZE) && tb->header.oem_revision == 1)
> +		xgene_8250 = true;
> +
> +	return xgene_8250;
> +}
> +
> +int acpi_arch_setup_console(struct acpi_table_spcr *table,
> +			    char *opts, char *uart, char *iotype,
> +			    int baud_rate, bool earlycon)
> +{
> +	if (table->header.revision < 2) {
> +		pr_err("wrong table version\n");
> +		return -ENOENT;
> +	}
> +
> +	switch (table->interface_type) {
> +	case ACPI_DBG2_ARM_SBSA_32BIT:
> +		snprintf(iotype, ACPI_SPCR_BUF_SIZE, "mmio32");
> +		/* fall through */
> +	case ACPI_DBG2_ARM_PL011:
> +	case ACPI_DBG2_ARM_SBSA_GENERIC:
> +	case ACPI_DBG2_BCM2835:
> +		snprintf(uart, ACPI_SPCR_BUF_SIZE, "pl011");
> +		break;
> +	default:
> +		if (strlen(uart) == 0)
> +			return -ENOENT;
> +	}
> +
> +	/*
> +	 * If the E44 erratum is required, then we need to tell the pl011
> +	 * driver to implement the work-around.
> +	 *
> +	 * The global variable is used by the probe function when it
> +	 * creates the UARTs, whether or not they're used as a console.
> +	 *
> +	 * If the user specifies "traditional" earlycon, the qdf2400_e44
> +	 * console name matches the EARLYCON_DECLARE() statement, and
> +	 * SPCR is not used.  Parameter "earlycon" is false.
> +	 *
> +	 * If the user specifies "SPCR" earlycon, then we need to update
> +	 * the console name so that it also says "qdf2400_e44".  Parameter
> +	 * "earlycon" is true.
> +	 *
> +	 * For consistency, if we change the console name, then we do it
> +	 * for everyone, not just earlycon.
> +	 */
> +	if (qdf2400_erratum_44_present(&table->header)) {
> +		qdf2400_e44_present = true;
> +		if (earlycon)
> +			snprintf(uart, ACPI_SPCR_BUF_SIZE, "qdf2400_e44");
> +	}
> +
> +	if (xgene_8250_erratum_present(table)) {
> +		snprintf(iotype, ACPI_SPCR_BUF_SIZE, "mmio32");
> +
> +		/* for xgene v1 and v2 we don't know the clock rate of the
> +		 * UART so don't attempt to change to the baud rate state
> +		 * in the table because driver cannot calculate the dividers
> +		 */
> +		snprintf(opts, ACPI_SPCR_OPTS_SIZE, "%s,%s,0x%llx", uart,
> +			 iotype, table->serial_port.address);
> +	} else {
> +		snprintf(opts, ACPI_SPCR_OPTS_SIZE, "%s,%s,0x%llx,%d", uart,
> +			 iotype, table->serial_port.address, baud_rate);
> +	}
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(acpi_arch_setup_console);
> +
>  /*
>   * acpi_boot_table_init() called from setup_arch(), always.
>   *	1. find RSDP and get its address, and then find XSDT
> @@ -230,10 +351,11 @@ void __init acpi_boot_table_init(void)
>  
>  done:
>  	if (acpi_disabled) {
> -		if (earlycon_init_is_deferred)
> +		if (console_acpi_spcr_enable)
>  			early_init_dt_scan_chosen_stdout();
>  	} else {
> -		parse_spcr(earlycon_init_is_deferred);
> +		/* Always enable the ACPI SPCR console */
> +		acpi_parse_spcr(console_acpi_spcr_enable);
>  		if (IS_ENABLED(CONFIG_ACPI_BGRT))
>  			acpi_table_parse(ACPI_SIG_BGRT, acpi_parse_bgrt);
>  	}
> diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
> index 46505396869e..9ae98eeada76 100644
> --- a/drivers/acpi/Kconfig
> +++ b/drivers/acpi/Kconfig
> @@ -79,7 +79,12 @@ config ACPI_DEBUGGER_USER
>  endif
>  
>  config ACPI_SPCR_TABLE
> -	bool
> +	bool "ACPI Serial Port Console Redirection Support"
> +	default y if ARM64
> +	help
> +	  Enable support for Serial Port Console Redirection (SPCR) Table.
> +	  This table provides information about the configuration of the
> +	  earlycon console.
>  
>  config ACPI_LPIT
>  	bool
> diff --git a/drivers/acpi/spcr.c b/drivers/acpi/spcr.c
> index 324b35bfe781..f4bb8110e404 100644
> --- a/drivers/acpi/spcr.c
> +++ b/drivers/acpi/spcr.c
> @@ -16,65 +16,18 @@
>  #include <linux/kernel.h>
>  #include <linux/serial_core.h>
>  
> -/*
> - * Erratum 44 for QDF2432v1 and QDF2400v1 SoCs describes the BUSY bit as
> - * occasionally getting stuck as 1. To avoid the potential for a hang, check
> - * TXFE == 0 instead of BUSY == 1. This may not be suitable for all UART
> - * implementations, so only do so if an affected platform is detected in
> - * parse_spcr().
> - */
> -bool qdf2400_e44_present;
> -EXPORT_SYMBOL(qdf2400_e44_present);
> -
> -/*
> - * Some Qualcomm Datacenter Technologies SoCs have a defective UART BUSY bit.
> - * Detect them by examining the OEM fields in the SPCR header, similiar to PCI
> - * quirk detection in pci_mcfg.c.
> - */
> -static bool qdf2400_erratum_44_present(struct acpi_table_header *h)
> -{
> -	if (memcmp(h->oem_id, "QCOM  ", ACPI_OEM_ID_SIZE))
> -		return false;
> -
> -	if (!memcmp(h->oem_table_id, "QDF2432 ", ACPI_OEM_TABLE_ID_SIZE))
> -		return true;
> -
> -	if (!memcmp(h->oem_table_id, "QDF2400 ", ACPI_OEM_TABLE_ID_SIZE) &&
> -			h->oem_revision == 1)
> -		return true;
> -
> -	return false;
> -}
> -
> -/*
> - * APM X-Gene v1 and v2 UART hardware is an 16550 like device but has its
> - * register aligned to 32-bit. In addition, the BIOS also encoded the
> - * access width to be 8 bits. This function detects this errata condition.
> - */
> -static bool xgene_8250_erratum_present(struct acpi_table_spcr *tb)
> +int __weak acpi_arch_setup_console(struct acpi_table_spcr *table,
> +				   char *opts, char *uart, char *iotype,
> +				   int baud_rate, bool earlycon)
>  {
> -	bool xgene_8250 = false;
> -
> -	if (tb->interface_type != ACPI_DBG2_16550_COMPATIBLE)
> -		return false;
> -
> -	if (memcmp(tb->header.oem_id, "APMC0D", ACPI_OEM_ID_SIZE) &&
> -	    memcmp(tb->header.oem_id, "HPE   ", ACPI_OEM_ID_SIZE))
> -		return false;
> -
> -	if (!memcmp(tb->header.oem_table_id, "XGENESPC",
> -	    ACPI_OEM_TABLE_ID_SIZE) && tb->header.oem_revision == 0)
> -		xgene_8250 = true;
> -
> -	if (!memcmp(tb->header.oem_table_id, "ProLiant",
> -	    ACPI_OEM_TABLE_ID_SIZE) && tb->header.oem_revision == 1)
> -		xgene_8250 = true;
> -
> -	return xgene_8250;
> +	snprintf(opts, ACPI_SPCR_OPTS_SIZE, "%s,%s,0x%llx,%d", uart, iotype,
> +		 table->serial_port.address, baud_rate);
> +	return 0;
>  }
>  
> +bool console_acpi_spcr_enable __initdata;
>  /**
> - * parse_spcr() - parse ACPI SPCR table and add preferred console
> + * acpi_parse_spcr() - parse ACPI SPCR table and add preferred console
>   *
>   * @earlycon: set up earlycon for the console specified by the table
>   *
> @@ -86,13 +39,13 @@ static bool xgene_8250_erratum_present(struct acpi_table_spcr *tb)
>   * from arch initialization code as soon as the DT/ACPI decision is made.
>   *
>   */
> -int __init parse_spcr(bool earlycon)
> +int __init acpi_parse_spcr(bool earlycon)
>  {
> -	static char opts[64];
> +	static char opts[ACPI_SPCR_OPTS_SIZE];
> +	static char uart[ACPI_SPCR_BUF_SIZE];
> +	static char iotype[ACPI_SPCR_BUF_SIZE];
>  	struct acpi_table_spcr *table;
>  	acpi_status status;
> -	char *uart;
> -	char *iotype;
>  	int baud_rate;
>  	int err;
>  
> @@ -105,48 +58,6 @@ int __init parse_spcr(bool earlycon)
>  	if (ACPI_FAILURE(status))
>  		return -ENOENT;
>  
> -	if (table->header.revision < 2) {
> -		err = -ENOENT;
> -		pr_err("wrong table version\n");
> -		goto done;
> -	}
> -
> -	if (table->serial_port.space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY) {
> -		switch (ACPI_ACCESS_BIT_WIDTH((
> -			table->serial_port.access_width))) {
> -		default:
> -			pr_err("Unexpected SPCR Access Width.  Defaulting to byte size\n");
> -		case 8:
> -			iotype = "mmio";
> -			break;
> -		case 16:
> -			iotype = "mmio16";
> -			break;
> -		case 32:
> -			iotype = "mmio32";
> -			break;
> -		}
> -	} else
> -		iotype = "io";
> -
> -	switch (table->interface_type) {
> -	case ACPI_DBG2_ARM_SBSA_32BIT:
> -		iotype = "mmio32";
> -		/* fall through */
> -	case ACPI_DBG2_ARM_PL011:
> -	case ACPI_DBG2_ARM_SBSA_GENERIC:
> -	case ACPI_DBG2_BCM2835:
> -		uart = "pl011";
> -		break;
> -	case ACPI_DBG2_16550_COMPATIBLE:
> -	case ACPI_DBG2_16550_SUBSET:
> -		uart = "uart";
> -		break;
> -	default:
> -		err = -ENOENT;
> -		goto done;
> -	}
> -
>  	switch (table->baud_rate) {
>  	case 3:
>  		baud_rate = 9600;
> @@ -165,43 +76,36 @@ int __init parse_spcr(bool earlycon)
>  		goto done;
>  	}
>  
> -	/*
> -	 * If the E44 erratum is required, then we need to tell the pl011
> -	 * driver to implement the work-around.
> -	 *
> -	 * The global variable is used by the probe function when it
> -	 * creates the UARTs, whether or not they're used as a console.
> -	 *
> -	 * If the user specifies "traditional" earlycon, the qdf2400_e44
> -	 * console name matches the EARLYCON_DECLARE() statement, and
> -	 * SPCR is not used.  Parameter "earlycon" is false.
> -	 *
> -	 * If the user specifies "SPCR" earlycon, then we need to update
> -	 * the console name so that it also says "qdf2400_e44".  Parameter
> -	 * "earlycon" is true.
> -	 *
> -	 * For consistency, if we change the console name, then we do it
> -	 * for everyone, not just earlycon.
> -	 */
> -	if (qdf2400_erratum_44_present(&table->header)) {
> -		qdf2400_e44_present = true;
> -		if (earlycon)
> -			uart = "qdf2400_e44";
> +	switch (table->interface_type) {
> +	case ACPI_DBG2_16550_COMPATIBLE:
> +	case ACPI_DBG2_16550_SUBSET:
> +		snprintf(uart, ACPI_SPCR_BUF_SIZE, "uart");
> +		break;
> +	default:
> +		break;
>  	}
>  
> -	if (xgene_8250_erratum_present(table)) {
> -		iotype = "mmio32";
> +	if (table->serial_port.space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY) {
> +		u8 width = ACPI_ACCESS_BIT_WIDTH((
> +					table->serial_port.access_width));
> +		switch (width) {
> +		default:
> +			pr_err("Unexpected SPCR Access Width.  Defaulting to byte size\n");
> +		case 8:
> +			snprintf(iotype, ACPI_SPCR_BUF_SIZE, "mmio");
> +			break;
> +		case 16:
> +		case 32:
> +			snprintf(iotype, ACPI_SPCR_BUF_SIZE, "mmio%d", width);
> +			break;
> +		}
> +	} else
> +		snprintf(iotype, ACPI_SPCR_BUF_SIZE, "io");
>  
> -		/* for xgene v1 and v2 we don't know the clock rate of the
> -		 * UART so don't attempt to change to the baud rate state
> -		 * in the table because driver cannot calculate the dividers
> -		 */
> -		snprintf(opts, sizeof(opts), "%s,%s,0x%llx", uart, iotype,
> -			 table->serial_port.address);
> -	} else {
> -		snprintf(opts, sizeof(opts), "%s,%s,0x%llx,%d", uart, iotype,
> -			 table->serial_port.address, baud_rate);
> -	}
> +	err = acpi_arch_setup_console(table, opts, uart, iotype, baud_rate,
> +				      earlycon);
> +	if (err)
> +		goto done;
>  
>  	pr_info("console: %s\n", opts);
>  
> @@ -209,7 +113,6 @@ int __init parse_spcr(bool earlycon)
>  		setup_earlycon(opts);
>  
>  	err = add_preferred_console(uart, 0, opts + strlen(uart) + 1);
> -
>  done:
>  	acpi_put_table((struct acpi_table_header *)table);
>  	return err;
> diff --git a/drivers/tty/serial/earlycon.c b/drivers/tty/serial/earlycon.c
> index 4c8b80f1c688..b22afb62c7a3 100644
> --- a/drivers/tty/serial/earlycon.c
> +++ b/drivers/tty/serial/earlycon.c
> @@ -196,26 +196,15 @@ int __init setup_earlycon(char *buf)
>  	return -ENOENT;
>  }
>  
> -/*
> - * When CONFIG_ACPI_SPCR_TABLE is defined, "earlycon" without parameters in
> - * command line does not start DT earlycon immediately, instead it defers
> - * starting it until DT/ACPI decision is made.  At that time if ACPI is enabled
> - * call parse_spcr(), else call early_init_dt_scan_chosen_stdout()
> - */
> -bool earlycon_init_is_deferred __initdata;
> -
>  /* early_param wrapper for setup_earlycon() */
>  static int __init param_setup_earlycon(char *buf)
>  {
>  	int err;
>  
> -	/*
> -	 * Just 'earlycon' is a valid param for devicetree earlycons;
> -	 * don't generate a warning from parse_early_params() in that case
> -	 */
> +	/* Just 'earlycon' is a valid param for devicetree and ACPI SPCR. */
>  	if (!buf || !buf[0]) {
>  		if (IS_ENABLED(CONFIG_ACPI_SPCR_TABLE)) {
> -			earlycon_init_is_deferred = true;
> +			console_acpi_spcr_enable = true;
>  			return 0;
>  		} else if (!buf) {
>  			return early_init_dt_scan_chosen_stdout();
> diff --git a/include/linux/acpi.h b/include/linux/acpi.h
> index dc1ebfeeb5ec..875d7327d91c 100644
> --- a/include/linux/acpi.h
> +++ b/include/linux/acpi.h
> @@ -1241,10 +1241,17 @@ static inline bool acpi_has_watchdog(void) { return false; }
>  #endif
>  
>  #ifdef CONFIG_ACPI_SPCR_TABLE
> +#define ACPI_SPCR_OPTS_SIZE 64
> +#define ACPI_SPCR_BUF_SIZE 32
>  extern bool qdf2400_e44_present;
> -int parse_spcr(bool earlycon);
> +extern bool console_acpi_spcr_enable __initdata;
> +extern int acpi_arch_setup_console(struct acpi_table_spcr *table,
> +				   char *opts, char *uart, char *iotype,
> +				   int baud_rate, bool earlycon);
> +int acpi_parse_spcr(bool earlycon);
>  #else
> -static inline int parse_spcr(bool earlycon) { return 0; }
> +static const bool console_acpi_spcr_enable;
> +static inline int acpi_parse_spcr(bool earlycon) { return 0; }
>  #endif
>  
>  #if IS_ENABLED(CONFIG_ACPI_GENERIC_GSI)
> diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
> index 37b044e78333..abfffb4b1c37 100644
> --- a/include/linux/serial_core.h
> +++ b/include/linux/serial_core.h
> @@ -376,10 +376,8 @@ extern int of_setup_earlycon(const struct earlycon_id *match,
>  			     const char *options);
>  
>  #ifdef CONFIG_SERIAL_EARLYCON
> -extern bool earlycon_init_is_deferred __initdata;
>  int setup_earlycon(char *buf);
>  #else
> -static const bool earlycon_init_is_deferred;
>  static inline int setup_earlycon(char *buf) { return 0; }
>  #endif
>  
> 

Re: [PATCH v2 1/2] acpi, spcr: Make SPCR avialable to other architectures

[Will Deacon]

[adding Lorenzo, Sudeep and Hanjun -- see Rafael's comment below]

On Wed, Dec 13, 2017 at 01:22:59AM +0100, Rafael J. Wysocki wrote:
> On Monday, December 11, 2017 4:50:58 PM CET Prarit Bhargava wrote:
> > Other architectures can use SPCR to setup an early console or console
> > but the current code is ARM64 specific.
> > 
> > Change the name of parse_spcr() to acpi_parse_spcr().  Add a weak
> > function acpi_arch_setup_console() that can be used for arch-specific
> > setup.  Move flags into ACPI code.  Update the Documention on the use of
> > the SPCR.
> > 
> > [v2]: Don't return an error in the baud_rate check of acpi_parse_spcr().
> > Keep ACPI_SPCR_TABLE selected for ARM64.  Fix 8-bit port access width
> > mmio value.  Move baud rate check earlier.
> > 
> > Signed-off-by: Prarit Bhargava <prarit@redhat.com>
> 
> This mostly affects ARM64, so ACKs from that side are requisite for it.
> 
> > Cc: linux-doc@vger.kernel.org
> > Cc: linux-kernel@vger.kernel.org
> > Cc: linux-arm-kernel@lists.infradead.org
> > Cc: linux-pm@vger.kernel.org
> > Cc: linux-acpi@vger.kernel.org
> > Cc: linux-serial@vger.kernel.org
> > Cc: Bhupesh Sharma <bhsharma@redhat.com>
> > Cc: Lv Zheng <lv.zheng@intel.com>
> > Cc: Thomas Gleixner <tglx@linutronix.de>
> > Cc: Ingo Molnar <mingo@redhat.com>
> > Cc: "H. Peter Anvin" <hpa@zytor.com>
> > Cc: x86@kernel.org
> > Cc: Jonathan Corbet <corbet@lwn.net>
> > Cc: Catalin Marinas <catalin.marinas@arm.com>
> > Cc: Will Deacon <will.deacon@arm.com>
> > Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> > Cc: Timur Tabi <timur@codeaurora.org>
> > ---
> >  Documentation/admin-guide/kernel-parameters.txt |   6 +-
> >  arch/arm64/kernel/acpi.c                        | 128 ++++++++++++++++-
> >  drivers/acpi/Kconfig                            |   7 +-
> >  drivers/acpi/spcr.c                             | 175 ++++++------------------
> >  drivers/tty/serial/earlycon.c                   |  15 +-
> >  include/linux/acpi.h                            |  11 +-
> >  include/linux/serial_core.h                     |   2 -
> >  7 files changed, 184 insertions(+), 160 deletions(-)
> > 
> > diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> > index 6571fbfdb2a1..0d173289c67e 100644
> > --- a/Documentation/admin-guide/kernel-parameters.txt
> > +++ b/Documentation/admin-guide/kernel-parameters.txt
> > @@ -914,9 +914,9 @@
> >  
> >  	earlycon=	[KNL] Output early console device and options.
> >  
> > -			When used with no options, the early console is
> > -			determined by the stdout-path property in device
> > -			tree's chosen node.
> > +			[ARM64] The early console is determined by the
> > +			stdout-path property in device tree's chosen node,
> > +			or determined by the ACPI SPCR table.
> >  
> >  		cdns,<addr>[,options]
> >  			Start an early, polled-mode console on a Cadence
> > diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c
> > index b3162715ed78..b3e33bbdf3b7 100644
> > --- a/arch/arm64/kernel/acpi.c
> > +++ b/arch/arm64/kernel/acpi.c
> > @@ -25,7 +25,6 @@
> >  #include <linux/memblock.h>
> >  #include <linux/of_fdt.h>
> >  #include <linux/smp.h>
> > -#include <linux/serial_core.h>
> >  
> >  #include <asm/cputype.h>
> >  #include <asm/cpu_ops.h>
> > @@ -177,6 +176,128 @@ static int __init acpi_fadt_sanity_check(void)
> >  	return ret;
> >  }
> >  
> > +/*
> > + * Erratum 44 for QDF2432v1 and QDF2400v1 SoCs describes the BUSY bit as
> > + * occasionally getting stuck as 1. To avoid the potential for a hang, check
> > + * TXFE == 0 instead of BUSY == 1. This may not be suitable for all UART
> > + * implementations, so only do so if an affected platform is detected in
> > + * acpi_parse_spcr().
> > + */
> > +bool qdf2400_e44_present;
> > +EXPORT_SYMBOL(qdf2400_e44_present);
> > +
> > +/*
> > + * Some Qualcomm Datacenter Technologies SoCs have a defective UART BUSY bit.
> > + * Detect them by examining the OEM fields in the SPCR header, similar to PCI
> > + * quirk detection in pci_mcfg.c.
> > + */
> > +static bool qdf2400_erratum_44_present(struct acpi_table_header *h)
> > +{
> > +	if (memcmp(h->oem_id, "QCOM  ", ACPI_OEM_ID_SIZE))
> > +		return false;
> > +
> > +	if (!memcmp(h->oem_table_id, "QDF2432 ", ACPI_OEM_TABLE_ID_SIZE))
> > +		return true;
> > +
> > +	if (!memcmp(h->oem_table_id, "QDF2400 ", ACPI_OEM_TABLE_ID_SIZE) &&
> > +			h->oem_revision == 1)
> > +		return true;
> > +
> > +	return false;
> > +}
> > +
> > +/*
> > + * APM X-Gene v1 and v2 UART hardware is an 16550 like device but has its
> > + * register aligned to 32-bit. In addition, the BIOS also encoded the
> > + * access width to be 8 bits. This function detects this errata condition.
> > + */
> > +static bool xgene_8250_erratum_present(struct acpi_table_spcr *tb)
> > +{
> > +	bool xgene_8250 = false;
> > +
> > +	if (tb->interface_type != ACPI_DBG2_16550_COMPATIBLE)
> > +		return false;
> > +
> > +	if (memcmp(tb->header.oem_id, "APMC0D", ACPI_OEM_ID_SIZE) &&
> > +	    memcmp(tb->header.oem_id, "HPE   ", ACPI_OEM_ID_SIZE))
> > +		return false;
> > +
> > +	if (!memcmp(tb->header.oem_table_id, "XGENESPC",
> > +	    ACPI_OEM_TABLE_ID_SIZE) && tb->header.oem_revision == 0)
> > +		xgene_8250 = true;
> > +
> > +	if (!memcmp(tb->header.oem_table_id, "ProLiant",
> > +	    ACPI_OEM_TABLE_ID_SIZE) && tb->header.oem_revision == 1)
> > +		xgene_8250 = true;
> > +
> > +	return xgene_8250;
> > +}
> > +
> > +int acpi_arch_setup_console(struct acpi_table_spcr *table,
> > +			    char *opts, char *uart, char *iotype,
> > +			    int baud_rate, bool earlycon)
> > +{
> > +	if (table->header.revision < 2) {
> > +		pr_err("wrong table version\n");
> > +		return -ENOENT;
> > +	}
> > +
> > +	switch (table->interface_type) {
> > +	case ACPI_DBG2_ARM_SBSA_32BIT:
> > +		snprintf(iotype, ACPI_SPCR_BUF_SIZE, "mmio32");
> > +		/* fall through */
> > +	case ACPI_DBG2_ARM_PL011:
> > +	case ACPI_DBG2_ARM_SBSA_GENERIC:
> > +	case ACPI_DBG2_BCM2835:
> > +		snprintf(uart, ACPI_SPCR_BUF_SIZE, "pl011");
> > +		break;
> > +	default:
> > +		if (strlen(uart) == 0)
> > +			return -ENOENT;
> > +	}
> > +
> > +	/*
> > +	 * If the E44 erratum is required, then we need to tell the pl011
> > +	 * driver to implement the work-around.
> > +	 *
> > +	 * The global variable is used by the probe function when it
> > +	 * creates the UARTs, whether or not they're used as a console.
> > +	 *
> > +	 * If the user specifies "traditional" earlycon, the qdf2400_e44
> > +	 * console name matches the EARLYCON_DECLARE() statement, and
> > +	 * SPCR is not used.  Parameter "earlycon" is false.
> > +	 *
> > +	 * If the user specifies "SPCR" earlycon, then we need to update
> > +	 * the console name so that it also says "qdf2400_e44".  Parameter
> > +	 * "earlycon" is true.
> > +	 *
> > +	 * For consistency, if we change the console name, then we do it
> > +	 * for everyone, not just earlycon.
> > +	 */
> > +	if (qdf2400_erratum_44_present(&table->header)) {
> > +		qdf2400_e44_present = true;
> > +		if (earlycon)
> > +			snprintf(uart, ACPI_SPCR_BUF_SIZE, "qdf2400_e44");
> > +	}
> > +
> > +	if (xgene_8250_erratum_present(table)) {
> > +		snprintf(iotype, ACPI_SPCR_BUF_SIZE, "mmio32");
> > +
> > +		/* for xgene v1 and v2 we don't know the clock rate of the
> > +		 * UART so don't attempt to change to the baud rate state
> > +		 * in the table because driver cannot calculate the dividers
> > +		 */
> > +		snprintf(opts, ACPI_SPCR_OPTS_SIZE, "%s,%s,0x%llx", uart,
> > +			 iotype, table->serial_port.address);
> > +	} else {
> > +		snprintf(opts, ACPI_SPCR_OPTS_SIZE, "%s,%s,0x%llx,%d", uart,
> > +			 iotype, table->serial_port.address, baud_rate);
> > +	}
> > +
> > +	return 0;
> > +}
> > +EXPORT_SYMBOL(acpi_arch_setup_console);
> > +
> >  /*
> >   * acpi_boot_table_init() called from setup_arch(), always.
> >   *	1. find RSDP and get its address, and then find XSDT
> > @@ -230,10 +351,11 @@ void __init acpi_boot_table_init(void)
> >  
> >  done:
> >  	if (acpi_disabled) {
> > -		if (earlycon_init_is_deferred)
> > +		if (console_acpi_spcr_enable)
> >  			early_init_dt_scan_chosen_stdout();
> >  	} else {
> > -		parse_spcr(earlycon_init_is_deferred);
> > +		/* Always enable the ACPI SPCR console */
> > +		acpi_parse_spcr(console_acpi_spcr_enable);
> >  		if (IS_ENABLED(CONFIG_ACPI_BGRT))
> >  			acpi_table_parse(ACPI_SIG_BGRT, acpi_parse_bgrt);
> >  	}
> > diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
> > index 46505396869e..9ae98eeada76 100644
> > --- a/drivers/acpi/Kconfig
> > +++ b/drivers/acpi/Kconfig
> > @@ -79,7 +79,12 @@ config ACPI_DEBUGGER_USER
> >  endif
> >  
> >  config ACPI_SPCR_TABLE
> > -	bool
> > +	bool "ACPI Serial Port Console Redirection Support"
> > +	default y if ARM64
> > +	help
> > +	  Enable support for Serial Port Console Redirection (SPCR) Table.
> > +	  This table provides information about the configuration of the
> > +	  earlycon console.
> >  
> >  config ACPI_LPIT
> >  	bool
> > diff --git a/drivers/acpi/spcr.c b/drivers/acpi/spcr.c
> > index 324b35bfe781..f4bb8110e404 100644
> > --- a/drivers/acpi/spcr.c
> > +++ b/drivers/acpi/spcr.c
> > @@ -16,65 +16,18 @@
> >  #include <linux/kernel.h>
> >  #include <linux/serial_core.h>
> >  
> > -/*
> > - * Erratum 44 for QDF2432v1 and QDF2400v1 SoCs describes the BUSY bit as
> > - * occasionally getting stuck as 1. To avoid the potential for a hang, check
> > - * TXFE == 0 instead of BUSY == 1. This may not be suitable for all UART
> > - * implementations, so only do so if an affected platform is detected in
> > - * parse_spcr().
> > - */
> > -bool qdf2400_e44_present;
> > -EXPORT_SYMBOL(qdf2400_e44_present);
> > -
> > -/*
> > - * Some Qualcomm Datacenter Technologies SoCs have a defective UART BUSY bit.
> > - * Detect them by examining the OEM fields in the SPCR header, similiar to PCI
> > - * quirk detection in pci_mcfg.c.
> > - */
> > -static bool qdf2400_erratum_44_present(struct acpi_table_header *h)
> > -{
> > -	if (memcmp(h->oem_id, "QCOM  ", ACPI_OEM_ID_SIZE))
> > -		return false;
> > -
> > -	if (!memcmp(h->oem_table_id, "QDF2432 ", ACPI_OEM_TABLE_ID_SIZE))
> > -		return true;
> > -
> > -	if (!memcmp(h->oem_table_id, "QDF2400 ", ACPI_OEM_TABLE_ID_SIZE) &&
> > -			h->oem_revision == 1)
> > -		return true;
> > -
> > -	return false;
> > -}
> > -
> > -/*
> > - * APM X-Gene v1 and v2 UART hardware is an 16550 like device but has its
> > - * register aligned to 32-bit. In addition, the BIOS also encoded the
> > - * access width to be 8 bits. This function detects this errata condition.
> > - */
> > -static bool xgene_8250_erratum_present(struct acpi_table_spcr *tb)
> > +int __weak acpi_arch_setup_console(struct acpi_table_spcr *table,
> > +				   char *opts, char *uart, char *iotype,
> > +				   int baud_rate, bool earlycon)
> >  {
> > -	bool xgene_8250 = false;
> > -
> > -	if (tb->interface_type != ACPI_DBG2_16550_COMPATIBLE)
> > -		return false;
> > -
> > -	if (memcmp(tb->header.oem_id, "APMC0D", ACPI_OEM_ID_SIZE) &&
> > -	    memcmp(tb->header.oem_id, "HPE   ", ACPI_OEM_ID_SIZE))
> > -		return false;
> > -
> > -	if (!memcmp(tb->header.oem_table_id, "XGENESPC",
> > -	    ACPI_OEM_TABLE_ID_SIZE) && tb->header.oem_revision == 0)
> > -		xgene_8250 = true;
> > -
> > -	if (!memcmp(tb->header.oem_table_id, "ProLiant",
> > -	    ACPI_OEM_TABLE_ID_SIZE) && tb->header.oem_revision == 1)
> > -		xgene_8250 = true;
> > -
> > -	return xgene_8250;
> > +	snprintf(opts, ACPI_SPCR_OPTS_SIZE, "%s,%s,0x%llx,%d", uart, iotype,
> > +		 table->serial_port.address, baud_rate);
> > +	return 0;
> >  }
> >  
> > +bool console_acpi_spcr_enable __initdata;
> >  /**
> > - * parse_spcr() - parse ACPI SPCR table and add preferred console
> > + * acpi_parse_spcr() - parse ACPI SPCR table and add preferred console
> >   *
> >   * @earlycon: set up earlycon for the console specified by the table
> >   *
> > @@ -86,13 +39,13 @@ static bool xgene_8250_erratum_present(struct acpi_table_spcr *tb)
> >   * from arch initialization code as soon as the DT/ACPI decision is made.
> >   *
> >   */
> > -int __init parse_spcr(bool earlycon)
> > +int __init acpi_parse_spcr(bool earlycon)
> >  {
> > -	static char opts[64];
> > +	static char opts[ACPI_SPCR_OPTS_SIZE];
> > +	static char uart[ACPI_SPCR_BUF_SIZE];
> > +	static char iotype[ACPI_SPCR_BUF_SIZE];
> >  	struct acpi_table_spcr *table;
> >  	acpi_status status;
> > -	char *uart;
> > -	char *iotype;
> >  	int baud_rate;
> >  	int err;
> >  
> > @@ -105,48 +58,6 @@ int __init parse_spcr(bool earlycon)
> >  	if (ACPI_FAILURE(status))
> >  		return -ENOENT;
> >  
> > -	if (table->header.revision < 2) {
> > -		err = -ENOENT;
> > -		pr_err("wrong table version\n");
> > -		goto done;
> > -	}
> > -
> > -	if (table->serial_port.space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY) {
> > -		switch (ACPI_ACCESS_BIT_WIDTH((
> > -			table->serial_port.access_width))) {
> > -		default:
> > -			pr_err("Unexpected SPCR Access Width.  Defaulting to byte size\n");
> > -		case 8:
> > -			iotype = "mmio";
> > -			break;
> > -		case 16:
> > -			iotype = "mmio16";
> > -			break;
> > -		case 32:
> > -			iotype = "mmio32";
> > -			break;
> > -		}
> > -	} else
> > -		iotype = "io";
> > -
> > -	switch (table->interface_type) {
> > -	case ACPI_DBG2_ARM_SBSA_32BIT:
> > -		iotype = "mmio32";
> > -		/* fall through */
> > -	case ACPI_DBG2_ARM_PL011:
> > -	case ACPI_DBG2_ARM_SBSA_GENERIC:
> > -	case ACPI_DBG2_BCM2835:
> > -		uart = "pl011";
> > -		break;
> > -	case ACPI_DBG2_16550_COMPATIBLE:
> > -	case ACPI_DBG2_16550_SUBSET:
> > -		uart = "uart";
> > -		break;
> > -	default:
> > -		err = -ENOENT;
> > -		goto done;
> > -	}
> > -
> >  	switch (table->baud_rate) {
> >  	case 3:
> >  		baud_rate = 9600;
> > @@ -165,43 +76,36 @@ int __init parse_spcr(bool earlycon)
> >  		goto done;
> >  	}
> >  
> > -	/*
> > -	 * If the E44 erratum is required, then we need to tell the pl011
> > -	 * driver to implement the work-around.
> > -	 *
> > -	 * The global variable is used by the probe function when it
> > -	 * creates the UARTs, whether or not they're used as a console.
> > -	 *
> > -	 * If the user specifies "traditional" earlycon, the qdf2400_e44
> > -	 * console name matches the EARLYCON_DECLARE() statement, and
> > -	 * SPCR is not used.  Parameter "earlycon" is false.
> > -	 *
> > -	 * If the user specifies "SPCR" earlycon, then we need to update
> > -	 * the console name so that it also says "qdf2400_e44".  Parameter
> > -	 * "earlycon" is true.
> > -	 *
> > -	 * For consistency, if we change the console name, then we do it
> > -	 * for everyone, not just earlycon.
> > -	 */
> > -	if (qdf2400_erratum_44_present(&table->header)) {
> > -		qdf2400_e44_present = true;
> > -		if (earlycon)
> > -			uart = "qdf2400_e44";
> > +	switch (table->interface_type) {
> > +	case ACPI_DBG2_16550_COMPATIBLE:
> > +	case ACPI_DBG2_16550_SUBSET:
> > +		snprintf(uart, ACPI_SPCR_BUF_SIZE, "uart");
> > +		break;
> > +	default:
> > +		break;
> >  	}
> >  
> > -	if (xgene_8250_erratum_present(table)) {
> > -		iotype = "mmio32";
> > +	if (table->serial_port.space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY) {
> > +		u8 width = ACPI_ACCESS_BIT_WIDTH((
> > +					table->serial_port.access_width));
> > +		switch (width) {
> > +		default:
> > +			pr_err("Unexpected SPCR Access Width.  Defaulting to byte size\n");
> > +		case 8:
> > +			snprintf(iotype, ACPI_SPCR_BUF_SIZE, "mmio");
> > +			break;
> > +		case 16:
> > +		case 32:
> > +			snprintf(iotype, ACPI_SPCR_BUF_SIZE, "mmio%d", width);
> > +			break;
> > +		}
> > +	} else
> > +		snprintf(iotype, ACPI_SPCR_BUF_SIZE, "io");
> >  
> > -		/* for xgene v1 and v2 we don't know the clock rate of the
> > -		 * UART so don't attempt to change to the baud rate state
> > -		 * in the table because driver cannot calculate the dividers
> > -		 */
> > -		snprintf(opts, sizeof(opts), "%s,%s,0x%llx", uart, iotype,
> > -			 table->serial_port.address);
> > -	} else {
> > -		snprintf(opts, sizeof(opts), "%s,%s,0x%llx,%d", uart, iotype,
> > -			 table->serial_port.address, baud_rate);
> > -	}
> > +	err = acpi_arch_setup_console(table, opts, uart, iotype, baud_rate,
> > +				      earlycon);
> > +	if (err)
> > +		goto done;
> >  
> >  	pr_info("console: %s\n", opts);
> >  
> > @@ -209,7 +113,6 @@ int __init parse_spcr(bool earlycon)
> >  		setup_earlycon(opts);
> >  
> >  	err = add_preferred_console(uart, 0, opts + strlen(uart) + 1);
> > -
> >  done:
> >  	acpi_put_table((struct acpi_table_header *)table);
> >  	return err;
> > diff --git a/drivers/tty/serial/earlycon.c b/drivers/tty/serial/earlycon.c
> > index 4c8b80f1c688..b22afb62c7a3 100644
> > --- a/drivers/tty/serial/earlycon.c
> > +++ b/drivers/tty/serial/earlycon.c
> > @@ -196,26 +196,15 @@ int __init setup_earlycon(char *buf)
> >  	return -ENOENT;
> >  }
> >  
> > -/*
> > - * When CONFIG_ACPI_SPCR_TABLE is defined, "earlycon" without parameters in
> > - * command line does not start DT earlycon immediately, instead it defers
> > - * starting it until DT/ACPI decision is made.  At that time if ACPI is enabled
> > - * call parse_spcr(), else call early_init_dt_scan_chosen_stdout()
> > - */
> > -bool earlycon_init_is_deferred __initdata;
> > -
> >  /* early_param wrapper for setup_earlycon() */
> >  static int __init param_setup_earlycon(char *buf)
> >  {
> >  	int err;
> >  
> > -	/*
> > -	 * Just 'earlycon' is a valid param for devicetree earlycons;
> > -	 * don't generate a warning from parse_early_params() in that case
> > -	 */
> > +	/* Just 'earlycon' is a valid param for devicetree and ACPI SPCR. */
> >  	if (!buf || !buf[0]) {
> >  		if (IS_ENABLED(CONFIG_ACPI_SPCR_TABLE)) {
> > -			earlycon_init_is_deferred = true;
> > +			console_acpi_spcr_enable = true;
> >  			return 0;
> >  		} else if (!buf) {
> >  			return early_init_dt_scan_chosen_stdout();
> > diff --git a/include/linux/acpi.h b/include/linux/acpi.h
> > index dc1ebfeeb5ec..875d7327d91c 100644
> > --- a/include/linux/acpi.h
> > +++ b/include/linux/acpi.h
> > @@ -1241,10 +1241,17 @@ static inline bool acpi_has_watchdog(void) { return false; }
> >  #endif
> >  
> >  #ifdef CONFIG_ACPI_SPCR_TABLE
> > +#define ACPI_SPCR_OPTS_SIZE 64
> > +#define ACPI_SPCR_BUF_SIZE 32
> >  extern bool qdf2400_e44_present;
> > -int parse_spcr(bool earlycon);
> > +extern bool console_acpi_spcr_enable __initdata;
> > +extern int acpi_arch_setup_console(struct acpi_table_spcr *table,
> > +				   char *opts, char *uart, char *iotype,
> > +				   int baud_rate, bool earlycon);
> > +int acpi_parse_spcr(bool earlycon);
> >  #else
> > -static inline int parse_spcr(bool earlycon) { return 0; }
> > +static const bool console_acpi_spcr_enable;
> > +static inline int acpi_parse_spcr(bool earlycon) { return 0; }
> >  #endif
> >  
> >  #if IS_ENABLED(CONFIG_ACPI_GENERIC_GSI)
> > diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
> > index 37b044e78333..abfffb4b1c37 100644
> > --- a/include/linux/serial_core.h
> > +++ b/include/linux/serial_core.h
> > @@ -376,10 +376,8 @@ extern int of_setup_earlycon(const struct earlycon_id *match,
> >  			     const char *options);
> >  
> >  #ifdef CONFIG_SERIAL_EARLYCON
> > -extern bool earlycon_init_is_deferred __initdata;
> >  int setup_earlycon(char *buf);
> >  #else
> > -static const bool earlycon_init_is_deferred;
> >  static inline int setup_earlycon(char *buf) { return 0; }
> >  #endif
> >  
> > 
> 
> 

[PATCH v2 1/2] acpi, spcr: Make SPCR avialable to other architectures

[Will Deacon]

[adding Lorenzo, Sudeep and Hanjun -- see Rafael's comment below]

On Wed, Dec 13, 2017 at 01:22:59AM +0100, Rafael J. Wysocki wrote:
> On Monday, December 11, 2017 4:50:58 PM CET Prarit Bhargava wrote:
> > Other architectures can use SPCR to setup an early console or console
> > but the current code is ARM64 specific.
> > 
> > Change the name of parse_spcr() to acpi_parse_spcr().  Add a weak
> > function acpi_arch_setup_console() that can be used for arch-specific
> > setup.  Move flags into ACPI code.  Update the Documention on the use of
> > the SPCR.
> > 
> > [v2]: Don't return an error in the baud_rate check of acpi_parse_spcr().
> > Keep ACPI_SPCR_TABLE selected for ARM64.  Fix 8-bit port access width
> > mmio value.  Move baud rate check earlier.
> > 
> > Signed-off-by: Prarit Bhargava <prarit@redhat.com>
> 
> This mostly affects ARM64, so ACKs from that side are requisite for it.
> 
> > Cc: linux-doc at vger.kernel.org
> > Cc: linux-kernel at vger.kernel.org
> > Cc: linux-arm-kernel at lists.infradead.org
> > Cc: linux-pm at vger.kernel.org
> > Cc: linux-acpi at vger.kernel.org
> > Cc: linux-serial at vger.kernel.org
> > Cc: Bhupesh Sharma <bhsharma@redhat.com>
> > Cc: Lv Zheng <lv.zheng@intel.com>
> > Cc: Thomas Gleixner <tglx@linutronix.de>
> > Cc: Ingo Molnar <mingo@redhat.com>
> > Cc: "H. Peter Anvin" <hpa@zytor.com>
> > Cc: x86 at kernel.org
> > Cc: Jonathan Corbet <corbet@lwn.net>
> > Cc: Catalin Marinas <catalin.marinas@arm.com>
> > Cc: Will Deacon <will.deacon@arm.com>
> > Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> > Cc: Timur Tabi <timur@codeaurora.org>
> > ---
> >  Documentation/admin-guide/kernel-parameters.txt |   6 +-
> >  arch/arm64/kernel/acpi.c                        | 128 ++++++++++++++++-
> >  drivers/acpi/Kconfig                            |   7 +-
> >  drivers/acpi/spcr.c                             | 175 ++++++------------------
> >  drivers/tty/serial/earlycon.c                   |  15 +-
> >  include/linux/acpi.h                            |  11 +-
> >  include/linux/serial_core.h                     |   2 -
> >  7 files changed, 184 insertions(+), 160 deletions(-)
> > 
> > diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> > index 6571fbfdb2a1..0d173289c67e 100644
> > --- a/Documentation/admin-guide/kernel-parameters.txt
> > +++ b/Documentation/admin-guide/kernel-parameters.txt
> > @@ -914,9 +914,9 @@
> >  
> >  	earlycon=	[KNL] Output early console device and options.
> >  
> > -			When used with no options, the early console is
> > -			determined by the stdout-path property in device
> > -			tree's chosen node.
> > +			[ARM64] The early console is determined by the
> > +			stdout-path property in device tree's chosen node,
> > +			or determined by the ACPI SPCR table.
> >  
> >  		cdns,<addr>[,options]
> >  			Start an early, polled-mode console on a Cadence
> > diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c
> > index b3162715ed78..b3e33bbdf3b7 100644
> > --- a/arch/arm64/kernel/acpi.c
> > +++ b/arch/arm64/kernel/acpi.c
> > @@ -25,7 +25,6 @@
> >  #include <linux/memblock.h>
> >  #include <linux/of_fdt.h>
> >  #include <linux/smp.h>
> > -#include <linux/serial_core.h>
> >  
> >  #include <asm/cputype.h>
> >  #include <asm/cpu_ops.h>
> > @@ -177,6 +176,128 @@ static int __init acpi_fadt_sanity_check(void)
> >  	return ret;
> >  }
> >  
> > +/*
> > + * Erratum 44 for QDF2432v1 and QDF2400v1 SoCs describes the BUSY bit as
> > + * occasionally getting stuck as 1. To avoid the potential for a hang, check
> > + * TXFE == 0 instead of BUSY == 1. This may not be suitable for all UART
> > + * implementations, so only do so if an affected platform is detected in
> > + * acpi_parse_spcr().
> > + */
> > +bool qdf2400_e44_present;
> > +EXPORT_SYMBOL(qdf2400_e44_present);
> > +
> > +/*
> > + * Some Qualcomm Datacenter Technologies SoCs have a defective UART BUSY bit.
> > + * Detect them by examining the OEM fields in the SPCR header, similar to PCI
> > + * quirk detection in pci_mcfg.c.
> > + */
> > +static bool qdf2400_erratum_44_present(struct acpi_table_header *h)
> > +{
> > +	if (memcmp(h->oem_id, "QCOM  ", ACPI_OEM_ID_SIZE))
> > +		return false;
> > +
> > +	if (!memcmp(h->oem_table_id, "QDF2432 ", ACPI_OEM_TABLE_ID_SIZE))
> > +		return true;
> > +
> > +	if (!memcmp(h->oem_table_id, "QDF2400 ", ACPI_OEM_TABLE_ID_SIZE) &&
> > +			h->oem_revision == 1)
> > +		return true;
> > +
> > +	return false;
> > +}
> > +
> > +/*
> > + * APM X-Gene v1 and v2 UART hardware is an 16550 like device but has its
> > + * register aligned to 32-bit. In addition, the BIOS also encoded the
> > + * access width to be 8 bits. This function detects this errata condition.
> > + */
> > +static bool xgene_8250_erratum_present(struct acpi_table_spcr *tb)
> > +{
> > +	bool xgene_8250 = false;
> > +
> > +	if (tb->interface_type != ACPI_DBG2_16550_COMPATIBLE)
> > +		return false;
> > +
> > +	if (memcmp(tb->header.oem_id, "APMC0D", ACPI_OEM_ID_SIZE) &&
> > +	    memcmp(tb->header.oem_id, "HPE   ", ACPI_OEM_ID_SIZE))
> > +		return false;
> > +
> > +	if (!memcmp(tb->header.oem_table_id, "XGENESPC",
> > +	    ACPI_OEM_TABLE_ID_SIZE) && tb->header.oem_revision == 0)
> > +		xgene_8250 = true;
> > +
> > +	if (!memcmp(tb->header.oem_table_id, "ProLiant",
> > +	    ACPI_OEM_TABLE_ID_SIZE) && tb->header.oem_revision == 1)
> > +		xgene_8250 = true;
> > +
> > +	return xgene_8250;
> > +}
> > +
> > +int acpi_arch_setup_console(struct acpi_table_spcr *table,
> > +			    char *opts, char *uart, char *iotype,
> > +			    int baud_rate, bool earlycon)
> > +{
> > +	if (table->header.revision < 2) {
> > +		pr_err("wrong table version\n");
> > +		return -ENOENT;
> > +	}
> > +
> > +	switch (table->interface_type) {
> > +	case ACPI_DBG2_ARM_SBSA_32BIT:
> > +		snprintf(iotype, ACPI_SPCR_BUF_SIZE, "mmio32");
> > +		/* fall through */
> > +	case ACPI_DBG2_ARM_PL011:
> > +	case ACPI_DBG2_ARM_SBSA_GENERIC:
> > +	case ACPI_DBG2_BCM2835:
> > +		snprintf(uart, ACPI_SPCR_BUF_SIZE, "pl011");
> > +		break;
> > +	default:
> > +		if (strlen(uart) == 0)
> > +			return -ENOENT;
> > +	}
> > +
> > +	/*
> > +	 * If the E44 erratum is required, then we need to tell the pl011
> > +	 * driver to implement the work-around.
> > +	 *
> > +	 * The global variable is used by the probe function when it
> > +	 * creates the UARTs, whether or not they're used as a console.
> > +	 *
> > +	 * If the user specifies "traditional" earlycon, the qdf2400_e44
> > +	 * console name matches the EARLYCON_DECLARE() statement, and
> > +	 * SPCR is not used.  Parameter "earlycon" is false.
> > +	 *
> > +	 * If the user specifies "SPCR" earlycon, then we need to update
> > +	 * the console name so that it also says "qdf2400_e44".  Parameter
> > +	 * "earlycon" is true.
> > +	 *
> > +	 * For consistency, if we change the console name, then we do it
> > +	 * for everyone, not just earlycon.
> > +	 */
> > +	if (qdf2400_erratum_44_present(&table->header)) {
> > +		qdf2400_e44_present = true;
> > +		if (earlycon)
> > +			snprintf(uart, ACPI_SPCR_BUF_SIZE, "qdf2400_e44");
> > +	}
> > +
> > +	if (xgene_8250_erratum_present(table)) {
> > +		snprintf(iotype, ACPI_SPCR_BUF_SIZE, "mmio32");
> > +
> > +		/* for xgene v1 and v2 we don't know the clock rate of the
> > +		 * UART so don't attempt to change to the baud rate state
> > +		 * in the table because driver cannot calculate the dividers
> > +		 */
> > +		snprintf(opts, ACPI_SPCR_OPTS_SIZE, "%s,%s,0x%llx", uart,
> > +			 iotype, table->serial_port.address);
> > +	} else {
> > +		snprintf(opts, ACPI_SPCR_OPTS_SIZE, "%s,%s,0x%llx,%d", uart,
> > +			 iotype, table->serial_port.address, baud_rate);
> > +	}
> > +
> > +	return 0;
> > +}
> > +EXPORT_SYMBOL(acpi_arch_setup_console);
> > +
> >  /*
> >   * acpi_boot_table_init() called from setup_arch(), always.
> >   *	1. find RSDP and get its address, and then find XSDT
> > @@ -230,10 +351,11 @@ void __init acpi_boot_table_init(void)
> >  
> >  done:
> >  	if (acpi_disabled) {
> > -		if (earlycon_init_is_deferred)
> > +		if (console_acpi_spcr_enable)
> >  			early_init_dt_scan_chosen_stdout();
> >  	} else {
> > -		parse_spcr(earlycon_init_is_deferred);
> > +		/* Always enable the ACPI SPCR console */
> > +		acpi_parse_spcr(console_acpi_spcr_enable);
> >  		if (IS_ENABLED(CONFIG_ACPI_BGRT))
> >  			acpi_table_parse(ACPI_SIG_BGRT, acpi_parse_bgrt);
> >  	}
> > diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
> > index 46505396869e..9ae98eeada76 100644
> > --- a/drivers/acpi/Kconfig
> > +++ b/drivers/acpi/Kconfig
> > @@ -79,7 +79,12 @@ config ACPI_DEBUGGER_USER
> >  endif
> >  
> >  config ACPI_SPCR_TABLE
> > -	bool
> > +	bool "ACPI Serial Port Console Redirection Support"
> > +	default y if ARM64
> > +	help
> > +	  Enable support for Serial Port Console Redirection (SPCR) Table.
> > +	  This table provides information about the configuration of the
> > +	  earlycon console.
> >  
> >  config ACPI_LPIT
> >  	bool
> > diff --git a/drivers/acpi/spcr.c b/drivers/acpi/spcr.c
> > index 324b35bfe781..f4bb8110e404 100644
> > --- a/drivers/acpi/spcr.c
> > +++ b/drivers/acpi/spcr.c
> > @@ -16,65 +16,18 @@
> >  #include <linux/kernel.h>
> >  #include <linux/serial_core.h>
> >  
> > -/*
> > - * Erratum 44 for QDF2432v1 and QDF2400v1 SoCs describes the BUSY bit as
> > - * occasionally getting stuck as 1. To avoid the potential for a hang, check
> > - * TXFE == 0 instead of BUSY == 1. This may not be suitable for all UART
> > - * implementations, so only do so if an affected platform is detected in
> > - * parse_spcr().
> > - */
> > -bool qdf2400_e44_present;
> > -EXPORT_SYMBOL(qdf2400_e44_present);
> > -
> > -/*
> > - * Some Qualcomm Datacenter Technologies SoCs have a defective UART BUSY bit.
> > - * Detect them by examining the OEM fields in the SPCR header, similiar to PCI
> > - * quirk detection in pci_mcfg.c.
> > - */
> > -static bool qdf2400_erratum_44_present(struct acpi_table_header *h)
> > -{
> > -	if (memcmp(h->oem_id, "QCOM  ", ACPI_OEM_ID_SIZE))
> > -		return false;
> > -
> > -	if (!memcmp(h->oem_table_id, "QDF2432 ", ACPI_OEM_TABLE_ID_SIZE))
> > -		return true;
> > -
> > -	if (!memcmp(h->oem_table_id, "QDF2400 ", ACPI_OEM_TABLE_ID_SIZE) &&
> > -			h->oem_revision == 1)
> > -		return true;
> > -
> > -	return false;
> > -}
> > -
> > -/*
> > - * APM X-Gene v1 and v2 UART hardware is an 16550 like device but has its
> > - * register aligned to 32-bit. In addition, the BIOS also encoded the
> > - * access width to be 8 bits. This function detects this errata condition.
> > - */
> > -static bool xgene_8250_erratum_present(struct acpi_table_spcr *tb)
> > +int __weak acpi_arch_setup_console(struct acpi_table_spcr *table,
> > +				   char *opts, char *uart, char *iotype,
> > +				   int baud_rate, bool earlycon)
> >  {
> > -	bool xgene_8250 = false;
> > -
> > -	if (tb->interface_type != ACPI_DBG2_16550_COMPATIBLE)
> > -		return false;
> > -
> > -	if (memcmp(tb->header.oem_id, "APMC0D", ACPI_OEM_ID_SIZE) &&
> > -	    memcmp(tb->header.oem_id, "HPE   ", ACPI_OEM_ID_SIZE))
> > -		return false;
> > -
> > -	if (!memcmp(tb->header.oem_table_id, "XGENESPC",
> > -	    ACPI_OEM_TABLE_ID_SIZE) && tb->header.oem_revision == 0)
> > -		xgene_8250 = true;
> > -
> > -	if (!memcmp(tb->header.oem_table_id, "ProLiant",
> > -	    ACPI_OEM_TABLE_ID_SIZE) && tb->header.oem_revision == 1)
> > -		xgene_8250 = true;
> > -
> > -	return xgene_8250;
> > +	snprintf(opts, ACPI_SPCR_OPTS_SIZE, "%s,%s,0x%llx,%d", uart, iotype,
> > +		 table->serial_port.address, baud_rate);
> > +	return 0;
> >  }
> >  
> > +bool console_acpi_spcr_enable __initdata;
> >  /**
> > - * parse_spcr() - parse ACPI SPCR table and add preferred console
> > + * acpi_parse_spcr() - parse ACPI SPCR table and add preferred console
> >   *
> >   * @earlycon: set up earlycon for the console specified by the table
> >   *
> > @@ -86,13 +39,13 @@ static bool xgene_8250_erratum_present(struct acpi_table_spcr *tb)
> >   * from arch initialization code as soon as the DT/ACPI decision is made.
> >   *
> >   */
> > -int __init parse_spcr(bool earlycon)
> > +int __init acpi_parse_spcr(bool earlycon)
> >  {
> > -	static char opts[64];
> > +	static char opts[ACPI_SPCR_OPTS_SIZE];
> > +	static char uart[ACPI_SPCR_BUF_SIZE];
> > +	static char iotype[ACPI_SPCR_BUF_SIZE];
> >  	struct acpi_table_spcr *table;
> >  	acpi_status status;
> > -	char *uart;
> > -	char *iotype;
> >  	int baud_rate;
> >  	int err;
> >  
> > @@ -105,48 +58,6 @@ int __init parse_spcr(bool earlycon)
> >  	if (ACPI_FAILURE(status))
> >  		return -ENOENT;
> >  
> > -	if (table->header.revision < 2) {
> > -		err = -ENOENT;
> > -		pr_err("wrong table version\n");
> > -		goto done;
> > -	}
> > -
> > -	if (table->serial_port.space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY) {
> > -		switch (ACPI_ACCESS_BIT_WIDTH((
> > -			table->serial_port.access_width))) {
> > -		default:
> > -			pr_err("Unexpected SPCR Access Width.  Defaulting to byte size\n");
> > -		case 8:
> > -			iotype = "mmio";
> > -			break;
> > -		case 16:
> > -			iotype = "mmio16";
> > -			break;
> > -		case 32:
> > -			iotype = "mmio32";
> > -			break;
> > -		}
> > -	} else
> > -		iotype = "io";
> > -
> > -	switch (table->interface_type) {
> > -	case ACPI_DBG2_ARM_SBSA_32BIT:
> > -		iotype = "mmio32";
> > -		/* fall through */
> > -	case ACPI_DBG2_ARM_PL011:
> > -	case ACPI_DBG2_ARM_SBSA_GENERIC:
> > -	case ACPI_DBG2_BCM2835:
> > -		uart = "pl011";
> > -		break;
> > -	case ACPI_DBG2_16550_COMPATIBLE:
> > -	case ACPI_DBG2_16550_SUBSET:
> > -		uart = "uart";
> > -		break;
> > -	default:
> > -		err = -ENOENT;
> > -		goto done;
> > -	}
> > -
> >  	switch (table->baud_rate) {
> >  	case 3:
> >  		baud_rate = 9600;
> > @@ -165,43 +76,36 @@ int __init parse_spcr(bool earlycon)
> >  		goto done;
> >  	}
> >  
> > -	/*
> > -	 * If the E44 erratum is required, then we need to tell the pl011
> > -	 * driver to implement the work-around.
> > -	 *
> > -	 * The global variable is used by the probe function when it
> > -	 * creates the UARTs, whether or not they're used as a console.
> > -	 *
> > -	 * If the user specifies "traditional" earlycon, the qdf2400_e44
> > -	 * console name matches the EARLYCON_DECLARE() statement, and
> > -	 * SPCR is not used.  Parameter "earlycon" is false.
> > -	 *
> > -	 * If the user specifies "SPCR" earlycon, then we need to update
> > -	 * the console name so that it also says "qdf2400_e44".  Parameter
> > -	 * "earlycon" is true.
> > -	 *
> > -	 * For consistency, if we change the console name, then we do it
> > -	 * for everyone, not just earlycon.
> > -	 */
> > -	if (qdf2400_erratum_44_present(&table->header)) {
> > -		qdf2400_e44_present = true;
> > -		if (earlycon)
> > -			uart = "qdf2400_e44";
> > +	switch (table->interface_type) {
> > +	case ACPI_DBG2_16550_COMPATIBLE:
> > +	case ACPI_DBG2_16550_SUBSET:
> > +		snprintf(uart, ACPI_SPCR_BUF_SIZE, "uart");
> > +		break;
> > +	default:
> > +		break;
> >  	}
> >  
> > -	if (xgene_8250_erratum_present(table)) {
> > -		iotype = "mmio32";
> > +	if (table->serial_port.space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY) {
> > +		u8 width = ACPI_ACCESS_BIT_WIDTH((
> > +					table->serial_port.access_width));
> > +		switch (width) {
> > +		default:
> > +			pr_err("Unexpected SPCR Access Width.  Defaulting to byte size\n");
> > +		case 8:
> > +			snprintf(iotype, ACPI_SPCR_BUF_SIZE, "mmio");
> > +			break;
> > +		case 16:
> > +		case 32:
> > +			snprintf(iotype, ACPI_SPCR_BUF_SIZE, "mmio%d", width);
> > +			break;
> > +		}
> > +	} else
> > +		snprintf(iotype, ACPI_SPCR_BUF_SIZE, "io");
> >  
> > -		/* for xgene v1 and v2 we don't know the clock rate of the
> > -		 * UART so don't attempt to change to the baud rate state
> > -		 * in the table because driver cannot calculate the dividers
> > -		 */
> > -		snprintf(opts, sizeof(opts), "%s,%s,0x%llx", uart, iotype,
> > -			 table->serial_port.address);
> > -	} else {
> > -		snprintf(opts, sizeof(opts), "%s,%s,0x%llx,%d", uart, iotype,
> > -			 table->serial_port.address, baud_rate);
> > -	}
> > +	err = acpi_arch_setup_console(table, opts, uart, iotype, baud_rate,
> > +				      earlycon);
> > +	if (err)
> > +		goto done;
> >  
> >  	pr_info("console: %s\n", opts);
> >  
> > @@ -209,7 +113,6 @@ int __init parse_spcr(bool earlycon)
> >  		setup_earlycon(opts);
> >  
> >  	err = add_preferred_console(uart, 0, opts + strlen(uart) + 1);
> > -
> >  done:
> >  	acpi_put_table((struct acpi_table_header *)table);
> >  	return err;
> > diff --git a/drivers/tty/serial/earlycon.c b/drivers/tty/serial/earlycon.c
> > index 4c8b80f1c688..b22afb62c7a3 100644
> > --- a/drivers/tty/serial/earlycon.c
> > +++ b/drivers/tty/serial/earlycon.c
> > @@ -196,26 +196,15 @@ int __init setup_earlycon(char *buf)
> >  	return -ENOENT;
> >  }
> >  
> > -/*
> > - * When CONFIG_ACPI_SPCR_TABLE is defined, "earlycon" without parameters in
> > - * command line does not start DT earlycon immediately, instead it defers
> > - * starting it until DT/ACPI decision is made.  At that time if ACPI is enabled
> > - * call parse_spcr(), else call early_init_dt_scan_chosen_stdout()
> > - */
> > -bool earlycon_init_is_deferred __initdata;
> > -
> >  /* early_param wrapper for setup_earlycon() */
> >  static int __init param_setup_earlycon(char *buf)
> >  {
> >  	int err;
> >  
> > -	/*
> > -	 * Just 'earlycon' is a valid param for devicetree earlycons;
> > -	 * don't generate a warning from parse_early_params() in that case
> > -	 */
> > +	/* Just 'earlycon' is a valid param for devicetree and ACPI SPCR. */
> >  	if (!buf || !buf[0]) {
> >  		if (IS_ENABLED(CONFIG_ACPI_SPCR_TABLE)) {
> > -			earlycon_init_is_deferred = true;
> > +			console_acpi_spcr_enable = true;
> >  			return 0;
> >  		} else if (!buf) {
> >  			return early_init_dt_scan_chosen_stdout();
> > diff --git a/include/linux/acpi.h b/include/linux/acpi.h
> > index dc1ebfeeb5ec..875d7327d91c 100644
> > --- a/include/linux/acpi.h
> > +++ b/include/linux/acpi.h
> > @@ -1241,10 +1241,17 @@ static inline bool acpi_has_watchdog(void) { return false; }
> >  #endif
> >  
> >  #ifdef CONFIG_ACPI_SPCR_TABLE
> > +#define ACPI_SPCR_OPTS_SIZE 64
> > +#define ACPI_SPCR_BUF_SIZE 32
> >  extern bool qdf2400_e44_present;
> > -int parse_spcr(bool earlycon);
> > +extern bool console_acpi_spcr_enable __initdata;
> > +extern int acpi_arch_setup_console(struct acpi_table_spcr *table,
> > +				   char *opts, char *uart, char *iotype,
> > +				   int baud_rate, bool earlycon);
> > +int acpi_parse_spcr(bool earlycon);
> >  #else
> > -static inline int parse_spcr(bool earlycon) { return 0; }
> > +static const bool console_acpi_spcr_enable;
> > +static inline int acpi_parse_spcr(bool earlycon) { return 0; }
> >  #endif
> >  
> >  #if IS_ENABLED(CONFIG_ACPI_GENERIC_GSI)
> > diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
> > index 37b044e78333..abfffb4b1c37 100644
> > --- a/include/linux/serial_core.h
> > +++ b/include/linux/serial_core.h
> > @@ -376,10 +376,8 @@ extern int of_setup_earlycon(const struct earlycon_id *match,
> >  			     const char *options);
> >  
> >  #ifdef CONFIG_SERIAL_EARLYCON
> > -extern bool earlycon_init_is_deferred __initdata;
> >  int setup_earlycon(char *buf);
> >  #else
> > -static const bool earlycon_init_is_deferred;
> >  static inline int setup_earlycon(char *buf) { return 0; }
> >  #endif
> >  
> > 
> 
> 

Re: [PATCH v2 1/2] acpi, spcr: Make SPCR avialable to other architectures

[Lorenzo Pieralisi]

[+Mark, Graeme]

In $SUBJECT, s/avialable/available

On Mon, Dec 11, 2017 at 10:50:58AM -0500, Prarit Bhargava wrote:
> Other architectures can use SPCR to setup an early console or console
> but the current code is ARM64 specific.

I see nothing ARM64 specific in current code (apart from some
ACPICA macros with an ARM tag in them) please explain to me
what's preventing you to reuse current code on x86.

> Change the name of parse_spcr() to acpi_parse_spcr().  Add a weak
> function acpi_arch_setup_console() that can be used for arch-specific
> setup.  Move flags into ACPI code.  Update the Documention on the use of
> the SPCR.
> 
> [v2]: Don't return an error in the baud_rate check of acpi_parse_spcr().
> Keep ACPI_SPCR_TABLE selected for ARM64.  Fix 8-bit port access width
> mmio value.  Move baud rate check earlier.

This does not belong in the commit log.

> Signed-off-by: Prarit Bhargava <prarit@redhat.com>
> Cc: linux-doc@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-pm@vger.kernel.org
> Cc: linux-acpi@vger.kernel.org
> Cc: linux-serial@vger.kernel.org
> Cc: Bhupesh Sharma <bhsharma@redhat.com>
> Cc: Lv Zheng <lv.zheng@intel.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: "H. Peter Anvin" <hpa@zytor.com>
> Cc: x86@kernel.org
> Cc: Jonathan Corbet <corbet@lwn.net>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> Cc: Timur Tabi <timur@codeaurora.org>
> ---
>  Documentation/admin-guide/kernel-parameters.txt |   6 +-
>  arch/arm64/kernel/acpi.c                        | 128 ++++++++++++++++-
>  drivers/acpi/Kconfig                            |   7 +-
>  drivers/acpi/spcr.c                             | 175 ++++++------------------
>  drivers/tty/serial/earlycon.c                   |  15 +-
>  include/linux/acpi.h                            |  11 +-
>  include/linux/serial_core.h                     |   2 -
>  7 files changed, 184 insertions(+), 160 deletions(-)
> 
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index 6571fbfdb2a1..0d173289c67e 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -914,9 +914,9 @@
>  
>  	earlycon=	[KNL] Output early console device and options.
>  
> -			When used with no options, the early console is
> -			determined by the stdout-path property in device
> -			tree's chosen node.
> +			[ARM64] The early console is determined by the
> +			stdout-path property in device tree's chosen node,
> +			or determined by the ACPI SPCR table.
>  
>  		cdns,<addr>[,options]
>  			Start an early, polled-mode console on a Cadence
> diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c
> index b3162715ed78..b3e33bbdf3b7 100644
> --- a/arch/arm64/kernel/acpi.c
> +++ b/arch/arm64/kernel/acpi.c
> @@ -25,7 +25,6 @@
>  #include <linux/memblock.h>
>  #include <linux/of_fdt.h>
>  #include <linux/smp.h>
> -#include <linux/serial_core.h>
>  
>  #include <asm/cputype.h>
>  #include <asm/cpu_ops.h>
> @@ -177,6 +176,128 @@ static int __init acpi_fadt_sanity_check(void)
>  	return ret;
>  }
>  
> +/*
> + * Erratum 44 for QDF2432v1 and QDF2400v1 SoCs describes the BUSY bit as
> + * occasionally getting stuck as 1. To avoid the potential for a hang, check
> + * TXFE == 0 instead of BUSY == 1. This may not be suitable for all UART
> + * implementations, so only do so if an affected platform is detected in
> + * acpi_parse_spcr().
> + */
> +bool qdf2400_e44_present;
> +EXPORT_SYMBOL(qdf2400_e44_present);

My eyes, this is horrible but it is not introduced by this patch. It
would have been much better if:

drivers/tty/serial/amba-pl011.c

parsed the SPCR table (again) to detect it instead of relying on this
horrible exported flag.

> +
> +/*
> + * Some Qualcomm Datacenter Technologies SoCs have a defective UART BUSY bit.
> + * Detect them by examining the OEM fields in the SPCR header, similar to PCI
> + * quirk detection in pci_mcfg.c.
> + */
> +static bool qdf2400_erratum_44_present(struct acpi_table_header *h)
> +{
> +	if (memcmp(h->oem_id, "QCOM  ", ACPI_OEM_ID_SIZE))
> +		return false;
> +
> +	if (!memcmp(h->oem_table_id, "QDF2432 ", ACPI_OEM_TABLE_ID_SIZE))
> +		return true;
> +
> +	if (!memcmp(h->oem_table_id, "QDF2400 ", ACPI_OEM_TABLE_ID_SIZE) &&
> +			h->oem_revision == 1)
> +		return true;
> +
> +	return false;
> +}
> +
> +/*
> + * APM X-Gene v1 and v2 UART hardware is an 16550 like device but has its
> + * register aligned to 32-bit. In addition, the BIOS also encoded the
> + * access width to be 8 bits. This function detects this errata condition.
> + */
> +static bool xgene_8250_erratum_present(struct acpi_table_spcr *tb)
> +{
> +	bool xgene_8250 = false;
> +
> +	if (tb->interface_type != ACPI_DBG2_16550_COMPATIBLE)
> +		return false;
> +
> +	if (memcmp(tb->header.oem_id, "APMC0D", ACPI_OEM_ID_SIZE) &&
> +	    memcmp(tb->header.oem_id, "HPE   ", ACPI_OEM_ID_SIZE))
> +		return false;
> +
> +	if (!memcmp(tb->header.oem_table_id, "XGENESPC",
> +	    ACPI_OEM_TABLE_ID_SIZE) && tb->header.oem_revision == 0)
> +		xgene_8250 = true;
> +
> +	if (!memcmp(tb->header.oem_table_id, "ProLiant",
> +	    ACPI_OEM_TABLE_ID_SIZE) && tb->header.oem_revision == 1)
> +		xgene_8250 = true;
> +
> +	return xgene_8250;
> +}
> +
> +int acpi_arch_setup_console(struct acpi_table_spcr *table,
> +			    char *opts, char *uart, char *iotype,
> +			    int baud_rate, bool earlycon)
> +{
> +	if (table->header.revision < 2) {
> +		pr_err("wrong table version\n");
> +		return -ENOENT;
> +	}
> +
> +	switch (table->interface_type) {
> +	case ACPI_DBG2_ARM_SBSA_32BIT:
> +		snprintf(iotype, ACPI_SPCR_BUF_SIZE, "mmio32");
> +		/* fall through */
> +	case ACPI_DBG2_ARM_PL011:
> +	case ACPI_DBG2_ARM_SBSA_GENERIC:
> +	case ACPI_DBG2_BCM2835:
> +		snprintf(uart, ACPI_SPCR_BUF_SIZE, "pl011");
> +		break;
> +	default:
> +		if (strlen(uart) == 0)
> +			return -ENOENT;
> +	}
> +
> +	/*
> +	 * If the E44 erratum is required, then we need to tell the pl011
> +	 * driver to implement the work-around.
> +	 *
> +	 * The global variable is used by the probe function when it
> +	 * creates the UARTs, whether or not they're used as a console.
> +	 *
> +	 * If the user specifies "traditional" earlycon, the qdf2400_e44
> +	 * console name matches the EARLYCON_DECLARE() statement, and
> +	 * SPCR is not used.  Parameter "earlycon" is false.
> +	 *
> +	 * If the user specifies "SPCR" earlycon, then we need to update
> +	 * the console name so that it also says "qdf2400_e44".  Parameter
> +	 * "earlycon" is true.
> +	 *
> +	 * For consistency, if we change the console name, then we do it
> +	 * for everyone, not just earlycon.
> +	 */
> +	if (qdf2400_erratum_44_present(&table->header)) {
> +		qdf2400_e44_present = true;
> +		if (earlycon)
> +			snprintf(uart, ACPI_SPCR_BUF_SIZE, "qdf2400_e44");
> +	}
> +
> +	if (xgene_8250_erratum_present(table)) {
> +		snprintf(iotype, ACPI_SPCR_BUF_SIZE, "mmio32");
> +
> +		/* for xgene v1 and v2 we don't know the clock rate of the
> +		 * UART so don't attempt to change to the baud rate state
> +		 * in the table because driver cannot calculate the dividers
> +		 */
> +		snprintf(opts, ACPI_SPCR_OPTS_SIZE, "%s,%s,0x%llx", uart,
> +			 iotype, table->serial_port.address);
> +	} else {
> +		snprintf(opts, ACPI_SPCR_OPTS_SIZE, "%s,%s,0x%llx,%d", uart,
> +			 iotype, table->serial_port.address, baud_rate);
> +	}
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(acpi_arch_setup_console);

EXPORT_SYMBOL() ? Why ?

BTW, why do we need an arch hook ? I do not see anything that prevents
you from using this code on x86 systems - is there anything arch
specific in the SPCR specification itself ?

> +
>  /*
>   * acpi_boot_table_init() called from setup_arch(), always.
>   *	1. find RSDP and get its address, and then find XSDT
> @@ -230,10 +351,11 @@ void __init acpi_boot_table_init(void)
>  
>  done:
>  	if (acpi_disabled) {
> -		if (earlycon_init_is_deferred)
> +		if (console_acpi_spcr_enable)
>  			early_init_dt_scan_chosen_stdout();
>  	} else {
> -		parse_spcr(earlycon_init_is_deferred);
> +		/* Always enable the ACPI SPCR console */
> +		acpi_parse_spcr(console_acpi_spcr_enable);
>  		if (IS_ENABLED(CONFIG_ACPI_BGRT))
>  			acpi_table_parse(ACPI_SIG_BGRT, acpi_parse_bgrt);
>  	}
> diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
> index 46505396869e..9ae98eeada76 100644
> --- a/drivers/acpi/Kconfig
> +++ b/drivers/acpi/Kconfig
> @@ -79,7 +79,12 @@ config ACPI_DEBUGGER_USER
>  endif
>  
>  config ACPI_SPCR_TABLE
> -	bool
> +	bool "ACPI Serial Port Console Redirection Support"
> +	default y if ARM64

You need to remove the selection in arch/arm64 then. Also, moving away
from a non-visible config may have consequences on ARM64, Graeme and
Mark are more familiar with the SPCR dependencies so please chime in.

> +	help
> +	  Enable support for Serial Port Console Redirection (SPCR) Table.
> +	  This table provides information about the configuration of the
> +	  earlycon console.
>  
>  config ACPI_LPIT
>  	bool
> diff --git a/drivers/acpi/spcr.c b/drivers/acpi/spcr.c
> index 324b35bfe781..f4bb8110e404 100644
> --- a/drivers/acpi/spcr.c
> +++ b/drivers/acpi/spcr.c
> @@ -16,65 +16,18 @@
>  #include <linux/kernel.h>
>  #include <linux/serial_core.h>
>  
> -/*
> - * Erratum 44 for QDF2432v1 and QDF2400v1 SoCs describes the BUSY bit as
> - * occasionally getting stuck as 1. To avoid the potential for a hang, check
> - * TXFE == 0 instead of BUSY == 1. This may not be suitable for all UART
> - * implementations, so only do so if an affected platform is detected in
> - * parse_spcr().
> - */
> -bool qdf2400_e44_present;
> -EXPORT_SYMBOL(qdf2400_e44_present);
> -
> -/*
> - * Some Qualcomm Datacenter Technologies SoCs have a defective UART BUSY bit.
> - * Detect them by examining the OEM fields in the SPCR header, similiar to PCI
> - * quirk detection in pci_mcfg.c.
> - */
> -static bool qdf2400_erratum_44_present(struct acpi_table_header *h)
> -{
> -	if (memcmp(h->oem_id, "QCOM  ", ACPI_OEM_ID_SIZE))
> -		return false;
> -
> -	if (!memcmp(h->oem_table_id, "QDF2432 ", ACPI_OEM_TABLE_ID_SIZE))
> -		return true;
> -
> -	if (!memcmp(h->oem_table_id, "QDF2400 ", ACPI_OEM_TABLE_ID_SIZE) &&
> -			h->oem_revision == 1)
> -		return true;
> -
> -	return false;
> -}
> -
> -/*
> - * APM X-Gene v1 and v2 UART hardware is an 16550 like device but has its
> - * register aligned to 32-bit. In addition, the BIOS also encoded the
> - * access width to be 8 bits. This function detects this errata condition.
> - */
> -static bool xgene_8250_erratum_present(struct acpi_table_spcr *tb)
> +int __weak acpi_arch_setup_console(struct acpi_table_spcr *table,
> +				   char *opts, char *uart, char *iotype,
> +				   int baud_rate, bool earlycon)
>  {
> -	bool xgene_8250 = false;
> -
> -	if (tb->interface_type != ACPI_DBG2_16550_COMPATIBLE)
> -		return false;
> -
> -	if (memcmp(tb->header.oem_id, "APMC0D", ACPI_OEM_ID_SIZE) &&
> -	    memcmp(tb->header.oem_id, "HPE   ", ACPI_OEM_ID_SIZE))
> -		return false;
> -
> -	if (!memcmp(tb->header.oem_table_id, "XGENESPC",
> -	    ACPI_OEM_TABLE_ID_SIZE) && tb->header.oem_revision == 0)
> -		xgene_8250 = true;
> -
> -	if (!memcmp(tb->header.oem_table_id, "ProLiant",
> -	    ACPI_OEM_TABLE_ID_SIZE) && tb->header.oem_revision == 1)
> -		xgene_8250 = true;
> -
> -	return xgene_8250;
> +	snprintf(opts, ACPI_SPCR_OPTS_SIZE, "%s,%s,0x%llx,%d", uart, iotype,
> +		 table->serial_port.address, baud_rate);
> +	return 0;
>  }
>  
> +bool console_acpi_spcr_enable __initdata;
>  /**
> - * parse_spcr() - parse ACPI SPCR table and add preferred console
> + * acpi_parse_spcr() - parse ACPI SPCR table and add preferred console
>   *
>   * @earlycon: set up earlycon for the console specified by the table
>   *
> @@ -86,13 +39,13 @@ static bool xgene_8250_erratum_present(struct acpi_table_spcr *tb)
>   * from arch initialization code as soon as the DT/ACPI decision is made.
>   *
>   */
> -int __init parse_spcr(bool earlycon)
> +int __init acpi_parse_spcr(bool earlycon)
>  {
> -	static char opts[64];
> +	static char opts[ACPI_SPCR_OPTS_SIZE];
> +	static char uart[ACPI_SPCR_BUF_SIZE];
> +	static char iotype[ACPI_SPCR_BUF_SIZE];
>  	struct acpi_table_spcr *table;
>  	acpi_status status;
> -	char *uart;
> -	char *iotype;
>  	int baud_rate;
>  	int err;
>  
> @@ -105,48 +58,6 @@ int __init parse_spcr(bool earlycon)
>  	if (ACPI_FAILURE(status))
>  		return -ENOENT;
>  
> -	if (table->header.revision < 2) {
> -		err = -ENOENT;
> -		pr_err("wrong table version\n");
> -		goto done;
> -	}
> -
> -	if (table->serial_port.space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY) {
> -		switch (ACPI_ACCESS_BIT_WIDTH((
> -			table->serial_port.access_width))) {
> -		default:
> -			pr_err("Unexpected SPCR Access Width.  Defaulting to byte size\n");
> -		case 8:
> -			iotype = "mmio";
> -			break;
> -		case 16:
> -			iotype = "mmio16";
> -			break;
> -		case 32:
> -			iotype = "mmio32";
> -			break;
> -		}
> -	} else
> -		iotype = "io";
> -
> -	switch (table->interface_type) {
> -	case ACPI_DBG2_ARM_SBSA_32BIT:
> -		iotype = "mmio32";
> -		/* fall through */
> -	case ACPI_DBG2_ARM_PL011:
> -	case ACPI_DBG2_ARM_SBSA_GENERIC:
> -	case ACPI_DBG2_BCM2835:
> -		uart = "pl011";
> -		break;
> -	case ACPI_DBG2_16550_COMPATIBLE:
> -	case ACPI_DBG2_16550_SUBSET:
> -		uart = "uart";
> -		break;
> -	default:
> -		err = -ENOENT;
> -		goto done;
> -	}
> -
>  	switch (table->baud_rate) {
>  	case 3:
>  		baud_rate = 9600;
> @@ -165,43 +76,36 @@ int __init parse_spcr(bool earlycon)
>  		goto done;
>  	}
>  
> -	/*
> -	 * If the E44 erratum is required, then we need to tell the pl011
> -	 * driver to implement the work-around.
> -	 *
> -	 * The global variable is used by the probe function when it
> -	 * creates the UARTs, whether or not they're used as a console.
> -	 *
> -	 * If the user specifies "traditional" earlycon, the qdf2400_e44
> -	 * console name matches the EARLYCON_DECLARE() statement, and
> -	 * SPCR is not used.  Parameter "earlycon" is false.
> -	 *
> -	 * If the user specifies "SPCR" earlycon, then we need to update
> -	 * the console name so that it also says "qdf2400_e44".  Parameter
> -	 * "earlycon" is true.
> -	 *
> -	 * For consistency, if we change the console name, then we do it
> -	 * for everyone, not just earlycon.
> -	 */
> -	if (qdf2400_erratum_44_present(&table->header)) {
> -		qdf2400_e44_present = true;
> -		if (earlycon)
> -			uart = "qdf2400_e44";
> +	switch (table->interface_type) {
> +	case ACPI_DBG2_16550_COMPATIBLE:
> +	case ACPI_DBG2_16550_SUBSET:
> +		snprintf(uart, ACPI_SPCR_BUF_SIZE, "uart");
> +		break;
> +	default:
> +		break;
>  	}
>  
> -	if (xgene_8250_erratum_present(table)) {
> -		iotype = "mmio32";
> +	if (table->serial_port.space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY) {
> +		u8 width = ACPI_ACCESS_BIT_WIDTH((
> +					table->serial_port.access_width));
> +		switch (width) {
> +		default:
> +			pr_err("Unexpected SPCR Access Width.  Defaulting to byte size\n");
> +		case 8:
> +			snprintf(iotype, ACPI_SPCR_BUF_SIZE, "mmio");
> +			break;
> +		case 16:
> +		case 32:
> +			snprintf(iotype, ACPI_SPCR_BUF_SIZE, "mmio%d", width);
> +			break;
> +		}
> +	} else
> +		snprintf(iotype, ACPI_SPCR_BUF_SIZE, "io");
>  
> -		/* for xgene v1 and v2 we don't know the clock rate of the
> -		 * UART so don't attempt to change to the baud rate state
> -		 * in the table because driver cannot calculate the dividers
> -		 */
> -		snprintf(opts, sizeof(opts), "%s,%s,0x%llx", uart, iotype,
> -			 table->serial_port.address);
> -	} else {
> -		snprintf(opts, sizeof(opts), "%s,%s,0x%llx,%d", uart, iotype,
> -			 table->serial_port.address, baud_rate);
> -	}
> +	err = acpi_arch_setup_console(table, opts, uart, iotype, baud_rate,
> +				      earlycon);
> +	if (err)
> +		goto done;
>  
>  	pr_info("console: %s\n", opts);
>  
> @@ -209,7 +113,6 @@ int __init parse_spcr(bool earlycon)
>  		setup_earlycon(opts);
>  
>  	err = add_preferred_console(uart, 0, opts + strlen(uart) + 1);
> -

Unintended change.

>  done:
>  	acpi_put_table((struct acpi_table_header *)table);
>  	return err;
> diff --git a/drivers/tty/serial/earlycon.c b/drivers/tty/serial/earlycon.c
> index 4c8b80f1c688..b22afb62c7a3 100644
> --- a/drivers/tty/serial/earlycon.c
> +++ b/drivers/tty/serial/earlycon.c
> @@ -196,26 +196,15 @@ int __init setup_earlycon(char *buf)
>  	return -ENOENT;
>  }
>  
> -/*
> - * When CONFIG_ACPI_SPCR_TABLE is defined, "earlycon" without parameters in
> - * command line does not start DT earlycon immediately, instead it defers
> - * starting it until DT/ACPI decision is made.  At that time if ACPI is enabled
> - * call parse_spcr(), else call early_init_dt_scan_chosen_stdout()
> - */
> -bool earlycon_init_is_deferred __initdata;
> -
>  /* early_param wrapper for setup_earlycon() */
>  static int __init param_setup_earlycon(char *buf)
>  {
>  	int err;
>  
> -	/*
> -	 * Just 'earlycon' is a valid param for devicetree earlycons;
> -	 * don't generate a warning from parse_early_params() in that case
> -	 */
> +	/* Just 'earlycon' is a valid param for devicetree and ACPI SPCR. */
>  	if (!buf || !buf[0]) {
>  		if (IS_ENABLED(CONFIG_ACPI_SPCR_TABLE)) {
> -			earlycon_init_is_deferred = true;
> +			console_acpi_spcr_enable = true;

I am not familiar with this code, I would ask Graeme and Mark to check
if this change is correct, the logic seems correct to me but I may be
missing some corner cases.

>  			return 0;
>  		} else if (!buf) {
>  			return early_init_dt_scan_chosen_stdout();
> diff --git a/include/linux/acpi.h b/include/linux/acpi.h
> index dc1ebfeeb5ec..875d7327d91c 100644
> --- a/include/linux/acpi.h
> +++ b/include/linux/acpi.h
> @@ -1241,10 +1241,17 @@ static inline bool acpi_has_watchdog(void) { return false; }
>  #endif
>  
>  #ifdef CONFIG_ACPI_SPCR_TABLE
> +#define ACPI_SPCR_OPTS_SIZE 64
> +#define ACPI_SPCR_BUF_SIZE 32
>  extern bool qdf2400_e44_present;
> -int parse_spcr(bool earlycon);
> +extern bool console_acpi_spcr_enable __initdata;
> +extern int acpi_arch_setup_console(struct acpi_table_spcr *table,
> +				   char *opts, char *uart, char *iotype,
> +				   int baud_rate, bool earlycon);
> +int acpi_parse_spcr(bool earlycon);
>  #else
> -static inline int parse_spcr(bool earlycon) { return 0; }
> +static const bool console_acpi_spcr_enable;

The assignment in param_setup_earlycon won't compile.

Lorenzo

[PATCH v2 1/2] acpi, spcr: Make SPCR avialable to other architectures

[Lorenzo Pieralisi]

[+Mark, Graeme]

In $SUBJECT, s/avialable/available

On Mon, Dec 11, 2017 at 10:50:58AM -0500, Prarit Bhargava wrote:
> Other architectures can use SPCR to setup an early console or console
> but the current code is ARM64 specific.

I see nothing ARM64 specific in current code (apart from some
ACPICA macros with an ARM tag in them) please explain to me
what's preventing you to reuse current code on x86.

> Change the name of parse_spcr() to acpi_parse_spcr().  Add a weak
> function acpi_arch_setup_console() that can be used for arch-specific
> setup.  Move flags into ACPI code.  Update the Documention on the use of
> the SPCR.
> 
> [v2]: Don't return an error in the baud_rate check of acpi_parse_spcr().
> Keep ACPI_SPCR_TABLE selected for ARM64.  Fix 8-bit port access width
> mmio value.  Move baud rate check earlier.

This does not belong in the commit log.

> Signed-off-by: Prarit Bhargava <prarit@redhat.com>
> Cc: linux-doc at vger.kernel.org
> Cc: linux-kernel at vger.kernel.org
> Cc: linux-arm-kernel at lists.infradead.org
> Cc: linux-pm at vger.kernel.org
> Cc: linux-acpi at vger.kernel.org
> Cc: linux-serial at vger.kernel.org
> Cc: Bhupesh Sharma <bhsharma@redhat.com>
> Cc: Lv Zheng <lv.zheng@intel.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: "H. Peter Anvin" <hpa@zytor.com>
> Cc: x86 at kernel.org
> Cc: Jonathan Corbet <corbet@lwn.net>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> Cc: Timur Tabi <timur@codeaurora.org>
> ---
>  Documentation/admin-guide/kernel-parameters.txt |   6 +-
>  arch/arm64/kernel/acpi.c                        | 128 ++++++++++++++++-
>  drivers/acpi/Kconfig                            |   7 +-
>  drivers/acpi/spcr.c                             | 175 ++++++------------------
>  drivers/tty/serial/earlycon.c                   |  15 +-
>  include/linux/acpi.h                            |  11 +-
>  include/linux/serial_core.h                     |   2 -
>  7 files changed, 184 insertions(+), 160 deletions(-)
> 
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index 6571fbfdb2a1..0d173289c67e 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -914,9 +914,9 @@
>  
>  	earlycon=	[KNL] Output early console device and options.
>  
> -			When used with no options, the early console is
> -			determined by the stdout-path property in device
> -			tree's chosen node.
> +			[ARM64] The early console is determined by the
> +			stdout-path property in device tree's chosen node,
> +			or determined by the ACPI SPCR table.
>  
>  		cdns,<addr>[,options]
>  			Start an early, polled-mode console on a Cadence
> diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c
> index b3162715ed78..b3e33bbdf3b7 100644
> --- a/arch/arm64/kernel/acpi.c
> +++ b/arch/arm64/kernel/acpi.c
> @@ -25,7 +25,6 @@
>  #include <linux/memblock.h>
>  #include <linux/of_fdt.h>
>  #include <linux/smp.h>
> -#include <linux/serial_core.h>
>  
>  #include <asm/cputype.h>
>  #include <asm/cpu_ops.h>
> @@ -177,6 +176,128 @@ static int __init acpi_fadt_sanity_check(void)
>  	return ret;
>  }
>  
> +/*
> + * Erratum 44 for QDF2432v1 and QDF2400v1 SoCs describes the BUSY bit as
> + * occasionally getting stuck as 1. To avoid the potential for a hang, check
> + * TXFE == 0 instead of BUSY == 1. This may not be suitable for all UART
> + * implementations, so only do so if an affected platform is detected in
> + * acpi_parse_spcr().
> + */
> +bool qdf2400_e44_present;
> +EXPORT_SYMBOL(qdf2400_e44_present);

My eyes, this is horrible but it is not introduced by this patch. It
would have been much better if:

drivers/tty/serial/amba-pl011.c

parsed the SPCR table (again) to detect it instead of relying on this
horrible exported flag.

> +
> +/*
> + * Some Qualcomm Datacenter Technologies SoCs have a defective UART BUSY bit.
> + * Detect them by examining the OEM fields in the SPCR header, similar to PCI
> + * quirk detection in pci_mcfg.c.
> + */
> +static bool qdf2400_erratum_44_present(struct acpi_table_header *h)
> +{
> +	if (memcmp(h->oem_id, "QCOM  ", ACPI_OEM_ID_SIZE))
> +		return false;
> +
> +	if (!memcmp(h->oem_table_id, "QDF2432 ", ACPI_OEM_TABLE_ID_SIZE))
> +		return true;
> +
> +	if (!memcmp(h->oem_table_id, "QDF2400 ", ACPI_OEM_TABLE_ID_SIZE) &&
> +			h->oem_revision == 1)
> +		return true;
> +
> +	return false;
> +}
> +
> +/*
> + * APM X-Gene v1 and v2 UART hardware is an 16550 like device but has its
> + * register aligned to 32-bit. In addition, the BIOS also encoded the
> + * access width to be 8 bits. This function detects this errata condition.
> + */
> +static bool xgene_8250_erratum_present(struct acpi_table_spcr *tb)
> +{
> +	bool xgene_8250 = false;
> +
> +	if (tb->interface_type != ACPI_DBG2_16550_COMPATIBLE)
> +		return false;
> +
> +	if (memcmp(tb->header.oem_id, "APMC0D", ACPI_OEM_ID_SIZE) &&
> +	    memcmp(tb->header.oem_id, "HPE   ", ACPI_OEM_ID_SIZE))
> +		return false;
> +
> +	if (!memcmp(tb->header.oem_table_id, "XGENESPC",
> +	    ACPI_OEM_TABLE_ID_SIZE) && tb->header.oem_revision == 0)
> +		xgene_8250 = true;
> +
> +	if (!memcmp(tb->header.oem_table_id, "ProLiant",
> +	    ACPI_OEM_TABLE_ID_SIZE) && tb->header.oem_revision == 1)
> +		xgene_8250 = true;
> +
> +	return xgene_8250;
> +}
> +
> +int acpi_arch_setup_console(struct acpi_table_spcr *table,
> +			    char *opts, char *uart, char *iotype,
> +			    int baud_rate, bool earlycon)
> +{
> +	if (table->header.revision < 2) {
> +		pr_err("wrong table version\n");
> +		return -ENOENT;
> +	}
> +
> +	switch (table->interface_type) {
> +	case ACPI_DBG2_ARM_SBSA_32BIT:
> +		snprintf(iotype, ACPI_SPCR_BUF_SIZE, "mmio32");
> +		/* fall through */
> +	case ACPI_DBG2_ARM_PL011:
> +	case ACPI_DBG2_ARM_SBSA_GENERIC:
> +	case ACPI_DBG2_BCM2835:
> +		snprintf(uart, ACPI_SPCR_BUF_SIZE, "pl011");
> +		break;
> +	default:
> +		if (strlen(uart) == 0)
> +			return -ENOENT;
> +	}
> +
> +	/*
> +	 * If the E44 erratum is required, then we need to tell the pl011
> +	 * driver to implement the work-around.
> +	 *
> +	 * The global variable is used by the probe function when it
> +	 * creates the UARTs, whether or not they're used as a console.
> +	 *
> +	 * If the user specifies "traditional" earlycon, the qdf2400_e44
> +	 * console name matches the EARLYCON_DECLARE() statement, and
> +	 * SPCR is not used.  Parameter "earlycon" is false.
> +	 *
> +	 * If the user specifies "SPCR" earlycon, then we need to update
> +	 * the console name so that it also says "qdf2400_e44".  Parameter
> +	 * "earlycon" is true.
> +	 *
> +	 * For consistency, if we change the console name, then we do it
> +	 * for everyone, not just earlycon.
> +	 */
> +	if (qdf2400_erratum_44_present(&table->header)) {
> +		qdf2400_e44_present = true;
> +		if (earlycon)
> +			snprintf(uart, ACPI_SPCR_BUF_SIZE, "qdf2400_e44");
> +	}
> +
> +	if (xgene_8250_erratum_present(table)) {
> +		snprintf(iotype, ACPI_SPCR_BUF_SIZE, "mmio32");
> +
> +		/* for xgene v1 and v2 we don't know the clock rate of the
> +		 * UART so don't attempt to change to the baud rate state
> +		 * in the table because driver cannot calculate the dividers
> +		 */
> +		snprintf(opts, ACPI_SPCR_OPTS_SIZE, "%s,%s,0x%llx", uart,
> +			 iotype, table->serial_port.address);
> +	} else {
> +		snprintf(opts, ACPI_SPCR_OPTS_SIZE, "%s,%s,0x%llx,%d", uart,
> +			 iotype, table->serial_port.address, baud_rate);
> +	}
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(acpi_arch_setup_console);

EXPORT_SYMBOL() ? Why ?

BTW, why do we need an arch hook ? I do not see anything that prevents
you from using this code on x86 systems - is there anything arch
specific in the SPCR specification itself ?

> +
>  /*
>   * acpi_boot_table_init() called from setup_arch(), always.
>   *	1. find RSDP and get its address, and then find XSDT
> @@ -230,10 +351,11 @@ void __init acpi_boot_table_init(void)
>  
>  done:
>  	if (acpi_disabled) {
> -		if (earlycon_init_is_deferred)
> +		if (console_acpi_spcr_enable)
>  			early_init_dt_scan_chosen_stdout();
>  	} else {
> -		parse_spcr(earlycon_init_is_deferred);
> +		/* Always enable the ACPI SPCR console */
> +		acpi_parse_spcr(console_acpi_spcr_enable);
>  		if (IS_ENABLED(CONFIG_ACPI_BGRT))
>  			acpi_table_parse(ACPI_SIG_BGRT, acpi_parse_bgrt);
>  	}
> diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
> index 46505396869e..9ae98eeada76 100644
> --- a/drivers/acpi/Kconfig
> +++ b/drivers/acpi/Kconfig
> @@ -79,7 +79,12 @@ config ACPI_DEBUGGER_USER
>  endif
>  
>  config ACPI_SPCR_TABLE
> -	bool
> +	bool "ACPI Serial Port Console Redirection Support"
> +	default y if ARM64

You need to remove the selection in arch/arm64 then. Also, moving away
from a non-visible config may have consequences on ARM64, Graeme and
Mark are more familiar with the SPCR dependencies so please chime in.

> +	help
> +	  Enable support for Serial Port Console Redirection (SPCR) Table.
> +	  This table provides information about the configuration of the
> +	  earlycon console.
>  
>  config ACPI_LPIT
>  	bool
> diff --git a/drivers/acpi/spcr.c b/drivers/acpi/spcr.c
> index 324b35bfe781..f4bb8110e404 100644
> --- a/drivers/acpi/spcr.c
> +++ b/drivers/acpi/spcr.c
> @@ -16,65 +16,18 @@
>  #include <linux/kernel.h>
>  #include <linux/serial_core.h>
>  
> -/*
> - * Erratum 44 for QDF2432v1 and QDF2400v1 SoCs describes the BUSY bit as
> - * occasionally getting stuck as 1. To avoid the potential for a hang, check
> - * TXFE == 0 instead of BUSY == 1. This may not be suitable for all UART
> - * implementations, so only do so if an affected platform is detected in
> - * parse_spcr().
> - */
> -bool qdf2400_e44_present;
> -EXPORT_SYMBOL(qdf2400_e44_present);
> -
> -/*
> - * Some Qualcomm Datacenter Technologies SoCs have a defective UART BUSY bit.
> - * Detect them by examining the OEM fields in the SPCR header, similiar to PCI
> - * quirk detection in pci_mcfg.c.
> - */
> -static bool qdf2400_erratum_44_present(struct acpi_table_header *h)
> -{
> -	if (memcmp(h->oem_id, "QCOM  ", ACPI_OEM_ID_SIZE))
> -		return false;
> -
> -	if (!memcmp(h->oem_table_id, "QDF2432 ", ACPI_OEM_TABLE_ID_SIZE))
> -		return true;
> -
> -	if (!memcmp(h->oem_table_id, "QDF2400 ", ACPI_OEM_TABLE_ID_SIZE) &&
> -			h->oem_revision == 1)
> -		return true;
> -
> -	return false;
> -}
> -
> -/*
> - * APM X-Gene v1 and v2 UART hardware is an 16550 like device but has its
> - * register aligned to 32-bit. In addition, the BIOS also encoded the
> - * access width to be 8 bits. This function detects this errata condition.
> - */
> -static bool xgene_8250_erratum_present(struct acpi_table_spcr *tb)
> +int __weak acpi_arch_setup_console(struct acpi_table_spcr *table,
> +				   char *opts, char *uart, char *iotype,
> +				   int baud_rate, bool earlycon)
>  {
> -	bool xgene_8250 = false;
> -
> -	if (tb->interface_type != ACPI_DBG2_16550_COMPATIBLE)
> -		return false;
> -
> -	if (memcmp(tb->header.oem_id, "APMC0D", ACPI_OEM_ID_SIZE) &&
> -	    memcmp(tb->header.oem_id, "HPE   ", ACPI_OEM_ID_SIZE))
> -		return false;
> -
> -	if (!memcmp(tb->header.oem_table_id, "XGENESPC",
> -	    ACPI_OEM_TABLE_ID_SIZE) && tb->header.oem_revision == 0)
> -		xgene_8250 = true;
> -
> -	if (!memcmp(tb->header.oem_table_id, "ProLiant",
> -	    ACPI_OEM_TABLE_ID_SIZE) && tb->header.oem_revision == 1)
> -		xgene_8250 = true;
> -
> -	return xgene_8250;
> +	snprintf(opts, ACPI_SPCR_OPTS_SIZE, "%s,%s,0x%llx,%d", uart, iotype,
> +		 table->serial_port.address, baud_rate);
> +	return 0;
>  }
>  
> +bool console_acpi_spcr_enable __initdata;
>  /**
> - * parse_spcr() - parse ACPI SPCR table and add preferred console
> + * acpi_parse_spcr() - parse ACPI SPCR table and add preferred console
>   *
>   * @earlycon: set up earlycon for the console specified by the table
>   *
> @@ -86,13 +39,13 @@ static bool xgene_8250_erratum_present(struct acpi_table_spcr *tb)
>   * from arch initialization code as soon as the DT/ACPI decision is made.
>   *
>   */
> -int __init parse_spcr(bool earlycon)
> +int __init acpi_parse_spcr(bool earlycon)
>  {
> -	static char opts[64];
> +	static char opts[ACPI_SPCR_OPTS_SIZE];
> +	static char uart[ACPI_SPCR_BUF_SIZE];
> +	static char iotype[ACPI_SPCR_BUF_SIZE];
>  	struct acpi_table_spcr *table;
>  	acpi_status status;
> -	char *uart;
> -	char *iotype;
>  	int baud_rate;
>  	int err;
>  
> @@ -105,48 +58,6 @@ int __init parse_spcr(bool earlycon)
>  	if (ACPI_FAILURE(status))
>  		return -ENOENT;
>  
> -	if (table->header.revision < 2) {
> -		err = -ENOENT;
> -		pr_err("wrong table version\n");
> -		goto done;
> -	}
> -
> -	if (table->serial_port.space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY) {
> -		switch (ACPI_ACCESS_BIT_WIDTH((
> -			table->serial_port.access_width))) {
> -		default:
> -			pr_err("Unexpected SPCR Access Width.  Defaulting to byte size\n");
> -		case 8:
> -			iotype = "mmio";
> -			break;
> -		case 16:
> -			iotype = "mmio16";
> -			break;
> -		case 32:
> -			iotype = "mmio32";
> -			break;
> -		}
> -	} else
> -		iotype = "io";
> -
> -	switch (table->interface_type) {
> -	case ACPI_DBG2_ARM_SBSA_32BIT:
> -		iotype = "mmio32";
> -		/* fall through */
> -	case ACPI_DBG2_ARM_PL011:
> -	case ACPI_DBG2_ARM_SBSA_GENERIC:
> -	case ACPI_DBG2_BCM2835:
> -		uart = "pl011";
> -		break;
> -	case ACPI_DBG2_16550_COMPATIBLE:
> -	case ACPI_DBG2_16550_SUBSET:
> -		uart = "uart";
> -		break;
> -	default:
> -		err = -ENOENT;
> -		goto done;
> -	}
> -
>  	switch (table->baud_rate) {
>  	case 3:
>  		baud_rate = 9600;
> @@ -165,43 +76,36 @@ int __init parse_spcr(bool earlycon)
>  		goto done;
>  	}
>  
> -	/*
> -	 * If the E44 erratum is required, then we need to tell the pl011
> -	 * driver to implement the work-around.
> -	 *
> -	 * The global variable is used by the probe function when it
> -	 * creates the UARTs, whether or not they're used as a console.
> -	 *
> -	 * If the user specifies "traditional" earlycon, the qdf2400_e44
> -	 * console name matches the EARLYCON_DECLARE() statement, and
> -	 * SPCR is not used.  Parameter "earlycon" is false.
> -	 *
> -	 * If the user specifies "SPCR" earlycon, then we need to update
> -	 * the console name so that it also says "qdf2400_e44".  Parameter
> -	 * "earlycon" is true.
> -	 *
> -	 * For consistency, if we change the console name, then we do it
> -	 * for everyone, not just earlycon.
> -	 */
> -	if (qdf2400_erratum_44_present(&table->header)) {
> -		qdf2400_e44_present = true;
> -		if (earlycon)
> -			uart = "qdf2400_e44";
> +	switch (table->interface_type) {
> +	case ACPI_DBG2_16550_COMPATIBLE:
> +	case ACPI_DBG2_16550_SUBSET:
> +		snprintf(uart, ACPI_SPCR_BUF_SIZE, "uart");
> +		break;
> +	default:
> +		break;
>  	}
>  
> -	if (xgene_8250_erratum_present(table)) {
> -		iotype = "mmio32";
> +	if (table->serial_port.space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY) {
> +		u8 width = ACPI_ACCESS_BIT_WIDTH((
> +					table->serial_port.access_width));
> +		switch (width) {
> +		default:
> +			pr_err("Unexpected SPCR Access Width.  Defaulting to byte size\n");
> +		case 8:
> +			snprintf(iotype, ACPI_SPCR_BUF_SIZE, "mmio");
> +			break;
> +		case 16:
> +		case 32:
> +			snprintf(iotype, ACPI_SPCR_BUF_SIZE, "mmio%d", width);
> +			break;
> +		}
> +	} else
> +		snprintf(iotype, ACPI_SPCR_BUF_SIZE, "io");
>  
> -		/* for xgene v1 and v2 we don't know the clock rate of the
> -		 * UART so don't attempt to change to the baud rate state
> -		 * in the table because driver cannot calculate the dividers
> -		 */
> -		snprintf(opts, sizeof(opts), "%s,%s,0x%llx", uart, iotype,
> -			 table->serial_port.address);
> -	} else {
> -		snprintf(opts, sizeof(opts), "%s,%s,0x%llx,%d", uart, iotype,
> -			 table->serial_port.address, baud_rate);
> -	}
> +	err = acpi_arch_setup_console(table, opts, uart, iotype, baud_rate,
> +				      earlycon);
> +	if (err)
> +		goto done;
>  
>  	pr_info("console: %s\n", opts);
>  
> @@ -209,7 +113,6 @@ int __init parse_spcr(bool earlycon)
>  		setup_earlycon(opts);
>  
>  	err = add_preferred_console(uart, 0, opts + strlen(uart) + 1);
> -

Unintended change.

>  done:
>  	acpi_put_table((struct acpi_table_header *)table);
>  	return err;
> diff --git a/drivers/tty/serial/earlycon.c b/drivers/tty/serial/earlycon.c
> index 4c8b80f1c688..b22afb62c7a3 100644
> --- a/drivers/tty/serial/earlycon.c
> +++ b/drivers/tty/serial/earlycon.c
> @@ -196,26 +196,15 @@ int __init setup_earlycon(char *buf)
>  	return -ENOENT;
>  }
>  
> -/*
> - * When CONFIG_ACPI_SPCR_TABLE is defined, "earlycon" without parameters in
> - * command line does not start DT earlycon immediately, instead it defers
> - * starting it until DT/ACPI decision is made.  At that time if ACPI is enabled
> - * call parse_spcr(), else call early_init_dt_scan_chosen_stdout()
> - */
> -bool earlycon_init_is_deferred __initdata;
> -
>  /* early_param wrapper for setup_earlycon() */
>  static int __init param_setup_earlycon(char *buf)
>  {
>  	int err;
>  
> -	/*
> -	 * Just 'earlycon' is a valid param for devicetree earlycons;
> -	 * don't generate a warning from parse_early_params() in that case
> -	 */
> +	/* Just 'earlycon' is a valid param for devicetree and ACPI SPCR. */
>  	if (!buf || !buf[0]) {
>  		if (IS_ENABLED(CONFIG_ACPI_SPCR_TABLE)) {
> -			earlycon_init_is_deferred = true;
> +			console_acpi_spcr_enable = true;

I am not familiar with this code, I would ask Graeme and Mark to check
if this change is correct, the logic seems correct to me but I may be
missing some corner cases.

>  			return 0;
>  		} else if (!buf) {
>  			return early_init_dt_scan_chosen_stdout();
> diff --git a/include/linux/acpi.h b/include/linux/acpi.h
> index dc1ebfeeb5ec..875d7327d91c 100644
> --- a/include/linux/acpi.h
> +++ b/include/linux/acpi.h
> @@ -1241,10 +1241,17 @@ static inline bool acpi_has_watchdog(void) { return false; }
>  #endif
>  
>  #ifdef CONFIG_ACPI_SPCR_TABLE
> +#define ACPI_SPCR_OPTS_SIZE 64
> +#define ACPI_SPCR_BUF_SIZE 32
>  extern bool qdf2400_e44_present;
> -int parse_spcr(bool earlycon);
> +extern bool console_acpi_spcr_enable __initdata;
> +extern int acpi_arch_setup_console(struct acpi_table_spcr *table,
> +				   char *opts, char *uart, char *iotype,
> +				   int baud_rate, bool earlycon);
> +int acpi_parse_spcr(bool earlycon);
>  #else
> -static inline int parse_spcr(bool earlycon) { return 0; }
> +static const bool console_acpi_spcr_enable;

The assignment in param_setup_earlycon won't compile.

Lorenzo

Re: [PATCH v2 1/2] acpi, spcr: Make SPCR avialable to other architectures

[Timur Tabi]

On 12/13/2017 06:45 AM, Lorenzo Pieralisi wrote:
>> +/*
>> + * Erratum 44 for QDF2432v1 and QDF2400v1 SoCs describes the BUSY bit as
>> + * occasionally getting stuck as 1. To avoid the potential for a hang, check
>> + * TXFE == 0 instead of BUSY == 1. This may not be suitable for all UART
>> + * implementations, so only do so if an affected platform is detected in
>> + * acpi_parse_spcr().
>> + */
>> +bool qdf2400_e44_present;
>> +EXPORT_SYMBOL(qdf2400_e44_present);
> 
> My eyes, this is horrible but it is not introduced by this patch. It
> would have been much better if:
> 
> drivers/tty/serial/amba-pl011.c
> 
> parsed the SPCR table (again) to detect it instead of relying on this
> horrible exported flag.

I didn't want to put any ACPI code in amba-pl011.c, so putting it in 
spcr.c made the most sense.  I agree the global variable is ugly.  If 
you have a better idea, I'm all ears.

If it's any consolation, this erratum affects only 1.x silicon, which is 
technically pre-production (although a lot of people have them).  This 
work-around will eventually be reverted.

>> diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
>> index 46505396869e..9ae98eeada76 100644
>> --- a/drivers/acpi/Kconfig
>> +++ b/drivers/acpi/Kconfig
>> @@ -79,7 +79,12 @@ config ACPI_DEBUGGER_USER
>>   endif
>>   
>>   config ACPI_SPCR_TABLE
>> -	bool
>> +	bool "ACPI Serial Port Console Redirection Support"
>> +	default y if ARM64
> 
> You need to remove the selection in arch/arm64 then. Also, moving away
> from a non-visible config may have consequences on ARM64, Graeme and
> Mark are more familiar with the SPCR dependencies so please chime in.

I did raise this as a concern in the previous version of the patch.  I 
also think it should not be a selectable option.

Keeping the "select" does force SPCR to be enabled on ARM64 ACPI 
platforms, but if it's an option for x86, it should be an option for ARM.

-- 
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm
Technologies, Inc.  Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.

[PATCH v2 1/2] acpi, spcr: Make SPCR avialable to other architectures

[Timur Tabi]

On 12/13/2017 06:45 AM, Lorenzo Pieralisi wrote:
>> +/*
>> + * Erratum 44 for QDF2432v1 and QDF2400v1 SoCs describes the BUSY bit as
>> + * occasionally getting stuck as 1. To avoid the potential for a hang, check
>> + * TXFE == 0 instead of BUSY == 1. This may not be suitable for all UART
>> + * implementations, so only do so if an affected platform is detected in
>> + * acpi_parse_spcr().
>> + */
>> +bool qdf2400_e44_present;
>> +EXPORT_SYMBOL(qdf2400_e44_present);
> 
> My eyes, this is horrible but it is not introduced by this patch. It
> would have been much better if:
> 
> drivers/tty/serial/amba-pl011.c
> 
> parsed the SPCR table (again) to detect it instead of relying on this
> horrible exported flag.

I didn't want to put any ACPI code in amba-pl011.c, so putting it in 
spcr.c made the most sense.  I agree the global variable is ugly.  If 
you have a better idea, I'm all ears.

If it's any consolation, this erratum affects only 1.x silicon, which is 
technically pre-production (although a lot of people have them).  This 
work-around will eventually be reverted.

>> diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
>> index 46505396869e..9ae98eeada76 100644
>> --- a/drivers/acpi/Kconfig
>> +++ b/drivers/acpi/Kconfig
>> @@ -79,7 +79,12 @@ config ACPI_DEBUGGER_USER
>>   endif
>>   
>>   config ACPI_SPCR_TABLE
>> -	bool
>> +	bool "ACPI Serial Port Console Redirection Support"
>> +	default y if ARM64
> 
> You need to remove the selection in arch/arm64 then. Also, moving away
> from a non-visible config may have consequences on ARM64, Graeme and
> Mark are more familiar with the SPCR dependencies so please chime in.

I did raise this as a concern in the previous version of the patch.  I 
also think it should not be a selectable option.

Keeping the "select" does force SPCR to be enabled on ARM64 ACPI 
platforms, but if it's an option for x86, it should be an option for ARM.

-- 
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm
Technologies, Inc.  Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.

Re: [PATCH v2 1/2] acpi, spcr: Make SPCR avialable to other architectures

[Lorenzo Pieralisi]

On Wed, Dec 13, 2017 at 03:11:33PM -0600, Timur Tabi wrote:
> On 12/13/2017 06:45 AM, Lorenzo Pieralisi wrote:
> >>+/*
> >>+ * Erratum 44 for QDF2432v1 and QDF2400v1 SoCs describes the BUSY bit as
> >>+ * occasionally getting stuck as 1. To avoid the potential for a hang, check
> >>+ * TXFE == 0 instead of BUSY == 1. This may not be suitable for all UART
> >>+ * implementations, so only do so if an affected platform is detected in
> >>+ * acpi_parse_spcr().
> >>+ */
> >>+bool qdf2400_e44_present;
> >>+EXPORT_SYMBOL(qdf2400_e44_present);
> >
> >My eyes, this is horrible but it is not introduced by this patch. It
> >would have been much better if:
> >
> >drivers/tty/serial/amba-pl011.c
> >
> >parsed the SPCR table (again) to detect it instead of relying on this
> >horrible exported flag.
> 
> I didn't want to put any ACPI code in amba-pl011.c, so putting it in spcr.c
> made the most sense.  I agree the global variable is ugly.  If you have a
> better idea, I'm all ears.

I told you my idea. It could have been made easier by reusing the
ACPI_DECLARE_PROBE_ENTRY() mechanism.

> If it's any consolation, this erratum affects only 1.x silicon, which is
> technically pre-production (although a lot of people have them).  This
> work-around will eventually be reverted.

The sooner the better.

Lorenzo

[PATCH v2 1/2] acpi, spcr: Make SPCR avialable to other architectures

[Lorenzo Pieralisi]

On Wed, Dec 13, 2017 at 03:11:33PM -0600, Timur Tabi wrote:
> On 12/13/2017 06:45 AM, Lorenzo Pieralisi wrote:
> >>+/*
> >>+ * Erratum 44 for QDF2432v1 and QDF2400v1 SoCs describes the BUSY bit as
> >>+ * occasionally getting stuck as 1. To avoid the potential for a hang, check
> >>+ * TXFE == 0 instead of BUSY == 1. This may not be suitable for all UART
> >>+ * implementations, so only do so if an affected platform is detected in
> >>+ * acpi_parse_spcr().
> >>+ */
> >>+bool qdf2400_e44_present;
> >>+EXPORT_SYMBOL(qdf2400_e44_present);
> >
> >My eyes, this is horrible but it is not introduced by this patch. It
> >would have been much better if:
> >
> >drivers/tty/serial/amba-pl011.c
> >
> >parsed the SPCR table (again) to detect it instead of relying on this
> >horrible exported flag.
> 
> I didn't want to put any ACPI code in amba-pl011.c, so putting it in spcr.c
> made the most sense.  I agree the global variable is ugly.  If you have a
> better idea, I'm all ears.

I told you my idea. It could have been made easier by reusing the
ACPI_DECLARE_PROBE_ENTRY() mechanism.

> If it's any consolation, this erratum affects only 1.x silicon, which is
> technically pre-production (although a lot of people have them).  This
> work-around will eventually be reverted.

The sooner the better.

Lorenzo

Re: [PATCH v2 1/2] acpi, spcr: Make SPCR avialable to other architectures

[Timur Tabi]

On 12/14/17 4:30 AM, Lorenzo Pieralisi wrote:
>> I didn't want to put any ACPI code in amba-pl011.c, so putting it in spcr.c
>> made the most sense.  I agree the global variable is ugly.  If you have a
>> better idea, I'm all ears.

> I told you my idea. It could have been made easier by reusing the
> ACPI_DECLARE_PROBE_ENTRY() mechanism.

Sorry, I don't mean to be difficult, but when did you tell *me* this 
idea of yours?  I don't see any email from you to me that mentions 
ACPI_DECLARE_PROBE_ENTRY().  I've never even heard of that macro before. 
  Please note that I'm not the original author of this code.

-- 
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm
Technologies, Inc.  Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.

[PATCH v2 1/2] acpi, spcr: Make SPCR avialable to other architectures

[Timur Tabi]

On 12/14/17 4:30 AM, Lorenzo Pieralisi wrote:
>> I didn't want to put any ACPI code in amba-pl011.c, so putting it in spcr.c
>> made the most sense.  I agree the global variable is ugly.  If you have a
>> better idea, I'm all ears.

> I told you my idea. It could have been made easier by reusing the
> ACPI_DECLARE_PROBE_ENTRY() mechanism.

Sorry, I don't mean to be difficult, but when did you tell *me* this 
idea of yours?  I don't see any email from you to me that mentions 
ACPI_DECLARE_PROBE_ENTRY().  I've never even heard of that macro before. 
  Please note that I'm not the original author of this code.

-- 
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm
Technologies, Inc.  Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.

Re: [PATCH v2 1/2] acpi, spcr: Make SPCR avialable to other architectures

[Lorenzo Pieralisi]

On Thu, Dec 14, 2017 at 08:08:08AM -0600, Timur Tabi wrote:
> On 12/14/17 4:30 AM, Lorenzo Pieralisi wrote:
> >>I didn't want to put any ACPI code in amba-pl011.c, so putting it in spcr.c
> >>made the most sense.  I agree the global variable is ugly.  If you have a
> >>better idea, I'm all ears.
> 
> >I told you my idea. It could have been made easier by reusing the
> >ACPI_DECLARE_PROBE_ENTRY() mechanism.
> 
> Sorry, I don't mean to be difficult, but when did you tell *me* this idea of
> yours?  I don't see any email from you to me that mentions

I said that IMO it would have been better if the quirk was managed in
amba-pl011.c - you had your reasons not to do it, end of the story.

> ACPI_DECLARE_PROBE_ENTRY().  I've never even heard of that macro before.
> Please note that I'm not the original author of this code.

It is what it is, let's move on, we will keep this in mind if a similar
quirk is required.

Thanks,
Lorenzo

[PATCH v2 1/2] acpi, spcr: Make SPCR avialable to other architectures

[Lorenzo Pieralisi]

On Thu, Dec 14, 2017 at 08:08:08AM -0600, Timur Tabi wrote:
> On 12/14/17 4:30 AM, Lorenzo Pieralisi wrote:
> >>I didn't want to put any ACPI code in amba-pl011.c, so putting it in spcr.c
> >>made the most sense.  I agree the global variable is ugly.  If you have a
> >>better idea, I'm all ears.
> 
> >I told you my idea. It could have been made easier by reusing the
> >ACPI_DECLARE_PROBE_ENTRY() mechanism.
> 
> Sorry, I don't mean to be difficult, but when did you tell *me* this idea of
> yours?  I don't see any email from you to me that mentions

I said that IMO it would have been better if the quirk was managed in
amba-pl011.c - you had your reasons not to do it, end of the story.

> ACPI_DECLARE_PROBE_ENTRY().  I've never even heard of that macro before.
> Please note that I'm not the original author of this code.

It is what it is, let's move on, we will keep this in mind if a similar
quirk is required.

Thanks,
Lorenzo

Re: [PATCH v2 1/2] acpi, spcr: Make SPCR avialable to other architectures

[Prarit Bhargava]

[Sorry everyone for the late response, I went away on vacation and pushed this
off until I returned.]

On 12/13/2017 07:45 AM, Lorenzo Pieralisi wrote:
> [+Mark, Graeme]
> 
> In $SUBJECT, s/avialable/available
> 
> On Mon, Dec 11, 2017 at 10:50:58AM -0500, Prarit Bhargava wrote:
>> Other architectures can use SPCR to setup an early console or console
>> but the current code is ARM64 specific.
> 
> I see nothing ARM64 specific in current code (apart from some
> ACPICA macros with an ARM tag in them) please explain to me
> what's preventing you to reuse current code on x86.

Ah, I didn't notice that.  I thought the ACPICA macros were ARM specific.  I'll
rework this patchset with that in mind.

> 
>> Change the name of parse_spcr() to acpi_parse_spcr().  Add a weak
>> function acpi_arch_setup_console() that can be used for arch-specific
>> setup.  Move flags into ACPI code.  Update the Documention on the use of
>> the SPCR.
>>
>> [v2]: Don't return an error in the baud_rate check of acpi_parse_spcr().
>> Keep ACPI_SPCR_TABLE selected for ARM64.  Fix 8-bit port access width
>> mmio value.  Move baud rate check earlier.
> 
> This does not belong in the commit log.

Yes, but some maintainers like to see what changed between v1 & v2.

> 
>> Signed-off-by: Prarit Bhargava <prarit@redhat.com>
>> Cc: linux-doc@vger.kernel.org
>> Cc: linux-kernel@vger.kernel.org
>> Cc: linux-arm-kernel@lists.infradead.org
>> Cc: linux-pm@vger.kernel.org
>> Cc: linux-acpi@vger.kernel.org
>> Cc: linux-serial@vger.kernel.org
>> Cc: Bhupesh Sharma <bhsharma@redhat.com>
>> Cc: Lv Zheng <lv.zheng@intel.com>
>> Cc: Thomas Gleixner <tglx@linutronix.de>
>> Cc: Ingo Molnar <mingo@redhat.com>
>> Cc: "H. Peter Anvin" <hpa@zytor.com>
>> Cc: x86@kernel.org
>> Cc: Jonathan Corbet <corbet@lwn.net>
>> Cc: Catalin Marinas <catalin.marinas@arm.com>
>> Cc: Will Deacon <will.deacon@arm.com>
>> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
>> Cc: Timur Tabi <timur@codeaurora.org>
>> ---
>>  Documentation/admin-guide/kernel-parameters.txt |   6 +-
>>  arch/arm64/kernel/acpi.c                        | 128 ++++++++++++++++-
>>  drivers/acpi/Kconfig                            |   7 +-
>>  drivers/acpi/spcr.c                             | 175 ++++++------------------
>>  drivers/tty/serial/earlycon.c                   |  15 +-
>>  include/linux/acpi.h                            |  11 +-
>>  include/linux/serial_core.h                     |   2 -
>>  7 files changed, 184 insertions(+), 160 deletions(-)
>>
>> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
>> index 6571fbfdb2a1..0d173289c67e 100644
>> --- a/Documentation/admin-guide/kernel-parameters.txt
>> +++ b/Documentation/admin-guide/kernel-parameters.txt
>> @@ -914,9 +914,9 @@
>>  
>>  	earlycon=	[KNL] Output early console device and options.
>>  
>> -			When used with no options, the early console is
>> -			determined by the stdout-path property in device
>> -			tree's chosen node.
>> +			[ARM64] The early console is determined by the
>> +			stdout-path property in device tree's chosen node,
>> +			or determined by the ACPI SPCR table.
>>  
>>  		cdns,<addr>[,options]
>>  			Start an early, polled-mode console on a Cadence
>> diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c
>> index b3162715ed78..b3e33bbdf3b7 100644
>> --- a/arch/arm64/kernel/acpi.c
>> +++ b/arch/arm64/kernel/acpi.c
>> @@ -25,7 +25,6 @@
>>  #include <linux/memblock.h>
>>  #include <linux/of_fdt.h>
>>  #include <linux/smp.h>
>> -#include <linux/serial_core.h>
>>  
>>  #include <asm/cputype.h>
>>  #include <asm/cpu_ops.h>
>> @@ -177,6 +176,128 @@ static int __init acpi_fadt_sanity_check(void)
>>  	return ret;
>>  }
>>  
>> +/*
>> + * Erratum 44 for QDF2432v1 and QDF2400v1 SoCs describes the BUSY bit as
>> + * occasionally getting stuck as 1. To avoid the potential for a hang, check
>> + * TXFE == 0 instead of BUSY == 1. This may not be suitable for all UART
>> + * implementations, so only do so if an affected platform is detected in
>> + * acpi_parse_spcr().
>> + */
>> +bool qdf2400_e44_present;
>> +EXPORT_SYMBOL(qdf2400_e44_present);
> 
> My eyes, this is horrible but it is not introduced by this patch. It
> would have been much better if:
> 
> drivers/tty/serial/amba-pl011.c
> 
> parsed the SPCR table (again) to detect it instead of relying on this
> horrible exported flag.

It looks like Timur & you had a discussion and the current consensus is to keep
the code the same.

> 
>> +
>> +/*
>> + * Some Qualcomm Datacenter Technologies SoCs have a defective UART BUSY bit.
>> + * Detect them by examining the OEM fields in the SPCR header, similar to PCI
>> + * quirk detection in pci_mcfg.c.
>> + */
>> +static bool qdf2400_erratum_44_present(struct acpi_table_header *h)
>> +{
>> +	if (memcmp(h->oem_id, "QCOM  ", ACPI_OEM_ID_SIZE))
>> +		return false;
>> +
>> +	if (!memcmp(h->oem_table_id, "QDF2432 ", ACPI_OEM_TABLE_ID_SIZE))
>> +		return true;
>> +
>> +	if (!memcmp(h->oem_table_id, "QDF2400 ", ACPI_OEM_TABLE_ID_SIZE) &&
>> +			h->oem_revision == 1)
>> +		return true;
>> +
>> +	return false;
>> +}
>> +
>> +/*
>> + * APM X-Gene v1 and v2 UART hardware is an 16550 like device but has its
>> + * register aligned to 32-bit. In addition, the BIOS also encoded the
>> + * access width to be 8 bits. This function detects this errata condition.
>> + */
>> +static bool xgene_8250_erratum_present(struct acpi_table_spcr *tb)
>> +{
>> +	bool xgene_8250 = false;
>> +
>> +	if (tb->interface_type != ACPI_DBG2_16550_COMPATIBLE)
>> +		return false;
>> +
>> +	if (memcmp(tb->header.oem_id, "APMC0D", ACPI_OEM_ID_SIZE) &&
>> +	    memcmp(tb->header.oem_id, "HPE   ", ACPI_OEM_ID_SIZE))
>> +		return false;
>> +
>> +	if (!memcmp(tb->header.oem_table_id, "XGENESPC",
>> +	    ACPI_OEM_TABLE_ID_SIZE) && tb->header.oem_revision == 0)
>> +		xgene_8250 = true;
>> +
>> +	if (!memcmp(tb->header.oem_table_id, "ProLiant",
>> +	    ACPI_OEM_TABLE_ID_SIZE) && tb->header.oem_revision == 1)
>> +		xgene_8250 = true;
>> +
>> +	return xgene_8250;
>> +}
>> +
>> +int acpi_arch_setup_console(struct acpi_table_spcr *table,
>> +			    char *opts, char *uart, char *iotype,
>> +			    int baud_rate, bool earlycon)
>> +{
>> +	if (table->header.revision < 2) {
>> +		pr_err("wrong table version\n");
>> +		return -ENOENT;
>> +	}
>> +
>> +	switch (table->interface_type) {
>> +	case ACPI_DBG2_ARM_SBSA_32BIT:
>> +		snprintf(iotype, ACPI_SPCR_BUF_SIZE, "mmio32");
>> +		/* fall through */
>> +	case ACPI_DBG2_ARM_PL011:
>> +	case ACPI_DBG2_ARM_SBSA_GENERIC:
>> +	case ACPI_DBG2_BCM2835:
>> +		snprintf(uart, ACPI_SPCR_BUF_SIZE, "pl011");
>> +		break;
>> +	default:
>> +		if (strlen(uart) == 0)
>> +			return -ENOENT;
>> +	}
>> +
>> +	/*
>> +	 * If the E44 erratum is required, then we need to tell the pl011
>> +	 * driver to implement the work-around.
>> +	 *
>> +	 * The global variable is used by the probe function when it
>> +	 * creates the UARTs, whether or not they're used as a console.
>> +	 *
>> +	 * If the user specifies "traditional" earlycon, the qdf2400_e44
>> +	 * console name matches the EARLYCON_DECLARE() statement, and
>> +	 * SPCR is not used.  Parameter "earlycon" is false.
>> +	 *
>> +	 * If the user specifies "SPCR" earlycon, then we need to update
>> +	 * the console name so that it also says "qdf2400_e44".  Parameter
>> +	 * "earlycon" is true.
>> +	 *
>> +	 * For consistency, if we change the console name, then we do it
>> +	 * for everyone, not just earlycon.
>> +	 */
>> +	if (qdf2400_erratum_44_present(&table->header)) {
>> +		qdf2400_e44_present = true;
>> +		if (earlycon)
>> +			snprintf(uart, ACPI_SPCR_BUF_SIZE, "qdf2400_e44");
>> +	}
>> +
>> +	if (xgene_8250_erratum_present(table)) {
>> +		snprintf(iotype, ACPI_SPCR_BUF_SIZE, "mmio32");
>> +
>> +		/* for xgene v1 and v2 we don't know the clock rate of the
>> +		 * UART so don't attempt to change to the baud rate state
>> +		 * in the table because driver cannot calculate the dividers
>> +		 */
>> +		snprintf(opts, ACPI_SPCR_OPTS_SIZE, "%s,%s,0x%llx", uart,
>> +			 iotype, table->serial_port.address);
>> +	} else {
>> +		snprintf(opts, ACPI_SPCR_OPTS_SIZE, "%s,%s,0x%llx,%d", uart,
>> +			 iotype, table->serial_port.address, baud_rate);
>> +	}
>> +
>> +	return 0;
>> +}
>> +EXPORT_SYMBOL(acpi_arch_setup_console);
> 
> EXPORT_SYMBOL() ? Why ?
> 
> BTW, why do we need an arch hook ? I do not see anything that prevents
> you from using this code on x86 systems - is there anything arch
> specific in the SPCR specification itself ?

See above comment.  But also keep in mind I have seen a lot of x86 systems that
do not define the ACPI table version correctly.  The table version is 0 or 1,
and the current spec says it must be 2.  It looks like ARM requires 2.

> 
>> +
>>  /*
>>   * acpi_boot_table_init() called from setup_arch(), always.
>>   *	1. find RSDP and get its address, and then find XSDT
>> @@ -230,10 +351,11 @@ void __init acpi_boot_table_init(void)
>>  
>>  done:
>>  	if (acpi_disabled) {
>> -		if (earlycon_init_is_deferred)
>> +		if (console_acpi_spcr_enable)
>>  			early_init_dt_scan_chosen_stdout();
>>  	} else {
>> -		parse_spcr(earlycon_init_is_deferred);
>> +		/* Always enable the ACPI SPCR console */
>> +		acpi_parse_spcr(console_acpi_spcr_enable);
>>  		if (IS_ENABLED(CONFIG_ACPI_BGRT))
>>  			acpi_table_parse(ACPI_SIG_BGRT, acpi_parse_bgrt);
>>  	}
>> diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
>> index 46505396869e..9ae98eeada76 100644
>> --- a/drivers/acpi/Kconfig
>> +++ b/drivers/acpi/Kconfig
>> @@ -79,7 +79,12 @@ config ACPI_DEBUGGER_USER
>>  endif
>>  
>>  config ACPI_SPCR_TABLE
>> -	bool
>> +	bool "ACPI Serial Port Console Redirection Support"
>> +	default y if ARM64
> 
> You need to remove the selection in arch/arm64 then. Also, moving away
> from a non-visible config may have consequences on ARM64, Graeme and
> Mark are more familiar with the SPCR dependencies so please chime in.

Ok, I'll double check this.

> 
>> +	help
>> +	  Enable support for Serial Port Console Redirection (SPCR) Table.
>> +	  This table provides information about the configuration of the
>> +	  earlycon console.
>>  
>>  config ACPI_LPIT
>>  	bool
>> diff --git a/drivers/acpi/spcr.c b/drivers/acpi/spcr.c
>> index 324b35bfe781..f4bb8110e404 100644
>> --- a/drivers/acpi/spcr.c
>> +++ b/drivers/acpi/spcr.c
>> @@ -16,65 +16,18 @@
>>  #include <linux/kernel.h>
>>  #include <linux/serial_core.h>
>>  
>> -/*
>> - * Erratum 44 for QDF2432v1 and QDF2400v1 SoCs describes the BUSY bit as
>> - * occasionally getting stuck as 1. To avoid the potential for a hang, check
>> - * TXFE == 0 instead of BUSY == 1. This may not be suitable for all UART
>> - * implementations, so only do so if an affected platform is detected in
>> - * parse_spcr().
>> - */
>> -bool qdf2400_e44_present;
>> -EXPORT_SYMBOL(qdf2400_e44_present);
>> -
>> -/*
>> - * Some Qualcomm Datacenter Technologies SoCs have a defective UART BUSY bit.
>> - * Detect them by examining the OEM fields in the SPCR header, similiar to PCI
>> - * quirk detection in pci_mcfg.c.
>> - */
>> -static bool qdf2400_erratum_44_present(struct acpi_table_header *h)
>> -{
>> -	if (memcmp(h->oem_id, "QCOM  ", ACPI_OEM_ID_SIZE))
>> -		return false;
>> -
>> -	if (!memcmp(h->oem_table_id, "QDF2432 ", ACPI_OEM_TABLE_ID_SIZE))
>> -		return true;
>> -
>> -	if (!memcmp(h->oem_table_id, "QDF2400 ", ACPI_OEM_TABLE_ID_SIZE) &&
>> -			h->oem_revision == 1)
>> -		return true;
>> -
>> -	return false;
>> -}
>> -
>> -/*
>> - * APM X-Gene v1 and v2 UART hardware is an 16550 like device but has its
>> - * register aligned to 32-bit. In addition, the BIOS also encoded the
>> - * access width to be 8 bits. This function detects this errata condition.
>> - */
>> -static bool xgene_8250_erratum_present(struct acpi_table_spcr *tb)
>> +int __weak acpi_arch_setup_console(struct acpi_table_spcr *table,
>> +				   char *opts, char *uart, char *iotype,
>> +				   int baud_rate, bool earlycon)
>>  {
>> -	bool xgene_8250 = false;
>> -
>> -	if (tb->interface_type != ACPI_DBG2_16550_COMPATIBLE)
>> -		return false;
>> -
>> -	if (memcmp(tb->header.oem_id, "APMC0D", ACPI_OEM_ID_SIZE) &&
>> -	    memcmp(tb->header.oem_id, "HPE   ", ACPI_OEM_ID_SIZE))
>> -		return false;
>> -
>> -	if (!memcmp(tb->header.oem_table_id, "XGENESPC",
>> -	    ACPI_OEM_TABLE_ID_SIZE) && tb->header.oem_revision == 0)
>> -		xgene_8250 = true;
>> -
>> -	if (!memcmp(tb->header.oem_table_id, "ProLiant",
>> -	    ACPI_OEM_TABLE_ID_SIZE) && tb->header.oem_revision == 1)
>> -		xgene_8250 = true;
>> -
>> -	return xgene_8250;
>> +	snprintf(opts, ACPI_SPCR_OPTS_SIZE, "%s,%s,0x%llx,%d", uart, iotype,
>> +		 table->serial_port.address, baud_rate);
>> +	return 0;
>>  }
>>  
>> +bool console_acpi_spcr_enable __initdata;
>>  /**
>> - * parse_spcr() - parse ACPI SPCR table and add preferred console
>> + * acpi_parse_spcr() - parse ACPI SPCR table and add preferred console
>>   *
>>   * @earlycon: set up earlycon for the console specified by the table
>>   *
>> @@ -86,13 +39,13 @@ static bool xgene_8250_erratum_present(struct acpi_table_spcr *tb)
>>   * from arch initialization code as soon as the DT/ACPI decision is made.
>>   *
>>   */
>> -int __init parse_spcr(bool earlycon)
>> +int __init acpi_parse_spcr(bool earlycon)
>>  {
>> -	static char opts[64];
>> +	static char opts[ACPI_SPCR_OPTS_SIZE];
>> +	static char uart[ACPI_SPCR_BUF_SIZE];
>> +	static char iotype[ACPI_SPCR_BUF_SIZE];
>>  	struct acpi_table_spcr *table;
>>  	acpi_status status;
>> -	char *uart;
>> -	char *iotype;
>>  	int baud_rate;
>>  	int err;
>>  
>> @@ -105,48 +58,6 @@ int __init parse_spcr(bool earlycon)
>>  	if (ACPI_FAILURE(status))
>>  		return -ENOENT;
>>  
>> -	if (table->header.revision < 2) {
>> -		err = -ENOENT;
>> -		pr_err("wrong table version\n");
>> -		goto done;
>> -	}
>> -
>> -	if (table->serial_port.space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY) {
>> -		switch (ACPI_ACCESS_BIT_WIDTH((
>> -			table->serial_port.access_width))) {
>> -		default:
>> -			pr_err("Unexpected SPCR Access Width.  Defaulting to byte size\n");
>> -		case 8:
>> -			iotype = "mmio";
>> -			break;
>> -		case 16:
>> -			iotype = "mmio16";
>> -			break;
>> -		case 32:
>> -			iotype = "mmio32";
>> -			break;
>> -		}
>> -	} else
>> -		iotype = "io";
>> -
>> -	switch (table->interface_type) {
>> -	case ACPI_DBG2_ARM_SBSA_32BIT:
>> -		iotype = "mmio32";
>> -		/* fall through */
>> -	case ACPI_DBG2_ARM_PL011:
>> -	case ACPI_DBG2_ARM_SBSA_GENERIC:
>> -	case ACPI_DBG2_BCM2835:
>> -		uart = "pl011";
>> -		break;
>> -	case ACPI_DBG2_16550_COMPATIBLE:
>> -	case ACPI_DBG2_16550_SUBSET:
>> -		uart = "uart";
>> -		break;
>> -	default:
>> -		err = -ENOENT;
>> -		goto done;
>> -	}
>> -
>>  	switch (table->baud_rate) {
>>  	case 3:
>>  		baud_rate = 9600;
>> @@ -165,43 +76,36 @@ int __init parse_spcr(bool earlycon)
>>  		goto done;
>>  	}
>>  
>> -	/*
>> -	 * If the E44 erratum is required, then we need to tell the pl011
>> -	 * driver to implement the work-around.
>> -	 *
>> -	 * The global variable is used by the probe function when it
>> -	 * creates the UARTs, whether or not they're used as a console.
>> -	 *
>> -	 * If the user specifies "traditional" earlycon, the qdf2400_e44
>> -	 * console name matches the EARLYCON_DECLARE() statement, and
>> -	 * SPCR is not used.  Parameter "earlycon" is false.
>> -	 *
>> -	 * If the user specifies "SPCR" earlycon, then we need to update
>> -	 * the console name so that it also says "qdf2400_e44".  Parameter
>> -	 * "earlycon" is true.
>> -	 *
>> -	 * For consistency, if we change the console name, then we do it
>> -	 * for everyone, not just earlycon.
>> -	 */
>> -	if (qdf2400_erratum_44_present(&table->header)) {
>> -		qdf2400_e44_present = true;
>> -		if (earlycon)
>> -			uart = "qdf2400_e44";
>> +	switch (table->interface_type) {
>> +	case ACPI_DBG2_16550_COMPATIBLE:
>> +	case ACPI_DBG2_16550_SUBSET:
>> +		snprintf(uart, ACPI_SPCR_BUF_SIZE, "uart");
>> +		break;
>> +	default:
>> +		break;
>>  	}
>>  
>> -	if (xgene_8250_erratum_present(table)) {
>> -		iotype = "mmio32";
>> +	if (table->serial_port.space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY) {
>> +		u8 width = ACPI_ACCESS_BIT_WIDTH((
>> +					table->serial_port.access_width));
>> +		switch (width) {
>> +		default:
>> +			pr_err("Unexpected SPCR Access Width.  Defaulting to byte size\n");
>> +		case 8:
>> +			snprintf(iotype, ACPI_SPCR_BUF_SIZE, "mmio");
>> +			break;
>> +		case 16:
>> +		case 32:
>> +			snprintf(iotype, ACPI_SPCR_BUF_SIZE, "mmio%d", width);
>> +			break;
>> +		}
>> +	} else
>> +		snprintf(iotype, ACPI_SPCR_BUF_SIZE, "io");
>>  
>> -		/* for xgene v1 and v2 we don't know the clock rate of the
>> -		 * UART so don't attempt to change to the baud rate state
>> -		 * in the table because driver cannot calculate the dividers
>> -		 */
>> -		snprintf(opts, sizeof(opts), "%s,%s,0x%llx", uart, iotype,
>> -			 table->serial_port.address);
>> -	} else {
>> -		snprintf(opts, sizeof(opts), "%s,%s,0x%llx,%d", uart, iotype,
>> -			 table->serial_port.address, baud_rate);
>> -	}
>> +	err = acpi_arch_setup_console(table, opts, uart, iotype, baud_rate,
>> +				      earlycon);
>> +	if (err)
>> +		goto done;
>>  
>>  	pr_info("console: %s\n", opts);
>>  
>> @@ -209,7 +113,6 @@ int __init parse_spcr(bool earlycon)
>>  		setup_earlycon(opts);
>>  
>>  	err = add_preferred_console(uart, 0, opts + strlen(uart) + 1);
>> -
> 
> Unintended change.
> 
>>  done:
>>  	acpi_put_table((struct acpi_table_header *)table);
>>  	return err;
>> diff --git a/drivers/tty/serial/earlycon.c b/drivers/tty/serial/earlycon.c
>> index 4c8b80f1c688..b22afb62c7a3 100644
>> --- a/drivers/tty/serial/earlycon.c
>> +++ b/drivers/tty/serial/earlycon.c
>> @@ -196,26 +196,15 @@ int __init setup_earlycon(char *buf)
>>  	return -ENOENT;
>>  }
>>  
>> -/*
>> - * When CONFIG_ACPI_SPCR_TABLE is defined, "earlycon" without parameters in
>> - * command line does not start DT earlycon immediately, instead it defers
>> - * starting it until DT/ACPI decision is made.  At that time if ACPI is enabled
>> - * call parse_spcr(), else call early_init_dt_scan_chosen_stdout()
>> - */
>> -bool earlycon_init_is_deferred __initdata;
>> -
>>  /* early_param wrapper for setup_earlycon() */
>>  static int __init param_setup_earlycon(char *buf)
>>  {
>>  	int err;
>>  
>> -	/*
>> -	 * Just 'earlycon' is a valid param for devicetree earlycons;
>> -	 * don't generate a warning from parse_early_params() in that case
>> -	 */
>> +	/* Just 'earlycon' is a valid param for devicetree and ACPI SPCR. */
>>  	if (!buf || !buf[0]) {
>>  		if (IS_ENABLED(CONFIG_ACPI_SPCR_TABLE)) {
>> -			earlycon_init_is_deferred = true;
>> +			console_acpi_spcr_enable = true;
> 
> I am not familiar with this code, I would ask Graeme and Mark to check
> if this change is correct, the logic seems correct to me but I may be
> missing some corner cases.
> 
>>  			return 0;
>>  		} else if (!buf) {
>>  			return early_init_dt_scan_chosen_stdout();
>> diff --git a/include/linux/acpi.h b/include/linux/acpi.h
>> index dc1ebfeeb5ec..875d7327d91c 100644
>> --- a/include/linux/acpi.h
>> +++ b/include/linux/acpi.h
>> @@ -1241,10 +1241,17 @@ static inline bool acpi_has_watchdog(void) { return false; }
>>  #endif
>>  
>>  #ifdef CONFIG_ACPI_SPCR_TABLE
>> +#define ACPI_SPCR_OPTS_SIZE 64
>> +#define ACPI_SPCR_BUF_SIZE 32
>>  extern bool qdf2400_e44_present;
>> -int parse_spcr(bool earlycon);
>> +extern bool console_acpi_spcr_enable __initdata;
>> +extern int acpi_arch_setup_console(struct acpi_table_spcr *table,
>> +				   char *opts, char *uart, char *iotype,
>> +				   int baud_rate, bool earlycon);
>> +int acpi_parse_spcr(bool earlycon);
>>  #else
>> -static inline int parse_spcr(bool earlycon) { return 0; }
>> +static const bool console_acpi_spcr_enable;
> 
> The assignment in param_setup_earlycon won't compile. 

Hmm ... I'm pretty sure it did.  But I'll check that before resubmitting.

P.

> 
> Lorenzo
> 

[PATCH v2 1/2] acpi, spcr: Make SPCR avialable to other architectures

[Prarit Bhargava]

[Sorry everyone for the late response, I went away on vacation and pushed this
off until I returned.]

On 12/13/2017 07:45 AM, Lorenzo Pieralisi wrote:
> [+Mark, Graeme]
> 
> In $SUBJECT, s/avialable/available
> 
> On Mon, Dec 11, 2017 at 10:50:58AM -0500, Prarit Bhargava wrote:
>> Other architectures can use SPCR to setup an early console or console
>> but the current code is ARM64 specific.
> 
> I see nothing ARM64 specific in current code (apart from some
> ACPICA macros with an ARM tag in them) please explain to me
> what's preventing you to reuse current code on x86.

Ah, I didn't notice that.  I thought the ACPICA macros were ARM specific.  I'll
rework this patchset with that in mind.

> 
>> Change the name of parse_spcr() to acpi_parse_spcr().  Add a weak
>> function acpi_arch_setup_console() that can be used for arch-specific
>> setup.  Move flags into ACPI code.  Update the Documention on the use of
>> the SPCR.
>>
>> [v2]: Don't return an error in the baud_rate check of acpi_parse_spcr().
>> Keep ACPI_SPCR_TABLE selected for ARM64.  Fix 8-bit port access width
>> mmio value.  Move baud rate check earlier.
> 
> This does not belong in the commit log.

Yes, but some maintainers like to see what changed between v1 & v2.

> 
>> Signed-off-by: Prarit Bhargava <prarit@redhat.com>
>> Cc: linux-doc at vger.kernel.org
>> Cc: linux-kernel at vger.kernel.org
>> Cc: linux-arm-kernel at lists.infradead.org
>> Cc: linux-pm at vger.kernel.org
>> Cc: linux-acpi at vger.kernel.org
>> Cc: linux-serial at vger.kernel.org
>> Cc: Bhupesh Sharma <bhsharma@redhat.com>
>> Cc: Lv Zheng <lv.zheng@intel.com>
>> Cc: Thomas Gleixner <tglx@linutronix.de>
>> Cc: Ingo Molnar <mingo@redhat.com>
>> Cc: "H. Peter Anvin" <hpa@zytor.com>
>> Cc: x86 at kernel.org
>> Cc: Jonathan Corbet <corbet@lwn.net>
>> Cc: Catalin Marinas <catalin.marinas@arm.com>
>> Cc: Will Deacon <will.deacon@arm.com>
>> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
>> Cc: Timur Tabi <timur@codeaurora.org>
>> ---
>>  Documentation/admin-guide/kernel-parameters.txt |   6 +-
>>  arch/arm64/kernel/acpi.c                        | 128 ++++++++++++++++-
>>  drivers/acpi/Kconfig                            |   7 +-
>>  drivers/acpi/spcr.c                             | 175 ++++++------------------
>>  drivers/tty/serial/earlycon.c                   |  15 +-
>>  include/linux/acpi.h                            |  11 +-
>>  include/linux/serial_core.h                     |   2 -
>>  7 files changed, 184 insertions(+), 160 deletions(-)
>>
>> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
>> index 6571fbfdb2a1..0d173289c67e 100644
>> --- a/Documentation/admin-guide/kernel-parameters.txt
>> +++ b/Documentation/admin-guide/kernel-parameters.txt
>> @@ -914,9 +914,9 @@
>>  
>>  	earlycon=	[KNL] Output early console device and options.
>>  
>> -			When used with no options, the early console is
>> -			determined by the stdout-path property in device
>> -			tree's chosen node.
>> +			[ARM64] The early console is determined by the
>> +			stdout-path property in device tree's chosen node,
>> +			or determined by the ACPI SPCR table.
>>  
>>  		cdns,<addr>[,options]
>>  			Start an early, polled-mode console on a Cadence
>> diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c
>> index b3162715ed78..b3e33bbdf3b7 100644
>> --- a/arch/arm64/kernel/acpi.c
>> +++ b/arch/arm64/kernel/acpi.c
>> @@ -25,7 +25,6 @@
>>  #include <linux/memblock.h>
>>  #include <linux/of_fdt.h>
>>  #include <linux/smp.h>
>> -#include <linux/serial_core.h>
>>  
>>  #include <asm/cputype.h>
>>  #include <asm/cpu_ops.h>
>> @@ -177,6 +176,128 @@ static int __init acpi_fadt_sanity_check(void)
>>  	return ret;
>>  }
>>  
>> +/*
>> + * Erratum 44 for QDF2432v1 and QDF2400v1 SoCs describes the BUSY bit as
>> + * occasionally getting stuck as 1. To avoid the potential for a hang, check
>> + * TXFE == 0 instead of BUSY == 1. This may not be suitable for all UART
>> + * implementations, so only do so if an affected platform is detected in
>> + * acpi_parse_spcr().
>> + */
>> +bool qdf2400_e44_present;
>> +EXPORT_SYMBOL(qdf2400_e44_present);
> 
> My eyes, this is horrible but it is not introduced by this patch. It
> would have been much better if:
> 
> drivers/tty/serial/amba-pl011.c
> 
> parsed the SPCR table (again) to detect it instead of relying on this
> horrible exported flag.

It looks like Timur & you had a discussion and the current consensus is to keep
the code the same.

> 
>> +
>> +/*
>> + * Some Qualcomm Datacenter Technologies SoCs have a defective UART BUSY bit.
>> + * Detect them by examining the OEM fields in the SPCR header, similar to PCI
>> + * quirk detection in pci_mcfg.c.
>> + */
>> +static bool qdf2400_erratum_44_present(struct acpi_table_header *h)
>> +{
>> +	if (memcmp(h->oem_id, "QCOM  ", ACPI_OEM_ID_SIZE))
>> +		return false;
>> +
>> +	if (!memcmp(h->oem_table_id, "QDF2432 ", ACPI_OEM_TABLE_ID_SIZE))
>> +		return true;
>> +
>> +	if (!memcmp(h->oem_table_id, "QDF2400 ", ACPI_OEM_TABLE_ID_SIZE) &&
>> +			h->oem_revision == 1)
>> +		return true;
>> +
>> +	return false;
>> +}
>> +
>> +/*
>> + * APM X-Gene v1 and v2 UART hardware is an 16550 like device but has its
>> + * register aligned to 32-bit. In addition, the BIOS also encoded the
>> + * access width to be 8 bits. This function detects this errata condition.
>> + */
>> +static bool xgene_8250_erratum_present(struct acpi_table_spcr *tb)
>> +{
>> +	bool xgene_8250 = false;
>> +
>> +	if (tb->interface_type != ACPI_DBG2_16550_COMPATIBLE)
>> +		return false;
>> +
>> +	if (memcmp(tb->header.oem_id, "APMC0D", ACPI_OEM_ID_SIZE) &&
>> +	    memcmp(tb->header.oem_id, "HPE   ", ACPI_OEM_ID_SIZE))
>> +		return false;
>> +
>> +	if (!memcmp(tb->header.oem_table_id, "XGENESPC",
>> +	    ACPI_OEM_TABLE_ID_SIZE) && tb->header.oem_revision == 0)
>> +		xgene_8250 = true;
>> +
>> +	if (!memcmp(tb->header.oem_table_id, "ProLiant",
>> +	    ACPI_OEM_TABLE_ID_SIZE) && tb->header.oem_revision == 1)
>> +		xgene_8250 = true;
>> +
>> +	return xgene_8250;
>> +}
>> +
>> +int acpi_arch_setup_console(struct acpi_table_spcr *table,
>> +			    char *opts, char *uart, char *iotype,
>> +			    int baud_rate, bool earlycon)
>> +{
>> +	if (table->header.revision < 2) {
>> +		pr_err("wrong table version\n");
>> +		return -ENOENT;
>> +	}
>> +
>> +	switch (table->interface_type) {
>> +	case ACPI_DBG2_ARM_SBSA_32BIT:
>> +		snprintf(iotype, ACPI_SPCR_BUF_SIZE, "mmio32");
>> +		/* fall through */
>> +	case ACPI_DBG2_ARM_PL011:
>> +	case ACPI_DBG2_ARM_SBSA_GENERIC:
>> +	case ACPI_DBG2_BCM2835:
>> +		snprintf(uart, ACPI_SPCR_BUF_SIZE, "pl011");
>> +		break;
>> +	default:
>> +		if (strlen(uart) == 0)
>> +			return -ENOENT;
>> +	}
>> +
>> +	/*
>> +	 * If the E44 erratum is required, then we need to tell the pl011
>> +	 * driver to implement the work-around.
>> +	 *
>> +	 * The global variable is used by the probe function when it
>> +	 * creates the UARTs, whether or not they're used as a console.
>> +	 *
>> +	 * If the user specifies "traditional" earlycon, the qdf2400_e44
>> +	 * console name matches the EARLYCON_DECLARE() statement, and
>> +	 * SPCR is not used.  Parameter "earlycon" is false.
>> +	 *
>> +	 * If the user specifies "SPCR" earlycon, then we need to update
>> +	 * the console name so that it also says "qdf2400_e44".  Parameter
>> +	 * "earlycon" is true.
>> +	 *
>> +	 * For consistency, if we change the console name, then we do it
>> +	 * for everyone, not just earlycon.
>> +	 */
>> +	if (qdf2400_erratum_44_present(&table->header)) {
>> +		qdf2400_e44_present = true;
>> +		if (earlycon)
>> +			snprintf(uart, ACPI_SPCR_BUF_SIZE, "qdf2400_e44");
>> +	}
>> +
>> +	if (xgene_8250_erratum_present(table)) {
>> +		snprintf(iotype, ACPI_SPCR_BUF_SIZE, "mmio32");
>> +
>> +		/* for xgene v1 and v2 we don't know the clock rate of the
>> +		 * UART so don't attempt to change to the baud rate state
>> +		 * in the table because driver cannot calculate the dividers
>> +		 */
>> +		snprintf(opts, ACPI_SPCR_OPTS_SIZE, "%s,%s,0x%llx", uart,
>> +			 iotype, table->serial_port.address);
>> +	} else {
>> +		snprintf(opts, ACPI_SPCR_OPTS_SIZE, "%s,%s,0x%llx,%d", uart,
>> +			 iotype, table->serial_port.address, baud_rate);
>> +	}
>> +
>> +	return 0;
>> +}
>> +EXPORT_SYMBOL(acpi_arch_setup_console);
> 
> EXPORT_SYMBOL() ? Why ?
> 
> BTW, why do we need an arch hook ? I do not see anything that prevents
> you from using this code on x86 systems - is there anything arch
> specific in the SPCR specification itself ?

See above comment.  But also keep in mind I have seen a lot of x86 systems that
do not define the ACPI table version correctly.  The table version is 0 or 1,
and the current spec says it must be 2.  It looks like ARM requires 2.

> 
>> +
>>  /*
>>   * acpi_boot_table_init() called from setup_arch(), always.
>>   *	1. find RSDP and get its address, and then find XSDT
>> @@ -230,10 +351,11 @@ void __init acpi_boot_table_init(void)
>>  
>>  done:
>>  	if (acpi_disabled) {
>> -		if (earlycon_init_is_deferred)
>> +		if (console_acpi_spcr_enable)
>>  			early_init_dt_scan_chosen_stdout();
>>  	} else {
>> -		parse_spcr(earlycon_init_is_deferred);
>> +		/* Always enable the ACPI SPCR console */
>> +		acpi_parse_spcr(console_acpi_spcr_enable);
>>  		if (IS_ENABLED(CONFIG_ACPI_BGRT))
>>  			acpi_table_parse(ACPI_SIG_BGRT, acpi_parse_bgrt);
>>  	}
>> diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
>> index 46505396869e..9ae98eeada76 100644
>> --- a/drivers/acpi/Kconfig
>> +++ b/drivers/acpi/Kconfig
>> @@ -79,7 +79,12 @@ config ACPI_DEBUGGER_USER
>>  endif
>>  
>>  config ACPI_SPCR_TABLE
>> -	bool
>> +	bool "ACPI Serial Port Console Redirection Support"
>> +	default y if ARM64
> 
> You need to remove the selection in arch/arm64 then. Also, moving away
> from a non-visible config may have consequences on ARM64, Graeme and
> Mark are more familiar with the SPCR dependencies so please chime in.

Ok, I'll double check this.

> 
>> +	help
>> +	  Enable support for Serial Port Console Redirection (SPCR) Table.
>> +	  This table provides information about the configuration of the
>> +	  earlycon console.
>>  
>>  config ACPI_LPIT
>>  	bool
>> diff --git a/drivers/acpi/spcr.c b/drivers/acpi/spcr.c
>> index 324b35bfe781..f4bb8110e404 100644
>> --- a/drivers/acpi/spcr.c
>> +++ b/drivers/acpi/spcr.c
>> @@ -16,65 +16,18 @@
>>  #include <linux/kernel.h>
>>  #include <linux/serial_core.h>
>>  
>> -/*
>> - * Erratum 44 for QDF2432v1 and QDF2400v1 SoCs describes the BUSY bit as
>> - * occasionally getting stuck as 1. To avoid the potential for a hang, check
>> - * TXFE == 0 instead of BUSY == 1. This may not be suitable for all UART
>> - * implementations, so only do so if an affected platform is detected in
>> - * parse_spcr().
>> - */
>> -bool qdf2400_e44_present;
>> -EXPORT_SYMBOL(qdf2400_e44_present);
>> -
>> -/*
>> - * Some Qualcomm Datacenter Technologies SoCs have a defective UART BUSY bit.
>> - * Detect them by examining the OEM fields in the SPCR header, similiar to PCI
>> - * quirk detection in pci_mcfg.c.
>> - */
>> -static bool qdf2400_erratum_44_present(struct acpi_table_header *h)
>> -{
>> -	if (memcmp(h->oem_id, "QCOM  ", ACPI_OEM_ID_SIZE))
>> -		return false;
>> -
>> -	if (!memcmp(h->oem_table_id, "QDF2432 ", ACPI_OEM_TABLE_ID_SIZE))
>> -		return true;
>> -
>> -	if (!memcmp(h->oem_table_id, "QDF2400 ", ACPI_OEM_TABLE_ID_SIZE) &&
>> -			h->oem_revision == 1)
>> -		return true;
>> -
>> -	return false;
>> -}
>> -
>> -/*
>> - * APM X-Gene v1 and v2 UART hardware is an 16550 like device but has its
>> - * register aligned to 32-bit. In addition, the BIOS also encoded the
>> - * access width to be 8 bits. This function detects this errata condition.
>> - */
>> -static bool xgene_8250_erratum_present(struct acpi_table_spcr *tb)
>> +int __weak acpi_arch_setup_console(struct acpi_table_spcr *table,
>> +				   char *opts, char *uart, char *iotype,
>> +				   int baud_rate, bool earlycon)
>>  {
>> -	bool xgene_8250 = false;
>> -
>> -	if (tb->interface_type != ACPI_DBG2_16550_COMPATIBLE)
>> -		return false;
>> -
>> -	if (memcmp(tb->header.oem_id, "APMC0D", ACPI_OEM_ID_SIZE) &&
>> -	    memcmp(tb->header.oem_id, "HPE   ", ACPI_OEM_ID_SIZE))
>> -		return false;
>> -
>> -	if (!memcmp(tb->header.oem_table_id, "XGENESPC",
>> -	    ACPI_OEM_TABLE_ID_SIZE) && tb->header.oem_revision == 0)
>> -		xgene_8250 = true;
>> -
>> -	if (!memcmp(tb->header.oem_table_id, "ProLiant",
>> -	    ACPI_OEM_TABLE_ID_SIZE) && tb->header.oem_revision == 1)
>> -		xgene_8250 = true;
>> -
>> -	return xgene_8250;
>> +	snprintf(opts, ACPI_SPCR_OPTS_SIZE, "%s,%s,0x%llx,%d", uart, iotype,
>> +		 table->serial_port.address, baud_rate);
>> +	return 0;
>>  }
>>  
>> +bool console_acpi_spcr_enable __initdata;
>>  /**
>> - * parse_spcr() - parse ACPI SPCR table and add preferred console
>> + * acpi_parse_spcr() - parse ACPI SPCR table and add preferred console
>>   *
>>   * @earlycon: set up earlycon for the console specified by the table
>>   *
>> @@ -86,13 +39,13 @@ static bool xgene_8250_erratum_present(struct acpi_table_spcr *tb)
>>   * from arch initialization code as soon as the DT/ACPI decision is made.
>>   *
>>   */
>> -int __init parse_spcr(bool earlycon)
>> +int __init acpi_parse_spcr(bool earlycon)
>>  {
>> -	static char opts[64];
>> +	static char opts[ACPI_SPCR_OPTS_SIZE];
>> +	static char uart[ACPI_SPCR_BUF_SIZE];
>> +	static char iotype[ACPI_SPCR_BUF_SIZE];
>>  	struct acpi_table_spcr *table;
>>  	acpi_status status;
>> -	char *uart;
>> -	char *iotype;
>>  	int baud_rate;
>>  	int err;
>>  
>> @@ -105,48 +58,6 @@ int __init parse_spcr(bool earlycon)
>>  	if (ACPI_FAILURE(status))
>>  		return -ENOENT;
>>  
>> -	if (table->header.revision < 2) {
>> -		err = -ENOENT;
>> -		pr_err("wrong table version\n");
>> -		goto done;
>> -	}
>> -
>> -	if (table->serial_port.space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY) {
>> -		switch (ACPI_ACCESS_BIT_WIDTH((
>> -			table->serial_port.access_width))) {
>> -		default:
>> -			pr_err("Unexpected SPCR Access Width.  Defaulting to byte size\n");
>> -		case 8:
>> -			iotype = "mmio";
>> -			break;
>> -		case 16:
>> -			iotype = "mmio16";
>> -			break;
>> -		case 32:
>> -			iotype = "mmio32";
>> -			break;
>> -		}
>> -	} else
>> -		iotype = "io";
>> -
>> -	switch (table->interface_type) {
>> -	case ACPI_DBG2_ARM_SBSA_32BIT:
>> -		iotype = "mmio32";
>> -		/* fall through */
>> -	case ACPI_DBG2_ARM_PL011:
>> -	case ACPI_DBG2_ARM_SBSA_GENERIC:
>> -	case ACPI_DBG2_BCM2835:
>> -		uart = "pl011";
>> -		break;
>> -	case ACPI_DBG2_16550_COMPATIBLE:
>> -	case ACPI_DBG2_16550_SUBSET:
>> -		uart = "uart";
>> -		break;
>> -	default:
>> -		err = -ENOENT;
>> -		goto done;
>> -	}
>> -
>>  	switch (table->baud_rate) {
>>  	case 3:
>>  		baud_rate = 9600;
>> @@ -165,43 +76,36 @@ int __init parse_spcr(bool earlycon)
>>  		goto done;
>>  	}
>>  
>> -	/*
>> -	 * If the E44 erratum is required, then we need to tell the pl011
>> -	 * driver to implement the work-around.
>> -	 *
>> -	 * The global variable is used by the probe function when it
>> -	 * creates the UARTs, whether or not they're used as a console.
>> -	 *
>> -	 * If the user specifies "traditional" earlycon, the qdf2400_e44
>> -	 * console name matches the EARLYCON_DECLARE() statement, and
>> -	 * SPCR is not used.  Parameter "earlycon" is false.
>> -	 *
>> -	 * If the user specifies "SPCR" earlycon, then we need to update
>> -	 * the console name so that it also says "qdf2400_e44".  Parameter
>> -	 * "earlycon" is true.
>> -	 *
>> -	 * For consistency, if we change the console name, then we do it
>> -	 * for everyone, not just earlycon.
>> -	 */
>> -	if (qdf2400_erratum_44_present(&table->header)) {
>> -		qdf2400_e44_present = true;
>> -		if (earlycon)
>> -			uart = "qdf2400_e44";
>> +	switch (table->interface_type) {
>> +	case ACPI_DBG2_16550_COMPATIBLE:
>> +	case ACPI_DBG2_16550_SUBSET:
>> +		snprintf(uart, ACPI_SPCR_BUF_SIZE, "uart");
>> +		break;
>> +	default:
>> +		break;
>>  	}
>>  
>> -	if (xgene_8250_erratum_present(table)) {
>> -		iotype = "mmio32";
>> +	if (table->serial_port.space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY) {
>> +		u8 width = ACPI_ACCESS_BIT_WIDTH((
>> +					table->serial_port.access_width));
>> +		switch (width) {
>> +		default:
>> +			pr_err("Unexpected SPCR Access Width.  Defaulting to byte size\n");
>> +		case 8:
>> +			snprintf(iotype, ACPI_SPCR_BUF_SIZE, "mmio");
>> +			break;
>> +		case 16:
>> +		case 32:
>> +			snprintf(iotype, ACPI_SPCR_BUF_SIZE, "mmio%d", width);
>> +			break;
>> +		}
>> +	} else
>> +		snprintf(iotype, ACPI_SPCR_BUF_SIZE, "io");
>>  
>> -		/* for xgene v1 and v2 we don't know the clock rate of the
>> -		 * UART so don't attempt to change to the baud rate state
>> -		 * in the table because driver cannot calculate the dividers
>> -		 */
>> -		snprintf(opts, sizeof(opts), "%s,%s,0x%llx", uart, iotype,
>> -			 table->serial_port.address);
>> -	} else {
>> -		snprintf(opts, sizeof(opts), "%s,%s,0x%llx,%d", uart, iotype,
>> -			 table->serial_port.address, baud_rate);
>> -	}
>> +	err = acpi_arch_setup_console(table, opts, uart, iotype, baud_rate,
>> +				      earlycon);
>> +	if (err)
>> +		goto done;
>>  
>>  	pr_info("console: %s\n", opts);
>>  
>> @@ -209,7 +113,6 @@ int __init parse_spcr(bool earlycon)
>>  		setup_earlycon(opts);
>>  
>>  	err = add_preferred_console(uart, 0, opts + strlen(uart) + 1);
>> -
> 
> Unintended change.
> 
>>  done:
>>  	acpi_put_table((struct acpi_table_header *)table);
>>  	return err;
>> diff --git a/drivers/tty/serial/earlycon.c b/drivers/tty/serial/earlycon.c
>> index 4c8b80f1c688..b22afb62c7a3 100644
>> --- a/drivers/tty/serial/earlycon.c
>> +++ b/drivers/tty/serial/earlycon.c
>> @@ -196,26 +196,15 @@ int __init setup_earlycon(char *buf)
>>  	return -ENOENT;
>>  }
>>  
>> -/*
>> - * When CONFIG_ACPI_SPCR_TABLE is defined, "earlycon" without parameters in
>> - * command line does not start DT earlycon immediately, instead it defers
>> - * starting it until DT/ACPI decision is made.  At that time if ACPI is enabled
>> - * call parse_spcr(), else call early_init_dt_scan_chosen_stdout()
>> - */
>> -bool earlycon_init_is_deferred __initdata;
>> -
>>  /* early_param wrapper for setup_earlycon() */
>>  static int __init param_setup_earlycon(char *buf)
>>  {
>>  	int err;
>>  
>> -	/*
>> -	 * Just 'earlycon' is a valid param for devicetree earlycons;
>> -	 * don't generate a warning from parse_early_params() in that case
>> -	 */
>> +	/* Just 'earlycon' is a valid param for devicetree and ACPI SPCR. */
>>  	if (!buf || !buf[0]) {
>>  		if (IS_ENABLED(CONFIG_ACPI_SPCR_TABLE)) {
>> -			earlycon_init_is_deferred = true;
>> +			console_acpi_spcr_enable = true;
> 
> I am not familiar with this code, I would ask Graeme and Mark to check
> if this change is correct, the logic seems correct to me but I may be
> missing some corner cases.
> 
>>  			return 0;
>>  		} else if (!buf) {
>>  			return early_init_dt_scan_chosen_stdout();
>> diff --git a/include/linux/acpi.h b/include/linux/acpi.h
>> index dc1ebfeeb5ec..875d7327d91c 100644
>> --- a/include/linux/acpi.h
>> +++ b/include/linux/acpi.h
>> @@ -1241,10 +1241,17 @@ static inline bool acpi_has_watchdog(void) { return false; }
>>  #endif
>>  
>>  #ifdef CONFIG_ACPI_SPCR_TABLE
>> +#define ACPI_SPCR_OPTS_SIZE 64
>> +#define ACPI_SPCR_BUF_SIZE 32
>>  extern bool qdf2400_e44_present;
>> -int parse_spcr(bool earlycon);
>> +extern bool console_acpi_spcr_enable __initdata;
>> +extern int acpi_arch_setup_console(struct acpi_table_spcr *table,
>> +				   char *opts, char *uart, char *iotype,
>> +				   int baud_rate, bool earlycon);
>> +int acpi_parse_spcr(bool earlycon);
>>  #else
>> -static inline int parse_spcr(bool earlycon) { return 0; }
>> +static const bool console_acpi_spcr_enable;
> 
> The assignment in param_setup_earlycon won't compile. 

Hmm ... I'm pretty sure it did.  But I'll check that before resubmitting.

P.

> 
> Lorenzo
>