linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/6] Post EDAC core fix patches
@ 2012-03-07 12:11 Mauro Carvalho Chehab
  2012-03-07 12:11 ` [PATCH 1/6] edac: Initialize the dimm label with the known information Mauro Carvalho Chehab
                   ` (5 more replies)
  0 siblings, 6 replies; 7+ messages in thread
From: Mauro Carvalho Chehab @ 2012-03-07 12:11 UTC (permalink / raw)
  Cc: Mauro Carvalho Chehab, Linux Edac Mailing List,
	Linux Kernel Mailing List

Addresses some other stuff that requires cleaning, fixes or improvements after 
applying the proper support for FB-DIMM and for the modern Intel DDR2/DDR3 
memory controllers changeset.

Mauro Carvalho Chehab (6):
  edac: Initialize the dimm label with the known information
  edac_mc_sysfs: don't create inactive errcount sysfs nodes
  i5000_edac: Fix the logic that retrieves memory information
  edac: add a sysfs node that stores the max possible memory location
  edac: Call the sysfs nodes as "rank" instead of "dimm" if chip select
    is used
  i5400_edac: improve debug messages to better represent the filled
    memory

 drivers/edac/edac_mc.c       |   21 +++++-
 drivers/edac/edac_mc_sysfs.c |  147 +++++++++++++++++++++++++++++------------
 drivers/edac/i5000_edac.c    |  150 ++++++++++++++++++++++--------------------
 drivers/edac/i5400_edac.c    |   15 ++++-
 4 files changed, 216 insertions(+), 117 deletions(-)

-- 
1.7.8


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 1/6] edac: Initialize the dimm label with the known information
  2012-03-07 12:11 [PATCH 0/6] Post EDAC core fix patches Mauro Carvalho Chehab
@ 2012-03-07 12:11 ` Mauro Carvalho Chehab
  2012-03-07 12:11 ` [PATCH 2/6] edac_mc_sysfs: don't create inactive errcount sysfs nodes Mauro Carvalho Chehab
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Mauro Carvalho Chehab @ 2012-03-07 12:11 UTC (permalink / raw)
  Cc: Mauro Carvalho Chehab, Linux Edac Mailing List,
	Linux Kernel Mailing List

While userspace doesn't fill the dimm labels, add there the dimm location,
as described by the used memory model. This could eventually match what
is described at the dmidecode, making easier for people to identify the
memory.

For example, on an Intel motherboard, the memory is described as:

Memory Device
	Array Handle: 0x0029
	Error Information Handle: Not Provided
	Total Width: 64 bits
	Data Width: 64 bits
	Size: 2048 MB
	Form Factor: DIMM
	Set: 1
	Locator: A1_DIMM0
	Bank Locator: A1_Node0_Channel0_Dimm0
	Type: <OUT OF SPEC>
	Type Detail: Synchronous
	Speed: 800 MHz
	Manufacturer: A1_Manufacturer0
	Serial Number: A1_SerNum0
	Asset Tag: A1_AssetTagNum0
	Part Number: A1_PartNum0

After this patch, the memory label will be filled with:
	/sys/devices/system/edac/mc/mc0/dimm0/dimm_label:mc#0channel#0slot#0

With somewhat matches what it is at the Bank Locator DMI information.
So, it is easier to associate the dimm labels, of course assuming that
the DMI has the Bank Locator filled, and the BIOS doesn't have any bugs.

Yet, even without it, several motherboards are provided with enough
info to map from channel/slot (or branch/channel/slot) into the DIMM
label. So, letting the EDAC core fill it, by default is a good thing.

It should noticed that, as the label filling happens at the
edac_mc_alloc(), drivers can override it to better describe the memories
(and some actually do it).

Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com>
---
 drivers/edac/edac_mc.c |   21 +++++++++++++++++----
 1 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/drivers/edac/edac_mc.c b/drivers/edac/edac_mc.c
