2018-10-15 05:32:11

by Manish Narani

[permalink] [raw]
Subject: [PATCH v9 0/6] EDAC: Enhancements to Synopsys EDAC driver

This patch series enhances the current EDAC driver to support different
platforms. This series adds support for ZynqMP DDRC controller in synopsys
EDAC driver. This series also adds Device tree properties and relevant
binding documentation.

Changes in v2:
- Moved checking of DDR_ECC_INTR_SUPPORT from (1/4) to (3/4) as it is
a feature of ZynqMP DDRC
- The Binding Documentation in (2/4) is modified as per the review
comments

Changes in v3:
- The commit message in (2/4) is modified (Synopsys EDAC Driver -->
ZynqMP DDRC)

Changes in v4:
- Updated the commit message in (1/4)
- Renamed function pointer names removing 'synps_' in (1/4)
- Shortened unnecessary long lines as per the review comment on (1/4)

Changes in v5:
- Updated the commit message in (2/4) and (4/4).
- Removed the unnecessary check for match data in probe() in (1/4)
- Some Indentation changes for better readability in (1/4) and (3/4)
- Removed repeated code in (3/4)
- Used 'zynq' and 'zynqmp' instead of 'synps_enh_edac' in function names

Changes in v6:
- Splitted the patches according to functionalities
- Addressed code style comments from v5 review
- Moved the Error Injection to CONFIG_EDAC_DEBUG mode

Changes in v7:
- Included DTS patch (6/7) which was missed in v6 patch set

Changes in v8:
- patch (1/7) from v7 is split in to 3 different logically different patches
1. functional changes like code cleanup
2. functions renaming
3. comments cleanup
- Added a separate patch (4) for making always successful functions as void
- Corrected 'Too many parentheses' review comment in patch (5)
- Corrected comments as per the v7 review feedback
- Made dedicated functions for IRQ setup, IRQ enable and IRQ disable in patch (8)
- Addressed review comments in patch (10)

Changes in v9:
- Added check for return value of of_device_get_match_data() function
in (1/6).
- From v8 the first 5 patches are removed in this series as they are
applied on:
https://git.kernel.org/pub/scm/linux/kernel/git/bp/bp.git/?h=edac-for-4.20-synps
- Updated Kconfig to check for ARCH_ZYNQMP instead of ARM64

Manish Narani (6):
edac: synopsys: Add error handling for NULL in probe()
dt: bindings: Document ZynqMP DDRC in Synopsys documentation
edac: synopsys: Add macro defines for ZynqMP DDRC
edac: synopsys: Add EDAC ECC support for ZynqMP DDRC
arm64: zynqmp: Add DDRC node
edac: synopsys: Add Error Injection support for ZynqMP DDRC

.../bindings/memory-controllers/synopsys.txt | 27 +-
arch/arm64/boot/dts/xilinx/zynqmp.dtsi | 7 +
drivers/edac/Kconfig | 2 +-
drivers/edac/synopsys_edac.c | 913 ++++++++++++++++++++-
4 files changed, 920 insertions(+), 29 deletions(-)

--
2.1.1



2018-10-15 05:31:26

by Manish Narani

[permalink] [raw]
Subject: [PATCH v9 5/6] arm64: zynqmp: Add DDRC node

Add ddrc memory controller node in dts. The size mentioned in dts is
0x30000, because we need to access DDR_QOS INTR registers located at
0xFD090208 from this driver.

Signed-off-by: Manish Narani <[email protected]>
---
arch/arm64/boot/dts/xilinx/zynqmp.dtsi | 7 +++++++
1 file changed, 7 insertions(+)

diff --git a/arch/arm64/boot/dts/xilinx/zynqmp.dtsi b/arch/arm64/boot/dts/xilinx/zynqmp.dtsi
index 29ce234..a81d3b16 100644
--- a/arch/arm64/boot/dts/xilinx/zynqmp.dtsi
+++ b/arch/arm64/boot/dts/xilinx/zynqmp.dtsi
@@ -355,6 +355,13 @@
xlnx,bus-width = <64>;
};

+ mc: memory-controller@fd070000 {
+ compatible = "xlnx,zynqmp-ddrc-2.40a";
+ reg = <0x0 0xfd070000 0x0 0x30000>;
+ interrupt-parent = <&gic>;
+ interrupts = <0 112 4>;
+ };
+
gem0: ethernet@ff0b0000 {
compatible = "cdns,zynqmp-gem", "cdns,gem";
status = "disabled";
--
2.1.1


2018-10-15 05:31:26

by Manish Narani

[permalink] [raw]
Subject: [PATCH v9 1/6] edac: synopsys: Add error handling for NULL in probe()

The function of_device_get_match_data() can return NULL in case of
error. Add error handling for the same in probe().

Signed-off-by: Manish Narani <[email protected]>
---
drivers/edac/synopsys_edac.c | 3 +++
1 file changed, 3 insertions(+)

diff --git a/drivers/edac/synopsys_edac.c b/drivers/edac/synopsys_edac.c
index 1c3795d..0005ef3 100644
--- a/drivers/edac/synopsys_edac.c
+++ b/drivers/edac/synopsys_edac.c
@@ -477,6 +477,9 @@ static int mc_probe(struct platform_device *pdev)
return PTR_ERR(baseaddr);

p_data = of_device_get_match_data(&pdev->dev);
+ if (!p_data)
+ return -ENODEV;
+
if (!p_data->get_ecc_state(baseaddr)) {
edac_printk(KERN_INFO, EDAC_MC, "ECC not enabled\n");
return -ENXIO;
--
2.1.1


2018-10-15 05:31:26

by Manish Narani

[permalink] [raw]
Subject: [PATCH v9 4/6] edac: synopsys: Add EDAC ECC support for ZynqMP DDRC

Add EDAC ECC support for ZynqMP DDRC IP. The IP supports interrupts for
corrected and uncorrected errors. Add interrupt handlers for the same.

Signed-off-by: Manish Narani <[email protected]>
---
drivers/edac/Kconfig | 2 +-
drivers/edac/synopsys_edac.c | 324 ++++++++++++++++++++++++++++++++++++++++---
2 files changed, 308 insertions(+), 18 deletions(-)

diff --git a/drivers/edac/Kconfig b/drivers/edac/Kconfig
index 57304b2..7c40eb2 100644
--- a/drivers/edac/Kconfig
+++ b/drivers/edac/Kconfig
@@ -441,7 +441,7 @@ config EDAC_ALTERA_SDMMC

config EDAC_SYNOPSYS
tristate "Synopsys DDR Memory Controller"
- depends on ARCH_ZYNQ
+ depends on ARCH_ZYNQ || ARCH_ZYNQMP
help
Support for error detection and correction on the Synopsys DDR
memory controller.
diff --git a/drivers/edac/synopsys_edac.c b/drivers/edac/synopsys_edac.c
index d1999e0..603c4bd 100644
--- a/drivers/edac/synopsys_edac.c
+++ b/drivers/edac/synopsys_edac.c
@@ -22,6 +22,7 @@
#include <linux/edac.h>
#include <linux/module.h>
#include <linux/platform_device.h>
+#include <linux/interrupt.h>
#include <linux/of.h>
#include <linux/of_device.h>

@@ -272,6 +273,8 @@
* @bank: Bank number.
* @bitpos: Bit position.
* @data: Data causing the error.
+ * @bankgrpnr: Bank group number.
+ * @blknr: Block number.
*/
struct ecc_error_info {
u32 row;
@@ -279,6 +282,8 @@ struct ecc_error_info {
u32 bank;
u32 bitpos;
u32 data;
+ u32 bankgrpnr;
+ u32 blknr;
};

/**
@@ -385,6 +390,66 @@ static int zynq_get_error_info(struct synps_edac_priv *priv)
}

/**
+ * zynqmp_get_error_info - Get the current ECC error info.
+ * @priv: DDR memory controller private instance data.
+ *
+ * Return: one if there is no error otherwise returns zero.
+ */
+static int zynqmp_get_error_info(struct synps_edac_priv *priv)
+{
+ struct synps_ecc_status *p;
+ u32 regval, clearval = 0;
+ void __iomem *base;
+
+ base = priv->baseaddr;
+ p = &priv->stat;
+
+ regval = readl(base + ECC_STAT_OFST);
+ if (!regval)
+ return 1;
+
+ p->ce_cnt = (regval & ECC_STAT_CECNT_MASK) >> ECC_STAT_CECNT_SHIFT;
+ p->ue_cnt = (regval & ECC_STAT_UECNT_MASK) >> ECC_STAT_UECNT_SHIFT;
+ p->ceinfo.bitpos = (regval & ECC_STAT_BITNUM_MASK);
+
+ regval = readl(base + ECC_CEADDR0_OFST);
+ if (!p->ce_cnt)
+ goto ue_err;
+
+ p->ceinfo.row = (regval & ECC_CEADDR0_RW_MASK);
+ regval = readl(base + ECC_CEADDR1_OFST);
+ p->ceinfo.bank = (regval & ECC_CEADDR1_BNKNR_MASK) >>
+ ECC_CEADDR1_BNKNR_SHIFT;
+ p->ceinfo.bankgrpnr = (regval & ECC_CEADDR1_BNKGRP_MASK) >>
+ ECC_CEADDR1_BNKGRP_SHIFT;
+ p->ceinfo.blknr = (regval & ECC_CEADDR1_BLKNR_MASK);
+ p->ceinfo.data = readl(base + ECC_CSYND0_OFST);
+ edac_dbg(2, "ECCCSYN0: 0x%08X ECCCSYN1: 0x%08X ECCCSYN2: 0x%08X\n",
+ readl(base + ECC_CSYND0_OFST), readl(base + ECC_CSYND1_OFST),
+ readl(base + ECC_CSYND2_OFST));
+ue_err:
+ regval = readl(base + ECC_UEADDR0_OFST);
+ if (!p->ue_cnt)
+ goto out;
+
+ p->ueinfo.row = (regval & ECC_CEADDR0_RW_MASK);
+ regval = readl(base + ECC_UEADDR1_OFST);
+ p->ueinfo.bankgrpnr = (regval & ECC_CEADDR1_BNKGRP_MASK) >>
+ ECC_CEADDR1_BNKGRP_SHIFT;
+ p->ueinfo.bank = (regval & ECC_CEADDR1_BNKNR_MASK) >>
+ ECC_CEADDR1_BNKNR_SHIFT;
+ p->ueinfo.blknr = (regval & ECC_CEADDR1_BLKNR_MASK);
+ p->ueinfo.data = readl(base + ECC_UESYND0_OFST);
+out:
+ clearval = ECC_CTRL_CLR_CE_ERR | ECC_CTRL_CLR_CE_ERRCNT;
+ clearval |= ECC_CTRL_CLR_UE_ERR | ECC_CTRL_CLR_UE_ERRCNT;
+ writel(clearval, base + ECC_CLR_OFST);
+ writel(0x0, base + ECC_CLR_OFST);
+
+ return 0;
+}
+
+/**
* handle_error - Handle Correctable and Uncorrectable errors.
* @mci: EDAC memory controller instance.
* @p: Synopsys ECC status structure.
@@ -398,9 +463,25 @@ static void handle_error(struct mem_ctl_info *mci, struct synps_ecc_status *p)

if (p->ce_cnt) {
pinf = &p->ceinfo;
- snprintf(priv->message, SYNPS_EDAC_MSG_SIZE,
- "DDR ECC error type :%s Row %d Bank %d Col %d ",
- "CE", pinf->row, pinf->bank, pinf->col);
+ if (!priv->p_data->quirks) {
+ snprintf(priv->message, SYNPS_EDAC_MSG_SIZE,
+ "DDR ECC error type:%s Row %d Bank %d Col %d ",
+ "CE", pinf->row, pinf->bank, pinf->col);
+ snprintf(priv->message, SYNPS_EDAC_MSG_SIZE,
+ "Bit Position: %d Data: 0x%08x\n",
+ pinf->bitpos, pinf->data);
+ } else {
+ snprintf(priv->message, SYNPS_EDAC_MSG_SIZE,
+ "DDR ECC error type:%s Row %d Bank %d Col %d ",
+ "CE", pinf->row, pinf->bank, pinf->col);
+ snprintf(priv->message, SYNPS_EDAC_MSG_SIZE,
+ "BankGroup Number %d Block Number %d ",
+ pinf->bankgrpnr, pinf->blknr);
+ snprintf(priv->message, SYNPS_EDAC_MSG_SIZE,
+ "Bit Position: %d Data: 0x%08x\n",
+ pinf->bitpos, pinf->data);
+ }
+
edac_mc_handle_error(HW_EVENT_ERR_CORRECTED, mci,
p->ce_cnt, 0, 0, 0, 0, 0, -1,
priv->message, "");
@@ -408,9 +489,19 @@ static void handle_error(struct mem_ctl_info *mci, struct synps_ecc_status *p)

if (p->ue_cnt) {
pinf = &p->ueinfo;
- snprintf(priv->message, SYNPS_EDAC_MSG_SIZE,
- "DDR ECC error type :%s Row %d Bank %d Col %d ",
- "UE", pinf->row, pinf->bank, pinf->col);
+ if (!priv->p_data->quirks) {
+ snprintf(priv->message, SYNPS_EDAC_MSG_SIZE,
+ "DDR ECC error type :%s Row %d Bank %d Col %d ",
+ "UE", pinf->row, pinf->bank, pinf->col);
+ } else {
+ snprintf(priv->message, SYNPS_EDAC_MSG_SIZE,
+ "DDR ECC error type :%s Row %d Bank %d Col %d ",
+ "UE", pinf->row, pinf->bank, pinf->col);
+ snprintf(priv->message, SYNPS_EDAC_MSG_SIZE,
+ "BankGroup Number %d Block Number %d",
+ pinf->bankgrpnr, pinf->blknr);
+ }
+
edac_mc_handle_error(HW_EVENT_ERR_UNCORRECTED, mci,
p->ue_cnt, 0, 0, 0, 0, 0, -1,
priv->message, "");
@@ -420,6 +511,42 @@ static void handle_error(struct mem_ctl_info *mci, struct synps_ecc_status *p)
}

/**
+ * intr_handler - Interrupt Handler for ECC interrupts.
+ * @irq: IRQ number.
+ * @dev_id: Device ID.
+ *
+ * Return: IRQ_NONE, if interrupt not set or IRQ_HANDLED otherwise.
+ */
+static irqreturn_t intr_handler(int irq, void *dev_id)
+{
+ const struct synps_platform_data *p_data;
+ struct mem_ctl_info *mci = dev_id;
+ struct synps_edac_priv *priv;
+ int status, regval;
+
+ priv = mci->pvt_info;
+ p_data = priv->p_data;
+
+ regval = readl(priv->baseaddr + DDR_QOS_IRQ_STAT_OFST);
+ regval &= (DDR_QOSCE_MASK | DDR_QOSUE_MASK);
+ if (!(regval & ECC_CE_UE_INTR_MASK))
+ return IRQ_NONE;
+
+ status = p_data->get_error_info(priv);
+ if (status)
+ return IRQ_NONE;
+
+ priv->ce_cnt += priv->stat.ce_cnt;
+ priv->ue_cnt += priv->stat.ue_cnt;
+ handle_error(mci, &priv->stat);
+
+ edac_dbg(3, "Total error count CE %d UE %d\n",
+ priv->ce_cnt, priv->ue_cnt);
+ writel(regval, priv->baseaddr + DDR_QOS_IRQ_STAT_OFST);
+ return IRQ_HANDLED;
+}
+
+/**
* check_errors - Check controller for ECC errors.
* @mci: EDAC memory controller instance.
*
@@ -427,10 +554,13 @@ static void handle_error(struct mem_ctl_info *mci, struct synps_ecc_status *p)
*/
static void check_errors(struct mem_ctl_info *mci)
{
- struct synps_edac_priv *priv = mci->pvt_info;
- const struct synps_platform_data *p_data = priv->p_data;
+ const struct synps_platform_data *p_data;
+ struct synps_edac_priv *priv;
int status;

+ priv = mci->pvt_info;
+ p_data = priv->p_data;
+
status = p_data->get_error_info(priv);
if (status)
return;
@@ -475,6 +605,39 @@ static enum dev_type zynq_get_dtype(const void __iomem *base)
}

/**
+ * zynqmp_get_dtype - Return the controller memory width.
+ * @base: DDR memory controller base address.
+ *
+ * Get the EDAC device type width appropriate for the current controller
+ * configuration.
+ *
+ * Return: a device type width enumeration.
+ */
+static enum dev_type zynqmp_get_dtype(const void __iomem *base)
+{
+ enum dev_type dt;
+ u32 width;
+
+ width = readl(base + CTRL_OFST);
+ width = (width & ECC_CTRL_BUSWIDTH_MASK) >> ECC_CTRL_BUSWIDTH_SHIFT;
+ switch (width) {
+ case DDRCTL_EWDTH_16:
+ dt = DEV_X2;
+ break;
+ case DDRCTL_EWDTH_32:
+ dt = DEV_X4;
+ break;
+ case DDRCTL_EWDTH_64:
+ dt = DEV_X8;
+ break;
+ default:
+ dt = DEV_UNKNOWN;
+ }
+
+ return dt;
+}
+
+/**
* zynq_get_ecc_state - Return the controller ECC enable/disable status.
* @base: DDR memory controller base address.
*
@@ -484,19 +647,43 @@ static enum dev_type zynq_get_dtype(const void __iomem *base)
*/
static bool zynq_get_ecc_state(void __iomem *base)
{
- bool state = false;
enum dev_type dt;
u32 ecctype;

dt = zynq_get_dtype(base);
if (dt == DEV_UNKNOWN)
- return state;
+ return false;

ecctype = readl(base + SCRUB_OFST) & SCRUB_MODE_MASK;
if ((ecctype == SCRUB_MODE_SECDED) && (dt == DEV_X2))
- state = true;
+ return true;

- return state;
+ return false;
+}
+
+/**
+ * zynqmp_get_ecc_state - Return the controller ECC enable/disable status.
+ * @base: DDR memory controller base address.
+ *
+ * Get the ECC enable/disable status for the controller.
+ *
+ * Return: a ECC status boolean i.e true/false - enabled/disabled.
+ */
+static bool zynqmp_get_ecc_state(void __iomem *base)
+{
+ enum dev_type dt;
+ u32 ecctype;
+
+ dt = zynqmp_get_dtype(base);
+ if (dt == DEV_UNKNOWN)
+ return false;
+
+ ecctype = readl(base + ECC_CFG0_OFST) & SCRUB_MODE_MASK;
+ if ((ecctype == SCRUB_MODE_SECDED) &&
+ ((dt == DEV_X2) || (dt == DEV_X4) || (dt == DEV_X8)))
+ return true;
+
+ return false;
}

/**
@@ -538,6 +725,34 @@ static enum mem_type zynq_get_mtype(const void __iomem *base)
}

/**
+ * zynqmp_get_mtype - Returns controller memory type.
+ * @base: Synopsys ECC status structure.
+ *
+ * Get the EDAC memory type appropriate for the current controller
+ * configuration.
+ *
+ * Return: a memory type enumeration.
+ */
+static enum mem_type zynqmp_get_mtype(const void __iomem *base)
+{
+ enum mem_type mt;
+ u32 memtype;
+
+ memtype = readl(base + CTRL_OFST);
+
+ if ((memtype & MEM_TYPE_DDR3) || (memtype & MEM_TYPE_LPDDR3))
+ mt = MEM_DDR3;
+ else if (memtype & MEM_TYPE_DDR2)
+ mt = MEM_RDDR2;
+ else if ((memtype & MEM_TYPE_LPDDR4) || (memtype & MEM_TYPE_DDR4))
+ mt = MEM_DDR4;
+ else
+ mt = MEM_EMPTY;
+
+ return mt;
+}
+
+/**
* init_csrows - Initialize the csrow data.
* @mci: EDAC memory controller instance.
*
@@ -598,13 +813,57 @@ static void mc_init(struct mem_ctl_info *mci, struct platform_device *pdev)
mci->dev_name = SYNPS_EDAC_MOD_STRING;
mci->mod_name = SYNPS_EDAC_MOD_VER;

- edac_op_state = EDAC_OPSTATE_POLL;
- mci->edac_check = check_errors;
+ if (priv->p_data->quirks & DDR_ECC_INTR_SUPPORT) {
+ edac_op_state = EDAC_OPSTATE_INT;
+ } else {
+ edac_op_state = EDAC_OPSTATE_POLL;
+ mci->edac_check = check_errors;
+ }
+
mci->ctl_page_to_phys = NULL;

init_csrows(mci);
}

+static void enable_intr(struct synps_edac_priv *priv)
+{
+ /* Enable UE/CE Interrupts */
+ writel(DDR_QOSUE_MASK | DDR_QOSCE_MASK,
+ priv->baseaddr + DDR_QOS_IRQ_EN_OFST);
+}
+
+static void disable_intr(struct synps_edac_priv *priv)
+{
+ /* Disable UE/CE Interrupts */
+ writel(DDR_QOSUE_MASK | DDR_QOSCE_MASK,
+ priv->baseaddr + DDR_QOS_IRQ_DB_OFST);
+}
+
+static int setup_irq(struct mem_ctl_info *mci,
+ struct platform_device *pdev)
+{
+ struct synps_edac_priv *priv = mci->pvt_info;
+ int ret, irq;
+
+ irq = platform_get_irq(pdev, 0);
+ if (irq < 0) {
+ edac_printk(KERN_ERR, EDAC_MC,
+ "No IRQ %d in DT\n", irq);
+ return irq;
+ }
+
+ ret = devm_request_irq(&pdev->dev, irq, intr_handler,
+ 0, dev_name(&pdev->dev), mci);
+ if (ret < 0) {
+ edac_printk(KERN_ERR, EDAC_MC, "Failed to request IRQ\n");
+ return ret;
+ }
+
+ enable_intr(priv);
+
+ return 0;
+}
+
static const struct synps_platform_data zynq_edac_def = {
.get_error_info = zynq_get_error_info,
.get_mtype = zynq_get_mtype,
@@ -613,9 +872,26 @@ static const struct synps_platform_data zynq_edac_def = {
.quirks = 0,
};

+static const struct synps_platform_data zynqmp_edac_def = {
+ .get_error_info = zynqmp_get_error_info,
+ .get_mtype = zynqmp_get_mtype,
+ .get_dtype = zynqmp_get_dtype,
+ .get_ecc_state = zynqmp_get_ecc_state,
+ .quirks = DDR_ECC_INTR_SUPPORT,
+};
+
static const struct of_device_id synps_edac_match[] = {
- { .compatible = "xlnx,zynq-ddrc-a05", .data = (void *)&zynq_edac_def },
- { /* end of table */ }
+ {
+ .compatible = "xlnx,zynq-ddrc-a05",
+ .data = (void *)&zynq_edac_def
+ },
+ {
+ .compatible = "xlnx,zynqmp-ddrc-2.40a",
+ .data = (void *)&zynqmp_edac_def
+ },
+ {
+ /* end of table */
+ }
};

MODULE_DEVICE_TABLE(of, synps_edac_match);
@@ -674,6 +950,12 @@ static int mc_probe(struct platform_device *pdev)

mc_init(mci, pdev);

+ if (priv->p_data->quirks & DDR_ECC_INTR_SUPPORT) {
+ rc = setup_irq(mci, pdev);
+ if (rc)
+ goto free_edac_mc;
+ }
+
rc = edac_mc_add_mc(mci);
if (rc) {
edac_printk(KERN_ERR, EDAC_MC,
@@ -685,7 +967,9 @@ static int mc_probe(struct platform_device *pdev)
* Start capturing the correctable and uncorrectable errors. A write of
* 0 starts the counters.
*/
- writel(0x0, baseaddr + ECC_CTRL_OFST);
+ if (!(priv->p_data->quirks & DDR_ECC_INTR_SUPPORT))
+ writel(0x0, baseaddr + ECC_CTRL_OFST);
+
return rc;

free_edac_mc:
@@ -703,6 +987,12 @@ static int mc_probe(struct platform_device *pdev)
static int mc_remove(struct platform_device *pdev)
{
struct mem_ctl_info *mci = platform_get_drvdata(pdev);
+ struct synps_edac_priv *priv;
+
+ priv = mci->pvt_info;
+
+ if (priv->p_data->quirks & DDR_ECC_INTR_SUPPORT)
+ disable_intr(priv);

edac_mc_del_mc(&pdev->dev);
edac_mc_free(mci);
--
2.1.1


2018-10-15 05:31:26

by Manish Narani

[permalink] [raw]
Subject: [PATCH v9 6/6] edac: synopsys: Add Error Injection support for ZynqMP DDRC

Add support for Error Injection for ZynqMP DDRC IP. For injecting
errors, the Row, Column, Bank, Bank Group and Rank bits positions are
determined via Address Map registers of Synopsys DDRC.

Signed-off-by: Manish Narani <[email protected]>
---
drivers/edac/synopsys_edac.c | 420 ++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 413 insertions(+), 7 deletions(-)

diff --git a/drivers/edac/synopsys_edac.c b/drivers/edac/synopsys_edac.c
index 603c4bd..1f86d5e 100644
--- a/drivers/edac/synopsys_edac.c
+++ b/drivers/edac/synopsys_edac.c
@@ -302,12 +302,18 @@ struct synps_ecc_status {

/**
* struct synps_edac_priv - DDR memory controller private instance data.
- * @baseaddr: Base address of the DDR controller.
- * @message: Buffer for framing the event specific info.
- * @stat: ECC status information.
- * @p_data: Platform data.
- * @ce_cnt: Correctable Error count.
- * @ue_cnt: Uncorrectable Error count.
+ * @baseaddr: Base address of the DDR controller.
+ * @message: Buffer for framing the event specific info.
+ * @stat: ECC status information.
+ * @p_data: Platform data.
+ * @ce_cnt: Correctable Error count.
+ * @ue_cnt: Uncorrectable Error count.
+ * @poison_addr: Data poison address.
+ * @row_shift: Bit shifts for row bit.
+ * @col_shift: Bit shifts for column bit.
+ * @bank_shift: Bit shifts for bank bit.
+ * @bankgrp_shift: Bit shifts for bank group bit.
+ * @rank_shift: Bit shifts for rank bit.
*/
struct synps_edac_priv {
void __iomem *baseaddr;
@@ -316,6 +322,14 @@ struct synps_edac_priv {
const struct synps_platform_data *p_data;
u32 ce_cnt;
u32 ue_cnt;
+#ifdef CONFIG_EDAC_DEBUG
+ ulong poison_addr;
+ u32 row_shift[18];
+ u32 col_shift[14];
+ u32 bank_shift[3];
+ u32 bankgrp_shift[2];
+ u32 rank_shift[1];
+#endif
};

/**
@@ -877,7 +891,11 @@ static const struct synps_platform_data zynqmp_edac_def = {
.get_mtype = zynqmp_get_mtype,
.get_dtype = zynqmp_get_dtype,
.get_ecc_state = zynqmp_get_ecc_state,
- .quirks = DDR_ECC_INTR_SUPPORT,
+ .quirks = (DDR_ECC_INTR_SUPPORT
+#ifdef CONFIG_EDAC_DEBUG
+ | DDR_ECC_DATA_POISON_SUPPORT
+#endif
+ ),
};

static const struct of_device_id synps_edac_match[] = {
@@ -896,6 +914,375 @@ static const struct of_device_id synps_edac_match[] = {

MODULE_DEVICE_TABLE(of, synps_edac_match);

+#ifdef CONFIG_EDAC_DEBUG
+#define to_mci(k) container_of(k, struct mem_ctl_info, dev)
+
+/**
+ * ddr_poison_setup - Update poison registers.
+ * @priv: DDR memory controller private instance data.
+ *
+ * Update poison registers as per DDR mapping.
+ * Return: none.
+ */
+static void ddr_poison_setup(struct synps_edac_priv *priv)
+{
+ int col = 0, row = 0, bank = 0, bankgrp = 0, rank = 0, regval;
+ int index;
+ ulong hif_addr = 0;
+
+ hif_addr = priv->poison_addr >> 3;
+
+ for (index = 0; index < DDR_MAX_ROW_SHIFT; index++) {
+ if (priv->row_shift[index])
+ row |= (((hif_addr >> priv->row_shift[index]) &
+ BIT(0)) << index);
+ else
+ break;
+ }
+
+ for (index = 0; index < DDR_MAX_COL_SHIFT; index++) {
+ if (priv->col_shift[index] || index < 3)
+ col |= (((hif_addr >> priv->col_shift[index]) &
+ BIT(0)) << index);
+ else
+ break;
+ }
+
+ for (index = 0; index < DDR_MAX_BANK_SHIFT; index++) {
+ if (priv->bank_shift[index])
+ bank |= (((hif_addr >> priv->bank_shift[index]) &
+ BIT(0)) << index);
+ else
+ break;
+ }
+
+ for (index = 0; index < DDR_MAX_BANKGRP_SHIFT; index++) {
+ if (priv->bankgrp_shift[index])
+ bankgrp |= (((hif_addr >> priv->bankgrp_shift[index])
+ & BIT(0)) << index);
+ else
+ break;
+ }
+
+ if (priv->rank_shift[0])
+ rank = (hif_addr >> priv->rank_shift[0]) & BIT(0);
+
+ regval = (rank << ECC_POISON0_RANK_SHIFT) & ECC_POISON0_RANK_MASK;
+ regval |= (col << ECC_POISON0_COLUMN_SHIFT) & ECC_POISON0_COLUMN_MASK;
+ writel(regval, priv->baseaddr + ECC_POISON0_OFST);
+
+ regval = (bankgrp << ECC_POISON1_BG_SHIFT) & ECC_POISON1_BG_MASK;
+ regval |= (bank << ECC_POISON1_BANKNR_SHIFT) & ECC_POISON1_BANKNR_MASK;
+ regval |= (row << ECC_POISON1_ROW_SHIFT) & ECC_POISON1_ROW_MASK;
+ writel(regval, priv->baseaddr + ECC_POISON1_OFST);
+}
+
+static ssize_t inject_data_error_show(struct device *dev,
+ struct device_attribute *mattr,
+ char *data)
+{
+ struct mem_ctl_info *mci = to_mci(dev);
+ struct synps_edac_priv *priv = mci->pvt_info;
+
+ return sprintf(data, "Poison0 Addr: 0x%08x\n\rPoison1 Addr: 0x%08x\n\r"
+ "Error injection Address: 0x%lx\n\r",
+ readl(priv->baseaddr + ECC_POISON0_OFST),
+ readl(priv->baseaddr + ECC_POISON1_OFST),
+ priv->poison_addr);
+}
+
+static ssize_t inject_data_error_store(struct device *dev,
+ struct device_attribute *mattr,
+ const char *data, size_t count)
+{
+ struct mem_ctl_info *mci = to_mci(dev);
+ struct synps_edac_priv *priv = mci->pvt_info;
+
+ if (kstrtoul(data, 0, &priv->poison_addr))
+ return -EINVAL;
+
+ ddr_poison_setup(priv);
+
+ return count;
+}
+
+static ssize_t inject_data_poison_show(struct device *dev,
+ struct device_attribute *mattr,
+ char *data)
+{
+ struct mem_ctl_info *mci = to_mci(dev);
+ struct synps_edac_priv *priv = mci->pvt_info;
+
+ return sprintf(data, "Data Poisoning: %s\n\r",
+ (((readl(priv->baseaddr + ECC_CFG1_OFST)) & 0x3) == 0x3)
+ ? ("Correctable Error") : ("UnCorrectable Error"));
+}
+
+static ssize_t inject_data_poison_store(struct device *dev,
+ struct device_attribute *mattr,
+ const char *data, size_t count)
+{
+ struct mem_ctl_info *mci = to_mci(dev);
+ struct synps_edac_priv *priv = mci->pvt_info;
+
+ writel(0, priv->baseaddr + DDRC_SWCTL);
+ if (strncmp(data, "CE", 2) == 0)
+ writel(ECC_CEPOISON_MASK, priv->baseaddr + ECC_CFG1_OFST);
+ else
+ writel(ECC_UEPOISON_MASK, priv->baseaddr + ECC_CFG1_OFST);
+ writel(1, priv->baseaddr + DDRC_SWCTL);
+
+ return count;
+}
+
+static DEVICE_ATTR_RW(inject_data_error);
+static DEVICE_ATTR_RW(inject_data_poison);
+
+static int edac_create_sysfs_attributes(struct mem_ctl_info *mci)
+{
+ int rc;
+
+ rc = device_create_file(&mci->dev, &dev_attr_inject_data_error);
+ if (rc < 0)
+ return rc;
+ rc = device_create_file(&mci->dev, &dev_attr_inject_data_poison);
+ if (rc < 0)
+ return rc;
+ return 0;
+}
+
+static void edac_remove_sysfs_attributes(struct mem_ctl_info *mci)
+{
+ device_remove_file(&mci->dev, &dev_attr_inject_data_error);
+ device_remove_file(&mci->dev, &dev_attr_inject_data_poison);
+}
+
+static void setup_row_address_map(struct synps_edac_priv *priv, u32 *addrmap)
+{
+ u32 addrmap_row_b2_10;
+ int index;
+
+ priv->row_shift[0] = (addrmap[5] & ROW_MAX_VAL_MASK) + ROW_B0_BASE;
+ priv->row_shift[1] = ((addrmap[5] >> 8) &
+ ROW_MAX_VAL_MASK) + ROW_B1_BASE;
+
+ addrmap_row_b2_10 = (addrmap[5] >> 16) & ROW_MAX_VAL_MASK;
+ if (addrmap_row_b2_10 != ROW_MAX_VAL_MASK) {
+ for (index = 2; index < 11; index++)
+ priv->row_shift[index] = addrmap_row_b2_10 +
+ index + ROW_B0_BASE;
+
+ } else {
+ priv->row_shift[2] = (addrmap[9] &
+ ROW_MAX_VAL_MASK) + ROW_B2_BASE;
+ priv->row_shift[3] = ((addrmap[9] >> 8) &
+ ROW_MAX_VAL_MASK) + ROW_B3_BASE;
+ priv->row_shift[4] = ((addrmap[9] >> 16) &
+ ROW_MAX_VAL_MASK) + ROW_B4_BASE;
+ priv->row_shift[5] = ((addrmap[9] >> 24) &
+ ROW_MAX_VAL_MASK) + ROW_B5_BASE;
+ priv->row_shift[6] = (addrmap[10] &
+ ROW_MAX_VAL_MASK) + ROW_B6_BASE;
+ priv->row_shift[7] = ((addrmap[10] >> 8) &
+ ROW_MAX_VAL_MASK) + ROW_B7_BASE;
+ priv->row_shift[8] = ((addrmap[10] >> 16) &
+ ROW_MAX_VAL_MASK) + ROW_B8_BASE;
+ priv->row_shift[9] = ((addrmap[10] >> 24) &
+ ROW_MAX_VAL_MASK) + ROW_B9_BASE;
+ priv->row_shift[10] = (addrmap[11] &
+ ROW_MAX_VAL_MASK) + ROW_B10_BASE;
+ }
+
+ priv->row_shift[11] = (((addrmap[5] >> 24) & ROW_MAX_VAL_MASK) ==
+ ROW_MAX_VAL_MASK) ? 0 : (((addrmap[5] >> 24) &
+ ROW_MAX_VAL_MASK) + ROW_B11_BASE);
+ priv->row_shift[12] = ((addrmap[6] & ROW_MAX_VAL_MASK) ==
+ ROW_MAX_VAL_MASK) ? 0 : ((addrmap[6] &
+ ROW_MAX_VAL_MASK) + ROW_B12_BASE);
+ priv->row_shift[13] = (((addrmap[6] >> 8) & ROW_MAX_VAL_MASK) ==
+ ROW_MAX_VAL_MASK) ? 0 : (((addrmap[6] >> 8) &
+ ROW_MAX_VAL_MASK) + ROW_B13_BASE);
+ priv->row_shift[14] = (((addrmap[6] >> 16) & ROW_MAX_VAL_MASK) ==
+ ROW_MAX_VAL_MASK) ? 0 : (((addrmap[6] >> 16) &
+ ROW_MAX_VAL_MASK) + ROW_B14_BASE);
+ priv->row_shift[15] = (((addrmap[6] >> 24) & ROW_MAX_VAL_MASK) ==
+ ROW_MAX_VAL_MASK) ? 0 : (((addrmap[6] >> 24) &
+ ROW_MAX_VAL_MASK) + ROW_B15_BASE);
+ priv->row_shift[16] = ((addrmap[7] & ROW_MAX_VAL_MASK) ==
+ ROW_MAX_VAL_MASK) ? 0 : ((addrmap[7] &
+ ROW_MAX_VAL_MASK) + ROW_B16_BASE);
+ priv->row_shift[17] = (((addrmap[7] >> 8) & ROW_MAX_VAL_MASK) ==
+ ROW_MAX_VAL_MASK) ? 0 : (((addrmap[7] >> 8) &
+ ROW_MAX_VAL_MASK) + ROW_B17_BASE);
+}
+
+static void setup_column_address_map(struct synps_edac_priv *priv, u32 *addrmap)
+{
+ u32 width, memtype;
+ int index;
+
+ memtype = readl(priv->baseaddr + CTRL_OFST);
+ width = (memtype & ECC_CTRL_BUSWIDTH_MASK) >> ECC_CTRL_BUSWIDTH_SHIFT;
+
+ priv->col_shift[0] = 0;
+ priv->col_shift[1] = 1;
+ priv->col_shift[2] = (addrmap[2] & COL_MAX_VAL_MASK) + COL_B2_BASE;
+ priv->col_shift[3] = ((addrmap[2] >> 8) &
+ COL_MAX_VAL_MASK) + COL_B3_BASE;
+ priv->col_shift[4] = (((addrmap[2] >> 16) & COL_MAX_VAL_MASK) ==
+ COL_MAX_VAL_MASK) ? 0 : (((addrmap[2] >> 16) &
+ COL_MAX_VAL_MASK) + COL_B4_BASE);
+ priv->col_shift[5] = (((addrmap[2] >> 24) & COL_MAX_VAL_MASK) ==
+ COL_MAX_VAL_MASK) ? 0 : (((addrmap[2] >> 24) &
+ COL_MAX_VAL_MASK) + COL_B5_BASE);
+ priv->col_shift[6] = ((addrmap[3] & COL_MAX_VAL_MASK) ==
+ COL_MAX_VAL_MASK) ? 0 : ((addrmap[3] &
+ COL_MAX_VAL_MASK) + COL_B6_BASE);
+ priv->col_shift[7] = (((addrmap[3] >> 8) & COL_MAX_VAL_MASK) ==
+ COL_MAX_VAL_MASK) ? 0 : (((addrmap[3] >> 8) &
+ COL_MAX_VAL_MASK) + COL_B7_BASE);
+ priv->col_shift[8] = (((addrmap[3] >> 16) & COL_MAX_VAL_MASK) ==
+ COL_MAX_VAL_MASK) ? 0 : (((addrmap[3] >> 16) &
+ COL_MAX_VAL_MASK) + COL_B8_BASE);
+ priv->col_shift[9] = (((addrmap[3] >> 24) & COL_MAX_VAL_MASK) ==
+ COL_MAX_VAL_MASK) ? 0 : (((addrmap[3] >> 24) &
+ COL_MAX_VAL_MASK) + COL_B9_BASE);
+ if (width == DDRCTL_EWDTH_64) {
+ if (memtype & MEM_TYPE_LPDDR3) {
+ priv->col_shift[10] = ((addrmap[4] &
+ COL_MAX_VAL_MASK) == COL_MAX_VAL_MASK) ? 0 :
+ ((addrmap[4] & COL_MAX_VAL_MASK) +
+ COL_B10_BASE);
+ priv->col_shift[11] = (((addrmap[4] >> 8) &
+ COL_MAX_VAL_MASK) == COL_MAX_VAL_MASK) ? 0 :
+ (((addrmap[4] >> 8) & COL_MAX_VAL_MASK) +
+ COL_B11_BASE);
+ } else {
+ priv->col_shift[11] = ((addrmap[4] &
+ COL_MAX_VAL_MASK) == COL_MAX_VAL_MASK) ? 0 :
+ ((addrmap[4] & COL_MAX_VAL_MASK) +
+ COL_B10_BASE);
+ priv->col_shift[13] = (((addrmap[4] >> 8) &
+ COL_MAX_VAL_MASK) == COL_MAX_VAL_MASK) ? 0 :
+ (((addrmap[4] >> 8) & COL_MAX_VAL_MASK) +
+ COL_B11_BASE);
+ }
+ } else if (width == DDRCTL_EWDTH_32) {
+ if (memtype & MEM_TYPE_LPDDR3) {
+ priv->col_shift[10] = (((addrmap[3] >> 24) &
+ COL_MAX_VAL_MASK) == COL_MAX_VAL_MASK) ? 0 :
+ (((addrmap[3] >> 24) & COL_MAX_VAL_MASK) +
+ COL_B9_BASE);
+ priv->col_shift[11] = ((addrmap[4] &
+ COL_MAX_VAL_MASK) == COL_MAX_VAL_MASK) ? 0 :
+ ((addrmap[4] & COL_MAX_VAL_MASK) +
+ COL_B10_BASE);
+ } else {
+ priv->col_shift[11] = (((addrmap[3] >> 24) &
+ COL_MAX_VAL_MASK) == COL_MAX_VAL_MASK) ? 0 :
+ (((addrmap[3] >> 24) & COL_MAX_VAL_MASK) +
+ COL_B9_BASE);
+ priv->col_shift[13] = ((addrmap[4] &
+ COL_MAX_VAL_MASK) == COL_MAX_VAL_MASK) ? 0 :
+ ((addrmap[4] & COL_MAX_VAL_MASK) +
+ COL_B10_BASE);
+ }
+ } else {
+ if (memtype & MEM_TYPE_LPDDR3) {
+ priv->col_shift[10] = (((addrmap[3] >> 16) &
+ COL_MAX_VAL_MASK) == COL_MAX_VAL_MASK) ? 0 :
+ (((addrmap[3] >> 16) & COL_MAX_VAL_MASK) +
+ COL_B8_BASE);
+ priv->col_shift[11] = (((addrmap[3] >> 24) &
+ COL_MAX_VAL_MASK) == COL_MAX_VAL_MASK) ? 0 :
+ (((addrmap[3] >> 24) & COL_MAX_VAL_MASK) +
+ COL_B9_BASE);
+ priv->col_shift[13] = ((addrmap[4] &
+ COL_MAX_VAL_MASK) == COL_MAX_VAL_MASK) ? 0 :
+ ((addrmap[4] & COL_MAX_VAL_MASK) +
+ COL_B10_BASE);
+ } else {
+ priv->col_shift[11] = (((addrmap[3] >> 16) &
+ COL_MAX_VAL_MASK) == COL_MAX_VAL_MASK) ? 0 :
+ (((addrmap[3] >> 16) & COL_MAX_VAL_MASK) +
+ COL_B8_BASE);
+ priv->col_shift[13] = (((addrmap[3] >> 24) &
+ COL_MAX_VAL_MASK) == COL_MAX_VAL_MASK) ? 0 :
+ (((addrmap[3] >> 24) & COL_MAX_VAL_MASK) +
+ COL_B9_BASE);
+ }
+ }
+
+ if (width) {
+ for (index = 9; index > width; index--) {
+ priv->col_shift[index] = priv->col_shift[index - width];
+ priv->col_shift[index - width] = 0;
+ }
+ }
+
+}
+
+static void setup_bank_address_map(struct synps_edac_priv *priv, u32 *addrmap)
+{
+ priv->bank_shift[0] = (addrmap[1] & BANK_MAX_VAL_MASK) + BANK_B0_BASE;
+ priv->bank_shift[1] = ((addrmap[1] >> 8) &
+ BANK_MAX_VAL_MASK) + BANK_B1_BASE;
+ priv->bank_shift[2] = (((addrmap[1] >> 16) &
+ BANK_MAX_VAL_MASK) == BANK_MAX_VAL_MASK) ? 0 :
+ (((addrmap[1] >> 16) & BANK_MAX_VAL_MASK) +
+ BANK_B2_BASE);
+
+}
+
+static void setup_bg_address_map(struct synps_edac_priv *priv, u32 *addrmap)
+{
+ priv->bankgrp_shift[0] = (addrmap[8] &
+ BANKGRP_MAX_VAL_MASK) + BANKGRP_B0_BASE;
+ priv->bankgrp_shift[1] = (((addrmap[8] >> 8) & BANKGRP_MAX_VAL_MASK) ==
+ BANKGRP_MAX_VAL_MASK) ? 0 : (((addrmap[8] >> 8)
+ & BANKGRP_MAX_VAL_MASK) + BANKGRP_B1_BASE);
+
+}
+
+static void setup_rank_address_map(struct synps_edac_priv *priv, u32 *addrmap)
+{
+ priv->rank_shift[0] = ((addrmap[0] & RANK_MAX_VAL_MASK) ==
+ RANK_MAX_VAL_MASK) ? 0 : ((addrmap[0] &
+ RANK_MAX_VAL_MASK) + RANK_B0_BASE);
+}
+
+/**
+ * setup_address_map - Set Address Map by querying ADDRMAP registers.
+ * @priv: DDR memory controller private instance data.
+ *
+ * Set Address Map by querying ADDRMAP registers.
+ *
+ * Return: none.
+ */
+static void setup_address_map(struct synps_edac_priv *priv)
+{
+ u32 addrmap[12];
+ int index;
+
+ for (index = 0; index < 12; index++) {
+ u32 addrmap_offset;
+
+ addrmap_offset = ECC_ADDRMAP0_OFFSET + (index * 4);
+ addrmap[index] = readl(priv->baseaddr + addrmap_offset);
+ }
+
+ setup_row_address_map(priv, addrmap);
+
+ setup_column_address_map(priv, addrmap);
+
+ setup_bank_address_map(priv, addrmap);
+
+ setup_bg_address_map(priv, addrmap);
+
+ setup_rank_address_map(priv, addrmap);
+}
+#endif /* CONFIG_EDAC_DEBUG */
+
/**
* mc_probe - Check controller and bind driver.
* @pdev: platform device.
@@ -963,6 +1350,20 @@ static int mc_probe(struct platform_device *pdev)
goto free_edac_mc;
}

+#ifdef CONFIG_EDAC_DEBUG
+ if (priv->p_data->quirks & DDR_ECC_DATA_POISON_SUPPORT) {
+ if (edac_create_sysfs_attributes(mci)) {
+ edac_printk(KERN_ERR, EDAC_MC,
+ "Failed to create sysfs entries\n");
+ goto free_edac_mc;
+ }
+ }
+
+ if (of_device_is_compatible(pdev->dev.of_node,
+ "xlnx,zynqmp-ddrc-2.40a"))
+ setup_address_map(priv);
+#endif
+
/*
* Start capturing the correctable and uncorrectable errors. A write of
* 0 starts the counters.
@@ -994,6 +1395,11 @@ static int mc_remove(struct platform_device *pdev)
if (priv->p_data->quirks & DDR_ECC_INTR_SUPPORT)
disable_intr(priv);

+#ifdef CONFIG_EDAC_DEBUG
+ if (priv->p_data->quirks & DDR_ECC_DATA_POISON_SUPPORT)
+ edac_remove_sysfs_attributes(mci);
+#endif
+
edac_mc_del_mc(&pdev->dev);
edac_mc_free(mci);

--
2.1.1


2018-10-15 05:31:27

by Manish Narani

[permalink] [raw]
Subject: [PATCH v9 3/6] edac: synopsys: Add macro defines for ZynqMP DDRC

Add macro defines for ZynqMP DDR controller. These macros will be used
for ZynqMP ECC operations.

Signed-off-by: Manish Narani <[email protected]>
---
drivers/edac/synopsys_edac.c | 168 +++++++++++++++++++++++++++++++++++++++++++
1 file changed, 168 insertions(+)

diff --git a/drivers/edac/synopsys_edac.c b/drivers/edac/synopsys_edac.c
index 0005ef3..d1999e0 100644
--- a/drivers/edac/synopsys_edac.c
+++ b/drivers/edac/synopsys_edac.c
@@ -97,6 +97,174 @@
#define SCRUB_MODE_MASK 0x7
#define SCRUB_MODE_SECDED 0x4

+/* DDR ECC Quirks */
+#define DDR_ECC_INTR_SUPPORT BIT(0)
+#define DDR_ECC_DATA_POISON_SUPPORT BIT(1)
+
+/* ZynqMP Enhanced DDR memory controller registers that are relevant to ECC */
+/* ECC Configuration Registers */
+#define ECC_CFG0_OFST 0x70
+#define ECC_CFG1_OFST 0x74
+
+/* ECC Status Register */
+#define ECC_STAT_OFST 0x78
+
+/* ECC Clear Register */
+#define ECC_CLR_OFST 0x7C
+
+/* ECC Error count Register */
+#define ECC_ERRCNT_OFST 0x80
+
+/* ECC Corrected Error Address Register */
+#define ECC_CEADDR0_OFST 0x84
+#define ECC_CEADDR1_OFST 0x88
+
+/* ECC Syndrome Registers */
+#define ECC_CSYND0_OFST 0x8C
+#define ECC_CSYND1_OFST 0x90
+#define ECC_CSYND2_OFST 0x94
+
+/* ECC Bit Mask0 Address Register */
+#define ECC_BITMASK0_OFST 0x98
+#define ECC_BITMASK1_OFST 0x9C
+#define ECC_BITMASK2_OFST 0xA0
+
+/* ECC UnCorrected Error Address Register */
+#define ECC_UEADDR0_OFST 0xA4
+#define ECC_UEADDR1_OFST 0xA8
+
+/* ECC Syndrome Registers */
+#define ECC_UESYND0_OFST 0xAC
+#define ECC_UESYND1_OFST 0xB0
+#define ECC_UESYND2_OFST 0xB4
+
+/* ECC Poison Address Reg */
+#define ECC_POISON0_OFST 0xB8
+#define ECC_POISON1_OFST 0xBC
+
+#define ECC_ADDRMAP0_OFFSET 0x200
+
+/* Control register bitfield definitions */
+#define ECC_CTRL_BUSWIDTH_MASK 0x3000
+#define ECC_CTRL_BUSWIDTH_SHIFT 12
+#define ECC_CTRL_CLR_CE_ERRCNT BIT(2)
+#define ECC_CTRL_CLR_UE_ERRCNT BIT(3)
+
+/* DDR Control Register width definitions */
+#define DDRCTL_EWDTH_16 2
+#define DDRCTL_EWDTH_32 1
+#define DDRCTL_EWDTH_64 0
+
+/* ECC status register definitions */
+#define ECC_STAT_UECNT_MASK 0xF0000
+#define ECC_STAT_UECNT_SHIFT 16
+#define ECC_STAT_CECNT_MASK 0xF00
+#define ECC_STAT_CECNT_SHIFT 8
+#define ECC_STAT_BITNUM_MASK 0x7F
+
+/* DDR QOS Interrupt register definitions */
+#define DDR_QOS_IRQ_STAT_OFST 0x20200
+#define DDR_QOSUE_MASK 0x4
+#define DDR_QOSCE_MASK 0x2
+#define ECC_CE_UE_INTR_MASK 0x6
+#define DDR_QOS_IRQ_EN_OFST 0x20208
+#define DDR_QOS_IRQ_DB_OFST 0x2020C
+
+/* ECC Corrected Error Register Mask and Shifts*/
+#define ECC_CEADDR0_RW_MASK 0x3FFFF
+#define ECC_CEADDR0_RNK_MASK BIT(24)
+#define ECC_CEADDR1_BNKGRP_MASK 0x3000000
+#define ECC_CEADDR1_BNKNR_MASK 0x70000
+#define ECC_CEADDR1_BLKNR_MASK 0xFFF
+#define ECC_CEADDR1_BNKGRP_SHIFT 24
+#define ECC_CEADDR1_BNKNR_SHIFT 16
+
+/* ECC Poison register shifts */
+#define ECC_POISON0_RANK_SHIFT 24
+#define ECC_POISON0_RANK_MASK BIT(24)
+#define ECC_POISON0_COLUMN_SHIFT 0
+#define ECC_POISON0_COLUMN_MASK 0xFFF
+#define ECC_POISON1_BG_SHIFT 28
+#define ECC_POISON1_BG_MASK 0x30000000
+#define ECC_POISON1_BANKNR_SHIFT 24
+#define ECC_POISON1_BANKNR_MASK 0x7000000
+#define ECC_POISON1_ROW_SHIFT 0
+#define ECC_POISON1_ROW_MASK 0x3FFFF
+
+/* DDR Memory type defines */
+#define MEM_TYPE_DDR3 0x1
+#define MEM_TYPE_LPDDR3 0x8
+#define MEM_TYPE_DDR2 0x4
+#define MEM_TYPE_DDR4 0x10
+#define MEM_TYPE_LPDDR4 0x20
+
+/* DDRC Software control register */
+#define DDRC_SWCTL 0x320
+
+/* DDRC ECC CE & UE poison mask */
+#define ECC_CEPOISON_MASK 0x3
+#define ECC_UEPOISON_MASK 0x1
+
+/* DDRC Device config masks */
+#define DDRC_MSTR_CFG_MASK 0xC0000000
+#define DDRC_MSTR_CFG_SHIFT 30
+#define DDRC_MSTR_CFG_X4_MASK 0x0
+#define DDRC_MSTR_CFG_X8_MASK 0x1
+#define DDRC_MSTR_CFG_X16_MASK 0x2
+#define DDRC_MSTR_CFG_X32_MASK 0x3
+
+#define DDR_MAX_ROW_SHIFT 18
+#define DDR_MAX_COL_SHIFT 14
+#define DDR_MAX_BANK_SHIFT 3
+#define DDR_MAX_BANKGRP_SHIFT 2
+
+#define ROW_MAX_VAL_MASK 0xF
+#define COL_MAX_VAL_MASK 0xF
+#define BANK_MAX_VAL_MASK 0x1F
+#define BANKGRP_MAX_VAL_MASK 0x1F
+#define RANK_MAX_VAL_MASK 0x1F
+
+#define ROW_B0_BASE 6
+#define ROW_B1_BASE 7
+#define ROW_B2_BASE 8
+#define ROW_B3_BASE 9
+#define ROW_B4_BASE 10
+#define ROW_B5_BASE 11
+#define ROW_B6_BASE 12
+#define ROW_B7_BASE 13
+#define ROW_B8_BASE 14
+#define ROW_B9_BASE 15
+#define ROW_B10_BASE 16
+#define ROW_B11_BASE 17
+#define ROW_B12_BASE 18
+#define ROW_B13_BASE 19
+#define ROW_B14_BASE 20
+#define ROW_B15_BASE 21
+#define ROW_B16_BASE 22
+#define ROW_B17_BASE 23
+
+#define COL_B2_BASE 2
+#define COL_B3_BASE 3
+#define COL_B4_BASE 4
+#define COL_B5_BASE 5
+#define COL_B6_BASE 6
+#define COL_B7_BASE 7
+#define COL_B8_BASE 8
+#define COL_B9_BASE 9
+#define COL_B10_BASE 10
+#define COL_B11_BASE 11
+#define COL_B12_BASE 12
+#define COL_B13_BASE 13
+
+#define BANK_B0_BASE 2
+#define BANK_B1_BASE 3
+#define BANK_B2_BASE 4
+
+#define BANKGRP_B0_BASE 2
+#define BANKGRP_B1_BASE 3
+
+#define RANK_B0_BASE 6
+
/**
* struct ecc_error_info - ECC error log information.
* @row: Row number.
--
2.1.1


2018-10-15 05:35:10

by Manish Narani

[permalink] [raw]
Subject: [PATCH v9 2/6] dt: bindings: Document ZynqMP DDRC in Synopsys documentation

Add information of ZynqMP DDRC which reports the single bit errors that
are corrected and the double bit errors that are detected.

Signed-off-by: Manish Narani <[email protected]>
Reviewed-by: Rob Herring <[email protected]>
---
.../bindings/memory-controllers/synopsys.txt | 27 ++++++++++++++++++----
1 file changed, 22 insertions(+), 5 deletions(-)

diff --git a/Documentation/devicetree/bindings/memory-controllers/synopsys.txt b/Documentation/devicetree/bindings/memory-controllers/synopsys.txt
index a43d26d..9d32762 100644
--- a/Documentation/devicetree/bindings/memory-controllers/synopsys.txt
+++ b/Documentation/devicetree/bindings/memory-controllers/synopsys.txt
@@ -1,15 +1,32 @@
Binding for Synopsys IntelliDDR Multi Protocol Memory Controller

-This controller has an optional ECC support in half-bus width (16-bit)
-configuration. The ECC controller corrects one bit error and detects
-two bit errors.
+The ZynqMP DDR ECC controller has an optional ECC support in 64-bit and 32-bit
+bus width configurations.
+
+The Zynq DDR ECC controller has an optional ECC support in half-bus width
+(16-bit) configuration.
+
+These both ECC controllers correct single bit ECC errors and detect double bit
+ECC errors.

Required properties:
- - compatible: Should be 'xlnx,zynq-ddrc-a05'
- - reg: Base address and size of the controllers memory area
+ - compatible: One of:
+ - 'xlnx,zynq-ddrc-a05' : Zynq DDR ECC controller
+ - 'xlnx,zynqmp-ddrc-2.40a' : ZynqMP DDR ECC controller
+ - reg: Should contain DDR controller registers location and length.
+
+Required properties for "xlnx,zynqmp-ddrc-2.40a":
+ - interrupts: Property with a value describing the interrupt number.

Example:
memory-controller@f8006000 {
compatible = "xlnx,zynq-ddrc-a05";
reg = <0xf8006000 0x1000>;
};
+
+ mc: memory-controller@fd070000 {
+ compatible = "xlnx,zynqmp-ddrc-2.40a";
+ reg = <0x0 0xfd070000 0x0 0x30000>;
+ interrupt-parent = <&gic>;
+ interrupts = <0 112 4>;
+ };
--
2.1.1


2018-10-17 16:31:48

by Borislav Petkov

[permalink] [raw]
Subject: Re: [PATCH v9 4/6] edac: synopsys: Add EDAC ECC support for ZynqMP DDRC

On Mon, Oct 15, 2018 at 10:59:46AM +0530, Manish Narani wrote:
> Add EDAC ECC support for ZynqMP DDRC IP. The IP supports interrupts for
> corrected and uncorrected errors. Add interrupt handlers for the same.
>
> Signed-off-by: Manish Narani <[email protected]>
> ---
> drivers/edac/Kconfig | 2 +-
> drivers/edac/synopsys_edac.c | 324 ++++++++++++++++++++++++++++++++++++++++---
> 2 files changed, 308 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/edac/Kconfig b/drivers/edac/Kconfig
> index 57304b2..7c40eb2 100644
> --- a/drivers/edac/Kconfig
> +++ b/drivers/edac/Kconfig
> @@ -441,7 +441,7 @@ config EDAC_ALTERA_SDMMC
>
> config EDAC_SYNOPSYS
> tristate "Synopsys DDR Memory Controller"
> - depends on ARCH_ZYNQ
> + depends on ARCH_ZYNQ || ARCH_ZYNQMP
> help
> Support for error detection and correction on the Synopsys DDR
> memory controller.
> diff --git a/drivers/edac/synopsys_edac.c b/drivers/edac/synopsys_edac.c
> index d1999e0..603c4bd 100644
> --- a/drivers/edac/synopsys_edac.c
> +++ b/drivers/edac/synopsys_edac.c
> @@ -22,6 +22,7 @@
> #include <linux/edac.h>
> #include <linux/module.h>
> #include <linux/platform_device.h>
> +#include <linux/interrupt.h>
> #include <linux/of.h>
> #include <linux/of_device.h>
>
> @@ -272,6 +273,8 @@
> * @bank: Bank number.
> * @bitpos: Bit position.
> * @data: Data causing the error.
> + * @bankgrpnr: Bank group number.
> + * @blknr: Block number.
> */
> struct ecc_error_info {
> u32 row;
> @@ -279,6 +282,8 @@ struct ecc_error_info {
> u32 bank;
> u32 bitpos;
> u32 data;
> + u32 bankgrpnr;
> + u32 blknr;
> };
>
> /**
> @@ -385,6 +390,66 @@ static int zynq_get_error_info(struct synps_edac_priv *priv)
> }
>
> /**
> + * zynqmp_get_error_info - Get the current ECC error info.
> + * @priv: DDR memory controller private instance data.
> + *
> + * Return: one if there is no error otherwise returns zero.
> + */
> +static int zynqmp_get_error_info(struct synps_edac_priv *priv)
> +{
> + struct synps_ecc_status *p;
> + u32 regval, clearval = 0;
> + void __iomem *base;
> +
> + base = priv->baseaddr;
> + p = &priv->stat;
> +
> + regval = readl(base + ECC_STAT_OFST);
> + if (!regval)
> + return 1;
> +
> + p->ce_cnt = (regval & ECC_STAT_CECNT_MASK) >> ECC_STAT_CECNT_SHIFT;
> + p->ue_cnt = (regval & ECC_STAT_UECNT_MASK) >> ECC_STAT_UECNT_SHIFT;
> + p->ceinfo.bitpos = (regval & ECC_STAT_BITNUM_MASK);
> +
> + regval = readl(base + ECC_CEADDR0_OFST);
> + if (!p->ce_cnt)
> + goto ue_err;

This check should happen right under the p->ce_cnt assignment a couple
of lines above it. Otherwise the readl() is useless if you jump to the
ue_err label.

IOW, those two readl()'s should happen *after* the ce/ue_cnt checks.

> +
> + p->ceinfo.row = (regval & ECC_CEADDR0_RW_MASK);
> + regval = readl(base + ECC_CEADDR1_OFST);
> + p->ceinfo.bank = (regval & ECC_CEADDR1_BNKNR_MASK) >>
> + ECC_CEADDR1_BNKNR_SHIFT;
> + p->ceinfo.bankgrpnr = (regval & ECC_CEADDR1_BNKGRP_MASK) >>
> + ECC_CEADDR1_BNKGRP_SHIFT;
> + p->ceinfo.blknr = (regval & ECC_CEADDR1_BLKNR_MASK);
> + p->ceinfo.data = readl(base + ECC_CSYND0_OFST);
> + edac_dbg(2, "ECCCSYN0: 0x%08X ECCCSYN1: 0x%08X ECCCSYN2: 0x%08X\n",
> + readl(base + ECC_CSYND0_OFST), readl(base + ECC_CSYND1_OFST),
> + readl(base + ECC_CSYND2_OFST));
> +ue_err:
> + regval = readl(base + ECC_UEADDR0_OFST);
> + if (!p->ue_cnt)
> + goto out;
> +
> + p->ueinfo.row = (regval & ECC_CEADDR0_RW_MASK);
> + regval = readl(base + ECC_UEADDR1_OFST);
> + p->ueinfo.bankgrpnr = (regval & ECC_CEADDR1_BNKGRP_MASK) >>
> + ECC_CEADDR1_BNKGRP_SHIFT;
> + p->ueinfo.bank = (regval & ECC_CEADDR1_BNKNR_MASK) >>
> + ECC_CEADDR1_BNKNR_SHIFT;
> + p->ueinfo.blknr = (regval & ECC_CEADDR1_BLKNR_MASK);
> + p->ueinfo.data = readl(base + ECC_UESYND0_OFST);
> +out:
> + clearval = ECC_CTRL_CLR_CE_ERR | ECC_CTRL_CLR_CE_ERRCNT;
> + clearval |= ECC_CTRL_CLR_UE_ERR | ECC_CTRL_CLR_UE_ERRCNT;
> + writel(clearval, base + ECC_CLR_OFST);
> + writel(0x0, base + ECC_CLR_OFST);
> +
> + return 0;
> +}
> +

...

> +static int setup_irq(struct mem_ctl_info *mci,
> + struct platform_device *pdev)

Align arguments on the opening brace.

> +{
> + struct synps_edac_priv *priv = mci->pvt_info;
> + int ret, irq;
> +
> + irq = platform_get_irq(pdev, 0);
> + if (irq < 0) {
> + edac_printk(KERN_ERR, EDAC_MC,
> + "No IRQ %d in DT\n", irq);
> + return irq;
> + }
> +
> + ret = devm_request_irq(&pdev->dev, irq, intr_handler,
> + 0, dev_name(&pdev->dev), mci);
> + if (ret < 0) {
> + edac_printk(KERN_ERR, EDAC_MC, "Failed to request IRQ\n");
> + return ret;
> + }
> +
> + enable_intr(priv);
> +
> + return 0;
> +}
> +
> static const struct synps_platform_data zynq_edac_def = {
> .get_error_info = zynq_get_error_info,
> .get_mtype = zynq_get_mtype,
> @@ -613,9 +872,26 @@ static const struct synps_platform_data zynq_edac_def = {
> .quirks = 0,
> };
>
> +static const struct synps_platform_data zynqmp_edac_def = {
> + .get_error_info = zynqmp_get_error_info,
> + .get_mtype = zynqmp_get_mtype,
> + .get_dtype = zynqmp_get_dtype,
> + .get_ecc_state = zynqmp_get_ecc_state,
> + .quirks = DDR_ECC_INTR_SUPPORT,
> +};
> +
> static const struct of_device_id synps_edac_match[] = {
> - { .compatible = "xlnx,zynq-ddrc-a05", .data = (void *)&zynq_edac_def },
> - { /* end of table */ }
> + {
> + .compatible = "xlnx,zynq-ddrc-a05",
> + .data = (void *)&zynq_edac_def
> + },
> + {
> + .compatible = "xlnx,zynqmp-ddrc-2.40a",
> + .data = (void *)&zynqmp_edac_def
> + },
> + {
> + /* end of table */
> + }
> };
>
> MODULE_DEVICE_TABLE(of, synps_edac_match);
> @@ -674,6 +950,12 @@ static int mc_probe(struct platform_device *pdev)
>
> mc_init(mci, pdev);
>
> + if (priv->p_data->quirks & DDR_ECC_INTR_SUPPORT) {
> + rc = setup_irq(mci, pdev);
> + if (rc)
> + goto free_edac_mc;
> + }
> +
> rc = edac_mc_add_mc(mci);
> if (rc) {
> edac_printk(KERN_ERR, EDAC_MC,
> @@ -685,7 +967,9 @@ static int mc_probe(struct platform_device *pdev)
> * Start capturing the correctable and uncorrectable errors. A write of
> * 0 starts the counters.
> */
> - writel(0x0, baseaddr + ECC_CTRL_OFST);
> + if (!(priv->p_data->quirks & DDR_ECC_INTR_SUPPORT))
> + writel(0x0, baseaddr + ECC_CTRL_OFST);
> +
> return rc;
>
> free_edac_mc:
> @@ -703,6 +987,12 @@ static int mc_probe(struct platform_device *pdev)
> static int mc_remove(struct platform_device *pdev)
> {
> struct mem_ctl_info *mci = platform_get_drvdata(pdev);
> + struct synps_edac_priv *priv;
> +
> + priv = mci->pvt_info;

Or simply:

struct synps_edac_priv *priv = mci->pvt_info;

> + if (priv->p_data->quirks & DDR_ECC_INTR_SUPPORT)
> + disable_intr(priv);
>
> edac_mc_del_mc(&pdev->dev);
> edac_mc_free(mci);
> --
> 2.1.1
>

--
Regards/Gruss,
Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

2018-10-17 16:54:09

by Borislav Petkov

[permalink] [raw]
Subject: Re: [PATCH v9 5/6] arm64: zynqmp: Add DDRC node

On Mon, Oct 15, 2018 at 10:59:47AM +0530, Manish Narani wrote:
> Add ddrc memory controller node in dts. The size mentioned in dts is
> 0x30000, because we need to access DDR_QOS INTR registers located at
> 0xFD090208 from this driver.
>
> Signed-off-by: Manish Narani <[email protected]>
> ---
> arch/arm64/boot/dts/xilinx/zynqmp.dtsi | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/xilinx/zynqmp.dtsi b/arch/arm64/boot/dts/xilinx/zynqmp.dtsi
> index 29ce234..a81d3b16 100644
> --- a/arch/arm64/boot/dts/xilinx/zynqmp.dtsi
> +++ b/arch/arm64/boot/dts/xilinx/zynqmp.dtsi
> @@ -355,6 +355,13 @@
> xlnx,bus-width = <64>;
> };
>
> + mc: memory-controller@fd070000 {
> + compatible = "xlnx,zynqmp-ddrc-2.40a";
> + reg = <0x0 0xfd070000 0x0 0x30000>;
> + interrupt-parent = <&gic>;
> + interrupts = <0 112 4>;
> + };
> +
> gem0: ethernet@ff0b0000 {
> compatible = "cdns,zynqmp-gem", "cdns,gem";
> status = "disabled";
> --

Still needs DT folks ACK.

--
Regards/Gruss,
Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.