Change in coreboot[master]: soc/intel/common/block/cse: Minor clean up some late comments
Sridhar Siricilla has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35546 )
Change subject: soc/intel/common/block/cse: Minor clean up some late comments ......................................................................
Patch Set 15:
(7 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 78: GR_ORIGIN_BIOS_POST
Is this a command id or group id? From the name and the comment, it is not really clear to me what t […]
It's not command id or group id. It's one of the value of request_origin field of GLOBAL_RESET command request.
https://review.coreboot.org/c/coreboot/+/35546/14/src/soc/intel/common/block... PS14, Line 84: operation and working modes
Actually, this should be CWS = Current Working State and COM = Current Operation Mode
Modified the comment in the patch: https://review.coreboot.org/c/coreboot/+/38247/1
https://review.coreboot.org/c/coreboot/+/35546/14/src/soc/intel/common/block... PS14, Line 85: HFS
I think it would be better to add HFSTS1 or HFS1 to indicate that this is a field in HFSTS1 register […]
Implemented the comment in the patch - https://review.coreboot.org/c/coreboot/+/38247/1
https://review.coreboot.org/c/coreboot/+/35546/14/src/soc/intel/common/block... PS14, Line 86: MODE
I think it would be better to call this COM = Current Operation Mode. […]
Implemented the comment in the patch - https://review.coreboot.org/c/coreboot/+/38247/1
https://review.coreboot.org/c/coreboot/+/35546/14/src/soc/intel/common/block... PS14, Line 678: }
Why is resp. […]
comment is addressed in the patch - https://review.coreboot.org/c/coreboot/+/37283
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 143:
nit: extra blank line not required.
Implemented in the patch : https://review.coreboot.org/c/coreboot/+/38248
https://review.coreboot.org/c/coreboot/+/35546/14/src/soc/intel/common/block... PS14, Line 152: *
nit: space after *
Change is done in the patch - https://review.coreboot.org/c/coreboot/+/38248
-
Sridhar Siricilla (Code Review)