Hello Felix Singer, V Sowmya, Nico Huber, Furquan Shaikh, Tim Wawrzynczak, Subrata Banik, Arthur Heymans, Michael Niewöhner, Patrick Rudolph,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/48578
to review the following change.
Change subject: soc/intel/braswell: Use Kconfig value for TSEG size ......................................................................
soc/intel/braswell: Use Kconfig value for TSEG size
SoC selects HAVE_SMI_HANDLER, so TsegSize is always set to 8 MiB. Also, use SMM_TSEG_SIZE in place of a magic number.
Change-Id: I139e1073426051fea5d30b6ce3dd9746e0e985a2 Signed-off-by: Angel Pons th3fanbus@gmail.com --- M src/soc/intel/braswell/romstage/romstage.c 1 file changed, 2 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/78/48578/1
diff --git a/src/soc/intel/braswell/romstage/romstage.c b/src/soc/intel/braswell/romstage/romstage.c index 37ee93c..1738679 100644 --- a/src/soc/intel/braswell/romstage/romstage.c +++ b/src/soc/intel/braswell/romstage/romstage.c @@ -2,6 +2,7 @@
#include <cbmem.h> #include <stdint.h> +#include <commonlib/helpers.h> #include <arch/io.h> #include <device/mmio.h> #include <console/console.h> @@ -113,7 +114,7 @@ config = config_of(dev); printk(BIOS_DEBUG, "Updating UPD values for MemoryInit\n");
- upd->PcdMrcInitTsegSize = CONFIG(HAVE_SMI_HANDLER) ? 8 : 0; + upd->PcdMrcInitTsegSize = CONFIG_SMM_TSEG_SIZE / MiB; upd->PcdMrcInitMmioSize = 0x800; upd->PcdMrcInitSpdAddr1 = config->PcdMrcInitSpdAddr1; upd->PcdMrcInitSpdAddr2 = config->PcdMrcInitSpdAddr2;
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48578 )
Change subject: soc/intel/braswell: Use Kconfig value for TSEG size ......................................................................
Patch Set 1: Code-Review+2
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48578 )
Change subject: soc/intel/braswell: Use Kconfig value for TSEG size ......................................................................
Patch Set 2: Code-Review+2
Frans Hendriks has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48578 )
Change subject: soc/intel/braswell: Use Kconfig value for TSEG size ......................................................................
Patch Set 2: Code-Review+2
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48578 )
Change subject: soc/intel/braswell: Use Kconfig value for TSEG size ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/48578/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/48578/2//COMMIT_MSG@10 PS2, Line 10: use SMM_TSEG_SIZE in place of a magic number. Another perspective; there are things other than SMI handler in TSEG, so the original !HAVE_SMI_HANDLER => TSEG_SIZE=0 was just wrong.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48578 )
Change subject: soc/intel/braswell: Use Kconfig value for TSEG size ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/48578/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/48578/2//COMMIT_MSG@10 PS2, Line 10: use SMM_TSEG_SIZE in place of a magic number.
Another perspective; there are things other than SMI handler in TSEG, so the original !HAVE_SMI_HAND […]
Yeah, I'm pretty sure no one tested this code without HAVE_SMI_HANDLER anyway
Angel Pons has submitted this change. ( https://review.coreboot.org/c/coreboot/+/48578 )
Change subject: soc/intel/braswell: Use Kconfig value for TSEG size ......................................................................
soc/intel/braswell: Use Kconfig value for TSEG size
SoC selects HAVE_SMI_HANDLER, so TsegSize is always set to 8 MiB. Also, use SMM_TSEG_SIZE in place of a magic number.
Change-Id: I139e1073426051fea5d30b6ce3dd9746e0e985a2 Signed-off-by: Angel Pons th3fanbus@gmail.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/48578 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Michael Niewöhner foss@mniewoehner.de Reviewed-by: Frans Hendriks fhendriks@eltan.com Reviewed-by: Furquan Shaikh furquan@google.com --- M src/soc/intel/braswell/romstage/romstage.c 1 file changed, 2 insertions(+), 1 deletion(-)
Approvals: build bot (Jenkins): Verified Furquan Shaikh: Looks good to me, approved Frans Hendriks: Looks good to me, approved Michael Niewöhner: Looks good to me, approved
diff --git a/src/soc/intel/braswell/romstage/romstage.c b/src/soc/intel/braswell/romstage/romstage.c index 37ee93c..1738679 100644 --- a/src/soc/intel/braswell/romstage/romstage.c +++ b/src/soc/intel/braswell/romstage/romstage.c @@ -2,6 +2,7 @@
#include <cbmem.h> #include <stdint.h> +#include <commonlib/helpers.h> #include <arch/io.h> #include <device/mmio.h> #include <console/console.h> @@ -113,7 +114,7 @@ config = config_of(dev); printk(BIOS_DEBUG, "Updating UPD values for MemoryInit\n");
- upd->PcdMrcInitTsegSize = CONFIG(HAVE_SMI_HANDLER) ? 8 : 0; + upd->PcdMrcInitTsegSize = CONFIG_SMM_TSEG_SIZE / MiB; upd->PcdMrcInitMmioSize = 0x800; upd->PcdMrcInitSpdAddr1 = config->PcdMrcInitSpdAddr1; upd->PcdMrcInitSpdAddr2 = config->PcdMrcInitSpdAddr2;
-
Angel Pons (Code Review) -
Frans Hendriks (Code Review)
-
Furquan Shaikh (Code Review)
-
Kyösti Mälkki (Code Review)
-
Michael Niewöhner (Code Review)