index 4ff8d28..1651e7e 100644
--- a/drivers/edac/edac_mc.c
+++ b/drivers/edac/edac_mc.c
@@ -205,10 +205,10 @@ struct mem_ctl_info *edac_mc_alloc(unsigned edac_index,
 	struct errcount_attribute_data *ercd;
 	struct dimm_info *dimm;
 	u32 *ce_per_layer[EDAC_MAX_LAYERS], *ue_per_layer[EDAC_MAX_LAYERS];
-	void *pvt;
+	void *pvt, *p;
 	unsigned size, tot_dimms, count, pos[EDAC_MAX_LAYERS];
 	unsigned tot_csrows, tot_cschannels, tot_errcount = 0;
-	int i, j;
+	int i, j, n, len;
 	int err;
 	int row, chn;
 
@@ -324,9 +324,22 @@ struct mem_ctl_info *edac_mc_alloc(unsigned edac_index,
 			i, (dimm - mci->dimms),
 			pos[0], pos[1], pos[2], row, chn);
 
-		/* Copy DIMM location */
-		for (j = 0; j < n_layers; j++)
+		/*
+		 * Copy DIMM location and initialize the memory location
+		 */
+		len = sizeof(dimm->label);
+		p = dimm->label;
+		n = snprintf(p, len, "mc#%u", edac_index);
+		p += n;
+		len -= n;
+		for (j = 0; j < n_layers; j++) {
+			n = snprintf(p, len, "%s#%u",
+				     edac_layer_name[layers[j].type],
+				     pos[j]);
+			p += n;
+			len -= n;
 			dimm->location[j] = pos[j];
+		}
 
 		/* Link it to the csrows old API data */
 		chan->dimm = dimm;
-- 
1.7.8


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 2/6] edac_mc_sysfs: don't create inactive errcount sysfs nodes
  2012-03-07 12:11 [PATCH 0/6] Post EDAC core fix patches Mauro Carvalho Chehab
  2012-03-07 12:11 ` [PATCH 1/6] edac: Initialize the dimm label with the known information Mauro Carvalho Chehab
@ 2012-03-07 12:11 ` Mauro Carvalho Chehab
  2012-03-07 12:11 ` [PATCH 3/6] i5000_edac: Fix the logic that retrieves memory information Mauro Carvalho Chehab
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Mauro Carvalho Chehab @ 2012-03-07 12:11 UTC (permalink / raw)
  Cc: Mauro Carvalho Chehab, Linux Edac Mailing List,
	Linux Kernel Mailing List

It doesn't make any sense to create an error count node for
something that is not active. So, disable the error counters
where all DIMMs below it, on the hierarchy aren't filled.

Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com>
---
 drivers/edac/edac_mc_sysfs.c |  110 +++++++++++++++++++++++++++--------------
 1 files changed, 72 insertions(+), 38 deletions(-)

diff --git a/drivers/edac/edac_mc_sysfs.c b/drivers/edac/edac_mc_sysfs.c
index f538f9e..116934c 100644
--- a/drivers/edac/edac_mc_sysfs.c
+++ b/drivers/edac/edac_mc_sysfs.c
@@ -892,6 +892,28 @@ static ssize_t errcount_ue_show(struct mem_ctl_info *mci, char *data,
 		       mci->ue_per_layer[ead->n_layers - 1][index]);
 }
 
+static bool is_dimms_filled(struct mem_ctl_info *mci, int n_layers,
+			  int pos[EDAC_MAX_LAYERS])
+{
+	static struct dimm_info *dimm;
+	int i, count = 1;
+
+	dimm = GET_POS(mci->layers, mci->dimms, mci->n_layers,
+		       pos[0], pos[1], pos[2]);
+	for (i = n_layers + 1; i < mci->n_layers; i++)
+		count *= mci->layers[i].size;
+
+	debugf2("%s: layers: %d, pos: %d:%d:%d, count = %d\n",
+		__func__, n_layers, pos[0], pos[1], pos[2], count);
+	for (i = 0; i < count; i++) {
+		if (dimm->nr_pages)
+			return true;
+		dimm++;
+	}
+
+	return false;
+}
+
 static int edac_create_errcount_layer(struct mem_ctl_info *mci,
 				      struct mcidev_sysfs_attribute **erc,
 				      struct errcount_attribute_data **ercd,
@@ -903,52 +925,64 @@ static int edac_create_errcount_layer(struct mem_ctl_info *mci,
 
 	memset(&pos, 0, sizeof(pos));
 	for (i = 0; i < count; i++) {
-		p = location;
-		for (j = 0; j <= layer; j++)
-			p += sprintf(p, "_%s%d",
-				     edac_layer_name[mci->layers[j].type],
-				     pos[j]);
-
-		(*erc)->attr.name = kasprintf(GFP_KERNEL, "ce%s", location);
-		debugf4("%s() creating %s\n", __func__, (*erc)->attr.name);
-		if (!(*erc)->attr.name)
-			return -ENOMEM;
-		(*erc)->attr.mode = S_IRUGO | S_IWUSR;
-		(*erc)->show = errcount_ce_show;
-		(*erc)->priv = *ercd;
-		(*ercd)->n_layers = layer + 1;
-		memcpy((*ercd)->pos, pos, sizeof(pos));
-		err = sysfs_create_file(&mci->edac_mci_kobj, &(*erc)->attr);
-		if (err < 0) {
-			printk(KERN_ERR "sysfs_create_file failed: %d\n", err);
-			return err;
-		}
-		(*erc)++;
-		(*ercd)++;
-
-		(*erc)->attr.name = kasprintf(GFP_KERNEL, "ue%s", location);
-		debugf4("%s() creating %s\n", __func__, (*erc)->attr.name);
-		if (!(*erc)->attr.name)
-			return -ENOMEM;
-		(*erc)->attr.mode = S_IRUGO | S_IWUSR;
-		(*erc)->show = errcount_ue_show;
-		(*erc)->priv = *ercd;
-		(*ercd)->n_layers = layer + 1;
-		memcpy((*ercd)->pos, pos, sizeof(pos));
-		err = sysfs_create_file(&mci->edac_mci_kobj, &(*erc)->attr);
-		if (err < 0) {
-			printk(KERN_ERR "sysfs_create_file failed: %d\n", err);
-			return err;
+		/* Only show the nodes if is there any filled DIMM */
+		if (is_dimms_filled(mci, layer, pos)) {
+			p = location;
+			for (j = 0; j <= layer; j++)
+				p += sprintf(p, "_%s%d",
+					edac_layer_name[mci->layers[j].type],
+					pos[j]);
+
+			(*erc)->attr.name = kasprintf(GFP_KERNEL, "ce%s",
+						      location);
+			debugf2("%s() creating %s\n", __func__,
+				(*erc)->attr.name);
+			if (!(*erc)->attr.name)
+				return -ENOMEM;
+			(*erc)->attr.mode = S_IRUGO | S_IWUSR;
+			(*erc)->show = errcount_ce_show;
+			(*erc)->priv = *ercd;
+			(*ercd)->n_layers = layer + 1;
+			memcpy((*ercd)->pos, pos, sizeof(pos));
+			err = sysfs_create_file(&mci->edac_mci_kobj,
+						&(*erc)->attr);
+			if (err < 0) {
+				printk(KERN_ERR
+				       "sysfs_create_file failed: %d\n", err);
+				return err;
+			}
+			(*erc)++;
+			(*ercd)++;
+
+			(*erc)->attr.name = kasprintf(GFP_KERNEL, "ue%s",
+						      location);
+			debugf2("%s() creating %s\n", __func__,
+				(*erc)->attr.name);
+			if (!(*erc)->attr.name)
+				return -ENOMEM;
+			(*erc)->attr.mode = S_IRUGO | S_IWUSR;
+			(*erc)->show = errcount_ue_show;
+			(*erc)->priv = *ercd;
+			(*ercd)->n_layers = layer + 1;
+			memcpy((*ercd)->pos, pos, sizeof(pos));
+			err = sysfs_create_file(&mci->edac_mci_kobj,
+						&(*erc)->attr);
+			if (err < 0) {
+				printk(KERN_ERR
+				       "sysfs_create_file failed: %d\n", err);
+				return err;
+			}
+			(*erc)++;
+			(*ercd)++;
 		}
 
+		/* increment to the next position */
 		for (j = layer; j >= 0; j--) {
 			pos[j]++;
 			if (pos[j] < mci->layers[j].size)
 				break;
 			pos[j] = 0;
 		}
-		(*erc)++;
-		(*ercd)++;
 	}
 
 	return 0;
-- 
1.7.8


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 3/6] i5000_edac: Fix the logic that retrieves memory information
  2012-03-07 12:11 [PATCH 0/6] Post EDAC core fix patches Mauro Carvalho Chehab
  2012-03-07 12:11 ` [PATCH 1/6] edac: Initialize the dimm label with the known information Mauro Carvalho Chehab
  2012-03-07 12:11 ` [PATCH 2/6] edac_mc_sysfs: don't create inactive errcount sysfs nodes Mauro Carvalho Chehab
@ 2012-03-07 12:11 ` Mauro Carvalho Chehab
  2012-03-07 12:11 ` [PATCH 4/6] edac: add a sysfs node that stores the max possible memory location Mauro Carvalho Chehab
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Mauro Carvalho Chehab @ 2012-03-07 12:11 UTC (permalink / raw)
  Cc: Mauro Carvalho Chehab, Linux Edac Mailing List,
	Linux Kernel Mailing List

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)

Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com>
---
 drivers/edac/i5000_edac.c |  150 ++++++++++++++++++++++++---------------------
 1 files changed, 79 insertions(+), 71 deletions(-)

diff --git a/drivers/edac/i5000_edac.c b/drivers/edac/i5000_edac.c
index 564fe09..14efb74 100644
--- a/drivers/edac/i5000_edac.c
+++ b/drivers/edac/i5000_edac.c
@@ -270,7 +270,8 @@
 #define MTR3		0x8C
 
 #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
  *	MTRx - Memory Technology Registers
@@ -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
  */
-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;
 
 	if (channel < CHANNELS_PER_BRANCH)
-		mtr = pvt->b0_mtr[csrow >> 1];
+		mtr = pvt->b0_mtr[slot];
 	else
-		mtr = pvt->b1_mtr[csrow >> 1];
+		mtr = pvt->b1_mtr[slot];
 
 	return 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)]);
 }
 
-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)
 {
 	int mtr;
 	int amb_present_reg;
 	int addrBits;
 
-	mtr = determine_mtr(pvt, csrow, channel);
+	mtr = determine_mtr(pvt, slot, channel);
 	if (MTR_DIMMS_PRESENT(mtr)) {
 		amb_present_reg = determine_amb_present_reg(pvt, channel);
 
-		/* Determine if there is  a  DIMM present in this DIMM slot */
-		if (amb_present_reg & (1 << (csrow >> 1))) {
+		/* Determine if there is a DIMM present in this DIMM slot */
+		if (amb_present_reg) {
 			dinfo->dual_rank = MTR_DIMM_RANK(mtr);
 
-			if (!((dinfo->dual_rank == 0) &&
-				((csrow & 0x1) == 0x1))) {
-				/* Start with the number of bits for a Bank
-				 * on the DRAM */
-				addrBits = MTR_DRAM_BANKS_ADDR_BITS(mtr);
-				/* Add thenumber of ROW bits */
-				addrBits += MTR_DIMM_ROWS_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 -= 3;	/* 8 bits per bytes */
-
-				dinfo->megabytes = 1 << addrBits;
-			}
+			/* Start with the number of bits for a Bank
+				* on the DRAM */
+			addrBits = MTR_DRAM_BANKS_ADDR_BITS(mtr);
+			/* Add the number of ROW bits */
+			addrBits += MTR_DIMM_ROWS_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 -= 3;	/* 8 bits per bytes */
+
+			dinfo->megabytes = 1 << addrBits;
 		}
 	}
 }
@@ -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)
 {
 	struct i5000_dimm_info *dinfo;
-	int csrow, max_csrows;
+	int slot, channel, branch;
 	char *p, *mem_buffer;
 	int space, n;
-	int channel;
 
 	/* ================= Generate some debug output ================= */
 	space = PAGE_SIZE;
@@ -1052,22 +1049,17 @@ static void calculate_dimm_size(struct i5000_pvt *pvt)
 		return;
 	}
 
-	n = snprintf(p, space, "\n");
-	p += n;
-	space -= n;
-
-	/* Scan all the actual CSROWS (which is # of DIMMS * 2)
+	/* Scan all the actual slots
 	 * and calculate the information for each DIMM
-	 * Start with the highest csrow first, to display it first
-	 * and work toward the 0th csrow
+	 * Start with the highest slot first, to display it first
+	 * and work toward the 0th slot
 	 */
-	max_csrows = pvt->maxdimmperch * 2;
-	for (csrow = max_csrows - 1; csrow >= 0; csrow--) {
+	for (slot = pvt->maxdimmperch - 1; slot >= 0; slot--) {
 
-		/* on an odd csrow, first output a 'boundary' marker,
+		/* on an odd slot, first output a 'boundary' marker,
 		 * then reset the message buffer  */
-		if (csrow & 0x1) {
-			n = snprintf(p, space, "---------------------------"
+		if (slot & 0x1) {
+			n = snprintf(p, space, "--------------------------"
 				"--------------------------------");
 			p += n;
 			space -= n;
@@ -1075,30 +1067,39 @@ static void calculate_dimm_size(struct i5000_pvt *pvt)
 			p = mem_buffer;
 			space = PAGE_SIZE;
 		}
-		n = snprintf(p, space, "csrow %2d    ", csrow);
+		n = snprintf(p, space, "slot %2d    ", slot);
 		p += n;
 		space -= n;
 
 		for (channel = 0; channel < pvt->maxch; channel++) {
-			dinfo = &pvt->dimm_info[csrow][channel];
-			handle_channel(pvt, csrow, channel, dinfo);
-			n = snprintf(p, space, "%4d MB   | ", dinfo->megabytes);
+			dinfo = &pvt->dimm_info[slot][channel];
+			handle_channel(pvt, slot, channel, dinfo);
+			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;
 			space -= n;
 		}
-		n = snprintf(p, space, "\n");
 		p += n;
 		space -= n;
+		debugf2("%s\n", mem_buffer);
+		p = mem_buffer;
+		space = PAGE_SIZE;
 	}
 
 	/* Output the last bottom 'boundary' marker */
-	n = snprintf(p, space, "---------------------------"
-		"--------------------------------\n");
+	n = snprintf(p, space, "--------------------------"
+		"--------------------------------");
 	p += n;
 	space -= n;
+	debugf2("%s\n", mem_buffer);
+	p = mem_buffer;
+	space = PAGE_SIZE;
 
 	/* now output the 'channel' labels */
-	n = snprintf(p, space, "            ");
+	n = snprintf(p, space, "           ");
 	p += n;
 	space -= n;
 	for (channel = 0; channel < pvt->maxch; channel++) {
@@ -1106,9 +1107,17 @@ static void calculate_dimm_size(struct i5000_pvt *pvt)
 		p += 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;
-	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 */
 	debugf2("%s\n", mem_buffer);
@@ -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)
 {
 	struct i5000_pvt *pvt;
-	struct csrow_info *p_csrow;
 	struct dimm_info *dimm;
 	int empty, channel_count;
 	int max_csrows;
-	int mtr, mtr1;
+	int mtr;
 	int csrow_megs;
 	int channel;
-	int csrow;
+	int slot;
 
 	pvt = mci->pvt_info;
 
@@ -1258,26 +1266,25 @@ static int i5000_init_csrows(struct mem_ctl_info *mci)
 	empty = 1;		/* Assume NO memory */
 
 	/*
-	 * TODO: it would be better to not use csrow here, filling
-	 * directly the dimm_info structs, based on branch, channel, dim number
+	 * FIXME: The memory layout used to map slot/channel into the
+	 * 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++) {
-		p_csrow = &mci->csrows[csrow];
+	for (slot = 0; slot < max_csrows; slot++) {
+		for (channel = 0; channel < pvt->maxch; channel++) {
 
-		p_csrow->csrow_idx = csrow;
+			mtr = determine_mtr(pvt, slot, channel);
 
-		/* use branch 0 for the basis */
-		mtr = pvt->b0_mtr[csrow >> 1];
-		mtr1 = pvt->b1_mtr[csrow >> 1];
+			if (!MTR_DIMMS_PRESENT(mtr))
+				continue;
 
-		/* if no DIMMS on this row, continue */
-		if (!MTR_DIMMS_PRESENT(mtr) && !MTR_DIMMS_PRESENT(mtr1))
-			continue;
+			dimm = GET_POS(mci->layers, mci->dimms, mci->n_layers,
+				       channel / MAX_BRANCHES,
+				       channel % MAX_BRANCHES, slot);
 
-		csrow_megs = 0;
-		for (channel = 0; channel < pvt->maxch; channel++) {
-			dimm = p_csrow->channels[channel].dimm;
-			csrow_megs += pvt->dimm_info[csrow][channel].megabytes;
+			csrow_megs = pvt->dimm_info[slot][channel].megabytes;
 			dimm->grain = 8;
 
 			/* Assume DDR2 for now */
@@ -1290,7 +1297,7 @@ static int i5000_init_csrows(struct mem_ctl_info *mci)
 				dimm->dtype = DEV_X4;
 
 			dimm->edac_mode = EDAC_S8ECD8ED;
-			dimm->nr_pages = (csrow_megs << 8) / pvt->maxch;
+			dimm->nr_pages = csrow_megs << 8;
 		}
 
 		empty = 0;
@@ -1337,7 +1344,7 @@ static void i5000_get_dimm_and_channel_counts(struct pci_dev *pdev,
 	 * supported on this memory controller
 	 */
 	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);
 	*num_channels = (int)value;
@@ -1387,11 +1394,12 @@ static int i5000_probe1(struct pci_dev *pdev, int dev_idx)
 		__func__, num_channels, num_dimms_per_channel);
 
 	/* allocate a new MC control structure */
+
 	layers[0].type = EDAC_MC_LAYER_BRANCH;
-	layers[0].size = 2;
-	layers[0].is_csrow = true;
+	layers[0].size = MAX_BRANCHES;
+	layers[0].is_csrow = false;
 	layers[1].type = EDAC_MC_LAYER_CHANNEL;
-	layers[1].size = num_channels;
+	layers[1].size = num_channels / MAX_BRANCHES;
 	layers[1].is_csrow = false;
 	layers[2].type = EDAC_MC_LAYER_SLOT;
 	layers[2].size = num_dimms_per_channel;
-- 
1.7.8


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 4/6] edac: add a sysfs node that stores the max possible memory location
  2012-03-07 12:11 [PATCH 0/6] Post EDAC core fix patches Mauro Carvalho Chehab
                   ` (2 preceding siblings ...)
  2012-03-07 12:11 ` [PATCH 3/6] i5000_edac: Fix the logic that retrieves memory information Mauro Carvalho Chehab
@ 2012-03-07 12:11 ` Mauro Carvalho Chehab
  2012-03-07 12:11 ` [PATCH 5/6] edac: Call the sysfs nodes as "rank" instead of "dimm" if chip select is used Mauro Carvalho Chehab
  2012-03-07 12:11 ` [PATCH 6/6] i5400_edac: improve debug messages to better represent the filled memory Mauro Carvalho Chehab
  5 siblings, 0 replies; 7+ messages in thread
