[PATCH 2/2] powerpc: MPC85xx EDAC device driver

[PATCH 2/2] powerpc: MPC85xx EDAC device driver

[Dave Jiang]

Freescale MPC85xx SoC support for EDAC. Used on PPC platforms. Development
and testing done on PPC Freescale MPC8548CDS.

The driver provides error reporting for L2 cache error registers, the
memory controller error registers, and the PCI error registers. The error
reporting can be done two ways, via interrupts or polling.

Signed-off-by: Dave Jiang <djiang@mvista.com>
Signed-off-by: Douglas Thompson <dougthompson@xmission.com>

---
commit 8ab17eba61575673d4e6a637a3987253cad9adaa
tree 1280fcac7b336cae61cd1e4a6dc9a82741b766ec
parent 8a80b43ddd3a4f7694df75869e13c3fc6e6c89f6
author Dave Jiang <djiang@mvista.com> Thu, 26 Jul 2007 14:53:20 -0700
committer Dave Jiang <djiang@blade.(none)> Thu, 26 Jul 2007 14:53:20 -0700

 drivers/edac/Kconfig        |    6 
 drivers/edac/Makefile       |    1 
 drivers/edac/mpc85xx_edac.c | 1048 +++++++++++++++++++++++++++++++++++++++++++
 drivers/edac/mpc85xx_edac.h |  162 +++++++
 4 files changed, 1217 insertions(+), 0 deletions(-)

diff --git a/drivers/edac/Kconfig b/drivers/edac/Kconfig
index 1724c41..cc50ab6 100644
--- a/drivers/edac/Kconfig
+++ b/drivers/edac/Kconfig
@@ -131,5 +131,11 @@ config EDAC_PASEMI
 	  Support for error detection and correction on PA Semi
 	  PWRficient.
 
+config EDAC_MPC85XX
+	tristate "Freescale MPC85xx"
+	depends on EDAC_MM_EDAC && FSL_SOC && MPC85xx
+	help
+	  Support for error detection and correction on the Freescale
+	  MPC8560, MPC8540, MPC8548
 
 endif # EDAC
diff --git a/drivers/edac/Makefile b/drivers/edac/Makefile
index 02c09f0..62696aa 100644
--- a/drivers/edac/Makefile
+++ b/drivers/edac/Makefile
@@ -28,4 +28,5 @@ obj-$(CONFIG_EDAC_I3000)		+= i3000_edac.o
 obj-$(CONFIG_EDAC_I82860)		+= i82860_edac.o
 obj-$(CONFIG_EDAC_R82600)		+= r82600_edac.o
 obj-$(CONFIG_EDAC_PASEMI)		+= pasemi_edac.o
+obj-$(CONFIG_EDAC_MPC85XX)		+= mpc85xx_edac.o
 
