diff mbox series

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

Message ID 20240709-imx95_edac-v1-1-3e9c146c1b01@nxp.com (mailing list archive)
State New
Headers show
Series EDAC: fsl-ddr, add imx9 support | expand

Commit Message

Frank Li July 9, 2024, 8:23 p.m. UTC
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(-)

Comments

Frank Li Aug. 29, 2024, 10:03 p.m. UTC | #1
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
>
Borislav Petkov Oct. 10, 2024, 3:33 p.m. UTC | #2
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.
Frank Li Oct. 10, 2024, 4:07 p.m. UTC | #3
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
diff mbox series

Patch

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);