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 15:
(6 comments)
https://review.coreboot.org/c/coreboot/+/35225/11/src/soc/intel/apollolake/c... File src/soc/intel/apollolake/cse.c:
https://review.coreboot.org/c/coreboot/+/35225/11/src/soc/intel/apollolake/c... PS11, Line 46: #define dump_status(index, hfs_reg) me_read_config32(hfs_reg)
Done
Done
https://review.coreboot.org/c/coreboot/+/35225/2/src/soc/intel/common/block/... File src/soc/intel/common/block/cse/cse.c:
https://review.coreboot.org/c/coreboot/+/35225/2/src/soc/intel/common/block/... PS2, Line 518: index = 1
for #1 refer to […]
Done
https://review.coreboot.org/c/coreboot/+/35225/13/src/soc/intel/common/block... File src/soc/intel/common/block/cse/cse.c:
https://review.coreboot.org/c/coreboot/+/35225/13/src/soc/intel/common/block... PS13, Line 524: : if (index != 0) : printk(BIOS_DEBUG, "CSE FWSTS%d: 0x%08x\n", index, reg); : else : printk(BIOS_DEBUG, "CSE [0x%x]=0x%08x\n", offset, reg);
why we need to print this everytime, leave it to caller ?
Done
https://review.coreboot.org/c/coreboot/+/35225/13/src/soc/intel/common/block... PS13, Line 535: PCH_DEV_CSE
add DEV NULL check ?
Done
https://review.coreboot.org/c/coreboot/+/35225/13/src/soc/intel/common/block... PS13, Line 537: if (!CONFIG(CONSOLE_SERIAL))
why we this guard?
Done
https://review.coreboot.org/c/coreboot/+/35225/13/src/soc/intel/common/block... File src/soc/intel/common/block/include/intelblocks/cse.h:
https://review.coreboot.org/c/coreboot/+/35225/13/src/soc/intel/common/block... PS13, Line 57: *
no need for multi line comments
Done