From: Mauro Carvalho Chehab @ 2012-03-07 12:11 UTC (permalink / raw)
  Cc: Mauro Carvalho Chehab, Linux Edac Mailing List,
	Linux Kernel Mailing List

This is useful for edac utilities that want to print the memory layout,
including the empty slots.

Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com>
---
 drivers/edac/edac_mc_sysfs.c |   17 +++++++++++++++++
 1 files changed, 17 insertions(+), 0 deletions(-)

diff --git a/drivers/edac/edac_mc_sysfs.c b/drivers/edac/edac_mc_sysfs.c
index 116934c..5b48528 100644
--- a/drivers/edac/edac_mc_sysfs.c
+++ b/drivers/edac/edac_mc_sysfs.c
@@ -744,6 +744,21 @@ static ssize_t mci_size_mb_show(struct mem_ctl_info *mci, char *data,
 	return sprintf(data, "%u\n", PAGES_TO_MiB(total_pages));
 }
 
+static ssize_t mci_max_location_show(struct mem_ctl_info *mci, char *data,
+				void *priv)
+{
+	int i;
+	char *p = data;
+
+	for (i = 0; i < mci->n_layers; i++) {
+		p += sprintf(p, "%s %d ",
+			     edac_layer_name[mci->layers[i].type],
+			     mci->layers[i].size - 1);
+	}
+
+	return p - data;
+}
+
 #ifdef CONFIG_EDAC_DEBUG
 static ssize_t edac_fake_inject_show(struct mem_ctl_info *mci,
 				     char *data, void *priv)