diff --git a/drivers/edac/mpc85xx_edac.c b/drivers/edac/mpc85xx_edac.c
new file mode 100644
index 0000000..8078d53
--- /dev/null
+++ b/drivers/edac/mpc85xx_edac.c
@@ -0,0 +1,1048 @@
+/*
+ * Freescale MPC85xx Memory Controller kenel module
+ *
+ * Author: Dave Jiang <djiang@mvista.com>
+ *
+ * 2006-2007 (c) MontaVista Software, Inc. This file is licensed under
+ * the terms of the GNU General Public License version 2. This program
+ * is licensed "as is" without any warranty of any kind, whether express
+ * or implied.
+ *
+ */
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/slab.h>
+#include <linux/interrupt.h>
+#include <linux/ctype.h>
+#include <linux/io.h>
+#include <linux/mod_devicetable.h>
+#include <linux/edac.h>
+#include <linux/of_device.h>
+#include <linux/of_platform.h>
+
+#include <asm/mpc85xx.h>
+#include "edac_module.h"
+#include "edac_core.h"
+#include "mpc85xx_edac.h"
+
+static int edac_dev_idx;
+static int edac_pci_idx;
+static int edac_mc_idx;
+
+static u32 orig_ddr_err_disable;
+static u32 orig_ddr_err_sbe;
+
+/*
+ * PCI Err defines
+ */
+#ifdef CONFIG_PCI
+static u32 orig_pci_err_cap_dr;
+static u32 orig_pci_err_en;
+#endif
+
+static u32 orig_l2_err_disable;
+static u32 orig_hid1;
+
+const char *mpc85xx_ctl_name = "MPC85xx";
+
+/************************ MC SYSFS parts ***********************************/
+
+static ssize_t mpc85xx_mc_inject_data_hi_show(struct mem_ctl_info *mci,
+					      char *data)
+{
+	struct mpc85xx_mc_pdata *pdata = mci->pvt_info;
+	return sprintf(data, "0x%08x",
+		       in_be32(pdata->mc_vbase +
+			       MPC85XX_MC_DATA_ERR_INJECT_HI));
+}
+
+static ssize_t mpc85xx_mc_inject_data_lo_show(struct mem_ctl_info *mci,
+					      char *data)
+{
+	struct mpc85xx_mc_pdata *pdata = mci->pvt_info;
+	return sprintf(data, "0x%08x",
+		       in_be32(pdata->mc_vbase +
+			       MPC85XX_MC_DATA_ERR_INJECT_LO));
+}
+
+static ssize_t mpc85xx_mc_inject_ctrl_show(struct mem_ctl_info *mci, char *data)
+{
+	struct mpc85xx_mc_pdata *pdata = mci->pvt_info;
+	return sprintf(data, "0x%08x",
+		       in_be32(pdata->mc_vbase + MPC85XX_MC_ECC_ERR_INJECT));
+}
+
+static ssize_t mpc85xx_mc_inject_data_hi_store(struct mem_ctl_info *mci,
+					       const char *data, size_t count)
+{
+	struct mpc85xx_mc_pdata *pdata = mci->pvt_info;
+	if (isdigit(*data)) {
+		out_be32(pdata->mc_vbase + MPC85XX_MC_DATA_ERR_INJECT_HI,
+			 simple_strtoul(data, NULL, 0));
+		return count;
+	}
+	return 0;
+}
+
+static ssize_t mpc85xx_mc_inject_data_lo_store(struct mem_ctl_info *mci,
+					       const char *data, size_t count)
+{
+	struct mpc85xx_mc_pdata *pdata = mci->pvt_info;
+	if (isdigit(*data)) {
+		out_be32(pdata->mc_vbase + MPC85XX_MC_DATA_ERR_INJECT_LO,
+			 simple_strtoul(data, NULL, 0));
+		return count;
+	}
+	return 0;
+}
+
+static ssize_t mpc85xx_mc_inject_ctrl_store(struct mem_ctl_info *mci,
+					    const char *data, size_t count)
+{
+	struct mpc85xx_mc_pdata *pdata = mci->pvt_info;
+	if (isdigit(*data)) {
+		out_be32(pdata->mc_vbase + MPC85XX_MC_ECC_ERR_INJECT,
+			 simple_strtoul(data, NULL, 0));
+		return count;
+	}
+	return 0;
+}
+
+static struct mcidev_sysfs_attribute mpc85xx_mc_sysfs_attributes[] = {
+	{
+	 .attr = {
+		  .name = "inject_data_hi",
+		  .mode = (S_IRUGO | S_IWUSR)
+		  },
+	 .show = mpc85xx_mc_inject_data_hi_show,
+	 .store = mpc85xx_mc_inject_data_hi_store},
+	{
+	 .attr = {
+		  .name = "inject_data_lo",
+		  .mode = (S_IRUGO | S_IWUSR)
+		  },
+	 .show = mpc85xx_mc_inject_data_lo_show,
+	 .store = mpc85xx_mc_inject_data_lo_store},
+	{
+	 .attr = {
+		  .name = "inject_ctrl",
+		  .mode = (S_IRUGO | S_IWUSR)
+		  },
+	 .show = mpc85xx_mc_inject_ctrl_show,
+	 .store = mpc85xx_mc_inject_ctrl_store},
+
+	/* End of list */
+	{
+	 .attr = {.name = NULL}
+	 }
+};
+
+static void mpc85xx_set_mc_sysfs_attributes(struct mem_ctl_info *mci)
+{
+	mci->mc_driver_sysfs_attributes = mpc85xx_mc_sysfs_attributes;
+}
+
+/**************************** PCI Err device ***************************/
+#ifdef CONFIG_PCI
+
+static void mpc85xx_pci_check(struct edac_pci_ctl_info *pci)
+{
+	struct mpc85xx_pci_pdata *pdata = pci->pvt_info;
+	u32 err_detect;
+
+	err_detect = in_be32(pdata->pci_vbase + MPC85XX_PCI_ERR_DR);
+
+	/* master aborts can happen during PCI config cycles */
+	if (!(err_detect & ~(PCI_EDE_MULTI_ERR | PCI_EDE_MST_ABRT))) {
+		out_be32(pdata->pci_vbase + MPC85XX_PCI_ERR_DR, err_detect);
+		return;
+	}
+
+	printk(KERN_ERR "PCI error(s) detected\n");
+	printk(KERN_ERR "PCI/X ERR_DR register: %#08x\n", err_detect);
+
+	printk(KERN_ERR "PCI/X ERR_ATTRIB register: %#08x\n",
+	       in_be32(pdata->pci_vbase + MPC85XX_PCI_ERR_ATTRIB));
+	printk(KERN_ERR "PCI/X ERR_ADDR register: %#08x\n",
+	       in_be32(pdata->pci_vbase + MPC85XX_PCI_ERR_ADDR));
+	printk(KERN_ERR "PCI/X ERR_EXT_ADDR register: %#08x\n",
+	       in_be32(pdata->pci_vbase + MPC85XX_PCI_ERR_EXT_ADDR));
+	printk(KERN_ERR "PCI/X ERR_DL register: %#08x\n",
+	       in_be32(pdata->pci_vbase + MPC85XX_PCI_ERR_DL));
+	printk(KERN_ERR "PCI/X ERR_DH register: %#08x\n",
+	       in_be32(pdata->pci_vbase + MPC85XX_PCI_ERR_DH));
+
+	/* clear error bits */
+	out_be32(pdata->pci_vbase + MPC85XX_PCI_ERR_DR, err_detect);
+
+	if (err_detect & PCI_EDE_PERR_MASK)
+		edac_pci_handle_pe(pci, pci->ctl_name);
+
+	if ((err_detect & ~PCI_EDE_MULTI_ERR) & ~PCI_EDE_PERR_MASK)
+		edac_pci_handle_npe(pci, pci->ctl_name);
+}
+
+static irqreturn_t mpc85xx_pci_isr(int irq, void *dev_id)
+{
+	struct edac_pci_ctl_info *pci = dev_id;
+	struct mpc85xx_pci_pdata *pdata = pci->pvt_info;
+	u32 err_detect;
+
+	err_detect = in_be32(pdata->pci_vbase + MPC85XX_PCI_ERR_DR);
+
+	if (!err_detect)
+		return IRQ_NONE;
+
+	return IRQ_HANDLED;
+}
+
+static int __devinit mpc85xx_pci_err_probe(struct of_device *op,
+					   const struct of_device_id *match)
+{
+	struct edac_pci_ctl_info *pci;
+	struct mpc85xx_pci_pdata *pdata;
+	struct resource r;
+	int res;
+
+	if (!devres_open_group(&op->dev, mpc85xx_pci_err_probe, GFP_KERNEL))
+		return -ENOMEM;
+
+	pci = edac_pci_alloc_ctl_info(sizeof(*pdata), "mpc85xx_pci_err");
+	if (!pci)
+		return -ENOMEM;
+
+	pdata = pci->pvt_info;
+	pdata->name = "mpc85xx_pci_err";
+	pdata->irq = NO_IRQ;
+	dev_set_drvdata(&op->dev, pci);
+	pci->dev = &op->dev;
+	pci->dev_name = op->dev.bus_id;
+	pci->mod_name = EDAC_MOD_STR;
+
+	if (edac_op_state == EDAC_OPSTATE_POLL)
+		pci->edac_check = mpc85xx_pci_check;
+
+	pdata->edac_idx = edac_pci_idx++;
+
+	res = of_address_to_resource(op->node, 0, &r);
+	if (res) {
+		printk(KERN_ERR "%s: Unable to get resource for "
+		       "PCI err regs\n", __func__);
+		goto err;
+	}
+
+	/* we only need the error registers */
+	r.start += 0xe00;
+
+	if (!devm_request_mem_region(&op->dev, r.start,
+				     r.end - r.start + 1, pdata->name)) {
+		printk(KERN_ERR "%s: Error while requesting mem region\n",
+		       __func__);
+		res = -EBUSY;
+		goto err;
+	}
+
+	pdata->pci_vbase = devm_ioremap(&op->dev, r.start, r.end - r.start + 1);
+	if (!pdata->pci_vbase) {
+		printk(KERN_ERR "%s: Unable to setup PCI err regs\n", __func__);
+		res = -ENOMEM;
+		goto err;
+	}
+
+	orig_pci_err_cap_dr =
+	    in_be32(pdata->pci_vbase + MPC85XX_PCI_ERR_CAP_DR);
+
+	/* PCI master abort is expected during config cycles */
+	out_be32(pdata->pci_vbase + MPC85XX_PCI_ERR_CAP_DR, 0x40);
+
+	orig_pci_err_en = in_be32(pdata->pci_vbase + MPC85XX_PCI_ERR_EN);
+
+	/* disable master abort reporting */
+	out_be32(pdata->pci_vbase + MPC85XX_PCI_ERR_EN, ~0x40);
+
+	/* clear error bits */
+	out_be32(pdata->pci_vbase + MPC85XX_PCI_ERR_DR, ~0);
+
+	if (edac_pci_add_device(pci, pdata->edac_idx) > 0) {
+		debugf3("%s(): failed edac_pci_add_device()\n", __func__);
+		goto err;
+	}
+
+	if (edac_op_state == EDAC_OPSTATE_INT) {
+		pdata->irq = irq_of_parse_and_map(op->node, 0);
+		res = devm_request_irq(&op->dev, pdata->irq,
+				       mpc85xx_pci_isr, IRQF_DISABLED,
+				       "[EDAC] PCI err", pci);
+		if (res < 0) {
+			printk(KERN_ERR
+			       "%s: Unable to requiest irq %d for "
+			       "MPC85xx PCI err\n", __func__, pdata->irq);
+			irq_dispose_mapping(pdata->irq);
+			res = -ENODEV;
+			goto err2;
+		}
+
+		printk(KERN_INFO EDAC_MOD_STR " acquired irq %d for PCI Err\n",
+		       pdata->irq);
+	}
+
+	devres_remove_group(&op->dev, mpc85xx_pci_err_probe);
+	debugf3("%s(): success\n", __func__);
+	printk(KERN_INFO EDAC_MOD_STR " PCI err registered\n");
+
+	return 0;
+
+err2:
+	edac_pci_del_device(&op->dev);
+err:
+	edac_pci_free_ctl_info(pci);
+	devres_release_group(&op->dev, mpc85xx_pci_err_probe);
+	return res;
+}
+
+static int mpc85xx_pci_err_remove(struct of_device *op)
+{
+	struct edac_pci_ctl_info *pci = dev_get_drvdata(&op->dev);
+	struct mpc85xx_pci_pdata *pdata = pci->pvt_info;
+
+	debugf0("%s()\n", __func__);
+
+	out_be32(pdata->pci_vbase + MPC85XX_PCI_ERR_CAP_DR,
+		 orig_pci_err_cap_dr);
+
+	out_be32(pdata->pci_vbase + MPC85XX_PCI_ERR_EN, orig_pci_err_en);
+
+	edac_pci_del_device(pci->dev);
+
+	if (edac_op_state == EDAC_OPSTATE_INT)
+		irq_dispose_mapping(pdata->irq);
+
+	edac_pci_free_ctl_info(pci);
+
+	return 0;
+}
+
+static struct of_device_id mpc85xx_pci_err_of_match[] = {
+	{
+	 .type = "pci",
+	 .compatible = "fsl,mpc8540-pci",
+	 },
+	{},
+};
+
+static struct of_platform_driver mpc85xx_pci_err_driver = {
+	.owner = THIS_MODULE,
+	.name = "mpc85xx_pci_err",
+	.match_table = mpc85xx_pci_err_of_match,
+	.probe = mpc85xx_pci_err_probe,
+	.remove = mpc85xx_pci_err_remove,
+	.driver = {
+		   .name = "mpc85xx_pci_err",
+		   .owner = THIS_MODULE,
+		   },
+};
+
+#endif				/* CONFIG_PCI */
+
+/**************************** L2 Err device ***************************/
+
+/************************ L2 SYSFS parts ***********************************/
+
+static ssize_t mpc85xx_l2_inject_data_hi_show(struct edac_device_ctl_info
+					      *edac_dev, char *data)
+{
+	struct mpc85xx_l2_pdata *pdata = edac_dev->pvt_info;
+	return sprintf(data, "0x%08x",
+		       in_be32(pdata->l2_vbase + MPC85XX_L2_ERRINJHI));
+}
+
+static ssize_t mpc85xx_l2_inject_data_lo_show(struct edac_device_ctl_info
+					      *edac_dev, char *data)
+{
+	struct mpc85xx_l2_pdata *pdata = edac_dev->pvt_info;
+	return sprintf(data, "0x%08x",
+		       in_be32(pdata->l2_vbase + MPC85XX_L2_ERRINJLO));
+}
+
+static ssize_t mpc85xx_l2_inject_ctrl_show(struct edac_device_ctl_info
+					   *edac_dev, char *data)
+{
+	struct mpc85xx_l2_pdata *pdata = edac_dev->pvt_info;
+	return sprintf(data, "0x%08x",
+		       in_be32(pdata->l2_vbase + MPC85XX_L2_ERRINJCTL));
+}
+
+static ssize_t mpc85xx_l2_inject_data_hi_store(struct edac_device_ctl_info
+					       *edac_dev, const char *data,
+					       size_t count)
+{
+	struct mpc85xx_l2_pdata *pdata = edac_dev->pvt_info;
+	if (isdigit(*data)) {
+		out_be32(pdata->l2_vbase + MPC85XX_L2_ERRINJHI,
+			 simple_strtoul(data, NULL, 0));
+		return count;
+	}
+	return 0;
+}
+
+static ssize_t mpc85xx_l2_inject_data_lo_store(struct edac_device_ctl_info
+					       *edac_dev, const char *data,
+					       size_t count)
+{
+	struct mpc85xx_l2_pdata *pdata = edac_dev->pvt_info;
+	if (isdigit(*data)) {
+		out_be32(pdata->l2_vbase + MPC85XX_L2_ERRINJLO,
+			 simple_strtoul(data, NULL, 0));
+		return count;
+	}
+	return 0;
+}
+
+static ssize_t mpc85xx_l2_inject_ctrl_store(struct edac_device_ctl_info
+					    *edac_dev, const char *data,
+					    size_t count)
+{
+	struct mpc85xx_l2_pdata *pdata = edac_dev->pvt_info;
+	if (isdigit(*data)) {
+		out_be32(pdata->l2_vbase + MPC85XX_L2_ERRINJCTL,
+			 simple_strtoul(data, NULL, 0));
+		return count;
+	}
+	return 0;
+}
+
+static struct edac_dev_sysfs_attribute mpc85xx_l2_sysfs_attributes[] = {
+	{
+	 .attr = {
+		  .name = "inject_data_hi",
+		  .mode = (S_IRUGO | S_IWUSR)
+		  },
+	 .show = mpc85xx_l2_inject_data_hi_show,
+	 .store = mpc85xx_l2_inject_data_hi_store},
+	{
+	 .attr = {
+		  .name = "inject_data_lo",
+		  .mode = (S_IRUGO | S_IWUSR)
+		  },
+	 .show = mpc85xx_l2_inject_data_lo_show,
+	 .store = mpc85xx_l2_inject_data_lo_store},
+	{
+	 .attr = {
+		  .name = "inject_ctrl",
+		  .mode = (S_IRUGO | S_IWUSR)
+		  },
+	 .show = mpc85xx_l2_inject_ctrl_show,
+	 .store = mpc85xx_l2_inject_ctrl_store},
+
+	/* End of list */
+	{
+	 .attr = {.name = NULL}
+	 }
+};
+
+static void mpc85xx_set_l2_sysfs_attributes(struct edac_device_ctl_info
+					    *edac_dev)
+{
+	edac_dev->sysfs_attributes = mpc85xx_l2_sysfs_attributes;
+}
+
+/***************************** L2 ops ***********************************/
+
+static void mpc85xx_l2_check(struct edac_device_ctl_info *edac_dev)
+{
+	struct mpc85xx_l2_pdata *pdata = edac_dev->pvt_info;
+	u32 err_detect;
+
+	err_detect = in_be32(pdata->l2_vbase + MPC85XX_L2_ERRDET);
+
+	if (!(err_detect & L2_EDE_MASK))
+		return;
+
+	printk(KERN_ERR "ECC Error in CPU L2 cache\n");
+	printk(KERN_ERR "L2 Error Detect Register: 0x%08x\n", err_detect);
+	printk(KERN_ERR "L2 Error Capture Data High Register: 0x%08x\n",
+	       in_be32(pdata->l2_vbase + MPC85XX_L2_CAPTDATAHI));
+	printk(KERN_ERR "L2 Error Capture Data Lo Register: 0x%08x\n",
+	       in_be32(pdata->l2_vbase + MPC85XX_L2_CAPTDATALO));
+	printk(KERN_ERR "L2 Error Syndrome Register: 0x%08x\n",
+	       in_be32(pdata->l2_vbase + MPC85XX_L2_CAPTECC));
+	printk(KERN_ERR "L2 Error Attributes Capture Register: 0x%08x\n",
+	       in_be32(pdata->l2_vbase + MPC85XX_L2_ERRATTR));
+	printk(KERN_ERR "L2 Error Address Capture Register: 0x%08x\n",
+	       in_be32(pdata->l2_vbase + MPC85XX_L2_ERRADDR));
+
+	/* clear error detect register */
+	out_be32(pdata->l2_vbase + MPC85XX_L2_ERRDET, err_detect);
+
+	if (err_detect & L2_EDE_CE_MASK)
+		edac_device_handle_ce(edac_dev, 0, 0, edac_dev->ctl_name);
+
+	if (err_detect & L2_EDE_UE_MASK)
+		edac_device_handle_ue(edac_dev, 0, 0, edac_dev->ctl_name);
+}
+
+static irqreturn_t mpc85xx_l2_isr(int irq, void *dev_id)
+{
+	struct edac_device_ctl_info *edac_dev = dev_id;
+	struct mpc85xx_l2_pdata *pdata = edac_dev->pvt_info;
+	u32 err_detect;
+
+	err_detect = in_be32(pdata->l2_vbase + MPC85XX_L2_ERRDET);
+
+	if (!(err_detect & L2_EDE_MASK))
+		return IRQ_NONE;
+
+	mpc85xx_l2_check(edac_dev);
+	return IRQ_HANDLED;
+}
+
+static int __devinit mpc85xx_l2_err_probe(struct of_device *op,
+					  const struct of_device_id *match)
+{
+	struct edac_device_ctl_info *edac_dev;
+	struct mpc85xx_l2_pdata *pdata;
+	struct resource r;
+	int res;
+
+	if (!devres_open_group(&op->dev, mpc85xx_l2_err_probe, GFP_KERNEL))
+		return -ENOMEM;
+
+	edac_dev = edac_device_alloc_ctl_info(sizeof(*pdata),
+					      "cpu", 1, "L", 1, 2, NULL, 0,
+					      edac_dev_idx);
+	if (!edac_dev) {
+		devres_release_group(&op->dev, mpc85xx_l2_err_probe);
+		return -ENOMEM;
+	}
+
+	pdata = edac_dev->pvt_info;
+	pdata->name = "mpc85xx_l2_err";
+	pdata->irq = NO_IRQ;
+	edac_dev->dev = &op->dev;
+	dev_set_drvdata(edac_dev->dev, edac_dev);
+
+	res = of_address_to_resource(op->node, 0, &r);
+	if (res) {
+		printk(KERN_ERR "%s: Unable to get resource for "
+		       "L2 err regs\n", __func__);
+		goto err;
+	}
+
+	/* we only need the error registers */
+	r.start += 0xe00;
+
+	if (!devm_request_mem_region(&op->dev, r.start,
+				     r.end - r.start + 1, pdata->name)) {
+		printk(KERN_ERR "%s: Error while requesting mem region\n",
+		       __func__);
+		res = -EBUSY;
+		goto err;
+	}
+
+	pdata->l2_vbase = devm_ioremap(&op->dev, r.start, r.end - r.start + 1);
+	if (!pdata->l2_vbase) {
+		printk(KERN_ERR "%s: Unable to setup L2 err regs\n", __func__);
+		res = -ENOMEM;
+		goto err;
+	}
+
+	out_be32(pdata->l2_vbase + MPC85XX_L2_ERRDET, ~0);
+
+	orig_l2_err_disable = in_be32(pdata->l2_vbase + MPC85XX_L2_ERRDIS);
+
+	/* clear the err_dis */
+	out_be32(pdata->l2_vbase + MPC85XX_L2_ERRDIS, 0);
+
+	edac_dev->mod_name = EDAC_MOD_STR;
+	edac_dev->ctl_name = pdata->name;
+
+	if (edac_op_state == EDAC_OPSTATE_POLL)
+		edac_dev->edac_check = mpc85xx_l2_check;
+
+	mpc85xx_set_l2_sysfs_attributes(edac_dev);
+
+	pdata->edac_idx = edac_dev_idx++;
+
+	if (edac_device_add_device(edac_dev) > 0) {
+		debugf3("%s(): failed edac_device_add_device()\n", __func__);
+		goto err;
+	}
+
+	if (edac_op_state == EDAC_OPSTATE_INT) {
+		pdata->irq = irq_of_parse_and_map(op->node, 0);
+		res = devm_request_irq(&op->dev, pdata->irq,
+				       mpc85xx_l2_isr, IRQF_DISABLED,
+				       "[EDAC] L2 err", edac_dev);
+		if (res < 0) {
+			printk(KERN_ERR
+			       "%s: Unable to requiest irq %d for "
+			       "MPC85xx L2 err\n", __func__, pdata->irq);
+			irq_dispose_mapping(pdata->irq);
+			res = -ENODEV;
+			goto err2;
+		}
+
+		printk(KERN_INFO EDAC_MOD_STR " acquired irq %d for L2 Err\n",
+		       pdata->irq);
+
+		edac_dev->op_state = OP_RUNNING_INTERRUPT;
+
+		out_be32(pdata->l2_vbase + MPC85XX_L2_ERRINTEN, L2_EIE_MASK);
+	}
+
+	devres_remove_group(&op->dev, mpc85xx_l2_err_probe);
+
+	debugf3("%s(): success\n", __func__);
+	printk(KERN_INFO EDAC_MOD_STR " L2 err registered\n");
+
+	return 0;
+
+err2:
+	edac_device_del_device(&op->dev);
+err:
+	devres_release_group(&op->dev, mpc85xx_l2_err_probe);
+	edac_device_free_ctl_info(edac_dev);
+	return res;
+}
+
+static int mpc85xx_l2_err_remove(struct of_device *op)
+{
+	struct edac_device_ctl_info *edac_dev = dev_get_drvdata(&op->dev);
+	struct mpc85xx_l2_pdata *pdata = edac_dev->pvt_info;
+
+	debugf0("%s()\n", __func__);
+
+	if (edac_op_state == EDAC_OPSTATE_INT) {
+		out_be32(pdata->l2_vbase + MPC85XX_L2_ERRINTEN, 0);
+		irq_dispose_mapping(pdata->irq);
+	}
+
+	out_be32(pdata->l2_vbase + MPC85XX_L2_ERRDIS, orig_l2_err_disable);
+	edac_device_del_device(&op->dev);
+	edac_device_free_ctl_info(edac_dev);
+	return 0;
+}
+
+static struct of_device_id mpc85xx_l2_err_of_match[] = {
+	{
+	 .compatible = "fsl,8540-l2-cache-controller",
+	 },
+	{
+	 .compatible = "fsl,8541-l2-cache-controller",
+	 },
+	{
+	 .compatible = "fsl,8544-l2-cache-controller",
+	 },
+	{
+	 .compatible = "fsl,8548-l2-cache-controller",
+	 },
+	{
+	 .compatible = "fsl,8555-l2-cache-controller",
+	 },
+	{
+	 .compatible = "fsl,8568-l2-cache-controller",
+	 },
+	{},
+};
+
+static struct of_platform_driver mpc85xx_l2_err_driver = {
+	.owner = THIS_MODULE,
+	.name = "mpc85xx_l2_err",
+	.match_table = mpc85xx_l2_err_of_match,
+	.probe = mpc85xx_l2_err_probe,
+	.remove = mpc85xx_l2_err_remove,
+	.driver = {
+		   .name = "mpc85xx_l2_err",
+		   .owner = THIS_MODULE,
+		   },
+};
+
+/**************************** MC Err device ***************************/
+
+static void mpc85xx_mc_check(struct mem_ctl_info *mci)
+{
+	struct mpc85xx_mc_pdata *pdata = mci->pvt_info;
+	struct csrow_info *csrow;
+	u32 err_detect;
+	u32 syndrome;
+	u32 err_addr;
+	u32 pfn;
+	int row_index;
+
+	err_detect = in_be32(pdata->mc_vbase + MPC85XX_MC_ERR_DETECT);
+	if (!unlikely(err_detect))
+		return;
+
+	mpc85xx_mc_printk(mci, KERN_ERR, "Err Detect Register: %#8.8x\n",
+			  err_detect);
+
+	/* no more processing if not ECC bit errors */
+	if (!(err_detect & (DDR_EDE_SBE | DDR_EDE_MBE))) {
+		out_be32(pdata->mc_vbase + MPC85XX_MC_ERR_DETECT, err_detect);
+		return;
+	}
+
+	syndrome = in_be32(pdata->mc_vbase + MPC85XX_MC_CAPTURE_ECC);
+	err_addr = in_be32(pdata->mc_vbase + MPC85XX_MC_CAPTURE_ADDRESS);
+	pfn = err_addr >> PAGE_SHIFT;
+
+	for (row_index = 0; row_index < mci->nr_csrows; row_index++) {
+		csrow = &mci->csrows[row_index];
+		if ((pfn >= csrow->first_page) && (pfn <= csrow->last_page))
+			break;
+	}
+
+	mpc85xx_mc_printk(mci, KERN_ERR, "Capture Data High: %#8.8x\n",
+			  in_be32(pdata->mc_vbase +
+				  MPC85XX_MC_CAPTURE_DATA_HI));
+	mpc85xx_mc_printk(mci, KERN_ERR, "Capture Data Low: %#8.8x\n",
+			  in_be32(pdata->mc_vbase +
+				  MPC85XX_MC_CAPTURE_DATA_LO));
+	mpc85xx_mc_printk(mci, KERN_ERR, "syndrome: %#8.8x\n", syndrome);
+	mpc85xx_mc_printk(mci, KERN_ERR, "err addr: %#8.8x\n", err_addr);
+	mpc85xx_mc_printk(mci, KERN_ERR, "PFN: %#8.8x\n", pfn);
+
+	/* we are out of range */
+	if (row_index == mci->nr_csrows)
+		mpc85xx_mc_printk(mci, KERN_ERR, "PFN out of range!\n");
+
+	if (err_detect & DDR_EDE_SBE)
+		edac_mc_handle_ce(mci, pfn, err_addr & PAGE_MASK,
+				  syndrome, row_index, 0, mci->ctl_name);
+
+	if (err_detect & DDR_EDE_MBE)
+		edac_mc_handle_ue(mci, pfn, err_addr & PAGE_MASK,
+				  row_index, mci->ctl_name);
+
+	out_be32(pdata->mc_vbase + MPC85XX_MC_ERR_DETECT, err_detect);
+}
+
+static irqreturn_t mpc85xx_mc_isr(int irq, void *dev_id)
+{
+	struct mem_ctl_info *mci = dev_id;
+	struct mpc85xx_mc_pdata *pdata = mci->pvt_info;
+	u32 err_detect;
+
+	err_detect = in_be32(pdata->mc_vbase + MPC85XX_MC_ERR_DETECT);
+	if (unlikely(!err_detect))
+		return IRQ_NONE;
+
+	mpc85xx_mc_check(mci);
+
+	return IRQ_HANDLED;
+}
+
+static void __devinit mpc85xx_init_csrows(struct mem_ctl_info *mci)
+{
+	struct mpc85xx_mc_pdata *pdata = mci->pvt_info;
+	struct csrow_info *csrow;
+	u32 sdram_ctl;
+	u32 sdtype;
+	enum mem_type mtype;
+	u32 cs_bnds;
+	int index;
+
+	sdram_ctl = in_be32(pdata->mc_vbase + MPC85XX_MC_DDR_SDRAM_CFG);
+
+	sdtype = sdram_ctl & DSC_SDTYPE_MASK;
+	if (sdram_ctl & DSC_RD_EN) {
+		switch (sdtype) {
+		case DSC_SDTYPE_DDR:
+			mtype = MEM_RDDR;
+			break;
+		case DSC_SDTYPE_DDR2:
+			mtype = MEM_RDDR2;
+			break;
+		default:
+			mtype = MEM_UNKNOWN;
+			break;
+		}
+	} else {
+		switch (sdtype) {
+		case DSC_SDTYPE_DDR:
+			mtype = MEM_DDR;
+			break;
+		case DSC_SDTYPE_DDR2:
+			mtype = MEM_DDR2;
+			break;
+		default:
+			mtype = MEM_UNKNOWN;
+			break;
+		}
+	}
+
+	for (index = 0; index < mci->nr_csrows; index++) {
+		u32 start;
+		u32 end;
+
+		csrow = &mci->csrows[index];
+		cs_bnds = in_be32(pdata->mc_vbase + MPC85XX_MC_CS_BNDS_0 +
+				  (index * MPC85XX_MC_CS_BNDS_OFS));
+		start = (cs_bnds & 0xfff0000) << 4;
+		end = ((cs_bnds & 0xfff) << 20);
+		if (start)
+			start |= 0xfffff;
+		if (end)
+			end |= 0xfffff;
+
+		if (start == end)
+			continue;	/* not populated */
+
+		csrow->first_page = start >> PAGE_SHIFT;
+		csrow->last_page = end >> PAGE_SHIFT;
+		csrow->nr_pages = csrow->last_page + 1 - csrow->first_page;
+		csrow->grain = 8;
+		csrow->mtype = mtype;
+		csrow->dtype = DEV_UNKNOWN;
+		if (sdram_ctl & DSC_X32_EN)
+			csrow->dtype = DEV_X32;
+		csrow->edac_mode = EDAC_SECDED;
+	}
+}
+
+static int __devinit mpc85xx_mc_err_probe(struct of_device *op,
+					  const struct of_device_id *match)
+{
+	struct mem_ctl_info *mci;
+	struct mpc85xx_mc_pdata *pdata;
+	struct resource r;
+	u32 sdram_ctl;
+	int res;
+
+	if (!devres_open_group(&op->dev, mpc85xx_mc_err_probe, GFP_KERNEL))
+		return -ENOMEM;
+
+	mci = edac_mc_alloc(sizeof(*pdata), 4, 1, edac_mc_idx);
+	if (!mci) {
+		devres_release_group(&op->dev, mpc85xx_mc_err_probe);
+		return -ENOMEM;
+	}
+
+	pdata = mci->pvt_info;
+	pdata->name = "mpc85xx_mc_err";
+	pdata->irq = NO_IRQ;
+	mci->dev = &op->dev;
+	pdata->edac_idx = edac_mc_idx++;
+	dev_set_drvdata(mci->dev, mci);
+
+	res = of_address_to_resource(op->node, 0, &r);
+	if (res) {
+		printk(KERN_ERR "%s: Unable to get resource for MC err regs\n",
+		       __func__);
+		goto err;
+	}
+
+	if (!devm_request_mem_region(&op->dev, r.start,
+				     r.end - r.start + 1, pdata->name)) {
+		printk(KERN_ERR "%s: Error while requesting mem region\n",
+		       __func__);
+		res = -EBUSY;
+		goto err;
+	}
+
+	pdata->mc_vbase = devm_ioremap(&op->dev, r.start, r.end - r.start + 1);
+	if (!pdata->mc_vbase) {
+		printk(KERN_ERR "%s: Unable to setup MC err regs\n", __func__);
+		res = -ENOMEM;
+		goto err;
+	}
+
+	sdram_ctl = in_be32(pdata->mc_vbase + MPC85XX_MC_DDR_SDRAM_CFG);
+	if (!(sdram_ctl & DSC_ECC_EN)) {
+		/* no ECC */
+		printk(KERN_WARNING "%s: No ECC DIMMs discovered\n", __func__);
+		res = -ENODEV;
+		goto err;
+	}
+
+	debugf3("%s(): init mci\n", __func__);
+	mci->mtype_cap = MEM_FLAG_RDDR | MEM_FLAG_RDDR2 |
+	    MEM_FLAG_DDR | MEM_FLAG_DDR2;
+	mci->edac_ctl_cap = EDAC_FLAG_NONE | EDAC_FLAG_SECDED;
+	mci->edac_cap = EDAC_FLAG_SECDED;
+	mci->mod_name = EDAC_MOD_STR;
+	mci->mod_ver = MPC85XX_REVISION;
+	mci->ctl_name = mpc85xx_ctl_name;
+
+	if (edac_op_state == EDAC_OPSTATE_POLL)
+		mci->edac_check = mpc85xx_mc_check;
+
+	mci->ctl_page_to_phys = NULL;
+
+	mci->scrub_mode = SCRUB_SW_SRC;
+
+	mpc85xx_set_mc_sysfs_attributes(mci);
+
+	mpc85xx_init_csrows(mci);
+
+#ifdef CONFIG_EDAC_DEBUG
+	edac_mc_register_mcidev_debug((struct attribute **)debug_attr);
+#endif
+
+	/* store the original error disable bits */
+	orig_ddr_err_disable =
+	    in_be32(pdata->mc_vbase + MPC85XX_MC_ERR_DISABLE);
+	out_be32(pdata->mc_vbase + MPC85XX_MC_ERR_DISABLE, 0);
+
+	/* clear all error bits */
+	out_be32(pdata->mc_vbase + MPC85XX_MC_ERR_DETECT, ~0);
+
+	if (edac_mc_add_mc(mci)) {
+		debugf3("%s(): failed edac_mc_add_mc()\n", __func__);
+		goto err;
+	}
+
+	if (edac_op_state == EDAC_OPSTATE_INT) {
+		out_be32(pdata->mc_vbase + MPC85XX_MC_ERR_INT_EN,
+			 DDR_EIE_MBEE | DDR_EIE_SBEE);
+
+		/* store the original error management threshold */
+		orig_ddr_err_sbe = in_be32(pdata->mc_vbase +
+					   MPC85XX_MC_ERR_SBE) & 0xff0000;
+
+		/* set threshold to 1 error per interrupt */
+		out_be32(pdata->mc_vbase + MPC85XX_MC_ERR_SBE, 0x10000);
+
+		/* register interrupts */
+		pdata->irq = irq_of_parse_and_map(op->node, 0);
+		res = devm_request_irq(&op->dev, pdata->irq,
+				       mpc85xx_mc_isr, IRQF_DISABLED,
+				       "[EDAC] MC err", mci);
+		if (res < 0) {
+			printk(KERN_ERR "%s: Unable to request irq %d for "
+			       "MPC85xx DRAM ERR\n", __func__, pdata->irq);
+			irq_dispose_mapping(pdata->irq);
+			res = -ENODEV;
+			goto err2;
+		}
+
+		printk(KERN_INFO EDAC_MOD_STR " acquired irq %d for MC\n",
+		       pdata->irq);
+	}
+
+	devres_remove_group(&op->dev, mpc85xx_mc_err_probe);
+	debugf3("%s(): success\n", __func__);
+	printk(KERN_INFO EDAC_MOD_STR " MC err registered\n");
+
+	return 0;
+
+err2:
+	edac_mc_del_mc(&op->dev);
+err:
+	devres_release_group(&op->dev, mpc85xx_mc_err_probe);
+	edac_mc_free(mci);
+	return res;
+}
+
+static int mpc85xx_mc_err_remove(struct of_device *op)
+{
+	struct mem_ctl_info *mci = dev_get_drvdata(&op->dev);
+	struct mpc85xx_mc_pdata *pdata = mci->pvt_info;
+
+	debugf0("%s()\n", __func__);
+
+	if (edac_op_state == EDAC_OPSTATE_INT) {
+		out_be32(pdata->mc_vbase + MPC85XX_MC_ERR_INT_EN, 0);
+		irq_dispose_mapping(pdata->irq);
+	}
+
+	out_be32(pdata->mc_vbase + MPC85XX_MC_ERR_DISABLE,
+		 orig_ddr_err_disable);
+	out_be32(pdata->mc_vbase + MPC85XX_MC_ERR_SBE, orig_ddr_err_sbe);
+
+	edac_mc_del_mc(&op->dev);
+	edac_mc_free(mci);
+	return 0;
+}
+
+static struct of_device_id mpc85xx_mc_err_of_match[] = {
+	{
+	 .compatible = "fsl,8540-memory-controller",
+	 },
+	{
+	 .compatible = "fsl,8541-memory-controller",
+	 },
+	{
+	 .compatible = "fsl,8544-memory-controller",
+	 },
+	{
+	 .compatible = "fsl,8548-memory-controller",
+	 },
+	{
+	 .compatible = "fsl,8555-memory-controller",
+	 },
+	{
+	 .compatible = "fsl,8568-memory-controller",
+	 },
+	{},
+};
+
+static struct of_platform_driver mpc85xx_mc_err_driver = {
+	.owner = THIS_MODULE,
+	.name = "mpc85xx_mc_err",
+	.match_table = mpc85xx_mc_err_of_match,
+	.probe = mpc85xx_mc_err_probe,
+	.remove = mpc85xx_mc_err_remove,
+	.driver = {
+		   .name = "mpc85xx_mc_err",
+		   .owner = THIS_MODULE,
+		   },
+};
+
+static int __init mpc85xx_mc_init(void)
+{
+	int res = 0;
+
+	printk(KERN_INFO "Freescale(R) MPC85xx EDAC driver, "
+	       "(C) 2006 Montavista Software\n");
+
+	/* make sure error reporting method is sane */
+	switch (edac_op_state) {
+	case EDAC_OPSTATE_POLL:
+	case EDAC_OPSTATE_INT:
+		break;
+	default:
+		edac_op_state = EDAC_OPSTATE_INT;
+		break;
+	}
+
+	res = of_register_platform_driver(&mpc85xx_mc_err_driver) ? : res;
+
+	res = of_register_platform_driver(&mpc85xx_l2_err_driver) ? : res;
+
+#ifdef CONFIG_PCI
+	res = of_register_platform_driver(&mpc85xx_pci_err_driver) ? : res;
+#endif
+
+	/*
+	 * need to clear HID1[RFXE] to disable machine check int
+	 * so we can catch it
+	 */
+	if (edac_op_state == EDAC_OPSTATE_INT) {
+		orig_hid1 = mfspr(SPRN_HID1);
+
+		mtspr(SPRN_HID1, (orig_hid1 & ~0x20000));
+	}
+
+	return res;
+}
+
+module_init(mpc85xx_mc_init);
+
+static void __exit mpc85xx_mc_exit(void)
+{
+	mtspr(SPRN_HID1, orig_hid1);
+#ifdef CONFIG_PCI
+	of_unregister_platform_driver(&mpc85xx_pci_err_driver);
+#endif
+	of_unregister_platform_driver(&mpc85xx_l2_err_driver);
+	of_unregister_platform_driver(&mpc85xx_mc_err_driver);
+}
+
+module_exit(mpc85xx_mc_exit);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Montavista Software, Inc.");
+module_param(edac_op_state, int, 0444);
+MODULE_PARM_DESC(edac_op_state,
+		 "EDAC Error Reporting state: 0=Poll, 2=Interrupt");
diff --git a/drivers/edac/mpc85xx_edac.h b/drivers/edac/mpc85xx_edac.h
new file mode 100644
index 0000000..135b353
--- /dev/null
+++ b/drivers/edac/mpc85xx_edac.h
@@ -0,0 +1,162 @@
+/*
+ * Freescale MPC85xx Memory Controller kenel module
+ * Author: Dave Jiang <djiang@mvista.com>
+ *
+ * 2006-2007 (c) MontaVista Software, Inc. This file is licensed under
+ * the terms of the GNU General Public License version 2. This program
+ * is licensed "as is" without any warranty of any kind, whether express
+ * or implied.
+ *
+ */
+#ifndef _MPC85XX_EDAC_H_
+#define _MPC85XX_EDAC_H_
+
+#define MPC85XX_REVISION " Ver: 2.0.0 " __DATE__
+#define EDAC_MOD_STR	"MPC85xx_edac"
+
+#define mpc85xx_printk(level, fmt, arg...) \
+	edac_printk(level, "MPC85xx", fmt, ##arg)
+
+#define mpc85xx_mc_printk(mci, level, fmt, arg...) \
+	edac_mc_chipset_printk(mci, level, "MPC85xx", fmt, ##arg)
+
+/*
+ * DRAM error defines
+ */
+
+/* DDR_SDRAM_CFG */
+#define MPC85XX_MC_DDR_SDRAM_CFG	0x0110
+#define MPC85XX_MC_CS_BNDS_0		0x0000
+#define MPC85XX_MC_CS_BNDS_1		0x0008
+#define MPC85XX_MC_CS_BNDS_2		0x0010
+#define MPC85XX_MC_CS_BNDS_3		0x0018
+#define MPC85XX_MC_CS_BNDS_OFS		0x0008
+
+#define MPC85XX_MC_DATA_ERR_INJECT_HI	0x0e00
+#define MPC85XX_MC_DATA_ERR_INJECT_LO	0x0e04
+#define MPC85XX_MC_ECC_ERR_INJECT	0x0e08
+#define MPC85XX_MC_CAPTURE_DATA_HI	0x0e20
+#define MPC85XX_MC_CAPTURE_DATA_LO	0x0e24
+#define MPC85XX_MC_CAPTURE_ECC		0x0e28
+#define MPC85XX_MC_ERR_DETECT		0x0e40
+#define MPC85XX_MC_ERR_DISABLE		0x0e44
+#define MPC85XX_MC_ERR_INT_EN		0x0e48
+#define MPC85XX_MC_CAPTURE_ATRIBUTES	0x0e4c
+#define MPC85XX_MC_CAPTURE_ADDRESS	0x0e50
+#define MPC85XX_MC_ERR_SBE		0x0e58
+
+#define DSC_MEM_EN	0x80000000
+#define DSC_ECC_EN	0x20000000
+#define DSC_RD_EN	0x10000000
+
+#define DSC_SDTYPE_MASK		0x07000000
+
+#define DSC_SDTYPE_DDR		0x02000000
+#define DSC_SDTYPE_DDR2		0x03000000
+#define DSC_X32_EN	0x00000020
+
+/* Err_Int_En */
+#define DDR_EIE_MSEE	0x1	/* memory select */
+#define DDR_EIE_SBEE	0x4	/* single-bit ECC error */
+#define DDR_EIE_MBEE	0x8	/* multi-bit ECC error */
+
+/* Err_Detect */
+#define DDR_EDE_MSE		0x1	/* memory select */
+#define DDR_EDE_SBE		0x4	/* single-bit ECC error */
+#define DDR_EDE_MBE		0x8	/* multi-bit ECC error */
+#define DDR_EDE_MME		0x80000000	/* multiple memory errors */
+
+/* Err_Disable */
+#define DDR_EDI_MSED	0x1	/* memory select disable */
+#define	DDR_EDI_SBED	0x4	/* single-bit ECC error disable */
+#define	DDR_EDI_MBED	0x8	/* multi-bit ECC error disable */
+
+/*
+ * L2 Err defines
+ */
+#define MPC85XX_L2_ERRINJHI	0x0000
+#define MPC85XX_L2_ERRINJLO	0x0004
+#define MPC85XX_L2_ERRINJCTL	0x0008
+#define MPC85XX_L2_CAPTDATAHI	0x0020
+#define MPC85XX_L2_CAPTDATALO	0x0024
+#define MPC85XX_L2_CAPTECC	0x0028
+#define MPC85XX_L2_ERRDET	0x0040
+#define MPC85XX_L2_ERRDIS	0x0044
+#define MPC85XX_L2_ERRINTEN	0x0048
+#define MPC85XX_L2_ERRATTR	0x004c
+#define MPC85XX_L2_ERRADDR	0x0050
+#define MPC85XX_L2_ERRCTL	0x0058
+
+/* Error Interrupt Enable */
+#define L2_EIE_L2CFGINTEN	0x1
+#define L2_EIE_SBECCINTEN	0x4
+#define L2_EIE_MBECCINTEN	0x8
+#define L2_EIE_TPARINTEN	0x10
+#define L2_EIE_MASK	(L2_EIE_L2CFGINTEN | L2_EIE_SBECCINTEN | \
+			L2_EIE_MBECCINTEN | L2_EIE_TPARINTEN)
+
+/* Error Detect */
+#define L2_EDE_L2CFGERR		0x1
+#define L2_EDE_SBECCERR		0x4
+#define L2_EDE_MBECCERR		0x8
+#define L2_EDE_TPARERR		0x10
+#define L2_EDE_MULL2ERR		0x80000000
+
+#define L2_EDE_CE_MASK	L2_EDE_SBECCERR
+#define L2_EDE_UE_MASK	(L2_EDE_L2CFGERR | L2_EDE_MBECCERR | \
+			L2_EDE_TPARERR)
+#define L2_EDE_MASK	(L2_EDE_L2CFGERR | L2_EDE_SBECCERR | \
+			L2_EDE_MBECCERR | L2_EDE_TPARERR | L2_EDE_MULL2ERR)
+
+/*
+ * PCI Err defines
+ */
+#define PCI_EDE_TOE			0x00000001
+#define PCI_EDE_SCM			0x00000002
+#define PCI_EDE_IRMSV			0x00000004
+#define PCI_EDE_ORMSV			0x00000008
+#define PCI_EDE_OWMSV			0x00000010
+#define PCI_EDE_TGT_ABRT		0x00000020
+#define PCI_EDE_MST_ABRT		0x00000040
+#define PCI_EDE_TGT_PERR		0x00000080
+#define PCI_EDE_MST_PERR		0x00000100
+#define PCI_EDE_RCVD_SERR		0x00000200
+#define PCI_EDE_ADDR_PERR		0x00000400
+#define PCI_EDE_MULTI_ERR		0x80000000
+
+#define PCI_EDE_PERR_MASK	(PCI_EDE_TGT_PERR | PCI_EDE_MST_PERR | \
+				PCI_EDE_ADDR_PERR)
+
+#define MPC85XX_PCI_ERR_DR		0x0000
+#define MPC85XX_PCI_ERR_CAP_DR		0x0004
+#define MPC85XX_PCI_ERR_EN		0x0008
+#define MPC85XX_PCI_ERR_ATTRIB		0x000c
+#define MPC85XX_PCI_ERR_ADDR		0x0010
+#define MPC85XX_PCI_ERR_EXT_ADDR	0x0014
+#define MPC85XX_PCI_ERR_DL		0x0018
+#define MPC85XX_PCI_ERR_DH		0x001c
+#define MPC85XX_PCI_GAS_TIMR		0x0020
+#define MPC85XX_PCI_PCIX_TIMR		0x0024
+
+struct mpc85xx_mc_pdata {
+	char *name;
+	int edac_idx;
+	void __iomem *mc_vbase;
+	int irq;
+};
+
+struct mpc85xx_l2_pdata {
+	char *name;
+	int edac_idx;
+	void __iomem *l2_vbase;
+	int irq;
+};
+
+struct mpc85xx_pci_pdata {
+	char *name;
+	int edac_idx;
+	void __iomem *pci_vbase;
+	int irq;
+};
+
+#endif

