Commit 64e1fdaf authored by Mauro Carvalho Chehab's avatar Mauro Carvalho Chehab

i5000_edac: Fix the logic that retrieves memory information

The logic there is broken: it basically creates two csrows for
each DIMM and assumes that all DIMM's are dual rank. Only one of
the csrows will contain the entire DIMM size. If single rank
memories are found, they'll be marked with 0 bytes.

The check if the AMB is present were also wrong.

Yet, as the error reports don't use the memory size in order to
credit an error to the right DIMM, that part of the driver seems
to work. That's why probably nobody detected the issue yet.

After this patch, the memory layout is now properly reported,
when debug mode is enabled, and the number of ranks per dimm is
now shown:

calculate_dimm_size: ----------------------------------------------------------
calculate_dimm_size: slot  3       0 MB   |    0 MB   |    0 MB   |    0 MB   |
calculate_dimm_size: slot  2       0 MB   |    0 MB   |    0 MB   |    0 MB   |
calculate_dimm_size: ----------------------------------------------------------
calculate_dimm_size: slot  1       0 MB   |    0 MB   |    0 MB   |    0 MB   |
calculate_dimm_size: slot  0     512 MB 1R|  512 MB 1R|  512 MB 1R|  512 MB 1R|
calculate_dimm_size: ----------------------------------------------------------
calculate_dimm_size:            channel 0 | channel 1 | channel 2 | channel 3 |
calculate_dimm_size:                   branch 0       |        branch 1       |

