diff mbox series

[v3,3/4] EDAC: synopsys: Add edac driver support for i.MX8MP

Message ID 1585790433-31465-4-git-send-email-sherry.sun@nxp.com (mailing list archive)
State New, archived
Headers show
Series Add edac driver for i.MX8MP based on synopsys edac driver | expand

Commit Message

Sherry Sun April 2, 2020, 1:20 a.m. UTC 
Since i.MX8MP use synopsys ddr controller IP, so add edac support
for i.MX8MP based on synopsys edac driver. i.MX8MP use LPDDR4 and
support interrupts for corrected and uncorrected errors. The main
difference between ZynqMP and i.MX8MP ddr controller is the interrupt
registers. So add another interrupt handler function, enable/disable
interrupt function to distinguish with ZynqMP.

Signed-off-by: Sherry Sun <sherry.sun@nxp.com>
---
 drivers/edac/synopsys_edac.c | 77 +++++++++++++++++++++++++++++++++++-
 1 file changed, 76 insertions(+), 1 deletion(-)

Comments

Robert Richter April 2, 2020, 7:22 a.m. UTC | #1 
On 02.04.20 09:20:32, Sherry Sun wrote:
> Since i.MX8MP use synopsys ddr controller IP, so add edac support
> for i.MX8MP based on synopsys edac driver. i.MX8MP use LPDDR4 and
> support interrupts for corrected and uncorrected errors. The main
> difference between ZynqMP and i.MX8MP ddr controller is the interrupt
> registers. So add another interrupt handler function, enable/disable
> interrupt function to distinguish with ZynqMP.
> 
> Signed-off-by: Sherry Sun <sherry.sun@nxp.com>
> ---
>  drivers/edac/synopsys_edac.c | 77 +++++++++++++++++++++++++++++++++++-
>  1 file changed, 76 insertions(+), 1 deletion(-)

> +static void enable_intr_imx8mp(struct synps_edac_priv *priv)
> +{
> +	int regval;
> +
> +	regval = readl(priv->baseaddr + ECC_CLR_OFST);
> +	regval |= (DDR_CE_INTR_EN_MASK | DDR_UE_INTR_EN_MASK);
> +	writel(regval, priv->baseaddr + ECC_CLR_OFST);
> +}
> +
> +static void disable_intr_imx8mp(struct synps_edac_priv *priv)
> +{
> +	int regval;
> +
> +	regval = readl(priv->baseaddr + ECC_CLR_OFST);
> +	regval &= ~(DDR_CE_INTR_EN_MASK | DDR_UE_INTR_EN_MASK);
> +	writel(regval, priv->baseaddr + ECC_CLR_OFST);
> +}
> +
> +/* Interrupt Handler for ECC interrupts on imx8mp platform. */
> +static irqreturn_t intr_handler_imx8mp(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 + ECC_STAT_OFST);
> +	if (!(regval & ECC_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);
> +	enable_intr_imx8mp(priv);

Why do you enable interrupts here?

-Robert

> +
> +	return IRQ_HANDLED;
> +}
Sherry Sun April 2, 2020, 9:06 a.m. UTC | #2 
Hi Robert,

