From a96e66a76f21c41b0c15db8d9df1d721f4a8a9af Mon Sep 17 00:00:00 2001 From: Nico Huber Date: Sun, 11 Nov 2018 02:51:14 +0100 Subject: [PATCH] soc/intel: Clean mess around UART_DEBUG MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Everything is wrong here, the Kconfig symbols are only the tip of the iceberg. Based on Kconfig prompts the SoC code performed pad configu- rations! I don't see why the person who configures coreboot should have the board schematics at hand. As a mitigation, we remove the prompts for UART_DEBUG, which is renamed to INTEL_LPSS_UART_FOR_CONSOLE (because the former didn't really say what it's about), and for UART_FOR_CONSOLE in case the former is selec- ted. Change-Id: Ibe2ed3cab0bb04bb23989c22da45299f088c758b Signed-off-by: Nico Huber Reviewed-on: https://review.coreboot.org/c/29573 Tested-by: build bot (Jenkins) Reviewed-by: Kyösti Mälkki --- src/console/Kconfig | 10 +++++++++- src/mainboard/intel/leafhill/Kconfig | 3 +-- src/mainboard/intel/minnow3/Kconfig | 3 +-- src/soc/intel/apollolake/Kconfig | 8 -------- src/soc/intel/apollolake/Makefile.inc | 12 ++++++------ src/soc/intel/apollolake/bootblock/bootblock.c | 2 +- src/soc/intel/apollolake/cse.c | 2 +- src/soc/intel/apollolake/romstage.c | 2 +- src/soc/intel/cannonlake/Kconfig | 17 ----------------- src/soc/intel/cannonlake/Makefile.inc | 12 ++++++------ src/soc/intel/cannonlake/bootblock/bootblock.c | 2 +- src/soc/intel/common/block/uart/Kconfig | 11 +++++++++++ src/soc/intel/common/block/uart/uart.c | 12 ++++++------ src/soc/intel/icelake/Kconfig | 17 ----------------- src/soc/intel/icelake/Makefile.inc | 12 ++++++------ src/soc/intel/icelake/bootblock/bootblock.c | 2 +- src/soc/intel/skylake/Kconfig | 16 ---------------- src/soc/intel/skylake/Makefile.inc | 12 ++++++------ src/soc/intel/skylake/bootblock/bootblock.c | 2 +- src/soc/intel/skylake/me.c | 2 +- 20 files changed, 59 insertions(+), 100 deletions(-) diff --git a/src/console/Kconfig b/src/console/Kconfig index e7e329848a..3db7005a10 100644 --- a/src/console/Kconfig +++ b/src/console/Kconfig @@ -48,8 +48,16 @@ if CONSOLE_SERIAL comment "device-specific UART" depends on HAVE_UART_SPECIAL +config FIXED_UART_FOR_CONSOLE + bool + help + Select to remove the prompt from UART_FOR_CONSOLE in case a + specific UART has to be used (e.g. when the platform code + performs dangerous configurations). + config UART_FOR_CONSOLE - int "Index for UART port to use for console" + int + prompt "Index for UART port to use for console" if !FIXED_UART_FOR_CONSOLE default 0 help Select an I/O port to use for serial console: diff --git a/src/mainboard/intel/leafhill/Kconfig b/src/mainboard/intel/leafhill/Kconfig index 94d0493953..69bfde764f 100644 --- a/src/mainboard/intel/leafhill/Kconfig +++ b/src/mainboard/intel/leafhill/Kconfig @@ -5,7 +5,7 @@ config BOARD_SPECIFIC_OPTIONS select SOC_INTEL_APOLLOLAKE select BOARD_ROMSIZE_KB_16384 select HAVE_ACPI_TABLES - select UART_DEBUG + select INTEL_LPSS_UART_FOR_CONSOLE config MAINBOARD_DIR string @@ -24,7 +24,6 @@ config FMDFILE default "src/mainboard/$(CONFIG_MAINBOARD_DIR)/leafhill.$(CONFIG_COREBOOT_ROMSIZE_KB).fmd" config UART_FOR_CONSOLE - int "Number of UART port to use for serial log" default 2 config NEED_IFWI diff --git a/src/mainboard/intel/minnow3/Kconfig b/src/mainboard/intel/minnow3/Kconfig index 531036a6e3..a787a2d571 100644 --- a/src/mainboard/intel/minnow3/Kconfig +++ b/src/mainboard/intel/minnow3/Kconfig @@ -5,7 +5,7 @@ config BOARD_SPECIFIC_OPTIONS select SOC_INTEL_APOLLOLAKE select BOARD_ROMSIZE_KB_16384 select HAVE_ACPI_TABLES - select UART_DEBUG + select INTEL_LPSS_UART_FOR_CONSOLE config MAINBOARD_DIR string @@ -20,7 +20,6 @@ config FMDFILE default "src/mainboard/$(CONFIG_MAINBOARD_DIR)/minnow3.fmd" config UART_FOR_CONSOLE - int "Number of UART port to use for serial log" default 2 config NEED_IFWI diff --git a/src/soc/intel/apollolake/Kconfig b/src/soc/intel/apollolake/Kconfig index 4a841be9ad..7a5a11c840 100644 --- a/src/soc/intel/apollolake/Kconfig +++ b/src/soc/intel/apollolake/Kconfig @@ -169,14 +169,6 @@ config DRIVERS_I2C_DESIGNWARE_CLOCK_MHZ int default 133 -config UART_DEBUG - bool "Enable SoC UART debug port selected by UART_FOR_CONSOLE." - default n - select CONSOLE_SERIAL - select DRIVERS_UART - select DRIVERS_UART_8250MEM_32 - select NO_UART_ON_SUPERIO - # 32KiB bootblock is all that is mapped in by the CSE at top of 4GiB. config C_ENV_BOOTBLOCK_SIZE hex diff --git a/src/soc/intel/apollolake/Makefile.inc b/src/soc/intel/apollolake/Makefile.inc index 19ebe7c55b..822158c3c1 100644 --- a/src/soc/intel/apollolake/Makefile.inc +++ b/src/soc/intel/apollolake/Makefile.inc @@ -18,14 +18,14 @@ bootblock-y += lpc.c bootblock-y += mmap_boot.c bootblock-y += pmutil.c bootblock-y += spi.c -bootblock-$(CONFIG_UART_DEBUG) += uart.c +bootblock-y += uart.c romstage-y += car.c romstage-$(CONFIG_PLATFORM_USES_FSP2_0) += romstage.c romstage-y += gspi.c romstage-y += heci.c romstage-y += i2c.c -romstage-$(CONFIG_UART_DEBUG) += uart.c +romstage-y += uart.c romstage-y += memmap.c romstage-y += meminit.c ifeq ($(CONFIG_SOC_INTEL_GLK),y) @@ -42,7 +42,7 @@ smm-y += mmap_boot.c smm-y += pmutil.c smm-y += smihandler.c smm-y += spi.c -smm-$(CONFIG_UART_DEBUG) += uart.c +smm-y += uart.c smm-y += elog.c ramstage-$(CONFIG_HAVE_ACPI_TABLES) += acpi.c @@ -57,7 +57,7 @@ ramstage-y += i2c.c ramstage-y += lpc.c ramstage-y += memmap.c ramstage-y += mmap_boot.c -ramstage-$(CONFIG_UART_DEBUG) += uart.c +ramstage-y += uart.c ramstage-y += nhlt.c ramstage-y += spi.c ramstage-y += systemagent.c @@ -74,7 +74,7 @@ postcar-y += spi.c postcar-y += i2c.c postcar-$(CONFIG_RESET_ON_INVALID_RAMSTAGE_CACHE) += heci.c postcar-$(CONFIG_RESET_ON_INVALID_RAMSTAGE_CACHE) += reset.c -postcar-$(CONFIG_UART_DEBUG) += uart.c +postcar-y += uart.c postcar-$(CONFIG_VBOOT_MEASURED_BOOT) += gspi.c verstage-y += car.c @@ -83,7 +83,7 @@ verstage-y += gspi.c verstage-y += heci.c verstage-y += memmap.c verstage-y += mmap_boot.c -verstage-$(CONFIG_UART_DEBUG) += uart.c +verstage-y += uart.c verstage-y += pmutil.c verstage-y += reset.c verstage-y += spi.c diff --git a/src/soc/intel/apollolake/bootblock/bootblock.c b/src/soc/intel/apollolake/bootblock/bootblock.c index ac3e0cc0ea..ce89f133ee 100644 --- a/src/soc/intel/apollolake/bootblock/bootblock.c +++ b/src/soc/intel/apollolake/bootblock/bootblock.c @@ -95,7 +95,7 @@ void bootblock_soc_early_init(void) pmc_global_reset_enable(0); /* Prepare UART for serial console. */ - if (IS_ENABLED(CONFIG_UART_DEBUG)) + if (IS_ENABLED(CONFIG_INTEL_LPSS_UART_FOR_CONSOLE)) uart_bootblock_init(); if (IS_ENABLED(CONFIG_TPM_ON_FAST_SPI)) diff --git a/src/soc/intel/apollolake/cse.c b/src/soc/intel/apollolake/cse.c index d761a6c6d8..0ff7dcc1ed 100644 --- a/src/soc/intel/apollolake/cse.c +++ b/src/soc/intel/apollolake/cse.c @@ -222,7 +222,7 @@ static void dump_cse_version(void *unused) * Print ME version only if UART debugging is enabled. Else, it takes * ~0.6 second to talk to ME and get this information. */ - if (!IS_ENABLED(CONFIG_UART_DEBUG)) + if (!IS_ENABLED(CONFIG_CONSOLE_SERIAL)) return; msg.mkhi_hdr.fields.group_id = MKHI_GROUP_ID_GEN; diff --git a/src/soc/intel/apollolake/romstage.c b/src/soc/intel/apollolake/romstage.c index 4f4f9f53c5..9cc516665a 100644 --- a/src/soc/intel/apollolake/romstage.c +++ b/src/soc/intel/apollolake/romstage.c @@ -278,7 +278,7 @@ asmlinkage void car_stage_entry(void) static void fill_console_params(FSPM_UPD *mupd) { if (IS_ENABLED(CONFIG_CONSOLE_SERIAL)) { - if (IS_ENABLED(CONFIG_UART_DEBUG)) { + if (IS_ENABLED(CONFIG_INTEL_LPSS_UART_FOR_CONSOLE)) { mupd->FspmConfig.SerialDebugPortDevice = CONFIG_UART_FOR_CONSOLE; /* use MMIO port type */ diff --git a/src/soc/intel/cannonlake/Kconfig b/src/soc/intel/cannonlake/Kconfig index 9e007b656e..f8193cd754 100644 --- a/src/soc/intel/cannonlake/Kconfig +++ b/src/soc/intel/cannonlake/Kconfig @@ -77,23 +77,6 @@ config CPU_SPECIFIC_OPTIONS select DISPLAY_FSP_VERSION_INFO select FSP_T_XIP if FSP_CAR -config UART_DEBUG - bool "Enable UART debug port." - default n - select CONSOLE_SERIAL - select BOOTBLOCK_CONSOLE - select DRIVERS_UART - select DRIVERS_UART_8250MEM_32 - select NO_UART_ON_SUPERIO - -config UART_FOR_CONSOLE - int "Index for LPSS UART port to use for console" - default 2 if DRIVERS_UART_8250MEM_32 - default 0 - help - Index for LPSS UART port to use for console: - 0 = LPSS UART0, 1 = LPSS UART1, 2 = LPSS UART2 - config DCACHE_RAM_BASE default 0xfef00000 diff --git a/src/soc/intel/cannonlake/Makefile.inc b/src/soc/intel/cannonlake/Makefile.inc index faa22f0501..fb2c849abb 100644 --- a/src/soc/intel/cannonlake/Makefile.inc +++ b/src/soc/intel/cannonlake/Makefile.inc @@ -19,7 +19,7 @@ bootblock-y += memmap.c bootblock-y += spi.c bootblock-y += lpc.c bootblock-y += p2sb.c -bootblock-$(CONFIG_UART_DEBUG) += uart.c +bootblock-y += uart.c romstage-$(CONFIG_SOC_INTEL_CANNONLAKE_MEMCFG_INIT) += cnl_memcfg_init.c romstage-y += gspi.c @@ -29,7 +29,7 @@ romstage-y += memmap.c romstage-y += pmutil.c romstage-y += reset.c romstage-y += spi.c -romstage-$(CONFIG_UART_DEBUG) += uart.c +romstage-y += uart.c ramstage-y += acpi.c ramstage-y += chip.c @@ -51,27 +51,27 @@ ramstage-$(CONFIG_PLATFORM_USES_FSP2_0) += reset.c ramstage-y += smmrelocate.c ramstage-y += spi.c ramstage-y += systemagent.c -ramstage-$(CONFIG_UART_DEBUG) += uart.c +ramstage-y += uart.c ramstage-y += vr_config.c ramstage-y += sd.c smm-y += p2sb.c smm-y += pmutil.c smm-y += smihandler.c -smm-$(CONFIG_UART_DEBUG) += uart.c +smm-y += uart.c postcar-y += memmap.c postcar-y += pmutil.c postcar-y += i2c.c postcar-y += gspi.c postcar-y += spi.c -postcar-$(CONFIG_UART_DEBUG) += uart.c +postcar-y += uart.c verstage-y += gspi.c verstage-y += i2c.c verstage-y += pmutil.c verstage-y += spi.c -verstage-$(CONFIG_UART_DEBUG) += uart.c +verstage-y += uart.c ifeq ($(CONFIG_SOC_INTEL_CANNONLAKE_PCH_H),y) bootblock-y += gpio_cnp_h.c diff --git a/src/soc/intel/cannonlake/bootblock/bootblock.c b/src/soc/intel/cannonlake/bootblock/bootblock.c index b7d00cd063..08a13ea860 100644 --- a/src/soc/intel/cannonlake/bootblock/bootblock.c +++ b/src/soc/intel/cannonlake/bootblock/bootblock.c @@ -53,7 +53,7 @@ void bootblock_soc_early_init(void) bootblock_pch_early_init(); bootblock_cpu_init(); pch_early_iorange_init(); - if (IS_ENABLED(CONFIG_UART_DEBUG)) + if (IS_ENABLED(CONFIG_INTEL_LPSS_UART_FOR_CONSOLE)) uart_bootblock_init(); } diff --git a/src/soc/intel/common/block/uart/Kconfig b/src/soc/intel/common/block/uart/Kconfig index f4a0e4e4b9..e731465b3b 100644 --- a/src/soc/intel/common/block/uart/Kconfig +++ b/src/soc/intel/common/block/uart/Kconfig @@ -15,3 +15,14 @@ config SOC_INTEL_COMMON_BLOCK_UART_LPSS_CLK_N_VAL hex help Clock m-divisor value for m/n divider + +config INTEL_LPSS_UART_FOR_CONSOLE + bool + depends on SOC_INTEL_COMMON_BLOCK_UART + select DRIVERS_UART_8250MEM_32 + select FIXED_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. diff --git a/src/soc/intel/common/block/uart/uart.c b/src/soc/intel/common/block/uart/uart.c index f7235cfe92..f0b6b24b70 100644 --- a/src/soc/intel/common/block/uart/uart.c +++ b/src/soc/intel/common/block/uart/uart.c @@ -88,12 +88,11 @@ void uart_common_init(struct device *device, uintptr_t baseaddr) struct device *uart_get_device(void) { /* - * This function will get called even if UART_DEBUG config options is - * not selected. - * By default returning NULL in case CONFIG_UART_DEBUG option is not - * selected to avoid compilation errors. + * This function will get called even if INTEL_LPSS_UART_FOR_CONSOLE + * config option is not selected. + * By default return NULL in this case to avoid compilation errors. */ - if (!IS_ENABLED(CONFIG_UART_DEBUG)) + if (!IS_ENABLED(CONFIG_INTEL_LPSS_UART_FOR_CONSOLE)) return NULL; int console_index = uart_get_valid_index(); @@ -157,7 +156,8 @@ static void uart_read_resources(struct device *dev) pci_dev_read_resources(dev); /* Set the configured UART base address for the debug port */ - if (IS_ENABLED(CONFIG_UART_DEBUG) && uart_is_debug_controller(dev)) { + if (IS_ENABLED(CONFIG_INTEL_LPSS_UART_FOR_CONSOLE) && + uart_is_debug_controller(dev)) { struct resource *res = find_resource(dev, PCI_BASE_ADDRESS_0); /* Need to set the base and size for the resource allocator. */ res->base = UART_BASE(CONFIG_UART_FOR_CONSOLE); diff --git a/src/soc/intel/icelake/Kconfig b/src/soc/intel/icelake/Kconfig index 6343ca5d0f..1d687f0224 100644 --- a/src/soc/intel/icelake/Kconfig +++ b/src/soc/intel/icelake/Kconfig @@ -62,23 +62,6 @@ config CPU_SPECIFIC_OPTIONS select UDK_2017_BINDING select DISPLAY_FSP_VERSION_INFO -config UART_DEBUG - bool "Enable UART debug port." - default n - select CONSOLE_SERIAL - select BOOTBLOCK_CONSOLE - select DRIVERS_UART - select DRIVERS_UART_8250MEM_32 - select NO_UART_ON_SUPERIO - -config UART_FOR_CONSOLE - int "Index for LPSS UART port to use for console" - default 2 if DRIVERS_UART_8250MEM_32 - default 0 - help - Index for LPSS UART port to use for console: - 0 = LPSS UART0, 1 = LPSS UART1, 2 = LPSS UART2 - config DCACHE_RAM_BASE default 0xfef00000 diff --git a/src/soc/intel/icelake/Makefile.inc b/src/soc/intel/icelake/Makefile.inc index a81edd46a0..74c9182067 100644 --- a/src/soc/intel/icelake/Makefile.inc +++ b/src/soc/intel/icelake/Makefile.inc @@ -20,7 +20,7 @@ bootblock-y += memmap.c bootblock-y += spi.c bootblock-y += lpc.c bootblock-y += p2sb.c -bootblock-$(CONFIG_UART_DEBUG) += uart.c +bootblock-y += uart.c romstage-y += gpio.c romstage-y += gspi.c @@ -30,7 +30,7 @@ romstage-y += memmap.c romstage-y += pmutil.c romstage-y += reset.c romstage-y += spi.c -romstage-$(CONFIG_UART_DEBUG) += uart.c +romstage-y += uart.c ramstage-y += acpi.c ramstage-y += chip.c @@ -52,27 +52,27 @@ ramstage-$(CONFIG_PLATFORM_USES_FSP2_0) += reset.c ramstage-y += smmrelocate.c ramstage-y += spi.c ramstage-y += systemagent.c -ramstage-$(CONFIG_UART_DEBUG) += uart.c +ramstage-y += uart.c ramstage-y += sd.c smm-y += gpio.c smm-y += p2sb.c smm-y += pmutil.c smm-y += smihandler.c -smm-$(CONFIG_UART_DEBUG) += uart.c +smm-y += uart.c postcar-y += memmap.c postcar-y += pmutil.c postcar-y += i2c.c postcar-y += gspi.c postcar-y += spi.c -postcar-$(CONFIG_UART_DEBUG) += uart.c +postcar-y += uart.c verstage-y += gspi.c verstage-y += i2c.c verstage-y += pmutil.c verstage-y += spi.c -verstage-$(CONFIG_UART_DEBUG) += uart.c +verstage-y += uart.c CPPFLAGS_common += -I$(src)/soc/intel/icelake CPPFLAGS_common += -I$(src)/soc/intel/icelake/include diff --git a/src/soc/intel/icelake/bootblock/bootblock.c b/src/soc/intel/icelake/bootblock/bootblock.c index 40c2d41b7a..d26fa4210d 100644 --- a/src/soc/intel/icelake/bootblock/bootblock.c +++ b/src/soc/intel/icelake/bootblock/bootblock.c @@ -32,7 +32,7 @@ void bootblock_soc_early_init(void) bootblock_pch_early_init(); bootblock_cpu_init(); pch_early_iorange_init(); - if (IS_ENABLED(CONFIG_UART_DEBUG)) + if (IS_ENABLED(CONFIG_INTEL_LPSS_UART_FOR_CONSOLE)) uart_bootblock_init(); } diff --git a/src/soc/intel/skylake/Kconfig b/src/soc/intel/skylake/Kconfig index c2aecfd5e4..7dcf8a997d 100644 --- a/src/soc/intel/skylake/Kconfig +++ b/src/soc/intel/skylake/Kconfig @@ -182,22 +182,6 @@ config VGA_BIOS_ID string default "8086,0406" -config UART_DEBUG - bool "Enable UART debug port." - default n - select CONSOLE_SERIAL - select DRIVERS_UART - select DRIVERS_UART_8250MEM_32 - select NO_UART_ON_SUPERIO - -config UART_FOR_CONSOLE - int "Index for LPSS UART port to use for console" - default 2 if DRIVERS_UART_8250MEM - default 0 - help - Index for LPSS UART port to use for console: - 0 = LPSS UART0, 1 = LPSS UART1, 2 = LPSS UART2 - config SKYLAKE_SOC_PCH_H bool default n diff --git a/src/soc/intel/skylake/Makefile.inc b/src/soc/intel/skylake/Makefile.inc index 480c71ecfb..ee2c928464 100644 --- a/src/soc/intel/skylake/Makefile.inc +++ b/src/soc/intel/skylake/Makefile.inc @@ -22,13 +22,13 @@ bootblock-y += p2sb.c bootblock-y += pmutil.c bootblock-y += spi.c bootblock-y += lpc.c -bootblock-$(CONFIG_UART_DEBUG) += uart.c +bootblock-y += uart.c verstage-y += gspi.c verstage-y += pmutil.c verstage-y += i2c.c verstage-y += spi.c -verstage-$(CONFIG_UART_DEBUG) += uart.c +verstage-y += uart.c romstage-y += gpio.c romstage-y += gspi.c @@ -40,7 +40,7 @@ romstage-y += pmc.c romstage-y += pmutil.c romstage-$(CONFIG_PLATFORM_USES_FSP2_0) += reset.c romstage-y += spi.c -romstage-$(CONFIG_UART_DEBUG) += uart.c +romstage-y += uart.c ramstage-$(CONFIG_HAVE_ACPI_TABLES) += acpi.c ramstage-$(CONFIG_PLATFORM_USES_FSP1_1) += chip.c @@ -67,7 +67,7 @@ ramstage-y += smmrelocate.c ramstage-y += spi.c ramstage-y += systemagent.c ramstage-y += thermal.c -ramstage-$(CONFIG_UART_DEBUG) += uart.c +ramstage-y += uart.c ramstage-y += vr_config.c smm-y += elog.c @@ -75,13 +75,13 @@ smm-y += gpio.c smm-y += p2sb.c smm-y += pmutil.c smm-y += smihandler.c -smm-$(CONFIG_UART_DEBUG) += uart.c +smm-y += uart.c postcar-y += memmap.c postcar-y += gspi.c postcar-y += spi.c postcar-y += i2c.c -postcar-$(CONFIG_UART_DEBUG) += uart.c +postcar-y += uart.c # Skylake D0 diff --git a/src/soc/intel/skylake/bootblock/bootblock.c b/src/soc/intel/skylake/bootblock/bootblock.c index a2bcaaf9af..acf25ffd48 100644 --- a/src/soc/intel/skylake/bootblock/bootblock.c +++ b/src/soc/intel/skylake/bootblock/bootblock.c @@ -32,7 +32,7 @@ void bootblock_soc_early_init(void) bootblock_cpu_init(); pch_early_iorange_init(); - if (IS_ENABLED(CONFIG_UART_DEBUG)) + if (IS_ENABLED(CONFIG_INTEL_LPSS_UART_FOR_CONSOLE)) uart_bootblock_init(); } diff --git a/src/soc/intel/skylake/me.c b/src/soc/intel/skylake/me.c index 64a94ab0f7..f67086b22e 100644 --- a/src/soc/intel/skylake/me.c +++ b/src/soc/intel/skylake/me.c @@ -239,7 +239,7 @@ static void print_me_version(void *unused) * Print ME version only if UART debugging is enabled. Else, it takes ~1 * second to talk to ME and get this information. */ - if (!IS_ENABLED(CONFIG_UART_DEBUG)) + if (!IS_ENABLED(CONFIG_CONSOLE_SERIAL)) return; hfs.data = me_read_config32(PCI_ME_HFSTS1);