Attention is currently required from: Sean Rhodes, Andy Pont, Tim Wawrzynczak, Paul Menzel, Patrick Rudolph. Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52800 )
Change subject: soc/intel: Allow enable/disable ME via CMOS ......................................................................
Patch Set 20:
(2 comments)
File src/soc/intel/common/block/cse/cse.c:
https://review.coreboot.org/c/coreboot/+/52800/comment/c1f00ddd_0d43d2ae PS20, Line 67: /* : * Initialize the device with provided temporary BAR. If BAR is 0 use a : * default. This is intended for pre-mem usage only where BARs haven't been : * assigned yet and devices are not enabled. : */ This comment is not in the right place anymore.
https://review.coreboot.org/c/coreboot/+/52800/comment/88df9997_6cc6c5a8 PS20, Line 847: print_me_fw_version
Would renaming the method be an acceptable solution? Otherwise, we're going to end up with 2 almost identical functions.
Why not enable/disable it in the cse_ops 'enable' function instead of highjacking a different function to do this? Also what happens to the PCI device after you send those messages? Does it dissapear? Are the BARs still working? If the PCI device cannot be enumerated after this message, you might want to add a different hook that is run earlier: before BS_DEV_ENUMERATE is run.
-
Arthur Heymans (Code Review)