[PATCH v2 5/6] EDAC/fsl_ddr: Add support for i.MX9 DDR controller

Frank Li posted 6 patches 1 month, 2 weeks ago
There is a newer version of this series
[PATCH v2 5/6] EDAC/fsl_ddr: Add support for i.MX9 DDR controller
Posted by Frank Li 1 month, 2 weeks ago
From: Ye Li <ye.li@nxp.com>

Add support for the i.MX9 DDR controller, which has different register
offsets and some function changes compared to the existing fsl_ddr
controller. The ECC and error injection functions are almost the same, so
update and reuse the driver for i.MX9. A special type 'TYPE_IMX9' is added
specifically for the i.MX9 controller to distinguish the differences.

Signed-off-by: Ye Li <ye.li@nxp.com>
Reviewed-by: Peng Fan <peng.fan@nxp.com>
Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
 drivers/edac/fsl_ddr_edac.c    | 48 ++++++++++++++++++++++++++++++++++++------
 drivers/edac/fsl_ddr_edac.h    | 10 +++++++++
 drivers/edac/layerscape_edac.c |  1 +
 3 files changed, 53 insertions(+), 6 deletions(-)

diff --git a/drivers/edac/fsl_ddr_edac.c b/drivers/edac/fsl_ddr_edac.c
index ccc13c2adfd6f..3e4c2869a07cd 100644
--- a/drivers/edac/fsl_ddr_edac.c
+++ b/drivers/edac/fsl_ddr_edac.c
@@ -31,16 +31,28 @@
 
 static int edac_mc_idx;
 
+static inline void __iomem *ddr_reg_addr(struct fsl_mc_pdata *pdata, unsigned int off)
+{
+	if (pdata->flag == TYPE_IMX9 && off >= FSL_MC_DATA_ERR_INJECT_HI && off <= FSL_MC_ERR_SBE)
+		return pdata->inject_vbase + off - FSL_MC_DATA_ERR_INJECT_HI
+		       + IMX9_MC_DATA_ERR_INJECT_OFF;
+
+	if (pdata->flag == TYPE_IMX9 && off >= IMX9_MC_ERR_EN)
+		return pdata->inject_vbase + off - IMX9_MC_ERR_EN;
+
+	return pdata->mc_vbase + off;
+}
+
 static inline u32 ddr_in32(struct fsl_mc_pdata *pdata, unsigned int off)
 {
-	void __iomem *addr = pdata->mc_vbase + off;
+	void __iomem *addr = ddr_reg_addr(pdata, off);
 
 	return pdata->little_endian ? ioread32(addr) : ioread32be(addr);
 }
 
 static inline void ddr_out32(struct fsl_mc_pdata *pdata, unsigned int off, u32 value)
 {
-	void __iomem *addr = pdata->mc_vbase + off;
+	void __iomem *addr = ddr_reg_addr(pdata, off);
 
 	if (pdata->little_endian)
 		iowrite32(value, addr);
@@ -438,6 +450,9 @@ static void fsl_ddr_init_csrows(struct mem_ctl_info *mci)
 		case 0x05000000:
 			mtype = MEM_DDR4;
 			break;
+		case 0x04000000:
+			mtype = MEM_LPDDR4;
+			break;
 		default:
 			mtype = MEM_UNKNOWN;
 			break;
@@ -471,7 +486,9 @@ static void fsl_ddr_init_csrows(struct mem_ctl_info *mci)
 		dimm->grain = 8;
 		dimm->mtype = mtype;
 		dimm->dtype = DEV_UNKNOWN;
-		if (sdram_ctl & DSC_X32_EN)
+		if (pdata->flag == TYPE_IMX9)
+			dimm->dtype = DEV_X16;
+		else if (sdram_ctl & DSC_X32_EN)
 			dimm->dtype = DEV_X32;
 		dimm->edac_mode = EDAC_SECDED;
 	}
