Subrata Banik 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 15:
(3 comments)
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:
remove extra line.
you mean error returns 0 and success returns -1? that strange should that be opposite ?
see what you have mentioned here https://review.coreboot.org/c/coreboot/+/35225/15/src/soc/intel/common/block...
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
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, it looks very nit.