Sridhar Siricilla has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35225 )
Change subject: soc/intel/common/block/cse: Move me_read_config32() to common code ......................................................................
Patch Set 17:
(5 comments)
https://review.coreboot.org/c/coreboot/+/35225/15/src/soc/intel/apollolake/c... File src/soc/intel/apollolake/cse.c:
https://review.coreboot.org/c/coreboot/+/35225/15/src/soc/intel/apollolake/c... PS15, Line 189:
remove extra line.
Done
https://review.coreboot.org/c/coreboot/+/35225/15/src/soc/intel/cannonlake/m... File src/soc/intel/cannonlake/me.c:
https://review.coreboot.org/c/coreboot/+/35225/15/src/soc/intel/cannonlake/m... PS15, Line 191:
remove extra line.
Done
https://review.coreboot.org/c/coreboot/+/35225/15/src/soc/intel/common/block... File src/soc/intel/common/block/cse/cse.c:
https://review.coreboot.org/c/coreboot/+/35225/15/src/soc/intel/common/block... PS15, Line 523:
you mean error returns 0 and success returns -1? that strange should that be opposite ? […]
Done
https://review.coreboot.org/c/coreboot/+/35225/15/src/soc/intel/common/block... File src/soc/intel/common/block/include/intelblocks/cse.h:
https://review.coreboot.org/c/coreboot/+/35225/15/src/soc/intel/common/block... PS15, Line 57: * Returns 0 on failure 1 on success.
you are doing something else in the function
Done
https://review.coreboot.org/c/coreboot/+/35225/15/src/soc/intel/skylake/me.c File src/soc/intel/skylake/me.c:
https://review.coreboot.org/c/coreboot/+/35225/15/src/soc/intel/skylake/me.c... PS15, Line 282: intel_me_status
are you also plan to move this into common code ? if yes then why don't you do it here in this CL so […]
No plans to move this code to common code as the HFSTS1 register bit defination is different for each socket