Re: [PATCH 2/2] powerpc: MPC85xx EDAC device driver

[Arnd Bergmann]

On Friday 27 July 2007, Dave Jiang wrote:
> +static struct of_device_id mpc85xx_pci_err_of_match[] =3D {
> +       {
> +        .type =3D "pci",
> +        .compatible =3D "fsl,mpc8540-pci",
> +        },
> +       {},
> +};
> +
> +static struct of_platform_driver mpc85xx_pci_err_driver =3D {
> +       .owner =3D THIS_MODULE,
> +       .name =3D "mpc85xx_pci_err",
> +       .match_table =3D mpc85xx_pci_err_of_match,
> +       .probe =3D mpc85xx_pci_err_probe,
> +       .remove =3D mpc85xx_pci_err_remove,
> +       .driver =3D {
> +                  .name =3D "mpc85xx_pci_err",
> +                  .owner =3D THIS_MODULE,
> +                  },
> +};

This is  a little problematic, if we want to make the PCI bus implementation
use the PCI code from arch/powerpc/kernel/of_platform.c in the future.
Right now this is not possible, because that code is still 64-bit only,
but that may change in the future. Since only one driver can bind
to the pci host bridge device, the mpc85xx_pci_err driver would conflict
with the PCI driver itself, which you probably don't intend.

I'd suggest either to integrate EDAC into the 85xx specific PCI code,
or to have an extra device in the device tree for this.

> +=A0=A0=A0=A0=A0=A0=A0res =3D of_register_platform_driver(&mpc85xx_mc_err=
_driver) ? : res;
> +
> +=A0=A0=A0=A0=A0=A0=A0res =3D of_register_platform_driver(&mpc85xx_l2_err=
_driver) ? : res;
> +
> +#ifdef CONFIG_PCI
> +=A0=A0=A0=A0=A0=A0=A0res =3D of_register_platform_driver(&mpc85xx_pci_er=
r_driver) ? : res;
> +#endif
> +
> +=A0=A0=A0=A0=A0=A0=A0/*
> +=A0=A0=A0=A0=A0=A0=A0 * need to clear HID1[RFXE] to disable machine chec=
k int
> +=A0=A0=A0=A0=A0=A0=A0 * so we can catch it
> +=A0=A0=A0=A0=A0=A0=A0 */
> +=A0=A0=A0=A0=A0=A0=A0if (edac_op_state =3D=3D EDAC_OPSTATE_INT) {
> +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0orig_hid1 =3D mfspr(SPRN_HI=
D1);
> +
> +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0mtspr(SPRN_HID1, (orig_hid1=
 & ~0x20000));
> +=A0=A0=A0=A0=A0=A0=A0}
> +
> +=A0=A0=A0=A0=A0=A0=A0return res;
> +}

