Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44775 )
Change subject: treewide: stop using hexdumps for spd files ......................................................................
Patch Set 7:
(2 comments)
Patch Set 7:
Patch Set 7:
Patch Set 7:
Patch Set 7: Code-Review+1
(1 comment)
Just a random thought: Is `src/mb/<vendor>/<board>/spd` the right place to store the spd files? I mean, it seems a bit fragmented to me. Would it make sense to store them at a central place like src/spd/<vendor>/spd_file.bin? This way they can be reused.
Might be, but that's a separate change.
Sure, just wanted to throw that in :)
Indeed a good idea; I've seen many duplicated spd files
I think we will have to be careful about this organization. I agree that there are good number of cases where SPD files are duplicated. But, there is also the requirement that different SoCs might expect the SPDs to be organized differently. This is seen especially in the case of LPx memories. Example: even though TGL and JSL are using the same LP4x memory parts, their SPDs are quite different. So, that will have to be taken into account when moving the SPDs to a common location.
https://review.coreboot.org/c/coreboot/+/44775/7/src/soc/intel/tigerlake/spd... File src/soc/intel/tigerlake/spd/lp4x/spd-5.hex:
PS7: These files were autogenerated using spd_tools. I think it would be best to fix the tool to generate the .bin file instead of directly updating these.
https://review.coreboot.org/c/coreboot/+/44775/7/util/spd_tools/intel/lp4x/g... File util/spd_tools/intel/lp4x/gen_spd.go:
https://review.coreboot.org/c/coreboot/+/44775/7/util/spd_tools/intel/lp4x/g... PS7, Line 669: "spd-%d.bin" This is going to require more than s/hex/bin. The tool will have to generate the binary file directly.