@@ -483,6 +500,7 @@ int fsl_mc_err_probe(struct platform_device *op)
 	struct edac_mc_layer layers[2];
 	struct fsl_mc_pdata *pdata;
 	struct resource r;
+	u32 ecc_en_mask;
 	u32 sdram_ctl;
 	int res;
 
@@ -510,6 +528,8 @@ int fsl_mc_err_probe(struct platform_device *op)
 	mci->ctl_name = pdata->name;
 	mci->dev_name = pdata->name;
 
+	pdata->flag = (unsigned long)device_get_match_data(&op->dev);
+
 	/*
 	 * Get the endianness of DDR controller registers.
 	 * Default is big endian.
@@ -538,8 +558,23 @@ int fsl_mc_err_probe(struct platform_device *op)
 		goto err;
 	}
 
-	sdram_ctl = ddr_in32(pdata, FSL_MC_DDR_SDRAM_CFG);
-	if (!(sdram_ctl & DSC_ECC_EN)) {
+	if (pdata->flag == TYPE_IMX9) {
+		pdata->inject_vbase = devm_platform_ioremap_resource_byname(op, "inject");
+		if (IS_ERR(pdata->inject_vbase)) {
+			res = -ENOMEM;
+			goto err;
+		}
+	}
+
+	if (pdata->flag == TYPE_IMX9) {
+		sdram_ctl = ddr_in32(pdata, IMX9_MC_ERR_EN);
+		ecc_en_mask = ERR_ECC_EN | ERR_INLINE_ECC;
+	} else {
+		sdram_ctl = ddr_in32(pdata, FSL_MC_DDR_SDRAM_CFG);
+		ecc_en_mask = DSC_ECC_EN;
+	}
+
+	if ((sdram_ctl & ecc_en_mask) != ecc_en_mask) {
 		/* no ECC */
 		pr_warn("%s: No ECC DIMMs discovered\n", __func__);
 		res = -ENODEV;
@@ -550,7 +585,8 @@ int fsl_mc_err_probe(struct platform_device *op)
 	mci->mtype_cap = MEM_FLAG_DDR | MEM_FLAG_RDDR |
 			 MEM_FLAG_DDR2 | MEM_FLAG_RDDR2 |
 			 MEM_FLAG_DDR3 | MEM_FLAG_RDDR3 |
-			 MEM_FLAG_DDR4 | MEM_FLAG_RDDR4;
+			 MEM_FLAG_DDR4 | MEM_FLAG_RDDR4 |
+			 MEM_FLAG_LPDDR4;
 	mci->edac_ctl_cap = EDAC_FLAG_NONE | EDAC_FLAG_SECDED;
 	mci->edac_cap = EDAC_FLAG_SECDED;
 	mci->mod_name = EDAC_MOD_STR;
diff --git a/drivers/edac/fsl_ddr_edac.h b/drivers/edac/fsl_ddr_edac.h
index de66f9822fba1..73618f79e587f 100644
--- a/drivers/edac/fsl_ddr_edac.h
+++ b/drivers/edac/fsl_ddr_edac.h
@@ -39,6 +39,9 @@
 #define FSL_MC_CAPTURE_EXT_ADDRESS	0x0e54
 #define FSL_MC_ERR_SBE		0x0e58
 
+#define IMX9_MC_ERR_EN			0x1000
+#define IMX9_MC_DATA_ERR_INJECT_OFF	0x100
+
 #define DSC_MEM_EN	0x80000000
 #define DSC_ECC_EN	0x20000000
 #define DSC_RD_EN	0x10000000
@@ -46,6 +49,9 @@
 #define DSC_DBW_32	0x00080000
 #define DSC_DBW_64	0x00000000
 
+#define ERR_ECC_EN      0x80000000
+#define ERR_INLINE_ECC  0x40000000
+
 #define DSC_SDTYPE_MASK		0x07000000
 #define DSC_X32_EN	0x00000020
 