The error handling could use some improvement here. In particular, you shou=
ld
unregister the buses in the failure path, maybe you need to clean up other
parts as well.

	Arnd <><

Re: [PATCH 2/2] powerpc: MPC85xx EDAC device driver

[Dave Jiang]

Arnd Bergmann wrote:
> On Friday 27 July 2007, Dave Jiang wrote:
>> +static struct of_device_id mpc85xx_pci_err_of_match[] = {
>> +       {
>> +        .type = "pci",
>> +        .compatible = "fsl,mpc8540-pci",
>> +        },
>> +       {},
>> +};
>> +
>> +static struct of_platform_driver mpc85xx_pci_err_driver = {
>> +       .owner = THIS_MODULE,
>> +       .name = "mpc85xx_pci_err",
>> +       .match_table = mpc85xx_pci_err_of_match,
>> +       .probe = mpc85xx_pci_err_probe,
>> +       .remove = mpc85xx_pci_err_remove,
>> +       .driver = {
>> +                  .name = "mpc85xx_pci_err",
>> +                  .owner = THIS_MODULE,
>> +                  },
>> +};
> 
> This is  a little problematic, if we want to make the PCI bus implementation
> use the PCI code from arch/powerpc/kernel/of_platform.c in the future.
> Right now this is not possible, because that code is still 64-bit only,
> but that may change in the future. Since only one driver can bind
> to the pci host bridge device, the mpc85xx_pci_err driver would conflict
> with the PCI driver itself, which you probably don't intend.
> 
> I'd suggest either to integrate EDAC into the 85xx specific PCI code,
> or to have an extra device in the device tree for this.

