Hello Felix Singer, build bot (Jenkins), Nico Huber, Patrick Georgi, Furquan Shaikh, Angel Pons, Michael Niewöhner,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/44999
to review the following change.
Change subject: Revert "util/spd_tools: output binaries instead of hexdumps" ......................................................................
Revert "util/spd_tools: output binaries instead of hexdumps"
This reverts commit f23794cf04030bb8d1d7ebe0a3634dffd092e2f7.
Reason for revert: This change breaks compatibility if the changes in CB:44775 are not also included. CB:44775 is still under discussion, so reverting this change to make spd_tools usable again.
Change-Id: I5840a1b895dcbc8b91c76d8b60df2f95b93a4370 --- M util/spd_tools/ddr4/README.md M util/spd_tools/ddr4/gen_part_id.go M util/spd_tools/ddr4/gen_spd.go M util/spd_tools/lp4x/README.md M util/spd_tools/lp4x/gen_spd.go 5 files changed, 47 insertions(+), 39 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/99/44999/1
diff --git a/util/spd_tools/ddr4/README.md b/util/spd_tools/ddr4/README.md index c78b06f..7527544 100644 --- a/util/spd_tools/ddr4/README.md +++ b/util/spd_tools/ddr4/README.md @@ -171,7 +171,7 @@ This tool generates the following files using the global list of memory parts in JSON format as described above: * De-duplicated SPDs required for the different memory parts. These - SPD files are named (ddr4-spd-1.bin, ddr4-spd-2.bin, and so on) + SPD files are named (ddr4-spd-1.hex, ddr4-spd-2.hex, and so on) and placed in the directory provided as an input to the tool. * CSV file representing which of the deduplicated SPD files is used by which memory part. This file is named as @@ -179,11 +179,11 @@ as an input to the tool along with the generated SPD files. Example CSV file: ``` - MEMORY_PART_A, ddr4-spd-1.bin - MEMORY_PART_B, ddr4-spd-2.bin - MEMORY_PART_C, ddr4-spd-3.bin - MEMORY_PART_D, ddr4-spd-2.bin - MEMORY_PART_E, ddr4-spd-2.bin + MEMORY_PART_A, ddr4-spd-1.hex + MEMORY_PART_B, ddr4-spd-2.hex + MEMORY_PART_C, ddr4-spd-3.hex + MEMORY_PART_D, ddr4-spd-2.hex + MEMORY_PART_E, ddr4-spd-2.hex ```
## Tool 2 - gen_part_id.go @@ -242,10 +242,10 @@ ## This is an auto-generated file. Do not edit!!
SPD_SOURCES = -SPD_SOURCES += ddr4-spd-1.bin # ID = 0(0b0000) Parts = MEMORY_PART_A -SPD_SOURCES += ddr4-spd-2.bin # ID = 1(0b0001) Parts = MEMORY_PART_B, MEMORY_PART_D -SPD_SOURCES += ddr4-spd-empty.bin # ID = 2(0b0010) -SPD_SOURCES += ddr4-spd-3.bin # ID = 2(0b0010) Parts = MEMORY_PART_C +SPD_SOURCES += ddr4-spd-1.hex # ID = 0(0b0000) Parts = MEMORY_PART_A +SPD_SOURCES += ddr4-spd-2.hex # ID = 1(0b0001) Parts = MEMORY_PART_B, MEMORY_PART_D +SPD_SOURCES += ddr4-spd-empty.hex # ID = 2(0b0010) +SPD_SOURCES += ddr4-spd-3.hex # ID = 2(0b0010) Parts = MEMORY_PART_C ``` NOTE: Empty entries may be required if there is a gap created by a memory part with a fixed id. diff --git a/util/spd_tools/ddr4/gen_part_id.go b/util/spd_tools/ddr4/gen_part_id.go index 29b91fa..c0098ab 100644 --- a/util/spd_tools/ddr4/gen_part_id.go +++ b/util/spd_tools/ddr4/gen_part_id.go @@ -265,7 +265,7 @@
for i := 0; i < len(partIdList); i++ { if partIdList[i].SPDFileName == "" { - s += fmt.Sprintf("SPD_SOURCES += %s ", "ddr4-spd-empty.bin") + s += fmt.Sprintf("SPD_SOURCES += %s ", "ddr4-spd-empty.hex") s += fmt.Sprintf(" # ID = %d(0b%04b)\n", i, int64(i)) } else { s += fmt.Sprintf("SPD_SOURCES += %s ", partIdList[i].SPDFileName) diff --git a/util/spd_tools/ddr4/gen_spd.go b/util/spd_tools/ddr4/gen_spd.go index b60ab03..3c8f71a 100644 --- a/util/spd_tools/ddr4/gen_spd.go +++ b/util/spd_tools/ddr4/gen_spd.go @@ -3,7 +3,6 @@ package main
import ( - "bytes" "encoding/json" "fmt" "io/ioutil" @@ -969,8 +968,8 @@ return e.constVal }
-func createSPD(memAttribs *memAttributes) bytes.Buffer { - var spd bytes.Buffer +func createSPD(memAttribs *memAttributes) string { + var s string
for i := 0; i < 512; i++ { var b byte = 0 @@ -978,10 +977,14 @@ b = getSPDByte(i, memAttribs) }
- spd.WriteByte(b) + if (i + 1) % 16 == 0 { + s += fmt.Sprintf("%02X\n", b) + } else { + s += fmt.Sprintf("%02X ", b) + } }
- return spd + return s }
func dedupeMemoryPart(dedupedParts []*memPart, memPart *memPart) bool { @@ -996,16 +999,16 @@ }
func generateSPD(memPart *memPart, SPDId int, SPDDirName string) { - spd := createSPD(&memPart.Attribs) - memPart.SPDFileName = fmt.Sprintf("ddr4-spd-%d.bin", SPDId) - ioutil.WriteFile(filepath.Join(SPDDirName, memPart.SPDFileName), spd.Bytes(), 0644) + s := createSPD(&memPart.Attribs) + memPart.SPDFileName = fmt.Sprintf("ddr4-spd-%d.hex", SPDId) + ioutil.WriteFile(filepath.Join(SPDDirName, memPart.SPDFileName), []byte(s), 0644) }
func generateEmptySPD(SPDDirName string) {
- spd := createSPD(nil) - SPDFileName := "ddr4-spd-empty.bin" - ioutil.WriteFile(filepath.Join(SPDDirName, SPDFileName), spd.Bytes(), 0644) + s := createSPD(nil) + SPDFileName := "ddr4-spd-empty.hex" + ioutil.WriteFile(filepath.Join(SPDDirName, SPDFileName), []byte(s), 0644) }
func readMemoryParts(memParts *memParts, memPartsFileName string) error { diff --git a/util/spd_tools/lp4x/README.md b/util/spd_tools/lp4x/README.md index 0c49dad..e614f25 100644 --- a/util/spd_tools/lp4x/README.md +++ b/util/spd_tools/lp4x/README.md @@ -168,7 +168,7 @@ This tool generates the following files using the global list of memory parts in JSON format as described above: * De-duplicated SPDs required for the different memory parts. These - SPD files are named (spd_1.bin, spd_2.bin, spd_3.bin and so on) + SPD files are named (spd_1.hex, spd_2.hex, spd_3.hex and so on) and placed in the directory provided as an input to the tool. * CSV file representing which of the deduplicated SPD files is used by which memory part. This file is named as @@ -176,11 +176,11 @@ as an input to the tool along with the generated SPD files. Example CSV file: ``` - MEMORY_PART_A, spd_1.bin - MEMORY_PART_B, spd_2.bin - MEMORY_PART_C, spd_3.bin - MEMORY_PART_D, spd_2.bin - MEMORY_PART_E, spd_2.bin + MEMORY_PART_A, spd_1.hex + MEMORY_PART_B, spd_2.hex + MEMORY_PART_C, spd_3.hex + MEMORY_PART_D, spd_2.hex + MEMORY_PART_E, spd_2.hex ```
## Tool 2 - gen_part_id.go @@ -222,9 +222,9 @@ ## This is an auto-generated file. Do not edit!!
SPD_SOURCES = -SPD_SOURCES += spd_1.bin # ID = 0(0b0000) Parts = MEMORY_PART_A -SPD_SOURCES += spd_2.bin # ID = 1(0b0001) Parts = MEMORY_PART_B, MEMORY_PART_D -SPD_SOURCES += spd_3.bin # ID = 2(0b0010) Parts = MEMORY_PART_C +SPD_SOURCES += spd_1.hex # ID = 0(0b0000) Parts = MEMORY_PART_A +SPD_SOURCES += spd_2.hex # ID = 1(0b0001) Parts = MEMORY_PART_B, MEMORY_PART_D +SPD_SOURCES += spd_3.hex # ID = 2(0b0010) Parts = MEMORY_PART_C ```
### Note of caution diff --git a/util/spd_tools/lp4x/gen_spd.go b/util/spd_tools/lp4x/gen_spd.go index 1738833..e63ca8d 100644 --- a/util/spd_tools/lp4x/gen_spd.go +++ b/util/spd_tools/lp4x/gen_spd.go @@ -3,7 +3,6 @@ package main
import ( - "bytes" "encoding/json" "fmt" "io/ioutil" @@ -638,14 +637,20 @@ return e.constVal }
-func createSPD(memAttribs *memAttributes) bytes.Buffer { - var spd bytes.Buffer +func createSPD(memAttribs *memAttributes) string { + var s string
for i := 0; i < 512; i++ { - spd.WriteByte(getSPDByte(i, memAttribs)) + b := getSPDByte(i, memAttribs) + + if (i + 1) % 16 == 0 { + s += fmt.Sprintf("%02X\n", b) + } else { + s += fmt.Sprintf("%02X ", b) + } }
- return spd + return s }
func dedupeMemoryPart(dedupedParts []*memPart, memPart *memPart) bool { @@ -660,9 +665,9 @@ }
func generateSPD(memPart *memPart, SPDId int, SPDDirName string) { - spd := createSPD(&memPart.Attribs) - memPart.SPDFileName = fmt.Sprintf("lp4x-spd-%d.bin", SPDId) - ioutil.WriteFile(filepath.Join(SPDDirName, memPart.SPDFileName), spd.Bytes(), 0644) + s := createSPD(&memPart.Attribs) + memPart.SPDFileName = fmt.Sprintf("lp4x-spd-%d.hex", SPDId) + ioutil.WriteFile(filepath.Join(SPDDirName, memPart.SPDFileName), []byte(s), 0644) }
func readMemoryParts(memParts *memParts, memPartsFileName string) error {