Attention is currently required from: Tim Crawford, Sean Rhodes, SRIDHAR SIRICILLA, Furquan Shaikh, Paul Menzel, Rizwan Qureshi, Tim Wawrzynczak, Subrata Banik, Sridhar Siricilla, Arthur Heymans, Evgeny Zinoviev, Patrick Rudolph. Angel Pons 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 74:
(8 comments)
File src/soc/intel/common/block/cse/cse.c:
https://review.coreboot.org/c/coreboot/+/52800/comment/6850a4c3_7fa927f6 PS74, Line 998: set_uint_option("me_state_counter", me_state_counter); Writing the option could fail, but I'm not sure how to handle this.
https://review.coreboot.org/c/coreboot/+/52800/comment/9f7f2840_c642b74c PS74, Line 999: %d %u
https://review.coreboot.org/c/coreboot/+/52800/comment/4c646d50_e4ea9e52 PS74, Line 1007: ; Not needed
https://review.coreboot.org/c/coreboot/+/52800/comment/3c3db3b1_bf4e0348 PS74, Line 1020: %d %u
https://review.coreboot.org/c/coreboot/+/52800/comment/0819362d_5fc65f0d PS74, Line 1038: cse_is_hfs1_com_soft_temp_disable() I'd store the result in a constant to avoid calling this function twice
https://review.coreboot.org/c/coreboot/+/52800/comment/c78894d3_a5e6804c PS74, Line 1053: (me_state_counter != UINT_MAX) && (me_state_counter != 0) inner braces not needed:
if (me_state_counter != UINT_MAX && me_state_counter != 0)
https://review.coreboot.org/c/coreboot/+/52800/comment/4702106b_ce444dcf PS74, Line 1056: ; Not needed
https://review.coreboot.org/c/coreboot/+/52800/comment/80d6a3f6_5cda072a PS74, Line 1059: printk(BIOS_DEBUG, "HECI: Sending command to change state\n"); This message is misleading, the command was sent earlier. I'd just remove it.
-
Angel Pons (Code Review)