How about I create a platform device just for EDAC and leave the PCI of_device
to the 85xx PCI code? That would be a lot less modification than adding a
device for every PCI hose per DTS file.... Just a thought....

>> +       res = of_register_platform_driver(&mpc85xx_mc_err_driver) ? : res;
>> +
>> +       res = of_register_platform_driver(&mpc85xx_l2_err_driver) ? : res;
>> +
>> +#ifdef CONFIG_PCI
>> +       res = of_register_platform_driver(&mpc85xx_pci_err_driver) ? : res;
>> +#endif
>> +
>> +       /*
>> +        * need to clear HID1[RFXE] to disable machine check int
>> +        * so we can catch it
>> +        */
>> +       if (edac_op_state == EDAC_OPSTATE_INT) {
>> +               orig_hid1 = mfspr(SPRN_HID1);
>> +
>> +               mtspr(SPRN_HID1, (orig_hid1 & ~0x20000));
>> +       }
>> +
>> +       return res;
>> +}
> 
> The error handling could use some improvement here. In particular, you should
> unregister the buses in the failure path, maybe you need to clean up other
> parts as well.

I think I want individual "devices" work even some may fail or be missing. For
example, even if L2 fails to register, I still want to be able to get the
memory controller to report errors. So I really don't want to unregister
everything that initialized properly even though some failures exists. Maybe I
need to clean it up a little bit.

-- 

------------------------------------------------------
Dave Jiang
Software Engineer
MontaVista Software, Inc.
http://www.mvista.com
------------------------------------------------------

Re: [PATCH 2/2] powerpc: MPC85xx EDAC device driver

[Arnd Bergmann]

On Monday 30 July 2007, Dave Jiang wrote:
> Arnd Bergmann wrote:

> > I'd suggest either to integrate EDAC into the 85xx specific PCI code,
> > or to have an extra device in the device tree for this.
> 
> How about I create a platform device just for EDAC and leave the PCI of_device
> to the 85xx PCI code? That would be a lot less modification than adding a
> device for every PCI hose per DTS file.... Just a thought....

Hmm, I can see your point about not having to change all the device trees,
but if you really want to have a device for each of them, an of_device seems
more natural to me than a platform_device.

What about the other option I mentioned, handling the EDAC stuff from the
fsl_add_bridge() function?
 
> > The error handling could use some improvement here. In particular, you should
> > unregister the buses in the failure path, maybe you need to clean up other
> > parts as well.
> 
> I think I want individual "devices" work even some may fail or be missing. For
> example, even if L2 fails to register, I still want to be able to get the
> memory controller to report errors. So I really don't want to unregister
> everything that initialized properly even though some failures exists. Maybe I
> need to clean it up a little bit.

In that case, you shouldn't return failure from the module_init() function,
because that will remove the module again, while leaving the drivers
registered.

Another option might be to have separate modules, or separate initcall()
statements if the code can not be modular.

	Arnd <><

Re: [PATCH 2/2] powerpc: MPC85xx EDAC device driver

[Dave Jiang]

Arnd Bergmann wrote:
> On Monday 30 July 2007, Dave Jiang wrote:
>> Arnd Bergmann wrote:
> 
>>> I'd suggest either to integrate EDAC into the 85xx specific PCI code,
>>> or to have an extra device in the device tree for this.
>> How about I create a platform device just for EDAC and leave the PCI of_device
>> to the 85xx PCI code? That would be a lot less modification than adding a
>> device for every PCI hose per DTS file.... Just a thought....
> 
> Hmm, I can see your point about not having to change all the device trees,
> but if you really want to have a device for each of them, an of_device seems
> more natural to me than a platform_device.
> 
> What about the other option I mentioned, handling the EDAC stuff from the
> fsl_add_bridge() function?

I'm not sure I follow your thought on this quite yet. Do you mean the setup of
either of_device or platform_device or do you mean the actual pci err driver?
I'm not understanding how doing the setup code in fsl_add_bridge() would allow
us to prevent the of_device being monopolized by the EDAC driver or the future
PCI code.... The EDAC driver still needs some sort of "device" to claim one way
or another right?

Of course I can go ahead and add the PCI err device in the DTS files for all
85xx if Kumar is okay with that idea....

-- 

------------------------------------------------------
Dave Jiang
Software Engineer
MontaVista Software, Inc.
http://www.mvista.com
------------------------------------------------------

Re: [PATCH 2/2] powerpc: MPC85xx EDAC device driver

[Arnd Bergmann]

On Monday 30 July 2007, Dave Jiang wrote:
> 
> > What about the other option I mentioned, handling the EDAC stuff from the
> > fsl_add_bridge() function?
> 
> I'm not sure I follow your thought on this quite yet. Do you mean the setup of
> either of_device or platform_device or do you mean the actual pci err driver?
> I'm not understanding how doing the setup code in fsl_add_bridge() would allow
> us to prevent the of_device being monopolized by the EDAC driver or the future
> PCI code.... The EDAC driver still needs some sort of "device" to claim one way
> or another right?

What I meant is calling the edac_pci_alloc_ctl_info() function directly
from fsl_add_bridge() where appropriate, with all the 85xx PCI EDAC
code moved into fsl_pci.c.

	Arnd <><

Re: [PATCH 2/2] powerpc: MPC85xx EDAC device driver

[Dave Jiang]

Arnd Bergmann wrote:
> On Monday 30 July 2007, Dave Jiang wrote:
>>> What about the other option I mentioned, handling the EDAC stuff from the
>>> fsl_add_bridge() function?
>> I'm not sure I follow your thought on this quite yet. Do you mean the setup of
>> either of_device or platform_device or do you mean the actual pci err driver?
>> I'm not understanding how doing the setup code in fsl_add_bridge() would allow
>> us to prevent the of_device being monopolized by the EDAC driver or the future
>> PCI code.... The EDAC driver still needs some sort of "device" to claim one way
>> or another right?
> 
> What I meant is calling the edac_pci_alloc_ctl_info() function directly
> from fsl_add_bridge() where appropriate, with all the 85xx PCI EDAC
> code moved into fsl_pci.c.

I don't believe that EDAC core has been loaded at the time of 85xx PCI
initialization. Plus, the EDAC driver can be loaded as a kernel module. So that
probably won't work.... Also, instead of having centralized EDAC chip driver,
now you have things scattered over various places. One probably needs to add
83xx and 86xx code as well and whatever else eventually.

Maybe we are just better off adding entries in the DTS to get around this
problem....

-- 

------------------------------------------------------
Dave Jiang
Software Engineer
MontaVista Software, Inc.
http://www.mvista.com
------------------------------------------------------

Re: [PATCH 2/2] powerpc: MPC85xx EDAC device driver

[Arnd Bergmann]

On Monday 30 July 2007, Dave Jiang wrote:
> I don't believe that EDAC core has been loaded at the time of 85xx PCI
> initialization. Plus, the EDAC driver can be loaded as a kernel module. So that
> probably won't work....

ok, good point.

> Also, instead of having centralized EDAC chip driver, 
> now you have things scattered over various places. One probably needs to add
> 83xx and 86xx code as well and whatever else eventually.
> 
> Maybe we are just better off adding entries in the DTS to get around this
> problem....

The best solution may be to look at how it's structured at the
register level. If the PCI EDAC registers are implemented separately
from the regular PCI registers, a device tree entry would be appropriate.
If not, your idea of registering a platform_device from fsl_add_bridge
is probably more sensible.

	Arnd <><

Re: [PATCH 2/2] powerpc: MPC85xx EDAC device driver

[Dave Jiang]

Arnd Bergmann wrote:
> On Monday 30 July 2007, Dave Jiang wrote:
>> Maybe we are just better off adding entries in the DTS to get around this
>> problem....
> 
> The best solution may be to look at how it's structured at the
> register level. If the PCI EDAC registers are implemented separately
> from the regular PCI registers, a device tree entry would be appropriate.
> If not, your idea of registering a platform_device from fsl_add_bridge
> is probably more sensible.
> 

We can probably do either. From looking at the 8560 and 8548 manuals, the PCI
error registers are 0xe00 offset of the start of PCI registers. For example,
the PCI registers would start at 0x8000 offset. And the PCI error registers
would be at 0xe00 offset from there and would be the very last block of
registers. Currently the PCI dts devices seem to claim 0x1000 for size. So the
way to have the least code changes would be to register a platform_device from
fsl_add_bridge() or where appropriate for the 85xx PCI code. Most likely the
83xx and 86xx will have similar code and maybe we can do some reuse out of this.

I can add dts entries instead and pick up of_devices if Kumar prefers it that
way instead....

-- 

------------------------------------------------------
Dave Jiang
Software Engineer
MontaVista Software, Inc.
http://www.mvista.com
------------------------------------------------------

Re: [PATCH 2/2] powerpc: MPC85xx EDAC device driver

[Linas Vepstas]

On Mon, Jul 30, 2007 at 01:17:40PM -0700, Dave Jiang wrote:
> Arnd Bergmann wrote:
> > The best solution may be to look at how it's structured at the
> > register level. If the PCI EDAC registers are implemented separately
> > from the regular PCI registers, a device tree entry would be appropriate.
> > If not, your idea of registering a platform_device from fsl_add_bridge
> > is probably more sensible.
> > 
> 
> We can probably do either. From looking at the 8560 and 8548 manuals, the PCI
> error registers are 0xe00 offset of the start of PCI registers. For example,
> the PCI registers would start at 0x8000 offset. And the PCI error registers
> would be at 0xe00 offset from there and would be the very last block of
> registers. 

Anywhere I can easily get an overview of these "PCI error registers"?

Also: please note that the linux kernel has a pci error recovery
mechanism built in; its used by pseries and PCI-E. I'm not clear
on what any of this has to do with EDAC, which I thought was supposed 
to be for RAM only. (The EDAC project once talked about doing pci error 
recovery, but that was years ago, and there is a separate system for
that, now.)

--linas

Re: [PATCH 2/2] powerpc: MPC85xx EDAC device driver

[Doug Thompson]

--- Linas Vepstas <linas@austin.ibm.com> wrote:

> On Mon, Jul 30, 2007 at 01:17:40PM -0700, Dave Jiang wrote:
> > Arnd Bergmann wrote:
> > > The best solution may be to look at how it's structured at the
> > > register level. If the PCI EDAC registers are implemented separately
> > > from the regular PCI registers, a device tree entry would be appropriate.
> > > If not, your idea of registering a platform_device from fsl_add_bridge
> > > is probably more sensible.
> > > 
> > 
> > We can probably do either. From looking at the 8560 and 8548 manuals, the PCI
> > error registers are 0xe00 offset of the start of PCI registers. For example,
> > the PCI registers would start at 0x8000 offset. And the PCI error registers
> > would be at 0xe00 offset from there and would be the very last block of
> > registers. 
> 
> Anywhere I can easily get an overview of these "PCI error registers"?
> 
> Also: please note that the linux kernel has a pci error recovery
> mechanism built in; its used by pseries and PCI-E. I'm not clear
> on what any of this has to do with EDAC, which I thought was supposed 
> to be for RAM only. (The EDAC project once talked about doing pci error 
> recovery, but that was years ago, and there is a separate system for
> that, now.)