(1R above means that all memories on my test machine are single-ranked)
Reviewed-by: default avatarAristeu Rozanski <arozansk@redhat.com>
Cc: Doug Thompson <norsk5@yahoo.com>
Signed-off-by: default avatarMauro Carvalho Chehab <mchehab@redhat.com>
parent 68d086f8
...@@ -270,7 +270,8 @@ ...@@ -270,7 +270,8 @@
#define MTR3 0x8C #define MTR3 0x8C
#define NUM_MTRS 4 #define NUM_MTRS 4
#define CHANNELS_PER_BRANCH (2) #define CHANNELS_PER_BRANCH 2
#define MAX_BRANCHES 2
/* Defines to extract the vaious fields from the /* Defines to extract the vaious fields from the
* MTRx - Memory Technology Registers * MTRx - Memory Technology Registers
...@@ -962,14 +963,14 @@ static int determine_amb_present_reg(struct i5000_pvt *pvt, int channel) ...@@ -962,14 +963,14 @@ static int determine_amb_present_reg(struct i5000_pvt *pvt, int channel)
* *
* return the proper MTR register as determine by the csrow and channel desired * return the proper MTR register as determine by the csrow and channel desired
*/ */
static int determine_mtr(struct i5000_pvt *pvt, int csrow, int channel) static int determine_mtr(struct i5000_pvt *pvt, int slot, int channel)
{ {
int mtr; int mtr;
if (channel < CHANNELS_PER_BRANCH) if (channel < CHANNELS_PER_BRANCH)
mtr = pvt->b0_mtr[csrow >> 1]; mtr = pvt->b0_mtr[slot];
else else
mtr = pvt->b1_mtr[csrow >> 1]; mtr = pvt->b1_mtr[slot];
return mtr; return mtr;
} }
...@@ -994,37 +995,34 @@ static void decode_mtr(int slot_row, u16 mtr) ...@@ -994,37 +995,34 @@ static void decode_mtr(int slot_row, u16 mtr)
debugf2("\t\tNUMCOL: %s\n", numcol_toString[MTR_DIMM_COLS(mtr)]); debugf2("\t\tNUMCOL: %s\n", numcol_toString[MTR_DIMM_COLS(mtr)]);
} }
static void handle_channel(struct i5000_pvt *pvt, int csrow, int channel, static void handle_channel(struct i5000_pvt *pvt, int slot, int channel,
struct i5000_dimm_info *dinfo) struct i5000_dimm_info *dinfo)
{ {
int mtr; int mtr;
int amb_present_reg; int amb_present_reg;
int addrBits; int addrBits;
mtr = determine_mtr(pvt, csrow, channel); mtr = determine_mtr(pvt, slot, channel);
if (MTR_DIMMS_PRESENT(mtr)) { if (MTR_DIMMS_PRESENT(mtr)) {
amb_present_reg = determine_amb_present_reg(pvt, channel); amb_present_reg = determine_amb_present_reg(pvt, channel);
/* Determine if there is a DIMM present in this DIMM slot */ /* Determine if there is a DIMM present in this DIMM slot */
if (amb_present_reg & (1 << (csrow >> 1))) { if (amb_present_reg) {
dinfo->dual_rank = MTR_DIMM_RANK(mtr); dinfo->dual_rank = MTR_DIMM_RANK(mtr);
if (!((dinfo->dual_rank == 0) && /* Start with the number of bits for a Bank
((csrow & 0x1) == 0x1))) { * on the DRAM */
/* Start with the number of bits for a Bank addrBits = MTR_DRAM_BANKS_ADDR_BITS(mtr);
* on the DRAM */ /* Add the number of ROW bits */
addrBits = MTR_DRAM_BANKS_ADDR_BITS(mtr); addrBits += MTR_DIMM_ROWS_ADDR_BITS(mtr);
/* Add thenumber of ROW bits */ /* add the number of COLUMN bits */
addrBits += MTR_DIMM_ROWS_ADDR_BITS(mtr); addrBits += MTR_DIMM_COLS_ADDR_BITS(mtr);
/* add the number of COLUMN bits */
addrBits += MTR_DIMM_COLS_ADDR_BITS(mtr); addrBits += 6; /* add 64 bits per DIMM */
addrBits -= 20; /* divide by 2^^20 */
addrBits += 6; /* add 64 bits per DIMM */ addrBits -= 3; /* 8 bits per bytes */
addrBits -= 20; /* divide by 2^^20 */
addrBits -= 3; /* 8 bits per bytes */ dinfo->megabytes = 1 << addrBits;
dinfo->megabytes = 1 << addrBits;
}
} }
} }
} }
...@@ -1038,10 +1036,9 @@ static void handle_channel(struct i5000_pvt *pvt, int csrow, int channel, ...@@ -1038,10 +1036,9 @@ static void handle_channel(struct i5000_pvt *pvt, int csrow, int channel,
static void calculate_dimm_size(struct i5000_pvt *pvt) static void calculate_dimm_size(struct i5000_pvt *pvt)
{ {
struct i5000_dimm_info *dinfo; struct i5000_dimm_info *dinfo;
int csrow, max_csrows; int slot, channel, branch;
char *p, *mem_buffer; char *p, *mem_buffer;
int space, n; int space, n;
int channel;
/* ================= Generate some debug output ================= */ /* ================= Generate some debug output ================= */
space = PAGE_SIZE; space = PAGE_SIZE;
...@@ -1052,22 +1049,17 @@ static void calculate_dimm_size(struct i5000_pvt *pvt) ...@@ -1052,22 +1049,17 @@ static void calculate_dimm_size(struct i5000_pvt *pvt)
return; return;
} }
n = snprintf(p, space, "\n"); /* Scan all the actual slots
p += n;
space -= n;
/* Scan all the actual CSROWS (which is # of DIMMS * 2)
* and calculate the information for each DIMM * and calculate the information for each DIMM
* Start with the highest csrow first, to display it first * Start with the highest slot first, to display it first
* and work toward the 0th csrow * and work toward the 0th slot
*/ */
max_csrows = pvt->maxdimmperch * 2; for (slot = pvt->maxdimmperch - 1; slot >= 0; slot--) {
for (csrow = max_csrows - 1; csrow >= 0; csrow--) {
/* on an odd csrow, first output a 'boundary' marker, /* on an odd slot, first output a 'boundary' marker,
* then reset the message buffer */ * then reset the message buffer */
if (csrow & 0x1) { if (slot & 0x1) {
n = snprintf(p, space, "---------------------------" n = snprintf(p, space, "--------------------------"
"--------------------------------"); "--------------------------------");
p += n; p += n;
space -= n; space -= n;
...@@ -1075,30 +1067,39 @@ static void calculate_dimm_size(struct i5000_pvt *pvt) ...@@ -1075,30 +1067,39 @@ static void calculate_dimm_size(struct i5000_pvt *pvt)
p = mem_buffer; p = mem_buffer;
space = PAGE_SIZE; space = PAGE_SIZE;
} }
n = snprintf(p, space, "csrow %2d ", csrow); n = snprintf(p, space, "slot %2d ", slot);
p += n; p += n;
space -= n; space -= n;
for (channel = 0; channel < pvt->maxch; channel++) { for (channel = 0; channel < pvt->maxch; channel++) {
dinfo = &pvt->dimm_info[csrow][channel]; dinfo = &pvt->dimm_info[slot][channel];
handle_channel(pvt, csrow, channel, dinfo); handle_channel(pvt, slot, channel, dinfo);
n = snprintf(p, space, "%4d MB | ", dinfo->megabytes); if (dinfo->megabytes)
n = snprintf(p, space, "%4d MB %dR| ",
dinfo->megabytes, dinfo->dual_rank + 1);
else
n = snprintf(p, space, "%4d MB | ", 0);
p += n; p += n;
space -= n; space -= n;
} }
n = snprintf(p, space, "\n");
p += n; p += n;
space -= n; space -= n;
debugf2("%s\n", mem_buffer);
p = mem_buffer;
space = PAGE_SIZE;
} }
/* Output the last bottom 'boundary' marker */ /* Output the last bottom 'boundary' marker */
n = snprintf(p, space, "---------------------------" n = snprintf(p, space, "--------------------------"
"--------------------------------\n"); "--------------------------------");
p += n; p += n;
space -= n; space -= n;
debugf2("%s\n", mem_buffer);
p = mem_buffer;
space = PAGE_SIZE;
/* now output the 'channel' labels */ /* now output the 'channel' labels */
n = snprintf(p, space, " "); n = snprintf(p, space, " ");
p += n; p += n;
space -= n; space -= n;
for (channel = 0; channel < pvt->maxch; channel++) { for (channel = 0; channel < pvt->maxch; channel++) {
...@@ -1106,9 +1107,17 @@ static void calculate_dimm_size(struct i5000_pvt *pvt) ...@@ -1106,9 +1107,17 @@ static void calculate_dimm_size(struct i5000_pvt *pvt)
p += n; p += n;
space -= n; space -= n;
} }
n = snprintf(p, space, "\n"); debugf2("%s\n", mem_buffer);
p = mem_buffer;
space = PAGE_SIZE;
n = snprintf(p, space, " ");
p += n; p += n;
space -= n; for (branch = 0; branch < MAX_BRANCHES; branch++) {
n = snprintf(p, space, " branch %d | ", branch);
p += n;
space -= n;
}
/* output the last message and free buffer */ /* output the last message and free buffer */
debugf2("%s\n", mem_buffer); debugf2("%s\n", mem_buffer);
...@@ -1241,14 +1250,13 @@ static void i5000_get_mc_regs(struct mem_ctl_info *mci) ...@@ -1241,14 +1250,13 @@ static void i5000_get_mc_regs(struct mem_ctl_info *mci)
static int i5000_init_csrows(struct mem_ctl_info *mci) static int i5000_init_csrows(struct mem_ctl_info *mci)
{ {
struct i5000_pvt *pvt; struct i5000_pvt *pvt;
struct csrow_info *p_csrow;
struct dimm_info *dimm; struct dimm_info *dimm;
int empty, channel_count; int empty, channel_count;
int max_csrows; int max_csrows;
int mtr, mtr1; int mtr;
int csrow_megs; int csrow_megs;
int channel; int channel;
int csrow; int slot;
pvt = mci->pvt_info; pvt = mci->pvt_info;
...@@ -1258,26 +1266,25 @@ static int i5000_init_csrows(struct mem_ctl_info *mci) ...@@ -1258,26 +1266,25 @@ static int i5000_init_csrows(struct mem_ctl_info *mci)
empty = 1; /* Assume NO memory */ empty = 1; /* Assume NO memory */
/* /*
* TODO: it would be better to not use csrow here, filling * FIXME: The memory layout used to map slot/channel into the
* directly the dimm_info structs, based on branch, channel, dim number * real memory architecture is weird: branch+slot are "csrows"
* and channel is channel. That required an extra array (dimm_info)
* to map the dimms. A good cleanup would be to remove this array,
* and do a loop here with branch, channel, slot
*/ */
for (csrow = 0; csrow < max_csrows; csrow++) { for (slot = 0; slot < max_csrows; slot++) {
p_csrow = &mci->csrows[csrow]; for (channel = 0; channel < pvt->maxch; channel++) {
p_csrow->csrow_idx = csrow; mtr = determine_mtr(pvt, slot, channel);
/* use branch 0 for the basis */ if (!MTR_DIMMS_PRESENT(mtr))
mtr = pvt->b0_mtr[csrow >> 1]; continue;
mtr1 = pvt->b1_mtr[csrow >> 1];
/* if no DIMMS on this row, continue */ dimm = EDAC_DIMM_PTR(mci->layers, mci->dimms, mci->n_layers,
if (!MTR_DIMMS_PRESENT(mtr) && !MTR_DIMMS_PRESENT(mtr1)) channel / MAX_BRANCHES,
continue; channel % MAX_BRANCHES, slot);
csrow_megs = 0; csrow_megs = pvt->dimm_info[slot][channel].megabytes;
for (channel = 0; channel < pvt->maxch; channel++) {
dimm = p_csrow->channels[channel].dimm;
csrow_megs += pvt->dimm_info[csrow][channel].megabytes;
dimm->grain = 8; dimm->grain = 8;
/* Assume DDR2 for now */ /* Assume DDR2 for now */
...@@ -1290,7 +1297,7 @@ static int i5000_init_csrows(struct mem_ctl_info *mci) ...@@ -1290,7 +1297,7 @@ static int i5000_init_csrows(struct mem_ctl_info *mci)
dimm->dtype = DEV_X4; dimm->dtype = DEV_X4;
dimm->edac_mode = EDAC_S8ECD8ED; dimm->edac_mode = EDAC_S8ECD8ED;
dimm->nr_pages = (csrow_megs << 8) / pvt->maxch; dimm->nr_pages = csrow_megs << 8;
} }
empty = 0; empty = 0;
...@@ -1337,7 +1344,7 @@ static void i5000_get_dimm_and_channel_counts(struct pci_dev *pdev, ...@@ -1337,7 +1344,7 @@ static void i5000_get_dimm_and_channel_counts(struct pci_dev *pdev,
* supported on this memory controller * supported on this memory controller
*/ */
pci_read_config_byte(pdev, MAXDIMMPERCH, &value); pci_read_config_byte(pdev, MAXDIMMPERCH, &value);
*num_dimms_per_channel = (int)value *2; *num_dimms_per_channel = (int)value;
pci_read_config_byte(pdev, MAXCH, &value); pci_read_config_byte(pdev, MAXCH, &value);
*num_channels = (int)value; *num_channels = (int)value;
...@@ -1387,11 +1394,12 @@ static int i5000_probe1(struct pci_dev *pdev, int dev_idx) ...@@ -1387,11 +1394,12 @@ static int i5000_probe1(struct pci_dev *pdev, int dev_idx)
__func__, num_channels, num_dimms_per_channel); __func__, num_channels, num_dimms_per_channel);
/* allocate a new MC control structure */ /* allocate a new MC control structure */
layers[0].type = EDAC_MC_LAYER_BRANCH; layers[0].type = EDAC_MC_LAYER_BRANCH;
layers[0].size = 2; layers[0].size = MAX_BRANCHES;
layers[0].is_virt_csrow = true; layers[0].is_virt_csrow = false;
layers[1].type = EDAC_MC_LAYER_CHANNEL; layers[1].type = EDAC_MC_LAYER_CHANNEL;
layers[1].size = num_channels; layers[1].size = num_channels / MAX_BRANCHES;
layers[1].is_virt_csrow = false; layers[1].is_virt_csrow = false;
layers[2].type = EDAC_MC_LAYER_SLOT; layers[2].type = EDAC_MC_LAYER_SLOT;
layers[2].size = num_dimms_per_channel; layers[2].size = num_dimms_per_channel;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment