[PATCH 0/6] EDAC: fsl-ddr, add imx9 support

[PATCH 0/6] EDAC: fsl-ddr, add imx9 support

[Frank Li]

Add imx9 support for fsl-ddr.

Patch 1-2 is prepare patch, no function chagne
Patch 3 is small fix for bit shift
Patch 4 is dt binding patch.
Patch 5 is driver change to support imx9
Patch 6 is imx93 dts change

Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
Frank Li (4):
      EDAC: fsl_ddr: Pass down fsl_mc_pdata in ddr_in32() and ddr_out32()
      EDAC, fsl_ddr: Move global variable into struct fsl_mc_pdata
      dt-bindings: memory: fsl: Add compatible string nxp,imx9-memory-controller
      arm64: dts: imx93: add ddr edac support

Priyanka Singh (1):
      EDAC: fsl: Fix bad bit shift operations

Ye Li (1):
      EDAC: fsl_ddr: Add support for i.MX9 DDR controller

 .../bindings/memory-controllers/fsl/fsl,ddr.yaml   |  31 ++++-
 arch/arm64/boot/dts/freescale/imx93.dtsi           |   8 ++
 drivers/edac/fsl_ddr_edac.c                        | 134 ++++++++++++++-------
 drivers/edac/fsl_ddr_edac.h                        |  13 ++
 drivers/edac/layerscape_edac.c                     |   1 +
 5 files changed, 141 insertions(+), 46 deletions(-)
---
base-commit: 82d01fe6ee52086035b201cfa1410a3b04384257
change-id: 20240704-imx95_edac-209cec208446

Best regards,
---
Frank Li <Frank.Li@nxp.com>

[PATCH 1/6] EDAC: fsl_ddr: Pass down fsl_mc_pdata in ddr_in32() and ddr_out32()

[Frank Li]

Pass down fsl_mc_data in help function ddr_in32() and ddr_out32() to
prepare add iMX9 support. iMX9 have a little difference register layout.

No functional change.

Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
 drivers/edac/fsl_ddr_edac.c | 62 ++++++++++++++++++++++++---------------------
 1 file changed, 33 insertions(+), 29 deletions(-)

diff --git a/drivers/edac/fsl_ddr_edac.c b/drivers/edac/fsl_ddr_edac.c
index d148d262d0d4d..a7982370e2381 100644
--- a/drivers/edac/fsl_ddr_edac.c
+++ b/drivers/edac/fsl_ddr_edac.c
@@ -35,13 +35,17 @@ static u32 orig_ddr_err_disable;
 static u32 orig_ddr_err_sbe;
 static bool little_endian;
 
-static inline u32 ddr_in32(void __iomem *addr)
+static inline u32 ddr_in32(struct fsl_mc_pdata *pdata, unsigned int off)
 {
+	void __iomem *addr = pdata->mc_vbase + off;
+
 	return little_endian ? ioread32(addr) : ioread32be(addr);
 }
 