@@ -839,6 +854,7 @@ MCIDEV_ATTR(ue_noinfo_count, S_IRUGO, mci_ue_noinfo_show, NULL);
 MCIDEV_ATTR(ce_noinfo_count, S_IRUGO, mci_ce_noinfo_show, NULL);
 MCIDEV_ATTR(ue_count, S_IRUGO, mci_ue_count_show, NULL);
 MCIDEV_ATTR(ce_count, S_IRUGO, mci_ce_count_show, NULL);
+MCIDEV_ATTR(max_location, S_IRUGO, mci_max_location_show, NULL);
 
 /* memory scrubber attribute file */
 MCIDEV_ATTR(sdram_scrub_rate, S_IRUGO | S_IWUSR, mci_sdram_scrub_rate_show,
@@ -854,6 +870,7 @@ static struct mcidev_sysfs_attribute *mci_attr[] = {
 	&mci_attr_ue_count,
 	&mci_attr_ce_count,
 	&mci_attr_sdram_scrub_rate,
+	&mci_attr_max_location,
 	NULL
 };
 
-- 
1.7.8


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 5/6] edac: Call the sysfs nodes as "rank" instead of "dimm" if chip select is used
  2012-03-07 12:11 [PATCH 0/6] Post EDAC core fix patches Mauro Carvalho Chehab
                   ` (3 preceding siblings ...)
  2012-03-07 12:11 ` [PATCH 4/6] edac: add a sysfs node that stores the max possible memory location Mauro Carvalho Chehab