@@ -65,14 +71,18 @@
 #define	DDR_EDI_SBED	0x4	/* single-bit ECC error disable */
 #define	DDR_EDI_MBED	0x8	/* multi-bit ECC error disable */
 
+#define TYPE_IMX9	0x1	/* MC used by iMX9 having registers changed */
+
 struct fsl_mc_pdata {
 	char *name;
 	int edac_idx;
 	void __iomem *mc_vbase;
+	void __iomem *inject_vbase;
 	int irq;
 	u32 orig_ddr_err_disable;
 	u32 orig_ddr_err_sbe;
 	bool little_endian;
+	unsigned long flag;
 };
 int fsl_mc_err_probe(struct platform_device *op);
 void fsl_mc_err_remove(struct platform_device *op);
diff --git a/drivers/edac/layerscape_edac.c b/drivers/edac/layerscape_edac.c
index 0d42c1238908b..9a0c92ebbc3c4 100644
--- a/drivers/edac/layerscape_edac.c
+++ b/drivers/edac/layerscape_edac.c
@@ -21,6 +21,7 @@
 
 static const struct of_device_id fsl_ddr_mc_err_of_match[] = {
 	{ .compatible = "fsl,qoriq-memory-controller", },
+	{ .compatible = "nxp,imx9-memory-controller", .data = (void *)TYPE_IMX9, },
 	{},
 };
 MODULE_DEVICE_TABLE(of, fsl_ddr_mc_err_of_match);

-- 
2.34.1
Re: [PATCH v2 5/6] EDAC/fsl_ddr: Add support for i.MX9 DDR controller
Posted by Borislav Petkov 1 month, 1 week ago
On Fri, Oct 11, 2024 at 11:31:33AM -0400, Frank Li wrote:
> From: Ye Li <ye.li@nxp.com>
> 
> Add support for the i.MX9 DDR controller, which has different register
> offsets and some function changes compared to the existing fsl_ddr
> controller. The ECC and error injection functions are almost the same, so
> update and reuse the driver for i.MX9. A special type 'TYPE_IMX9' is added
> specifically for the i.MX9 controller to distinguish the differences.
> 
> Signed-off-by: Ye Li <ye.li@nxp.com>
> Reviewed-by: Peng Fan <peng.fan@nxp.com>
> Signed-off-by: Frank Li <Frank.Li@nxp.com>
> ---
>  drivers/edac/fsl_ddr_edac.c    | 48 ++++++++++++++++++++++++++++++++++++------
>  drivers/edac/fsl_ddr_edac.h    | 10 +++++++++
>  drivers/edac/layerscape_edac.c |  1 +
>  3 files changed, 53 insertions(+), 6 deletions(-)

checking file drivers/edac/fsl_ddr_edac.c
Hunk #1 FAILED at 31.
Hunk #2 succeeded at 442 (offset 4 lines).
Hunk #3 succeeded at 478 (offset 4 lines).
Hunk #4 succeeded at 492 (offset 4 lines).
Hunk #5 succeeded at 520 (offset 4 lines).
Hunk #6 succeeded at 550 (offset 4 lines).
Hunk #7 succeeded at 577 (offset 4 lines).
1 out of 7 hunks FAILED
checking file drivers/edac/fsl_ddr_edac.h
Hunk #3 FAILED at 71.
1 out of 3 hunks FAILED
checking file drivers/edac/layerscape_edac.c

What tree have you created your patches against?

> diff --git a/drivers/edac/fsl_ddr_edac.c b/drivers/edac/fsl_ddr_edac.c
> index ccc13c2adfd6f..3e4c2869a07cd 100644
> --- a/drivers/edac/fsl_ddr_edac.c
> +++ b/drivers/edac/fsl_ddr_edac.c
> @@ -31,16 +31,28 @@
>  
>  static int edac_mc_idx;

I see here:

|static int edac_mc_idx;
|
|static u32 orig_ddr_err_disable; 
|static u32 orig_ddr_err_sbe;
|static bool little_endian;
|
|static inline u32 ddr_in32(struct fsl_mc_pdata *pdata, unsigned int off)

