Attention is currently required from: David Wu, Krishna P Bhat D, Reka Norman, Reka Norman, Stefan Reinauer, Subrata Banik.
Tyler Wang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81928?usp=email )
Change subject: util/ifdtool: Add support for checking GPR0 status ......................................................................
Patch Set 12:
(6 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/81928/comment/95acd6be_e2faac02 : PS11, Line 34: Start address = 0x00004000 : End address = 0x00322fff
I think this should print the actual range, not the enabled range, so these should be 0 (I also left […]
Thank you for your advise, I've update test result in patchset12. Please help to review, thanks!
File util/ifdtool/ifdtool.c:
https://review.coreboot.org/c/coreboot/+/81928/comment/dcb63ad5_b6dca9cb : PS11, Line 1749: struct fpsba *fpsba, uint32_t gpr0_offset,
These arguments are not necessary
Done, thanks!
https://review.coreboot.org/c/coreboot/+/81928/comment/ed20ae1f_ec6ec3ed : PS11, Line 1753: enabled_gprd_reg.value = fpsba->pchstrp[gpr0_offset];
This is not necessary (and doesn't do anything since you overwrite all the fields below).
Done, thanks!
https://review.coreboot.org/c/coreboot/+/81928/comment/dfd8d566_b89afe26 : PS11, Line 1786: union gprd enabled_gprd; : enabled_gprd = get_enabled_gprd(fpsba, gpr0_offset, image, size); :
nit: combine onto one line
Done, thanks!
https://review.coreboot.org/c/coreboot/+/81928/comment/075792c7_7643ea5e : PS11, Line 1807: union gprd enabled_gprd; : enabled_gprd = get_enabled_gprd(fpsba, gpr0_offset, image, size);
nit: combine onto one line
Done, thanks!
https://review.coreboot.org/c/coreboot/+/81928/comment/60f1d036_148363b8 : PS11, Line 1818: enabled_gprd
This should be `fpsba->pchstrp[gpr0_offset]` (print the actual range, not the enabled range).
Done, thanks!