Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752357AbdLMKI7 (ORCPT ); Wed, 13 Dec 2017 05:08:59 -0500 Received: from foss.arm.com ([217.140.101.70]:55368 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751121AbdLMKIv (ORCPT ); Wed, 13 Dec 2017 05:08:51 -0500 Date: Wed, 13 Dec 2017 10:08:59 +0000 From: Will Deacon To: "Rafael J. Wysocki" Cc: Prarit Bhargava , linux-acpi@vger.kernel.org, linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-pm@vger.kernel.org, linux-serial@vger.kernel.org, Bhupesh Sharma , Lv Zheng , Thomas Gleixner , Ingo Molnar , "H. Peter Anvin" , x86@kernel.org, Jonathan Corbet , Catalin Marinas , Timur Tabi , sudeep.holla@arm.com, hanjun.guo@linaro.org, lorenzo.pieralisi@arm.com Subject: Re: [PATCH v2 1/2] acpi, spcr: Make SPCR avialable to other architectures Message-ID: <20171213100858.GA27635@arm.com> References: <20171211155059.17062-1-prarit@redhat.com> <20171211155059.17062-2-prarit@redhat.com> <7352752.eljzo8O4u9@aspire.rjw.lan> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <7352752.eljzo8O4u9@aspire.rjw.lan> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 19341 Lines: 536 [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 > > 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 > > Cc: Lv Zheng > > Cc: Thomas Gleixner > > Cc: Ingo Molnar > > Cc: "H. Peter Anvin" > > Cc: x86@kernel.org > > Cc: Jonathan Corbet > > Cc: Catalin Marinas > > Cc: Will Deacon > > Cc: "Rafael J. Wysocki" > > Cc: Timur Tabi > > --- > > 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,[,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 > > #include > > #include > > -#include > > > > #include > > #include > > @@ -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 > > #include > > > > -/* > > - * 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 > > > > > >