Change in coreboot[master]: common/block/imc: Add Integrated Memory Controller (IMC) driver
Andrey Petrov has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34678 )
Change subject: common/block/imc: Add Integrated Memory Controller (IMC) driver ......................................................................
Patch Set 4:
(5 comments)
https://review.coreboot.org/c/coreboot/+/34678/1/src/soc/intel/common/block/... File src/soc/intel/common/block/imc/Kconfig:
https://review.coreboot.org/c/coreboot/+/34678/1/src/soc/intel/common/block/... PS1, Line 6: that is found on some Xeon processors
Please use the full text-width and add a dot/period at the end of sentences.
Done
https://review.coreboot.org/c/coreboot/+/34678/1/src/soc/intel/common/block/... File src/soc/intel/common/block/imc/imc.c:
https://review.coreboot.org/c/coreboot/+/34678/1/src/soc/intel/common/block/... PS1, Line 4: * Copyright 2019 Facebook Inc.
Comma after Facebook?
Done
https://review.coreboot.org/c/coreboot/+/34678/1/src/soc/intel/common/block/... PS1, Line 70:
Add a spew message, to output the time?
Done
https://review.coreboot.org/c/coreboot/+/34678/1/src/soc/intel/common/block/... PS1, Line 153: ret = -1;
I do not believe this should be reported. […]
Done
https://review.coreboot.org/c/coreboot/+/34678/1/src/soc/intel/common/block/... File src/soc/intel/common/block/include/intelblocks/imc.h:
https://review.coreboot.org/c/coreboot/+/34678/1/src/soc/intel/common/block/... PS1, Line 38:
Use tabs?
Done
-
Andrey Petrov (Code Review)