-static inline void ddr_out32(void __iomem *addr, u32 value)
+static inline void ddr_out32(struct fsl_mc_pdata *pdata, unsigned int off, u32 value)
 {
+	void __iomem *addr = pdata->mc_vbase + off;
+
 	if (little_endian)
 		iowrite32(value, addr);
 	else
@@ -60,7 +64,7 @@ static ssize_t fsl_mc_inject_data_hi_show(struct device *dev,
 	struct mem_ctl_info *mci = to_mci(dev);
 	struct fsl_mc_pdata *pdata = mci->pvt_info;
 	return sprintf(data, "0x%08x",
-		       ddr_in32(pdata->mc_vbase + FSL_MC_DATA_ERR_INJECT_HI));
+		       ddr_in32(pdata, FSL_MC_DATA_ERR_INJECT_HI));
 }
 
 static ssize_t fsl_mc_inject_data_lo_show(struct device *dev,
@@ -70,7 +74,7 @@ static ssize_t fsl_mc_inject_data_lo_show(struct device *dev,
 	struct mem_ctl_info *mci = to_mci(dev);
 	struct fsl_mc_pdata *pdata = mci->pvt_info;
 	return sprintf(data, "0x%08x",
-		       ddr_in32(pdata->mc_vbase + FSL_MC_DATA_ERR_INJECT_LO));
+		       ddr_in32(pdata, FSL_MC_DATA_ERR_INJECT_LO));
 }
 
 static ssize_t fsl_mc_inject_ctrl_show(struct device *dev,
@@ -80,7 +84,7 @@ static ssize_t fsl_mc_inject_ctrl_show(struct device *dev,
 	struct mem_ctl_info *mci = to_mci(dev);
 	struct fsl_mc_pdata *pdata = mci->pvt_info;
 	return sprintf(data, "0x%08x",
-		       ddr_in32(pdata->mc_vbase + FSL_MC_ECC_ERR_INJECT));
+		       ddr_in32(pdata, FSL_MC_ECC_ERR_INJECT));
 }
 
 static ssize_t fsl_mc_inject_data_hi_store(struct device *dev,
@@ -97,7 +101,7 @@ static ssize_t fsl_mc_inject_data_hi_store(struct device *dev,
 		if (rc)
 			return rc;
 
-		ddr_out32(pdata->mc_vbase + FSL_MC_DATA_ERR_INJECT_HI, val);
+		ddr_out32(pdata, FSL_MC_DATA_ERR_INJECT_HI, val);
 		return count;
 	}
 	return 0;
@@ -117,7 +121,7 @@ static ssize_t fsl_mc_inject_data_lo_store(struct device *dev,
 		if (rc)
 			return rc;
 
-		ddr_out32(pdata->mc_vbase + FSL_MC_DATA_ERR_INJECT_LO, val);
+		ddr_out32(pdata, FSL_MC_DATA_ERR_INJECT_LO, val);
 		return count;
 	}
 	return 0;
@@ -137,7 +141,7 @@ static ssize_t fsl_mc_inject_ctrl_store(struct device *dev,
 		if (rc)
 			return rc;
 
-		ddr_out32(pdata->mc_vbase + FSL_MC_ECC_ERR_INJECT, val);
+		ddr_out32(pdata, FSL_MC_ECC_ERR_INJECT, val);
 		return count;
 	}
 	return 0;
@@ -286,7 +290,7 @@ static void fsl_mc_check(struct mem_ctl_info *mci)
 	int bad_data_bit;
 	int bad_ecc_bit;
 
-	err_detect = ddr_in32(pdata->mc_vbase + FSL_MC_ERR_DETECT);
+	err_detect = ddr_in32(pdata, FSL_MC_ERR_DETECT);
 	if (!err_detect)
 		return;
 
@@ -295,14 +299,14 @@ static void fsl_mc_check(struct mem_ctl_info *mci)
 
 	/* no more processing if not ECC bit errors */
 	if (!(err_detect & (DDR_EDE_SBE | DDR_EDE_MBE))) {
-		ddr_out32(pdata->mc_vbase + FSL_MC_ERR_DETECT, err_detect);
+		ddr_out32(pdata, FSL_MC_ERR_DETECT, err_detect);
 		return;
 	}
 
-	syndrome = ddr_in32(pdata->mc_vbase + FSL_MC_CAPTURE_ECC);
+	syndrome = ddr_in32(pdata, FSL_MC_CAPTURE_ECC);
 
 	/* Mask off appropriate bits of syndrome based on bus width */
-	bus_width = (ddr_in32(pdata->mc_vbase + FSL_MC_DDR_SDRAM_CFG) &
+	bus_width = (ddr_in32(pdata, FSL_MC_DDR_SDRAM_CFG) &
 		     DSC_DBW_MASK) ? 32 : 64;
 	if (bus_width == 64)
 		syndrome &= 0xff;
@@ -310,8 +314,8 @@ static void fsl_mc_check(struct mem_ctl_info *mci)
 		syndrome &= 0xffff;
 
 	err_addr = make64(
-		ddr_in32(pdata->mc_vbase + FSL_MC_CAPTURE_EXT_ADDRESS),
-		ddr_in32(pdata->mc_vbase + FSL_MC_CAPTURE_ADDRESS));
+		ddr_in32(pdata, FSL_MC_CAPTURE_EXT_ADDRESS),
+		ddr_in32(pdata, FSL_MC_CAPTURE_ADDRESS));
 	pfn = err_addr >> PAGE_SHIFT;
 
 	for (row_index = 0; row_index < mci->nr_csrows; row_index++) {
@@ -320,8 +324,8 @@ static void fsl_mc_check(struct mem_ctl_info *mci)
 			break;
 	}
 
-	cap_high = ddr_in32(pdata->mc_vbase + FSL_MC_CAPTURE_DATA_HI);
-	cap_low = ddr_in32(pdata->mc_vbase + FSL_MC_CAPTURE_DATA_LO);
+	cap_high = ddr_in32(pdata, FSL_MC_CAPTURE_DATA_HI);
+	cap_low = ddr_in32(pdata, FSL_MC_CAPTURE_DATA_LO);
 
 	/*
 	 * Analyze single-bit errors on 64-bit wide buses
@@ -367,7 +371,7 @@ static void fsl_mc_check(struct mem_ctl_info *mci)
 				     row_index, 0, -1,
 				     mci->ctl_name, "");
 
-	ddr_out32(pdata->mc_vbase + FSL_MC_ERR_DETECT, err_detect);
+	ddr_out32(pdata, FSL_MC_ERR_DETECT, err_detect);
 }
 
 static irqreturn_t fsl_mc_isr(int irq, void *dev_id)
@@ -376,7 +380,7 @@ static irqreturn_t fsl_mc_isr(int irq, void *dev_id)
 	struct fsl_mc_pdata *pdata = mci->pvt_info;
 	u32 err_detect;
 
-	err_detect = ddr_in32(pdata->mc_vbase + FSL_MC_ERR_DETECT);
+	err_detect = ddr_in32(pdata, FSL_MC_ERR_DETECT);
 	if (!err_detect)
 		return IRQ_NONE;
 
@@ -396,7 +400,7 @@ static void fsl_ddr_init_csrows(struct mem_ctl_info *mci)
 	u32 cs_bnds;
 	int index;
 
-	sdram_ctl = ddr_in32(pdata->mc_vbase + FSL_MC_DDR_SDRAM_CFG);
+	sdram_ctl = ddr_in32(pdata, FSL_MC_DDR_SDRAM_CFG);
 
 	sdtype = sdram_ctl & DSC_SDTYPE_MASK;
 	if (sdram_ctl & DSC_RD_EN) {
@@ -444,7 +448,7 @@ static void fsl_ddr_init_csrows(struct mem_ctl_info *mci)
 		csrow = mci->csrows[index];
 		dimm = csrow->channels[0]->dimm;
 
-		cs_bnds = ddr_in32(pdata->mc_vbase + FSL_MC_CS_BNDS_0 +
+		cs_bnds = ddr_in32(pdata, FSL_MC_CS_BNDS_0 +
 				   (index * FSL_MC_CS_BNDS_OFS));
 
 		start = (cs_bnds & 0xffff0000) >> 16;
@@ -558,11 +562,11 @@ int fsl_mc_err_probe(struct platform_device *op)
 	fsl_ddr_init_csrows(mci);
 
 	/* store the original error disable bits */
-	orig_ddr_err_disable = ddr_in32(pdata->mc_vbase + FSL_MC_ERR_DISABLE);
-	ddr_out32(pdata->mc_vbase + FSL_MC_ERR_DISABLE, 0);
+	orig_ddr_err_disable = ddr_in32(pdata, FSL_MC_ERR_DISABLE);
+	ddr_out32(pdata, FSL_MC_ERR_DISABLE, 0);
 
 	/* clear all error bits */
-	ddr_out32(pdata->mc_vbase + FSL_MC_ERR_DETECT, ~0);
+	ddr_out32(pdata, FSL_MC_ERR_DETECT, ~0);
 
 	res = edac_mc_add_mc_with_groups(mci, fsl_ddr_dev_groups);
 	if (res) {
@@ -571,15 +575,15 @@ int fsl_mc_err_probe(struct platform_device *op)
 	}
 
 	if (edac_op_state == EDAC_OPSTATE_INT) {
-		ddr_out32(pdata->mc_vbase + FSL_MC_ERR_INT_EN,
+		ddr_out32(pdata, FSL_MC_ERR_INT_EN,
 			  DDR_EIE_MBEE | DDR_EIE_SBEE);
 
 		/* store the original error management threshold */
-		orig_ddr_err_sbe = ddr_in32(pdata->mc_vbase +
+		orig_ddr_err_sbe = ddr_in32(pdata,
 					    FSL_MC_ERR_SBE) & 0xff0000;
 
 		/* set threshold to 1 error per interrupt */
-		ddr_out32(pdata->mc_vbase + FSL_MC_ERR_SBE, 0x10000);
+		ddr_out32(pdata, FSL_MC_ERR_SBE, 0x10000);
 
 		/* register interrupts */
 		pdata->irq = platform_get_irq(op, 0);
@@ -620,12 +624,12 @@ void fsl_mc_err_remove(struct platform_device *op)
 	edac_dbg(0, "\n");
 
 	if (edac_op_state == EDAC_OPSTATE_INT) {
-		ddr_out32(pdata->mc_vbase + FSL_MC_ERR_INT_EN, 0);
+		ddr_out32(pdata, FSL_MC_ERR_INT_EN, 0);
 	}
 
-	ddr_out32(pdata->mc_vbase + FSL_MC_ERR_DISABLE,
+	ddr_out32(pdata, FSL_MC_ERR_DISABLE,
 		  orig_ddr_err_disable);
-	ddr_out32(pdata->mc_vbase + FSL_MC_ERR_SBE, orig_ddr_err_sbe);
+	ddr_out32(pdata, FSL_MC_ERR_SBE, orig_ddr_err_sbe);
 
 	edac_mc_del_mc(&op->dev);
 	edac_mc_free(mci);

-- 
2.34.1

Re: [PATCH 1/6] EDAC: fsl_ddr: Pass down fsl_mc_pdata in ddr_in32() and ddr_out32()

[Frank Li]

On Tue, Jul 09, 2024 at 04:23:02PM -0400, Frank Li wrote:
> Pass down fsl_mc_data in help function ddr_in32() and ddr_out32() to
> prepare add iMX9 support. iMX9 have a little difference register layout.
>
> No functional change.
>
> Signed-off-by: Frank Li <Frank.Li@nxp.com>
> ---

Ping?

Frank

>  drivers/edac/fsl_ddr_edac.c | 62 ++++++++++++++++++++++++---------------------
>  1 file changed, 33 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/edac/fsl_ddr_edac.c b/drivers/edac/fsl_ddr_edac.c
> index d148d262d0d4d..a7982370e2381 100644
> --- a/drivers/edac/fsl_ddr_edac.c
> +++ b/drivers/edac/fsl_ddr_edac.c
> @@ -35,13 +35,17 @@ static u32 orig_ddr_err_disable;
>  static u32 orig_ddr_err_sbe;
>  static bool little_endian;
>
> -static inline u32 ddr_in32(void __iomem *addr)
> +static inline u32 ddr_in32(struct fsl_mc_pdata *pdata, unsigned int off)
>  {
> +	void __iomem *addr = pdata->mc_vbase + off;
> +
>  	return little_endian ? ioread32(addr) : ioread32be(addr);
>  }
>
> -static inline void ddr_out32(void __iomem *addr, u32 value)
> +static inline void ddr_out32(struct fsl_mc_pdata *pdata, unsigned int off, u32 value)
>  {
> +	void __iomem *addr = pdata->mc_vbase + off;
> +
>  	if (little_endian)
>  		iowrite32(value, addr);
>  	else
> @@ -60,7 +64,7 @@ static ssize_t fsl_mc_inject_data_hi_show(struct device *dev,
>  	struct mem_ctl_info *mci = to_mci(dev);
>  	struct fsl_mc_pdata *pdata = mci->pvt_info;
>  	return sprintf(data, "0x%08x",
> -		       ddr_in32(pdata->mc_vbase + FSL_MC_DATA_ERR_INJECT_HI));
> +		       ddr_in32(pdata, FSL_MC_DATA_ERR_INJECT_HI));
>  }
>
>  static ssize_t fsl_mc_inject_data_lo_show(struct device *dev,
> @@ -70,7 +74,7 @@ static ssize_t fsl_mc_inject_data_lo_show(struct device *dev,
>  	struct mem_ctl_info *mci = to_mci(dev);
>  	struct fsl_mc_pdata *pdata = mci->pvt_info;
>  	return sprintf(data, "0x%08x",
> -		       ddr_in32(pdata->mc_vbase + FSL_MC_DATA_ERR_INJECT_LO));
> +		       ddr_in32(pdata, FSL_MC_DATA_ERR_INJECT_LO));
>  }
>
>  static ssize_t fsl_mc_inject_ctrl_show(struct device *dev,
> @@ -80,7 +84,7 @@ static ssize_t fsl_mc_inject_ctrl_show(struct device *dev,
>  	struct mem_ctl_info *mci = to_mci(dev);
>  	struct fsl_mc_pdata *pdata = mci->pvt_info;
>  	return sprintf(data, "0x%08x",
> -		       ddr_in32(pdata->mc_vbase + FSL_MC_ECC_ERR_INJECT));
> +		       ddr_in32(pdata, FSL_MC_ECC_ERR_INJECT));
>  }
>
>  static ssize_t fsl_mc_inject_data_hi_store(struct device *dev,
> @@ -97,7 +101,7 @@ static ssize_t fsl_mc_inject_data_hi_store(struct device *dev,
>  		if (rc)
>  			return rc;
>
> -		ddr_out32(pdata->mc_vbase + FSL_MC_DATA_ERR_INJECT_HI, val);
> +		ddr_out32(pdata, FSL_MC_DATA_ERR_INJECT_HI, val);
>  		return count;
>  	}
>  	return 0;
> @@ -117,7 +121,7 @@ static ssize_t fsl_mc_inject_data_lo_store(struct device *dev,
>  		if (rc)
>  			return rc;
>
> -		ddr_out32(pdata->mc_vbase + FSL_MC_DATA_ERR_INJECT_LO, val);
> +		ddr_out32(pdata, FSL_MC_DATA_ERR_INJECT_LO, val);
>  		return count;
>  	}
>  	return 0;
> @@ -137,7 +141,7 @@ static ssize_t fsl_mc_inject_ctrl_store(struct device *dev,
>  		if (rc)
>  			return rc;
>
> -		ddr_out32(pdata->mc_vbase + FSL_MC_ECC_ERR_INJECT, val);
> +		ddr_out32(pdata, FSL_MC_ECC_ERR_INJECT, val);
>  		return count;
>  	}
>  	return 0;
> @@ -286,7 +290,7 @@ static void fsl_mc_check(struct mem_ctl_info *mci)
>  	int bad_data_bit;
>  	int bad_ecc_bit;
>
> -	err_detect = ddr_in32(pdata->mc_vbase + FSL_MC_ERR_DETECT);
> +	err_detect = ddr_in32(pdata, FSL_MC_ERR_DETECT);
>  	if (!err_detect)
>  		return;
>
> @@ -295,14 +299,14 @@ static void fsl_mc_check(struct mem_ctl_info *mci)
>
>  	/* no more processing if not ECC bit errors */
>  	if (!(err_detect & (DDR_EDE_SBE | DDR_EDE_MBE))) {
> -		ddr_out32(pdata->mc_vbase + FSL_MC_ERR_DETECT, err_detect);
> +		ddr_out32(pdata, FSL_MC_ERR_DETECT, err_detect);
>  		return;
>  	}
>
> -	syndrome = ddr_in32(pdata->mc_vbase + FSL_MC_CAPTURE_ECC);
> +	syndrome = ddr_in32(pdata, FSL_MC_CAPTURE_ECC);
>
>  	/* Mask off appropriate bits of syndrome based on bus width */
> -	bus_width = (ddr_in32(pdata->mc_vbase + FSL_MC_DDR_SDRAM_CFG) &
> +	bus_width = (ddr_in32(pdata, FSL_MC_DDR_SDRAM_CFG) &
>  		     DSC_DBW_MASK) ? 32 : 64;
>  	if (bus_width == 64)
>  		syndrome &= 0xff;
> @@ -310,8 +314,8 @@ static void fsl_mc_check(struct mem_ctl_info *mci)
>  		syndrome &= 0xffff;
>
>  	err_addr = make64(
> -		ddr_in32(pdata->mc_vbase + FSL_MC_CAPTURE_EXT_ADDRESS),
> -		ddr_in32(pdata->mc_vbase + FSL_MC_CAPTURE_ADDRESS));
> +		ddr_in32(pdata, FSL_MC_CAPTURE_EXT_ADDRESS),
> +		ddr_in32(pdata, FSL_MC_CAPTURE_ADDRESS));
>  	pfn = err_addr >> PAGE_SHIFT;
>
>  	for (row_index = 0; row_index < mci->nr_csrows; row_index++) {
> @@ -320,8 +324,8 @@ static void fsl_mc_check(struct mem_ctl_info *mci)
>  			break;
>  	}
>
> -	cap_high = ddr_in32(pdata->mc_vbase + FSL_MC_CAPTURE_DATA_HI);
> -	cap_low = ddr_in32(pdata->mc_vbase + FSL_MC_CAPTURE_DATA_LO);
> +	cap_high = ddr_in32(pdata, FSL_MC_CAPTURE_DATA_HI);
> +	cap_low = ddr_in32(pdata, FSL_MC_CAPTURE_DATA_LO);
>
>  	/*
>  	 * Analyze single-bit errors on 64-bit wide buses
> @@ -367,7 +371,7 @@ static void fsl_mc_check(struct mem_ctl_info *mci)
>  				     row_index, 0, -1,
>  				     mci->ctl_name, "");
>
> -	ddr_out32(pdata->mc_vbase + FSL_MC_ERR_DETECT, err_detect);
> +	ddr_out32(pdata, FSL_MC_ERR_DETECT, err_detect);
>  }
>
>  static irqreturn_t fsl_mc_isr(int irq, void *dev_id)
> @@ -376,7 +380,7 @@ static irqreturn_t fsl_mc_isr(int irq, void *dev_id)
>  	struct fsl_mc_pdata *pdata = mci->pvt_info;
>  	u32 err_detect;
>
> -	err_detect = ddr_in32(pdata->mc_vbase + FSL_MC_ERR_DETECT);
> +	err_detect = ddr_in32(pdata, FSL_MC_ERR_DETECT);
>  	if (!err_detect)
>  		return IRQ_NONE;
>
> @@ -396,7 +400,7 @@ static void fsl_ddr_init_csrows(struct mem_ctl_info *mci)
>  	u32 cs_bnds;
>  	int index;
>
> -	sdram_ctl = ddr_in32(pdata->mc_vbase + FSL_MC_DDR_SDRAM_CFG);
> +	sdram_ctl = ddr_in32(pdata, FSL_MC_DDR_SDRAM_CFG);
>
>  	sdtype = sdram_ctl & DSC_SDTYPE_MASK;
>  	if (sdram_ctl & DSC_RD_EN) {
> @@ -444,7 +448,7 @@ static void fsl_ddr_init_csrows(struct mem_ctl_info *mci)
>  		csrow = mci->csrows[index];
>  		dimm = csrow->channels[0]->dimm;
>
> -		cs_bnds = ddr_in32(pdata->mc_vbase + FSL_MC_CS_BNDS_0 +
> +		cs_bnds = ddr_in32(pdata, FSL_MC_CS_BNDS_0 +
>  				   (index * FSL_MC_CS_BNDS_OFS));
>
>  		start = (cs_bnds & 0xffff0000) >> 16;
> @@ -558,11 +562,11 @@ int fsl_mc_err_probe(struct platform_device *op)
>  	fsl_ddr_init_csrows(mci);
>
>  	/* store the original error disable bits */
> -	orig_ddr_err_disable = ddr_in32(pdata->mc_vbase + FSL_MC_ERR_DISABLE);
> -	ddr_out32(pdata->mc_vbase + FSL_MC_ERR_DISABLE, 0);
> +	orig_ddr_err_disable = ddr_in32(pdata, FSL_MC_ERR_DISABLE);
> +	ddr_out32(pdata, FSL_MC_ERR_DISABLE, 0);
>
>  	/* clear all error bits */
> -	ddr_out32(pdata->mc_vbase + FSL_MC_ERR_DETECT, ~0);
> +	ddr_out32(pdata, FSL_MC_ERR_DETECT, ~0);
>
>  	res = edac_mc_add_mc_with_groups(mci, fsl_ddr_dev_groups);
>  	if (res) {
> @@ -571,15 +575,15 @@ int fsl_mc_err_probe(struct platform_device *op)
>  	}
>
>  	if (edac_op_state == EDAC_OPSTATE_INT) {
> -		ddr_out32(pdata->mc_vbase + FSL_MC_ERR_INT_EN,
> +		ddr_out32(pdata, FSL_MC_ERR_INT_EN,
>  			  DDR_EIE_MBEE | DDR_EIE_SBEE);
>
>  		/* store the original error management threshold */
> -		orig_ddr_err_sbe = ddr_in32(pdata->mc_vbase +
> +		orig_ddr_err_sbe = ddr_in32(pdata,
>  					    FSL_MC_ERR_SBE) & 0xff0000;
>
>  		/* set threshold to 1 error per interrupt */
> -		ddr_out32(pdata->mc_vbase + FSL_MC_ERR_SBE, 0x10000);
> +		ddr_out32(pdata, FSL_MC_ERR_SBE, 0x10000);
>
>  		/* register interrupts */
>  		pdata->irq = platform_get_irq(op, 0);
> @@ -620,12 +624,12 @@ void fsl_mc_err_remove(struct platform_device *op)
>  	edac_dbg(0, "\n");
>
>  	if (edac_op_state == EDAC_OPSTATE_INT) {
> -		ddr_out32(pdata->mc_vbase + FSL_MC_ERR_INT_EN, 0);
> +		ddr_out32(pdata, FSL_MC_ERR_INT_EN, 0);
>  	}
>
> -	ddr_out32(pdata->mc_vbase + FSL_MC_ERR_DISABLE,
> +	ddr_out32(pdata, FSL_MC_ERR_DISABLE,
>  		  orig_ddr_err_disable);
> -	ddr_out32(pdata->mc_vbase + FSL_MC_ERR_SBE, orig_ddr_err_sbe);
> +	ddr_out32(pdata, FSL_MC_ERR_SBE, orig_ddr_err_sbe);
>
>  	edac_mc_del_mc(&op->dev);
>  	edac_mc_free(mci);
>
> --
> 2.34.1
>

Re: [PATCH 1/6] EDAC: fsl_ddr: Pass down fsl_mc_pdata in ddr_in32() and ddr_out32()

[Borislav Petkov]

On Tue, Jul 09, 2024 at 04:23:02PM -0400, Frank Li wrote:
Subject: Re: [PATCH 1/6] EDAC: fsl_ddr: Pass down fsl_mc_pdata in ddr_in32() and ddr_out32()

The subject prefixes in the EDAC subsystem have the following format:

"EDAC/driver: <Sentence starting with a capital letter"

> Pass down fsl_mc_data in help function ddr_in32() and ddr_out32() to
> prepare add iMX9 support. iMX9 have a little difference register layout.

Since everyone is using AI nowadays, you could have it verify your commit
messages. I did this:

| ChatGPT, Please verify and correct the grammar in this english text: "Pass
| down fsl_mc_data in help function ddr_in32() and ddr_out32() to prepare add
| iMX9 support. iMX9 have a little difference register layout."

It replied with:

| "Pass down fsl_mc_data in the helper functions ddr_in32() and ddr_out32() to
| prepare for adding iMX9 support. The iMX9 has a slightly different register
| layout."
| 
| Changes made:
| 1. "help function" changed to "helper functions" for clarity.
| 
| 2. "prepare add iMX9 support" changed to "prepare for adding iMX9 support" for
|    grammatical correctness.
| 
| 3. "iMX9 have a little difference register layout" changed to "The iMX9 has
|    a slightly different register layout" for subject-verb agreement and
|    smoother phrasing.

And this all looks good to me.

With all the cringe we all get from AI, I think it is very useful for
verifying commit messages.

Do that for all your commit messages pls.

Thx.

> Signed-off-by: Frank Li <Frank.Li@nxp.com>
> ---
>  drivers/edac/fsl_ddr_edac.c | 62 ++++++++++++++++++++++++---------------------
>  1 file changed, 33 insertions(+), 29 deletions(-)

How did you test these patches of yours?

They don't even build!

drivers/edac/fsl_ddr_edac.c: In function 'fsl_mc_err_probe':
drivers/edac/fsl_ddr_edac.c:538:21: error: too few arguments to function 'ddr_in32'
  538 |         sdram_ctl = ddr_in32(pdata->mc_vbase + FSL_MC_DDR_SDRAM_CFG);
      |                     ^~~~~~~~
drivers/edac/fsl_ddr_edac.c:38:19: note: declared here
   38 | static inline u32 ddr_in32(struct fsl_mc_pdata *pdata, unsigned int off)
      |                   ^~~~~~~~
make[4]: *** [scripts/Makefile.build:229: drivers/edac/fsl_ddr_edac.o] Error 1
make[3]: *** [scripts/Makefile.build:478: drivers/edac] Error 2
make[3]: *** Waiting for unfinished jobs....
make[2]: *** [scripts/Makefile.build:478: drivers] Error 2
make[2]: *** Waiting for unfinished jobs....
make[1]: *** [/mnt/kernel/kernel/2nd/linux/Makefile:1936: .] Error 2
make: *** [Makefile:224: __sub-make] Error 2

Before you submit next time, build-test *every* *single* patch of yours and
test the driver with all of them.

This should not ever happen in submission.

Stopping review here.

-- 
Regards/Gruss,
    Boris.

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

Re: [PATCH 1/6] EDAC: fsl_ddr: Pass down fsl_mc_pdata in ddr_in32() and ddr_out32()

[Frank Li]

On Thu, Oct 10, 2024 at 05:33:20PM +0200, Borislav Petkov wrote:
> On Tue, Jul 09, 2024 at 04:23:02PM -0400, Frank Li wrote:
> Subject: Re: [PATCH 1/6] EDAC: fsl_ddr: Pass down fsl_mc_pdata in ddr_in32() and ddr_out32()
>
> The subject prefixes in the EDAC subsystem have the following format:
>
> "EDAC/driver: <Sentence starting with a capital letter"
>
> > Pass down fsl_mc_data in help function ddr_in32() and ddr_out32() to
> > prepare add iMX9 support. iMX9 have a little difference register layout.
>
> Since everyone is using AI nowadays, you could have it verify your commit
> messages. I did this:
>
> | ChatGPT, Please verify and correct the grammar in this english text: "Pass
> | down fsl_mc_data in help function ddr_in32() and ddr_out32() to prepare add
> | iMX9 support. iMX9 have a little difference register layout."
>
> It replied with:
>
> | "Pass down fsl_mc_data in the helper functions ddr_in32() and ddr_out32() to
> | prepare for adding iMX9 support. The iMX9 has a slightly different register
> | layout."
> |
> | Changes made:
> | 1. "help function" changed to "helper functions" for clarity.
> |
> | 2. "prepare add iMX9 support" changed to "prepare for adding iMX9 support" for
> |    grammatical correctness.
> |
> | 3. "iMX9 have a little difference register layout" changed to "The iMX9 has
> |    a slightly different register layout" for subject-verb agreement and
> |    smoother phrasing.
>
> And this all looks good to me.
>
> With all the cringe we all get from AI, I think it is very useful for
> verifying commit messages.
>
> Do that for all your commit messages pls.
>
> Thx.
>
> > Signed-off-by: Frank Li <Frank.Li@nxp.com>
> > ---
> >  drivers/edac/fsl_ddr_edac.c | 62 ++++++++++++++++++++++++---------------------
> >  1 file changed, 33 insertions(+), 29 deletions(-)
>
> How did you test these patches of yours?
>
> They don't even build!
>
> drivers/edac/fsl_ddr_edac.c: In function 'fsl_mc_err_probe':
> drivers/edac/fsl_ddr_edac.c:538:21: error: too few arguments to function 'ddr_in32'
>   538 |         sdram_ctl = ddr_in32(pdata->mc_vbase + FSL_MC_DDR_SDRAM_CFG);
>       |                     ^~~~~~~~
> drivers/edac/fsl_ddr_edac.c:38:19: note: declared here
>    38 | static inline u32 ddr_in32(struct fsl_mc_pdata *pdata, unsigned int off)
>       |                   ^~~~~~~~
> make[4]: *** [scripts/Makefile.build:229: drivers/edac/fsl_ddr_edac.o] Error 1
> make[3]: *** [scripts/Makefile.build:478: drivers/edac] Error 2
> make[3]: *** Waiting for unfinished jobs....
> make[2]: *** [scripts/Makefile.build:478: drivers] Error 2
> make[2]: *** Waiting for unfinished jobs....
> make[1]: *** [/mnt/kernel/kernel/2nd/linux/Makefile:1936: .] Error 2
> make: *** [Makefile:224: __sub-make] Error 2
>
> Before you submit next time, build-test *every* *single* patch of yours and
> test the driver with all of them.
>
> This should not ever happen in submission.

Really sorry about this. I need debug why my check script have not found
this problem.

Frank

>
> Stopping review here.
>
> --
> Regards/Gruss,
>     Boris.
>
> https://people.kernel.org/tglx/notes-about-netiquette

[PATCH 2/6] EDAC, fsl_ddr: Move global variable into struct fsl_mc_pdata

[Frank Li]

Move global variable into struct fsl_mc_data in case there are more than
one ddr controller in system.

No function change.

Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
 drivers/edac/fsl_ddr_edac.c | 21 +++++++++------------
 drivers/edac/fsl_ddr_edac.h |  3 +++
 2 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/edac/fsl_ddr_edac.c b/drivers/edac/fsl_ddr_edac.c
index a7982370e2381..10b0a46669f3d 100644
--- a/drivers/edac/fsl_ddr_edac.c
+++ b/drivers/edac/fsl_ddr_edac.c
@@ -31,22 +31,18 @@
 
 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)
 {
 	void __iomem *addr = pdata->mc_vbase + off;
 
-	return little_endian ? ioread32(addr) : ioread32be(addr);
+	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;
 
-	if (little_endian)
+	if (pdata->little_endian)
 		iowrite32(value, addr);
 	else
 		iowrite32be(value, addr);
@@ -511,7 +507,7 @@ int fsl_mc_err_probe(struct platform_device *op)
 	 * Get the endianness of DDR controller registers.
 	 * Default is big endian.
 	 */
-	little_endian = of_property_read_bool(op->dev.of_node, "little-endian");
+	pdata->little_endian = of_property_read_bool(op->dev.of_node, "little-endian");
 
 	res = of_address_to_resource(op->dev.of_node, 0, &r);
 	if (res) {
@@ -535,7 +531,7 @@ int fsl_mc_err_probe(struct platform_device *op)
 		goto err;
 	}
 
-	sdram_ctl = ddr_in32(pdata->mc_vbase + FSL_MC_DDR_SDRAM_CFG);
+	sdram_ctl = ddr_in32(pdata, FSL_MC_DDR_SDRAM_CFG);
 	if (!(sdram_ctl & DSC_ECC_EN)) {
 		/* no ECC */
 		pr_warn("%s: No ECC DIMMs discovered\n", __func__);
@@ -562,7 +558,7 @@ int fsl_mc_err_probe(struct platform_device *op)
 	fsl_ddr_init_csrows(mci);
 
 	/* store the original error disable bits */
-	orig_ddr_err_disable = ddr_in32(pdata, FSL_MC_ERR_DISABLE);
+	pdata->orig_ddr_err_disable = ddr_in32(pdata, FSL_MC_ERR_DISABLE);
 	ddr_out32(pdata, FSL_MC_ERR_DISABLE, 0);
 
 	/* clear all error bits */
@@ -579,7 +575,7 @@ int fsl_mc_err_probe(struct platform_device *op)
 			  DDR_EIE_MBEE | DDR_EIE_SBEE);
 
 		/* store the original error management threshold */
-		orig_ddr_err_sbe = ddr_in32(pdata,
+		pdata->orig_ddr_err_sbe = ddr_in32(pdata,
 					    FSL_MC_ERR_SBE) & 0xff0000;
 
 		/* set threshold to 1 error per interrupt */
@@ -628,8 +624,9 @@ void fsl_mc_err_remove(struct platform_device *op)
 	}
 
 	ddr_out32(pdata, FSL_MC_ERR_DISABLE,
-		  orig_ddr_err_disable);
-	ddr_out32(pdata, FSL_MC_ERR_SBE, orig_ddr_err_sbe);
+		  pdata->orig_ddr_err_disable);
+	ddr_out32(pdata, FSL_MC_ERR_SBE, pdata->orig_ddr_err_sbe);
+
 
 	edac_mc_del_mc(&op->dev);
 	edac_mc_free(mci);
diff --git a/drivers/edac/fsl_ddr_edac.h b/drivers/edac/fsl_ddr_edac.h
index c0994a2a003c2..de66f9822fba1 100644
--- a/drivers/edac/fsl_ddr_edac.h
+++ b/drivers/edac/fsl_ddr_edac.h
@@ -70,6 +70,9 @@ struct fsl_mc_pdata {
 	int edac_idx;
 	void __iomem *mc_vbase;
 	int irq;
+	u32 orig_ddr_err_disable;
+	u32 orig_ddr_err_sbe;
+	bool little_endian;
 };
 int fsl_mc_err_probe(struct platform_device *op);
 void fsl_mc_err_remove(struct platform_device *op);

-- 
2.34.1

[PATCH 3/6] EDAC: fsl: Fix bad bit shift operations

[Frank Li]

From: Priyanka Singh <priyanka.singh@nxp.com>

The original code:
	cap_high ^ (1 << (bad_data_bit - 32))

The variable bad_data_bit ranges from 0 to 63. If bad_data_bit is below 32,
bad_data_bit - 32 will be a negative value. Left shifting a negative
value in C is undefined behavior.

Fix this by checking the range of bad_data_bit.

Fixes: ea2eb9a8b620 ("EDAC, fsl-ddr: Separate FSL DDR driver from MPC85xx")
Signed-off-by: Priyanka Singh <priyanka.singh@nxp.com>
Reviewed-by: Sherry Sun <sherry.sun@nxp.com>
Signed-off-by: Li Yang <leoyang.li@nxp.com>
Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
 drivers/edac/fsl_ddr_edac.c | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/drivers/edac/fsl_ddr_edac.c b/drivers/edac/fsl_ddr_edac.c
index 10b0a46669f3d..da743cf8a95e9 100644
--- a/drivers/edac/fsl_ddr_edac.c
+++ b/drivers/edac/fsl_ddr_edac.c
@@ -338,11 +338,18 @@ static void fsl_mc_check(struct mem_ctl_info *mci)
 			fsl_mc_printk(mci, KERN_ERR,
 				"Faulty ECC bit: %d\n", bad_ecc_bit);
 
-		fsl_mc_printk(mci, KERN_ERR,
-			"Expected Data / ECC:\t%#8.8x_%08x / %#2.2x\n",
-			cap_high ^ (1 << (bad_data_bit - 32)),
-			cap_low ^ (1 << bad_data_bit),
-			syndrome ^ (1 << bad_ecc_bit));
+		if ((bad_data_bit > 0 && bad_data_bit < 32) && bad_ecc_bit > 0) {
+			fsl_mc_printk(mci, KERN_ERR,
+				"Expected Data / ECC:\t%#8.8x_%08x / %#2.2x\n",
+				cap_high, cap_low ^ (1 << bad_data_bit),
+				syndrome ^ (1 << bad_ecc_bit));
+		}
+		if (bad_data_bit >= 32 && bad_ecc_bit > 0) {
+			fsl_mc_printk(mci, KERN_ERR,
+				"Expected Data / ECC:\t%#8.8x_%08x / %#2.2x\n",
+				cap_high ^ (1 << (bad_data_bit - 32)),
+				cap_low, syndrome ^ (1 << bad_ecc_bit));
+		}
 	}
 
 	fsl_mc_printk(mci, KERN_ERR,

-- 
2.34.1

[PATCH 4/6] dt-bindings: memory: fsl: Add compatible string nxp,imx9-memory-controller

[Frank Li]

iMX9 memory controller is similar with other layerscape chips. But some
register layout has a little bit difference, so add new compatible string
'nxp,imx9-memory-controller' for it.

imx9 need two 'reg', one for DDR controller and the other is ECC inject
engine register space. Keep the same restriction for other compatible
string.

Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
 .../bindings/memory-controllers/fsl/fsl,ddr.yaml   | 31 +++++++++++++++++++++-
 1 file changed, 30 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/memory-controllers/fsl/fsl,ddr.yaml b/Documentation/devicetree/bindings/memory-controllers/fsl/fsl,ddr.yaml
index 84f778a99546b..e0786153eec73 100644
--- a/Documentation/devicetree/bindings/memory-controllers/fsl/fsl,ddr.yaml
+++ b/Documentation/devicetree/bindings/memory-controllers/fsl/fsl,ddr.yaml
@@ -40,6 +40,7 @@ properties:
           - fsl,p1021-memory-controller
           - fsl,p2020-memory-controller
           - fsl,qoriq-memory-controller
+          - nxp,imx9-memory-controller
 
   interrupts:
     maxItems: 1
@@ -51,13 +52,41 @@ properties:
     type: boolean
 
   reg:
-    maxItems: 1
+    items:
+      - description: Controller register space
+      - description: Inject register space
+    minItems: 1
+
+  reg-names:
+    items:
+      - const: ctrl
+      - const: inject
+    minItems: 1
 
 required:
   - compatible
   - interrupts
   - reg
 
+allOf:
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - nxp,imx9-memory-controller
+    then:
+      properties:
+        reg:
+          minItems: 2
+        reg-names:
+          minItems: 2
+    else:
+      properties:
+        reg:
+          maxItems: 1
+        reg-names: false
+
 additionalProperties: false
 
 examples:

-- 
2.34.1

Re: [PATCH 4/6] dt-bindings: memory: fsl: Add compatible string nxp,imx9-memory-controller

[Krzysztof Kozlowski]

On 09/07/2024 22:23, Frank Li wrote:
> iMX9 memory controller is similar with other layerscape chips. But some
> register layout has a little bit difference, so add new compatible string
> 'nxp,imx9-memory-controller' for it.
> 
> imx9 need two 'reg', one for DDR controller and the other is ECC inject
> engine register space. Keep the same restriction for other compatible
> string.
> 
> Signed-off-by: Frank Li <Frank.Li@nxp.com>

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

I assume this will go via EDAC tree (not memory-controllers tree).

Best regards,
Krzysztof

Re: [PATCH 4/6] dt-bindings: memory: fsl: Add compatible string nxp, imx9-memory-controller

[Alexander Stein]

Hi Frank,

Am Dienstag, 9. Juli 2024, 22:23:05 CEST schrieb Frank Li:
> iMX9 memory controller is similar with other layerscape chips. But some
> register layout has a little bit difference, so add new compatible string
> 'nxp,imx9-memory-controller' for it.

Is this controller the same for all i.MX9 SoC? E.g. i.MX91, i.MX93,
i.MX95 and any future variants?

Best regards,
Alexander

> imx9 need two 'reg', one for DDR controller and the other is ECC inject
> engine register space. Keep the same restriction for other compatible
> string.
> 
> Signed-off-by: Frank Li <Frank.Li@nxp.com>
> ---
>  .../bindings/memory-controllers/fsl/fsl,ddr.yaml   | 31 +++++++++++++++++++++-
>  1 file changed, 30 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/memory-controllers/fsl/fsl,ddr.yaml b/Documentation/devicetree/bindings/memory-controllers/fsl/fsl,ddr.yaml
> index 84f778a99546b..e0786153eec73 100644
> --- a/Documentation/devicetree/bindings/memory-controllers/fsl/fsl,ddr.yaml
> +++ b/Documentation/devicetree/bindings/memory-controllers/fsl/fsl,ddr.yaml
> @@ -40,6 +40,7 @@ properties:
>            - fsl,p1021-memory-controller
>            - fsl,p2020-memory-controller
>            - fsl,qoriq-memory-controller
> +          - nxp,imx9-memory-controller
>  
>    interrupts:
>      maxItems: 1
> @@ -51,13 +52,41 @@ properties:
>      type: boolean
>  
>    reg:
> -    maxItems: 1
> +    items:
> +      - description: Controller register space
> +      - description: Inject register space
> +    minItems: 1
> +
> +  reg-names:
> +    items:
> +      - const: ctrl
> +      - const: inject
> +    minItems: 1
>  
>  required:
>    - compatible
>    - interrupts
>    - reg
>  
> +allOf:
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - nxp,imx9-memory-controller
> +    then:
> +      properties:
> +        reg:
> +          minItems: 2
> +        reg-names:
> +          minItems: 2
> +    else:
> +      properties:
> +        reg:
> +          maxItems: 1
> +        reg-names: false
> +
>  additionalProperties: false
>  
>  examples:
> 
> 


-- 
TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht München, HRB 105018
Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
http://www.tq-group.com/

Re: [PATCH 4/6] dt-bindings: memory: fsl: Add compatible string nxp, imx9-memory-controller

[Frank Li]

On Wed, Jul 10, 2024 at 09:18:16AM +0200, Alexander Stein wrote:
> Hi Frank,
> 
> Am Dienstag, 9. Juli 2024, 22:23:05 CEST schrieb Frank Li:
> > iMX9 memory controller is similar with other layerscape chips. But some
> > register layout has a little bit difference, so add new compatible string
> > 'nxp,imx9-memory-controller' for it.
> 
> Is this controller the same for all i.MX9 SoC? E.g. i.MX91, i.MX93,
> i.MX95 and any future variants?

So far it is the same. I can't prodicting future. Not plan to change it
yet.

Frank 

> 
> Best regards,
> Alexander
> 
> > imx9 need two 'reg', one for DDR controller and the other is ECC inject
> > engine register space. Keep the same restriction for other compatible
> > string.
> > 
> > Signed-off-by: Frank Li <Frank.Li@nxp.com>
> > ---
> >  .../bindings/memory-controllers/fsl/fsl,ddr.yaml   | 31 +++++++++++++++++++++-
> >  1 file changed, 30 insertions(+), 1 deletion(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/memory-controllers/fsl/fsl,ddr.yaml b/Documentation/devicetree/bindings/memory-controllers/fsl/fsl,ddr.yaml
> > index 84f778a99546b..e0786153eec73 100644
> > --- a/Documentation/devicetree/bindings/memory-controllers/fsl/fsl,ddr.yaml
> > +++ b/Documentation/devicetree/bindings/memory-controllers/fsl/fsl,ddr.yaml
> > @@ -40,6 +40,7 @@ properties:
> >            - fsl,p1021-memory-controller
> >            - fsl,p2020-memory-controller
> >            - fsl,qoriq-memory-controller
> > +          - nxp,imx9-memory-controller
> >  
> >    interrupts:
> >      maxItems: 1
> > @@ -51,13 +52,41 @@ properties:
> >      type: boolean
> >  
> >    reg:
> > -    maxItems: 1
> > +    items:
> > +      - description: Controller register space
> > +      - description: Inject register space
> > +    minItems: 1
> > +
> > +  reg-names:
> > +    items:
> > +      - const: ctrl
> > +      - const: inject
> > +    minItems: 1
> >  
> >  required:
> >    - compatible
> >    - interrupts
> >    - reg
> >  
> > +allOf:
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          contains:
> > +            enum:
> > +              - nxp,imx9-memory-controller
> > +    then:
> > +      properties:
> > +        reg:
> > +          minItems: 2
> > +        reg-names:
> > +          minItems: 2
> > +    else:
> > +      properties:
> > +        reg:
> > +          maxItems: 1
> > +        reg-names: false
> > +
> >  additionalProperties: false
> >  
> >  examples:
> > 
> > 
> 
> 
> -- 
> TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
> Amtsgericht München, HRB 105018
> Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
> http://www.tq-group.com/
> 
> 

Re: [PATCH 4/6] dt-bindings: memory: fsl: Add compatible string nxp, imx9-memory-controller

[Alexander Stein]

Hi Frank,

Am Freitag, 12. Juli 2024, 05:27:54 CEST schrieb Frank Li:
> On Wed, Jul 10, 2024 at 09:18:16AM +0200, Alexander Stein wrote:
> > Hi Frank,
> > 
> > Am Dienstag, 9. Juli 2024, 22:23:05 CEST schrieb Frank Li:
> > > iMX9 memory controller is similar with other layerscape chips. But some
> > > register layout has a little bit difference, so add new compatible string
> > > 'nxp,imx9-memory-controller' for it.
> > 
> > Is this controller the same for all i.MX9 SoC? E.g. i.MX91, i.MX93,
> > i.MX95 and any future variants?
> 
> So far it is the same. I can't prodicting future. Not plan to change it
> yet.

Okay, thanks for clarification. If DT maintainers are okay this way, I do not object.

Best regards,
Alexander

> Frank 
> 
> > 
> > Best regards,
> > Alexander
> > 
> > > imx9 need two 'reg', one for DDR controller and the other is ECC inject
> > > engine register space. Keep the same restriction for other compatible
> > > string.
> > > 
> > > Signed-off-by: Frank Li <Frank.Li@nxp.com>
> > > ---
> > >  .../bindings/memory-controllers/fsl/fsl,ddr.yaml   | 31 +++++++++++++++++++++-
> > >  1 file changed, 30 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/memory-controllers/fsl/fsl,ddr.yaml b/Documentation/devicetree/bindings/memory-controllers/fsl/fsl,ddr.yaml
> > > index 84f778a99546b..e0786153eec73 100644
> > > --- a/Documentation/devicetree/bindings/memory-controllers/fsl/fsl,ddr.yaml
> > > +++ b/Documentation/devicetree/bindings/memory-controllers/fsl/fsl,ddr.yaml
> > > @@ -40,6 +40,7 @@ properties:
> > >            - fsl,p1021-memory-controller
> > >            - fsl,p2020-memory-controller
> > >            - fsl,qoriq-memory-controller
> > > +          - nxp,imx9-memory-controller
> > >  
> > >    interrupts:
> > >      maxItems: 1
> > > @@ -51,13 +52,41 @@ properties:
> > >      type: boolean
> > >  
> > >    reg:
> > > -    maxItems: 1
> > > +    items:
> > > +      - description: Controller register space
> > > +      - description: Inject register space
> > > +    minItems: 1
> > > +
> > > +  reg-names:
> > > +    items:
> > > +      - const: ctrl
> > > +      - const: inject
> > > +    minItems: 1
> > >  
> > >  required:
> > >    - compatible
> > >    - interrupts
> > >    - reg
> > >  
> > > +allOf:
> > > +  - if:
> > > +      properties:
> > > +        compatible:
> > > +          contains:
> > > +            enum:
> > > +              - nxp,imx9-memory-controller
> > > +    then:
> > > +      properties:
> > > +        reg:
> > > +          minItems: 2
> > > +        reg-names:
> > > +          minItems: 2
> > > +    else:
> > > +      properties:
> > > +        reg:
> > > +          maxItems: 1
> > > +        reg-names: false
> > > +
> > >  additionalProperties: false
> > >  
> > >  examples:
> > > 
> > > 
> > 
> > 
> 
> 
> 


-- 
TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht München, HRB 105018
Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
http://www.tq-group.com/

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

[Frank Li]

From: Ye Li <ye.li@nxp.com>

i.MX9 uses an DDR controller that has registers offset and some function
changed, but the ECC and Error inject functions are almost same with
fsl_ddr controller. So update and re-use the driver for i.MX9. A special
type is added for i.MX9 controller only to distinguish the difference.

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 da743cf8a95e9..438b995066f2a 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

[PATCH 6/6] arm64: dts: imx93: add ddr edac support

[Frank Li]

Add ddr edac support for imx93.

Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
 arch/arm64/boot/dts/freescale/imx93.dtsi | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/imx93.dtsi b/arch/arm64/boot/dts/freescale/imx93.dtsi
index 4a3f42355cb8f..6faba848fe286 100644
--- a/arch/arm64/boot/dts/freescale/imx93.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx93.dtsi
@@ -1279,6 +1279,14 @@ usbmisc2: usbmisc@4c200200 {
 			#index-cells = <1>;
 		};
 
+		memory-controller@4e300000 {
+			compatible = "nxp,imx9-memory-controller";
+			reg = <0x4e300000 0x800>, <0x4e301000 0x1000>;
+			reg-names = "ctrl", "inject";
+			interrupts = <GIC_SPI 91 IRQ_TYPE_LEVEL_HIGH>;
+			little-endian;
+		};
+
 		ddr-pmu@4e300dc0 {
 			compatible = "fsl,imx93-ddr-pmu";
 			reg = <0x4e300dc0 0x200>;

-- 
2.34.1

RE: [PATCH 6/6] arm64: dts: imx93: add ddr edac support

[Peng Fan]

> Subject: [PATCH 6/6] arm64: dts: imx93: add ddr edac support
> 
> Add ddr edac support for imx93.
> 
> Signed-off-by: Frank Li <Frank.Li@nxp.com>
> ---
>  arch/arm64/boot/dts/freescale/imx93.dtsi | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/freescale/imx93.dtsi
> b/arch/arm64/boot/dts/freescale/imx93.dtsi
> index 4a3f42355cb8f..6faba848fe286 100644
> --- a/arch/arm64/boot/dts/freescale/imx93.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx93.dtsi
> @@ -1279,6 +1279,14 @@ usbmisc2: usbmisc@4c200200 {
>  			#index-cells = <1>;
>  		};
> 
> +		memory-controller@4e300000 {
> +			compatible = "nxp,imx9-memory-controller";
> +			reg = <0x4e300000 0x800>, <0x4e301000
> 0x1000>;
> +			reg-names = "ctrl", "inject";
> +			interrupts = <GIC_SPI 91
> IRQ_TYPE_LEVEL_HIGH>;
> +			little-endian;
> +		};
> +
>  		ddr-pmu@4e300dc0 {

Should the ddr-pmu be part of memory controller?

Regards,
Peng.

>  			compatible = "fsl,imx93-ddr-pmu";
>  			reg = <0x4e300dc0 0x200>;
> 
> --
> 2.34.1
> 

Re: [PATCH 6/6] arm64: dts: imx93: add ddr edac support

[Frank Li]

On Wed, Jul 10, 2024 at 02:25:02AM +0000, Peng Fan wrote:
> > Subject: [PATCH 6/6] arm64: dts: imx93: add ddr edac support
> > 
> > Add ddr edac support for imx93.
> > 
> > Signed-off-by: Frank Li <Frank.Li@nxp.com>
> > ---
> >  arch/arm64/boot/dts/freescale/imx93.dtsi | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> > 
> > diff --git a/arch/arm64/boot/dts/freescale/imx93.dtsi
> > b/arch/arm64/boot/dts/freescale/imx93.dtsi
> > index 4a3f42355cb8f..6faba848fe286 100644
> > --- a/arch/arm64/boot/dts/freescale/imx93.dtsi
> > +++ b/arch/arm64/boot/dts/freescale/imx93.dtsi
> > @@ -1279,6 +1279,14 @@ usbmisc2: usbmisc@4c200200 {
> >  			#index-cells = <1>;
> >  		};
> > 
> > +		memory-controller@4e300000 {
> > +			compatible = "nxp,imx9-memory-controller";
> > +			reg = <0x4e300000 0x800>, <0x4e301000
> > 0x1000>;
> > +			reg-names = "ctrl", "inject";
> > +			interrupts = <GIC_SPI 91
> > IRQ_TYPE_LEVEL_HIGH>;
> > +			little-endian;
> > +		};
> > +
> >  		ddr-pmu@4e300dc0 {
> 
> Should the ddr-pmu be part of memory controller?

imx9-memory-controller is actually EDAC, which independent with ddr pmu.
I think ddr-pmu should be seperate node.

Frank

> 
> Regards,
> Peng.
> 
> >  			compatible = "fsl,imx93-ddr-pmu";
> >  			reg = <0x4e300dc0 0x200>;
> > 
> > --
> > 2.34.1
> > 
> 

Re: [PATCH 0/6] EDAC: fsl-ddr, add imx9 support

[Frank Li]

On Tue, Jul 09, 2024 at 04:23:01PM -0400, Frank Li wrote:
> Add imx9 support for fsl-ddr.
>
> Patch 1-2 is prepare patch, no function chagne
> Patch 3 is small fix for bit shift
> Patch 4 is dt binding patch.
> Patch 5 is driver change to support imx9
> Patch 6 is imx93 dts change
>
> Signed-off-by: Frank Li <Frank.Li@nxp.com>
> ---

Borislav Petkov:

	More than 2 monthes. I ping at Thu, 29 Aug
https://lore.kernel.org/imx/ZtDwG2xFGaUssJVN@lizhi-Precision-Tower-5810/

	Any reason why not pick these EDAC patches?  Or do you have
additional comment to improve patches?

Frank


> Frank Li (4):
>       EDAC: fsl_ddr: Pass down fsl_mc_pdata in ddr_in32() and ddr_out32()
>       EDAC, fsl_ddr: Move global variable into struct fsl_mc_pdata
>       dt-bindings: memory: fsl: Add compatible string nxp,imx9-memory-controller
>       arm64: dts: imx93: add ddr edac support
>
> Priyanka Singh (1):
>       EDAC: fsl: Fix bad bit shift operations
>
> Ye Li (1):
>       EDAC: fsl_ddr: Add support for i.MX9 DDR controller
>
>  .../bindings/memory-controllers/fsl/fsl,ddr.yaml   |  31 ++++-
>  arch/arm64/boot/dts/freescale/imx93.dtsi           |   8 ++
>  drivers/edac/fsl_ddr_edac.c                        | 134 ++++++++++++++-------
>  drivers/edac/fsl_ddr_edac.h                        |  13 ++
>  drivers/edac/layerscape_edac.c                     |   1 +
>  5 files changed, 141 insertions(+), 46 deletions(-)
> ---
> base-commit: 82d01fe6ee52086035b201cfa1410a3b04384257
> change-id: 20240704-imx95_edac-209cec208446
>
> Best regards,
> ---
> Frank Li <Frank.Li@nxp.com>
>

Re: [PATCH 0/6] EDAC: fsl-ddr, add imx9 support

[Borislav Petkov]

On Mon, Sep 30, 2024 at 04:42:14PM -0400, Frank Li wrote:
> On Tue, Jul 09, 2024 at 04:23:01PM -0400, Frank Li wrote:
> > Add imx9 support for fsl-ddr.
> >
> > Patch 1-2 is prepare patch, no function chagne
> > Patch 3 is small fix for bit shift
> > Patch 4 is dt binding patch.
> > Patch 5 is driver change to support imx9
> > Patch 6 is imx93 dts change
> >
> > Signed-off-by: Frank Li <Frank.Li@nxp.com>
> > ---
> 
> Borislav Petkov:
> 
> 	More than 2 monthes. I ping at Thu, 29 Aug
> https://lore.kernel.org/imx/ZtDwG2xFGaUssJVN@lizhi-Precision-Tower-5810/
> 
> 	Any reason why not pick these EDAC patches?

$ ./scripts/get_maintainer.pl -f  drivers/edac/fsl_ddr_edac.c
York Sun <york.sun@nxp.com> (maintainer:EDAC-FSL_DDR)
Borislav Petkov <bp@alien8.de> (supporter:EDAC-CORE)
Tony Luck <tony.luck@intel.com> (supporter:EDAC-CORE)
James Morse <james.morse@arm.com> (reviewer:EDAC-CORE)
Mauro Carvalho Chehab <mchehab@kernel.org> (reviewer:EDAC-CORE)
Robert Richter <rric@kernel.org> (reviewer:EDAC-CORE)
linux-edac@vger.kernel.org (open list:EDAC-FSL_DDR)
linux-kernel@vger.kernel.org (open list)

This driver has a maintainer. Is he going to review it or can I remove
him from MAINTAINERS?

-- 
Regards/Gruss,
    Boris.

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

Re: [PATCH 0/6] EDAC: fsl-ddr, add imx9 support

[Krzysztof Kozlowski]

On 02/10/2024 11:08, Borislav Petkov wrote:
> On Mon, Sep 30, 2024 at 04:42:14PM -0400, Frank Li wrote:
>> On Tue, Jul 09, 2024 at 04:23:01PM -0400, Frank Li wrote:
>>> Add imx9 support for fsl-ddr.
>>>
>>> Patch 1-2 is prepare patch, no function chagne
>>> Patch 3 is small fix for bit shift
>>> Patch 4 is dt binding patch.
>>> Patch 5 is driver change to support imx9
>>> Patch 6 is imx93 dts change
>>>
>>> Signed-off-by: Frank Li <Frank.Li@nxp.com>
>>> ---
>>
>> Borislav Petkov:
>>
>> 	More than 2 monthes. I ping at Thu, 29 Aug
>> https://lore.kernel.org/imx/ZtDwG2xFGaUssJVN@lizhi-Precision-Tower-5810/
>>
>> 	Any reason why not pick these EDAC patches?
> 
> $ ./scripts/get_maintainer.pl -f  drivers/edac/fsl_ddr_edac.c
> York Sun <york.sun@nxp.com> (maintainer:EDAC-FSL_DDR)
> Borislav Petkov <bp@alien8.de> (supporter:EDAC-CORE)
> Tony Luck <tony.luck@intel.com> (supporter:EDAC-CORE)
> James Morse <james.morse@arm.com> (reviewer:EDAC-CORE)
> Mauro Carvalho Chehab <mchehab@kernel.org> (reviewer:EDAC-CORE)
> Robert Richter <rric@kernel.org> (reviewer:EDAC-CORE)
> linux-edac@vger.kernel.org (open list:EDAC-FSL_DDR)
> linux-kernel@vger.kernel.org (open list)
> 
> This driver has a maintainer. Is he going to review it or can I remove
> him from MAINTAINERS?

Let's drop York, it's a stale maintainer entry.

Best regards,
Krzysztof

Re: [PATCH 0/6] EDAC: fsl-ddr, add imx9 support

[Krzysztof Kozlowski]

On 02/10/2024 12:48, Krzysztof Kozlowski wrote:
> On 02/10/2024 11:08, Borislav Petkov wrote:
>> On Mon, Sep 30, 2024 at 04:42:14PM -0400, Frank Li wrote:
>>> On Tue, Jul 09, 2024 at 04:23:01PM -0400, Frank Li wrote:
>>>> Add imx9 support for fsl-ddr.
>>>>
>>>> Patch 1-2 is prepare patch, no function chagne
>>>> Patch 3 is small fix for bit shift
>>>> Patch 4 is dt binding patch.
>>>> Patch 5 is driver change to support imx9
>>>> Patch 6 is imx93 dts change
>>>>
>>>> Signed-off-by: Frank Li <Frank.Li@nxp.com>
>>>> ---
>>>
>>> Borislav Petkov:
>>>
>>> 	More than 2 monthes. I ping at Thu, 29 Aug
>>> https://lore.kernel.org/imx/ZtDwG2xFGaUssJVN@lizhi-Precision-Tower-5810/
>>>
>>> 	Any reason why not pick these EDAC patches?
>>
>> $ ./scripts/get_maintainer.pl -f  drivers/edac/fsl_ddr_edac.c
>> York Sun <york.sun@nxp.com> (maintainer:EDAC-FSL_DDR)
>> Borislav Petkov <bp@alien8.de> (supporter:EDAC-CORE)
>> Tony Luck <tony.luck@intel.com> (supporter:EDAC-CORE)
>> James Morse <james.morse@arm.com> (reviewer:EDAC-CORE)
>> Mauro Carvalho Chehab <mchehab@kernel.org> (reviewer:EDAC-CORE)
>> Robert Richter <rric@kernel.org> (reviewer:EDAC-CORE)
>> linux-edac@vger.kernel.org (open list:EDAC-FSL_DDR)
>> linux-kernel@vger.kernel.org (open list)
>>
>> This driver has a maintainer. Is he going to review it or can I remove
>> him from MAINTAINERS?
> 
> Let's drop York, it's a stale maintainer entry.

I sent a patch now for this.

Best regards,
Krzysztof

Re: [PATCH 0/6] EDAC: fsl-ddr, add imx9 support

[Frank Li]

On Wed, Oct 02, 2024 at 11:08:34AM +0200, Borislav Petkov wrote:
> On Mon, Sep 30, 2024 at 04:42:14PM -0400, Frank Li wrote:
> > On Tue, Jul 09, 2024 at 04:23:01PM -0400, Frank Li wrote:
> > > Add imx9 support for fsl-ddr.
> > >
> > > Patch 1-2 is prepare patch, no function chagne
> > > Patch 3 is small fix for bit shift
> > > Patch 4 is dt binding patch.
> > > Patch 5 is driver change to support imx9
> > > Patch 6 is imx93 dts change
> > >
> > > Signed-off-by: Frank Li <Frank.Li@nxp.com>
> > > ---
> >
> > Borislav Petkov:
> >
> > 	More than 2 monthes. I ping at Thu, 29 Aug
> > https://lore.kernel.org/imx/ZtDwG2xFGaUssJVN@lizhi-Precision-Tower-5810/
> >
> > 	Any reason why not pick these EDAC patches?
>
> $ ./scripts/get_maintainer.pl -f  drivers/edac/fsl_ddr_edac.c
> York Sun <york.sun@nxp.com> (maintainer:EDAC-FSL_DDR)
> Borislav Petkov <bp@alien8.de> (supporter:EDAC-CORE)
> Tony Luck <tony.luck@intel.com> (supporter:EDAC-CORE)
> James Morse <james.morse@arm.com> (reviewer:EDAC-CORE)
> Mauro Carvalho Chehab <mchehab@kernel.org> (reviewer:EDAC-CORE)
> Robert Richter <rric@kernel.org> (reviewer:EDAC-CORE)
> linux-edac@vger.kernel.org (open list:EDAC-FSL_DDR)
> linux-kernel@vger.kernel.org (open list)
>
> This driver has a maintainer. Is he going to review it or can I remove
> him from MAINTAINERS?

Borislav:

Krzy already sent patch remove him from MAINTANINERS.

Do you have any more concerns about this patch series?

Frank

>
> --
> Regards/Gruss,
>     Boris.
>
> https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCH 0/6] EDAC: fsl-ddr, add imx9 support

[Borislav Petkov]

Frank,

On Wed, Oct 09, 2024 at 11:48:00AM -0400, Frank Li wrote:
> Krzy already sent patch remove him from MAINTANINERS.
> 
> Do you have any more concerns about this patch series?

What are you actually asking?

Whether I should drop everything I'm doing and review your patches?

Do you need to read about the kernel development process and when new stuff
gets queued for the next merge window?

Let's cut to the chase and you explain to me what the reason is for you not
waiting patiently for your turn to come but keep pinging.

So, which is it?

-- 
Regards/Gruss,
    Boris.

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

Re: [PATCH 0/6] EDAC: fsl-ddr, add imx9 support

[Frank Li]

On Wed, Oct 09, 2024 at 06:20:38PM +0200, Borislav Petkov wrote:
> Frank,
>
> On Wed, Oct 09, 2024 at 11:48:00AM -0400, Frank Li wrote:
> > Krzy already sent patch remove him from MAINTANINERS.
> >
> > Do you have any more concerns about this patch series?
>
> What are you actually asking?

I ask if there are any other things to prevent this moving foreward.

>
> Whether I should drop everything I'm doing and review your patches?
>
> Do you need to read about the kernel development process and when new stuff
> gets queued for the next merge window?

I think I understood the process since
git log --oneline --author='Frank Li' v6.10..v6.11 | wc
87

It is first time to work with EDAC. The difference maintainer has differece
method to collect new stuff.

>
> Let's cut to the chase and you explain to me what the reason is for you not
> waiting patiently for your turn to come but keep pinging.
>
> So, which is it?

Generally, 7-10 days is reasonable frequent to ask. Contributor also takes
their time and efforts to make kernel better. Why they have to endure a
questioning or accusatory tone!

These patches was already takes more than 3 months. I ask just because
avoid to hold for the another 3 months just because some none technical
reason.

Frank
>
> --
> Regards/Gruss,
>     Boris.
>
> https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCH 0/6] EDAC: fsl-ddr, add imx9 support

[Borislav Petkov]

On Wed, Oct 09, 2024 at 02:31:10PM -0400, Frank Li wrote:
> Generally, 7-10 days is reasonable frequent to ask. Contributor also takes
> their time and efforts to make kernel better. Why they have to endure a
> questioning

You're asking me about your patches! I'm enduring questioning!

> or accusatory tone!

Where is that accusatory tone exactly? Please point me to it.

> These patches was already takes more than 3 months. I ask just because
> avoid to hold for the another 3 months just because some none technical
> reason.

I'm sorry that you have to hold for three months. I, like all maintainers, am
swamped with work for the next 10 years. You asking every 7-10 days does not
really help.

You're on the TODO list and I will get to your patches when I get to them.

-- 
Regards/Gruss,
    Boris.

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