> -----Original Message-----
> From: Robert Richter <rrichter@marvell.com>
> Sent: 2020年4月2日 15:22
> To: Sherry Sun <sherry.sun@nxp.com>
> Cc: bp@alien8.de; mchehab@kernel.org; tony.luck@intel.com;
> james.morse@arm.com; michal.simek@xilinx.com;
> manish.narani@xilinx.com; linux-edac@vger.kernel.org; linux-
> kernel@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>; Frank Li
> <frank.li@nxp.com>
> Subject: Re: [patch v3 3/4] EDAC: synopsys: Add edac driver support for
> i.MX8MP
> 
> On 02.04.20 09:20:32, Sherry Sun wrote:
> > Since i.MX8MP use synopsys ddr controller IP, so add edac support for
> > i.MX8MP based on synopsys edac driver. i.MX8MP use LPDDR4 and support
> > interrupts for corrected and uncorrected errors. The main difference
> > between ZynqMP and i.MX8MP ddr controller is the interrupt registers.
> > So add another interrupt handler function, enable/disable interrupt
> > function to distinguish with ZynqMP.
> >
> > Signed-off-by: Sherry Sun <sherry.sun@nxp.com>
> > ---
> >  drivers/edac/synopsys_edac.c | 77
> > +++++++++++++++++++++++++++++++++++-
> >  1 file changed, 76 insertions(+), 1 deletion(-)
> 
> > +static void enable_intr_imx8mp(struct synps_edac_priv *priv) {
> > +	int regval;
> > +
> > +	regval = readl(priv->baseaddr + ECC_CLR_OFST);
> > +	regval |= (DDR_CE_INTR_EN_MASK | DDR_UE_INTR_EN_MASK);
> > +	writel(regval, priv->baseaddr + ECC_CLR_OFST); }
> > +
> > +static void disable_intr_imx8mp(struct synps_edac_priv *priv) {
> > +	int regval;
> > +
> > +	regval = readl(priv->baseaddr + ECC_CLR_OFST);
> > +	regval &= ~(DDR_CE_INTR_EN_MASK | DDR_UE_INTR_EN_MASK);
> > +	writel(regval, priv->baseaddr + ECC_CLR_OFST); }
> > +
> > +/* Interrupt Handler for ECC interrupts on imx8mp platform. */ static
> > +irqreturn_t intr_handler_imx8mp(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 + ECC_STAT_OFST);
> > +	if (!(regval & ECC_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);
> > +	enable_intr_imx8mp(priv);
> 
> Why do you enable interrupts here?

Because zynqmp_get_error_info() wrote 0 to ECC_CLR_OFST, so here have to re-enable the interrupts.

As said in the commit, the main difference between ZynqMP and i.MX8MP ddr controller is the interrupt registers.
ZynqMP use DDR QOS Interrupt registers,  but i.MX8MP use ECC_CLR_OFST Register(bit8 and bit9) to enable/disable the ce/ue interrupts. 

In zynqmp_get_error_info(), Zynqmp wrote 0 to ECC_CLR_OFST register to clear CE/UE error flags and counts, it has no effect on Zynqmp interrupts. 
But for i.MX8MP, wirte 0 to ECC_CLR_OFST will disable i.MX8MP CE/UE interrupt, so need re-enable the interrupts.

Best regards
Sherry Sun

> 
> -Robert
> 
> > +
> > +	return IRQ_HANDLED;
> > +}
Robert Richter April 2, 2020, 11:17 a.m. UTC | #3 
On 02.04.20 09:06:27, Sherry Sun wrote:
> > From: Robert Richter <rrichter@marvell.com>
> > On 02.04.20 09:20:32, Sherry Sun wrote:

