Skip to contentSkip to conversation

decode-dimms: Module Configuration Type not reported for DDR2/3

DISCUSSION Andriy Gapon 2 participants 5 messages Apr 16, 2012 – May 13, 2012 4 messages not in the archive I2C subsystem host drivers

lore
Share & tools
lore
#0 lore raw

It seems that Module Configuration Type ("ECC", "No Parity", etc) is not reported for DDR2 and DDR3 module types. Not sure if resolution could be as easy as adding sdram_module_configuration_type() calls in the corresponding decode procedures.

Signature · 1 line
Andriy Gapon

A message that is not in the archive (sent off-list or not yet received): [email protected]

#2 lore raw

Hi Andriy,

Quoted 4 lines · Andriy Gapon

It seems that Module Configuration Type ("ECC", "No Parity", etc) is not reported for DDR2 and DDR3 module types. Not sure if resolution could be as easy as adding sdram_module_configuration_type() calls in the corresponding decode procedures.

Won't be so easy. For DDR2 there is one new bit to decode (Address/Command Parity). For DDR3 byte 11 has a completely different meaning, and the information seem to be no longer available. As I read the specification, the only way to tell if a DDR3 module has any form of parity or ECC is to check the extra bus width in bits 4-3 of byte 8. But this is a boolean type of information then, telling "there is some form of data error detection/correction". No way to tell between parity and ECC AFAICS, and information about Address/Command Parity is gone too.

I am attaching two patches, one for DDR2, one for DDR3, please give them a try.

Signature · 1 line
Jean Delvare
---
 eeprom/decode-dimms |   22 +++++++++++++++-------
 1 file changed, 15 insertions(+), 7 deletions(-)
eeprom/decode-dimms +15 -7
@@ -525,16 +525,21 @@ sub sdram_voltage_interface_level($)
 	return ($_[0] < @levels) ? $levels[$_[0]] : "Undefined!";
 }
 
-# Common to SDR and DDR SDRAM
+# Common to SDR, DDR and DDR2 SDRAM
 sub sdram_module_configuration_type($)
 {
-	my @types = (
-		"No Parity",			# 0
-		"Parity",			# 1
-		"ECC",				# 2
-	);
+	my $byte = $_[0] & 0x07;
+	my @edc;
 
-	return ($_[0] < @types) ? $types[$_[0]] : "Undefined!";
+	return "No Parity" if $byte == 0;
+
+	# Data ECC includes Data Parity so don't print both
+	push @edc, "Data Parity" if ($byte & 0x03) == 0x01;
+	push @edc, "Data ECC" if ($byte & 0x02);
+	# New in DDR2 specification
+	push @edc, "Address/Command Parity" if ($byte & 0x04);
+
+	return join ", ", @edc;
 }
 
 # Parameter: EEPROM bytes 0-127 (using 3-62)
@@ -1019,6 +1024,9 @@ sub decode_ddr2_sdram($)
 	printl("Voltage Interface Level",
 	       sdram_voltage_interface_level($bytes->[8]));
 
+	printl("Module Configuration Type",
+	       sdram_module_configuration_type($bytes->[11]));
+
 	printl("Refresh Rate", ddr2_refresh_rate($bytes->[12]));
 
 	my @burst;
---
 eeprom/decode-dimms |    2 ++
 1 file changed, 2 insertions(+)
eeprom/decode-dimms +2 -0
@@ -1169,6 +1169,8 @@ sub decode_ddr3_sdram($)
 
 	printl("SDRAM Device Width", (1 << (($bytes->[7] & 7) + 2))." bits");
 
+	printl("Bus Width Extension", ($bytes->[8] & 24)." bits");
+
 	my $taa;
 	my $trcd;
 	my $trp;

A message that is not in the archive (sent off-list or not yet received): [email protected]

#4 lore raw

on 17/04/2012 14:18 Jean Delvare said the following:

Quoted 18 lines

Hi Andriy,

Quoted 4 lines · Andriy Gapon

It seems that Module Configuration Type ("ECC", "No Parity", etc) is not reported for DDR2 and DDR3 module types. Not sure if resolution could be as easy as adding sdram_module_configuration_type() calls in the corresponding decode procedures.

