Krishna P Bhat D has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39128 )
Change subject: soc/intel/icelake: Add function to dump ME firmware status information ......................................................................
Patch Set 8:
(5 comments)
https://review.coreboot.org/c/coreboot/+/39128/7/src/soc/intel/icelake/Makef... File src/soc/intel/icelake/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/39128/7/src/soc/intel/icelake/Makef... PS7, Line 46: ramstage-y += me.c
do this in the patch where you add the file.
Done
https://review.coreboot.org/c/coreboot/+/39128/7/src/soc/intel/icelake/inclu... File src/soc/intel/icelake/include/soc/me.h:
https://review.coreboot.org/c/coreboot/+/39128/7/src/soc/intel/icelake/inclu... PS7, Line 55: ;
unnecessary change
Ack
https://review.coreboot.org/c/coreboot/+/39128/7/src/soc/intel/icelake/me.c File src/soc/intel/icelake/me.c:
https://review.coreboot.org/c/coreboot/+/39128/7/src/soc/intel/icelake/me.c@... PS7, Line 125: BIOS_DEBUG
I’d suggest `BIOS_INFO`.
In line with other soc to maintain uniformity.
https://review.coreboot.org/c/coreboot/+/39128/7/src/soc/intel/icelake/me.c@... PS7, Line 126: hfsts1.data);
This line should fit in 96 characters.
Done
https://review.coreboot.org/c/coreboot/+/39128/7/src/soc/intel/icelake/me.c@... PS7, Line 138: printk(BIOS_DEBUG, "ME: Manufacturing Mode : %s\n", : hfsts1.fields.mfg_mode ? "YES" : "NO");
like in TGL, do similar conditions for ICL?
It is only for TGL, not applicable for ICL.