> > > +static void enable_intr_imx8mp(struct synps_edac_priv *priv) {
> > > +	int regval;
> > > +
> > > +	regval = readl(priv->baseaddr + ECC_CLR_OFST);
> > > +	regval |= (DDR_CE_INTR_EN_MASK | DDR_UE_INTR_EN_MASK);
> > > +	writel(regval, priv->baseaddr + ECC_CLR_OFST); }
> > > +
> > > +static void disable_intr_imx8mp(struct synps_edac_priv *priv) {
> > > +	int regval;
> > > +
> > > +	regval = readl(priv->baseaddr + ECC_CLR_OFST);
> > > +	regval &= ~(DDR_CE_INTR_EN_MASK | DDR_UE_INTR_EN_MASK);
> > > +	writel(regval, priv->baseaddr + ECC_CLR_OFST); }
> > > +
> > > +/* Interrupt Handler for ECC interrupts on imx8mp platform. */ static
> > > +irqreturn_t intr_handler_imx8mp(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 + ECC_STAT_OFST);
> > > +	if (!(regval & ECC_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);
> > > +	enable_intr_imx8mp(priv);
> > 
> > Why do you enable interrupts here?
> 
> Because zynqmp_get_error_info() wrote 0 to ECC_CLR_OFST, so here have to re-enable the interrupts.

This does not seem to be the right place for it.

> As said in the commit, the main difference between ZynqMP and i.MX8MP ddr controller is the interrupt registers.
> ZynqMP use DDR QOS Interrupt registers,  but i.MX8MP use ECC_CLR_OFST Register(bit8 and bit9) to enable/disable the ce/ue interrupts. 
> 
> In zynqmp_get_error_info(), Zynqmp wrote 0 to ECC_CLR_OFST register to clear CE/UE error flags and counts, it has no effect on Zynqmp interrupts. 
> But for i.MX8MP, wirte 0 to ECC_CLR_OFST will disable i.MX8MP CE/UE interrupt, so need re-enable the interrupts.

All this shows one more time there should be separate handlers. You
should get rid most callbacks in struct synps_platform_data and
instead have separate probe functions for both flavors that share
common code.

-Robert
Sherry Sun April 2, 2020, 1:09 p.m. UTC | #4 
Hi Robert,

> -----Original Message-----
> From: Robert Richter <rrichter@marvell.com>
> Sent: 2020年4月2日 19:17
> To: Sherry Sun <sherry.sun@nxp.com>
> Cc: bp@alien8.de; mchehab@kernel.org; tony.luck@intel.com;
> james.morse@arm.com; michal.simek@xilinx.com;
> manish.narani@xilinx.com; linux-edac@vger.kernel.org; linux-
> kernel@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>; Frank Li
> <frank.li@nxp.com>
> Subject: Re: [patch v3 3/4] EDAC: synopsys: Add edac driver support for
> i.MX8MP
> 
> On 02.04.20 09:06:27, Sherry Sun wrote:
> > > From: Robert Richter <rrichter@marvell.com> On 02.04.20 09:20:32,
> > > Sherry Sun wrote:
> 
> > > > +static void enable_intr_imx8mp(struct synps_edac_priv *priv) {
> > > > +	int regval;
> > > > +
> > > > +	regval = readl(priv->baseaddr + ECC_CLR_OFST);
> > > > +	regval |= (DDR_CE_INTR_EN_MASK | DDR_UE_INTR_EN_MASK);
> > > > +	writel(regval, priv->baseaddr + ECC_CLR_OFST); }
> > > > +
> > > > +static void disable_intr_imx8mp(struct synps_edac_priv *priv) {
> > > > +	int regval;
> > > > +
> > > > +	regval = readl(priv->baseaddr + ECC_CLR_OFST);
> > > > +	regval &= ~(DDR_CE_INTR_EN_MASK | DDR_UE_INTR_EN_MASK);
> > > > +	writel(regval, priv->baseaddr + ECC_CLR_OFST); }
> > > > +
> > > > +/* Interrupt Handler for ECC interrupts on imx8mp platform. */
> > > > +static irqreturn_t intr_handler_imx8mp(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 + ECC_STAT_OFST);
> > > > +	if (!(regval & ECC_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);
> > > > +	enable_intr_imx8mp(priv);
> > >
> > > Why do you enable interrupts here?
> >
> > Because zynqmp_get_error_info() wrote 0 to ECC_CLR_OFST, so here have
> to re-enable the interrupts.
> 
> This does not seem to be the right place for it.
> 
> > As said in the commit, the main difference between ZynqMP and i.MX8MP
> ddr controller is the interrupt registers.
> > ZynqMP use DDR QOS Interrupt registers,  but i.MX8MP use ECC_CLR_OFST
> Register(bit8 and bit9) to enable/disable the ce/ue interrupts.
> >
> > In zynqmp_get_error_info(), Zynqmp wrote 0 to ECC_CLR_OFST register to
> clear CE/UE error flags and counts, it has no effect on Zynqmp interrupts.
> > But for i.MX8MP, wirte 0 to ECC_CLR_OFST will disable i.MX8MP CE/UE
> interrupt, so need re-enable the interrupts.
> 
> All this shows one more time there should be separate handlers. You should
> get rid most callbacks in struct synps_platform_data and instead have
> separate probe functions for both flavors that share common code.

Do you mean that I should create specific callbacks in struct synps_platform_data for i.MX8MP instead reuse the ZynqMP functions?
But except this operation of write 0 to ECC_CLR_OFST in zynqmp_get_error_info(), 
the other callbacks in struct synps_platform_data are all the same between ZynqMP and i.MX8MP.

Best regards
Sherry Sun
Robert Richter April 8, 2020, 6:40 a.m. UTC | #5 
On 02.04.20 13:09:57, Sherry Sun wrote:
> > From: Robert Richter <rrichter@marvell.com>
> > On 02.04.20 09:06:27, Sherry Sun wrote:
> > > > From: Robert Richter <rrichter@marvell.com> On 02.04.20 09:20:32,

> > > > > +/* Interrupt Handler for ECC interrupts on imx8mp platform. */
> > > > > +static irqreturn_t intr_handler_imx8mp(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 + ECC_STAT_OFST);
> > > > > +	if (!(regval & ECC_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);
 > > > > > +	enable_intr_imx8mp(priv);
> > > >
> > > > Why do you enable interrupts here?
> > >
> > > Because zynqmp_get_error_info() wrote 0 to ECC_CLR_OFST, so here have
> > to re-enable the interrupts.
> > 
> > This does not seem to be the right place for it.
> > 
> > > As said in the commit, the main difference between ZynqMP and i.MX8MP
> > ddr controller is the interrupt registers.
> > > ZynqMP use DDR QOS Interrupt registers,  but i.MX8MP use ECC_CLR_OFST
> > Register(bit8 and bit9) to enable/disable the ce/ue interrupts.
> > >
> > > In zynqmp_get_error_info(), Zynqmp wrote 0 to ECC_CLR_OFST register to
> > clear CE/UE error flags and counts, it has no effect on Zynqmp interrupts.
> > > But for i.MX8MP, wirte 0 to ECC_CLR_OFST will disable i.MX8MP CE/UE
> > interrupt, so need re-enable the interrupts.
> > 
> > All this shows one more time there should be separate handlers. You should
> > get rid most callbacks in struct synps_platform_data and instead have
> > separate probe functions for both flavors that share common code.
> 
> Do you mean that I should create specific callbacks in struct synps_platform_data for i.MX8MP instead reuse the ZynqMP functions?
> But except this operation of write 0 to ECC_CLR_OFST in zynqmp_get_error_info(), 
> the other callbacks in struct synps_platform_data are all the same between ZynqMP and i.MX8MP.

Most callbacks in struct synps_platform_data will become obsolete if
there are either device tailored functions or if the width parameter
would be stored in private data and used by a common function.

-Robert
diff mbox series

Patch

diff --git a/drivers/edac/synopsys_edac.c b/drivers/edac/synopsys_edac.c
index 12211dc040e8..bf4202a24683 100644
--- a/drivers/edac/synopsys_edac.c
+++ b/drivers/edac/synopsys_edac.c
@@ -101,6 +101,7 @@ 
 /* DDR ECC Quirks */
 #define DDR_ECC_INTR_SUPPORT		BIT(0)
 #define DDR_ECC_DATA_POISON_SUPPORT	BIT(1)
+#define DDR_ECC_IMX8MP			BIT(2)
 
 /* ZynqMP Enhanced DDR memory controller registers that are relevant to ECC */
 /* ECC Configuration Registers */
@@ -266,6 +267,11 @@ 
 
 #define RANK_B0_BASE			6
 
+/* ECCCTL UE/CE Interrupt enable/disable for IMX8MP*/
+#define DDR_CE_INTR_EN_MASK			0x100
+#define DDR_UE_INTR_EN_MASK			0x200
+#define ECC_INTR_MASK				0x10100
+
 /**
  * struct ecc_error_info - ECC error log information.
  * @row:	Row number.
@@ -516,6 +522,54 @@  static void handle_error(struct mem_ctl_info *mci, struct synps_ecc_status *p)
 	memset(p, 0, sizeof(*p));
 }
 
+static void enable_intr_imx8mp(struct synps_edac_priv *priv)
+{
+	int regval;
+
+	regval = readl(priv->baseaddr + ECC_CLR_OFST);
+	regval |= (DDR_CE_INTR_EN_MASK | DDR_UE_INTR_EN_MASK);
+	writel(regval, priv->baseaddr + ECC_CLR_OFST);
+}
+
+static void disable_intr_imx8mp(struct synps_edac_priv *priv)
+{
+	int regval;
+
+	regval = readl(priv->baseaddr + ECC_CLR_OFST);
+	regval &= ~(DDR_CE_INTR_EN_MASK | DDR_UE_INTR_EN_MASK);
+	writel(regval, priv->baseaddr + ECC_CLR_OFST);
+}
+
+/* Interrupt Handler for ECC interrupts on imx8mp platform. */
+static irqreturn_t intr_handler_imx8mp(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 + ECC_STAT_OFST);
+	if (!(regval & ECC_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);
+	enable_intr_imx8mp(priv);
+
+	return IRQ_HANDLED;
+}
+
 /**
  * intr_handler - Interrupt Handler for ECC interrupts.
  * @irq:        IRQ number.
@@ -533,6 +587,9 @@  static irqreturn_t intr_handler(int irq, void *dev_id)
 	priv = mci->pvt_info;
 	p_data = priv->p_data;
 
+	if (p_data->quirks & DDR_ECC_IMX8MP)
+		return intr_handler_imx8mp(irq, dev_id);
+
 	regval = readl(priv->baseaddr + DDR_QOS_IRQ_STAT_OFST);
 	regval &= (DDR_QOSCE_MASK | DDR_QOSUE_MASK);
 	if (!(regval & ECC_CE_UE_INTR_MASK))
@@ -809,7 +866,7 @@  static void mc_init(struct mem_ctl_info *mci, struct platform_device *pdev)
 	platform_set_drvdata(pdev, mci);
 
 	/* Initialize controller capabilities and configuration */
-	mci->mtype_cap = MEM_FLAG_DDR3 | MEM_FLAG_DDR2;
+	mci->mtype_cap = MEM_FLAG_LRDDR4 | MEM_FLAG_DDR3 | MEM_FLAG_DDR2;
 	mci->edac_ctl_cap = EDAC_FLAG_NONE | EDAC_FLAG_SECDED;
 	mci->scrub_cap = SCRUB_HW_SRC;
 	mci->scrub_mode = SCRUB_NONE;
@@ -834,6 +891,9 @@  static void mc_init(struct mem_ctl_info *mci, struct platform_device *pdev)
 static void enable_intr(struct synps_edac_priv *priv)
 {
 	/* Enable UE/CE Interrupts */
+	if (priv->p_data->quirks & DDR_ECC_IMX8MP)
+		return enable_intr_imx8mp(priv);
+
 	writel(DDR_QOSUE_MASK | DDR_QOSCE_MASK,
 			priv->baseaddr + DDR_QOS_IRQ_EN_OFST);
 }
@@ -841,6 +901,9 @@  static void enable_intr(struct synps_edac_priv *priv)
 static void disable_intr(struct synps_edac_priv *priv)
 {
 	/* Disable UE/CE Interrupts */
+	if (priv->p_data->quirks & DDR_ECC_IMX8MP)
+		return disable_intr_imx8mp(priv);
+
 	writel(DDR_QOSUE_MASK | DDR_QOSCE_MASK,
 			priv->baseaddr + DDR_QOS_IRQ_DB_OFST);
 }
@@ -890,6 +953,14 @@  static const struct synps_platform_data zynqmp_edac_def = {
 			  ),
 };
 
+static const struct synps_platform_data imx8mp_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 | DDR_ECC_IMX8MP),
+};
+
 static const struct of_device_id synps_edac_match[] = {
 	{
 		.compatible = "xlnx,zynq-ddrc-a05",
@@ -899,6 +970,10 @@  static const struct of_device_id synps_edac_match[] = {
 		.compatible = "xlnx,zynqmp-ddrc-2.40a",
 		.data = (void *)&zynqmp_edac_def
 	},
+	{
+		.compatible = "fsl,imx8mp-ddrc",
+		.data = (void *)&imx8mp_edac_def
+	},
 	{
 		/* end of table */
 	}