Sridhar Siricilla has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35226 )
Change subject: src/soc/intel/common/block/cse: Make hfsts1 common & add helper functions ......................................................................
Patch Set 18:
(8 comments)
https://review.coreboot.org/c/coreboot/+/35226/17/src/soc/intel/common/block... File src/soc/intel/common/block/cse/cse.c:
https://review.coreboot.org/c/coreboot/+/35226/17/src/soc/intel/common/block... PS17, Line 247: int
unsigned int?
Done
https://review.coreboot.org/c/coreboot/+/35226/17/src/soc/intel/common/block... PS17, Line 253: else : return 0;
you can skip the else and directly return 0, if above condition is true the function would anyways r […]
Done
https://review.coreboot.org/c/coreboot/+/35226/17/src/soc/intel/common/block... PS17, Line 258: set_host_ready
i hope you have named _ready because setting CSR_READY bit but that is one part of this function. […]
No, this function makes host(BIOS/CB) ready to send messages to CSE. Hence, function named as set_host_ready().
https://review.coreboot.org/c/coreboot/+/35226/17/src/soc/intel/common/block... PS17, Line 258: set_host_ready
also why not static? i don't see any caller to standalone using this function?
The function will be used by subsequent patches so it can not be made static.
https://review.coreboot.org/c/coreboot/+/35226/17/src/soc/intel/common/block... PS17, Line 268: int
unsigned int?
Done
https://review.coreboot.org/c/coreboot/+/35226/17/src/soc/intel/common/block... File src/soc/intel/common/block/include/intelblocks/cse.h:
https://review.coreboot.org/c/coreboot/+/35226/17/src/soc/intel/common/block... PS17, Line 32: hfsts1
nit:me_hsts1? to be consistent with SOC definitions for rest of the hstsx used in SOC code. […]
Done
https://review.coreboot.org/c/coreboot/+/35226/17/src/soc/intel/skylake/me.c File src/soc/intel/skylake/me.c:
https://review.coreboot.org/c/coreboot/+/35226/17/src/soc/intel/skylake/me.c... PS17, Line 285: hfs
hsf1?
Done
https://review.coreboot.org/c/coreboot/+/35226/17/src/soc/intel/skylake/me.c... PS17, Line 487: hfs
same.
Done