Change in coreboot[master]: soc/intel/icelake: Add function to dump ME firmware status information
Wonkyu Kim 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 9:
(7 comments)
https://review.coreboot.org/c/coreboot/+/39128/3/src/soc/intel/icelake/inclu... File src/soc/intel/icelake/include/soc/me.h:
https://review.coreboot.org/c/coreboot/+/39128/3/src/soc/intel/icelake/inclu... PS3, Line 60: unused
; is missing.But, the prototype is not required here.
Done
https://review.coreboot.org/c/coreboot/+/39128/1/src/soc/intel/icelake/me.c File src/soc/intel/icelake/me.c:
https://review.coreboot.org/c/coreboot/+/39128/1/src/soc/intel/icelake/me.c@... PS1, Line 26: /* Miscellaneous constants */ : enum { : MKHI_GEN_GROUP_ID = 0xFF, : MKHI_GET_FW_VERSION = 0x02, : ME_OPMODE_NORMAL = 0x00, : ME_WSTATE_NORMAL = 0x05, : };
Not required. You should pick the values from common.
Done
https://review.coreboot.org/c/coreboot/+/39128/1/src/soc/intel/icelake/me.c@... PS1, Line 36: raw
Rename the field to data.
Done
https://review.coreboot.org/c/coreboot/+/39128/3/src/soc/intel/icelake/me.c File src/soc/intel/icelake/me.c:
https://review.coreboot.org/c/coreboot/+/39128/3/src/soc/intel/icelake/me.c@... PS3, Line 16: #include <device/pci_ops.h> : #include <bootstate.h> : #include <commonlib/helpers.h> : #include <console/console.h> : #include <device/pci.h> : #include <intelblocks/cse.h> : #include <soc/me.h> : #include <soc/pci_devs.h> : #include <stdint.h>
Only below header files are required. […]
Done
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
In line with other soc to maintain uniformity.
Done
https://review.coreboot.org/c/coreboot/+/39128/7/src/soc/intel/icelake/me.c@... PS7, Line 126: hfsts1.data);
Done
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");
It is only for TGL, not applicable for ICL.
Done
-
Wonkyu Kim (Code Review)