[coreboot-gerrit] New patch to review for coreboot: Revert "arch/x86/smbios: Correct manufacturer ID"

[Nico Huber (nico.h@gmx.de)]

Nico Huber (nico.h at gmx.de) just uploaded a new patch set to gerrit, which you can find at https://review.coreboot.org/17873

-gerrit

commit c49df4927df0a56ab46446dd2b3d66bc4d2cfab2
Author: Nico Huber <nico.h at gmx.de>
Date:   Wed Dec 14 23:48:09 2016 +0100

    Revert "arch/x86/smbios: Correct manufacturer ID"
    
    This reverts commit c86da67436827c25919a2f5966049485a58fc984.
    
    Alas, I have to disagree with this in every single line. The comment
    added to the top of the file only applies to a single function therein
    which sits over a hundred lines below. That's not much helpful. More-
    over, the link in the comment is already down ofc.
    
    The comment is also irritating as it doesn't state in which way (enco-
    ding!) it applies to the code, which presumably led to the wrong in-
    terpretation of the IDs.
    
    At last, if anything should have changed it is the strings, the IDs
    are resolved to. `smbios_fill_dimm_manufacturer_from_id()` has to
    resolve the IDs it gets actually fed and not a random selection from
    any spec.
    
    Since I digged into it, here's why the numbers are correct: The func-
    tion started with the SPD encoding of DDR3 in mind. There, the lower
    byte is the number of a "bank" of IDs with an odd-parity in the upper
    most bit. The upper byte is the ID within the bank. The "correction"
    was to clear the parity bit for naught. The function was later exten-
    ded with IDs in the DDR2-SPD encoding (which is actually 64-bit not
    16). There, a byte, starting from the lowest, is either an ID below
    127 plus odd-parity, or 127 which means look in the next byte/bank.
    Unused bytes seem to be filled with 0xff, I guess from the 0xff2c.
    
    Change-Id: Icdb48e4f2c102f619fbdca856e938e85135cfb18
---
 src/arch/x86/smbios.c | 21 +++++++++------------
 1 file changed, 9 insertions(+), 12 deletions(-)

diff --git a/src/arch/x86/smbios.c b/src/arch/x86/smbios.c
index 0c7f3ac..8640200 100644
--- a/src/arch/x86/smbios.c
+++ b/src/arch/x86/smbios.c
@@ -15,13 +15,6 @@
  * GNU General Public License for more details.
  */
 
-/*
- * Standard Manufacturer's Identification Code
- * JEP106AS (Revision of JEP106AR, October 2015)
- * MAY 2016
- * http://www.jedec.org/standards-documents/results/JEP106AS
- */
-
 #include <stdlib.h>
 #include <string.h>
 #include <smbios.h>
@@ -134,7 +127,7 @@ static int smbios_processor_name(char *start)
 void smbios_fill_dimm_manufacturer_from_id(uint16_t mod_id, struct smbios_type17 *t)
 {
 	switch (mod_id) {
-		case 0x9b05:
+		case 0x2c80:
 			t->manufacturer = smbios_add_string(t->eos,
 							    "Crucial");
 			break;
@@ -150,7 +143,7 @@ void smbios_fill_dimm_manufacturer_from_id(uint16_t mod_id, struct smbios_type17
 			t->manufacturer = smbios_add_string(t->eos,
 							    "Kingston");
 			break;
-		case 0xad00:
+		case 0x987f:
 			t->manufacturer = smbios_add_string(t->eos,
 							    "Hynix");
 			break;
@@ -162,7 +155,11 @@ void smbios_fill_dimm_manufacturer_from_id(uint16_t mod_id, struct smbios_type17
 			t->manufacturer = smbios_add_string(t->eos,
 							    "OCZ");
 			break;
-		case 0x3406:
+		case 0xad80:
+			t->manufacturer = smbios_add_string(t->eos,
+							    "Hynix/Hyundai");
+			break;
+		case 0xb502:
 			t->manufacturer = smbios_add_string(t->eos,
 							    "SuperTalent");
 			break;
@@ -170,7 +167,7 @@ void smbios_fill_dimm_manufacturer_from_id(uint16_t mod_id, struct smbios_type17
 			t->manufacturer = smbios_add_string(t->eos,
 							    "GSkill");
 			break;
-		case 0xce00:
+		case 0xce80:
 			t->manufacturer = smbios_add_string(t->eos,
 							    "Samsung");
 			break;
@@ -178,7 +175,7 @@ void smbios_fill_dimm_manufacturer_from_id(uint16_t mod_id, struct smbios_type17
 			t->manufacturer = smbios_add_string(t->eos,
 							    "Elpida");
 			break;
-		case 0x2c00:
+		case 0xff2c:
 			t->manufacturer = smbios_add_string(t->eos,
 							    "Micron");
 			break;