Julius Werner has posted comments on this change. ( https://review.coreboot.org/28104 )
Change subject: lib/fit_payload: Add coreboot tables support for FDT. ......................................................................
Patch Set 3:
(4 comments)
https://review.coreboot.org/#/c/28104/2/src/lib/fit_payload.c File src/lib/fit_payload.c:
https://review.coreboot.org/#/c/28104/2/src/lib/fit_payload.c@112 PS2, Line 112: // Need to add 'ranges' to the intermediate node to make 'reg' work. Remember to convert everything to coreboot comment style.
https://review.coreboot.org/#/c/28104/2/src/lib/fit_payload.c@119 PS2, Line 119: (char *) Do you need this cast? Looks like we didn't need it in depthcharge so I'm confused...
Maybe we should change all the dt_add_..._prop() stuff to take const values?
https://review.coreboot.org/#/c/28104/2/src/lib/fit_payload.c@138 PS2, Line 138: reg_sizes[1] = cbmem_overhead_size(); These aren't the right numbers... at least not the numbers used by depthcharge. We don't actually have any uses for this field yet, so it's not that big of a deal, but we should try to keep it consistent.
The correct range (that depthcharge uses) is found by cbmem_add_bootmem(). Maybe we should expand that function to a slightly more generic cbmem_full_area(&start, &size) or something like that, so that we can use it both for bootmem and here.
https://review.coreboot.org/#/c/28104/2/src/lib/fit_payload.c@143 PS2, Line 143: exported from coreboot nit: not really "exported from coreboot" anymore now, since we're still in coreboot
-
Julius Werner (Code Review)