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:
(5 comments)
https://review.coreboot.org/c/coreboot/+/41612/3/util/mainboard/google/spd_t... File util/mainboard/google/spd_tools/lp4x/gen_spd.go:
https://review.coreboot.org/c/coreboot/+/41612/3/util/mainboard/google/spd_t... PS3, Line 280: Intel
This is byte 16 (signal loading).
Checked MRC. This is not even a defined field in MRC structure. Dropping from the tool.
https://review.coreboot.org/c/coreboot/+/41612/3/util/mainboard/google/spd_t... PS3, Line 280: /* Unsure about this ??? Check with Intel!!! */
I have asked for clarification here: https://b.corp.google. […]
Done
https://review.coreboot.org/c/coreboot/+/41612/3/util/mainboard/google/spd_t... PS3, Line 617: func validateChannels(channels int) {
What is a channel in LPDDR4x? Likewise, what does datawidth represent? Per die?
Channel as input from the JSON file is the physical channel which can be x8 or x16. Datawidth is supposed to be per channel. Should there be more info/comments added to README or this file?
https://review.coreboot.org/c/coreboot/+/41612/8/util/spd_tools/intel/lp4x/g... File util/spd_tools/intel/lp4x/gen_spd.go:
https://review.coreboot.org/c/coreboot/+/41612/8/util/spd_tools/intel/lp4x/g... PS8, Line 333: SPD_INDEX_OPTIONAL_FEATURES = 7
You are looking at the right file. […]
I have left it as 0x00 for now. We can revisit this with Intel.
https://review.coreboot.org/c/coreboot/+/41612/8/util/spd_tools/intel/lp4x/g... PS8, Line 335: SPD_INDEX_BUS_WIDTH = 13
Byte 14 is not used by MRC: https://docs.google. […]
Left as 0x00.