From 405f2296892c10a48db50cd66c2eb364cde0806e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20Niew=C3=B6hner?= Date: Mon, 21 Dec 2020 03:46:58 +0100 Subject: [PATCH] soc/intel/*: drop UART pad configuration from common code MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit UART pad configuration should not be done in common code, because that may cause short circuits, when the user sets a wrong UART index. Since all boards do pad setup on their own now, finally drop the pad configuration from SoC common code. Change-Id: Id03719eb8bd0414083148471ed05dea62a895126 Signed-off-by: Michael Niewöhner Reviewed-on: https://review.coreboot.org/c/coreboot/+/48829 Tested-by: build bot (Jenkins) Reviewed-by: Lance Zhao --- src/include/device/pci_type.h | 3 +- src/soc/intel/alderlake/uart.c | 39 ++----------- src/soc/intel/apollolake/uart.c | 57 +++---------------- .../intel/cannonlake/bootblock/bootblock.c | 1 + src/soc/intel/cannonlake/uart.c | 40 ++----------- .../common/block/include/intelblocks/uart.h | 13 +---- src/soc/intel/common/block/uart/Kconfig | 2 - src/soc/intel/common/block/uart/uart.c | 39 +++---------- src/soc/intel/elkhartlake/uart.c | 40 ++----------- src/soc/intel/icelake/uart.c | 40 ++----------- src/soc/intel/jasperlake/uart.c | 40 ++----------- src/soc/intel/skylake/uart.c | 39 ++----------- src/soc/intel/tigerlake/uart.c | 40 ++----------- 13 files changed, 61 insertions(+), 332 deletions(-) diff --git a/src/include/device/pci_type.h b/src/include/device/pci_type.h index 713016b547..267f950864 100644 --- a/src/include/device/pci_type.h +++ b/src/include/device/pci_type.h @@ -16,7 +16,8 @@ typedef u32 pci_devfn_t; (((DEV) & 0x1F) << 15) | \ (((FN) & 0x07) << 12)) -#define PCI_DEV_INVALID (0xffffffffU) +#define PCI_DEV_INVALID (0xffffffffU) +#define PCI_DEVFN_INVALID (0xffffffffU) #if 1 /* FIXME: For most of the time in ramstage, we get valid device pointer diff --git a/src/soc/intel/alderlake/uart.c b/src/soc/intel/alderlake/uart.c index e1a77b3181..2344a6a717 100644 --- a/src/soc/intel/alderlake/uart.c +++ b/src/soc/intel/alderlake/uart.c @@ -6,40 +6,13 @@ * Chapter number: 9 */ -#include -#include -#include -#include -#include -#include +#include #include -#include -const struct uart_controller_config uart_ctrlr_config[] = { - { - .console_index = 0, - .devfn = PCH_DEVFN_UART0, - .gpios = { - PAD_CFG_NF(GPP_H10, NONE, DEEP, NF2), /* UART0 RX */ - PAD_CFG_NF(GPP_H11, NONE, DEEP, NF2), /* UART0 TX */ - }, - }, - { - .console_index = 1, - .devfn = PCH_DEVFN_UART1, - .gpios = { - PAD_CFG_NF(GPP_C12, NONE, DEEP, NF1), /* UART1 RX */ - PAD_CFG_NF(GPP_C13, NONE, DEEP, NF1), /* UART1 TX */ - }, - }, - { - .console_index = 2, - .devfn = PCH_DEVFN_UART2, - .gpios = { - PAD_CFG_NF(GPP_C20, NONE, DEEP, NF1), /* UART2 RX */ - PAD_CFG_NF(GPP_C21, NONE, DEEP, NF1), /* UART2 TX */ - }, - } +const unsigned int uart_devices[] = { + PCH_DEVFN_UART0, + PCH_DEVFN_UART1, + PCH_DEVFN_UART2, }; -const int uart_ctrlr_config_size = ARRAY_SIZE(uart_ctrlr_config); +const int uart_devices_size = ARRAY_SIZE(uart_devices); diff --git a/src/soc/intel/apollolake/uart.c b/src/soc/intel/apollolake/uart.c index b8758f56e6..348c2876ad 100644 --- a/src/soc/intel/apollolake/uart.c +++ b/src/soc/intel/apollolake/uart.c @@ -6,59 +6,18 @@ * shouldn't cause any fragmentation. */ -#include -#include +#include +#include #include -const struct uart_controller_config uart_ctrlr_config[] = { +const unsigned int uart_devices[] = { + PCH_DEVFN_UART0, #if CONFIG(SOC_INTEL_GEMINILAKE) - { - .console_index = 0, - .devfn = PCH_DEVFN_UART0, - .gpios = { - PAD_CFG_NF_IOSSTATE_IOSTERM(GPIO_60, NATIVE, DEEP, NF1, - HIZCRx1, DISPUPD), /* LPSS_UART0_RXD */ - PAD_CFG_NF_IOSSTATE_IOSTERM(GPIO_61, NATIVE, DEEP, NF1, - HIZCRx1, DISPUPD), /* LPSS_UART0_TXD */ - - }, - }, - { - .console_index = 2, - .devfn = PCH_DEVFN_UART2, - .gpios = { - PAD_CFG_NF_IOSSTATE_IOSTERM(GPIO_64, NATIVE, DEEP, NF1, - HIZCRx1, DISPUPD), /* LPSS_UART2_RXD */ - PAD_CFG_NF_IOSSTATE_IOSTERM(GPIO_65, NATIVE, DEEP, NF1, - HIZCRx1, DISPUPD), /* LPSS_UART2_TXD */ - }, - }, + PCI_DEVFN_INVALID, #else - { - .console_index = 0, - .devfn = PCH_DEVFN_UART0, - .gpios = { - PAD_CFG_NF(GPIO_38, NATIVE, DEEP, NF1), /* UART0 RX */ - PAD_CFG_NF(GPIO_39, NATIVE, DEEP, NF1), /* UART0 TX */ - }, - }, - { - .console_index = 1, - .devfn = PCH_DEVFN_UART1, - .gpios = { - PAD_CFG_NF(GPIO_42, NATIVE, DEEP, NF1), /* UART1 RX */ - PAD_CFG_NF(GPIO_43, NATIVE, DEEP, NF1), /* UART1 TX */ - }, - }, - { - .console_index = 2, - .devfn = PCH_DEVFN_UART2, - .gpios = { - PAD_CFG_NF(GPIO_46, NATIVE, DEEP, NF1), /* UART2 RX */ - PAD_CFG_NF(GPIO_47, NATIVE, DEEP, NF1), /* UART2 TX */ - }, - }, + PCH_DEVFN_UART1, #endif + PCH_DEVFN_UART2, }; -const int uart_ctrlr_config_size = ARRAY_SIZE(uart_ctrlr_config); +const int uart_devices_size = ARRAY_SIZE(uart_devices); diff --git a/src/soc/intel/cannonlake/bootblock/bootblock.c b/src/soc/intel/cannonlake/bootblock/bootblock.c index 1354c43a22..01329bf9b1 100644 --- a/src/soc/intel/cannonlake/bootblock/bootblock.c +++ b/src/soc/intel/cannonlake/bootblock/bootblock.c @@ -3,6 +3,7 @@ #include #include #include +#include #include #include #include diff --git a/src/soc/intel/cannonlake/uart.c b/src/soc/intel/cannonlake/uart.c index da0306c9fa..9946880263 100644 --- a/src/soc/intel/cannonlake/uart.c +++ b/src/soc/intel/cannonlake/uart.c @@ -1,40 +1,12 @@ /* SPDX-License-Identifier: GPL-2.0-only */ -#include -#include -#include -#include -#include -#include -#include +#include #include -#include -const struct uart_controller_config uart_ctrlr_config[] = { - { - .console_index = 0, - .devfn = PCH_DEVFN_UART0, - .gpios = { - PAD_CFG_NF(GPP_C8, NONE, DEEP, NF1), /* UART0 RX */ - PAD_CFG_NF(GPP_C9, NONE, DEEP, NF1), /* UART0 TX */ - }, - }, - { - .console_index = 1, - .devfn = PCH_DEVFN_UART1, - .gpios = { - PAD_CFG_NF(GPP_C12, NONE, DEEP, NF1), /* UART1 RX */ - PAD_CFG_NF(GPP_C13, NONE, DEEP, NF1), /* UART1 TX */ - }, - }, - { - .console_index = 2, - .devfn = PCH_DEVFN_UART2, - .gpios = { - PAD_CFG_NF(GPP_C20, NONE, DEEP, NF1), /* UART2 RX */ - PAD_CFG_NF(GPP_C21, NONE, DEEP, NF1), /* UART2 TX */ - }, - } +const unsigned int uart_devices[] = { + PCH_DEVFN_UART0, + PCH_DEVFN_UART1, + PCH_DEVFN_UART2, }; -const int uart_ctrlr_config_size = ARRAY_SIZE(uart_ctrlr_config); +const int uart_devices_size = ARRAY_SIZE(uart_devices); diff --git a/src/soc/intel/common/block/include/intelblocks/uart.h b/src/soc/intel/common/block/include/intelblocks/uart.h index c3a22bc96b..f140fc87fd 100644 --- a/src/soc/intel/common/block/include/intelblocks/uart.h +++ b/src/soc/intel/common/block/include/intelblocks/uart.h @@ -5,24 +5,13 @@ #include #include -#include #include -#define MAX_GPIO_PAD_PER_UART 2 - -struct uart_controller_config { - int console_index; - /* devfn in PCI_DEVFN() format */ - unsigned int devfn; - struct pad_config gpios[MAX_GPIO_PAD_PER_UART]; -}; - /* * While using this common UART block for any SOC following is expected from soc * 1. SOC will define proper UART_BASE which is base address for UART console. * 2. SOC will return correct device pointer based on console index - * 3. SOC will provide appropriate GPIO pad configuration for UART console - * 4. SOC will allow common code to set UART into legacy mode if supported. + * 3. SOC will allow common code to set UART into legacy mode if supported. */ /* diff --git a/src/soc/intel/common/block/uart/Kconfig b/src/soc/intel/common/block/uart/Kconfig index 3437ec7477..7ba22b5c3e 100644 --- a/src/soc/intel/common/block/uart/Kconfig +++ b/src/soc/intel/common/block/uart/Kconfig @@ -13,7 +13,5 @@ config INTEL_LPSS_UART_FOR_CONSOLE help Selected by mainboards that use one of the SoC's LPSS UARTS for the coreboot console. - WARNING: UART_FOR_CONSOLE has to be set to a correct value, - otherwise wrong pad configurations might be selected. endif diff --git a/src/soc/intel/common/block/uart/uart.c b/src/soc/intel/common/block/uart/uart.c index 4a84935ad4..9b34031fe6 100644 --- a/src/soc/intel/common/block/uart/uart.c +++ b/src/soc/intel/common/block/uart/uart.c @@ -18,10 +18,9 @@ #include "chip.h" #define UART_PCI_ENABLE (PCI_COMMAND_MEMORY | PCI_COMMAND_MASTER) -#define UART_CONSOLE_INVALID_INDEX 0xFF -extern const struct uart_controller_config uart_ctrlr_config[]; -extern const int uart_ctrlr_config_size; +extern const unsigned int uart_devices[]; +extern const int uart_devices_size; static void uart_lpss_init(pci_devfn_t dev, uintptr_t baseaddr) { @@ -45,23 +44,9 @@ uintptr_t uart_platform_base(unsigned int idx) } #endif -static int uart_get_valid_index(void) -{ - int index; - - for (index = 0; index < uart_ctrlr_config_size; index++) { - if (uart_ctrlr_config[index].console_index == - CONFIG_UART_FOR_CONSOLE) - return index; - } - /* For valid index, code should not reach here */ - return UART_CONSOLE_INVALID_INDEX; -} - static pci_devfn_t uart_console_get_pci_bdf(void) { int devfn; - int index; /* * This function will get called even if INTEL_LPSS_UART_FOR_CONSOLE @@ -71,11 +56,13 @@ static pci_devfn_t uart_console_get_pci_bdf(void) if (!CONFIG(INTEL_LPSS_UART_FOR_CONSOLE)) return PCI_DEV_INVALID; - index = uart_get_valid_index(); - if (index == UART_CONSOLE_INVALID_INDEX) + if (CONFIG_UART_FOR_CONSOLE > uart_devices_size) + return PCI_DEV_INVALID; + + devfn = uart_devices[CONFIG_UART_FOR_CONSOLE]; + if (devfn == PCI_DEVFN_INVALID) return PCI_DEV_INVALID; - devfn = uart_ctrlr_config[index].devfn; return PCI_DEV(0, PCI_SLOT(devfn), PCI_FUNC(devfn)); } @@ -107,15 +94,6 @@ bool uart_is_controller_initialized(void) return !lpss_is_controller_in_reset(base); } -static void uart_configure_gpio_pads(void) -{ - int index = uart_get_valid_index(); - - if (index != UART_CONSOLE_INVALID_INDEX) - gpio_configure_pads(uart_ctrlr_config[index].gpios, - MAX_GPIO_PAD_PER_UART); -} - void uart_bootblock_init(void) { const uint32_t baseaddr = CONFIG_CONSOLE_UART_BASE_ADDRESS; @@ -131,9 +109,6 @@ void uart_bootblock_init(void) pci_s_write_config16(dev, PCI_COMMAND, UART_PCI_ENABLE); uart_lpss_init(dev, baseaddr); - - /* Configure the 2 pads per UART. */ - uart_configure_gpio_pads(); } #if ENV_RAMSTAGE diff --git a/src/soc/intel/elkhartlake/uart.c b/src/soc/intel/elkhartlake/uart.c index da0306c9fa..9946880263 100644 --- a/src/soc/intel/elkhartlake/uart.c +++ b/src/soc/intel/elkhartlake/uart.c @@ -1,40 +1,12 @@ /* SPDX-License-Identifier: GPL-2.0-only */ -#include -#include -#include -#include -#include -#include -#include +#include #include -#include -const struct uart_controller_config uart_ctrlr_config[] = { - { - .console_index = 0, - .devfn = PCH_DEVFN_UART0, - .gpios = { - PAD_CFG_NF(GPP_C8, NONE, DEEP, NF1), /* UART0 RX */ - PAD_CFG_NF(GPP_C9, NONE, DEEP, NF1), /* UART0 TX */ - }, - }, - { - .console_index = 1, - .devfn = PCH_DEVFN_UART1, - .gpios = { - PAD_CFG_NF(GPP_C12, NONE, DEEP, NF1), /* UART1 RX */ - PAD_CFG_NF(GPP_C13, NONE, DEEP, NF1), /* UART1 TX */ - }, - }, - { - .console_index = 2, - .devfn = PCH_DEVFN_UART2, - .gpios = { - PAD_CFG_NF(GPP_C20, NONE, DEEP, NF1), /* UART2 RX */ - PAD_CFG_NF(GPP_C21, NONE, DEEP, NF1), /* UART2 TX */ - }, - } +const unsigned int uart_devices[] = { + PCH_DEVFN_UART0, + PCH_DEVFN_UART1, + PCH_DEVFN_UART2, }; -const int uart_ctrlr_config_size = ARRAY_SIZE(uart_ctrlr_config); +const int uart_devices_size = ARRAY_SIZE(uart_devices); diff --git a/src/soc/intel/icelake/uart.c b/src/soc/intel/icelake/uart.c index da0306c9fa..9946880263 100644 --- a/src/soc/intel/icelake/uart.c +++ b/src/soc/intel/icelake/uart.c @@ -1,40 +1,12 @@ /* SPDX-License-Identifier: GPL-2.0-only */ -#include -#include -#include -#include -#include -#include -#include +#include #include -#include -const struct uart_controller_config uart_ctrlr_config[] = { - { - .console_index = 0, - .devfn = PCH_DEVFN_UART0, - .gpios = { - PAD_CFG_NF(GPP_C8, NONE, DEEP, NF1), /* UART0 RX */ - PAD_CFG_NF(GPP_C9, NONE, DEEP, NF1), /* UART0 TX */ - }, - }, - { - .console_index = 1, - .devfn = PCH_DEVFN_UART1, - .gpios = { - PAD_CFG_NF(GPP_C12, NONE, DEEP, NF1), /* UART1 RX */ - PAD_CFG_NF(GPP_C13, NONE, DEEP, NF1), /* UART1 TX */ - }, - }, - { - .console_index = 2, - .devfn = PCH_DEVFN_UART2, - .gpios = { - PAD_CFG_NF(GPP_C20, NONE, DEEP, NF1), /* UART2 RX */ - PAD_CFG_NF(GPP_C21, NONE, DEEP, NF1), /* UART2 TX */ - }, - } +const unsigned int uart_devices[] = { + PCH_DEVFN_UART0, + PCH_DEVFN_UART1, + PCH_DEVFN_UART2, }; -const int uart_ctrlr_config_size = ARRAY_SIZE(uart_ctrlr_config); +const int uart_devices_size = ARRAY_SIZE(uart_devices); diff --git a/src/soc/intel/jasperlake/uart.c b/src/soc/intel/jasperlake/uart.c index da0306c9fa..9946880263 100644 --- a/src/soc/intel/jasperlake/uart.c +++ b/src/soc/intel/jasperlake/uart.c @@ -1,40 +1,12 @@ /* SPDX-License-Identifier: GPL-2.0-only */ -#include -#include -#include -#include -#include -#include -#include +#include #include -#include -const struct uart_controller_config uart_ctrlr_config[] = { - { - .console_index = 0, - .devfn = PCH_DEVFN_UART0, - .gpios = { - PAD_CFG_NF(GPP_C8, NONE, DEEP, NF1), /* UART0 RX */ - PAD_CFG_NF(GPP_C9, NONE, DEEP, NF1), /* UART0 TX */ - }, - }, - { - .console_index = 1, - .devfn = PCH_DEVFN_UART1, - .gpios = { - PAD_CFG_NF(GPP_C12, NONE, DEEP, NF1), /* UART1 RX */ - PAD_CFG_NF(GPP_C13, NONE, DEEP, NF1), /* UART1 TX */ - }, - }, - { - .console_index = 2, - .devfn = PCH_DEVFN_UART2, - .gpios = { - PAD_CFG_NF(GPP_C20, NONE, DEEP, NF1), /* UART2 RX */ - PAD_CFG_NF(GPP_C21, NONE, DEEP, NF1), /* UART2 TX */ - }, - } +const unsigned int uart_devices[] = { + PCH_DEVFN_UART0, + PCH_DEVFN_UART1, + PCH_DEVFN_UART2, }; -const int uart_ctrlr_config_size = ARRAY_SIZE(uart_ctrlr_config); +const int uart_devices_size = ARRAY_SIZE(uart_devices); diff --git a/src/soc/intel/skylake/uart.c b/src/soc/intel/skylake/uart.c index 6eb8c3f893..9946880263 100644 --- a/src/soc/intel/skylake/uart.c +++ b/src/soc/intel/skylake/uart.c @@ -1,39 +1,12 @@ /* SPDX-License-Identifier: GPL-2.0-only */ -#include -#include -#include -#include -#include -#include +#include #include -#include -const struct uart_controller_config uart_ctrlr_config[] = { - { - .console_index = 0, - .devfn = PCH_DEVFN_UART0, - .gpios = { - PAD_CFG_NF(GPP_C8, NONE, DEEP, NF1), /* UART0 RX */ - PAD_CFG_NF(GPP_C9, NONE, DEEP, NF1), /* UART0 TX */ - }, - }, - { - .console_index = 1, - .devfn = PCH_DEVFN_UART1, - .gpios = { - PAD_CFG_NF(GPP_C12, NONE, DEEP, NF1), /* UART1 RX */ - PAD_CFG_NF(GPP_C13, NONE, DEEP, NF1), /* UART1 TX */ - }, - }, - { - .console_index = 2, - .devfn = PCH_DEVFN_UART2, - .gpios = { - PAD_CFG_NF(GPP_C20, NONE, DEEP, NF1), /* UART2 RX */ - PAD_CFG_NF(GPP_C21, NONE, DEEP, NF1), /* UART2 TX */ - }, - } +const unsigned int uart_devices[] = { + PCH_DEVFN_UART0, + PCH_DEVFN_UART1, + PCH_DEVFN_UART2, }; -const int uart_ctrlr_config_size = ARRAY_SIZE(uart_ctrlr_config); +const int uart_devices_size = ARRAY_SIZE(uart_devices); diff --git a/src/soc/intel/tigerlake/uart.c b/src/soc/intel/tigerlake/uart.c index da392f434b..fa4760c118 100644 --- a/src/soc/intel/tigerlake/uart.c +++ b/src/soc/intel/tigerlake/uart.c @@ -6,41 +6,13 @@ * Chapter number: 9 */ -#include -#include -#include -#include -#include -#include -#include +#include #include -#include -const struct uart_controller_config uart_ctrlr_config[] = { - { - .console_index = 0, - .devfn = PCH_DEVFN_UART0, - .gpios = { - PAD_CFG_NF(GPP_C8, NONE, DEEP, NF1), /* UART0 RX */ - PAD_CFG_NF(GPP_C9, NONE, DEEP, NF1), /* UART0 TX */ - }, - }, - { - .console_index = 1, - .devfn = PCH_DEVFN_UART1, - .gpios = { - PAD_CFG_NF(GPP_C12, NONE, DEEP, NF1), /* UART1 RX */ - PAD_CFG_NF(GPP_C13, NONE, DEEP, NF1), /* UART1 TX */ - }, - }, - { - .console_index = 2, - .devfn = PCH_DEVFN_UART2, - .gpios = { - PAD_CFG_NF(GPP_C20, NONE, DEEP, NF1), /* UART2 RX */ - PAD_CFG_NF(GPP_C21, NONE, DEEP, NF1), /* UART2 TX */ - }, - } +const unsigned int uart_devices[] = { + PCH_DEVFN_UART0, + PCH_DEVFN_UART1, + PCH_DEVFN_UART2, }; -const int uart_ctrlr_config_size = ARRAY_SIZE(uart_ctrlr_config); +const int uart_devices_size = ARRAY_SIZE(uart_devices);