Attention is currently required from: Jason Glenesk, Raul Rangel, Furquan Shaikh, Marshall Dawson, Karthik Ramasubramanian, Felix Held. Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56491 )
Change subject: lib/cbfs: Add cbfs_preload() ......................................................................
Patch Set 11: Code-Review+2
(3 comments)
Patchset:
PS11: LGTM after one nit
File src/lib/Kconfig:
https://review.coreboot.org/c/coreboot/+/56491/comment/b3dbae96_aff6a5ff PS7, Line 108: config CBFS_PRELOAD
The reason I want the Kconfig is that `cbfs_preload` should only try to preload the file when SOC_AM […]
Well, it shouldn't really make a practical difference if the controller uses PIO, it'll just not give you a speed-up. The order in which threads are processed is not meant to be predictable anyway. If spending all the time on SPI read first would cause actual issues, that's a problem with the design in general.
But I guess it still makes sense to save all the overhead in those cases, so okay, let's have a Kconfig.
File src/lib/cbfs.c:
https://review.coreboot.org/c/coreboot/+/56491/comment/722bebdc_fcabbcbc PS11, Line 511: cbfs_unmap(rdev_mmap_full(&rdev)); Rather than duplicating everything in the error path I think it would be better to just merge them, e.g.
void *loc = NULL;
...<if anything goes wrong just `goto out`>...
if (!size) loc = NULL;
out: if (preload_successful) cbfs_unmap(...);
return loc;
-
Julius Werner (Code Review)