Attention is currently required from: Bora Guvendik, Jamie Ryu, Zhixing Ma.
Jérémy Compostella has posted comments on this change by Bora Guvendik. ( https://review.coreboot.org/c/coreboot/+/86991?usp=email )
Change subject: mb/intel/ptlrvp: Introduce PTL RVP External and Internal EC Configurations ......................................................................
Patch Set 11:
(6 comments)
File src/mainboard/intel/ptlrvp/Kconfig:
https://review.coreboot.org/c/coreboot/+/86991/comment/ea3bcddb_1b549f03?usp... : PS11, Line 61: P_EXT_EC Could it be `BOARD_INTEL_PTLRVP_GOOGLE_CHROMEEC` ?
File src/mainboard/intel/ptlrvp/Kconfig.name:
https://review.coreboot.org/c/coreboot/+/86991/comment/08c82bb7_8eda149d?usp... : PS11, Line 9: Ptlrvp External EC Google Chrome EC
File src/mainboard/intel/ptlrvp/chromeos.c:
https://review.coreboot.org/c/coreboot/+/86991/comment/211a10e2_f52d2ee8?usp... : PS11, Line 27: int get_lid_switch(void) In an effort to limit file differences with fatcat, I would suggest a dedicated C file like non_google.c to hold those, or something even more suitable. What do you think?
File src/mainboard/intel/ptlrvp/ec.c:
https://review.coreboot.org/c/coreboot/+/86991/comment/da8791a9_b4b3c7ed?usp... : PS11, Line 24: google_chromeec_events_init Could we have defined as a stub in my suggested non_intel.c source file?
File src/mainboard/intel/ptlrvp/smihandler.c:
https://review.coreboot.org/c/coreboot/+/86991/comment/e9fcb0b9_1fffc2df?usp... : PS11, Line 10: void mainboard_smi_sleep(u8 slp_typ) Could we have a non_chromeec_smihandler.c compiled instead ?
File src/mainboard/intel/ptlrvp/variants/baseboard/ptlrvp/ramstage.c:
https://review.coreboot.org/c/coreboot/+/86991/comment/01fcef90_83b130c5?usp... : PS11, Line 55: EC_GOOGLE_CHROMEEC Couldn't we just skip compiling this file is not EC_GOOGLE_CHROMEEC ?