Attention is currently required from: Jamie Ryu, Subrata Banik, Ethan Tsao, Ravishankar Sarawadi, Paul Menzel, Tim Wawrzynczak, Angel Pons, Nick Vaccaro. Wonkyu Kim has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63198 )
Change subject: soc/intel/common: implement ioc driver ......................................................................
Patch Set 12:
(6 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/63198/comment/78a39708_711a201c PS11, Line 9: IOC(I/O Cache)
Please add a space before (.
Ack
File src/soc/intel/common/block/gpmr/gpmr.c:
https://review.coreboot.org/c/coreboot/+/63198/comment/2a3b6306_d828ca37 PS5, Line 11: if (CONFIG(SOC_INTEL_COMMON_BLOCK_IOC)) : return ioc_reg_read32(offset); : else : return pcr_read32(PID_DMI, offset);
nit: you can implement as https://review.coreboot. […]
Let's use like this for now.
File src/soc/intel/common/block/gpmr/gpmr.c:
https://review.coreboot.org/c/coreboot/+/63198/comment/5d6c8904_57d6c4e0 PS12, Line 27: uint32_t data32; : : data32 = gpmr_read32(offset); : data32 |= ordata; : gpmr_write32(offset, data32);
this also need change for IOC isn't it ?
you mean ioc_or32?
File src/soc/intel/common/block/ioc/ioc.c:
https://review.coreboot.org/c/coreboot/+/63198/comment/fce7d35a_5e56d78a PS11, Line 16: return value;
Is the temporary variable needed?
Ack
File src/soc/intel/common/block/ioc/ioc.c:
https://review.coreboot.org/c/coreboot/+/63198/comment/4f90a6fc_e43baa56 PS12, Line 15: read32
isn't I have suggested to use read32p ?
Ack
File src/soc/intel/common/pch/lockdown/lockdown.c:
https://review.coreboot.org/c/coreboot/+/63198/comment/a2f96aca_02356379 PS12, Line 32: GPMR_DMICTL
is that because you don't have `GPMR_DMICTL` for IOC ?
Yes, With IOC interface, there is no CTL register and it does not need to do it.