Hello Philip Chen,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/41980
to review the following change.
Change subject: google/trogdor: Remove RO_FSG region ......................................................................
google/trogdor: Remove RO_FSG region
We decided to store the FSG on eMMC instead of SPI flash, so we don't need this region anymore. Getting rid of it allows us to put more space into CBFS (to store hi-res bitmaps). Also grow VPD by some remaining amount to keep the FMAP alignment reasonable.
Signed-off-by: Julius Werner jwerner@chromium.org Change-Id: If73450b65718affae71b6ada70ded5c5f45cfb4c --- M src/mainboard/google/trogdor/chromeos.fmd 1 file changed, 3 insertions(+), 4 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/80/41980/1
diff --git a/src/mainboard/google/trogdor/chromeos.fmd b/src/mainboard/google/trogdor/chromeos.fmd index 2f93712..3aa0473 100644 --- a/src/mainboard/google/trogdor/chromeos.fmd +++ b/src/mainboard/google/trogdor/chromeos.fmd @@ -2,17 +2,16 @@
FLASH@0x0 8M { WP_RO 4M { - RO_SECTION 0x204000 { + RO_SECTION 0x3c4000 { BOOTBLOCK 96K COREBOOT(CBFS) - FMAP@0x200000 0x1000 + FMAP@0x3c0000 0x1000 GBB 0x2f00 RO_FRID 0x100 } - RO_VPD(PRESERVE) 16K + RO_VPD(PRESERVE) 228K RO_DDR_TRAINING(PRESERVE) 8K RO_LIMITS_CFG(PRESERVE) 4K - RO_FSG(PRESERVE) }
RW_VPD(PRESERVE) 32K
Philip Chen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41980 )
Change subject: google/trogdor: Remove RO_FSG region ......................................................................
Patch Set 1: Code-Review+2
Philip Chen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41980 )
Change subject: google/trogdor: Remove RO_FSG region ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/41980/1/src/mainboard/google/trogdo... File src/mainboard/google/trogdor/chromeos.fmd:
https://review.coreboot.org/c/coreboot/+/41980/1/src/mainboard/google/trogdo... PS1, Line 12: RO_VPD(PRESERVE) 228K This seems to be more than enough. But I don't know if there is any better use of the space.
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41980 )
Change subject: google/trogdor: Remove RO_FSG region ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/41980/1/src/mainboard/google/trogdo... File src/mainboard/google/trogdor/chromeos.fmd:
https://review.coreboot.org/c/coreboot/+/41980/1/src/mainboard/google/trogdo... PS1, Line 12: RO_VPD(PRESERVE) 228K
This seems to be more than enough. But I don't know if there is any better use of the space.
Yeah I can either put the space here or in CBFS. I don't think it's really needed on either side. Putting it here makes the FMAP alignment a bit higher, which is good.
Julius Werner has submitted this change. ( https://review.coreboot.org/c/coreboot/+/41980 )
Change subject: google/trogdor: Remove RO_FSG region ......................................................................
google/trogdor: Remove RO_FSG region
We decided to store the FSG on eMMC instead of SPI flash, so we don't need this region anymore. Getting rid of it allows us to put more space into CBFS (to store hi-res bitmaps). Also grow VPD by some remaining amount to keep the FMAP alignment reasonable.
Signed-off-by: Julius Werner jwerner@chromium.org Change-Id: If73450b65718affae71b6ada70ded5c5f45cfb4c Reviewed-on: https://review.coreboot.org/c/coreboot/+/41980 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Philip Chen philipchen@google.com --- M src/mainboard/google/trogdor/chromeos.fmd 1 file changed, 3 insertions(+), 4 deletions(-)
Approvals: build bot (Jenkins): Verified Philip Chen: Looks good to me, approved
diff --git a/src/mainboard/google/trogdor/chromeos.fmd b/src/mainboard/google/trogdor/chromeos.fmd index 2f93712..3aa0473 100644 --- a/src/mainboard/google/trogdor/chromeos.fmd +++ b/src/mainboard/google/trogdor/chromeos.fmd @@ -2,17 +2,16 @@
FLASH@0x0 8M { WP_RO 4M { - RO_SECTION 0x204000 { + RO_SECTION 0x3c4000 { BOOTBLOCK 96K COREBOOT(CBFS) - FMAP@0x200000 0x1000 + FMAP@0x3c0000 0x1000 GBB 0x2f00 RO_FRID 0x100 } - RO_VPD(PRESERVE) 16K + RO_VPD(PRESERVE) 228K RO_DDR_TRAINING(PRESERVE) 8K RO_LIMITS_CFG(PRESERVE) 4K - RO_FSG(PRESERVE) }
RW_VPD(PRESERVE) 32K
9elements QA has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41980 )
Change subject: google/trogdor: Remove RO_FSG region ......................................................................
Patch Set 2:
Automatic boot test returned (PASS/FAIL/TOTAL): 4/0/4 Emulation targets: "QEMU x86 q35/ich9" using payload TianoCore : SUCCESS : https://lava.9esec.io/r/4931 "QEMU x86 q35/ich9" using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/4930 "QEMU x86 i440fx/piix4" using payload SeaBIOS : SUCCESS : https://lava.9esec.io/r/4929 "QEMU AArch64" using payload LinuxBoot_u-root_kexec : SUCCESS : https://lava.9esec.io/r/4928
Please note: This test is under development and might not be accurate at all!
-
9elements QA (Code Review) -
Julius Werner (Code Review)
-
Philip Chen (Code Review)