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 12:
(3 comments)
Patch Set 11:
(1 comment)
It's very straight forward implementation.
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)
you can remove dump_status from everywhere and just use me_read_config32.
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
its not done.
There is no benefit/optimization going with static structure. Hence, prefer to keep "else if" ladder implementation.
https://review.coreboot.org/c/coreboot/+/35225/11/src/soc/intel/common/block... File src/soc/intel/common/block/cse/cse.c:
https://review.coreboot.org/c/coreboot/+/35225/11/src/soc/intel/common/block... PS11, Line 510: if (offset == PCI_ME_HFSTS1) : index = 1; : else if (offset == PCI_ME_HFSTS2) : index = 2; : else if (offset == PCI_ME_HFSTS3) : index = 3; : else if (offset == PCI_ME_HFSTS4) : index = 4; : else if (offset == PCI_ME_HFSTS5) : index = 5; : else if (offset == PCI_ME_HFSTS6) : index = 6;
would using a static structure beautify it? like below.. […]
Done