Subrata Banik has posted comments on this change. ( https://review.coreboot.org/18669 )
Change subject: soc/intel/common/block: Add Intel common PCR support ......................................................................
Patch Set 7:
(4 comments)
https://review.coreboot.org/#/c/18669/7/src/soc/intel/common/block/include/i... File src/soc/intel/common/block/include/intelblocks/pcr.h:
Line 25: int pcr_read8(uint8_t pid, uint16_t offset, uint8_t *outdata);
Why would the writes() be asymmetric with the reads w.r.t. return value.
make sense, align pcr_rd prototype with pcr-wr, introducing assert there if returns -1. Actual function don't return anything as in caller function we don;t handle return type as such
https://review.coreboot.org/#/c/18669/7/src/soc/intel/common/block/pcr/pcr.c File src/soc/intel/common/block/pcr/pcr.c:
PS7, Line 23: uintptr_t reg_addr; : : /* Create an address based off of port id and offset. */ : reg_addr = CONFIG_PCR_BASE_ADDRESS; : reg_addr += ((uintptr_t)pid) << PCR_PORTID_SHIFT; : reg_addr += ((uintptr_t)offset & PCR_REG_MASK); : : return (void *)reg_addr;
lets move this below and get rid of same purpose function
Done
Line 43: static int pch_pcr_read(uint8_t pid, uint16_t offset, size_t size, void *data)
Again, same argument as the writes. I thought you understood it was a gener
Done
Line 120: assert(pch_pcr_write(pid, offset, sizeof(indata), indata) == 0);
This function should be the following:
Done