@ 2012-03-07 12:11 ` Mauro Carvalho Chehab
  2012-03-07 12:11 ` [PATCH 6/6] i5400_edac: improve debug messages to better represent the filled memory Mauro Carvalho Chehab
  5 siblings, 0 replies; 7+ messages in thread
From: Mauro Carvalho Chehab @ 2012-03-07 12:11 UTC (permalink / raw)
  Cc: Mauro Carvalho Chehab, Linux Edac Mailing List,
	Linux Kernel Mailing List

The minimum hierarchy unit for csrows-based MC's is rank, and not dimm.

There are a few possible fixes here:

1) add a per-rank location at the dimm struct, and don't create one
   dimm_info entry per rank;

2) Convert the drivers to internally convert ranks into dimm's;

3) rename "dimm" with "rank";

A few drivers do (2) internally: some of them have per-dimm registers,
instead of per-rank ones; a few others have some routines to translate
between the two representations of the memory. Those drivers report the
memories via a DIMM slot and a channel.

While no better solution is done, let's do (2) for the drivers that work
with csrows/channel layers to represent a rank.

Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com>
---
 drivers/edac/edac_mc_sysfs.c |   20 +++++++++++++++++---
 1 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/drivers/edac/edac_mc_sysfs.c b/drivers/edac/edac_mc_sysfs.c