no, edac can/does harvest PCI bus errors, via polling and other hardware error detectors.

The pci error recovery added a couple of NEW device callback functions in the driver interface, 
which the bus layer can call to notify drivers that a PCI bus error occurred. Then the driver can
do some action on the event.

But at the current time, few PCI device drivers initialize those callback functions and
thus errors are lost and some IO transactions fail.

Over time, as drivers get updated (might take some time) then drivers
can take some sort of action FOR THEMSELVES

Yet, there is no tracking of errors - except for a log message in the log file.

There is NO meter on frequency of errors, etc. One must grep the log file and that is not a very
cycle friendly mechanism.

The reason I added PCI parity/error device scanning, was that when I was at Linux Networx, we had
parity errors on the PCI-X bus, but didn't know the cause.  After we discovered that a simple
PCI-X riser card had manufacturing problems (quality) and didn't drive lines properly, it caused
parity errors. This feature allowed us to track nodes that were having parity problems, but we had
no METER to know it.

Recovery is a good thing, BUT how do you know you having LOTS of errors/recovery events? You need
a meter. EDAC provides that METER

I met with Yanmin Zhang of Intel at OLS after his paper presentation on PCI Express Advanced Error
Reporting in the Kernel, and we talked about this same thing. I am talking with him on having the
recovery code present information into EDAC sysfs area. (hopefully, anyway)

The recovery generates log messages BUT having to periodically 'grep' the log file looking for
errors is not a good use of CPU cycles. grep once for a count and then grep later for a count and
then compare the counts for a delta count per unit time. ugly.

The EDAC solution is to be able to have a Listener thread in user space that can be notified (via
poll()) that an event has occurred.

There are more than one consumer (error recover) of error events:

1) driver recovery after a transaction (which is the recovery consumer above)
2) Management agents for health of a node
3) Maintainance agents for predictive component replacement

Rates of change of errors can be gathered as well.

EDAC allows for presentation of error counts via sysfs entries, from which user space
programs can harvest for over-time profiling

We have MEMORY (edac_mc) devices for chipsets now, but via the new edac_device class, such things
as ECC error tracking on DMA error checkers, FABRIC switchs, L1 and L2 cache ECC events, core CPU
data ECC checkers, etc can be done. I have an out of kernel tree MIPS driver do just this. Other
types of harvesters can be generated as well for other and/or new hardware error detectors.

doug thompson

> 
> --linas
> 
> 
> 

Re: [PATCH 2/2] powerpc: MPC85xx EDAC device driver

[Dave Jiang]

Linas Vepstas wrote:
> On Mon, Jul 30, 2007 at 01:17:40PM -0700, Dave Jiang wrote:
>> Arnd Bergmann wrote:
>>> The best solution may be to look at how it's structured at the
>>> register level. If the PCI EDAC registers are implemented separately
>>> from the regular PCI registers, a device tree entry would be appropriate.
>>> If not, your idea of registering a platform_device from fsl_add_bridge
>>> is probably more sensible.
>>>
>> We can probably do either. From looking at the 8560 and 8548 manuals, the PCI
>> error registers are 0xe00 offset of the start of PCI registers. For example,
>> the PCI registers would start at 0x8000 offset. And the PCI error registers
>> would be at 0xe00 offset from there and would be the very last block of
>> registers. 
> 
> Anywhere I can easily get an overview of these "PCI error registers"?

http://www.freescale.com/files/32bit/doc/ref_manual/MPC8548ERM.pdf?fsrch=1
Page 966. Section 16.3.1.4.
Is this what you mean?

Re: [PATCH 2/2] powerpc: MPC85xx EDAC device driver

[Dave Jiang]

Arnd Bergmann wrote:
> On Monday 30 July 2007, Dave Jiang wrote:
>> I don't believe that EDAC core has been loaded at the time of 85xx PCI
>> initialization. Plus, the EDAC driver can be loaded as a kernel module. So that
>> probably won't work....
> 
> ok, good point.
> 
>> Also, instead of having centralized EDAC chip driver, 
>> now you have things scattered over various places. One probably needs to add
>> 83xx and 86xx code as well and whatever else eventually.
>>
>> Maybe we are just better off adding entries in the DTS to get around this
>> problem....
> 
> The best solution may be to look at how it's structured at the
> register level. If the PCI EDAC registers are implemented separately
> from the regular PCI registers, a device tree entry would be appropriate.
> If not, your idea of registering a platform_device from fsl_add_bridge
> is probably more sensible.

Actually it seems that for me to grab the interrupt number I have to do the
platform device creation in fsl_soc.c and call arch_init() instead of doing it
from fsl_add_bridge(). fsl_add_bridge() is called way too early and the mpic
interrupt mapping has not been setup yet for me to acquire the interrupt number
from of_interrupt_to_resource() call.

-- 

------------------------------------------------------
Dave Jiang
Software Engineer
MontaVista Software, Inc.
http://www.mvista.com
------------------------------------------------------

Re: [PATCH 2/2] powerpc: MPC85xx EDAC device driver

[Arnd Bergmann]

On Tuesday 31 July 2007, Dave Jiang wrote:
> Actually it seems that for me to grab the interrupt number I have to do the
> platform device creation in fsl_soc.c and call arch_init() instead of doing it
> from fsl_add_bridge(). fsl_add_bridge() is called way too early and the mpic
> interrupt mapping has not been setup yet for me to acquire the interrupt number
> from of_interrupt_to_resource() call.

I think in general, it would be a better solution to have the add_bridge
code called much later, like we do with the 64 bit PCI code when
using an of_device for the PCI bridge. I don't think it's easy to
do though, so that shouldn't stop you from doing it the other way.

Please make the platform_device a child of the PCI host bridge though,
so it doesn't need to change if/when the PCI initialization gets changed
to allow doing it from fsl_pci.

	Arnd <><

Re: [PATCH 2/2] powerpc: MPC85xx EDAC device driver

[Dave Jiang]

Arnd Bergmann wrote:
> On Tuesday 31 July 2007, Dave Jiang wrote:
>> Actually it seems that for me to grab the interrupt number I have to do the
>> platform device creation in fsl_soc.c and call arch_init() instead of doing it
>> from fsl_add_bridge(). fsl_add_bridge() is called way too early and the mpic
>> interrupt mapping has not been setup yet for me to acquire the interrupt number
>> from of_interrupt_to_resource() call.
> 
> I think in general, it would be a better solution to have the add_bridge
> code called much later, like we do with the 64 bit PCI code when
> using an of_device for the PCI bridge. I don't think it's easy to
> do though, so that shouldn't stop you from doing it the other way.
> 
> Please make the platform_device a child of the PCI host bridge though,
> so it doesn't need to change if/when the PCI initialization gets changed
> to allow doing it from fsl_pci.
> 

Doh! I sent out the reworked patches right before your comments. Do you happen
to know where I can find an example of how to do this? In regards to making a
platform_device a child of the PCI host bridge that is.... Thanks!

-- 

------------------------------------------------------
Dave Jiang
Software Engineer
MontaVista Software, Inc.
http://www.mvista.com
------------------------------------------------------

Re: [PATCH 2/2] powerpc: MPC85xx EDAC device driver

[Arnd Bergmann]

On Wednesday 01 August 2007, Dave Jiang wrote:
> Doh! I sent out the reworked patches right before your comments. Do you happen
> to know where I can find an example of how to do this? In regards to making a
> platform_device a child of the PCI host bridge that is.... Thanks!
> 

When you use an open-coded version of platform_device_register_simple(),
you can set dev->parent before calling platform_device_add.

	Arnd <><

Re: [PATCH 2/2] powerpc: MPC85xx EDAC device driver

[Dave Jiang]

Arnd Bergmann wrote:
> On Wednesday 01 August 2007, Dave Jiang wrote:
>> Doh! I sent out the reworked patches right before your comments. Do you happen
>> to know where I can find an example of how to do this? In regards to making a
>> platform_device a child of the PCI host bridge that is.... Thanks!
>>
> 
> When you use an open-coded version of platform_device_register_simple(),
> you can set dev->parent before calling platform_device_add.
> 
> 	Arnd <><

Do you mean the pci_controller hose that's allocated in fsl_add_bridge? Couple
questions....
1. How do I even get access to that? (in fsl_soc.c that is)
2. pci_controller has no "struct device" member. It only has "struct device
*parent". What do I set the dev->parent of the platform device to?


-- 

------------------------------------------------------
Dave Jiang
Software Engineer
MontaVista Software, Inc.
http://www.mvista.com
------------------------------------------------------

Re: [PATCH 2/2] powerpc: MPC85xx EDAC device driver

[Arnd Bergmann]

On Wednesday 01 August 2007, Dave Jiang wrote:
> Do you mean the pci_controller hose that's allocated in fsl_add_bridge? Couple
> questions....
> 1. How do I even get access to that? (in fsl_soc.c that is)
> 2. pci_controller has no "struct device" member. It only has "struct device
> *parent". What do I set the dev->parent of the platform device to?

You already have the device_node, and can simply call
of_find_device_by_node() on it to get the of_device.

The current fsl_pci code does not use the of_device of the hose but
adds the hose at the top of the device tree, afaics, but that can
change when the ppc32 PCI initialization gets changed to do the
initialization later during boot.

	Arnd <><

EDAC & PCI error recovery (was Re: [PATCH 2/2] powerpc: MPC85xx EDAC device driver)

[Linas Vepstas]

On Mon, Jul 30, 2007 at 03:47:05PM -0700, Doug Thompson wrote:
> 
> --- Linas Vepstas <linas@austin.ibm.com> wrote:
> > Also: please note that the linux kernel has a pci error recovery
> > mechanism built in; its used by pseries and PCI-E. I'm not clear
> > on what any of this has to do with EDAC, which I thought was supposed 
> > to be for RAM only. (The EDAC project once talked about doing pci error 
> > recovery, but that was years ago, and there is a separate system for
> > that, now.)
> 
> no, edac can/does harvest PCI bus errors, via polling and other hardware error detectors.

Ehh! I had no idea. A few years ago, when I was working on the PCI error
recovery, I sent a number of emails to the various EDAC people and mailing 
lists that I could find, and never got a response.  I assumed the
project was dead. I guess its not ... 

> But at the current time, few PCI device drivers initialize those callback functions and
> thus errors are lost and some IO transactions fail.

There are patches for 6 drivers in mainline (e100, e1000, ixgb, s2io,
ipr, lpfc), and two more pending (sym53cxxx, tg3).  So far, I've written 
all of them. 

> Over time, as drivers get updated (might take some time) then drivers
> can take some sort of action FOR THEMSELVES

I think I need to do more to raise awareness and interest.

> Yet, there is no tracking of errors - except for a log message in the log file.
> 
> There is NO meter on frequency of errors, etc. One must grep the log file and that is not a very
> cycle friendly mechanism.

