Krishna P Bhat D has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40570 )
Change subject: soc/intel/jasperlake: Add function to display ME firmware status info ......................................................................
Patch Set 3:
(5 comments)
https://review.coreboot.org/c/coreboot/+/40570/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/40570/2//COMMIT_MSG@7 PS2, Line 7: information
Just *info* to make it shorter.
Done
https://review.coreboot.org/c/coreboot/+/40570/2//COMMIT_MSG@11 PS2, Line 11: firmware version information.
Please use the full textwidth of 75 characters.
Done
https://review.coreboot.org/c/coreboot/+/40570/2//COMMIT_MSG@12 PS2, Line 12:
Did you copy the code from somewhere? Why can’t it be common code?
HFSTS register is not same the across SoCs. Because of this reason we cannot avoid SoC specific macros if move the function to common code. Hence, making it SoC specific.
https://review.coreboot.org/c/coreboot/+/40570/2//COMMIT_MSG@15 PS2, Line 15: TEST=Build and boot jslrvp.
Please paste the new output.
Done
https://review.coreboot.org/c/coreboot/+/40570/2/src/soc/intel/jasperlake/in... File src/soc/intel/jasperlake/include/soc/me.h:
https://review.coreboot.org/c/coreboot/+/40570/2/src/soc/intel/jasperlake/in... PS2, Line 12: u32 mfg_mode: 1;
That seems unrelated.
Initially assumption was TGL and JSL can co-exist, when split, me.h was copied from TGL. In TGL, spi_protection_mode is there, in JSL, we have manufacturing mode.