index 5b48528..dd934c2 100644
--- a/drivers/edac/edac_mc_sysfs.c
+++ b/drivers/edac/edac_mc_sysfs.c
@@ -567,10 +567,12 @@ static struct kobj_type ktype_dimm = {
 };
 /* Create a CSROW object under specifed edac_mc_device */
 static int edac_create_dimm_object(struct mem_ctl_info *mci,
-					struct dimm_info *dimm, int index)
+					struct dimm_info *dimm, int index,
+					bool is_rank)
 {
 	struct kobject *kobj_mci = &mci->edac_mci_kobj;
 	struct kobject *kobj;
+	const char *nodename;
 	int err;
 
 	/* generate ..../edac/mc/mc<id>/dimm<index>   */
@@ -585,8 +587,12 @@ static int edac_create_dimm_object(struct mem_ctl_info *mci,
 	}
 
 	/* Instanstiate the dimm object */
+	if (!is_rank)
+		nodename = "dimm%d";
+	else
+		nodename = "rank%d";
 	err = kobject_init_and_add(&dimm->kobj, &ktype_dimm, kobj_mci,
-				   "dimm%d", index);
+				nodename, index);
 	if (err)
 		goto err_release_top_kobj;
 
@@ -1338,6 +1344,7 @@ int edac_create_sysfs_mci_device(struct mem_ctl_info *mci)
 	int err;
 	struct csrow_info *csrow;
 	struct kobject *kobj_mci = &mci->edac_mci_kobj;
