util/autoport: Assign SPD addresses in devicetree

Commit 45e4ab4a660c (mb/*: Update SPD mapping for sandybridge boards)
changed the way in which SPD addresses are set up for SNB/IVB boards,
but autoport was not updated to reflect these changes. Result is:

    register "spd_addresses" = "{0x50, 0x51, 0x52, 0x53}" # FIXME: Put proper SPD map here"

The stray quote at the end is irritating, but is hard to get rid of
without substantial refactoring of autoport's guts. But, given that
this is a FIXME comment, anyone using autoport should just drop the
comment after verifying the SPD map, so it's not a big deal.

In addition, update the corresponding section of the README, which
was horrendously out-of-date.

Change-Id: I6ad38f53afc4fafb45be7f086723cc0782a965ed
Signed-off-by: Angel Pons <th3fanbus@gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/82405
Reviewed-by: Keith Hui <buurin@gmail.com>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>

3 files changed, 46 insertions, 60 deletions

diff --git a/util/autoport/bd82x6x.go b/util/autoport/bd82x6x.go
index e2dc827f1d..a4ae2459b5 100644
--- a/util/autoport/bd82x6x.go
+++ b/util/autoport/bd82x6x.go
@@ -297,7 +297,6 @@ func (b bd82x6x) Scan(ctx Context, addr PCIDevData) {
 	sb.WriteString(`
 #include <bootblock_common.h>
 #include <device/pci_ops.h>
-#include <northbridge/intel/sandybridge/raminit_native.h>
 #include <southbridge/intel/bd82x6x/pch.h>
 
 `)
@@ -344,8 +343,6 @@ func (b bd82x6x) Scan(ctx Context, addr PCIDevData) {
 	}
 	sb.WriteString("};\n")
 
-	guessedMap := GuessSPDMap(ctx)
-
 	sb.WriteString(`
 void bootblock_mainboard_early_init(void)
 {
@@ -354,15 +351,6 @@ void bootblock_mainboard_early_init(void)
 
 	RestorePCI16Simple(sb, addr, 0x80)
 
-	sb.WriteString(`}
-
-/* FIXME: Put proper SPD map here. */
-void mainboard_get_spd(spd_raw_data *spd, bool id_only)
-{
-`)
-	for i, spd := range guessedMap {
-		fmt.Fprintf(sb, "\tread_spd(&spd[%d], 0x%02x, id_only);\n", i, spd)
-	}
 	sb.WriteString("}\n")
 
 	gnvs := Create(ctx, "acpi_tables.c")
diff --git a/util/autoport/readme.md b/util/autoport/readme.md
index b546120fca..149e1ca1ed 100644
--- a/util/autoport/readme.md
+++ b/util/autoport/readme.md
@@ -109,15 +109,19 @@ Refer to Rookie Guide, Lesson 2 for instructions on how to submit a patch.
 
 ## Manual fixes
 ### SPD
-In order to initialize the RAM memory, coreboot needs to know its timings, which vary between
+In order to initialize the RAM (memory), coreboot needs to know its timings, which vary between
 modules. Socketed RAM has a small EEPROM chip, which is accessible via SMBus and contains the
 timing data. This data is usually known as SPD. Unfortunately, the SMBus addresses may not
 correlate with the RAM slots and cannot always be detected automatically. The address map is
-encoded in function `mainboard_get_spd` in `romstage.c`. By default, autoport uses the most
-common map `0x50, 0x51, 0x52, 0x53` on everything except for Lenovo systems, which are known
-to use `0x50, 0x52, 0x51, 0x53`. To detect the correct memory map, the easiest way is to boot
-on the vendor firmware with just one module in channel 0, slot 0, and check the SMBus address
-the EEPROM has. Under Linux, you can use these commands to see what is on SMBus:
+usually in the devicetree, `register "spd_addresses"`. For mainboards with memory-down (where
+the RAM chips are soldered directly to the mainboard), there is no EEPROM to get SPD data from,
+so function `mb_get_spd_map` in `early_init.c` has to populate the SPD data from a file in CBFS.
+
+By default, autoport uses the most common map `0x50, 0x51, 0x52, 0x53` on everything except for
+Lenovo systems, which are known to use `0x50, 0x52, 0x51, 0x53`. To detect the correct memory
+map, the easiest way is to boot on the vendor firmware with just one module in channel 0, slot
+0, and check the SMBus address the EEPROM has. Under Linux, you can use these commands to see
+which devices appear on SMBus:
 
 	$ sudo modprobe i2c-dev
 	$ sudo modprobe i2c-i801
@@ -148,23 +152,20 @@ the EEPROM has. Under Linux, you can use these commands to see what is on SMBus:
 	60: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --
 	70: -- -- -- -- -- -- -- --
 
+Note: if some devices appear as `UU`, it means a kernel module is loaded for this device, like
+`at24` or `ee1004`. You can use the `decode-dimms` command to get more information about SPDs.
+
 Make sure to replace the `9` on the last command with the bus number for SMBus on
 your system. Here, there is a module at address `0x50`. Since only one module was
 installed on the first slot of the first channel, we know the first position of
-the SPD array must be `0x50`. After testing all the slots, your `mainboard_get_spd`
+the SPD array must be `0x50`. After testing all the slots, your `spd_addresses`
 should look similar to this:
 
-	void mainboard_get_spd(spd_raw_data *spd) {
-		read_spd(&spd[0], 0x50);
-		read_spd(&spd[1], 0x51);
-		read_spd(&spd[2], 0x52);
-		read_spd(&spd[3], 0x53);
-	}
-
-Note that there should be one line per memory slot on the mainboard.
+	register "spd_addresses" = "{0x50,    0, 0x52,    0}" # 2-slot mainboard / laptop
+	register "spd_addresses" = "{0x53, 0x52, 0x51, 0x50}" # 4-slot BTX mainboard
 
-Note: slot labelling may be missing or unreliable. Use `inteltool` to see
-which slots have modules in them.
+Note: slot labelling may be missing or unreliable. Use `inteltool` to see which slots have
+modules in them.
 
 This procedure is ideal, if your RAM is socketed. If you have soldered RAM,
 remove any socketed memory modules and check if any EEPROM appears on SMBus.
@@ -212,12 +213,11 @@ should be something like this:
 	e0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
 	f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
 
-This is not a full-fledged SPD dump, as it only lists
-the currently-used speed configuration, and lacks info
-such as a serial number, vendor and model. Use `xxd`
-to create a binary file with this SPD data:
+This is not a full-fledged SPD dump, as it only lists the currently-used speed configuration,
+and lacks info such as a serial number, vendor and model. To create a SPD hex file, one has to
+trim the offset numbers from the leftmost column:
 
-	$ cat | xxd -r > spd.bin  <<EOF
+	$ cat | cut -d ' ' -f 2- > data.spd.hex
 	00: 92 11 0b 03 04 00 00 09 03 52 01 08 0a 00 80 00
 	10: 6e 78 6e 32 6e 11 18 81 20 08 3c 3c 00 f0 00 00
 	20: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
@@ -236,37 +236,25 @@ to create a binary file with this SPD data:
 	f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
 	EOF (press Ctrl + D)
 
-Then, move the generated file into your mainboard's directory
-and hook it up to the build system by adding the following
-lines to `Makefile.mk`:
-
-	cbfs-files-y += spd.bin
-	spd.bin-file := spd.bin
-	spd.bin-type := raw
+Then, move the generated file into your mainboard's directory and hook it up to the build
+system. It is recommended to check what other mainboards with soldered memory do. The main
+switch to use SPD files is `select HAVE_SPD_IN_CBFS` in Kconfig.
 
-Now we need coreboot to use this SPD file. The following example
-shows a hybrid configuration, in which one module is soldered and
-the other one is socketed:
+Now we need coreboot to use this SPD file. The following example shows a hybrid configuration,
+in which one module is soldered down and the other one is socketed:
 
-	void mainboard_get_spd(spd_raw_data *spd)
+	void mb_get_spd_map(struct spd_info *spdi)
 	{
-		void *spd_file;
-		size_t spd_file_len = 0;
-		/* C0S0 is a soldered RAM with no real SPD. Use stored SPD. */
-		spd_file = cbfs_boot_map_with_leak("spd.bin", CBFS_TYPE_RAW,
-						 &spd_file_len);
-		if (spd_file && spd_file_len >= 128)
-			memcpy(&spd[0], spd_file, 128);
-
-		/* C1S0 is a physical slot. */
-		read_spd(&spd[2], 0x52);
+		spdi->spd_index = 0;
+		/* C0S0 is soldered RAM, use stored SPD */
+		spdi->addresses[0] = SPD_MEMORY_DOWN;
+		/* C1S0 is a physical slot, use SPD address on SMBus */
+		spdi->addresses[2] = 0x52;
 	}
 
-If several slots are soldered there are two ways to handle them:
-
-* If all use the same SPD data, use the same file for all the slots. Do
-  not forget to copy the data on all the array elements that need it.
-* If they use different data, use several files.
+If several slots are soldered, the system only accounts for a single set of SPD data. So all
+slots would need to use the same SPD data, if possible. If not possible, the API needs to be
+adapted accordingly, which is significantly more involved.
 
 If memory initialization is not working, in particular write training (timB)
 on DIMM's second rank fails, try enabling rank 1 mirroring, which can't be
diff --git a/util/autoport/sandybridge.go b/util/autoport/sandybridge.go
index bd7f0f0c30..29b57b46ef 100644
--- a/util/autoport/sandybridge.go
+++ b/util/autoport/sandybridge.go
@@ -1,10 +1,19 @@
 package main
 
 import "fmt"
+import "strings"
 
 type sandybridgemc struct {
 }
 
+func MakeSPDMap(ctx Context) string {
+	var values []string
+	for _, addr := range GuessSPDMap(ctx) {
+		values = append(values, fmt.Sprintf("0x%02x", addr))
+	}
+	return "{"+strings.Join(values, ", ")+"}"
+}
+
 func (i sandybridgemc) Scan(ctx Context, addr PCIDevData) {
 	inteltool := ctx.InfoSource.GetInteltool()
 
@@ -33,6 +42,7 @@ func (i sandybridgemc) Scan(ctx Context, addr PCIDevData) {
 			"gpu_cpu_backlight":                   FormatHex32(inteltool.IGD[0x48254]),
 			"gpu_pch_backlight":                   FormatHex32((inteltool.IGD[0xc8254] >> 16) * 0x10001),
 			"gfx": fmt.Sprintf("GMA_STATIC_DISPLAYS(%d)", (inteltool.IGD[0xc6200] >> 12) & 1),
+			"spd_addresses": MakeSPDMap(ctx)+"\" # FIXME: Put proper SPD map here",
 		},
 		Children: []DevTreeNode{
 			{