New patch to review for coreboot: 10fa51c Exynos5250: make vendor enums in the timing array much less prone to aliasing.

[Ronald G. Minnich]

Ronald G. Minnich ([e-mail]) just uploaded a new patch set to gerrit, which you can find at http://review.coreboot.org/2221

-gerrit

commit 10fa51c00d431f1d8de5f8d1432618893c357a58
Author: Ronald G. Minnich 
Date:   Tue Jan 29 11:52:05 2013 -0800

Exynos5250: make vendor enums in the timing array much less prone to aliasing.

The timing array is crucial to proper operation of DRAM. Getting a valid
    pointer to it is hence very important. Unfortunately, the constants
    chosen for the vendor were '1', and '2', (this in a 32-bit word) which
    in a debug print makes it almost impossible to tell if you've got a
    misaligned pointer. Note: coreboot people did not choose them :-)

So, give them values which are extremely unlikely to occur elsewhere
    in the array (or in memory, for that matter).

Given the frequency with which this check occurs, i.e. once,
    I would much prefer strings
    but I expect I'd get shouted down on that one. Constants in this case
    are an almost useless optimization but we'll go with them for now. Note
    no space is saved by not using strings: there's an entire function somewhere
    devoted to mapping the enum to a string!

Debug prints of pointers to structs in this array are now far more
    useful than they were.

See snarky comment in the code (left there to make sure nobody gets
    tempted to get fancy again).

Change-Id: I30bc44719f321f791fd82ded60e29393399d9e3d
    Signed-off-by: Ronald G. Minnich 
---
 src/cpu/samsung/exynos5250/dmc.h | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/src/cpu/samsung/exynos5250/dmc.h b/src/cpu/samsung/exynos5250/dmc.h
index d1dfbb1..5345664 100644
--- a/src/cpu/samsung/exynos5250/dmc.h
+++ b/src/cpu/samsung/exynos5250/dmc.h
@@ -152,12 +152,25 @@ enum ddr_mode {
    DDR_MODE_COUNT,
 };

+/* for reasons unknown, people are in the habit of taking a 32-bit
+ * field with 2 possible values and packing it with, say, 2 bits. This
+ * works fine until it doesn't, and when it doesn't is just want you
+ * want a robust encoding. So, let's be a bit smart here. First, yes
+ * yes yes it's clever to let the enum count entries for you, except
+ * in the common case when there are two of them, and a quick scan
+ * reveals that fact. So let's go ahead and do the really hard part of
+ * counting them. And, let's set the values to something we can easily
+ * scan for in memory. Since '1' and '2' are rather common, we pick
+ * something that's actually of some value when things go south.
+ * This setup motivated by a use case: something's going wrong and
+ * having a manuf name of '1' or '2' is completely useless!
+ */
 enum mem_manuf {
    MEM_MANUF_AUTODETECT,
-	MEM_MANUF_ELPIDA,
-	MEM_MANUF_SAMSUNG,
+	MEM_MANUF_ELPIDA = 0xe7b1da,
+	MEM_MANUF_SAMSUNG = 0x5a5096,

-	MEM_MANUF_COUNT,
+	MEM_MANUF_COUNT = 2, // fancy that. 
 };

/* CONCONTROL register fields */