Won't be so easy. For DDR2 there is one new bit to decode (Address/Command Parity). For DDR3 byte 11 has a completely different meaning, and the information seem to be no longer available. As I read the specification, the only way to tell if a DDR3 module has any form of parity or ECC is to check the extra bus width in bits 4-3 of byte 8. But this is a boolean type of information then, telling "there is some form of data error detection/correction". No way to tell between parity and ECC AFAICS, and information about Address/Command Parity is gone too.

Quoted 3 lines

I am attaching two patches, one for DDR2, one for DDR3, please give them a try.

Thanks a lot!
I am able to test only the DDR2 part at the moment.
With the patch I am getting the following
Module Configuration Type Data ECC
for these modules: http://www.valueram.com/datasheets/kvr800d2e5_2g.pdf

For some definitely non-ECC modules I get the expected
Module Configuration Type No Parity

P.S.
I've noticed that the "Decoding EEPROM" message is printed with $dimm[$current]->{eeprom} when $opt_side_by_side is true but it is printed with $dimm[$current]->{file} in the other case. Not sure if this is on purpose.

Signature · 1 line
Andriy Gapon

A message that is not in the archive (sent off-list or not yet received): [email protected]

#6 lore raw

Hi Andriy,

Quoted 4 lines · Andriy Gapon

on 17/04/2012 14:18 Jean Delvare said the following:

Quoted 2 lines

I am attaching two patches, one for DDR2, one for DDR3, please give them a try.

Quoted 8 lines

Thanks a lot!
I am able to test only the DDR2 part at the moment.
With the patch I am getting the following
Module Configuration Type Data ECC
for these modules: http://www.valueram.com/datasheets/kvr800d2e5_2g.pdf

For some definitely non-ECC modules I get the expected
Module Configuration Type No Parity

Thanks for the report, I've applied both patches.

Quoted 4 lines

P.S.
I've noticed that the "Decoding EEPROM" message is printed with $dimm[$current]->{eeprom} when $opt_side_by_side is true but it is printed with $dimm[$current]->{file} in the other case. Not sure if this is on purpose.

I think it is on purpose. ->{file} contains the full path so it is rather long, ->{eeprom} only contains the file name so it is much shorter. The full path would make side-by-side output very wide, that wouldn't be convenient, that's why the short name is used in this output mode. In legacy output mode, the long name fits.

We _could_ use the short name in legacy output mode, as the long name doesn't necessarily add a lot of value. I think I tried to leave the legacy output unchanged when implementing the side-by-side output, to avoid any regression in case someone was parsing the output of decode-dimms. If you believe it would be better to always use the short name, we could do that, this may even allow for a small code clean-up.

Signature · 1 line
Jean Delvare

A message that is not in the archive (sent off-list or not yet received): [email protected]

#8 lore raw

on 18/04/2012 11:30 Jean Delvare said the following:

Quoted 13 lines

Hi Andriy,

Quoted 10 lines · Andriy Gapon

on 17/04/2012 14:18 Jean Delvare said the following:

Quoted 2 lines

I am attaching two patches, one for DDR2, one for DDR3, please give them a try.

Thanks a lot!
I am able to test only the DDR2 part at the moment.
With the patch I am getting the following
Module Configuration Type Data ECC
for these modules: http://www.valueram.com/datasheets/kvr800d2e5_2g.pdf

Quoted 4 lines
Quoted 2 lines

For some definitely non-ECC modules I get the expected
Module Configuration Type No Parity

Thanks for the report, I've applied both patches.

Thank you!

Quoted 11 lines
Quoted 4 lines

P.S.
I've noticed that the "Decoding EEPROM" message is printed with $dimm[$current]->{eeprom} when $opt_side_by_side is true but it is printed with $dimm[$current]->{file} in the other case. Not sure if this is on purpose.

I think it is on purpose. ->{file} contains the full path so it is rather long, ->{eeprom} only contains the file name so it is much shorter. The full path would make side-by-side output very wide, that wouldn't be convenient, that's why the short name is used in this output mode. In legacy output mode, the long name fits.

Quoted 6 lines

We _could_ use the short name in legacy output mode, as the long name doesn't necessarily add a lot of value. I think I tried to leave the legacy output unchanged when implementing the side-by-side output, to avoid any regression in case someone was parsing the output of decode-dimms. If you believe it would be better to always use the short name, we could do that, this may even allow for a small code clean-up.

I don't have any strong opinion about this. I see pros and contras for both choices and I can not make up my mind :)

Signature · 1 line
Andriy Gapon