Nick Vaccaro has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38606 )
Change subject: soc/intel/tigerlake: add memory configuration support ......................................................................
Patch Set 11:
(2 comments)
This change is ready for review.
https://review.coreboot.org/c/coreboot/+/38606/8/src/soc/intel/tigerlake/inc... File src/soc/intel/tigerlake/include/soc/meminit_tgl.h:
https://review.coreboot.org/c/coreboot/+/38606/8/src/soc/intel/tigerlake/inc... PS8, Line 26: lpddr4x_
nit: I don't think this prefix is required. This can be just named: enum spd_read_type.
I added "lpddr4x" because we dropped READ_SMBUS from this list, but I wasn't sure if it could potentially be needeed by other memory types (DDR4 ?). Is the fact volteer won't need READ_SMBUS due to tigerlake architecture, or the fact volteer is using lpddr4x?
Should I still drop the prefix?
https://review.coreboot.org/c/coreboot/+/38606/8/src/soc/intel/tigerlake/inc... PS8, Line 71: meminit_lpddr4x_dimm0
Just a thought: […]
Yes, I like your idea. I had been thinking about how to reduce the double-memcpy of the current scheme to make this more efficient, now would be a good time to do so.