Attention is currently required from: Nico Huber, Jamie Ryu, Subrata Banik, Ethan Tsao, Ravishankar Sarawadi, Paul Menzel, Angel Pons, Arthur Heymans, Nick Vaccaro, Michael Niewöhner, Tim Wawrzynczak. Wonkyu Kim has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63170 )
Change subject: intel/common/block: Add gpmr common driver ......................................................................
Patch Set 14:
(8 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/63170/comment/08ab9f51_f391e36a PS14, Line 9: Move GPMR(General Purpose Memory Range) APIs to gpmr driver from dmi.c : For this, 3 patches are used. : 1. Add GPMR common driver in IA common code(CB:63170) : 2. Migrate all DMI API usage to GPMR(CB:63471) : 3. Drop DMI driver (CB:63472) :
Thank you, guys!
Done
File src/soc/intel/common/block/gpmr/gpmr.c:
https://review.coreboot.org/c/coreboot/+/63170/comment/21197886_4954feaf PS13, Line 39: printk(BIOS_ERR, "%s: No available free gpmr found\n", __func__);
Maybe also log MAX_GPMR_REGS.
Done
https://review.coreboot.org/c/coreboot/+/63170/comment/beeea50c_fa90bd73 PS13, Line 44: uint32_t
Why?
Done
https://review.coreboot.org/c/coreboot/+/63170/comment/1e0e1f14_f39455bb PS13, Line 46: int
Indeed, but maybe a better name can be found? `num` is misleading at list to me.
Will rename existing dmi.c to gpmr.c
File src/soc/intel/common/block/gpmr/gpmr.c:
https://review.coreboot.org/c/coreboot/+/63170/comment/7b2b5ce7_16c3e02c PS14, Line 22: data32 = gpmr_read32(offset); : data32 |= ordata; : gpmr_write32(offset, data32);
pcr_or32(PID_DMI, offset, ordata)
Done
https://review.coreboot.org/c/coreboot/+/63170/comment/76f2326f_f2d16339 PS14, Line 46: base & ~(GPMR_BASE_MASK << GPMR_BASE_SHIFT)
use IS_ALIGNED
Done
https://review.coreboot.org/c/coreboot/+/63170/comment/f8e6dd82_9c1e9d92 PS14, Line 47: printk(BIOS_ERR, "base is not 64-KiB aligned!\n");
Maybe make it more explicit (!IS_ALIGNED(base, 64 * KiB))?
Done
File src/soc/intel/common/block/include/intelblocks/pcr_gpmr.h:
https://review.coreboot.org/c/coreboot/+/63170/comment/40142ea5_f38b9fd8 PS14, Line 21: GPMR_BASE_SHIFT
I'm not sure how common this is, but I'm much more familiar with CPP definitions of shifts being lef […]
Done
-
Wonkyu Kim (Code Review)