Nico Huber has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/30832
Change subject: soc/intel: Clean mess around UART_DEBUG ......................................................................
soc/intel: Clean mess around UART_DEBUG
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.
Repush after revert. Includes a fix of dependencies for FIXED_UART_FOR_ CONSOLE.
Original-Change-Id: Ibe2ed3cab0bb04bb23989c22da45299f088c758b Original-Signed-off-by: Nico Huber nico.h@gmx.de Original-Reviewed-on: https://review.coreboot.org/c/29573 Original-Tested-by: build bot (Jenkins) no-reply@coreboot.org Original-Reviewed-by: Kyösti Mälkki kyosti.malkki@gmail.com
Change-Id: I0ad582b8d69807707ce7f9a287a12d0224f024fb Signed-off-by: Nico Huber nico.huber@secunet.com --- M src/console/Kconfig M src/mainboard/intel/leafhill/Kconfig M src/mainboard/intel/minnow3/Kconfig M src/soc/intel/apollolake/Kconfig M src/soc/intel/apollolake/Makefile.inc M src/soc/intel/apollolake/bootblock/bootblock.c M src/soc/intel/apollolake/cse.c M src/soc/intel/apollolake/romstage.c M src/soc/intel/cannonlake/Kconfig M src/soc/intel/cannonlake/Makefile.inc M src/soc/intel/cannonlake/bootblock/bootblock.c M src/soc/intel/common/block/uart/Kconfig M src/soc/intel/common/block/uart/uart.c M src/soc/intel/icelake/Kconfig M src/soc/intel/icelake/Makefile.inc M src/soc/intel/icelake/bootblock/bootblock.c M src/soc/intel/skylake/Kconfig M src/soc/intel/skylake/Makefile.inc M src/soc/intel/skylake/bootblock/bootblock.c M src/soc/intel/skylake/me.c 20 files changed, 59 insertions(+), 100 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/32/30832/1
diff --git a/src/console/Kconfig b/src/console/Kconfig index e7e3298..9cacab0 100644 --- a/src/console/Kconfig +++ b/src/console/Kconfig @@ -37,6 +37,13 @@ shown on the following menu line. Supporting multiple different types of UARTs in one build is not supported.
+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). + if CONSOLE_SERIAL
comment "I/O mapped, 8250-compatible" @@ -49,7 +56,8 @@ depends on HAVE_UART_SPECIAL
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 94d0493..69bfde7 100644 --- a/src/mainboard/intel/leafhill/Kconfig +++ b/src/mainboard/intel/leafhill/Kconfig @@ -5,7 +5,7 @@ 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 @@ 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 531036a..a787a2d 100644 --- a/src/mainboard/intel/minnow3/Kconfig +++ b/src/mainboard/intel/minnow3/Kconfig @@ -5,7 +5,7 @@ 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 @@ 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 167f5aa..dcf1b1a 100644 --- a/src/soc/intel/apollolake/Kconfig +++ b/src/soc/intel/apollolake/Kconfig @@ -171,14 +171,6 @@ 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 19ebe7c..822158c 100644 --- a/src/soc/intel/apollolake/Makefile.inc +++ b/src/soc/intel/apollolake/Makefile.inc @@ -18,14 +18,14 @@ 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 += 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 += 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 += 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 += 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 65eabaa..cf3e839 100644 --- a/src/soc/intel/apollolake/bootblock/bootblock.c +++ b/src/soc/intel/apollolake/bootblock/bootblock.c @@ -94,7 +94,7 @@ 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 d761a6c..0ff7dcc 100644 --- a/src/soc/intel/apollolake/cse.c +++ b/src/soc/intel/apollolake/cse.c @@ -222,7 +222,7 @@ * 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 4f4f9f5..9cc5166 100644 --- a/src/soc/intel/apollolake/romstage.c +++ b/src/soc/intel/apollolake/romstage.c @@ -278,7 +278,7 @@ 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 5b34a1f..a3390d8 100644 --- a/src/soc/intel/cannonlake/Kconfig +++ b/src/soc/intel/cannonlake/Kconfig @@ -78,23 +78,6 @@ 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 faa22f0..fb2c849 100644 --- a/src/soc/intel/cannonlake/Makefile.inc +++ b/src/soc/intel/cannonlake/Makefile.inc @@ -19,7 +19,7 @@ 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 += 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-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 b7d00cd..08a13ea 100644 --- a/src/soc/intel/cannonlake/bootblock/bootblock.c +++ b/src/soc/intel/cannonlake/bootblock/bootblock.c @@ -53,7 +53,7 @@ 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 f4a0e4e..e731465 100644 --- a/src/soc/intel/common/block/uart/Kconfig +++ b/src/soc/intel/common/block/uart/Kconfig @@ -15,3 +15,14 @@ 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 f7235cf..f0b6b24 100644 --- a/src/soc/intel/common/block/uart/uart.c +++ b/src/soc/intel/common/block/uart/uart.c @@ -88,12 +88,11 @@ 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 @@ 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 5484372..34af339 100644 --- a/src/soc/intel/icelake/Kconfig +++ b/src/soc/intel/icelake/Kconfig @@ -63,23 +63,6 @@ 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 a81edd4..74c9182 100644 --- a/src/soc/intel/icelake/Makefile.inc +++ b/src/soc/intel/icelake/Makefile.inc @@ -20,7 +20,7 @@ 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 += 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-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 40c2d41..d26fa42 100644 --- a/src/soc/intel/icelake/bootblock/bootblock.c +++ b/src/soc/intel/icelake/bootblock/bootblock.c @@ -32,7 +32,7 @@ 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 c2aecfd..7dcf8a9 100644 --- a/src/soc/intel/skylake/Kconfig +++ b/src/soc/intel/skylake/Kconfig @@ -182,22 +182,6 @@ 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 480c71e..ee2c928 100644 --- a/src/soc/intel/skylake/Makefile.inc +++ b/src/soc/intel/skylake/Makefile.inc @@ -22,13 +22,13 @@ 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 += 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 += 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 += 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 a2bcaaf..acf25ff 100644 --- a/src/soc/intel/skylake/bootblock/bootblock.c +++ b/src/soc/intel/skylake/bootblock/bootblock.c @@ -32,7 +32,7 @@ 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 64a94ab..f67086b 100644 --- a/src/soc/intel/skylake/me.c +++ b/src/soc/intel/skylake/me.c @@ -239,7 +239,7 @@ * 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);
Nico Huber has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/30832 )
Change subject: soc/intel: Clean mess around UART_DEBUG ......................................................................
Abandoned
revert was abandoned
-
Nico Huber (Code Review)