Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/41612 )
Change subject: util: Add spd_tools to generate SPDs for TGL boards ......................................................................
Patch Set 12:
(4 comments)
https://review.coreboot.org/c/coreboot/+/41612/8/util/spd_tools/intel/lp4x/R... File util/spd_tools/intel/lp4x/README.md:
https://review.coreboot.org/c/coreboot/+/41612/8/util/spd_tools/intel/lp4x/R... PS8, Line 46: densityPerChannelGb
Is it the same as density per Die i.e. […]
No, this is updated to density per physical channel in the latest patchset.
https://review.coreboot.org/c/coreboot/+/41612/10/util/spd_tools/intel/lp4x/... File util/spd_tools/intel/lp4x/README.md:
https://review.coreboot.org/c/coreboot/+/41612/10/util/spd_tools/intel/lp4x/... PS10, Line 65: SDRAM minimum cycle time
https://review.coreboot.org/c/coreboot/+/41612/8/util/spd_tools/intel/lp4x/g... […]
Done. Added as optional attribute.
https://review.coreboot.org/c/coreboot/+/41612/10/util/spd_tools/intel/lp4x/... PS10, Line 117: "diesPerPackage": 2,
Nit: diesPerPackage has been removed in the later patchset. We have to use channelsPerDie.
Added it back.
https://review.coreboot.org/c/coreboot/+/41612/10/util/spd_tools/intel/lp4x/... File util/spd_tools/intel/lp4x/gen_spd.go:
https://review.coreboot.org/c/coreboot/+/41612/10/util/spd_tools/intel/lp4x/... PS10, Line 119: CASFirstByte: CAS6 | CAS10 | CAS14,
This is not right. CAS is also dependent on byte-mode v/s x16 mode. […]
Done