+	bool is_rank = false;
 
 	debugf0("%s() idx=%d\n", __func__, mci->mc_idx);
 
@@ -1386,6 +1393,13 @@ int edac_create_sysfs_mci_device(struct mem_ctl_info *mci)
 		}
 	}
 
+	for (i = 0; i < mci->n_layers; i++) {
+		if (mci->layers[i].type == EDAC_MC_LAYER_CHIP_SELECT) {
+			is_rank = true;
+			break;
+		}
+	}
+
 	/*
 	 * Make directories for each DIMM object under the mc<id> kobject
 	 */
@@ -1406,7 +1420,7 @@ int edac_create_sysfs_mci_device(struct mem_ctl_info *mci)
 			printk(KERN_CONT "\n");
 		}
 #endif
-		err = edac_create_dimm_object(mci, dimm, j);
+		err = edac_create_dimm_object(mci, dimm, j, is_rank);
 		if (err) {
 			debugf1("%s() failure: create dimm %d obj\n",
 				__func__, j);
-- 
1.7.8


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 6/6] i5400_edac: improve debug messages to better represent the filled memory
  2012-03-07 12:11 [PATCH 0/6] Post EDAC core fix patches Mauro Carvalho Chehab
                   ` (4 preceding siblings ...)
  2012-03-07 12:11 ` [PATCH 5/6] edac: Call the sysfs nodes as "rank" instead of "dimm" if chip select is used Mauro Carvalho Chehab
