Sridhar Siricilla has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35546 )
Change subject: soc/intel/{common,skl,cnl,icl,apl,tgl}: Make me_hfsts1 structure SoC specific ......................................................................
Patch Set 24:
(11 comments)
https://review.coreboot.org/c/coreboot/+/35546/14/src/soc/intel/common/block... File src/soc/intel/common/block/cse/cse.c:
https://review.coreboot.org/c/coreboot/+/35546/14/src/soc/intel/common/block... PS14, Line 609: memset(&reply, 0, reply_size);
Not required. Removed the code.
Done
https://review.coreboot.org/c/coreboot/+/35546/14/src/soc/intel/common/block... PS14, Line 650: padding
Done in the patch @ https://review.coreboot. […]
Done
https://review.coreboot.org/c/coreboot/+/35546/14/src/soc/intel/common/block... PS14, Line 662: or temporary disable mode
Please refer patch: https://review.coreboot.org/c/coreboot/+/37283/13 for latest. […]
Done
https://review.coreboot.org/c/coreboot/+/35546/14/src/soc/intel/common/block... PS14, Line 664: hfs1.fields.working_state
Helper function wouldn't be unnecessay function call & additional operations to return the bitfield.
Helper function wouldn't be unnecessay function calls & additional operations to return the bitfield?
https://review.coreboot.org/c/coreboot/+/35546/14/src/soc/intel/common/block... PS14, Line 679: return 1;
keeping global reset out of this command handler will help supporting all flows of CSE FW Update.
Done
https://review.coreboot.org/c/coreboot/+/35546/14/src/soc/intel/common/block... PS14, Line 687: DISABLES
DISABLED
Implemented in patch - https://review.coreboot.org/c/coreboot/+/37283/17
https://review.coreboot.org/c/coreboot/+/35546/14/src/soc/intel/common/block... PS14, Line 698: padding
reserved
Implemented in https://review.coreboot.org/c/coreboot/+/37283/17
https://review.coreboot.org/c/coreboot/+/35546/14/src/soc/intel/common/block... PS14, Line 711:
This command can be sent only if CWS is normal. […]
Yes, I add 'TODO' statement, and will update HMRFPO_STATUS once Chrome SKU is available.
https://review.coreboot.org/c/coreboot/+/35546/15/src/soc/intel/common/block... File src/soc/intel/common/block/include/intelblocks/cse.h:
https://review.coreboot.org/c/coreboot/+/35546/15/src/soc/intel/common/block... PS15, Line 47: #if CONFIG(SOC_INTEL_APOLLOLAKE)
correct.
moved me_hfs1 to SoC specific folders.
https://review.coreboot.org/c/coreboot/+/35546/14/src/soc/intel/common/block... File src/soc/intel/common/block/include/intelblocks/cse.h:
https://review.coreboot.org/c/coreboot/+/35546/14/src/soc/intel/common/block... PS14, Line 152: *
Change is done in the patch - https://review.coreboot. […]
Done
https://review.coreboot.org/c/coreboot/+/35546/13/src/soc/intel/common/block... File src/soc/intel/common/block/include/intelblocks/cse.h:
https://review.coreboot.org/c/coreboot/+/35546/13/src/soc/intel/common/block... PS13, Line 49: u32 bist_test_result: 1;
The bit fields have been documented in the respective SOC's ME BIOS Writer Guide.
Done