Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35902 )
Change subject: soc/intel/apl,skl,cnl,icl: implement a PRMRR size selection algorithm ......................................................................
Uploaded patch set 5.
(4 comments)
https://review.coreboot.org/c/coreboot/+/35902/4//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/35902/4//COMMIT_MSG@33 PS4, Line 33:
Tested how?
did some short tests yet, but needs definitely excessive testing; will be added as soon as this happended
https://review.coreboot.org/c/coreboot/+/35902/4/src/soc/intel/common/block/... File src/soc/intel/common/block/cpu/cpulib.c:
https://review.coreboot.org/c/coreboot/+/35902/4/src/soc/intel/common/block/... PS4, Line 328: valid
valid sounds like a boolean to me. […]
one should never derive the datatype from a variable name IMHO, but valid_size is ok for me, will change it
https://review.coreboot.org/c/coreboot/+/35902/4/src/soc/intel/common/block/... PS4, Line 334: <<
Please add spaces around the operator.
not convinced. I would aggree for any other operator, but not for shift. What do the others say?
https://review.coreboot.org/c/coreboot/+/35902/4/src/soc/intel/common/block/... PS4, Line 343: die("Unsupported PRMRR size limit!");
Add more information? Look in the devicetree or something similar? Also print the value of `MSR_PRMR […]
Done