Nico Huber has uploaded this change for review. ( https://review.coreboot.org/29573
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.
Also fixes a bug in `{cannonlake,icelake}/bootblock/pch.c` where the LPC forwarding for an i/o based UART could never have been written because the two checked Kconfigs were mutually exclusive (at least when you tried to link the final binaries together).
Change-Id: Ibe2ed3cab0bb04bb23989c22da45299f088c758b Signed-off-by: Nico Huber nico.h@gmx.de --- 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/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/cannonlake/bootblock/pch.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/icelake/bootblock/pch.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 21 files changed, 60 insertions(+), 103 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/73/29573/1
diff --git a/src/console/Kconfig b/src/console/Kconfig index e7e3298..3db7005 100644 --- a/src/console/Kconfig +++ b/src/console/Kconfig @@ -48,8 +48,16 @@ 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 79ee3d0..a2ecf60 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 @@ -28,7 +28,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 0c0fca9..4a412f1 100644 --- a/src/soc/intel/apollolake/Kconfig +++ b/src/soc/intel/apollolake/Kconfig @@ -169,14 +169,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 6168f86..e4fe453 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
verstage-y += car.c verstage-y += i2c.c @@ -82,7 +82,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 bc5c170..ccfe104 100644 --- a/src/soc/intel/apollolake/bootblock/bootblock.c +++ b/src/soc/intel/apollolake/bootblock/bootblock.c @@ -95,7 +95,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/romstage.c b/src/soc/intel/apollolake/romstage.c index c20097e..f36817f 100644 --- a/src/soc/intel/apollolake/romstage.c +++ b/src/soc/intel/apollolake/romstage.c @@ -279,7 +279,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 9452b6d..53e147d 100644 --- a/src/soc/intel/cannonlake/Kconfig +++ b/src/soc/intel/cannonlake/Kconfig @@ -77,23 +77,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 2452f50..bfee784 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 @@ -50,27 +50,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 4eeef59..fcd815d 100644 --- a/src/soc/intel/cannonlake/bootblock/bootblock.c +++ b/src/soc/intel/cannonlake/bootblock/bootblock.c @@ -50,7 +50,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/cannonlake/bootblock/pch.c b/src/soc/intel/cannonlake/bootblock/pch.c index 3ef084a..d4427c2 100644 --- a/src/soc/intel/cannonlake/bootblock/pch.c +++ b/src/soc/intel/cannonlake/bootblock/pch.c @@ -173,8 +173,7 @@ uint16_t dec_rng, dec_en = 0;
/* IO Decode Range */ - if (IS_ENABLED(CONFIG_DRIVERS_UART_8250IO) && - IS_ENABLED(CONFIG_UART_DEBUG)) { + if (IS_ENABLED(CONFIG_DRIVERS_UART_8250IO)) { dec_rng = COMA_RANGE | (COMB_RANGE << 4); dec_en = COMA_LPC_EN | COMB_LPC_EN; pci_write_config16(PCH_DEV_LPC, LPC_IO_DEC, dec_rng); 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 c4ee841..df2dbc3 100644 --- a/src/soc/intel/icelake/Kconfig +++ b/src/soc/intel/icelake/Kconfig @@ -62,23 +62,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/icelake/bootblock/pch.c b/src/soc/intel/icelake/bootblock/pch.c index 96722e2..ee07d92 100644 --- a/src/soc/intel/icelake/bootblock/pch.c +++ b/src/soc/intel/icelake/bootblock/pch.c @@ -151,8 +151,7 @@ uint16_t dec_rng, dec_en = 0;
/* IO Decode Range */ - if (IS_ENABLED(CONFIG_DRIVERS_UART_8250IO) && - IS_ENABLED(CONFIG_UART_DEBUG)) { + if (IS_ENABLED(CONFIG_DRIVERS_UART_8250IO)) { dec_rng = COMA_RANGE | (COMB_RANGE << 4); dec_en = COMA_LPC_EN | COMB_LPC_EN; pci_write_config16(PCH_DEV_LPC, LPC_IO_DEC, dec_rng); diff --git a/src/soc/intel/skylake/Kconfig b/src/soc/intel/skylake/Kconfig index 2db8217..834422d 100644 --- a/src/soc/intel/skylake/Kconfig +++ b/src/soc/intel/skylake/Kconfig @@ -191,22 +191,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 6baa568..18c07c4 100644 --- a/src/soc/intel/skylake/me.c +++ b/src/soc/intel/skylake/me.c @@ -240,7 +240,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 (Code Review)