@ 2012-03-07 12:11 ` Mauro Carvalho Chehab
  5 siblings, 0 replies; 7+ messages in thread
From: Mauro Carvalho Chehab @ 2012-03-07 12:11 UTC (permalink / raw)
  Cc: Mauro Carvalho Chehab, Linux Edac Mailing List,
	Linux Kernel Mailing List

Improves the debug output message, in order to better represent the
memory controller hierarchy, when outputing the debug messages.

No functional changes when debug is disabled.

Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com>
---
 drivers/edac/i5400_edac.c |   15 ++++++++++++++-
 1 files changed, 14 insertions(+), 1 deletions(-)

diff --git a/drivers/edac/i5400_edac.c b/drivers/edac/i5400_edac.c
index 3aa2a1e..5debda9 100644
--- a/drivers/edac/i5400_edac.c
+++ b/drivers/edac/i5400_edac.c
@@ -963,7 +963,7 @@ static void calculate_dimm_size(struct i5400_pvt *pvt)
 	int dimm, max_dimms;
 	char *p, *mem_buffer;
 	int space, n;
-	int channel;
+	int channel, branch;
 
 	/* ================= Generate some debug output ================= */
 	space = PAGE_SIZE;
@@ -1028,6 +1028,19 @@ static void calculate_dimm_size(struct i5400_pvt *pvt)
 		space -= n;
 	}
 
+	space -= n;
+	debugf2("%s\n", mem_buffer);
+	p = mem_buffer;
+	space = PAGE_SIZE;
+
+	n = snprintf(p, space, "           ");
+	p += 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 */
 	debugf2("%s\n", mem_buffer);
 	kfree(mem_buffer);
-- 
1.7.8


^ permalink raw reply related	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2012-03-07 12:13 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-03-07 12:11 [PATCH 0/6] Post EDAC core fix patches Mauro Carvalho Chehab
2012-03-07 12:11 ` [PATCH 1/6] edac: Initialize the dimm label with the known information Mauro Carvalho Chehab
2012-03-07 12:11 ` [PATCH 2/6] edac_mc_sysfs: don't create inactive errcount sysfs nodes Mauro Carvalho Chehab
2012-03-07 12:11 ` [PATCH 3/6] i5000_edac: Fix the logic that retrieves memory information Mauro Carvalho Chehab
2012-03-07 12:11 ` [PATCH 4/6] edac: add a sysfs node that stores the max possible memory location Mauro Carvalho Chehab
2012-03-07 12:11 ` [PATCH 5/6] edac: Call the sysfs nodes as "rank" instead of "dimm" if chip select is used Mauro Carvalho Chehab
2012-03-07 12:11 ` [PATCH 6/6] i5400_edac: improve debug messages to better represent the filled memory Mauro Carvalho Chehab

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).