Hello Subrata Banik, Arthur Heymans,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/37251
to review the following change.
Change subject: soc/intel/skl: Drop FSP_CAR remnants ......................................................................
soc/intel/skl: Drop FSP_CAR remnants
FSP-T support was abandoned long ago for Skylake. With FSP1.1 support also dropped now, it's more visible that this code is unused.
Change-Id: I83a9130ef403b498e2beea01749c178e547b0f08 Signed-off-by: Nico Huber nico.huber@secunet.com --- M src/soc/intel/skylake/Kconfig M src/soc/intel/skylake/Makefile.inc D src/soc/intel/skylake/fspcar.c 3 files changed, 0 insertions(+), 45 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/51/37251/1
diff --git a/src/soc/intel/skylake/Kconfig b/src/soc/intel/skylake/Kconfig index 032ded4..528fd4a 100644 --- a/src/soc/intel/skylake/Kconfig +++ b/src/soc/intel/skylake/Kconfig @@ -31,7 +31,6 @@ select CPU_INTEL_FIRMWARE_INTERFACE_TABLE select CPU_INTEL_COMMON_HYPERTHREADING select FSP_M_XIP - select FSP_T_XIP if FSP_CAR select GENERIC_GPIO_LIB select HAVE_FSP_GOP select INTEL_DESCRIPTOR_MODE_CAPABLE diff --git a/src/soc/intel/skylake/Makefile.inc b/src/soc/intel/skylake/Makefile.inc index b049e84..c093738 100644 --- a/src/soc/intel/skylake/Makefile.inc +++ b/src/soc/intel/skylake/Makefile.inc @@ -11,7 +11,6 @@ subdirs-y += ../../../cpu/x86/tsc
bootblock-y += bootblock/bootblock.c -bootblock-$(CONFIG_FSP_CAR) += fspcar.c bootblock-y += bootblock/cpu.c bootblock-y += i2c.c bootblock-y += bootblock/pch.c diff --git a/src/soc/intel/skylake/fspcar.c b/src/soc/intel/skylake/fspcar.c deleted file mode 100644 index 0d27f57..0000000 --- a/src/soc/intel/skylake/fspcar.c +++ /dev/null @@ -1,43 +0,0 @@ -/* - * This file is part of the coreboot project. - * - * Copyright (C) 2018 Intel Corp. - * - * This program is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License as published by - * the Free Software Foundation; either version 2 of the License, or - * (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - */ -#include <bootblock_common.h> -#include <FsptUpd.h> - -const FSPT_UPD temp_ram_init_params = { - .FspUpdHeader = { - .Signature = 0x545F4450554C424B, /* 'KBLUPD_T' */ - .Revision = 1, - .Reserved = {0}, - }, - .FsptCoreUpd = { - /* - * It is a requirement for firmware to have Firmware Interface Table - * (FIT), which contains pointers to each microcode update. - * The microcode update is loaded for all logical processors before - * cpu reset vector. - * - * All SoC since Gen-4 has above mechanism in place to load microcode - * even before hitting CPU reset vector. Hence skipping FSP-T loading - * microcode after CPU reset by passing '0' value to - * FSPT_UPD.MicrocodeRegionBase and FSPT_UPD.MicrocodeRegionLength. - */ - .MicrocodeRegionBase = 0, - .MicrocodeRegionLength = 0, - .CodeRegionBase = - (uint32_t)(0x100000000ULL - CONFIG_ROM_SIZE), - .CodeRegionSize = (uint32_t)CONFIG_ROM_SIZE, - }, -};
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37251 )
Change subject: soc/intel/skl: Drop FSP_CAR remnants ......................................................................
Patch Set 1: Code-Review+2
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/37251 )
Change subject: soc/intel/skl: Drop FSP_CAR remnants ......................................................................
soc/intel/skl: Drop FSP_CAR remnants
FSP-T support was abandoned long ago for Skylake. With FSP1.1 support also dropped now, it's more visible that this code is unused.
Change-Id: I83a9130ef403b498e2beea01749c178e547b0f08 Signed-off-by: Nico Huber nico.huber@secunet.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/37251 Reviewed-by: Arthur Heymans arthur@aheymans.xyz Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/soc/intel/skylake/Kconfig M src/soc/intel/skylake/Makefile.inc D src/soc/intel/skylake/fspcar.c 3 files changed, 0 insertions(+), 45 deletions(-)
Approvals: build bot (Jenkins): Verified Arthur Heymans: Looks good to me, approved
diff --git a/src/soc/intel/skylake/Kconfig b/src/soc/intel/skylake/Kconfig index 032ded4..528fd4a 100644 --- a/src/soc/intel/skylake/Kconfig +++ b/src/soc/intel/skylake/Kconfig @@ -31,7 +31,6 @@ select CPU_INTEL_FIRMWARE_INTERFACE_TABLE select CPU_INTEL_COMMON_HYPERTHREADING select FSP_M_XIP - select FSP_T_XIP if FSP_CAR select GENERIC_GPIO_LIB select HAVE_FSP_GOP select INTEL_DESCRIPTOR_MODE_CAPABLE diff --git a/src/soc/intel/skylake/Makefile.inc b/src/soc/intel/skylake/Makefile.inc index b049e84..c093738 100644 --- a/src/soc/intel/skylake/Makefile.inc +++ b/src/soc/intel/skylake/Makefile.inc @@ -11,7 +11,6 @@ subdirs-y += ../../../cpu/x86/tsc
bootblock-y += bootblock/bootblock.c -bootblock-$(CONFIG_FSP_CAR) += fspcar.c bootblock-y += bootblock/cpu.c bootblock-y += i2c.c bootblock-y += bootblock/pch.c diff --git a/src/soc/intel/skylake/fspcar.c b/src/soc/intel/skylake/fspcar.c deleted file mode 100644 index 0d27f57..0000000 --- a/src/soc/intel/skylake/fspcar.c +++ /dev/null @@ -1,43 +0,0 @@ -/* - * This file is part of the coreboot project. - * - * Copyright (C) 2018 Intel Corp. - * - * This program is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License as published by - * the Free Software Foundation; either version 2 of the License, or - * (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - */ -#include <bootblock_common.h> -#include <FsptUpd.h> - -const FSPT_UPD temp_ram_init_params = { - .FspUpdHeader = { - .Signature = 0x545F4450554C424B, /* 'KBLUPD_T' */ - .Revision = 1, - .Reserved = {0}, - }, - .FsptCoreUpd = { - /* - * It is a requirement for firmware to have Firmware Interface Table - * (FIT), which contains pointers to each microcode update. - * The microcode update is loaded for all logical processors before - * cpu reset vector. - * - * All SoC since Gen-4 has above mechanism in place to load microcode - * even before hitting CPU reset vector. Hence skipping FSP-T loading - * microcode after CPU reset by passing '0' value to - * FSPT_UPD.MicrocodeRegionBase and FSPT_UPD.MicrocodeRegionLength. - */ - .MicrocodeRegionBase = 0, - .MicrocodeRegionLength = 0, - .CodeRegionBase = - (uint32_t)(0x100000000ULL - CONFIG_ROM_SIZE), - .CodeRegionSize = (uint32_t)CONFIG_ROM_SIZE, - }, -};
-
Arthur Heymans (Code Review) -
Nico Huber (Code Review)
-
Patrick Georgi (Code Review)