Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31493 )
Change subject: cbfstool: Add ifittool ......................................................................
Patch Set 12:
(16 comments)
https://review.coreboot.org/#/c/31493/11/util/cbfstool/fit.c File util/cbfstool/fit.c:
https://review.coreboot.org/#/c/31493/11/util/cbfstool/fit.c@a132 PS11, Line 132:
We can still assume that it contains at least the header.
Done
https://review.coreboot.org/#/c/31493/11/util/cbfstool/fit.c@45 PS11, Line 45: and must be set to 0
Only until a platform starts to use them. It seems at least APL does.
Then they violate the spec.
https://review.coreboot.org/#/c/31493/11/util/cbfstool/fit.c@58 PS11, Line 58: * Bit 7 indicates whether component has valid checksum.
mention C_V? as it's referred to as that below
Done
https://review.coreboot.org/#/c/31493/11/util/cbfstool/fit.c@182 PS11, Line 182: but only mentioned on page 22 of
It is mentioned below "FIT Ordering Rules" in all specifications I know.
Ack
https://review.coreboot.org/#/c/31493/11/util/cbfstool/fit.c@188 PS11, Line 188: if (fit_table_entries(fit) <= 1) : return;
What difference does it make?
None. Removed.
https://review.coreboot.org/#/c/31493/11/util/cbfstool/fit.c@256 PS11, Line 256: cbfs_file_get_header(&buf, &header);
`buf.offset` (and `. […]
Done
https://review.coreboot.org/#/c/31493/11/util/cbfstool/fit.c@263 PS11, Line 263: ssize_t
why not `size_t`?
Done
https://review.coreboot.org/#/c/31493/11/util/cbfstool/fit.c@264 PS11, Line 264: total_entries
isn't this more like `max_entries`?
Done
https://review.coreboot.org/#/c/31493/11/util/cbfstool/fit.c@332 PS11, Line 332: zero or multiple
or one? […]
Done
https://review.coreboot.org/#/c/31493/11/util/cbfstool/fit.c@443 PS11, Line 443: entires
entries
Done
https://review.coreboot.org/#/c/31493/11/util/cbfstool/fit.c@443 PS11, Line 443: ERROR("Maximum of FIT entires reached.\n");
free(mcus);
Done
https://review.coreboot.org/#/c/31493/11/util/cbfstool/fit.c@450 PS11, Line 450: mcus[i].offset),
NB. Looking through the code this still only works if the CBFS is memory […]
Nobody cared until this point.
https://review.coreboot.org/#/c/31493/11/util/cbfstool/fit.c@459 PS11, Line 459:
sort?
no as fit_add_entry sorts already
https://review.coreboot.org/#/c/31493/11/util/cbfstool/fit.c@480 PS11, Line 480: fit_pointer[1] != 0
Any hint that it's a 64-bit pointer? otherwise it wouldn't be specified what […]
@Aaron As you added the code, where do you got this from?
https://review.coreboot.org/#/c/31493/11/util/cbfstool/fit.c@533 PS11, Line 533: break;
`break` is usually indented.
Done
https://review.coreboot.org/#/c/31493/11/util/cbfstool/fit.c@571 PS11, Line 571: lx
PRIx64
Done