Felix Held has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/42706 )
Change subject: mb/amd/mandolin: maximize CBFS size ......................................................................
mb/amd/mandolin: maximize CBFS size
Change-Id: Ib829da0972bb7ec98f66fe8fe683289d91ad58dc Signed-off-by: Felix Held felix-coreboot@felixheld.de --- M src/mainboard/amd/mandolin/Kconfig 1 file changed, 2 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/06/42706/1
diff --git a/src/mainboard/amd/mandolin/Kconfig b/src/mainboard/amd/mandolin/Kconfig index 6ec77bf..eefa0df 100644 --- a/src/mainboard/amd/mandolin/Kconfig +++ b/src/mainboard/amd/mandolin/Kconfig @@ -27,9 +27,9 @@ Note that Kconfig does not currently enforce this restriction.
config CBFS_SIZE - default 0x780000 + default 0x7cf000 help - TODO: Adjust this to maximize CBFS size + Maximum CBFS size for the Mandolin FMAP.
config MAINBOARD_DIR string
Jason Glenesk has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42706 )
Change subject: mb/amd/mandolin: maximize CBFS size ......................................................................
Patch Set 1: Code-Review+1
Raul Rangel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42706 )
Change subject: mb/amd/mandolin: maximize CBFS size ......................................................................
Patch Set 1: Code-Review+2
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42706 )
Change subject: mb/amd/mandolin: maximize CBFS size ......................................................................
Patch Set 1:
(2 comments)
https://review.coreboot.org/c/coreboot/+/42706/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/42706/1//COMMIT_MSG@7 PS1, Line 7: mb/amd/mandolin: maximize CBFS size Maximize CBFS size (316 kB increase)
https://review.coreboot.org/c/coreboot/+/42706/1/src/mainboard/amd/mandolin/... File src/mainboard/amd/mandolin/Kconfig:
https://review.coreboot.org/c/coreboot/+/42706/1/src/mainboard/amd/mandolin/... PS1, Line 32: Mandolin For being able to compare different board configurations, the board name shouldn’t be in the description.
Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42706 )
Change subject: mb/amd/mandolin: maximize CBFS size ......................................................................
Patch Set 1:
(2 comments)
https://review.coreboot.org/c/coreboot/+/42706/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/42706/1//COMMIT_MSG@7 PS1, Line 7: mb/amd/mandolin: maximize CBFS size
Maximize CBFS size (316 kB increase)
don't think that it matters much how much the cbfs grew. the old value was a "close enough" value and this is the calculated one after moving some things around
https://review.coreboot.org/c/coreboot/+/42706/1/src/mainboard/amd/mandolin/... File src/mainboard/amd/mandolin/Kconfig:
https://review.coreboot.org/c/coreboot/+/42706/1/src/mainboard/amd/mandolin/... PS1, Line 32: Mandolin
For being able to compare different board configurations, the board name shouldn’t be in the descrip […]
This maximum cbfs size is very specific to the mandolin fmap, so I don't see much reason to remove mandolin from the description. I could just remove the text; added that to make clear where that value comes from
Hello Jason Glenesk, build bot (Jenkins), Raul Rangel,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/42706
to look at the new patch set (#2).
Change subject: mb/amd/mandolin: maximize CBFS size ......................................................................
mb/amd/mandolin: maximize CBFS size
Change-Id: Ib829da0972bb7ec98f66fe8fe683289d91ad58dc Signed-off-by: Felix Held felix-coreboot@felixheld.de --- M src/mainboard/amd/mandolin/Kconfig 1 file changed, 2 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/06/42706/2
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42706 )
Change subject: mb/amd/mandolin: maximize CBFS size ......................................................................
Patch Set 2:
(2 comments)
https://review.coreboot.org/c/coreboot/+/42706/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/42706/1//COMMIT_MSG@7 PS1, Line 7: mb/amd/mandolin: maximize CBFS size
don't think that it matters much how much the cbfs grew. […]
How should reviewers verify, that your changed values are correct?
https://review.coreboot.org/c/coreboot/+/42706/1/src/mainboard/amd/mandolin/... File src/mainboard/amd/mandolin/Kconfig:
https://review.coreboot.org/c/coreboot/+/42706/1/src/mainboard/amd/mandolin/... PS1, Line 32: Mandolin
This maximum cbfs size is very specific to the mandolin fmap, so I don't see much reason to remove m […]
It’s `src/mainboard/amd/madolin/Kconfig`, so it’s clear it’s for Mandolin?
Anyway, the other boards do not seem to have a description at all, so I’d remove it.
Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42706 )
Change subject: mb/amd/mandolin: maximize CBFS size ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/42706/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/42706/1//COMMIT_MSG@7 PS1, Line 7: mb/amd/mandolin: maximize CBFS size
How should reviewers verify, that your changed values are correct?
by looking at mandolin's fmap which is why I added that to the help text of the CBFS_SIZE option to not have a magic value there
Hello build bot (Jenkins), Jason Glenesk, Raul Rangel,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/42706
to look at the new patch set (#3).
Change subject: mb/amd/mandolin: maximize CBFS size ......................................................................
mb/amd/mandolin: maximize CBFS size
Change-Id: Ib829da0972bb7ec98f66fe8fe683289d91ad58dc Signed-off-by: Felix Held felix-coreboot@felixheld.de --- M src/mainboard/amd/mandolin/Kconfig 1 file changed, 1 insertion(+), 3 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/06/42706/3
Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42706 )
Change subject: mb/amd/mandolin: maximize CBFS size ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/42706/1/src/mainboard/amd/mandolin/... File src/mainboard/amd/mandolin/Kconfig:
https://review.coreboot.org/c/coreboot/+/42706/1/src/mainboard/amd/mandolin/... PS1, Line 32: Mandolin
It’s `src/mainboard/amd/madolin/Kconfig`, so it’s clear it’s for Mandolin? […]
turned the help text into a comment. The mandolin part is necessary, since there will be another variant with different FMAP soon
Raul Rangel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42706 )
Change subject: mb/amd/mandolin: maximize CBFS size ......................................................................
Patch Set 3: Code-Review+2
Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/42706 )
Change subject: mb/amd/mandolin: maximize CBFS size ......................................................................
mb/amd/mandolin: maximize CBFS size
Change-Id: Ib829da0972bb7ec98f66fe8fe683289d91ad58dc Signed-off-by: Felix Held felix-coreboot@felixheld.de Reviewed-on: https://review.coreboot.org/c/coreboot/+/42706 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Raul Rangel rrangel@chromium.org --- M src/mainboard/amd/mandolin/Kconfig 1 file changed, 1 insertion(+), 3 deletions(-)
Approvals: build bot (Jenkins): Verified Raul Rangel: Looks good to me, approved
diff --git a/src/mainboard/amd/mandolin/Kconfig b/src/mainboard/amd/mandolin/Kconfig index 6ec77bf..a087b59 100644 --- a/src/mainboard/amd/mandolin/Kconfig +++ b/src/mainboard/amd/mandolin/Kconfig @@ -27,9 +27,7 @@ Note that Kconfig does not currently enforce this restriction.
config CBFS_SIZE - default 0x780000 - help - TODO: Adjust this to maximize CBFS size + default 0x7cf000 # Maximum size for the Mandolin FMAP
config MAINBOARD_DIR string
-
Felix Held (Code Review) -
Jason Glenesk (Code Review)
-
Paul Menzel (Code Review)
-
Raul Rangel (Code Review)