Nick Vaccaro has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40463 )
Change subject: mb/google/volteer: add generic SPDs ......................................................................
Patch Set 16:
(1 comment)
https://review.coreboot.org/c/coreboot/+/40463/16/src/mainboard/google/volte... File src/mainboard/google/volteer/spd/SPD_LPDDR4X_200b_2R_32Gb_QDP_4266.spd.hex:
PS16:
There is already a SPD_LPDDR4X_200b_2R_32Gb_QDP_4267 spd. […]
Yes, Micron/Hynix (SPD_LPDDR4X_200b_2R_32Gb_QDP_4267) and Samsung (SPD_LPDDR4X_200b_2R_32Gb_QDP_4266) support different CAS Latencies and have different Min/Max SDRAM Cycle Times. They are both 4267 speed. I believe Intel changed the speed text used in the filename to 4266 for Samsung to avoid the name clash with the SPD used for Micron/Hynix.
I would like to fix the speed value, but we will need to add at least one category to the filenaming convention to do so. If you believe a longer filename that includes the other differentiable categories would lead to less confusion overall, I will rename all of the generic SPDs to include the three extra categories.
So far, the SPD_LPDDR4X_200b_2R_32Gb_QDP_4266.spd.hex file is the only one that runs into this filename clash issue, but we may see more down the road.
-
Nick Vaccaro (Code Review)