| Message ID | 20241011-imx95_edac-v2-5-011b68290951@nxp.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | EDAC/fsl-ddr: Add imx9 support | expand |
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?
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
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.
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);