Change in coreboot[master]: util/ifittool: Add an option to set the FIT pointer a CBFS file
Attention is currently required from: Patrick Rudolph, Arthur Heymans. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/50925 )
Change subject: util/ifittool: Add an option to set the FIT pointer a CBFS file ......................................................................
Patch Set 5:
(10 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/50925/comment/b2614bea_02532360 PS5, Line 10: separatly typo: separat*e*ly
https://review.coreboot.org/c/coreboot/+/50925/comment/f518d71c_48dcb37f PS5, Line 13: an typo: an*d*
https://review.coreboot.org/c/coreboot/+/50925/comment/224c8d92_6ad43ace PS5, Line 14: trying `trying *to add*`, maybe?
File util/cbfstool/fit.c:
https://review.coreboot.org/c/coreboot/+/50925/comment/0ab65af4_1b88308d PS5, Line 531: offset
why is it called offset? it looks like this is an absolute address.
I'd rename this to `new_address` or similar
https://review.coreboot.org/c/coreboot/+/50925/comment/ba0858bd_ef650470 PS5, Line 538: ptr_to_offset(offset_fn, bootblock, FIT_POINTER_LOCATION - topswap_size)); Uh, why not use `get_fit_ptr` here?
File util/cbfstool/ifittool.c:
https://review.coreboot.org/c/coreboot/+/50925/comment/5e13a7c0_34b3275b PS5, Line 26: set-fit I'd use `set-fit-pointer` instead.
https://review.coreboot.org/c/coreboot/+/50925/comment/e7c408be_5443f5d1 PS5, Line 47: set-fit `set-fit-pointer`
https://review.coreboot.org/c/coreboot/+/50925/comment/a4c1d729_a63b5471 PS5, Line 133: SET_FIT_OP I'd use `SET_FIT_PTR_OP` instead.
https://review.coreboot.org/c/coreboot/+/50925/comment/df173533_fd8782c6 PS5, Line 377: offset This is not an offset, but an absolute address. Maybe rename it to `new_address` or similar?
https://review.coreboot.org/c/coreboot/+/50925/comment/27184a80_929db01a PS5, Line 398: break; nit: that `break;` should be within the scope, for consistency with the other cases.
-
Angel Pons (Code Review)