Attention is currently required from: Martin Roth.
Simon Glass has posted comments on this change by Simon Glass. ( https://review.coreboot.org/c/coreboot/+/83293?usp=email )
Change subject: Support CCB at a fixed SPI-flash offset ......................................................................
Patch Set 3:
(7 comments)
File Documentation/technotes/ccb.md:
https://review.coreboot.org/c/coreboot/+/83293/comment/3e79b469_c18251fd?usp... : PS2, Line 88: Care should be taken that : this region is in read-only flash, if preventing users from changing it is : important. :
This is true for chromeos, but not every platform.
Indeed. What change do you suggest to the language here?
File Makefile.mk:
https://review.coreboot.org/c/coreboot/+/83293/comment/b95540f7_5f26ccac?usp... : PS2, Line 1124: ifeq ($(CONFIG_CCB_FMAP),y)
Maybe print a warning or just error out if the CCB FMAP alignment is different than what's in the fm […]
Do you mean the .fmd file? I cannot find the alignment you are referring to. The code here seems to be what sets the alignment, so far as I can tell.
https://review.coreboot.org/c/coreboot/+/83293/comment/549bcfbe_7f70c092?usp... : PS2, Line 1259: ifneq ($(CONFIG_ARCH_X86),)
Make this the `else` of the above? Reverse the logic?
Done
https://review.coreboot.org/c/coreboot/+/83293/comment/67b0168e_534bda4b?usp... : PS2, Line 1274: /usr/bin/printf
Maybe use `env printf` instead of the path?
Done
File src/lib/bootblock.c:
https://review.coreboot.org/c/coreboot/+/83293/comment/817e0762_3b05269b?usp... : PS2, Line 63: or FMAP
Why does it need to be initialized late if it's at a known location in the SPI rom? Should this be […]
I am using the FMAP to indicate the region, so have to read the FMAP. But...
File src/lib/ccb.c:
https://review.coreboot.org/c/coreboot/+/83293/comment/d4c3abf4_0b6b90f8?usp... : PS2, Line 102: if (fmap_locate_area_as_rdev(CCB_REGION, rdev)) {
It seems to me that the advantage of using FMAP vs CBFS is that we can parse the fmap at build time […]
...yes, we could plumb the FMAP address through to the build and avoid that.
I can take a look at this, if you think it would make a big difference?
I was hoping to show the different trade-offs for the three methods.
File util/cbfstool/cbfstool.c:
https://review.coreboot.org/c/coreboot/+/83293/comment/ccdb3cbb_6035e4d8?usp... : PS2, Line 731: /* Now try FMAP */
Based on the Kconfig, we can't have CCB in more than one location. […]
coreboot itself is set to use one of the three options, but there is only one build of cbfstool, so it must support all three at once. It can tell which is being used by looking at the image, as it does here.