Yeah, there was low interest in stats. There's a core set of stats in
/proc/pp64/eeh, but these are clearly arch-specific. I'd ike to move
away from those.  Some recent patches added stats to the /sys tree,
under the individual pci bridge and device nodes.  Again, these are
arch-specific; I'd like to move to some geeral/standardized presentation.

> The reason I added PCI parity/error device scanning, was that when I was at Linux Networx, we had
> parity errors on the PCI-X bus, but didn't know the cause.  After we discovered that a simple
> PCI-X riser card had manufacturing problems (quality) and didn't drive lines properly, it caused
> parity errors. 

Heh. Not unusual. I've seen/heard of cases with voltages being low,
and/or ground-bounce in slots near the end. There's a whole zoo of
hardware/firmware bugs that we've had to painfully crawl through and
fix. That's why the IBM boxes cost big $$$; here's to hoping that 
customers understand why.

> This feature allowed us to track nodes that were having parity problems, but we had
> no METER to know it.
> 
> Recovery is a good thing, BUT how do you know you having LOTS of errors/recovery events? You need
> a meter. EDAC provides that METER

I'm lazy. What source code should I be looking at?  I'm concerned about
duplication of function and proliferation of interfaces. I've got my 
metering data under (for example)
/sys/bus/pci/devices/0001:c0:01.0/eeh_*, mostly very arch specific.
The code for this is in arch/powerpc/platforms/pseries/eeh_sysfs.c

> I met with Yanmin Zhang of Intel at OLS after his paper presentation on PCI Express Advanced Error
> Reporting in the Kernel, and we talked about this same thing. I am talking with him on having the
> recovery code present information into EDAC sysfs area. (hopefully, anyway)

Hmm. OK, where's that?  Back when, I'd talked to Yamin about coming up 
with a generic, arch-indep way of driving the recovery routines. But
this wasn't exactly easy, and we were still grappling with just getting
things working.  Now that things are working, its time to broaden
horizons.

Can you point me to the current edac code?
find . -print |grep edac is not particuarly revealing at the moment.

> The recovery generates log messages BUT having to periodically 'grep' the log file looking for
> errors is not a good use of CPU cycles. grep once for a count and then grep later for a count and
> then compare the counts for a delta count per unit time. ugly.

Yep. Maybe send events up to udev?

> The EDAC solution is to be able to have a Listener thread in user space that can be notified (via
> poll()) that an event has occurred.

Hmm. OK, I'm alarmingly nave about udev, but my initial gut instinct is
to pipe all such events to udev. Most of user-space has already been
given the marching orders to use udev and/or hal for this kind of stuff.
So this makes sense to me.

> There are more than one consumer (error recover) of error events:
> 1) driver recovery after a transaction (which is the recovery consumer above)

I had to argue loudly for recovery in the kernel. The problem was that
it was impossible to recover erros on scsi devics from userspace (since
the block device and filesystems would go bonkers).

> 2) Management agents for health of a node
> 3) Maintainance agents for predictive component replacement

Yes, agreed. Care to ask your management agent friends for where they'd
like to get these events from (i.e. udev, or somewhere else?)

> We have MEMORY (edac_mc) devices for chipsets now, but via the new edac_device class, such things
> as ECC error tracking on DMA error checkers, FABRIC switchs, L1 and L2 cache ECC events, core CPU
> data ECC checkers, etc can be done. I have an out of kernel tree MIPS driver do just this. Other
> types of harvesters can be generated as well for other and/or new hardware error detectors.

Ohh. I've got hardware tha does this, but its not currently usng EDAC.
There must be some edac mailing list I'm not subscribed to??

--linas

Re: EDAC stats & PCI error recovery (was Re: [PATCH 2/2] powerpc: MPC85xx EDAC device driver)

[Doug Thompson]

--- Linas Vepstas <linas@austin.ibm.com> wrote:

> On Mon, Jul 30, 2007 at 03:47:05PM -0700, Doug Thompson wrote:
> > 
> > --- Linas Vepstas <linas@austin.ibm.com> wrote:
> > > Also: please note that the linux kernel has a pci error recovery
> > > mechanism built in; its used by pseries and PCI-E. I'm not clear
> > > on what any of this has to do with EDAC, which I thought was supposed 
> > > to be for RAM only. (The EDAC project once talked about doing pci error 
> > > recovery, but that was years ago, and there is a separate system for
> > > that, now.)
> > 
> > no, edac can/does harvest PCI bus errors, via polling and other hardware error detectors.
> 
> Ehh! I had no idea. A few years ago, when I was working on the PCI error
> recovery, I sent a number of emails to the various EDAC people and mailing 
> lists that I could find, and never got a response.  I assumed the
> project was dead. I guess its not ... 

No its not, just some company lay offs stirred the pot, at least for me, for awhile.
I did see the ibm patches go by, but didn't have the time to check up at that time. I actually,
didn't know the recovery interface had gotten into the kernel (My failure to watch for them), so I
was pleasantly surprised at this last OLS to attend the presentation.

> 
> > But at the current time, few PCI device drivers initialize those callback functions and
> > thus errors are lost and some IO transactions fail.
> 
> There are patches for 6 drivers in mainline (e100, e1000, ixgb, s2io,
> ipr, lpfc), and two more pending (sym53cxxx, tg3).  So far, I've written 
> all of them. 

Great.
EDAC does nothing for recovery, just logging and stats gathering and presentation.

> 
> > Over time, as drivers get updated (might take some time) then drivers
> > can take some sort of action FOR THEMSELVES
> 
> I think I need to do more to raise awareness and interest.

good point

> 
> > Yet, there is no tracking of errors - except for a log message in the log file.
> > 
> > There is NO meter on frequency of errors, etc. One must grep the log file and that is not a
> very
> > cycle friendly mechanism.
> 
> Yeah, there was low interest in stats. There's a core set of stats in
> /proc/pp64/eeh, but these are clearly arch-specific. I'd ike to move
> away from those.  Some recent patches added stats to the /sys tree,
> under the individual pci bridge and device nodes.  Again, these are
> arch-specific; I'd like to move to some geeral/standardized presentation.

the memory error consumers really like the stats of EDAC. Allows them to track trends.
Cluster types, with thousands of nodes, like the monitoring for both memory and PCI, as well as
some newer hardware detector harvesting.

> 
> > The reason I added PCI parity/error device scanning, was that when I was at Linux Networx, we
> had
> > parity errors on the PCI-X bus, but didn't know the cause.  After we discovered that a simple
> > PCI-X riser card had manufacturing problems (quality) and didn't drive lines properly, it
> caused
> > parity errors. 
> 
> Heh. Not unusual. I've seen/heard of cases with voltages being low,
> and/or ground-bounce in slots near the end. There's a whole zoo of
> hardware/firmware bugs that we've had to painfully crawl through and
> fix. That's why the IBM boxes cost big $$$; here's to hoping that 
> customers understand why.

I understand


> 
> > This feature allowed us to track nodes that were having parity problems, but we had
> > no METER to know it.
> > 
> > Recovery is a good thing, BUT how do you know you having LOTS of errors/recovery events? You
> need
> > a meter. EDAC provides that METER
> 
> I'm lazy. What source code should I be looking at?  I'm concerned about
> duplication of function and proliferation of interfaces. I've got my 
> metering data under (for example)
> /sys/bus/pci/devices/0001:c0:01.0/eeh_*, mostly very arch specific.
> The code for this is in arch/powerpc/platforms/pseries/eeh_sysfs.c

http://bluesmoke.sourceforge.net/

is the SF project zone (bluesmoke was the out-of-tree name, changed to EDAC when it came into
tree, and source forge doesn't allow renaming)

EDAC info is under:

/sys/devices/system/edac/....

mc for memory controllers
pci for pci info.

very basic, just counters and some controls


> 
> > I met with Yanmin Zhang of Intel at OLS after his paper presentation on PCI Express Advanced
> Error
> > Reporting in the Kernel, and we talked about this same thing. I am talking with him on having
> the
> > recovery code present information into EDAC sysfs area. (hopefully, anyway)
> 
> Hmm. OK, where's that?  Back when, I'd talked to Yamin about coming up 
> with a generic, arch-indep way of driving the recovery routines. But
> this wasn't exactly easy, and we were still grappling with just getting
> things working.  Now that things are working, its time to broaden
> horizons.

Not very far, but I see the potential.
When EDAC was received, it was placed where it was in the sysfs from various kernel developers as
a good spot on its own.

> 
> Can you point me to the current edac code?
> find . -print |grep edac is not particuarly revealing at the moment.

drivers/edac

latest is in 2.6.23-rc1.   

-rc2 will have a few vital bug fixes. This release is fairly large since 2.6.26 when it first was
in the tree.

> 
> > The recovery generates log messages BUT having to periodically 'grep' the log file looking for
> > errors is not a good use of CPU cycles. grep once for a count and then grep later for a count
> and
> > then compare the counts for a delta count per unit time. ugly.
> 
> Yep. Maybe send events up to udev?

That is a possibility, yet EDAC consumers like the stats being available as well.

> 
> > The EDAC solution is to be able to have a Listener thread in user space that can be notified
> (via
> > poll()) that an event has occurred.
> 
> Hmm. OK, I'm alarmingly nave about udev, but my initial gut instinct is
> to pipe all such events to udev. Most of user-space has already been
> given the marching orders to use udev and/or hal for this kind of stuff.
> So this makes sense to me.

I need a learning process as well on udev.
It being the unified highway of event notification from kernel to user space. Although, when
memory errors fires, sometimes they can generate MASSIVE Number of events on every memory access.
PCI being different might have different constraints. Each needs investigation and classification
of timing, etc.

> 
> > There are more than one consumer (error recover) of error events:
> > 1) driver recovery after a transaction (which is the recovery consumer above)
> 
> I had to argue loudly for recovery in the kernel. The problem was that
> it was impossible to recover erros on scsi devics from userspace (since
> the block device and filesystems would go bonkers).

I hear you. It took the cluster consumers to demand ROBUST "meters" to for their machines to give
'validation' (so to speak) of their calculations. Bluesmoke/EDAC became that in our sandbox.

> 
> > 2) Management agents for health of a node
> > 3) Maintainance agents for predictive component replacement
> 
> Yes, agreed. Care to ask your management agent friends for where they'd
> like to get these events from (i.e. udev, or somewhere else?)

They are in a learning curve as well.
Error event processing is still so "young" under linux. Getting people aware of it is the current
push.

> 
> > We have MEMORY (edac_mc) devices for chipsets now, but via the new edac_device class, such
> things
> > as ECC error tracking on DMA error checkers, FABRIC switchs, L1 and L2 cache ECC events, core
> CPU
> > data ECC checkers, etc can be done. I have an out of kernel tree MIPS driver do just this.
> Other
> > types of harvesters can be generated as well for other and/or new hardware error detectors.
> 
> Ohh. I've got hardware tha does this, but its not currently usng EDAC.
> There must be some edac mailing list I'm not subscribed to??

the new edac_device class  rolled out in 2.6.23-rc1
No in tree drivers, but I have a test_device_edac module that uses it and it is in the svn repos.

I need to update the edac tarball to contain this stuff.

for mailing list, goto 

http://bluesmoke.sourceforge.net/

find the subscribe to mailing list action item. Fairly low bandwidth, but has notices, and some
discussion on things now and then.

bluesmoke-devel@lists.sourceforge.net is the development email


doug t

> 
> --linas
> 
>