and you don't have those in your diff.

What's up?

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH v2 5/6] EDAC/fsl_ddr: Add support for i.MX9 DDR controller
Posted by Frank Li 1 month, 1 week ago
On Tue, Oct 15, 2024 at 04:52:34PM +0200, Borislav Petkov wrote:
> On Fri, Oct 11, 2024 at 11:31:33AM -0400, Frank Li wrote:
> > From: Ye Li <ye.li@nxp.com>
> >
> > Add support for the i.MX9 DDR controller, which has different register
> > offsets and some function changes compared to the existing fsl_ddr
> > controller. The ECC and error injection functions are almost the same, so
> > update and reuse the driver for i.MX9. A special type 'TYPE_IMX9' is added
> > specifically for the i.MX9 controller to distinguish the differences.
> >
> > Signed-off-by: Ye Li <ye.li@nxp.com>
> > Reviewed-by: Peng Fan <peng.fan@nxp.com>
> > Signed-off-by: Frank Li <Frank.Li@nxp.com>
> > ---
> >  drivers/edac/fsl_ddr_edac.c    | 48 ++++++++++++++++++++++++++++++++++++------
> >  drivers/edac/fsl_ddr_edac.h    | 10 +++++++++
> >  drivers/edac/layerscape_edac.c |  1 +
> >  3 files changed, 53 insertions(+), 6 deletions(-)
>
> checking file drivers/edac/fsl_ddr_edac.c
> Hunk #1 FAILED at 31.
> Hunk #2 succeeded at 442 (offset 4 lines).
> Hunk #3 succeeded at 478 (offset 4 lines).
> Hunk #4 succeeded at 492 (offset 4 lines).
> Hunk #5 succeeded at 520 (offset 4 lines).
> Hunk #6 succeeded at 550 (offset 4 lines).
> Hunk #7 succeeded at 577 (offset 4 lines).
> 1 out of 7 hunks FAILED
> checking file drivers/edac/fsl_ddr_edac.h
> Hunk #3 FAILED at 71.
> 1 out of 3 hunks FAILED
> checking file drivers/edac/layerscape_edac.c
>
> What tree have you created your patches against?

I base on linux-next: next-20241010. I can rebase to edac-for-next
https://git.kernel.org/pub/scm/linux/kernel/git/ras/ras.git/

Is it what you expect base?

Frank

>
> > diff --git a/drivers/edac/fsl_ddr_edac.c b/drivers/edac/fsl_ddr_edac.c
> > index ccc13c2adfd6f..3e4c2869a07cd 100644
> > --- a/drivers/edac/fsl_ddr_edac.c
> > +++ b/drivers/edac/fsl_ddr_edac.c
> > @@ -31,16 +31,28 @@
> >
> >  static int edac_mc_idx;
>
> I see here:
>
> |static int edac_mc_idx;
> |
> |static u32 orig_ddr_err_disable;
> |static u32 orig_ddr_err_sbe;
> |static bool little_endian;
> |
> |static inline u32 ddr_in32(struct fsl_mc_pdata *pdata, unsigned int off)
>
> and you don't have those in your diff.
>
> What's up?
>
> --
> Regards/Gruss,
>     Boris.
>
> https://people.kernel.org/tglx/notes-about-netiquette
Re: [PATCH v2 5/6] EDAC/fsl_ddr: Add support for i.MX9 DDR controller
Posted by Borislav Petkov 1 month, 1 week ago
On Tue, Oct 15, 2024 at 11:19:42AM -0400, Frank Li wrote:
> I base on linux-next: next-20241010.

Found what the problem is. Your patch 2/6 was in my spam folder. And that one
removes those variables.

> I can rebase to edac-for-next
> https://git.kernel.org/pub/scm/linux/kernel/git/ras/ras.git/
> 
> Is it what you expect base?

Yes, please.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette