[Patch v3 01/11] arch/powerpc/pci: Fix compiling error for mpc85xx_edac

[Patch v3 01/11] arch/powerpc/pci: Fix compiling error for mpc85xx_edac

[York Sun]

Two symbols are missing if mpc85xx_edac driver is compiled as module.

Signed-off-by: York Sun <york.sun@nxp.com>

---
Change log
  v3: Change subject tag
  v2: no change

 arch/powerpc/kernel/pci-common.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c
index 0f7a60f..86bc484 100644
--- a/arch/powerpc/kernel/pci-common.c
+++ b/arch/powerpc/kernel/pci-common.c
@@ -226,6 +226,7 @@ struct pci_controller* pci_find_hose_for_OF_device(struct device_node* node)
 	}
 	return NULL;
 }
+EXPORT_SYMBOL(pci_find_hose_for_OF_device);
 
 /*
  * Reads the interrupt pin to determine if interrupt is use by card.
@@ -1585,6 +1586,7 @@ int early_find_capability(struct pci_controller *hose, int bus, int devfn,
 {
 	return pci_bus_find_capability(fake_pci_bus(hose, bus), devfn, cap);
 }
+EXPORT_SYMBOL(early_find_capability);
 
 struct device_node *pcibios_get_phb_of_node(struct pci_bus *bus)
 {
-- 
2.7.4

Re: [Patch v3 01/11] arch/powerpc/pci: Fix compiling error for mpc85xx_edac

[Andrew Donnellan]

On 05/08/16 08:58, York Sun wrote:
> Two symbols are missing if mpc85xx_edac driver is compiled as module.
>
> Signed-off-by: York Sun <york.sun@nxp.com>

Good catch! One comment below.

Reviewed-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>

>  /*
>   * Reads the interrupt pin to determine if interrupt is use by card.
> @@ -1585,6 +1586,7 @@ int early_find_capability(struct pci_controller *hose, int bus, int devfn,
>  {
>  	return pci_bus_find_capability(fake_pci_bus(hose, bus), devfn, cap);
>  }
> +EXPORT_SYMBOL(early_find_capability);

It'd be nicer for this to be renamed as "pci_early_find_capability" or 
something like that with a "namespace", I think.


Andrew

-- 
Andrew Donnellan              OzLabs, ADL Canberra
andrew.donnellan@au1.ibm.com  IBM Australia Limited

Re: [Patch v3 01/11] arch/powerpc/pci: Fix compiling error for mpc85xx_edac

[york sun]

On 08/04/2016 04:36 PM, Andrew Donnellan wrote:
> On 05/08/16 08:58, York Sun wrote:
>> Two symbols are missing if mpc85xx_edac driver is compiled as module.
>>
>> Signed-off-by: York Sun <york.sun@nxp.com>
>
> Good catch! One comment below.
>
> Reviewed-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>
>
>>  /*
>>   * Reads the interrupt pin to determine if interrupt is use by card.
>> @@ -1585,6 +1586,7 @@ int early_find_capability(struct pci_controller *hose, int bus, int devfn,
>>  {
>>  	return pci_bus_find_capability(fake_pci_bus(hose, bus), devfn, cap);
>>  }
>> +EXPORT_SYMBOL(early_find_capability);
>
> It'd be nicer for this to be renamed as "pci_early_find_capability" or
> something like that with a "namespace", I think.
>

I will rename it if I respin this patch for any reason. Otherwise, I 
will send out another patch to rename it after merging.

York

Re: [Patch v3 01/11] arch/powerpc/pci: Fix compiling error for mpc85xx_edac

[Borislav Petkov]

On Thu, Aug 04, 2016 at 11:39:14PM +0000, york sun wrote:
> I will rename it if I respin this patch for any reason. Otherwise, I 
> will send out another patch to rename it after merging.

Feel free to send an updated one as a reply to this thread.

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--

Re: [Patch v3 01/11] arch/powerpc/pci: Fix compiling error for mpc85xx_edac

[Michael Ellerman]

York Sun <york.sun@nxp.com> writes:

> Two symbols are missing if mpc85xx_edac driver is compiled as module.
>
> Signed-off-by: York Sun <york.sun@nxp.com>
>
> ---
> Change log
>   v3: Change subject tag
>   v2: no change
>
>  arch/powerpc/kernel/pci-common.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c
> index 0f7a60f..86bc484 100644
> --- a/arch/powerpc/kernel/pci-common.c
> +++ b/arch/powerpc/kernel/pci-common.c
> @@ -226,6 +226,7 @@ struct pci_controller* pci_find_hose_for_OF_device(struct device_node* node)
>  	}
>  	return NULL;
>  }
> +EXPORT_SYMBOL(pci_find_hose_for_OF_device);
>  
>  /*
>   * Reads the interrupt pin to determine if interrupt is use by card.
> @@ -1585,6 +1586,7 @@ int early_find_capability(struct pci_controller *hose, int bus, int devfn,
>  {
>  	return pci_bus_find_capability(fake_pci_bus(hose, bus), devfn, cap);
>  }
> +EXPORT_SYMBOL(early_find_capability);

Does the driver really need to use these routines? They're meant for use
early in boot, before PCI is setup.

AFAICS this is just a regular driver, so when it's probed the PCI
devices should have already been scanned. In which case pci_get_device()
could work couldn't it? (I see other edac drivers doing that).

cheers

Re: [Patch v3 01/11] arch/powerpc/pci: Fix compiling error for mpc85xx_edac

[york sun]

On 08/04/2016 08:43 PM, Michael Ellerman wrote:
> York Sun <york.sun@nxp.com> writes:
>
>> Two symbols are missing if mpc85xx_edac driver is compiled as module.
>>
>> Signed-off-by: York Sun <york.sun@nxp.com>
>>
>> ---
>> Change log
>>   v3: Change subject tag
>>   v2: no change
>>
>>  arch/powerpc/kernel/pci-common.c | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c
>> index 0f7a60f..86bc484 100644
>> --- a/arch/powerpc/kernel/pci-common.c
>> +++ b/arch/powerpc/kernel/pci-common.c
>> @@ -226,6 +226,7 @@ struct pci_controller* pci_find_hose_for_OF_device(struct device_node* node)
>>  	}
>>  	return NULL;
>>  }
>> +EXPORT_SYMBOL(pci_find_hose_for_OF_device);
>>
>>  /*
>>   * Reads the interrupt pin to determine if interrupt is use by card.
>> @@ -1585,6 +1586,7 @@ int early_find_capability(struct pci_controller *hose, int bus, int devfn,
>>  {
>>  	return pci_bus_find_capability(fake_pci_bus(hose, bus), devfn, cap);
>>  }
>> +EXPORT_SYMBOL(early_find_capability);
>
> Does the driver really need to use these routines? They're meant for use
> early in boot, before PCI is setup.
>
> AFAICS this is just a regular driver, so when it's probed the PCI
> devices should have already been scanned. In which case pci_get_device()
> could work couldn't it? (I see other edac drivers doing that).
>

I don't have deep knowledge of this driver. What I am trying is to 
separate the common DDR part and share it with ARM platforms. Along the 
way, I found the compiling error if build a module. If exposing these 
functions becomes a concern, I can live without it.

York

Re: [Patch v3 01/11] arch/powerpc/pci: Fix compiling error for mpc85xx_edac

[Borislav Petkov]

On Fri, Aug 05, 2016 at 04:26:26AM +0000, york sun wrote:
> I don't have deep knowledge of this driver. What I am trying is to 
> separate the common DDR part and share it with ARM platforms. Along the 
> way, I found the compiling error if build a module. If exposing these 
> functions becomes a concern, I can live without it.

Perhaps you or Johannes could fix this properly to use pci_get_device()
as the rest of the EDAC drivers do, instead of exporting core PCI
functions...

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--

Re: [Patch v3 01/11] arch/powerpc/pci: Fix compiling error for mpc85xx_edac

[Johannes Thumshirn]

On Fri, Aug 05, 2016 at 09:01:26AM +0200, Borislav Petkov wrote:
> On Fri, Aug 05, 2016 at 04:26:26AM +0000, york sun wrote:
> > I don't have deep knowledge of this driver. What I am trying is to 
> > separate the common DDR part and share it with ARM platforms. Along the 
> > way, I found the compiling error if build a module. If exposing these 
> > functions becomes a concern, I can live without it.
> 
> Perhaps you or Johannes could fix this properly to use pci_get_device()
> as the rest of the EDAC drivers do, instead of exporting core PCI
> functions...

I can give it a shot, but I don't have too much spare time atm and no hardware
to test, so it'll have a strong RFC smell attached to it.

Byte,
	Johannes

-- 
Johannes Thumshirn                                          Storage
jthumshirn@suse.de                                +49 911 74053 689
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

Re: [Patch v3 01/11] arch/powerpc/pci: Fix compiling error for mpc85xx_edac

[york sun]

On 08/05/2016 12:01 AM, Borislav Petkov wrote:
> On Fri, Aug 05, 2016 at 04:26:26AM +0000, york sun wrote:
>> I don't have deep knowledge of this driver. What I am trying is to
>> separate the common DDR part and share it with ARM platforms. Along the
>> way, I found the compiling error if build a module. If exposing these
>> functions becomes a concern, I can live without it.
>
> Perhaps you or Johannes could fix this properly to use pci_get_device()
> as the rest of the EDAC drivers do, instead of exporting core PCI
> functions...
>

Boris,

I'd like to separate the first two patches from this set. They are not 
really related to the DDR part I am working on. It will take me a while 
to sort out the correct fix.

York

Re: [Patch v3 01/11] arch/powerpc/pci: Fix compiling error for mpc85xx_edac

[york sun]

On 08/04/2016 08:43 PM, Michael Ellerman wrote:
> York Sun <york.sun@nxp.com> writes:
>
>> Two symbols are missing if mpc85xx_edac driver is compiled as module.
>>
>> Signed-off-by: York Sun <york.sun@nxp.com>
>>
>> ---
>> Change log
>>   v3: Change subject tag
>>   v2: no change
>>
>>  arch/powerpc/kernel/pci-common.c | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c
>> index 0f7a60f..86bc484 100644
>> --- a/arch/powerpc/kernel/pci-common.c
>> +++ b/arch/powerpc/kernel/pci-common.c
>> @@ -226,6 +226,7 @@ struct pci_controller* pci_find_hose_for_OF_device(struct device_node* node)
>>  	}
>>  	return NULL;
>>  }
>> +EXPORT_SYMBOL(pci_find_hose_for_OF_device);
>>
>>  /*
>>   * Reads the interrupt pin to determine if interrupt is use by card.
>> @@ -1585,6 +1586,7 @@ int early_find_capability(struct pci_controller *hose, int bus, int devfn,
>>  {
>>  	return pci_bus_find_capability(fake_pci_bus(hose, bus), devfn, cap);
>>  }
>> +EXPORT_SYMBOL(early_find_capability);
>
> Does the driver really need to use these routines? They're meant for use
> early in boot, before PCI is setup.
>
> AFAICS this is just a regular driver, so when it's probed the PCI
> devices should have already been scanned. In which case pci_get_device()
> could work couldn't it? (I see other edac drivers doing that).

I am trying to fix this but need some help. We are dealing with PCIe 
controller here. Does it have a bus number assigned at this point? If 
yes, how can I find it? I seem not able to find out where the 
platform_data is filled as well. Can someone kindly point it out to me?

York

Re: [Patch v3 01/11] arch/powerpc/pci: Fix compiling error for mpc85xx_edac

[Scott Wood]

On Fri, 2016-08-05 at 20:29 +0000, york sun wrote:
> On 08/04/2016 08:43 PM, Michael Ellerman wrote:
> > 
> > Does the driver really need to use these routines? They're meant for use
> > early in boot, before PCI is setup.
> > 
> > AFAICS this is just a regular driver, so when it's probed the PCI
> > devices should have already been scanned. In which case pci_get_device()
> > could work couldn't it? (I see other edac drivers doing that).
> I am trying to fix this but need some help. We are dealing with PCIe 
> controller here. Does it have a bus number assigned at this point? If 
> yes, how can I find it? I seem not able to find out where the 
> platform_data is filled as well. Can someone kindly point it out to me?


The platform data comes from add_err_dev() in arch/powerpc/sysdev/fsl_pci.c.

-Scott

Re: [Patch v3 01/11] arch/powerpc/pci: Fix compiling error for mpc85xx_edac

[york sun]

On 08/05/2016 02:09 PM, Scott Wood wrote:
> On Fri, 2016-08-05 at 20:29 +0000, york sun wrote:
>> On 08/04/2016 08:43 PM, Michael Ellerman wrote:
>>>
>>> Does the driver really need to use these routines? They're meant for use
>>> early in boot, before PCI is setup.
>>>
>>> AFAICS this is just a regular driver, so when it's probed the PCI
>>> devices should have already been scanned. In which case pci_get_device()
>>> could work couldn't it? (I see other edac drivers doing that).
>> I am trying to fix this but need some help. We are dealing with PCIe
>> controller here. Does it have a bus number assigned at this point? If
>> yes, how can I find it? I seem not able to find out where the
>> platform_data is filled as well. Can someone kindly point it out to me?
>
>
> The platform data comes from add_err_dev() in arch/powerpc/sysdev/fsl_pci.c.
>

Thanks, Scott.

When add_err_dev() is called, pci is not scanned, is using 
early_find_capability() justified?

York

Re: [Patch v3 01/11] arch/powerpc/pci: Fix compiling error for mpc85xx_edac

[Scott Wood]

On Fri, 2016-08-05 at 21:20 +0000, york sun wrote:
> On 08/05/2016 02:09 PM, Scott Wood wrote:
> > 
> > On Fri, 2016-08-05 at 20:29 +0000, york sun wrote:
> > > 
> > > On 08/04/2016 08:43 PM, Michael Ellerman wrote:
> > > > 
> > > > 
> > > > Does the driver really need to use these routines? They're meant for
> > > > use
> > > > early in boot, before PCI is setup.
> > > > 
> > > > AFAICS this is just a regular driver, so when it's probed the PCI
> > > > devices should have already been scanned. In which case
> > > > pci_get_device()
> > > > could work couldn't it? (I see other edac drivers doing that).
> > > I am trying to fix this but need some help. We are dealing with PCIe
> > > controller here. Does it have a bus number assigned at this point? If
> > > yes, how can I find it? I seem not able to find out where the
> > > platform_data is filled as well. Can someone kindly point it out to me?
> > 
> > The platform data comes from add_err_dev() in
> > arch/powerpc/sysdev/fsl_pci.c.
> > 
> Thanks, Scott.
> 
> When add_err_dev() is called, pci is not scanned, is using 
> early_find_capability() justified?

The edac driver is registered with a normal device-level initcall.  The PCI
scanning appears to happen at the subsys initcall level.

-Scott

[Patch v3 02/11] arch/microblaze/pci: Drop early_find_capability()

[York Sun]

This function is a copy from powerpc, and is never used on microblaze.

Signed-off-by: York Sun <york.sun@nxp.com>
Suggested-by: Bjorn Helgaas <helgaas@kernel.org>

---
This patch has nothing to do with this EDAC patch set, but suggested by
Bjorn Helgaas as good code hygiene, along with patch 01/11 to expose
early_find_capability() for powerpc.

Change log
  v3: new patch

 arch/microblaze/include/asm/pci-bridge.h | 3 ---
 arch/microblaze/pci/pci-common.c         | 7 -------
 2 files changed, 10 deletions(-)

diff --git a/arch/microblaze/include/asm/pci-bridge.h b/arch/microblaze/include/asm/pci-bridge.h
index cb5d397..a0f4a91 100644
--- a/arch/microblaze/include/asm/pci-bridge.h
+++ b/arch/microblaze/include/asm/pci-bridge.h
@@ -121,9 +121,6 @@ extern int early_write_config_word(struct pci_controller *hose, int bus,
 extern int early_write_config_dword(struct pci_controller *hose, int bus,
 			int dev_fn, int where, u32 val);
 
-extern int early_find_capability(struct pci_controller *hose, int bus,
-				 int dev_fn, int cap);
-
 extern void setup_indirect_pci(struct pci_controller *hose,
 			       resource_size_t cfg_addr,
 			       resource_size_t cfg_data, u32 flags);
diff --git a/arch/microblaze/pci/pci-common.c b/arch/microblaze/pci/pci-common.c
index 14cba60..2f4c79e 100644
--- a/arch/microblaze/pci/pci-common.c
+++ b/arch/microblaze/pci/pci-common.c
@@ -1461,10 +1461,3 @@ EARLY_PCI_OP(read, dword, u32 *)
 EARLY_PCI_OP(write, byte, u8)
 EARLY_PCI_OP(write, word, u16)
 EARLY_PCI_OP(write, dword, u32)
-
-int early_find_capability(struct pci_controller *hose, int bus, int devfn,
-			  int cap)
-{
-	return pci_bus_find_capability(fake_pci_bus(hose, bus), devfn, cap);
-}
-
-- 
2.7.4

[Patch v3 03/11] driver/edac/mpc85xx_edac: Drop setting/clearing RFXE bit in HID1

[York Sun]

On e500v1, read fault exception enable (RFXE) controls whether
assertion of core_fault_in causes a machine check interrupt.
Assertion of core_fault_in can result from uncorrectable data
error, such as  an L2 multibit ECC error. It can also occur from
a system error if logic on the integrated device signals a fault
for nonfatal errors. RFXE bit is cleared out of reset, and should
be left clear for normal operation. Assertion of core_fault_in does
not cause a machine check.

RFXE is set specifically for RIO (Rapid IO) and PCI for book E to
catch the errors by machine check. With this bit set, EDAC driver
can't get the interrupt in case of uncorrectable error. So this
bit is cleared in favor of EDAC. However, the benefit of catching
such uncorrectable error doesn't outweight the other errors which
may hang the system. Beside, e500v2 has different errors maksed
by RFXE, and e500mc doesn't support this bit. It is more reasonable
to leave RFXE as is in EDAC driver, and leave the uncorrectable
errors triggering machine check for e500v1.

Signed-off-by: York Sun <york.sun@nxp.com>
Suggested-by: Scott Wood <oss@buserror.net>

---
Change log
  v3: Revise commit message
  v2: new patch in this set

 drivers/edac/mpc85xx_edac.c | 40 ----------------------------------------
 1 file changed, 40 deletions(-)

diff --git a/drivers/edac/mpc85xx_edac.c b/drivers/edac/mpc85xx_edac.c
index ca63d0d..fdc3d9b 100644
--- a/drivers/edac/mpc85xx_edac.c
+++ b/drivers/edac/mpc85xx_edac.c
@@ -46,9 +46,6 @@ static u32 orig_pci_err_en;
 #endif
 
 static u32 orig_l2_err_disable;
-#ifdef CONFIG_FSL_SOC_BOOKE
-static u32 orig_hid1[2];
-#endif
 
 /************************ MC SYSFS parts ***********************************/
 
@@ -1225,14 +1222,6 @@ static struct platform_driver mpc85xx_mc_err_driver = {
 	},
 };
 
-#ifdef CONFIG_FSL_SOC_BOOKE
-static void __init mpc85xx_mc_clear_rfxe(void *data)
-{
-	orig_hid1[smp_processor_id()] = mfspr(SPRN_HID1);
-	mtspr(SPRN_HID1, (orig_hid1[smp_processor_id()] & ~HID1_RFXE));
-}
-#endif
-
 static struct platform_driver * const drivers[] = {
 	&mpc85xx_mc_err_driver,
 	&mpc85xx_l2_err_driver,
@@ -1263,42 +1252,13 @@ static int __init mpc85xx_mc_init(void)
 	if (res)
 		printk(KERN_WARNING EDAC_MOD_STR "drivers fail to register\n");
 
-#ifdef CONFIG_FSL_SOC_BOOKE
-	pvr = mfspr(SPRN_PVR);
-
-	if ((PVR_VER(pvr) == PVR_VER_E500V1) ||
-	    (PVR_VER(pvr) == PVR_VER_E500V2)) {
-		/*
-		 * need to clear HID1[RFXE] to disable machine check int
-		 * so we can catch it
-		 */
-		if (edac_op_state == EDAC_OPSTATE_INT)
-			on_each_cpu(mpc85xx_mc_clear_rfxe, NULL, 0);
-	}
-#endif
-
 	return 0;
 }
 
 module_init(mpc85xx_mc_init);
 
-#ifdef CONFIG_FSL_SOC_BOOKE
-static void __exit mpc85xx_mc_restore_hid1(void *data)
-{
-	mtspr(SPRN_HID1, orig_hid1[smp_processor_id()]);
-}
-#endif
-
 static void __exit mpc85xx_mc_exit(void)
 {
-#ifdef CONFIG_FSL_SOC_BOOKE
-	u32 pvr = mfspr(SPRN_PVR);
-
-	if ((PVR_VER(pvr) == PVR_VER_E500V1) ||
-	    (PVR_VER(pvr) == PVR_VER_E500V2)) {
-		on_each_cpu(mpc85xx_mc_restore_hid1, NULL, 0);
-	}
-#endif
 	platform_unregister_drivers(drivers, ARRAY_SIZE(drivers));
 }
 
-- 
2.7.4

Re: [Patch v3 03/11] driver/edac/mpc85xx_edac: Drop setting/clearing RFXE bit in HID1

[Borislav Petkov]

On Thu, Aug 04, 2016 at 03:58:28PM -0700, York Sun wrote:
> On e500v1, read fault exception enable (RFXE) controls whether
> assertion of core_fault_in causes a machine check interrupt.
> Assertion of core_fault_in can result from uncorrectable data
> error, such as  an L2 multibit ECC error. It can also occur from
> a system error if logic on the integrated device signals a fault
> for nonfatal errors. RFXE bit is cleared out of reset, and should
> be left clear for normal operation. Assertion of core_fault_in does
> not cause a machine check.
> 
> RFXE is set specifically for RIO (Rapid IO) and PCI for book E to
> catch the errors by machine check. With this bit set, EDAC driver
> can't get the interrupt in case of uncorrectable error. So this
> bit is cleared in favor of EDAC. However, the benefit of catching
> such uncorrectable error doesn't outweight the other errors which
> may hang the system. Beside, e500v2 has different errors maksed
> by RFXE, and e500mc doesn't support this bit. It is more reasonable
> to leave RFXE as is in EDAC driver, and leave the uncorrectable
> errors triggering machine check for e500v1.

Very nice, thanks for expanding it!

Two final remarks:

- please use a spell checker

- now, what happens if you leave RFXE clear and mpc85xx_edac gets the
error? Is it going to do proper error handling of the uncorrectable
error or are we better off handling the error in the #MC interrupt
handler?

IOW, is mpc85xx_edac well equipped to handle those multibit errors or
should we leave the current setting as is?

Thanks.

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--

Re: [Patch v3 03/11] driver/edac/mpc85xx_edac: Drop setting/clearing RFXE bit in HID1

[york sun]

On 08/08/2016 12:11 AM, Borislav Petkov wrote:
> On Thu, Aug 04, 2016 at 03:58:28PM -0700, York Sun wrote:
>> On e500v1, read fault exception enable (RFXE) controls whether
>> assertion of core_fault_in causes a machine check interrupt.
>> Assertion of core_fault_in can result from uncorrectable data
>> error, such as  an L2 multibit ECC error. It can also occur from
>> a system error if logic on the integrated device signals a fault
>> for nonfatal errors. RFXE bit is cleared out of reset, and should
>> be left clear for normal operation. Assertion of core_fault_in does
>> not cause a machine check.
>>
>> RFXE is set specifically for RIO (Rapid IO) and PCI for book E to
>> catch the errors by machine check. With this bit set, EDAC driver
>> can't get the interrupt in case of uncorrectable error. So this
>> bit is cleared in favor of EDAC. However, the benefit of catching
>> such uncorrectable error doesn't outweight the other errors which
>> may hang the system. Beside, e500v2 has different errors maksed
>> by RFXE, and e500mc doesn't support this bit. It is more reasonable
>> to leave RFXE as is in EDAC driver, and leave the uncorrectable
>> errors triggering machine check for e500v1.
>
> Very nice, thanks for expanding it!
>
> Two final remarks:
>
> - please use a spell checker
>
> - now, what happens if you leave RFXE clear and mpc85xx_edac gets the
> error? Is it going to do proper error handling of the uncorrectable
> error or are we better off handling the error in the #MC interrupt
> handler?
>
> IOW, is mpc85xx_edac well equipped to handle those multibit errors or
> should we leave the current setting as is?
>

RFXE is cleared by default. So for most SoCs, this is not even a concern 
at all. But for e500v1, when RIO or PCI are used, this bit is set 
specifically to catch an error by machine check (see commit 4e0e3435). 
This is not the uncorrectable error from DDR. We will be better off to 
let this error happen.

York

Re: [Patch v3 03/11] driver/edac/mpc85xx_edac: Drop setting/clearing RFXE bit in HID1

[Borislav Petkov]

On Mon, Aug 08, 2016 at 03:39:44PM +0000, york sun wrote:
> RFXE is cleared by default. So for most SoCs, this is not even a concern 
> at all. But for e500v1, when RIO or PCI are used, this bit is set 
> specifically to catch an error by machine check (see commit 4e0e3435). 
> This is not the uncorrectable error from DDR. We will be better off to 
> let this error happen.

So I'm reading this: "With this bit set, EDAC driver can't get the
interrupt in case of uncorrectable error. So this bit is cleared in
favor of EDAC."

AFAIU, it means, RFXE bit remains clear so EDAC will get the interrupt
for the uncorrectable error (UE). So on those !e500v1 systems, EDAC be
handling those UEs.

Am I close?

If so, can EDAC handle the UE?

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--

Re: [Patch v3 03/11] driver/edac/mpc85xx_edac: Drop setting/clearing RFXE bit in HID1

[york sun]

On 08/08/2016 08:32 PM, Borislav Petkov wrote:
> On Mon, Aug 08, 2016 at 03:39:44PM +0000, york sun wrote:
>> RFXE is cleared by default. So for most SoCs, this is not even a concern
>> at all. But for e500v1, when RIO or PCI are used, this bit is set
>> specifically to catch an error by machine check (see commit 4e0e3435).
>> This is not the uncorrectable error from DDR. We will be better off to
>> let this error happen.
>
> So I'm reading this: "With this bit set, EDAC driver can't get the
> interrupt in case of uncorrectable error. So this bit is cleared in
> favor of EDAC."
>
> AFAIU, it means, RFXE bit remains clear so EDAC will get the interrupt
> for the uncorrectable error (UE). So on those !e500v1 systems, EDAC be
> handling those UEs.
>
> Am I close?
>
> If so, can EDAC handle the UE?
>

Yes, for most SoCs RFXE remains cleared. Uncorrectable errors are 
handled by EDAC.

York

Re: [Patch v3 03/11] driver/edac/mpc85xx_edac: Drop setting/clearing RFXE bit in HID1

[Borislav Petkov]

On Tue, Aug 09, 2016 at 04:31:19AM +0000, york sun wrote:
> Yes, for most SoCs RFXE remains cleared. Uncorrectable errors are 
> handled by EDAC.

And how is mpc85_xxx EDAC handling them?

mpc85xx_mc_check() only reports them.

And now to get to my original question: is it *enough* to report
uncorrectable errors on those platforms or do they need more
sophisticated error handling in order to disable data corruption?

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--

Re: [Patch v3 03/11] driver/edac/mpc85xx_edac: Drop setting/clearing RFXE bit in HID1

[york sun]

On 08/08/2016 10:01 PM, Borislav Petkov wrote:
> On Tue, Aug 09, 2016 at 04:31:19AM +0000, york sun wrote:
>> Yes, for most SoCs RFXE remains cleared. Uncorrectable errors are
>> handled by EDAC.
>
> And how is mpc85_xxx EDAC handling them?
>
> mpc85xx_mc_check() only reports them.

Correct. It can only report this kind of error. It is not correctable. 
Nothing more can be done.

>
> And now to get to my original question: is it *enough* to report
> uncorrectable errors on those platforms or do they need more
> sophisticated error handling in order to disable data corruption?
>

It is uncorrectable. DDR controller can only report the error. I don't 
believe EDAC driver can do more. For the same reason I said we can leave 
RXFE as is, even for e500v1 case (with RIO or PCI is enabled). Nothing 
can be done with uncorrectable error.

York

Re: [Patch v3 03/11] driver/edac/mpc85xx_edac: Drop setting/clearing RFXE bit in HID1

[Borislav Petkov]

On Tue, Aug 09, 2016 at 05:06:39AM +0000, york sun wrote:
> It is uncorrectable. DDR controller can only report the error. I don't 
> believe EDAC driver can do more. For the same reason I said we can leave 
> RXFE as is, even for e500v1 case (with RIO or PCI is enabled). Nothing 
> can be done with uncorrectable error.

Of course it can: it can panic the machine so that it doesn't corrupt
data on secondary storage. You might consider whether this is a better
course of action instead of only reporting the error.

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--

Re: [Patch v3 03/11] driver/edac/mpc85xx_edac: Drop setting/clearing RFXE bit in HID1

[york sun]

On 08/08/2016 11:56 PM, Borislav Petkov wrote:
> On Tue, Aug 09, 2016 at 05:06:39AM +0000, york sun wrote:
>> It is uncorrectable. DDR controller can only report the error. I don't
>> believe EDAC driver can do more. For the same reason I said we can leave
>> RXFE as is, even for e500v1 case (with RIO or PCI is enabled). Nothing
>> can be done with uncorrectable error.
>
> Of course it can: it can panic the machine so that it doesn't corrupt
> data on secondary storage. You might consider whether this is a better
> course of action instead of only reporting the error.
>

Boris,

My opinion is the error shouldn't happen at the first place. It usually 
means wrong configuration or physical error on the hardware. An EDAC 
driver can report these errors. How far you want to go on the error 
handling is up for discussion. I don't think we should include those 
improvement in this patch set.

York

Re: [Patch v3 03/11] driver/edac/mpc85xx_edac: Drop setting/clearing RFXE bit in HID1

[york sun]

On 08/09/2016 08:57 AM, york.sun@nxp.com wrote:
> On 08/08/2016 11:56 PM, Borislav Petkov wrote:
>> On Tue, Aug 09, 2016 at 05:06:39AM +0000, york sun wrote:
>>> It is uncorrectable. DDR controller can only report the error. I don't
>>> believe EDAC driver can do more. For the same reason I said we can leave
>>> RXFE as is, even for e500v1 case (with RIO or PCI is enabled). Nothing
>>> can be done with uncorrectable error.
>>
>> Of course it can: it can panic the machine so that it doesn't corrupt
>> data on secondary storage. You might consider whether this is a better
>> course of action instead of only reporting the error.
>>
>
> Boris,
>
> My opinion is the error shouldn't happen at the first place. It usually
> means wrong configuration or physical error on the hardware. An EDAC
> driver can report these errors. How far you want to go on the error
> handling is up for discussion. I don't think we should include those
> improvement in this patch set.
>

I want to add this, normally uncorrectable errors don't trigger machine 
check on e500v1. RXFE controls different interrupt on e500v2. e500mc 
doesn't support RXFE. Together with the reason I explained, I believe 
EDAC driver shouldn't change RXFE.

I hope I made it clear.

York

Re: [Patch v3 03/11] driver/edac/mpc85xx_edac: Drop setting/clearing RFXE bit in HID1

[Borislav Petkov]

On Tue, Aug 09, 2016 at 04:40:44PM +0000, york sun wrote:
> I want to add this, normally uncorrectable errors don't trigger machine 
> check on e500v1. RXFE controls different interrupt on e500v2. e500mc 
> doesn't support RXFE. Together with the reason I explained, I believe 
> EDAC driver shouldn't change RXFE.
> 
> I hope I made it clear.

Yes, you did, thanks. I reread Scott's previous mail too and yes, it
doesn't make a whole lotta sense to touch the RXFE bit in EDAC. I'm
assuming firmware leaves it cleared coming out of reset so we're all
good.

Thanks.

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--

[Patch v3 04/11] driver/edac/mpc85xx_edac: Replace printk with proper pr_* format

[York Sun]

Replace printk with more preferred pr_err/pr_warn/pr_info format.

Signed-off-by: York Sun <york.sun@nxp.com>

---
Change log
  v3: no change
  v2: Reordered patch. Change more printk statement than v1 patch.

 drivers/edac/mpc85xx_edac.c | 72 ++++++++++++++++++++++-----------------------
 1 file changed, 35 insertions(+), 37 deletions(-)

diff --git a/drivers/edac/mpc85xx_edac.c b/drivers/edac/mpc85xx_edac.c
index fdc3d9b..c0b0951 100644
--- a/drivers/edac/mpc85xx_edac.c
+++ b/drivers/edac/mpc85xx_edac.c
@@ -157,18 +157,18 @@ static void mpc85xx_pci_check(struct edac_pci_ctl_info *pci)
 		return;
 	}
 
-	printk(KERN_ERR "PCI error(s) detected\n");
-	printk(KERN_ERR "PCI/X ERR_DR register: %#08x\n", err_detect);
+	pr_err("PCI error(s) detected\n");
+	pr_err("PCI/X ERR_DR register: %#08x\n", err_detect);
 
-	printk(KERN_ERR "PCI/X ERR_ATTRIB register: %#08x\n",
+	pr_err("PCI/X ERR_ATTRIB register: %#08x\n",
 	       in_be32(pdata->pci_vbase + MPC85XX_PCI_ERR_ATTRIB));
-	printk(KERN_ERR "PCI/X ERR_ADDR register: %#08x\n",
+	pr_err("PCI/X ERR_ADDR register: %#08x\n",
 	       in_be32(pdata->pci_vbase + MPC85XX_PCI_ERR_ADDR));
-	printk(KERN_ERR "PCI/X ERR_EXT_ADDR register: %#08x\n",
+	pr_err("PCI/X ERR_EXT_ADDR register: %#08x\n",
 	       in_be32(pdata->pci_vbase + MPC85XX_PCI_ERR_EXT_ADDR));
-	printk(KERN_ERR "PCI/X ERR_DL register: %#08x\n",
+	pr_err("PCI/X ERR_DL register: %#08x\n",
 	       in_be32(pdata->pci_vbase + MPC85XX_PCI_ERR_DL));
-	printk(KERN_ERR "PCI/X ERR_DH register: %#08x\n",
+	pr_err("PCI/X ERR_DH register: %#08x\n",
 	       in_be32(pdata->pci_vbase + MPC85XX_PCI_ERR_DH));
 
 	/* clear error bits */
@@ -294,7 +294,7 @@ static int mpc85xx_pci_err_probe(struct platform_device *op)
 
 	res = of_address_to_resource(of_node, 0, &r);
 	if (res) {
-		printk(KERN_ERR "%s: Unable to get resource for "
+		pr_err("%s: Unable to get resource for "
 		       "PCI err regs\n", __func__);
 		goto err;
 	}
@@ -304,7 +304,7 @@ static int mpc85xx_pci_err_probe(struct platform_device *op)
 
 	if (!devm_request_mem_region(&op->dev, r.start, resource_size(&r),
 					pdata->name)) {
-		printk(KERN_ERR "%s: Error while requesting mem region\n",
+		pr_err("%s: Error while requesting mem region\n",
 		       __func__);
 		res = -EBUSY;
 		goto err;
@@ -312,7 +312,7 @@ static int mpc85xx_pci_err_probe(struct platform_device *op)
 
 	pdata->pci_vbase = devm_ioremap(&op->dev, r.start, resource_size(&r));
 	if (!pdata->pci_vbase) {
-		printk(KERN_ERR "%s: Unable to setup PCI err regs\n", __func__);
+		pr_err("%s: Unable to setup PCI err regs\n", __func__);
 		res = -ENOMEM;
 		goto err;
 	}
@@ -353,15 +353,14 @@ static int mpc85xx_pci_err_probe(struct platform_device *op)
 				       IRQF_SHARED,
 				       "[EDAC] PCI err", pci);
 		if (res < 0) {
-			printk(KERN_ERR
-			       "%s: Unable to request irq %d for "
+			pr_err("%s: Unable to request irq %d for "
 			       "MPC85xx PCI err\n", __func__, pdata->irq);
 			irq_dispose_mapping(pdata->irq);
 			res = -ENODEV;
 			goto err2;
 		}
 
-		printk(KERN_INFO EDAC_MOD_STR " acquired irq %d for PCI Err\n",
+		pr_info(EDAC_MOD_STR " acquired irq %d for PCI Err\n",
 		       pdata->irq);
 	}
 
@@ -383,7 +382,7 @@ static int mpc85xx_pci_err_probe(struct platform_device *op)
 
 	devres_remove_group(&op->dev, mpc85xx_pci_err_probe);
 	edac_dbg(3, "success\n");
-	printk(KERN_INFO EDAC_MOD_STR " PCI err registered\n");
+	pr_info(EDAC_MOD_STR " PCI err registered\n");
 
 	return 0;
 
@@ -526,17 +525,17 @@ static void mpc85xx_l2_check(struct edac_device_ctl_info *edac_dev)
 	if (!(err_detect & L2_EDE_MASK))
 		return;
 
-	printk(KERN_ERR "ECC Error in CPU L2 cache\n");
-	printk(KERN_ERR "L2 Error Detect Register: 0x%08x\n", err_detect);
-	printk(KERN_ERR "L2 Error Capture Data High Register: 0x%08x\n",
+	pr_err("ECC Error in CPU L2 cache\n");
+	pr_err("L2 Error Detect Register: 0x%08x\n", err_detect);
+	pr_err("L2 Error Capture Data High Register: 0x%08x\n",
 	       in_be32(pdata->l2_vbase + MPC85XX_L2_CAPTDATAHI));
-	printk(KERN_ERR "L2 Error Capture Data Lo Register: 0x%08x\n",
+	pr_err("L2 Error Capture Data Lo Register: 0x%08x\n",
 	       in_be32(pdata->l2_vbase + MPC85XX_L2_CAPTDATALO));
-	printk(KERN_ERR "L2 Error Syndrome Register: 0x%08x\n",
+	pr_err("L2 Error Syndrome Register: 0x%08x\n",
 	       in_be32(pdata->l2_vbase + MPC85XX_L2_CAPTECC));
-	printk(KERN_ERR "L2 Error Attributes Capture Register: 0x%08x\n",
+	pr_err("L2 Error Attributes Capture Register: 0x%08x\n",
 	       in_be32(pdata->l2_vbase + MPC85XX_L2_ERRATTR));
-	printk(KERN_ERR "L2 Error Address Capture Register: 0x%08x\n",
+	pr_err("L2 Error Address Capture Register: 0x%08x\n",
 	       in_be32(pdata->l2_vbase + MPC85XX_L2_ERRADDR));
 
 	/* clear error detect register */
@@ -593,7 +592,7 @@ static int mpc85xx_l2_err_probe(struct platform_device *op)
 
 	res = of_address_to_resource(op->dev.of_node, 0, &r);
 	if (res) {
-		printk(KERN_ERR "%s: Unable to get resource for "
+		pr_err("%s: Unable to get resource for "
 		       "L2 err regs\n", __func__);
 		goto err;
 	}
@@ -603,7 +602,7 @@ static int mpc85xx_l2_err_probe(struct platform_device *op)
 
 	if (!devm_request_mem_region(&op->dev, r.start, resource_size(&r),
 				     pdata->name)) {
-		printk(KERN_ERR "%s: Error while requesting mem region\n",
+		pr_err("%s: Error while requesting mem region\n",
 		       __func__);
 		res = -EBUSY;
 		goto err;
@@ -611,7 +610,7 @@ static int mpc85xx_l2_err_probe(struct platform_device *op)
 
 	pdata->l2_vbase = devm_ioremap(&op->dev, r.start, resource_size(&r));
 	if (!pdata->l2_vbase) {
-		printk(KERN_ERR "%s: Unable to setup L2 err regs\n", __func__);
+		pr_err("%s: Unable to setup L2 err regs\n", __func__);
 		res = -ENOMEM;
 		goto err;
 	}
@@ -643,15 +642,14 @@ static int mpc85xx_l2_err_probe(struct platform_device *op)
 				       mpc85xx_l2_isr, IRQF_SHARED,
 				       "[EDAC] L2 err", edac_dev);
 		if (res < 0) {
-			printk(KERN_ERR
-			       "%s: Unable to request irq %d for "
+			pr_err("%s: Unable to request irq %d for "
 			       "MPC85xx L2 err\n", __func__, pdata->irq);
 			irq_dispose_mapping(pdata->irq);
 			res = -ENODEV;
 			goto err2;
 		}
 
-		printk(KERN_INFO EDAC_MOD_STR " acquired irq %d for L2 Err\n",
+		pr_info(EDAC_MOD_STR " acquired irq %d for L2 Err\n",
 		       pdata->irq);
 
 		edac_dev->op_state = OP_RUNNING_INTERRUPT;
@@ -662,7 +660,7 @@ static int mpc85xx_l2_err_probe(struct platform_device *op)
 	devres_remove_group(&op->dev, mpc85xx_l2_err_probe);
 
 	edac_dbg(3, "success\n");
-	printk(KERN_INFO EDAC_MOD_STR " L2 err registered\n");
+	pr_info(EDAC_MOD_STR " L2 err registered\n");
 
 	return 0;
 
@@ -1064,14 +1062,14 @@ static int mpc85xx_mc_err_probe(struct platform_device *op)
 
 	res = of_address_to_resource(op->dev.of_node, 0, &r);
 	if (res) {
-		printk(KERN_ERR "%s: Unable to get resource for MC err regs\n",
+		pr_err("%s: Unable to get resource for MC err regs\n",
 		       __func__);
 		goto err;
 	}
 
 	if (!devm_request_mem_region(&op->dev, r.start, resource_size(&r),
 				     pdata->name)) {
-		printk(KERN_ERR "%s: Error while requesting mem region\n",
+		pr_err("%s: Error while requesting mem region\n",
 		       __func__);
 		res = -EBUSY;
 		goto err;
@@ -1079,7 +1077,7 @@ static int mpc85xx_mc_err_probe(struct platform_device *op)
 
 	pdata->mc_vbase = devm_ioremap(&op->dev, r.start, resource_size(&r));
 	if (!pdata->mc_vbase) {
-		printk(KERN_ERR "%s: Unable to setup MC err regs\n", __func__);
+		pr_err("%s: Unable to setup MC err regs\n", __func__);
 		res = -ENOMEM;
 		goto err;
 	}
@@ -1087,7 +1085,7 @@ static int mpc85xx_mc_err_probe(struct platform_device *op)
 	sdram_ctl = in_be32(pdata->mc_vbase + MPC85XX_MC_DDR_SDRAM_CFG);
 	if (!(sdram_ctl & DSC_ECC_EN)) {
 		/* no ECC */
-		printk(KERN_WARNING "%s: No ECC DIMMs discovered\n", __func__);
+		pr_warn("%s: No ECC DIMMs discovered\n", __func__);
 		res = -ENODEV;
 		goto err;
 	}
@@ -1140,20 +1138,20 @@ static int mpc85xx_mc_err_probe(struct platform_device *op)
 				       IRQF_SHARED,
 				       "[EDAC] MC err", mci);
 		if (res < 0) {
-			printk(KERN_ERR "%s: Unable to request irq %d for "
+			pr_err("%s: Unable to request irq %d for "
 			       "MPC85xx DRAM ERR\n", __func__, pdata->irq);
 			irq_dispose_mapping(pdata->irq);
 			res = -ENODEV;
 			goto err2;
 		}
 
-		printk(KERN_INFO EDAC_MOD_STR " acquired irq %d for MC\n",
+		pr_info(EDAC_MOD_STR " acquired irq %d for MC\n",
 		       pdata->irq);
 	}
 
 	devres_remove_group(&op->dev, mpc85xx_mc_err_probe);
 	edac_dbg(3, "success\n");
-	printk(KERN_INFO EDAC_MOD_STR " MC err registered\n");
+	pr_info(EDAC_MOD_STR " MC err registered\n");
 
 	return 0;
 
@@ -1235,7 +1233,7 @@ static int __init mpc85xx_mc_init(void)
 	int res = 0;
 	u32 __maybe_unused pvr = 0;
 
-	printk(KERN_INFO "Freescale(R) MPC85xx EDAC driver, "
+	pr_info("Freescale(R) MPC85xx EDAC driver, "
 	       "(C) 2006 Montavista Software\n");
 
 	/* make sure error reporting method is sane */
@@ -1250,7 +1248,7 @@ static int __init mpc85xx_mc_init(void)
 
 	res = platform_register_drivers(drivers, ARRAY_SIZE(drivers));
 	if (res)
-		printk(KERN_WARNING EDAC_MOD_STR "drivers fail to register\n");
+		pr_warn(EDAC_MOD_STR "drivers fail to register\n");
 
 	return 0;
 }
-- 
2.7.4

[Patch v3 05/11] driver/edac/fsl-ddr: Separate FSL DDR EDAC driver from MPC85xx

[York Sun]

The mpc85xx compatible DDR controllers are used on ARM-based SoCs.
Separate the DDR part from mpc85xx EDAC driver and prepare to support
both architecture.

Signed-off-by: York Sun <york.sun@nxp.com>

---
Change log
  v3: Fix compiling errors and warnings caused by patch ordering
  v2: Reordered patch
      Separate FSL DDR commont code as shared object, not another driver
      This patch is generated with "git format-patch -M40 -C40" to show
      copy-and-delete.

 drivers/edac/Makefile                           |   5 +-
 drivers/edac/{mpc85xx_edac.c => fsl_ddr_edac.c} | 699 +-----------------------
 drivers/edac/{mpc85xx_edac.h => fsl_ddr_edac.h} | 106 +---
 drivers/edac/mpc85xx_edac.c                     | 559 +------------------
 drivers/edac/mpc85xx_edac.h                     |  66 ---
 5 files changed, 22 insertions(+), 1413 deletions(-)
 copy drivers/edac/{mpc85xx_edac.c => fsl_ddr_edac.c} (43%)
 copy drivers/edac/{mpc85xx_edac.h => fsl_ddr_edac.h} (43%)

diff --git a/drivers/edac/Makefile b/drivers/edac/Makefile
index f9e4a3e..ee047a4 100644
--- a/drivers/edac/Makefile
+++ b/drivers/edac/Makefile
@@ -50,7 +50,10 @@ amd64_edac_mod-$(CONFIG_EDAC_AMD64_ERROR_INJECTION) += amd64_edac_inj.o
 obj-$(CONFIG_EDAC_AMD64)		+= amd64_edac_mod.o
 
 obj-$(CONFIG_EDAC_PASEMI)		+= pasemi_edac.o
-obj-$(CONFIG_EDAC_MPC85XX)		+= mpc85xx_edac.o
+
+mpc85xx_edac_mod-y			:= fsl_ddr_edac.o mpc85xx_edac.o
+obj-$(CONFIG_EDAC_MPC85XX)		+= mpc85xx_edac_mod.o
+
 obj-$(CONFIG_EDAC_MV64X60)		+= mv64x60_edac.o
 obj-$(CONFIG_EDAC_CELL)			+= cell_edac.o
 obj-$(CONFIG_EDAC_PPC4XX)		+= ppc4xx_edac.o
diff --git a/drivers/edac/mpc85xx_edac.c b/drivers/edac/fsl_ddr_edac.c
similarity index 43%
copy from drivers/edac/mpc85xx_edac.c
copy to drivers/edac/fsl_ddr_edac.c
index c0b0951..280797e 100644
--- a/drivers/edac/mpc85xx_edac.c
+++ b/drivers/edac/fsl_ddr_edac.c
@@ -1,5 +1,5 @@
 /*
- * Freescale MPC85xx Memory Controller kernel module
+ * Freescale Memory Controller kernel module
  *
  * Parts Copyrighted (c) 2013 by Freescale Semiconductor, Inc.
  *
@@ -20,33 +20,20 @@
 #include <linux/edac.h>
 #include <linux/smp.h>
 #include <linux/gfp.h>
-#include <linux/fsl/edac.h>
 
 #include <linux/of_platform.h>
 #include <linux/of_device.h>
 #include "edac_module.h"
 #include "edac_core.h"
-#include "mpc85xx_edac.h"
+#include "fsl_ddr_edac.h"
+
+#define EDAC_MOD_STR	"FSL_DDR_EDAC"
 
-static int edac_dev_idx;
-#ifdef CONFIG_PCI
-static int edac_pci_idx;
-#endif
 static int edac_mc_idx;
 
 static u32 orig_ddr_err_disable;
 static u32 orig_ddr_err_sbe;
 
-/*
- * PCI Err defines
- */
-#ifdef CONFIG_PCI
-static u32 orig_pci_err_cap_dr;
-static u32 orig_pci_err_en;
-#endif
-
-static u32 orig_l2_err_disable;
-
 /************************ MC SYSFS parts ***********************************/
 
 #define to_mci(k) container_of(k, struct mem_ctl_info, dev)
@@ -141,589 +128,6 @@ static struct attribute *mpc85xx_dev_attrs[] = {
 
 ATTRIBUTE_GROUPS(mpc85xx_dev);
 
-/**************************** PCI Err device ***************************/
-#ifdef CONFIG_PCI
-
-static void mpc85xx_pci_check(struct edac_pci_ctl_info *pci)
-{
-	struct mpc85xx_pci_pdata *pdata = pci->pvt_info;
-	u32 err_detect;
-
-	err_detect = in_be32(pdata->pci_vbase + MPC85XX_PCI_ERR_DR);
-
-	/* master aborts can happen during PCI config cycles */
-	if (!(err_detect & ~(PCI_EDE_MULTI_ERR | PCI_EDE_MST_ABRT))) {
-		out_be32(pdata->pci_vbase + MPC85XX_PCI_ERR_DR, err_detect);
-		return;
-	}
-
-	pr_err("PCI error(s) detected\n");
-	pr_err("PCI/X ERR_DR register: %#08x\n", err_detect);
-
-	pr_err("PCI/X ERR_ATTRIB register: %#08x\n",
-	       in_be32(pdata->pci_vbase + MPC85XX_PCI_ERR_ATTRIB));
-	pr_err("PCI/X ERR_ADDR register: %#08x\n",
-	       in_be32(pdata->pci_vbase + MPC85XX_PCI_ERR_ADDR));
-	pr_err("PCI/X ERR_EXT_ADDR register: %#08x\n",
-	       in_be32(pdata->pci_vbase + MPC85XX_PCI_ERR_EXT_ADDR));
-	pr_err("PCI/X ERR_DL register: %#08x\n",
-	       in_be32(pdata->pci_vbase + MPC85XX_PCI_ERR_DL));
-	pr_err("PCI/X ERR_DH register: %#08x\n",
-	       in_be32(pdata->pci_vbase + MPC85XX_PCI_ERR_DH));
-
-	/* clear error bits */
-	out_be32(pdata->pci_vbase + MPC85XX_PCI_ERR_DR, err_detect);
-
-	if (err_detect & PCI_EDE_PERR_MASK)
-		edac_pci_handle_pe(pci, pci->ctl_name);
-
-	if ((err_detect & ~PCI_EDE_MULTI_ERR) & ~PCI_EDE_PERR_MASK)
-		edac_pci_handle_npe(pci, pci->ctl_name);
-}
-
-static void mpc85xx_pcie_check(struct edac_pci_ctl_info *pci)
-{
-	struct mpc85xx_pci_pdata *pdata = pci->pvt_info;
-	u32 err_detect;
-
-	err_detect = in_be32(pdata->pci_vbase + MPC85XX_PCI_ERR_DR);
-
-	pr_err("PCIe error(s) detected\n");
-	pr_err("PCIe ERR_DR register: 0x%08x\n", err_detect);
-	pr_err("PCIe ERR_CAP_STAT register: 0x%08x\n",
-			in_be32(pdata->pci_vbase + MPC85XX_PCI_GAS_TIMR));
-	pr_err("PCIe ERR_CAP_R0 register: 0x%08x\n",
-			in_be32(pdata->pci_vbase + MPC85XX_PCIE_ERR_CAP_R0));
-	pr_err("PCIe ERR_CAP_R1 register: 0x%08x\n",
-			in_be32(pdata->pci_vbase + MPC85XX_PCIE_ERR_CAP_R1));
-	pr_err("PCIe ERR_CAP_R2 register: 0x%08x\n",
-			in_be32(pdata->pci_vbase + MPC85XX_PCIE_ERR_CAP_R2));
-	pr_err("PCIe ERR_CAP_R3 register: 0x%08x\n",
-			in_be32(pdata->pci_vbase + MPC85XX_PCIE_ERR_CAP_R3));
-
-	/* clear error bits */
-	out_be32(pdata->pci_vbase + MPC85XX_PCI_ERR_DR, err_detect);
-}
-
-static int mpc85xx_pcie_find_capability(struct device_node *np)
-{
-	struct pci_controller *hose;
-
-	if (!np)
-		return -EINVAL;
-
-	hose = pci_find_hose_for_OF_device(np);
-
-	return early_find_capability(hose, 0, 0, PCI_CAP_ID_EXP);
-}
-
-static irqreturn_t mpc85xx_pci_isr(int irq, void *dev_id)
-{
-	struct edac_pci_ctl_info *pci = dev_id;
-	struct mpc85xx_pci_pdata *pdata = pci->pvt_info;
-	u32 err_detect;
-
-	err_detect = in_be32(pdata->pci_vbase + MPC85XX_PCI_ERR_DR);
-
-	if (!err_detect)
-		return IRQ_NONE;
-
-	if (pdata->is_pcie)
-		mpc85xx_pcie_check(pci);
-	else
-		mpc85xx_pci_check(pci);
-
-	return IRQ_HANDLED;
-}
-
-static int mpc85xx_pci_err_probe(struct platform_device *op)
-{
-	struct edac_pci_ctl_info *pci;
-	struct mpc85xx_pci_pdata *pdata;
-	struct mpc85xx_edac_pci_plat_data *plat_data;
-	struct device_node *of_node;
-	struct resource r;
-	int res = 0;
-
-	if (!devres_open_group(&op->dev, mpc85xx_pci_err_probe, GFP_KERNEL))
-		return -ENOMEM;
-
-	pci = edac_pci_alloc_ctl_info(sizeof(*pdata), "mpc85xx_pci_err");
-	if (!pci)
-		return -ENOMEM;
-
-	/* make sure error reporting method is sane */
-	switch (edac_op_state) {
-	case EDAC_OPSTATE_POLL:
-	case EDAC_OPSTATE_INT:
-		break;
-	default:
-		edac_op_state = EDAC_OPSTATE_INT;
-		break;
-	}
-
-	pdata = pci->pvt_info;
-	pdata->name = "mpc85xx_pci_err";
-	pdata->irq = NO_IRQ;
-
-	plat_data = op->dev.platform_data;
-	if (!plat_data) {
-		dev_err(&op->dev, "no platform data");
-		res = -ENXIO;
-		goto err;
-	}
-	of_node = plat_data->of_node;
-
-	if (mpc85xx_pcie_find_capability(of_node) > 0)
-		pdata->is_pcie = true;
-
-	dev_set_drvdata(&op->dev, pci);
-	pci->dev = &op->dev;
-	pci->mod_name = EDAC_MOD_STR;
-	pci->ctl_name = pdata->name;
-	pci->dev_name = dev_name(&op->dev);
-
-	if (edac_op_state == EDAC_OPSTATE_POLL) {
-		if (pdata->is_pcie)
-			pci->edac_check = mpc85xx_pcie_check;
-		else
-			pci->edac_check = mpc85xx_pci_check;
-	}
-
-	pdata->edac_idx = edac_pci_idx++;
-
-	res = of_address_to_resource(of_node, 0, &r);
-	if (res) {
-		pr_err("%s: Unable to get resource for "
-		       "PCI err regs\n", __func__);
-		goto err;
-	}
-
-	/* we only need the error registers */
-	r.start += 0xe00;
-
-	if (!devm_request_mem_region(&op->dev, r.start, resource_size(&r),
-					pdata->name)) {
-		pr_err("%s: Error while requesting mem region\n",
-		       __func__);
-		res = -EBUSY;
-		goto err;
-	}
-
-	pdata->pci_vbase = devm_ioremap(&op->dev, r.start, resource_size(&r));
-	if (!pdata->pci_vbase) {
-		pr_err("%s: Unable to setup PCI err regs\n", __func__);
-		res = -ENOMEM;
-		goto err;
-	}
-
-	if (pdata->is_pcie) {
-		orig_pci_err_cap_dr =
-		    in_be32(pdata->pci_vbase + MPC85XX_PCI_ERR_ADDR);
-		out_be32(pdata->pci_vbase + MPC85XX_PCI_ERR_ADDR, ~0);
-		orig_pci_err_en =
-		    in_be32(pdata->pci_vbase + MPC85XX_PCI_ERR_EN);
-		out_be32(pdata->pci_vbase + MPC85XX_PCI_ERR_EN, 0);
-	} else {
-		orig_pci_err_cap_dr =
-		    in_be32(pdata->pci_vbase + MPC85XX_PCI_ERR_CAP_DR);
-
-		/* PCI master abort is expected during config cycles */
-		out_be32(pdata->pci_vbase + MPC85XX_PCI_ERR_CAP_DR, 0x40);
-
-		orig_pci_err_en =
-		    in_be32(pdata->pci_vbase + MPC85XX_PCI_ERR_EN);
-
-		/* disable master abort reporting */
-		out_be32(pdata->pci_vbase + MPC85XX_PCI_ERR_EN, ~0x40);
-	}
-
-	/* clear error bits */
-	out_be32(pdata->pci_vbase + MPC85XX_PCI_ERR_DR, ~0);
-
-	if (edac_pci_add_device(pci, pdata->edac_idx) > 0) {
-		edac_dbg(3, "failed edac_pci_add_device()\n");
-		goto err;
-	}
-
-	if (edac_op_state == EDAC_OPSTATE_INT) {
-		pdata->irq = irq_of_parse_and_map(of_node, 0);
-		res = devm_request_irq(&op->dev, pdata->irq,
-				       mpc85xx_pci_isr,
-				       IRQF_SHARED,
-				       "[EDAC] PCI err", pci);
-		if (res < 0) {
-			pr_err("%s: Unable to request irq %d for "
-			       "MPC85xx PCI err\n", __func__, pdata->irq);
-			irq_dispose_mapping(pdata->irq);
-			res = -ENODEV;
-			goto err2;
-		}
-
-		pr_info(EDAC_MOD_STR " acquired irq %d for PCI Err\n",
-		       pdata->irq);
-	}
-
-	if (pdata->is_pcie) {
-		/*
-		 * Enable all PCIe error interrupt & error detect except invalid
-		 * PEX_CONFIG_ADDR/PEX_CONFIG_DATA access interrupt generation
-		 * enable bit and invalid PEX_CONFIG_ADDR/PEX_CONFIG_DATA access
-		 * detection enable bit. Because PCIe bus code to initialize and
-		 * configure these PCIe devices on booting will use some invalid
-		 * PEX_CONFIG_ADDR/PEX_CONFIG_DATA, edac driver prints the much
-		 * notice information. So disable this detect to fix ugly print.
-		 */
-		out_be32(pdata->pci_vbase + MPC85XX_PCI_ERR_EN, ~0
-			 & ~PEX_ERR_ICCAIE_EN_BIT);
-		out_be32(pdata->pci_vbase + MPC85XX_PCI_ERR_ADDR, 0
-			 | PEX_ERR_ICCAD_DISR_BIT);
-	}
-
-	devres_remove_group(&op->dev, mpc85xx_pci_err_probe);
-	edac_dbg(3, "success\n");
-	pr_info(EDAC_MOD_STR " PCI err registered\n");
-
-	return 0;
-
-err2:
-	edac_pci_del_device(&op->dev);
-err:
-	edac_pci_free_ctl_info(pci);
-	devres_release_group(&op->dev, mpc85xx_pci_err_probe);
-	return res;
-}
-
-static const struct platform_device_id mpc85xx_pci_err_match[] = {
-	{
-		.name = "mpc85xx-pci-edac"
-	},
-	{}
-};
-
-static struct platform_driver mpc85xx_pci_err_driver = {
-	.probe = mpc85xx_pci_err_probe,
-	.id_table = mpc85xx_pci_err_match,
-	.driver = {
-		.name = "mpc85xx_pci_err",
-		.suppress_bind_attrs = true,
-	},
-};
-#endif				/* CONFIG_PCI */
-
-/**************************** L2 Err device ***************************/
-
-/************************ L2 SYSFS parts ***********************************/
-
-static ssize_t mpc85xx_l2_inject_data_hi_show(struct edac_device_ctl_info
-					      *edac_dev, char *data)
-{
-	struct mpc85xx_l2_pdata *pdata = edac_dev->pvt_info;
-	return sprintf(data, "0x%08x",
-		       in_be32(pdata->l2_vbase + MPC85XX_L2_ERRINJHI));
-}
-
-static ssize_t mpc85xx_l2_inject_data_lo_show(struct edac_device_ctl_info
-					      *edac_dev, char *data)
-{
-	struct mpc85xx_l2_pdata *pdata = edac_dev->pvt_info;
-	return sprintf(data, "0x%08x",
-		       in_be32(pdata->l2_vbase + MPC85XX_L2_ERRINJLO));
-}
-
-static ssize_t mpc85xx_l2_inject_ctrl_show(struct edac_device_ctl_info
-					   *edac_dev, char *data)
-{
-	struct mpc85xx_l2_pdata *pdata = edac_dev->pvt_info;
-	return sprintf(data, "0x%08x",
-		       in_be32(pdata->l2_vbase + MPC85XX_L2_ERRINJCTL));
-}
-
-static ssize_t mpc85xx_l2_inject_data_hi_store(struct edac_device_ctl_info
-					       *edac_dev, const char *data,
-					       size_t count)
-{
-	struct mpc85xx_l2_pdata *pdata = edac_dev->pvt_info;
-	if (isdigit(*data)) {
-		out_be32(pdata->l2_vbase + MPC85XX_L2_ERRINJHI,
-			 simple_strtoul(data, NULL, 0));
-		return count;
-	}
-	return 0;
-}
-
-static ssize_t mpc85xx_l2_inject_data_lo_store(struct edac_device_ctl_info
-					       *edac_dev, const char *data,
-					       size_t count)
-{
-	struct mpc85xx_l2_pdata *pdata = edac_dev->pvt_info;
-	if (isdigit(*data)) {
-		out_be32(pdata->l2_vbase + MPC85XX_L2_ERRINJLO,
-			 simple_strtoul(data, NULL, 0));
-		return count;
-	}
-	return 0;
-}
-
-static ssize_t mpc85xx_l2_inject_ctrl_store(struct edac_device_ctl_info
-					    *edac_dev, const char *data,
-					    size_t count)
-{
-	struct mpc85xx_l2_pdata *pdata = edac_dev->pvt_info;
-	if (isdigit(*data)) {
-		out_be32(pdata->l2_vbase + MPC85XX_L2_ERRINJCTL,
-			 simple_strtoul(data, NULL, 0));
-		return count;
-	}
-	return 0;
-}
-
-static struct edac_dev_sysfs_attribute mpc85xx_l2_sysfs_attributes[] = {
-	{
-	 .attr = {
-		  .name = "inject_data_hi",
-		  .mode = (S_IRUGO | S_IWUSR)
-		  },
-	 .show = mpc85xx_l2_inject_data_hi_show,
-	 .store = mpc85xx_l2_inject_data_hi_store},
-	{
-	 .attr = {
-		  .name = "inject_data_lo",
-		  .mode = (S_IRUGO | S_IWUSR)
-		  },
-	 .show = mpc85xx_l2_inject_data_lo_show,
-	 .store = mpc85xx_l2_inject_data_lo_store},
-	{
-	 .attr = {
-		  .name = "inject_ctrl",
-		  .mode = (S_IRUGO | S_IWUSR)
-		  },
-	 .show = mpc85xx_l2_inject_ctrl_show,
-	 .store = mpc85xx_l2_inject_ctrl_store},
-
-	/* End of list */
-	{
-	 .attr = {.name = NULL}
-	 }
-};
-
-static void mpc85xx_set_l2_sysfs_attributes(struct edac_device_ctl_info
-					    *edac_dev)
-{
-	edac_dev->sysfs_attributes = mpc85xx_l2_sysfs_attributes;
-}
-
-/***************************** L2 ops ***********************************/
-
-static void mpc85xx_l2_check(struct edac_device_ctl_info *edac_dev)
-{
-	struct mpc85xx_l2_pdata *pdata = edac_dev->pvt_info;
-	u32 err_detect;
-
-	err_detect = in_be32(pdata->l2_vbase + MPC85XX_L2_ERRDET);
-
-	if (!(err_detect & L2_EDE_MASK))
-		return;
-
-	pr_err("ECC Error in CPU L2 cache\n");
-	pr_err("L2 Error Detect Register: 0x%08x\n", err_detect);
-	pr_err("L2 Error Capture Data High Register: 0x%08x\n",
-	       in_be32(pdata->l2_vbase + MPC85XX_L2_CAPTDATAHI));
-	pr_err("L2 Error Capture Data Lo Register: 0x%08x\n",
-	       in_be32(pdata->l2_vbase + MPC85XX_L2_CAPTDATALO));
-	pr_err("L2 Error Syndrome Register: 0x%08x\n",
-	       in_be32(pdata->l2_vbase + MPC85XX_L2_CAPTECC));
-	pr_err("L2 Error Attributes Capture Register: 0x%08x\n",
-	       in_be32(pdata->l2_vbase + MPC85XX_L2_ERRATTR));
-	pr_err("L2 Error Address Capture Register: 0x%08x\n",
-	       in_be32(pdata->l2_vbase + MPC85XX_L2_ERRADDR));
-
-	/* clear error detect register */
-	out_be32(pdata->l2_vbase + MPC85XX_L2_ERRDET, err_detect);
-
-	if (err_detect & L2_EDE_CE_MASK)
-		edac_device_handle_ce(edac_dev, 0, 0, edac_dev->ctl_name);
-
-	if (err_detect & L2_EDE_UE_MASK)
-		edac_device_handle_ue(edac_dev, 0, 0, edac_dev->ctl_name);
-}
-
-static irqreturn_t mpc85xx_l2_isr(int irq, void *dev_id)
-{
-	struct edac_device_ctl_info *edac_dev = dev_id;
-	struct mpc85xx_l2_pdata *pdata = edac_dev->pvt_info;
-	u32 err_detect;
-
-	err_detect = in_be32(pdata->l2_vbase + MPC85XX_L2_ERRDET);
-
-	if (!(err_detect & L2_EDE_MASK))
-		return IRQ_NONE;
-
-	mpc85xx_l2_check(edac_dev);
-
-	return IRQ_HANDLED;
-}
-
-static int mpc85xx_l2_err_probe(struct platform_device *op)
-{
-	struct edac_device_ctl_info *edac_dev;
-	struct mpc85xx_l2_pdata *pdata;
-	struct resource r;
-	int res;
-
-	if (!devres_open_group(&op->dev, mpc85xx_l2_err_probe, GFP_KERNEL))
-		return -ENOMEM;
-
-	edac_dev = edac_device_alloc_ctl_info(sizeof(*pdata),
-					      "cpu", 1, "L", 1, 2, NULL, 0,
-					      edac_dev_idx);
-	if (!edac_dev) {
-		devres_release_group(&op->dev, mpc85xx_l2_err_probe);
-		return -ENOMEM;
-	}
-
-	pdata = edac_dev->pvt_info;
-	pdata->name = "mpc85xx_l2_err";
-	pdata->irq = NO_IRQ;
-	edac_dev->dev = &op->dev;
-	dev_set_drvdata(edac_dev->dev, edac_dev);
-	edac_dev->ctl_name = pdata->name;
-	edac_dev->dev_name = pdata->name;
-
-	res = of_address_to_resource(op->dev.of_node, 0, &r);
-	if (res) {
-		pr_err("%s: Unable to get resource for "
-		       "L2 err regs\n", __func__);
-		goto err;
-	}
-
-	/* we only need the error registers */
-	r.start += 0xe00;
-
-	if (!devm_request_mem_region(&op->dev, r.start, resource_size(&r),
-				     pdata->name)) {
-		pr_err("%s: Error while requesting mem region\n",
-		       __func__);
-		res = -EBUSY;
-		goto err;
-	}
-
-	pdata->l2_vbase = devm_ioremap(&op->dev, r.start, resource_size(&r));
-	if (!pdata->l2_vbase) {
-		pr_err("%s: Unable to setup L2 err regs\n", __func__);
-		res = -ENOMEM;
-		goto err;
-	}
-
-	out_be32(pdata->l2_vbase + MPC85XX_L2_ERRDET, ~0);
-
-	orig_l2_err_disable = in_be32(pdata->l2_vbase + MPC85XX_L2_ERRDIS);
-
-	/* clear the err_dis */
-	out_be32(pdata->l2_vbase + MPC85XX_L2_ERRDIS, 0);
-
-	edac_dev->mod_name = EDAC_MOD_STR;
-
-	if (edac_op_state == EDAC_OPSTATE_POLL)
-		edac_dev->edac_check = mpc85xx_l2_check;
-
-	mpc85xx_set_l2_sysfs_attributes(edac_dev);
-
-	pdata->edac_idx = edac_dev_idx++;
-
-	if (edac_device_add_device(edac_dev) > 0) {
-		edac_dbg(3, "failed edac_device_add_device()\n");
-		goto err;
-	}
-
-	if (edac_op_state == EDAC_OPSTATE_INT) {
-		pdata->irq = irq_of_parse_and_map(op->dev.of_node, 0);
-		res = devm_request_irq(&op->dev, pdata->irq,
-				       mpc85xx_l2_isr, IRQF_SHARED,
-				       "[EDAC] L2 err", edac_dev);
-		if (res < 0) {
-			pr_err("%s: Unable to request irq %d for "
-			       "MPC85xx L2 err\n", __func__, pdata->irq);
-			irq_dispose_mapping(pdata->irq);
-			res = -ENODEV;
-			goto err2;
-		}
-
-		pr_info(EDAC_MOD_STR " acquired irq %d for L2 Err\n",
-		       pdata->irq);
-
-		edac_dev->op_state = OP_RUNNING_INTERRUPT;
-
-		out_be32(pdata->l2_vbase + MPC85XX_L2_ERRINTEN, L2_EIE_MASK);
-	}
-
-	devres_remove_group(&op->dev, mpc85xx_l2_err_probe);
-
-	edac_dbg(3, "success\n");
-	pr_info(EDAC_MOD_STR " L2 err registered\n");
-
-	return 0;
-
-err2:
-	edac_device_del_device(&op->dev);
-err:
-	devres_release_group(&op->dev, mpc85xx_l2_err_probe);
-	edac_device_free_ctl_info(edac_dev);
-	return res;
-}
-
-static int mpc85xx_l2_err_remove(struct platform_device *op)
-{
-	struct edac_device_ctl_info *edac_dev = dev_get_drvdata(&op->dev);
-	struct mpc85xx_l2_pdata *pdata = edac_dev->pvt_info;
-
-	edac_dbg(0, "\n");
-
-	if (edac_op_state == EDAC_OPSTATE_INT) {
-		out_be32(pdata->l2_vbase + MPC85XX_L2_ERRINTEN, 0);
-		irq_dispose_mapping(pdata->irq);
-	}
-
-	out_be32(pdata->l2_vbase + MPC85XX_L2_ERRDIS, orig_l2_err_disable);
-	edac_device_del_device(&op->dev);
-	edac_device_free_ctl_info(edac_dev);
-	return 0;
-}
-
-static const struct of_device_id mpc85xx_l2_err_of_match[] = {
-/* deprecate the fsl,85.. forms in the future, 2.6.30? */
-	{ .compatible = "fsl,8540-l2-cache-controller", },
-	{ .compatible = "fsl,8541-l2-cache-controller", },
-	{ .compatible = "fsl,8544-l2-cache-controller", },
-	{ .compatible = "fsl,8548-l2-cache-controller", },
-	{ .compatible = "fsl,8555-l2-cache-controller", },
-	{ .compatible = "fsl,8568-l2-cache-controller", },
-	{ .compatible = "fsl,mpc8536-l2-cache-controller", },
-	{ .compatible = "fsl,mpc8540-l2-cache-controller", },
-	{ .compatible = "fsl,mpc8541-l2-cache-controller", },
-	{ .compatible = "fsl,mpc8544-l2-cache-controller", },
-	{ .compatible = "fsl,mpc8548-l2-cache-controller", },
-	{ .compatible = "fsl,mpc8555-l2-cache-controller", },
-	{ .compatible = "fsl,mpc8560-l2-cache-controller", },
-	{ .compatible = "fsl,mpc8568-l2-cache-controller", },
-	{ .compatible = "fsl,mpc8569-l2-cache-controller", },
-	{ .compatible = "fsl,mpc8572-l2-cache-controller", },
-	{ .compatible = "fsl,p1020-l2-cache-controller", },
-	{ .compatible = "fsl,p1021-l2-cache-controller", },
-	{ .compatible = "fsl,p2020-l2-cache-controller", },
-	{},
-};
-MODULE_DEVICE_TABLE(of, mpc85xx_l2_err_of_match);
-
-static struct platform_driver mpc85xx_l2_err_driver = {
-	.probe = mpc85xx_l2_err_probe,
-	.remove = mpc85xx_l2_err_remove,
-	.driver = {
-		.name = "mpc85xx_l2_err",
-		.of_match_table = mpc85xx_l2_err_of_match,
-	},
-};
-
 /**************************** MC Err device ***************************/
 
 /*
@@ -1026,7 +430,7 @@ static void mpc85xx_init_csrows(struct mem_ctl_info *mci)
 	}
 }
 
-static int mpc85xx_mc_err_probe(struct platform_device *op)
+int mpc85xx_mc_err_probe(struct platform_device *op)
 {
 	struct mem_ctl_info *mci;
 	struct edac_mc_layer layers[2];
@@ -1096,7 +500,6 @@ static int mpc85xx_mc_err_probe(struct platform_device *op)
 	mci->edac_ctl_cap = EDAC_FLAG_NONE | EDAC_FLAG_SECDED;
 	mci->edac_cap = EDAC_FLAG_SECDED;
 	mci->mod_name = EDAC_MOD_STR;
-	mci->mod_ver = MPC85XX_REVISION;
 
 	if (edac_op_state == EDAC_OPSTATE_POLL)
 		mci->edac_check = mpc85xx_mc_check;
@@ -1138,8 +541,8 @@ static int mpc85xx_mc_err_probe(struct platform_device *op)
 				       IRQF_SHARED,
 				       "[EDAC] MC err", mci);
 		if (res < 0) {
-			pr_err("%s: Unable to request irq %d for "
-			       "MPC85xx DRAM ERR\n", __func__, pdata->irq);
+			pr_err("%s: Unable to request irq %d for MPC85xx DRAM ERR\n",
+			       __func__, pdata->irq);
 			irq_dispose_mapping(pdata->irq);
 			res = -ENODEV;
 			goto err2;
@@ -1162,8 +565,9 @@ err:
 	edac_mc_free(mci);
 	return res;
 }
+EXPORT_SYMBOL_GPL(mpc85xx_mc_err_probe);
 
-static int mpc85xx_mc_err_remove(struct platform_device *op)
+int mpc85xx_mc_err_remove(struct platform_device *op)
 {
 	struct mem_ctl_info *mci = dev_get_drvdata(&op->dev);
 	struct mpc85xx_mc_pdata *pdata = mci->pvt_info;
@@ -1183,87 +587,4 @@ static int mpc85xx_mc_err_remove(struct platform_device *op)
 	edac_mc_free(mci);
 	return 0;
 }
-
-static const struct of_device_id mpc85xx_mc_err_of_match[] = {
-/* deprecate the fsl,85.. forms in the future, 2.6.30? */
-	{ .compatible = "fsl,8540-memory-controller", },
-	{ .compatible = "fsl,8541-memory-controller", },
-	{ .compatible = "fsl,8544-memory-controller", },
-	{ .compatible = "fsl,8548-memory-controller", },
-	{ .compatible = "fsl,8555-memory-controller", },
-	{ .compatible = "fsl,8568-memory-controller", },
-	{ .compatible = "fsl,mpc8536-memory-controller", },
-	{ .compatible = "fsl,mpc8540-memory-controller", },
-	{ .compatible = "fsl,mpc8541-memory-controller", },
-	{ .compatible = "fsl,mpc8544-memory-controller", },
-	{ .compatible = "fsl,mpc8548-memory-controller", },
-	{ .compatible = "fsl,mpc8555-memory-controller", },
-	{ .compatible = "fsl,mpc8560-memory-controller", },
-	{ .compatible = "fsl,mpc8568-memory-controller", },
-	{ .compatible = "fsl,mpc8569-memory-controller", },
-	{ .compatible = "fsl,mpc8572-memory-controller", },
-	{ .compatible = "fsl,mpc8349-memory-controller", },
-	{ .compatible = "fsl,p1020-memory-controller", },
-	{ .compatible = "fsl,p1021-memory-controller", },
-	{ .compatible = "fsl,p2020-memory-controller", },
-	{ .compatible = "fsl,qoriq-memory-controller", },
-	{},
-};
-MODULE_DEVICE_TABLE(of, mpc85xx_mc_err_of_match);
-
-static struct platform_driver mpc85xx_mc_err_driver = {
-	.probe = mpc85xx_mc_err_probe,
-	.remove = mpc85xx_mc_err_remove,
-	.driver = {
-		.name = "mpc85xx_mc_err",
-		.of_match_table = mpc85xx_mc_err_of_match,
-	},
-};
-
-static struct platform_driver * const drivers[] = {
-	&mpc85xx_mc_err_driver,
-	&mpc85xx_l2_err_driver,
-#ifdef CONFIG_PCI
-	&mpc85xx_pci_err_driver,
-#endif
-};
-
-static int __init mpc85xx_mc_init(void)
-{
-	int res = 0;
-	u32 __maybe_unused pvr = 0;
-
-	pr_info("Freescale(R) MPC85xx EDAC driver, "
-	       "(C) 2006 Montavista Software\n");
-
-	/* make sure error reporting method is sane */
-	switch (edac_op_state) {
-	case EDAC_OPSTATE_POLL:
-	case EDAC_OPSTATE_INT:
-		break;
-	default:
-		edac_op_state = EDAC_OPSTATE_INT;
-		break;
-	}
-
-	res = platform_register_drivers(drivers, ARRAY_SIZE(drivers));
-	if (res)
-		pr_warn(EDAC_MOD_STR "drivers fail to register\n");
-
-	return 0;
-}
-
-module_init(mpc85xx_mc_init);
-
-static void __exit mpc85xx_mc_exit(void)
-{
-	platform_unregister_drivers(drivers, ARRAY_SIZE(drivers));
-}
-
-module_exit(mpc85xx_mc_exit);
-
-MODULE_LICENSE("GPL");
-MODULE_AUTHOR("Montavista Software, Inc.");
-module_param(edac_op_state, int, 0444);
-MODULE_PARM_DESC(edac_op_state,
-		 "EDAC Error Reporting state: 0=Poll, 2=Interrupt");
+EXPORT_SYMBOL_GPL(mpc85xx_mc_err_remove);
diff --git a/drivers/edac/mpc85xx_edac.h b/drivers/edac/fsl_ddr_edac.h
similarity index 43%
copy from drivers/edac/mpc85xx_edac.h
copy to drivers/edac/fsl_ddr_edac.h
index 9352e88..94c6907 100644
--- a/drivers/edac/mpc85xx_edac.h
+++ b/drivers/edac/fsl_ddr_edac.h
@@ -1,5 +1,6 @@
 /*
- * Freescale MPC85xx Memory Controller kernel module
+ * Freescale Memory Controller kernel module
+ *
  * Author: Dave Jiang <djiang@mvista.com>
  *
  * 2006-2007 (c) MontaVista Software, Inc. This file is licensed under
@@ -8,17 +9,11 @@
  * or implied.
  *
  */
-#ifndef _MPC85XX_EDAC_H_
-#define _MPC85XX_EDAC_H_
-
-#define MPC85XX_REVISION " Ver: 2.0.0"
-#define EDAC_MOD_STR	"MPC85xx_edac"
-
-#define mpc85xx_printk(level, fmt, arg...) \
-	edac_printk(level, "MPC85xx", fmt, ##arg)
+#ifndef _FSL_DDR_EDAC_H_
+#define _FSL_DDR_EDAC_H_
 
 #define mpc85xx_mc_printk(mci, level, fmt, arg...) \
-	edac_mc_chipset_printk(mci, level, "MPC85xx", fmt, ##arg)
+	edac_mc_chipset_printk(mci, level, "FSL_DDR", fmt, ##arg)
 
 /*
  * DRAM error defines
@@ -76,99 +71,12 @@
 #define	DDR_EDI_SBED	0x4	/* single-bit ECC error disable */
 #define	DDR_EDI_MBED	0x8	/* multi-bit ECC error disable */
 
-/*
- * L2 Err defines
- */
-#define MPC85XX_L2_ERRINJHI	0x0000
-#define MPC85XX_L2_ERRINJLO	0x0004
-#define MPC85XX_L2_ERRINJCTL	0x0008
-#define MPC85XX_L2_CAPTDATAHI	0x0020
-#define MPC85XX_L2_CAPTDATALO	0x0024
-#define MPC85XX_L2_CAPTECC	0x0028
-#define MPC85XX_L2_ERRDET	0x0040
-#define MPC85XX_L2_ERRDIS	0x0044
-#define MPC85XX_L2_ERRINTEN	0x0048
-#define MPC85XX_L2_ERRATTR	0x004c
-#define MPC85XX_L2_ERRADDR	0x0050
-#define MPC85XX_L2_ERRCTL	0x0058
-
-/* Error Interrupt Enable */
-#define L2_EIE_L2CFGINTEN	0x1
-#define L2_EIE_SBECCINTEN	0x4
-#define L2_EIE_MBECCINTEN	0x8
-#define L2_EIE_TPARINTEN	0x10
-#define L2_EIE_MASK	(L2_EIE_L2CFGINTEN | L2_EIE_SBECCINTEN | \
-			L2_EIE_MBECCINTEN | L2_EIE_TPARINTEN)
-
-/* Error Detect */
-#define L2_EDE_L2CFGERR		0x1
-#define L2_EDE_SBECCERR		0x4
-#define L2_EDE_MBECCERR		0x8
-#define L2_EDE_TPARERR		0x10
-#define L2_EDE_MULL2ERR		0x80000000
-
-#define L2_EDE_CE_MASK	L2_EDE_SBECCERR
-#define L2_EDE_UE_MASK	(L2_EDE_L2CFGERR | L2_EDE_MBECCERR | \
-			L2_EDE_TPARERR)
-#define L2_EDE_MASK	(L2_EDE_L2CFGERR | L2_EDE_SBECCERR | \
-			L2_EDE_MBECCERR | L2_EDE_TPARERR | L2_EDE_MULL2ERR)
-
-/*
- * PCI Err defines
- */
-#define PCI_EDE_TOE			0x00000001
-#define PCI_EDE_SCM			0x00000002
-#define PCI_EDE_IRMSV			0x00000004
-#define PCI_EDE_ORMSV			0x00000008
-#define PCI_EDE_OWMSV			0x00000010
-#define PCI_EDE_TGT_ABRT		0x00000020
-#define PCI_EDE_MST_ABRT		0x00000040
-#define PCI_EDE_TGT_PERR		0x00000080
-#define PCI_EDE_MST_PERR		0x00000100
-#define PCI_EDE_RCVD_SERR		0x00000200
-#define PCI_EDE_ADDR_PERR		0x00000400
-#define PCI_EDE_MULTI_ERR		0x80000000
-
-#define PCI_EDE_PERR_MASK	(PCI_EDE_TGT_PERR | PCI_EDE_MST_PERR | \
-				PCI_EDE_ADDR_PERR)
-
-#define MPC85XX_PCI_ERR_DR		0x0000
-#define MPC85XX_PCI_ERR_CAP_DR		0x0004
-#define MPC85XX_PCI_ERR_EN		0x0008
-#define   PEX_ERR_ICCAIE_EN_BIT		0x00020000
-#define MPC85XX_PCI_ERR_ATTRIB		0x000c
-#define MPC85XX_PCI_ERR_ADDR		0x0010
-#define   PEX_ERR_ICCAD_DISR_BIT	0x00020000
-#define MPC85XX_PCI_ERR_EXT_ADDR	0x0014
-#define MPC85XX_PCI_ERR_DL		0x0018
-#define MPC85XX_PCI_ERR_DH		0x001c
-#define MPC85XX_PCI_GAS_TIMR		0x0020
-#define MPC85XX_PCI_PCIX_TIMR		0x0024
-#define MPC85XX_PCIE_ERR_CAP_R0		0x0028
-#define MPC85XX_PCIE_ERR_CAP_R1		0x002c
-#define MPC85XX_PCIE_ERR_CAP_R2		0x0030
-#define MPC85XX_PCIE_ERR_CAP_R3		0x0034
-
 struct mpc85xx_mc_pdata {
 	char *name;
 	int edac_idx;
 	void __iomem *mc_vbase;
 	int irq;
 };
-
-struct mpc85xx_l2_pdata {
-	char *name;
-	int edac_idx;
-	void __iomem *l2_vbase;
-	int irq;
-};
-
-struct mpc85xx_pci_pdata {
-	char *name;
-	bool is_pcie;
-	int edac_idx;
-	void __iomem *pci_vbase;
-	int irq;
-};
-
+int mpc85xx_mc_err_probe(struct platform_device *op);
+int mpc85xx_mc_err_remove(struct platform_device *op);
 #endif
diff --git a/drivers/edac/mpc85xx_edac.c b/drivers/edac/mpc85xx_edac.c
index c0b0951..af9ce77 100644
--- a/drivers/edac/mpc85xx_edac.c
+++ b/drivers/edac/mpc85xx_edac.c
@@ -27,15 +27,12 @@
 #include "edac_module.h"
 #include "edac_core.h"
 #include "mpc85xx_edac.h"
+#include "fsl_ddr_edac.h"
 
 static int edac_dev_idx;
 #ifdef CONFIG_PCI
 static int edac_pci_idx;
 #endif
-static int edac_mc_idx;
-
-static u32 orig_ddr_err_disable;
-static u32 orig_ddr_err_sbe;
 
 /*
  * PCI Err defines
@@ -47,100 +44,6 @@ static u32 orig_pci_err_en;
 
 static u32 orig_l2_err_disable;
 
-/************************ MC SYSFS parts ***********************************/
-
-#define to_mci(k) container_of(k, struct mem_ctl_info, dev)
-
-static ssize_t mpc85xx_mc_inject_data_hi_show(struct device *dev,
-					      struct device_attribute *mattr,
-					      char *data)
-{
-	struct mem_ctl_info *mci = to_mci(dev);
-	struct mpc85xx_mc_pdata *pdata = mci->pvt_info;
-	return sprintf(data, "0x%08x",
-		       in_be32(pdata->mc_vbase +
-			       MPC85XX_MC_DATA_ERR_INJECT_HI));
-}
-
-static ssize_t mpc85xx_mc_inject_data_lo_show(struct device *dev,
-					      struct device_attribute *mattr,
-					      char *data)
-{
-	struct mem_ctl_info *mci = to_mci(dev);
-	struct mpc85xx_mc_pdata *pdata = mci->pvt_info;
-	return sprintf(data, "0x%08x",
-		       in_be32(pdata->mc_vbase +
-			       MPC85XX_MC_DATA_ERR_INJECT_LO));
-}
-
-static ssize_t mpc85xx_mc_inject_ctrl_show(struct device *dev,
-					   struct device_attribute *mattr,
-					   char *data)
-{
-	struct mem_ctl_info *mci = to_mci(dev);
-	struct mpc85xx_mc_pdata *pdata = mci->pvt_info;
-	return sprintf(data, "0x%08x",
-		       in_be32(pdata->mc_vbase + MPC85XX_MC_ECC_ERR_INJECT));
-}
-
-static ssize_t mpc85xx_mc_inject_data_hi_store(struct device *dev,
-					       struct device_attribute *mattr,
-					       const char *data, size_t count)
-{
-	struct mem_ctl_info *mci = to_mci(dev);
-	struct mpc85xx_mc_pdata *pdata = mci->pvt_info;
-	if (isdigit(*data)) {
-		out_be32(pdata->mc_vbase + MPC85XX_MC_DATA_ERR_INJECT_HI,
-			 simple_strtoul(data, NULL, 0));
-		return count;
-	}
-	return 0;
-}
-
-static ssize_t mpc85xx_mc_inject_data_lo_store(struct device *dev,
-					       struct device_attribute *mattr,
-					       const char *data, size_t count)
-{
-	struct mem_ctl_info *mci = to_mci(dev);
-	struct mpc85xx_mc_pdata *pdata = mci->pvt_info;
-	if (isdigit(*data)) {
-		out_be32(pdata->mc_vbase + MPC85XX_MC_DATA_ERR_INJECT_LO,
-			 simple_strtoul(data, NULL, 0));
-		return count;
-	}
-	return 0;
-}
-
-static ssize_t mpc85xx_mc_inject_ctrl_store(struct device *dev,
-					       struct device_attribute *mattr,
-					       const char *data, size_t count)
-{
-	struct mem_ctl_info *mci = to_mci(dev);
-	struct mpc85xx_mc_pdata *pdata = mci->pvt_info;
-	if (isdigit(*data)) {
-		out_be32(pdata->mc_vbase + MPC85XX_MC_ECC_ERR_INJECT,
-			 simple_strtoul(data, NULL, 0));
-		return count;
-	}
-	return 0;
-}
-
-DEVICE_ATTR(inject_data_hi, S_IRUGO | S_IWUSR,
-	    mpc85xx_mc_inject_data_hi_show, mpc85xx_mc_inject_data_hi_store);
-DEVICE_ATTR(inject_data_lo, S_IRUGO | S_IWUSR,
-	    mpc85xx_mc_inject_data_lo_show, mpc85xx_mc_inject_data_lo_store);
-DEVICE_ATTR(inject_ctrl, S_IRUGO | S_IWUSR,
-	    mpc85xx_mc_inject_ctrl_show, mpc85xx_mc_inject_ctrl_store);
-
-static struct attribute *mpc85xx_dev_attrs[] = {
-	&dev_attr_inject_data_hi.attr,
-	&dev_attr_inject_data_lo.attr,
-	&dev_attr_inject_ctrl.attr,
-	NULL
-};
-
-ATTRIBUTE_GROUPS(mpc85xx_dev);
-
 /**************************** PCI Err device ***************************/
 #ifdef CONFIG_PCI
 
@@ -724,466 +627,6 @@ static struct platform_driver mpc85xx_l2_err_driver = {
 	},
 };
 
-/**************************** MC Err device ***************************/
-
-/*
- * Taken from table 8-55 in the MPC8641 User's Manual and/or 9-61 in the
- * MPC8572 User's Manual.  Each line represents a syndrome bit column as a
- * 64-bit value, but split into an upper and lower 32-bit chunk.  The labels
- * below correspond to Freescale's manuals.
- */
-static unsigned int ecc_table[16] = {
-	/* MSB           LSB */
-	/* [0:31]    [32:63] */
-	0xf00fe11e, 0xc33c0ff7,	/* Syndrome bit 7 */
-	0x00ff00ff, 0x00fff0ff,
-	0x0f0f0f0f, 0x0f0fff00,
-	0x11113333, 0x7777000f,
-	0x22224444, 0x8888222f,
-	0x44448888, 0xffff4441,
-	0x8888ffff, 0x11118882,
-	0xffff1111, 0x22221114,	/* Syndrome bit 0 */
-};
-
-/*
- * Calculate the correct ECC value for a 64-bit value specified by high:low
- */
-static u8 calculate_ecc(u32 high, u32 low)
-{
-	u32 mask_low;
-	u32 mask_high;
-	int bit_cnt;
-	u8 ecc = 0;
-	int i;
-	int j;
-
-	for (i = 0; i < 8; i++) {
-		mask_high = ecc_table[i * 2];
-		mask_low = ecc_table[i * 2 + 1];
-		bit_cnt = 0;
-
-		for (j = 0; j < 32; j++) {
-			if ((mask_high >> j) & 1)
-				bit_cnt ^= (high >> j) & 1;
-			if ((mask_low >> j) & 1)
-				bit_cnt ^= (low >> j) & 1;
-		}
-
-		ecc |= bit_cnt << i;
-	}
-
-	return ecc;
-}
-
-/*
- * Create the syndrome code which is generated if the data line specified by
- * 'bit' failed.  Eg generate an 8-bit codes seen in Table 8-55 in the MPC8641
- * User's Manual and 9-61 in the MPC8572 User's Manual.
- */
-static u8 syndrome_from_bit(unsigned int bit) {
-	int i;
-	u8 syndrome = 0;
-
-	/*
-	 * Cycle through the upper or lower 32-bit portion of each value in
-	 * ecc_table depending on if 'bit' is in the upper or lower half of
-	 * 64-bit data.
-	 */
-	for (i = bit < 32; i < 16; i += 2)
-		syndrome |= ((ecc_table[i] >> (bit % 32)) & 1) << (i / 2);
-
-	return syndrome;
-}
-
-/*
- * Decode data and ecc syndrome to determine what went wrong
- * Note: This can only decode single-bit errors
- */
-static void sbe_ecc_decode(u32 cap_high, u32 cap_low, u32 cap_ecc,
-		       int *bad_data_bit, int *bad_ecc_bit)
-{
-	int i;
-	u8 syndrome;
-
-	*bad_data_bit = -1;
-	*bad_ecc_bit = -1;
-
-	/*
-	 * Calculate the ECC of the captured data and XOR it with the captured
-	 * ECC to find an ECC syndrome value we can search for
-	 */
-	syndrome = calculate_ecc(cap_high, cap_low) ^ cap_ecc;
-
-	/* Check if a data line is stuck... */
-	for (i = 0; i < 64; i++) {
-		if (syndrome == syndrome_from_bit(i)) {
-			*bad_data_bit = i;
-			return;
-		}
-	}
-
-	/* If data is correct, check ECC bits for errors... */
-	for (i = 0; i < 8; i++) {
-		if ((syndrome >> i) & 0x1) {
-			*bad_ecc_bit = i;
-			return;
-		}
-	}
-}
-
-#define make64(high, low) (((u64)(high) << 32) | (low))
-
-static void mpc85xx_mc_check(struct mem_ctl_info *mci)
-{
-	struct mpc85xx_mc_pdata *pdata = mci->pvt_info;
-	struct csrow_info *csrow;
-	u32 bus_width;
-	u32 err_detect;
-	u32 syndrome;
-	u64 err_addr;
-	u32 pfn;
-	int row_index;
-	u32 cap_high;
-	u32 cap_low;
-	int bad_data_bit;
-	int bad_ecc_bit;
-
-	err_detect = in_be32(pdata->mc_vbase + MPC85XX_MC_ERR_DETECT);
-	if (!err_detect)
-		return;
-
-	mpc85xx_mc_printk(mci, KERN_ERR, "Err Detect Register: %#8.8x\n",
-			  err_detect);
-
-	/* no more processing if not ECC bit errors */
-	if (!(err_detect & (DDR_EDE_SBE | DDR_EDE_MBE))) {
-		out_be32(pdata->mc_vbase + MPC85XX_MC_ERR_DETECT, err_detect);
-		return;
-	}
-
-	syndrome = in_be32(pdata->mc_vbase + MPC85XX_MC_CAPTURE_ECC);
-
-	/* Mask off appropriate bits of syndrome based on bus width */
-	bus_width = (in_be32(pdata->mc_vbase + MPC85XX_MC_DDR_SDRAM_CFG) &
-			DSC_DBW_MASK) ? 32 : 64;
-	if (bus_width == 64)
-		syndrome &= 0xff;
-	else
-		syndrome &= 0xffff;
-
-	err_addr = make64(
-		in_be32(pdata->mc_vbase + MPC85XX_MC_CAPTURE_EXT_ADDRESS),
-		in_be32(pdata->mc_vbase + MPC85XX_MC_CAPTURE_ADDRESS));
-	pfn = err_addr >> PAGE_SHIFT;
-
-	for (row_index = 0; row_index < mci->nr_csrows; row_index++) {
-		csrow = mci->csrows[row_index];
-		if ((pfn >= csrow->first_page) && (pfn <= csrow->last_page))
-			break;
-	}
-
-	cap_high = in_be32(pdata->mc_vbase + MPC85XX_MC_CAPTURE_DATA_HI);
-	cap_low = in_be32(pdata->mc_vbase + MPC85XX_MC_CAPTURE_DATA_LO);
-
-	/*
-	 * Analyze single-bit errors on 64-bit wide buses
-	 * TODO: Add support for 32-bit wide buses
-	 */
-	if ((err_detect & DDR_EDE_SBE) && (bus_width == 64)) {
-		sbe_ecc_decode(cap_high, cap_low, syndrome,
-				&bad_data_bit, &bad_ecc_bit);
-
-		if (bad_data_bit != -1)
-			mpc85xx_mc_printk(mci, KERN_ERR,
-				"Faulty Data bit: %d\n", bad_data_bit);
-		if (bad_ecc_bit != -1)
-			mpc85xx_mc_printk(mci, KERN_ERR,
-				"Faulty ECC bit: %d\n", bad_ecc_bit);
-
-		mpc85xx_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));
-	}
-
-	mpc85xx_mc_printk(mci, KERN_ERR,
-			"Captured Data / ECC:\t%#8.8x_%08x / %#2.2x\n",
-			cap_high, cap_low, syndrome);
-	mpc85xx_mc_printk(mci, KERN_ERR, "Err addr: %#8.8llx\n", err_addr);
-	mpc85xx_mc_printk(mci, KERN_ERR, "PFN: %#8.8x\n", pfn);
-
-	/* we are out of range */
-	if (row_index == mci->nr_csrows)
-		mpc85xx_mc_printk(mci, KERN_ERR, "PFN out of range!\n");
-
-	if (err_detect & DDR_EDE_SBE)
-		edac_mc_handle_error(HW_EVENT_ERR_CORRECTED, mci, 1,
-				     pfn, err_addr & ~PAGE_MASK, syndrome,
-				     row_index, 0, -1,
-				     mci->ctl_name, "");
-
-	if (err_detect & DDR_EDE_MBE)
-		edac_mc_handle_error(HW_EVENT_ERR_UNCORRECTED, mci, 1,
-				     pfn, err_addr & ~PAGE_MASK, syndrome,
-				     row_index, 0, -1,
-				     mci->ctl_name, "");
-
-	out_be32(pdata->mc_vbase + MPC85XX_MC_ERR_DETECT, err_detect);
-}
-
-static irqreturn_t mpc85xx_mc_isr(int irq, void *dev_id)
-{
-	struct mem_ctl_info *mci = dev_id;
-	struct mpc85xx_mc_pdata *pdata = mci->pvt_info;
-	u32 err_detect;
-
-	err_detect = in_be32(pdata->mc_vbase + MPC85XX_MC_ERR_DETECT);
-	if (!err_detect)
-		return IRQ_NONE;
-
-	mpc85xx_mc_check(mci);
-
-	return IRQ_HANDLED;
-}
-
-static void mpc85xx_init_csrows(struct mem_ctl_info *mci)
-{
-	struct mpc85xx_mc_pdata *pdata = mci->pvt_info;
-	struct csrow_info *csrow;
-	struct dimm_info *dimm;
-	u32 sdram_ctl;
-	u32 sdtype;
-	enum mem_type mtype;
-	u32 cs_bnds;
-	int index;
-
-	sdram_ctl = in_be32(pdata->mc_vbase + MPC85XX_MC_DDR_SDRAM_CFG);
-
-	sdtype = sdram_ctl & DSC_SDTYPE_MASK;
-	if (sdram_ctl & DSC_RD_EN) {
-		switch (sdtype) {
-		case DSC_SDTYPE_DDR:
-			mtype = MEM_RDDR;
-			break;
-		case DSC_SDTYPE_DDR2:
-			mtype = MEM_RDDR2;
-			break;
-		case DSC_SDTYPE_DDR3:
-			mtype = MEM_RDDR3;
-			break;
-		default:
-			mtype = MEM_UNKNOWN;
-			break;
-		}
-	} else {
-		switch (sdtype) {
-		case DSC_SDTYPE_DDR:
-			mtype = MEM_DDR;
-			break;
-		case DSC_SDTYPE_DDR2:
-			mtype = MEM_DDR2;
-			break;
-		case DSC_SDTYPE_DDR3:
-			mtype = MEM_DDR3;
-			break;
-		default:
-			mtype = MEM_UNKNOWN;
-			break;
-		}
-	}
-
-	for (index = 0; index < mci->nr_csrows; index++) {
-		u32 start;
-		u32 end;
-
-		csrow = mci->csrows[index];
-		dimm = csrow->channels[0]->dimm;
-
-		cs_bnds = in_be32(pdata->mc_vbase + MPC85XX_MC_CS_BNDS_0 +
-				  (index * MPC85XX_MC_CS_BNDS_OFS));
-
-		start = (cs_bnds & 0xffff0000) >> 16;
-		end   = (cs_bnds & 0x0000ffff);
-
-		if (start == end)
-			continue;	/* not populated */
-
-		start <<= (24 - PAGE_SHIFT);
-		end   <<= (24 - PAGE_SHIFT);
-		end    |= (1 << (24 - PAGE_SHIFT)) - 1;
-
-		csrow->first_page = start;
-		csrow->last_page = end;
-
-		dimm->nr_pages = end + 1 - start;
-		dimm->grain = 8;
-		dimm->mtype = mtype;
-		dimm->dtype = DEV_UNKNOWN;
-		if (sdram_ctl & DSC_X32_EN)
-			dimm->dtype = DEV_X32;
-		dimm->edac_mode = EDAC_SECDED;
-	}
-}
-
-static int mpc85xx_mc_err_probe(struct platform_device *op)
-{
-	struct mem_ctl_info *mci;
-	struct edac_mc_layer layers[2];
-	struct mpc85xx_mc_pdata *pdata;
-	struct resource r;
-	u32 sdram_ctl;
-	int res;
-
-	if (!devres_open_group(&op->dev, mpc85xx_mc_err_probe, GFP_KERNEL))
-		return -ENOMEM;
-
-	layers[0].type = EDAC_MC_LAYER_CHIP_SELECT;
-	layers[0].size = 4;
-	layers[0].is_virt_csrow = true;
-	layers[1].type = EDAC_MC_LAYER_CHANNEL;
-	layers[1].size = 1;
-	layers[1].is_virt_csrow = false;
-	mci = edac_mc_alloc(edac_mc_idx, ARRAY_SIZE(layers), layers,
-			    sizeof(*pdata));
-	if (!mci) {
-		devres_release_group(&op->dev, mpc85xx_mc_err_probe);
-		return -ENOMEM;
-	}
-
-	pdata = mci->pvt_info;
-	pdata->name = "mpc85xx_mc_err";
-	pdata->irq = NO_IRQ;
-	mci->pdev = &op->dev;
-	pdata->edac_idx = edac_mc_idx++;
-	dev_set_drvdata(mci->pdev, mci);
-	mci->ctl_name = pdata->name;
-	mci->dev_name = pdata->name;
-
-	res = of_address_to_resource(op->dev.of_node, 0, &r);
-	if (res) {
-		pr_err("%s: Unable to get resource for MC err regs\n",
-		       __func__);
-		goto err;
-	}
-
-	if (!devm_request_mem_region(&op->dev, r.start, resource_size(&r),
-				     pdata->name)) {
-		pr_err("%s: Error while requesting mem region\n",
-		       __func__);
-		res = -EBUSY;
-		goto err;
-	}
-
-	pdata->mc_vbase = devm_ioremap(&op->dev, r.start, resource_size(&r));
-	if (!pdata->mc_vbase) {
-		pr_err("%s: Unable to setup MC err regs\n", __func__);
-		res = -ENOMEM;
-		goto err;
-	}
-
-	sdram_ctl = in_be32(pdata->mc_vbase + MPC85XX_MC_DDR_SDRAM_CFG);
-	if (!(sdram_ctl & DSC_ECC_EN)) {
-		/* no ECC */
-		pr_warn("%s: No ECC DIMMs discovered\n", __func__);
-		res = -ENODEV;
-		goto err;
-	}
-
-	edac_dbg(3, "init mci\n");
-	mci->mtype_cap = MEM_FLAG_RDDR | MEM_FLAG_RDDR2 |
-	    MEM_FLAG_DDR | MEM_FLAG_DDR2;
-	mci->edac_ctl_cap = EDAC_FLAG_NONE | EDAC_FLAG_SECDED;
-	mci->edac_cap = EDAC_FLAG_SECDED;
-	mci->mod_name = EDAC_MOD_STR;
-	mci->mod_ver = MPC85XX_REVISION;
-
-	if (edac_op_state == EDAC_OPSTATE_POLL)
-		mci->edac_check = mpc85xx_mc_check;
-
-	mci->ctl_page_to_phys = NULL;
-
-	mci->scrub_mode = SCRUB_SW_SRC;
-
-	mpc85xx_init_csrows(mci);
-
-	/* store the original error disable bits */
-	orig_ddr_err_disable =
-	    in_be32(pdata->mc_vbase + MPC85XX_MC_ERR_DISABLE);
-	out_be32(pdata->mc_vbase + MPC85XX_MC_ERR_DISABLE, 0);
-
-	/* clear all error bits */
-	out_be32(pdata->mc_vbase + MPC85XX_MC_ERR_DETECT, ~0);
-
-	if (edac_mc_add_mc_with_groups(mci, mpc85xx_dev_groups)) {
-		edac_dbg(3, "failed edac_mc_add_mc()\n");
-		goto err;
-	}
-
-	if (edac_op_state == EDAC_OPSTATE_INT) {
-		out_be32(pdata->mc_vbase + MPC85XX_MC_ERR_INT_EN,
-			 DDR_EIE_MBEE | DDR_EIE_SBEE);
-
-		/* store the original error management threshold */
-		orig_ddr_err_sbe = in_be32(pdata->mc_vbase +
-					   MPC85XX_MC_ERR_SBE) & 0xff0000;
-
-		/* set threshold to 1 error per interrupt */
-		out_be32(pdata->mc_vbase + MPC85XX_MC_ERR_SBE, 0x10000);
-
-		/* register interrupts */
-		pdata->irq = irq_of_parse_and_map(op->dev.of_node, 0);
-		res = devm_request_irq(&op->dev, pdata->irq,
-				       mpc85xx_mc_isr,
-				       IRQF_SHARED,
-				       "[EDAC] MC err", mci);
-		if (res < 0) {
-			pr_err("%s: Unable to request irq %d for "
-			       "MPC85xx DRAM ERR\n", __func__, pdata->irq);
-			irq_dispose_mapping(pdata->irq);
-			res = -ENODEV;
-			goto err2;
-		}
-
-		pr_info(EDAC_MOD_STR " acquired irq %d for MC\n",
-		       pdata->irq);
-	}
-
-	devres_remove_group(&op->dev, mpc85xx_mc_err_probe);
-	edac_dbg(3, "success\n");
-	pr_info(EDAC_MOD_STR " MC err registered\n");
-
-	return 0;
-
-err2:
-	edac_mc_del_mc(&op->dev);
-err:
-	devres_release_group(&op->dev, mpc85xx_mc_err_probe);
-	edac_mc_free(mci);
-	return res;
-}
-
-static int mpc85xx_mc_err_remove(struct platform_device *op)
-{
-	struct mem_ctl_info *mci = dev_get_drvdata(&op->dev);
-	struct mpc85xx_mc_pdata *pdata = mci->pvt_info;
-
-	edac_dbg(0, "\n");
-
-	if (edac_op_state == EDAC_OPSTATE_INT) {
-		out_be32(pdata->mc_vbase + MPC85XX_MC_ERR_INT_EN, 0);
-		irq_dispose_mapping(pdata->irq);
-	}
-
-	out_be32(pdata->mc_vbase + MPC85XX_MC_ERR_DISABLE,
-		 orig_ddr_err_disable);
-	out_be32(pdata->mc_vbase + MPC85XX_MC_ERR_SBE, orig_ddr_err_sbe);
-
-	edac_mc_del_mc(&op->dev);
-	edac_mc_free(mci);
-	return 0;
-}
-
 static const struct of_device_id mpc85xx_mc_err_of_match[] = {
 /* deprecate the fsl,85.. forms in the future, 2.6.30? */
 	{ .compatible = "fsl,8540-memory-controller", },
diff --git a/drivers/edac/mpc85xx_edac.h b/drivers/edac/mpc85xx_edac.h
index 9352e88..3f6fb16 100644
--- a/drivers/edac/mpc85xx_edac.h
+++ b/drivers/edac/mpc85xx_edac.h
@@ -17,65 +17,6 @@
 #define mpc85xx_printk(level, fmt, arg...) \
 	edac_printk(level, "MPC85xx", fmt, ##arg)
 
-#define mpc85xx_mc_printk(mci, level, fmt, arg...) \
-	edac_mc_chipset_printk(mci, level, "MPC85xx", fmt, ##arg)
-
-/*
- * DRAM error defines
- */
-
-/* DDR_SDRAM_CFG */
-#define MPC85XX_MC_DDR_SDRAM_CFG	0x0110
-#define MPC85XX_MC_CS_BNDS_0		0x0000
-#define MPC85XX_MC_CS_BNDS_1		0x0008
-#define MPC85XX_MC_CS_BNDS_2		0x0010
-#define MPC85XX_MC_CS_BNDS_3		0x0018
-#define MPC85XX_MC_CS_BNDS_OFS		0x0008
-
-#define MPC85XX_MC_DATA_ERR_INJECT_HI	0x0e00
-#define MPC85XX_MC_DATA_ERR_INJECT_LO	0x0e04
-#define MPC85XX_MC_ECC_ERR_INJECT	0x0e08
-#define MPC85XX_MC_CAPTURE_DATA_HI	0x0e20
-#define MPC85XX_MC_CAPTURE_DATA_LO	0x0e24
-#define MPC85XX_MC_CAPTURE_ECC		0x0e28
-#define MPC85XX_MC_ERR_DETECT		0x0e40
-#define MPC85XX_MC_ERR_DISABLE		0x0e44
-#define MPC85XX_MC_ERR_INT_EN		0x0e48
-#define MPC85XX_MC_CAPTURE_ATRIBUTES	0x0e4c
-#define MPC85XX_MC_CAPTURE_ADDRESS	0x0e50
-#define MPC85XX_MC_CAPTURE_EXT_ADDRESS	0x0e54
-#define MPC85XX_MC_ERR_SBE		0x0e58
-
-#define DSC_MEM_EN	0x80000000
-#define DSC_ECC_EN	0x20000000
-#define DSC_RD_EN	0x10000000
-#define DSC_DBW_MASK	0x00180000
-#define DSC_DBW_32	0x00080000
-#define DSC_DBW_64	0x00000000
-
-#define DSC_SDTYPE_MASK		0x07000000
-
-#define DSC_SDTYPE_DDR		0x02000000
-#define DSC_SDTYPE_DDR2		0x03000000
-#define DSC_SDTYPE_DDR3		0x07000000
-#define DSC_X32_EN	0x00000020
-
-/* Err_Int_En */
-#define DDR_EIE_MSEE	0x1	/* memory select */
-#define DDR_EIE_SBEE	0x4	/* single-bit ECC error */
-#define DDR_EIE_MBEE	0x8	/* multi-bit ECC error */
-
-/* Err_Detect */
-#define DDR_EDE_MSE		0x1	/* memory select */
-#define DDR_EDE_SBE		0x4	/* single-bit ECC error */
-#define DDR_EDE_MBE		0x8	/* multi-bit ECC error */
-#define DDR_EDE_MME		0x80000000	/* multiple memory errors */
-
-/* Err_Disable */
-#define DDR_EDI_MSED	0x1	/* memory select disable */
-#define	DDR_EDI_SBED	0x4	/* single-bit ECC error disable */
-#define	DDR_EDI_MBED	0x8	/* multi-bit ECC error disable */
-
 /*
  * L2 Err defines
  */
@@ -149,13 +90,6 @@
 #define MPC85XX_PCIE_ERR_CAP_R2		0x0030
 #define MPC85XX_PCIE_ERR_CAP_R3		0x0034
 
-struct mpc85xx_mc_pdata {
-	char *name;
-	int edac_idx;
-	void __iomem *mc_vbase;
-	int irq;
-};
-
 struct mpc85xx_l2_pdata {
 	char *name;
 	int edac_idx;
-- 
2.7.4

Re: [Patch v3 05/11] driver/edac/fsl-ddr: Separate FSL DDR EDAC driver from MPC85xx

[Borislav Petkov]

On Thu, Aug 04, 2016 at 03:58:30PM -0700, York Sun wrote:
> The mpc85xx compatible DDR controllers are used on ARM-based SoCs.
> Separate the DDR part from mpc85xx EDAC driver and prepare to support
> both architecture.
> 
> Signed-off-by: York Sun <york.sun@nxp.com>
> 
> ---
> Change log
>   v3: Fix compiling errors and warnings caused by patch ordering
>   v2: Reordered patch
>       Separate FSL DDR commont code as shared object, not another driver
>       This patch is generated with "git format-patch -M40 -C40" to show
>       copy-and-delete.
> 
>  drivers/edac/Makefile                           |   5 +-
>  drivers/edac/{mpc85xx_edac.c => fsl_ddr_edac.c} | 699 +-----------------------
>  drivers/edac/{mpc85xx_edac.h => fsl_ddr_edac.h} | 106 +---
>  drivers/edac/mpc85xx_edac.c                     | 559 +------------------
>  drivers/edac/mpc85xx_edac.h                     |  66 ---
>  5 files changed, 22 insertions(+), 1413 deletions(-)
>  copy drivers/edac/{mpc85xx_edac.c => fsl_ddr_edac.c} (43%)
>  copy drivers/edac/{mpc85xx_edac.h => fsl_ddr_edac.h} (43%)
> 
> diff --git a/drivers/edac/Makefile b/drivers/edac/Makefile
> index f9e4a3e..ee047a4 100644
> --- a/drivers/edac/Makefile
> +++ b/drivers/edac/Makefile
> @@ -50,7 +50,10 @@ amd64_edac_mod-$(CONFIG_EDAC_AMD64_ERROR_INJECTION) += amd64_edac_inj.o
>  obj-$(CONFIG_EDAC_AMD64)		+= amd64_edac_mod.o
>  
>  obj-$(CONFIG_EDAC_PASEMI)		+= pasemi_edac.o
> -obj-$(CONFIG_EDAC_MPC85XX)		+= mpc85xx_edac.o
> +
> +mpc85xx_edac_mod-y			:= fsl_ddr_edac.o mpc85xx_edac.o
> +obj-$(CONFIG_EDAC_MPC85XX)		+= mpc85xx_edac_mod.o
> +
>  obj-$(CONFIG_EDAC_MV64X60)		+= mv64x60_edac.o
>  obj-$(CONFIG_EDAC_CELL)			+= cell_edac.o
>  obj-$(CONFIG_EDAC_PPC4XX)		+= ppc4xx_edac.o
> diff --git a/drivers/edac/mpc85xx_edac.c b/drivers/edac/fsl_ddr_edac.c
> similarity index 43%
> copy from drivers/edac/mpc85xx_edac.c
> copy to drivers/edac/fsl_ddr_edac.c
> index c0b0951..280797e 100644
> --- a/drivers/edac/mpc85xx_edac.c
> +++ b/drivers/edac/fsl_ddr_edac.c
> @@ -1,5 +1,5 @@
>  /*
> - * Freescale MPC85xx Memory Controller kernel module
> + * Freescale Memory Controller kernel modul

This should say something like "Layerscape SoC blabla... Originally
split out from mpc85xx_edac EDAC driver."

Ditto for the header below.

>   *
>   * Parts Copyrighted (c) 2013 by Freescale Semiconductor, Inc.
>   *
> @@ -20,33 +20,20 @@
>  #include <linux/edac.h>
>  #include <linux/smp.h>
>  #include <linux/gfp.h>
> -#include <linux/fsl/edac.h>
>  
>  #include <linux/of_platform.h>
>  #include <linux/of_device.h>
>  #include "edac_module.h"
>  #include "edac_core.h"
> -#include "mpc85xx_edac.h"
> +#include "fsl_ddr_edac.h"
> +
> +#define EDAC_MOD_STR	"FSL_DDR_EDAC"

Lowercase please.

>  
> -static int edac_dev_idx;
> -#ifdef CONFIG_PCI
> -static int edac_pci_idx;
> -#endif
>  static int edac_mc_idx;
>  
>  static u32 orig_ddr_err_disable;
>  static u32 orig_ddr_err_sbe;
>  
> -/*
> - * PCI Err defines
> - */
> -#ifdef CONFIG_PCI
> -static u32 orig_pci_err_cap_dr;
> -static u32 orig_pci_err_en;
> -#endif
> -
> -static u32 orig_l2_err_disable;
> -
>  /************************ MC SYSFS parts ***********************************/
>  
>  #define to_mci(k) container_of(k, struct mem_ctl_info, dev)

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--

Re: [Patch v3 05/11] driver/edac/fsl-ddr: Separate FSL DDR EDAC driver from MPC85xx

[york sun]

On 08/08/2016 12:37 AM, Borislav Petkov wrote:
> On Thu, Aug 04, 2016 at 03:58:30PM -0700, York Sun wrote:
>> The mpc85xx compatible DDR controllers are used on ARM-based SoCs.
>> Separate the DDR part from mpc85xx EDAC driver and prepare to support
>> both architecture.
>>
>> Signed-off-by: York Sun <york.sun@nxp.com>
>>
>> ---
>> Change log
>>   v3: Fix compiling errors and warnings caused by patch ordering
>>   v2: Reordered patch
>>       Separate FSL DDR commont code as shared object, not another driver
>>       This patch is generated with "git format-patch -M40 -C40" to show
>>       copy-and-delete.
>>
>>  drivers/edac/Makefile                           |   5 +-
>>  drivers/edac/{mpc85xx_edac.c => fsl_ddr_edac.c} | 699 +-----------------------
>>  drivers/edac/{mpc85xx_edac.h => fsl_ddr_edac.h} | 106 +---
>>  drivers/edac/mpc85xx_edac.c                     | 559 +------------------
>>  drivers/edac/mpc85xx_edac.h                     |  66 ---
>>  5 files changed, 22 insertions(+), 1413 deletions(-)
>>  copy drivers/edac/{mpc85xx_edac.c => fsl_ddr_edac.c} (43%)
>>  copy drivers/edac/{mpc85xx_edac.h => fsl_ddr_edac.h} (43%)
>>
>> diff --git a/drivers/edac/Makefile b/drivers/edac/Makefile
>> index f9e4a3e..ee047a4 100644
>> --- a/drivers/edac/Makefile
>> +++ b/drivers/edac/Makefile
>> @@ -50,7 +50,10 @@ amd64_edac_mod-$(CONFIG_EDAC_AMD64_ERROR_INJECTION) += amd64_edac_inj.o
>>  obj-$(CONFIG_EDAC_AMD64)		+= amd64_edac_mod.o
>>
>>  obj-$(CONFIG_EDAC_PASEMI)		+= pasemi_edac.o
>> -obj-$(CONFIG_EDAC_MPC85XX)		+= mpc85xx_edac.o
>> +
>> +mpc85xx_edac_mod-y			:= fsl_ddr_edac.o mpc85xx_edac.o
>> +obj-$(CONFIG_EDAC_MPC85XX)		+= mpc85xx_edac_mod.o
>> +
>>  obj-$(CONFIG_EDAC_MV64X60)		+= mv64x60_edac.o
>>  obj-$(CONFIG_EDAC_CELL)			+= cell_edac.o
>>  obj-$(CONFIG_EDAC_PPC4XX)		+= ppc4xx_edac.o
>> diff --git a/drivers/edac/mpc85xx_edac.c b/drivers/edac/fsl_ddr_edac.c
>> similarity index 43%
>> copy from drivers/edac/mpc85xx_edac.c
>> copy to drivers/edac/fsl_ddr_edac.c
>> index c0b0951..280797e 100644
>> --- a/drivers/edac/mpc85xx_edac.c
>> +++ b/drivers/edac/fsl_ddr_edac.c
>> @@ -1,5 +1,5 @@
>>  /*
>> - * Freescale MPC85xx Memory Controller kernel module
>> + * Freescale Memory Controller kernel modul
>
> This should say something like "Layerscape SoC blabla... Originally
> split out from mpc85xx_edac EDAC driver."
>
> Ditto for the header below.

OK.

>
>>   *
>>   * Parts Copyrighted (c) 2013 by Freescale Semiconductor, Inc.
>>   *
>> @@ -20,33 +20,20 @@
>>  #include <linux/edac.h>
>>  #include <linux/smp.h>
>>  #include <linux/gfp.h>
>> -#include <linux/fsl/edac.h>
>>
>>  #include <linux/of_platform.h>
>>  #include <linux/of_device.h>
>>  #include "edac_module.h"
>>  #include "edac_core.h"
>> -#include "mpc85xx_edac.h"
>> +#include "fsl_ddr_edac.h"
>> +
>> +#define EDAC_MOD_STR	"FSL_DDR_EDAC"
>
> Lowercase please.

I followed old name style. Will change.

York

[Patch v3 06/11] driver/edac/fsl_ddr: Rename macros and names

[York Sun]

Use generic names for macros, variables and functions.

Signed-off-by: York Sun <york.sun@nxp.com>

---
Change log
  v3: Absort changes from previous patch after reording
  v2: Separated from "House cleaning" patch of v1

 drivers/edac/fsl_ddr_edac.c | 153 ++++++++++++++++++++++----------------------
 drivers/edac/fsl_ddr_edac.h |  43 ++++++-------
 drivers/edac/mpc85xx_edac.c |   4 +-
 3 files changed, 98 insertions(+), 102 deletions(-)

diff --git a/drivers/edac/fsl_ddr_edac.c b/drivers/edac/fsl_ddr_edac.c
index 280797e..60761c0 100644
--- a/drivers/edac/fsl_ddr_edac.c
+++ b/drivers/edac/fsl_ddr_edac.c
@@ -38,74 +38,74 @@ static u32 orig_ddr_err_sbe;
 
 #define to_mci(k) container_of(k, struct mem_ctl_info, dev)
 
-static ssize_t mpc85xx_mc_inject_data_hi_show(struct device *dev,
+static ssize_t fsl_ddr_mc_inject_data_hi_show(struct device *dev,
 					      struct device_attribute *mattr,
 					      char *data)
 {
 	struct mem_ctl_info *mci = to_mci(dev);
-	struct mpc85xx_mc_pdata *pdata = mci->pvt_info;
+	struct fsl_ddr_mc_pdata *pdata = mci->pvt_info;
 	return sprintf(data, "0x%08x",
 		       in_be32(pdata->mc_vbase +
-			       MPC85XX_MC_DATA_ERR_INJECT_HI));
+			       MC_DATA_ERR_INJECT_HI));
 }
 
-static ssize_t mpc85xx_mc_inject_data_lo_show(struct device *dev,
+static ssize_t fsl_ddr_mc_inject_data_lo_show(struct device *dev,
 					      struct device_attribute *mattr,
 					      char *data)
 {
 	struct mem_ctl_info *mci = to_mci(dev);
-	struct mpc85xx_mc_pdata *pdata = mci->pvt_info;
+	struct fsl_ddr_mc_pdata *pdata = mci->pvt_info;
 	return sprintf(data, "0x%08x",
 		       in_be32(pdata->mc_vbase +
-			       MPC85XX_MC_DATA_ERR_INJECT_LO));
+			       MC_DATA_ERR_INJECT_LO));
 }
 
-static ssize_t mpc85xx_mc_inject_ctrl_show(struct device *dev,
+static ssize_t fsl_ddr_mc_inject_ctrl_show(struct device *dev,
 					   struct device_attribute *mattr,
 					   char *data)
 {
 	struct mem_ctl_info *mci = to_mci(dev);
-	struct mpc85xx_mc_pdata *pdata = mci->pvt_info;
+	struct fsl_ddr_mc_pdata *pdata = mci->pvt_info;
 	return sprintf(data, "0x%08x",
-		       in_be32(pdata->mc_vbase + MPC85XX_MC_ECC_ERR_INJECT));
+		       in_be32(pdata->mc_vbase + MC_ECC_ERR_INJECT));
 }
 
-static ssize_t mpc85xx_mc_inject_data_hi_store(struct device *dev,
+static ssize_t fsl_ddr_mc_inject_data_hi_store(struct device *dev,
 					       struct device_attribute *mattr,
 					       const char *data, size_t count)
 {
 	struct mem_ctl_info *mci = to_mci(dev);
-	struct mpc85xx_mc_pdata *pdata = mci->pvt_info;
+	struct fsl_ddr_mc_pdata *pdata = mci->pvt_info;
 	if (isdigit(*data)) {
-		out_be32(pdata->mc_vbase + MPC85XX_MC_DATA_ERR_INJECT_HI,
+		out_be32(pdata->mc_vbase + MC_DATA_ERR_INJECT_HI,
 			 simple_strtoul(data, NULL, 0));
 		return count;
 	}
 	return 0;
 }
 
-static ssize_t mpc85xx_mc_inject_data_lo_store(struct device *dev,
+static ssize_t fsl_ddr_mc_inject_data_lo_store(struct device *dev,
 					       struct device_attribute *mattr,
 					       const char *data, size_t count)
 {
 	struct mem_ctl_info *mci = to_mci(dev);
-	struct mpc85xx_mc_pdata *pdata = mci->pvt_info;
+	struct fsl_ddr_mc_pdata *pdata = mci->pvt_info;
 	if (isdigit(*data)) {
-		out_be32(pdata->mc_vbase + MPC85XX_MC_DATA_ERR_INJECT_LO,
+		out_be32(pdata->mc_vbase + MC_DATA_ERR_INJECT_LO,
 			 simple_strtoul(data, NULL, 0));
 		return count;
 	}
 	return 0;
 }
 
-static ssize_t mpc85xx_mc_inject_ctrl_store(struct device *dev,
+static ssize_t fsl_ddr_mc_inject_ctrl_store(struct device *dev,
 					       struct device_attribute *mattr,
 					       const char *data, size_t count)
 {
 	struct mem_ctl_info *mci = to_mci(dev);
-	struct mpc85xx_mc_pdata *pdata = mci->pvt_info;
+	struct fsl_ddr_mc_pdata *pdata = mci->pvt_info;
 	if (isdigit(*data)) {
-		out_be32(pdata->mc_vbase + MPC85XX_MC_ECC_ERR_INJECT,
+		out_be32(pdata->mc_vbase + MC_ECC_ERR_INJECT,
 			 simple_strtoul(data, NULL, 0));
 		return count;
 	}
@@ -113,20 +113,20 @@ static ssize_t mpc85xx_mc_inject_ctrl_store(struct device *dev,
 }
 
 DEVICE_ATTR(inject_data_hi, S_IRUGO | S_IWUSR,
-	    mpc85xx_mc_inject_data_hi_show, mpc85xx_mc_inject_data_hi_store);
+	    fsl_ddr_mc_inject_data_hi_show, fsl_ddr_mc_inject_data_hi_store);
 DEVICE_ATTR(inject_data_lo, S_IRUGO | S_IWUSR,
-	    mpc85xx_mc_inject_data_lo_show, mpc85xx_mc_inject_data_lo_store);
+	    fsl_ddr_mc_inject_data_lo_show, fsl_ddr_mc_inject_data_lo_store);
 DEVICE_ATTR(inject_ctrl, S_IRUGO | S_IWUSR,
-	    mpc85xx_mc_inject_ctrl_show, mpc85xx_mc_inject_ctrl_store);
+	    fsl_ddr_mc_inject_ctrl_show, fsl_ddr_mc_inject_ctrl_store);
 
-static struct attribute *mpc85xx_dev_attrs[] = {
+static struct attribute *fsl_ddr_dev_attrs[] = {
 	&dev_attr_inject_data_hi.attr,
 	&dev_attr_inject_data_lo.attr,
 	&dev_attr_inject_ctrl.attr,
 	NULL
 };
 
-ATTRIBUTE_GROUPS(mpc85xx_dev);
+ATTRIBUTE_GROUPS(fsl_ddr_dev);
 
 /**************************** MC Err device ***************************/
 
@@ -237,9 +237,9 @@ static void sbe_ecc_decode(u32 cap_high, u32 cap_low, u32 cap_ecc,
 
 #define make64(high, low) (((u64)(high) << 32) | (low))
 
-static void mpc85xx_mc_check(struct mem_ctl_info *mci)
+static void fsl_ddr_mc_check(struct mem_ctl_info *mci)
 {
-	struct mpc85xx_mc_pdata *pdata = mci->pvt_info;
+	struct fsl_ddr_mc_pdata *pdata = mci->pvt_info;
 	struct csrow_info *csrow;
 	u32 bus_width;
 	u32 err_detect;
@@ -252,23 +252,23 @@ static void mpc85xx_mc_check(struct mem_ctl_info *mci)
 	int bad_data_bit;
 	int bad_ecc_bit;
 
-	err_detect = in_be32(pdata->mc_vbase + MPC85XX_MC_ERR_DETECT);
+	err_detect = in_be32(pdata->mc_vbase + MC_ERR_DETECT);
 	if (!err_detect)
 		return;
 
-	mpc85xx_mc_printk(mci, KERN_ERR, "Err Detect Register: %#8.8x\n",
+	fsl_ddr_mc_printk(mci, KERN_ERR, "Err Detect Register: %#8.8x\n",
 			  err_detect);
 
 	/* no more processing if not ECC bit errors */
 	if (!(err_detect & (DDR_EDE_SBE | DDR_EDE_MBE))) {
-		out_be32(pdata->mc_vbase + MPC85XX_MC_ERR_DETECT, err_detect);
+		out_be32(pdata->mc_vbase + MC_ERR_DETECT, err_detect);
 		return;
 	}
 
-	syndrome = in_be32(pdata->mc_vbase + MPC85XX_MC_CAPTURE_ECC);
+	syndrome = in_be32(pdata->mc_vbase + MC_CAPTURE_ECC);
 
 	/* Mask off appropriate bits of syndrome based on bus width */
-	bus_width = (in_be32(pdata->mc_vbase + MPC85XX_MC_DDR_SDRAM_CFG) &
+	bus_width = (in_be32(pdata->mc_vbase + MC_DDR_SDRAM_CFG) &
 			DSC_DBW_MASK) ? 32 : 64;
 	if (bus_width == 64)
 		syndrome &= 0xff;
@@ -276,8 +276,8 @@ static void mpc85xx_mc_check(struct mem_ctl_info *mci)
 		syndrome &= 0xffff;
 
 	err_addr = make64(
-		in_be32(pdata->mc_vbase + MPC85XX_MC_CAPTURE_EXT_ADDRESS),
-		in_be32(pdata->mc_vbase + MPC85XX_MC_CAPTURE_ADDRESS));
+		in_be32(pdata->mc_vbase + MC_CAPTURE_EXT_ADDRESS),
+		in_be32(pdata->mc_vbase + MC_CAPTURE_ADDRESS));
 	pfn = err_addr >> PAGE_SHIFT;
 
 	for (row_index = 0; row_index < mci->nr_csrows; row_index++) {
@@ -286,8 +286,8 @@ static void mpc85xx_mc_check(struct mem_ctl_info *mci)
 			break;
 	}
 
-	cap_high = in_be32(pdata->mc_vbase + MPC85XX_MC_CAPTURE_DATA_HI);
-	cap_low = in_be32(pdata->mc_vbase + MPC85XX_MC_CAPTURE_DATA_LO);
+	cap_high = in_be32(pdata->mc_vbase + MC_CAPTURE_DATA_HI);
+	cap_low = in_be32(pdata->mc_vbase + MC_CAPTURE_DATA_LO);
 
 	/*
 	 * Analyze single-bit errors on 64-bit wide buses
@@ -298,28 +298,28 @@ static void mpc85xx_mc_check(struct mem_ctl_info *mci)
 				&bad_data_bit, &bad_ecc_bit);
 
 		if (bad_data_bit != -1)
-			mpc85xx_mc_printk(mci, KERN_ERR,
+			fsl_ddr_mc_printk(mci, KERN_ERR,
 				"Faulty Data bit: %d\n", bad_data_bit);
 		if (bad_ecc_bit != -1)
-			mpc85xx_mc_printk(mci, KERN_ERR,
+			fsl_ddr_mc_printk(mci, KERN_ERR,
 				"Faulty ECC bit: %d\n", bad_ecc_bit);
 
-		mpc85xx_mc_printk(mci, KERN_ERR,
+		fsl_ddr_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));
 	}
 
-	mpc85xx_mc_printk(mci, KERN_ERR,
+	fsl_ddr_mc_printk(mci, KERN_ERR,
 			"Captured Data / ECC:\t%#8.8x_%08x / %#2.2x\n",
 			cap_high, cap_low, syndrome);
-	mpc85xx_mc_printk(mci, KERN_ERR, "Err addr: %#8.8llx\n", err_addr);
-	mpc85xx_mc_printk(mci, KERN_ERR, "PFN: %#8.8x\n", pfn);
+	fsl_ddr_mc_printk(mci, KERN_ERR, "Err addr: %#8.8llx\n", err_addr);
+	fsl_ddr_mc_printk(mci, KERN_ERR, "PFN: %#8.8x\n", pfn);
 
 	/* we are out of range */
 	if (row_index == mci->nr_csrows)
-		mpc85xx_mc_printk(mci, KERN_ERR, "PFN out of range!\n");
+		fsl_ddr_mc_printk(mci, KERN_ERR, "PFN out of range!\n");
 
 	if (err_detect & DDR_EDE_SBE)
 		edac_mc_handle_error(HW_EVENT_ERR_CORRECTED, mci, 1,
@@ -333,27 +333,27 @@ static void mpc85xx_mc_check(struct mem_ctl_info *mci)
 				     row_index, 0, -1,
 				     mci->ctl_name, "");
 
-	out_be32(pdata->mc_vbase + MPC85XX_MC_ERR_DETECT, err_detect);
+	out_be32(pdata->mc_vbase + MC_ERR_DETECT, err_detect);
 }
 
-static irqreturn_t mpc85xx_mc_isr(int irq, void *dev_id)
+static irqreturn_t fsl_ddr_mc_isr(int irq, void *dev_id)
 {
 	struct mem_ctl_info *mci = dev_id;
-	struct mpc85xx_mc_pdata *pdata = mci->pvt_info;
+	struct fsl_ddr_mc_pdata *pdata = mci->pvt_info;
 	u32 err_detect;
 
-	err_detect = in_be32(pdata->mc_vbase + MPC85XX_MC_ERR_DETECT);
+	err_detect = in_be32(pdata->mc_vbase + MC_ERR_DETECT);
 	if (!err_detect)
 		return IRQ_NONE;
 
-	mpc85xx_mc_check(mci);
+	fsl_ddr_mc_check(mci);
 
 	return IRQ_HANDLED;
 }
 
-static void mpc85xx_init_csrows(struct mem_ctl_info *mci)
+static void fsl_ddr_init_csrows(struct mem_ctl_info *mci)
 {
-	struct mpc85xx_mc_pdata *pdata = mci->pvt_info;
+	struct fsl_ddr_mc_pdata *pdata = mci->pvt_info;
 	struct csrow_info *csrow;
 	struct dimm_info *dimm;
 	u32 sdram_ctl;
@@ -362,7 +362,7 @@ static void mpc85xx_init_csrows(struct mem_ctl_info *mci)
 	u32 cs_bnds;
 	int index;
 
-	sdram_ctl = in_be32(pdata->mc_vbase + MPC85XX_MC_DDR_SDRAM_CFG);
+	sdram_ctl = in_be32(pdata->mc_vbase + MC_DDR_SDRAM_CFG);
 
 	sdtype = sdram_ctl & DSC_SDTYPE_MASK;
 	if (sdram_ctl & DSC_RD_EN) {
@@ -404,8 +404,8 @@ static void mpc85xx_init_csrows(struct mem_ctl_info *mci)
 		csrow = mci->csrows[index];
 		dimm = csrow->channels[0]->dimm;
 
-		cs_bnds = in_be32(pdata->mc_vbase + MPC85XX_MC_CS_BNDS_0 +
-				  (index * MPC85XX_MC_CS_BNDS_OFS));
+		cs_bnds = in_be32(pdata->mc_vbase + MC_CS_BNDS_0 +
+				  (index * MC_CS_BNDS_OFS));
 
 		start = (cs_bnds & 0xffff0000) >> 16;
 		end   = (cs_bnds & 0x0000ffff);
@@ -430,16 +430,16 @@ static void mpc85xx_init_csrows(struct mem_ctl_info *mci)
 	}
 }
 
-int mpc85xx_mc_err_probe(struct platform_device *op)
+int fsl_ddr_mc_err_probe(struct platform_device *op)
 {
 	struct mem_ctl_info *mci;
 	struct edac_mc_layer layers[2];
-	struct mpc85xx_mc_pdata *pdata;
+	struct fsl_ddr_mc_pdata *pdata;
 	struct resource r;
 	u32 sdram_ctl;
 	int res;
 
-	if (!devres_open_group(&op->dev, mpc85xx_mc_err_probe, GFP_KERNEL))
+	if (!devres_open_group(&op->dev, fsl_ddr_mc_err_probe, GFP_KERNEL))
 		return -ENOMEM;
 
 	layers[0].type = EDAC_MC_LAYER_CHIP_SELECT;
@@ -451,12 +451,12 @@ int mpc85xx_mc_err_probe(struct platform_device *op)
 	mci = edac_mc_alloc(edac_mc_idx, ARRAY_SIZE(layers), layers,
 			    sizeof(*pdata));
 	if (!mci) {
-		devres_release_group(&op->dev, mpc85xx_mc_err_probe);
+		devres_release_group(&op->dev, fsl_ddr_mc_err_probe);
 		return -ENOMEM;
 	}
 
 	pdata = mci->pvt_info;
-	pdata->name = "mpc85xx_mc_err";
+	pdata->name = "fsl_ddr_mc_err";
 	pdata->irq = NO_IRQ;
 	mci->pdev = &op->dev;
 	pdata->edac_idx = edac_mc_idx++;
@@ -486,7 +486,7 @@ int mpc85xx_mc_err_probe(struct platform_device *op)
 		goto err;
 	}
 
-	sdram_ctl = in_be32(pdata->mc_vbase + MPC85XX_MC_DDR_SDRAM_CFG);
+	sdram_ctl = in_be32(pdata->mc_vbase + MC_DDR_SDRAM_CFG);
 	if (!(sdram_ctl & DSC_ECC_EN)) {
 		/* no ECC */
 		pr_warn("%s: No ECC DIMMs discovered\n", __func__);
@@ -502,46 +502,46 @@ int mpc85xx_mc_err_probe(struct platform_device *op)
 	mci->mod_name = EDAC_MOD_STR;
 
 	if (edac_op_state == EDAC_OPSTATE_POLL)
-		mci->edac_check = mpc85xx_mc_check;
+		mci->edac_check = fsl_ddr_mc_check;
 
 	mci->ctl_page_to_phys = NULL;
 
 	mci->scrub_mode = SCRUB_SW_SRC;
 
-	mpc85xx_init_csrows(mci);
+	fsl_ddr_init_csrows(mci);
 
 	/* store the original error disable bits */
 	orig_ddr_err_disable =
-	    in_be32(pdata->mc_vbase + MPC85XX_MC_ERR_DISABLE);
-	out_be32(pdata->mc_vbase + MPC85XX_MC_ERR_DISABLE, 0);
+	    in_be32(pdata->mc_vbase + MC_ERR_DISABLE);
+	out_be32(pdata->mc_vbase + MC_ERR_DISABLE, 0);
 
 	/* clear all error bits */
-	out_be32(pdata->mc_vbase + MPC85XX_MC_ERR_DETECT, ~0);
+	out_be32(pdata->mc_vbase + MC_ERR_DETECT, ~0);
 
-	if (edac_mc_add_mc_with_groups(mci, mpc85xx_dev_groups)) {
+	if (edac_mc_add_mc_with_groups(mci, fsl_ddr_dev_groups)) {
 		edac_dbg(3, "failed edac_mc_add_mc()\n");
 		goto err;
 	}
 
 	if (edac_op_state == EDAC_OPSTATE_INT) {
-		out_be32(pdata->mc_vbase + MPC85XX_MC_ERR_INT_EN,
+		out_be32(pdata->mc_vbase + MC_ERR_INT_EN,
 			 DDR_EIE_MBEE | DDR_EIE_SBEE);
 
 		/* store the original error management threshold */
 		orig_ddr_err_sbe = in_be32(pdata->mc_vbase +
-					   MPC85XX_MC_ERR_SBE) & 0xff0000;
+					   MC_ERR_SBE) & 0xff0000;
 
 		/* set threshold to 1 error per interrupt */
-		out_be32(pdata->mc_vbase + MPC85XX_MC_ERR_SBE, 0x10000);
+		out_be32(pdata->mc_vbase + MC_ERR_SBE, 0x10000);
 
 		/* register interrupts */
 		pdata->irq = irq_of_parse_and_map(op->dev.of_node, 0);
 		res = devm_request_irq(&op->dev, pdata->irq,
-				       mpc85xx_mc_isr,
+				       fsl_ddr_mc_isr,
 				       IRQF_SHARED,
 				       "[EDAC] MC err", mci);
 		if (res < 0) {
-			pr_err("%s: Unable to request irq %d for MPC85xx DRAM ERR\n",
+			pr_err("%s: Unable to request irq %d for FSL DDR DRAM ERR\n",
 			       __func__, pdata->irq);
 			irq_dispose_mapping(pdata->irq);
 			res = -ENODEV;
@@ -552,7 +552,7 @@ int mpc85xx_mc_err_probe(struct platform_device *op)
 		       pdata->irq);
 	}
 
-	devres_remove_group(&op->dev, mpc85xx_mc_err_probe);
+	devres_remove_group(&op->dev, fsl_ddr_mc_err_probe);
 	edac_dbg(3, "success\n");
 	pr_info(EDAC_MOD_STR " MC err registered\n");
 
@@ -561,30 +561,29 @@ int mpc85xx_mc_err_probe(struct platform_device *op)
 err2:
 	edac_mc_del_mc(&op->dev);
 err:
-	devres_release_group(&op->dev, mpc85xx_mc_err_probe);
+	devres_release_group(&op->dev, fsl_ddr_mc_err_probe);
 	edac_mc_free(mci);
 	return res;
 }
-EXPORT_SYMBOL_GPL(mpc85xx_mc_err_probe);
 
-int mpc85xx_mc_err_remove(struct platform_device *op)
+int fsl_ddr_mc_err_remove(struct platform_device *op)
 {
 	struct mem_ctl_info *mci = dev_get_drvdata(&op->dev);
-	struct mpc85xx_mc_pdata *pdata = mci->pvt_info;
+	struct fsl_ddr_mc_pdata *pdata = mci->pvt_info;
 
 	edac_dbg(0, "\n");
 
 	if (edac_op_state == EDAC_OPSTATE_INT) {
-		out_be32(pdata->mc_vbase + MPC85XX_MC_ERR_INT_EN, 0);
+		out_be32(pdata->mc_vbase + MC_ERR_INT_EN, 0);
 		irq_dispose_mapping(pdata->irq);
 	}
 
-	out_be32(pdata->mc_vbase + MPC85XX_MC_ERR_DISABLE,
+	out_be32(pdata->mc_vbase + MC_ERR_DISABLE,
 		 orig_ddr_err_disable);
-	out_be32(pdata->mc_vbase + MPC85XX_MC_ERR_SBE, orig_ddr_err_sbe);
+	out_be32(pdata->mc_vbase + MC_ERR_SBE, orig_ddr_err_sbe);
 
 	edac_mc_del_mc(&op->dev);
 	edac_mc_free(mci);
 	return 0;
 }
-EXPORT_SYMBOL_GPL(mpc85xx_mc_err_remove);
+EXPORT_SYMBOL_GPL(fsl_ddr_mc_err_remove);
diff --git a/drivers/edac/fsl_ddr_edac.h b/drivers/edac/fsl_ddr_edac.h
index 94c6907..556bac5 100644
--- a/drivers/edac/fsl_ddr_edac.h
+++ b/drivers/edac/fsl_ddr_edac.h
@@ -12,7 +12,7 @@
 #ifndef _FSL_DDR_EDAC_H_
 #define _FSL_DDR_EDAC_H_
 
-#define mpc85xx_mc_printk(mci, level, fmt, arg...) \
+#define fsl_ddr_mc_printk(mci, level, fmt, arg...) \
 	edac_mc_chipset_printk(mci, level, "FSL_DDR", fmt, ##arg)
 
 /*
@@ -20,26 +20,23 @@
  */
 
 /* DDR_SDRAM_CFG */
-#define MPC85XX_MC_DDR_SDRAM_CFG	0x0110
-#define MPC85XX_MC_CS_BNDS_0		0x0000
-#define MPC85XX_MC_CS_BNDS_1		0x0008
-#define MPC85XX_MC_CS_BNDS_2		0x0010
-#define MPC85XX_MC_CS_BNDS_3		0x0018
-#define MPC85XX_MC_CS_BNDS_OFS		0x0008
+#define MC_DDR_SDRAM_CFG	0x0110
+#define MC_CS_BNDS_0		0x0000
+#define MC_CS_BNDS_OFS		0x0008
 
-#define MPC85XX_MC_DATA_ERR_INJECT_HI	0x0e00
-#define MPC85XX_MC_DATA_ERR_INJECT_LO	0x0e04
-#define MPC85XX_MC_ECC_ERR_INJECT	0x0e08
-#define MPC85XX_MC_CAPTURE_DATA_HI	0x0e20
-#define MPC85XX_MC_CAPTURE_DATA_LO	0x0e24
-#define MPC85XX_MC_CAPTURE_ECC		0x0e28
-#define MPC85XX_MC_ERR_DETECT		0x0e40
-#define MPC85XX_MC_ERR_DISABLE		0x0e44
-#define MPC85XX_MC_ERR_INT_EN		0x0e48
-#define MPC85XX_MC_CAPTURE_ATRIBUTES	0x0e4c
-#define MPC85XX_MC_CAPTURE_ADDRESS	0x0e50
-#define MPC85XX_MC_CAPTURE_EXT_ADDRESS	0x0e54
-#define MPC85XX_MC_ERR_SBE		0x0e58
+#define MC_DATA_ERR_INJECT_HI	0x0e00
+#define MC_DATA_ERR_INJECT_LO	0x0e04
+#define MC_ECC_ERR_INJECT	0x0e08
+#define MC_CAPTURE_DATA_HI	0x0e20
+#define MC_CAPTURE_DATA_LO	0x0e24
+#define MC_CAPTURE_ECC		0x0e28
+#define MC_ERR_DETECT		0x0e40
+#define MC_ERR_DISABLE		0x0e44
+#define MC_ERR_INT_EN		0x0e48
+#define MC_CAPTURE_ATRIBUTES	0x0e4c
+#define MC_CAPTURE_ADDRESS	0x0e50
+#define MC_CAPTURE_EXT_ADDRESS	0x0e54
+#define MC_ERR_SBE		0x0e58
 
 #define DSC_MEM_EN	0x80000000
 #define DSC_ECC_EN	0x20000000
@@ -71,12 +68,12 @@
 #define	DDR_EDI_SBED	0x4	/* single-bit ECC error disable */
 #define	DDR_EDI_MBED	0x8	/* multi-bit ECC error disable */
 
-struct mpc85xx_mc_pdata {
+struct fsl_ddr_mc_pdata {
 	char *name;
 	int edac_idx;
 	void __iomem *mc_vbase;
 	int irq;
 };
-int mpc85xx_mc_err_probe(struct platform_device *op);
-int mpc85xx_mc_err_remove(struct platform_device *op);
+int fsl_ddr_mc_err_probe(struct platform_device *op);
+int fsl_ddr_mc_err_remove(struct platform_device *op);
 #endif
diff --git a/drivers/edac/mpc85xx_edac.c b/drivers/edac/mpc85xx_edac.c
index af9ce77..351b136 100644
--- a/drivers/edac/mpc85xx_edac.c
+++ b/drivers/edac/mpc85xx_edac.c
@@ -655,8 +655,8 @@ static const struct of_device_id mpc85xx_mc_err_of_match[] = {
 MODULE_DEVICE_TABLE(of, mpc85xx_mc_err_of_match);
 
 static struct platform_driver mpc85xx_mc_err_driver = {
-	.probe = mpc85xx_mc_err_probe,
-	.remove = mpc85xx_mc_err_remove,
+	.probe = fsl_ddr_mc_err_probe,
+	.remove = fsl_ddr_mc_err_remove,
 	.driver = {
 		.name = "mpc85xx_mc_err",
 		.of_match_table = mpc85xx_mc_err_of_match,
-- 
2.7.4

Re: [Patch v3 06/11] driver/edac/fsl_ddr: Rename macros and names

[Borislav Petkov]

On Thu, Aug 04, 2016 at 03:58:31PM -0700, York Sun wrote:
> Use generic names for macros, variables and functions.
> 
> Signed-off-by: York Sun <york.sun@nxp.com>
> 
> ---
> Change log
>   v3: Absort changes from previous patch after reording
>   v2: Separated from "House cleaning" patch of v1
> 
>  drivers/edac/fsl_ddr_edac.c | 153 ++++++++++++++++++++++----------------------
>  drivers/edac/fsl_ddr_edac.h |  43 ++++++-------
>  drivers/edac/mpc85xx_edac.c |   4 +-
>  3 files changed, 98 insertions(+), 102 deletions(-)

...

> diff --git a/drivers/edac/fsl_ddr_edac.h b/drivers/edac/fsl_ddr_edac.h
> index 94c6907..556bac5 100644
> --- a/drivers/edac/fsl_ddr_edac.h
> +++ b/drivers/edac/fsl_ddr_edac.h
> @@ -12,7 +12,7 @@
>  #ifndef _FSL_DDR_EDAC_H_
>  #define _FSL_DDR_EDAC_H_
>  
> -#define mpc85xx_mc_printk(mci, level, fmt, arg...) \
> +#define fsl_ddr_mc_printk(mci, level, fmt, arg...) \
>  	edac_mc_chipset_printk(mci, level, "FSL_DDR", fmt, ##arg)
>  
>  /*
> @@ -20,26 +20,23 @@
>   */
>  
>  /* DDR_SDRAM_CFG */
> -#define MPC85XX_MC_DDR_SDRAM_CFG	0x0110
> -#define MPC85XX_MC_CS_BNDS_0		0x0000
> -#define MPC85XX_MC_CS_BNDS_1		0x0008
> -#define MPC85XX_MC_CS_BNDS_2		0x0010
> -#define MPC85XX_MC_CS_BNDS_3		0x0018
> -#define MPC85XX_MC_CS_BNDS_OFS		0x0008
> +#define MC_DDR_SDRAM_CFG	0x0110
> +#define MC_CS_BNDS_0		0x0000
> +#define MC_CS_BNDS_OFS		0x0008
>  
> -#define MPC85XX_MC_DATA_ERR_INJECT_HI	0x0e00
> -#define MPC85XX_MC_DATA_ERR_INJECT_LO	0x0e04
> -#define MPC85XX_MC_ECC_ERR_INJECT	0x0e08
> -#define MPC85XX_MC_CAPTURE_DATA_HI	0x0e20
> -#define MPC85XX_MC_CAPTURE_DATA_LO	0x0e24
> -#define MPC85XX_MC_CAPTURE_ECC		0x0e28
> -#define MPC85XX_MC_ERR_DETECT		0x0e40
> -#define MPC85XX_MC_ERR_DISABLE		0x0e44
> -#define MPC85XX_MC_ERR_INT_EN		0x0e48
> -#define MPC85XX_MC_CAPTURE_ATRIBUTES	0x0e4c
> -#define MPC85XX_MC_CAPTURE_ADDRESS	0x0e50
> -#define MPC85XX_MC_CAPTURE_EXT_ADDRESS	0x0e54
> -#define MPC85XX_MC_ERR_SBE		0x0e58
> +#define MC_DATA_ERR_INJECT_HI	0x0e00
> +#define MC_DATA_ERR_INJECT_LO	0x0e04
> +#define MC_ECC_ERR_INJECT	0x0e08
> +#define MC_CAPTURE_DATA_HI	0x0e20
> +#define MC_CAPTURE_DATA_LO	0x0e24
> +#define MC_CAPTURE_ECC		0x0e28
> +#define MC_ERR_DETECT		0x0e40
> +#define MC_ERR_DISABLE		0x0e44
> +#define MC_ERR_INT_EN		0x0e48
> +#define MC_CAPTURE_ATRIBUTES	0x0e4c
> +#define MC_CAPTURE_ADDRESS	0x0e50
> +#define MC_CAPTURE_EXT_ADDRESS	0x0e54
> +#define MC_ERR_SBE		0x0e58

You should choose one name prefix for all driver symbols and stick to
it. Either "FSL" or "LSC" for "Layerscape" or whatever but please be
consistent.

MC_ is too generic.

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--

Re: [Patch v3 06/11] driver/edac/fsl_ddr: Rename macros and names

[york sun]

On 08/08/2016 12:41 AM, Borislav Petkov wrote:
> On Thu, Aug 04, 2016 at 03:58:31PM -0700, York Sun wrote:
>> Use generic names for macros, variables and functions.
>>
>> Signed-off-by: York Sun <york.sun@nxp.com>
>>
>> ---
>> Change log
>>   v3: Absort changes from previous patch after reording
>>   v2: Separated from "House cleaning" patch of v1
>>
>>  drivers/edac/fsl_ddr_edac.c | 153 ++++++++++++++++++++++----------------------
>>  drivers/edac/fsl_ddr_edac.h |  43 ++++++-------
>>  drivers/edac/mpc85xx_edac.c |   4 +-
>>  3 files changed, 98 insertions(+), 102 deletions(-)
>
> ...
>
>> diff --git a/drivers/edac/fsl_ddr_edac.h b/drivers/edac/fsl_ddr_edac.h
>> index 94c6907..556bac5 100644
>> --- a/drivers/edac/fsl_ddr_edac.h
>> +++ b/drivers/edac/fsl_ddr_edac.h
>> @@ -12,7 +12,7 @@
>>  #ifndef _FSL_DDR_EDAC_H_
>>  #define _FSL_DDR_EDAC_H_
>>
>> -#define mpc85xx_mc_printk(mci, level, fmt, arg...) \
>> +#define fsl_ddr_mc_printk(mci, level, fmt, arg...) \
>>  	edac_mc_chipset_printk(mci, level, "FSL_DDR", fmt, ##arg)
>>
>>  /*
>> @@ -20,26 +20,23 @@
>>   */
>>
>>  /* DDR_SDRAM_CFG */
>> -#define MPC85XX_MC_DDR_SDRAM_CFG	0x0110
>> -#define MPC85XX_MC_CS_BNDS_0		0x0000
>> -#define MPC85XX_MC_CS_BNDS_1		0x0008
>> -#define MPC85XX_MC_CS_BNDS_2		0x0010
>> -#define MPC85XX_MC_CS_BNDS_3		0x0018
>> -#define MPC85XX_MC_CS_BNDS_OFS		0x0008
>> +#define MC_DDR_SDRAM_CFG	0x0110
>> +#define MC_CS_BNDS_0		0x0000
>> +#define MC_CS_BNDS_OFS		0x0008
>>
>> -#define MPC85XX_MC_DATA_ERR_INJECT_HI	0x0e00
>> -#define MPC85XX_MC_DATA_ERR_INJECT_LO	0x0e04
>> -#define MPC85XX_MC_ECC_ERR_INJECT	0x0e08
>> -#define MPC85XX_MC_CAPTURE_DATA_HI	0x0e20
>> -#define MPC85XX_MC_CAPTURE_DATA_LO	0x0e24
>> -#define MPC85XX_MC_CAPTURE_ECC		0x0e28
>> -#define MPC85XX_MC_ERR_DETECT		0x0e40
>> -#define MPC85XX_MC_ERR_DISABLE		0x0e44
>> -#define MPC85XX_MC_ERR_INT_EN		0x0e48
>> -#define MPC85XX_MC_CAPTURE_ATRIBUTES	0x0e4c
>> -#define MPC85XX_MC_CAPTURE_ADDRESS	0x0e50
>> -#define MPC85XX_MC_CAPTURE_EXT_ADDRESS	0x0e54
>> -#define MPC85XX_MC_ERR_SBE		0x0e58
>> +#define MC_DATA_ERR_INJECT_HI	0x0e00
>> +#define MC_DATA_ERR_INJECT_LO	0x0e04
>> +#define MC_ECC_ERR_INJECT	0x0e08
>> +#define MC_CAPTURE_DATA_HI	0x0e20
>> +#define MC_CAPTURE_DATA_LO	0x0e24
>> +#define MC_CAPTURE_ECC		0x0e28
>> +#define MC_ERR_DETECT		0x0e40
>> +#define MC_ERR_DISABLE		0x0e44
>> +#define MC_ERR_INT_EN		0x0e48
>> +#define MC_CAPTURE_ATRIBUTES	0x0e4c
>> +#define MC_CAPTURE_ADDRESS	0x0e50
>> +#define MC_CAPTURE_EXT_ADDRESS	0x0e54
>> +#define MC_ERR_SBE		0x0e58
>
> You should choose one name prefix for all driver symbols and stick to
> it. Either "FSL" or "LSC" for "Layerscape" or whatever but please be
> consistent.

Will replace MPC85XX with FSL.

York

[Patch v3 07/11] driver/edac/fsl_ddr: Add DDR4 type

[York Sun]

Signed-off-by: York Sun <york.sun@nxp.com>

---
Change log
  v3: no change
  v2: no change

 drivers/edac/fsl_ddr_edac.c | 12 ++++++++++--
 drivers/edac/fsl_ddr_edac.h |  1 +
 2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/edac/fsl_ddr_edac.c b/drivers/edac/fsl_ddr_edac.c
index 60761c0..88ecf7d 100644
--- a/drivers/edac/fsl_ddr_edac.c
+++ b/drivers/edac/fsl_ddr_edac.c
@@ -376,6 +376,9 @@ static void fsl_ddr_init_csrows(struct mem_ctl_info *mci)
 		case DSC_SDTYPE_DDR3:
 			mtype = MEM_RDDR3;
 			break;
+		case DSC_SDTYPE_DDR4:
+			mtype = MEM_RDDR4;
+			break;
 		default:
 			mtype = MEM_UNKNOWN;
 			break;
@@ -391,6 +394,9 @@ static void fsl_ddr_init_csrows(struct mem_ctl_info *mci)
 		case DSC_SDTYPE_DDR3:
 			mtype = MEM_DDR3;
 			break;
+		case DSC_SDTYPE_DDR4:
+			mtype = MEM_DDR4;
+			break;
 		default:
 			mtype = MEM_UNKNOWN;
 			break;
@@ -495,8 +501,10 @@ int fsl_ddr_mc_err_probe(struct platform_device *op)
 	}
 
 	edac_dbg(3, "init mci\n");
-	mci->mtype_cap = MEM_FLAG_RDDR | MEM_FLAG_RDDR2 |
-	    MEM_FLAG_DDR | MEM_FLAG_DDR2;
+	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;
 	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 556bac5..c7b7dbf 100644
--- a/drivers/edac/fsl_ddr_edac.h
+++ b/drivers/edac/fsl_ddr_edac.h
@@ -50,6 +50,7 @@
 #define DSC_SDTYPE_DDR		0x02000000
 #define DSC_SDTYPE_DDR2		0x03000000
 #define DSC_SDTYPE_DDR3		0x07000000
+#define DSC_SDTYPE_DDR4		0x05000000
 #define DSC_X32_EN	0x00000020
 
 /* Err_Int_En */
-- 
2.7.4

Re: [Patch v3 07/11] driver/edac/fsl_ddr: Add DDR4 type

[Borislav Petkov]

On Thu, Aug 04, 2016 at 03:58:32PM -0700, York Sun wrote:

<--- Missing commit message.

> Signed-off-by: York Sun <york.sun@nxp.com>
>
> ---
> Change log
>   v3: no change
>   v2: no change
> 
>  drivers/edac/fsl_ddr_edac.c | 12 ++++++++++--
>  drivers/edac/fsl_ddr_edac.h |  1 +
>  2 files changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/edac/fsl_ddr_edac.c b/drivers/edac/fsl_ddr_edac.c
> index 60761c0..88ecf7d 100644
> --- a/drivers/edac/fsl_ddr_edac.c
> +++ b/drivers/edac/fsl_ddr_edac.c
> @@ -376,6 +376,9 @@ static void fsl_ddr_init_csrows(struct mem_ctl_info *mci)
>  		case DSC_SDTYPE_DDR3:

Btw, those DSC_SDTYPE_* defines are used only here to map to the MEM_*
ones. You can just as well use the naked numbers here and drop the
DSC_SDTYPE* ones as it is clear what the naked numbers mean based on how
they're being used.

>  			mtype = MEM_RDDR3;
>  			break;
> +		case DSC_SDTYPE_DDR4:
> +			mtype = MEM_RDDR4;
> +			break;
>  		default:
>  			mtype = MEM_UNKNOWN;
>  			break;
> @@ -391,6 +394,9 @@ static void fsl_ddr_init_csrows(struct mem_ctl_info *mci)
>  		case DSC_SDTYPE_DDR3:
>  			mtype = MEM_DDR3;
>  			break;
> +		case DSC_SDTYPE_DDR4:
> +			mtype = MEM_DDR4;
> +			break;
>  		default:
>  			mtype = MEM_UNKNOWN;
>  			break;
> @@ -495,8 +501,10 @@ int fsl_ddr_mc_err_probe(struct platform_device *op)
>  	}
>  
>  	edac_dbg(3, "init mci\n");
> -	mci->mtype_cap = MEM_FLAG_RDDR | MEM_FLAG_RDDR2 |
> -	    MEM_FLAG_DDR | MEM_FLAG_DDR2;
> +	mci->mtype_cap = MEM_FLAG_DDR | MEM_FLAG_RDDR |
> +			 MEM_FLAG_DDR2 | MEM_FLAG_RDDR2 |
> +			 MEM_FLAG_DDR3 | MEM_FLAG_RDDR3 |

DDR3 is silently added too, you can talk about that in the commit
message, for example.

> +			 MEM_FLAG_DDR4 | MEM_FLAG_RDDR4;
>  	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 556bac5..c7b7dbf 100644
> --- a/drivers/edac/fsl_ddr_edac.h
> +++ b/drivers/edac/fsl_ddr_edac.h
> @@ -50,6 +50,7 @@
>  #define DSC_SDTYPE_DDR		0x02000000
>  #define DSC_SDTYPE_DDR2		0x03000000
>  #define DSC_SDTYPE_DDR3		0x07000000
> +#define DSC_SDTYPE_DDR4		0x05000000
>  #define DSC_X32_EN	0x00000020
>  
>  /* Err_Int_En */
> -- 
> 2.7.4
> 
> 

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--

Re: [Patch v3 07/11] driver/edac/fsl_ddr: Add DDR4 type

[york sun]

On 08/08/2016 01:31 AM, Borislav Petkov wrote:
> On Thu, Aug 04, 2016 at 03:58:32PM -0700, York Sun wrote:
>
> <--- Missing commit message.
>
>> Signed-off-by: York Sun <york.sun@nxp.com>
>>
>> ---
>> Change log
>>   v3: no change
>>   v2: no change
>>
>>  drivers/edac/fsl_ddr_edac.c | 12 ++++++++++--
>>  drivers/edac/fsl_ddr_edac.h |  1 +
>>  2 files changed, 11 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/edac/fsl_ddr_edac.c b/drivers/edac/fsl_ddr_edac.c
>> index 60761c0..88ecf7d 100644
>> --- a/drivers/edac/fsl_ddr_edac.c
>> +++ b/drivers/edac/fsl_ddr_edac.c
>> @@ -376,6 +376,9 @@ static void fsl_ddr_init_csrows(struct mem_ctl_info *mci)
>>  		case DSC_SDTYPE_DDR3:
>
> Btw, those DSC_SDTYPE_* defines are used only here to map to the MEM_*
> ones. You can just as well use the naked numbers here and drop the
> DSC_SDTYPE* ones as it is clear what the naked numbers mean based on how
> they're being used.

OK. Will drop all DSC_SDTYPE_*.

>
>>  			mtype = MEM_RDDR3;
>>  			break;
>> +		case DSC_SDTYPE_DDR4:
>> +			mtype = MEM_RDDR4;
>> +			break;
>>  		default:
>>  			mtype = MEM_UNKNOWN;
>>  			break;
>> @@ -391,6 +394,9 @@ static void fsl_ddr_init_csrows(struct mem_ctl_info *mci)
>>  		case DSC_SDTYPE_DDR3:
>>  			mtype = MEM_DDR3;
>>  			break;
>> +		case DSC_SDTYPE_DDR4:
>> +			mtype = MEM_DDR4;
>> +			break;
>>  		default:
>>  			mtype = MEM_UNKNOWN;
>>  			break;
>> @@ -495,8 +501,10 @@ int fsl_ddr_mc_err_probe(struct platform_device *op)
>>  	}
>>
>>  	edac_dbg(3, "init mci\n");
>> -	mci->mtype_cap = MEM_FLAG_RDDR | MEM_FLAG_RDDR2 |
>> -	    MEM_FLAG_DDR | MEM_FLAG_DDR2;
>> +	mci->mtype_cap = MEM_FLAG_DDR | MEM_FLAG_RDDR |
>> +			 MEM_FLAG_DDR2 | MEM_FLAG_RDDR2 |
>> +			 MEM_FLAG_DDR3 | MEM_FLAG_RDDR3 |
>
> DDR3 is silently added too, you can talk about that in the commit
> message, for example.

Right. That gives me something to say in the commit message.

York

[Patch v3 08/11] driver/edac/fsl_ddr: Add support of little endian

[York Sun]

Get endianness from device tree. Both big endian and little endian
are supported. Default to big endian for backward compatibility to
MPC85xx.

Signed-off-by: York Sun <york.sun@nxp.com>

---
Change log
  v3: no change
  v2: Separated from "Add support for ARM-based SoCs" patch

 .../fsl/ddr.txt}                                   |  6 ++
 drivers/edac/fsl_ddr_edac.c                        | 83 ++++++++++++++--------
 2 files changed, 58 insertions(+), 31 deletions(-)
 rename Documentation/devicetree/bindings/{powerpc/fsl/mem-ctrlr.txt => memory-controllers/fsl/ddr.txt} (74%)

diff --git a/Documentation/devicetree/bindings/powerpc/fsl/mem-ctrlr.txt b/Documentation/devicetree/bindings/memory-controllers/fsl/ddr.txt
similarity index 74%
rename from Documentation/devicetree/bindings/powerpc/fsl/mem-ctrlr.txt
rename to Documentation/devicetree/bindings/memory-controllers/fsl/ddr.txt
index f87856f..62623f9 100644
--- a/Documentation/devicetree/bindings/powerpc/fsl/mem-ctrlr.txt
+++ b/Documentation/devicetree/bindings/memory-controllers/fsl/ddr.txt
@@ -7,6 +7,10 @@ Properties:
 		  "fsl,qoriq-memory-controller".
 - reg		: Address and size of DDR controller registers
 - interrupts	: Error interrupt of DDR controller
+- little-endian	: Specifies little-endian access to registers
+- big-endian	: Specifies big-endian access to registers
+
+If both little-endian and big-endian are omitted, big-endian will be used.
 
 Example 1:
 
@@ -14,6 +18,7 @@ Example 1:
 		compatible = "fsl,bsc9132-memory-controller";
 		reg = <0x2000 0x1000>;
 		interrupts = <16 2 1 8>;
+		big-endian;
 	};
 
 
@@ -24,4 +29,5 @@ Example 2:
 				"fsl,qoriq-memory-controller";
 		reg = <0x8000 0x1000>;
 		interrupts = <16 2 1 23>;
+		big-endian;
 	};
diff --git a/drivers/edac/fsl_ddr_edac.c b/drivers/edac/fsl_ddr_edac.c
index 88ecf7d..b1b7924 100644
--- a/drivers/edac/fsl_ddr_edac.c
+++ b/drivers/edac/fsl_ddr_edac.c
@@ -9,7 +9,6 @@
  * the terms of the GNU General Public License version 2. This program
  * is licensed "as is" without any warranty of any kind, whether express
  * or implied.
- *
  */
 #include <linux/module.h>
 #include <linux/init.h>
@@ -33,6 +32,20 @@ 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(void __iomem *addr)
+{
+	return little_endian ? ioread32(addr) : ioread32be(addr);
+}
+
+static inline void ddr_out32(void __iomem *addr, u32 value)
+{
+	if (little_endian)
+		iowrite32(value, addr);
+	else
+		iowrite32be(value, addr);
+}
 
 /************************ MC SYSFS parts ***********************************/
 
@@ -45,7 +58,7 @@ static ssize_t fsl_ddr_mc_inject_data_hi_show(struct device *dev,
 	struct mem_ctl_info *mci = to_mci(dev);
 	struct fsl_ddr_mc_pdata *pdata = mci->pvt_info;
 	return sprintf(data, "0x%08x",
-		       in_be32(pdata->mc_vbase +
+		       ddr_in32(pdata->mc_vbase +
 			       MC_DATA_ERR_INJECT_HI));
 }
 
@@ -56,7 +69,7 @@ static ssize_t fsl_ddr_mc_inject_data_lo_show(struct device *dev,
 	struct mem_ctl_info *mci = to_mci(dev);
 	struct fsl_ddr_mc_pdata *pdata = mci->pvt_info;
 	return sprintf(data, "0x%08x",
-		       in_be32(pdata->mc_vbase +
+		       ddr_in32(pdata->mc_vbase +
 			       MC_DATA_ERR_INJECT_LO));
 }
 
@@ -67,7 +80,7 @@ static ssize_t fsl_ddr_mc_inject_ctrl_show(struct device *dev,
 	struct mem_ctl_info *mci = to_mci(dev);
 	struct fsl_ddr_mc_pdata *pdata = mci->pvt_info;
 	return sprintf(data, "0x%08x",
-		       in_be32(pdata->mc_vbase + MC_ECC_ERR_INJECT));
+		       ddr_in32(pdata->mc_vbase + MC_ECC_ERR_INJECT));
 }
 
 static ssize_t fsl_ddr_mc_inject_data_hi_store(struct device *dev,
@@ -77,7 +90,7 @@ static ssize_t fsl_ddr_mc_inject_data_hi_store(struct device *dev,
 	struct mem_ctl_info *mci = to_mci(dev);
 	struct fsl_ddr_mc_pdata *pdata = mci->pvt_info;
 	if (isdigit(*data)) {
-		out_be32(pdata->mc_vbase + MC_DATA_ERR_INJECT_HI,
+		ddr_out32(pdata->mc_vbase + MC_DATA_ERR_INJECT_HI,
 			 simple_strtoul(data, NULL, 0));
 		return count;
 	}
@@ -91,7 +104,7 @@ static ssize_t fsl_ddr_mc_inject_data_lo_store(struct device *dev,
 	struct mem_ctl_info *mci = to_mci(dev);
 	struct fsl_ddr_mc_pdata *pdata = mci->pvt_info;
 	if (isdigit(*data)) {
-		out_be32(pdata->mc_vbase + MC_DATA_ERR_INJECT_LO,
+		ddr_out32(pdata->mc_vbase + MC_DATA_ERR_INJECT_LO,
 			 simple_strtoul(data, NULL, 0));
 		return count;
 	}
@@ -105,7 +118,7 @@ static ssize_t fsl_ddr_mc_inject_ctrl_store(struct device *dev,
 	struct mem_ctl_info *mci = to_mci(dev);
 	struct fsl_ddr_mc_pdata *pdata = mci->pvt_info;
 	if (isdigit(*data)) {
-		out_be32(pdata->mc_vbase + MC_ECC_ERR_INJECT,
+		ddr_out32(pdata->mc_vbase + MC_ECC_ERR_INJECT,
 			 simple_strtoul(data, NULL, 0));
 		return count;
 	}
@@ -252,7 +265,7 @@ static void fsl_ddr_mc_check(struct mem_ctl_info *mci)
 	int bad_data_bit;
 	int bad_ecc_bit;
 
-	err_detect = in_be32(pdata->mc_vbase + MC_ERR_DETECT);
+	err_detect = ddr_in32(pdata->mc_vbase + MC_ERR_DETECT);
 	if (!err_detect)
 		return;
 
@@ -261,14 +274,14 @@ static void fsl_ddr_mc_check(struct mem_ctl_info *mci)
 
 	/* no more processing if not ECC bit errors */
 	if (!(err_detect & (DDR_EDE_SBE | DDR_EDE_MBE))) {
-		out_be32(pdata->mc_vbase + MC_ERR_DETECT, err_detect);
+		ddr_out32(pdata->mc_vbase + MC_ERR_DETECT, err_detect);
 		return;
 	}
 
-	syndrome = in_be32(pdata->mc_vbase + MC_CAPTURE_ECC);
+	syndrome = ddr_in32(pdata->mc_vbase + MC_CAPTURE_ECC);
 
 	/* Mask off appropriate bits of syndrome based on bus width */
-	bus_width = (in_be32(pdata->mc_vbase + MC_DDR_SDRAM_CFG) &
+	bus_width = (ddr_in32(pdata->mc_vbase + MC_DDR_SDRAM_CFG) &
 			DSC_DBW_MASK) ? 32 : 64;
 	if (bus_width == 64)
 		syndrome &= 0xff;
@@ -276,8 +289,8 @@ static void fsl_ddr_mc_check(struct mem_ctl_info *mci)
 		syndrome &= 0xffff;
 
 	err_addr = make64(
-		in_be32(pdata->mc_vbase + MC_CAPTURE_EXT_ADDRESS),
-		in_be32(pdata->mc_vbase + MC_CAPTURE_ADDRESS));
+		ddr_in32(pdata->mc_vbase + MC_CAPTURE_EXT_ADDRESS),
+		ddr_in32(pdata->mc_vbase + MC_CAPTURE_ADDRESS));
 	pfn = err_addr >> PAGE_SHIFT;
 
 	for (row_index = 0; row_index < mci->nr_csrows; row_index++) {
@@ -286,8 +299,8 @@ static void fsl_ddr_mc_check(struct mem_ctl_info *mci)
 			break;
 	}
 
-	cap_high = in_be32(pdata->mc_vbase + MC_CAPTURE_DATA_HI);
-	cap_low = in_be32(pdata->mc_vbase + MC_CAPTURE_DATA_LO);
+	cap_high = ddr_in32(pdata->mc_vbase + MC_CAPTURE_DATA_HI);
+	cap_low = ddr_in32(pdata->mc_vbase + MC_CAPTURE_DATA_LO);
 
 	/*
 	 * Analyze single-bit errors on 64-bit wide buses
@@ -333,7 +346,7 @@ static void fsl_ddr_mc_check(struct mem_ctl_info *mci)
 				     row_index, 0, -1,
 				     mci->ctl_name, "");
 
-	out_be32(pdata->mc_vbase + MC_ERR_DETECT, err_detect);
+	ddr_out32(pdata->mc_vbase + MC_ERR_DETECT, err_detect);
 }
 
 static irqreturn_t fsl_ddr_mc_isr(int irq, void *dev_id)
@@ -342,7 +355,7 @@ static irqreturn_t fsl_ddr_mc_isr(int irq, void *dev_id)
 	struct fsl_ddr_mc_pdata *pdata = mci->pvt_info;
 	u32 err_detect;
 
-	err_detect = in_be32(pdata->mc_vbase + MC_ERR_DETECT);
+	err_detect = ddr_in32(pdata->mc_vbase + MC_ERR_DETECT);
 	if (!err_detect)
 		return IRQ_NONE;
 
@@ -362,7 +375,7 @@ static void fsl_ddr_init_csrows(struct mem_ctl_info *mci)
 	u32 cs_bnds;
 	int index;
 
-	sdram_ctl = in_be32(pdata->mc_vbase + MC_DDR_SDRAM_CFG);
+	sdram_ctl = ddr_in32(pdata->mc_vbase + MC_DDR_SDRAM_CFG);
 
 	sdtype = sdram_ctl & DSC_SDTYPE_MASK;
 	if (sdram_ctl & DSC_RD_EN) {
@@ -410,7 +423,7 @@ static void fsl_ddr_init_csrows(struct mem_ctl_info *mci)
 		csrow = mci->csrows[index];
 		dimm = csrow->channels[0]->dimm;
 
-		cs_bnds = in_be32(pdata->mc_vbase + MC_CS_BNDS_0 +
+		cs_bnds = ddr_in32(pdata->mc_vbase + MC_CS_BNDS_0 +
 				  (index * MC_CS_BNDS_OFS));
 
 		start = (cs_bnds & 0xffff0000) >> 16;
@@ -470,6 +483,13 @@ int fsl_ddr_mc_err_probe(struct platform_device *op)
 	mci->ctl_name = pdata->name;
 	mci->dev_name = pdata->name;
 
+	if (of_find_property(op->dev.of_node, "little-endian", NULL))
+		little_endian = true;
+	else if (of_find_property(op->dev.of_node, "big-endian", NULL))
+		little_endian = false;
+	else
+		little_endian = false;
+
 	res = of_address_to_resource(op->dev.of_node, 0, &r);
 	if (res) {
 		pr_err("%s: Unable to get resource for MC err regs\n",
@@ -492,7 +512,7 @@ int fsl_ddr_mc_err_probe(struct platform_device *op)
 		goto err;
 	}
 
-	sdram_ctl = in_be32(pdata->mc_vbase + MC_DDR_SDRAM_CFG);
+	sdram_ctl = ddr_in32(pdata->mc_vbase + MC_DDR_SDRAM_CFG);
 	if (!(sdram_ctl & DSC_ECC_EN)) {
 		/* no ECC */
 		pr_warn("%s: No ECC DIMMs discovered\n", __func__);
@@ -520,11 +540,11 @@ int fsl_ddr_mc_err_probe(struct platform_device *op)
 
 	/* store the original error disable bits */
 	orig_ddr_err_disable =
-	    in_be32(pdata->mc_vbase + MC_ERR_DISABLE);
-	out_be32(pdata->mc_vbase + MC_ERR_DISABLE, 0);
+	    ddr_in32(pdata->mc_vbase + MC_ERR_DISABLE);
+	ddr_out32(pdata->mc_vbase + MC_ERR_DISABLE, 0);
 
 	/* clear all error bits */
-	out_be32(pdata->mc_vbase + MC_ERR_DETECT, ~0);
+	ddr_out32(pdata->mc_vbase + MC_ERR_DETECT, ~0);
 
 	if (edac_mc_add_mc_with_groups(mci, fsl_ddr_dev_groups)) {
 		edac_dbg(3, "failed edac_mc_add_mc()\n");
@@ -532,15 +552,15 @@ int fsl_ddr_mc_err_probe(struct platform_device *op)
 	}
 
 	if (edac_op_state == EDAC_OPSTATE_INT) {
-		out_be32(pdata->mc_vbase + MC_ERR_INT_EN,
-			 DDR_EIE_MBEE | DDR_EIE_SBEE);
+		ddr_out32(pdata->mc_vbase + MC_ERR_INT_EN,
+			  DDR_EIE_MBEE | DDR_EIE_SBEE);
 
 		/* store the original error management threshold */
-		orig_ddr_err_sbe = in_be32(pdata->mc_vbase +
+		orig_ddr_err_sbe = ddr_in32(pdata->mc_vbase +
 					   MC_ERR_SBE) & 0xff0000;
 
 		/* set threshold to 1 error per interrupt */
-		out_be32(pdata->mc_vbase + MC_ERR_SBE, 0x10000);
+		ddr_out32(pdata->mc_vbase + MC_ERR_SBE, 0x10000);
 
 		/* register interrupts */
 		pdata->irq = irq_of_parse_and_map(op->dev.of_node, 0);
@@ -573,6 +593,7 @@ err:
 	edac_mc_free(mci);
 	return res;
 }
+EXPORT_SYMBOL_GPL(fsl_ddr_mc_err_probe);
 
 int fsl_ddr_mc_err_remove(struct platform_device *op)
 {
@@ -582,13 +603,13 @@ int fsl_ddr_mc_err_remove(struct platform_device *op)
 	edac_dbg(0, "\n");
 
 	if (edac_op_state == EDAC_OPSTATE_INT) {
-		out_be32(pdata->mc_vbase + MC_ERR_INT_EN, 0);
 		irq_dispose_mapping(pdata->irq);
+		ddr_out32(pdata->mc_vbase + MC_ERR_INT_EN, 0);
 	}
 
-	out_be32(pdata->mc_vbase + MC_ERR_DISABLE,
-		 orig_ddr_err_disable);
-	out_be32(pdata->mc_vbase + MC_ERR_SBE, orig_ddr_err_sbe);
+	ddr_out32(pdata->mc_vbase + MC_ERR_DISABLE,
+		  orig_ddr_err_disable);
+	ddr_out32(pdata->mc_vbase + MC_ERR_SBE, orig_ddr_err_sbe);
 
 	edac_mc_del_mc(&op->dev);
 	edac_mc_free(mci);
-- 
2.7.4

Re: [Patch v3 08/11] driver/edac/fsl_ddr: Add support of little endian

[Borislav Petkov]

On Thu, Aug 04, 2016 at 03:58:33PM -0700, York Sun wrote:
> Get endianness from device tree. Both big endian and little endian
> are supported. Default to big endian for backward compatibility to
> MPC85xx.
> 
> Signed-off-by: York Sun <york.sun@nxp.com>
> 
> ---
> Change log
>   v3: no change
>   v2: Separated from "Add support for ARM-based SoCs" patch
> 
>  .../fsl/ddr.txt}                                   |  6 ++
>  drivers/edac/fsl_ddr_edac.c                        | 83 ++++++++++++++--------
>  2 files changed, 58 insertions(+), 31 deletions(-)
>  rename Documentation/devicetree/bindings/{powerpc/fsl/mem-ctrlr.txt => memory-controllers/fsl/ddr.txt} (74%)
> 
> diff --git a/Documentation/devicetree/bindings/powerpc/fsl/mem-ctrlr.txt b/Documentation/devicetree/bindings/memory-controllers/fsl/ddr.txt
> similarity index 74%
> rename from Documentation/devicetree/bindings/powerpc/fsl/mem-ctrlr.txt
> rename to Documentation/devicetree/bindings/memory-controllers/fsl/ddr.txt
> index f87856f..62623f9 100644
> --- a/Documentation/devicetree/bindings/powerpc/fsl/mem-ctrlr.txt
> +++ b/Documentation/devicetree/bindings/memory-controllers/fsl/ddr.txt
> @@ -7,6 +7,10 @@ Properties:
>  		  "fsl,qoriq-memory-controller".
>  - reg		: Address and size of DDR controller registers
>  - interrupts	: Error interrupt of DDR controller
> +- little-endian	: Specifies little-endian access to registers
> +- big-endian	: Specifies big-endian access to registers
> +
> +If both little-endian and big-endian are omitted, big-endian will be used.
>  
>  Example 1:
>  
> @@ -14,6 +18,7 @@ Example 1:
>  		compatible = "fsl,bsc9132-memory-controller";
>  		reg = <0x2000 0x1000>;
>  		interrupts = <16 2 1 8>;
> +		big-endian;
>  	};
>  
>  
> @@ -24,4 +29,5 @@ Example 2:
>  				"fsl,qoriq-memory-controller";
>  		reg = <0x8000 0x1000>;
>  		interrupts = <16 2 1 23>;
> +		big-endian;
>  	};

This needs an ACK from DT maintainers. They're on CC.

> diff --git a/drivers/edac/fsl_ddr_edac.c b/drivers/edac/fsl_ddr_edac.c
> index 88ecf7d..b1b7924 100644
> --- a/drivers/edac/fsl_ddr_edac.c
> +++ b/drivers/edac/fsl_ddr_edac.c

...

> @@ -470,6 +483,13 @@ int fsl_ddr_mc_err_probe(struct platform_device *op)
>  	mci->ctl_name = pdata->name;
>  	mci->dev_name = pdata->name;
>  
> +	if (of_find_property(op->dev.of_node, "little-endian", NULL))
> +		little_endian = true;
> +	else if (of_find_property(op->dev.of_node, "big-endian", NULL))
> +		little_endian = false;
> +	else
> +		little_endian = false;

What happened here?

Considering little_endian is static, this should suffice for its
initialization:

	if (of_find_property(op->dev.of_node, "little-endian", NULL))
		little_endian = true;




>  	res = of_address_to_resource(op->dev.of_node, 0, &r);
>  	if (res) {
>  		pr_err("%s: Unable to get resource for MC err regs\n",
> @@ -492,7 +512,7 @@ int fsl_ddr_mc_err_probe(struct platform_device *op)
>  		goto err;
>  	}
>  
> -	sdram_ctl = in_be32(pdata->mc_vbase + MC_DDR_SDRAM_CFG);
> +	sdram_ctl = ddr_in32(pdata->mc_vbase + MC_DDR_SDRAM_CFG);
>  	if (!(sdram_ctl & DSC_ECC_EN)) {
>  		/* no ECC */
>  		pr_warn("%s: No ECC DIMMs discovered\n", __func__);
> @@ -520,11 +540,11 @@ int fsl_ddr_mc_err_probe(struct platform_device *op)
>  
>  	/* store the original error disable bits */
>  	orig_ddr_err_disable =
> -	    in_be32(pdata->mc_vbase + MC_ERR_DISABLE);
> -	out_be32(pdata->mc_vbase + MC_ERR_DISABLE, 0);
> +	    ddr_in32(pdata->mc_vbase + MC_ERR_DISABLE);
> +	ddr_out32(pdata->mc_vbase + MC_ERR_DISABLE, 0);
>  
>  	/* clear all error bits */
> -	out_be32(pdata->mc_vbase + MC_ERR_DETECT, ~0);
> +	ddr_out32(pdata->mc_vbase + MC_ERR_DETECT, ~0);
>  
>  	if (edac_mc_add_mc_with_groups(mci, fsl_ddr_dev_groups)) {
>  		edac_dbg(3, "failed edac_mc_add_mc()\n");
> @@ -532,15 +552,15 @@ int fsl_ddr_mc_err_probe(struct platform_device *op)
>  	}
>  
>  	if (edac_op_state == EDAC_OPSTATE_INT) {
> -		out_be32(pdata->mc_vbase + MC_ERR_INT_EN,
> -			 DDR_EIE_MBEE | DDR_EIE_SBEE);
> +		ddr_out32(pdata->mc_vbase + MC_ERR_INT_EN,
> +			  DDR_EIE_MBEE | DDR_EIE_SBEE);
>  
>  		/* store the original error management threshold */
> -		orig_ddr_err_sbe = in_be32(pdata->mc_vbase +
> +		orig_ddr_err_sbe = ddr_in32(pdata->mc_vbase +
>  					   MC_ERR_SBE) & 0xff0000;
>  
>  		/* set threshold to 1 error per interrupt */
> -		out_be32(pdata->mc_vbase + MC_ERR_SBE, 0x10000);
> +		ddr_out32(pdata->mc_vbase + MC_ERR_SBE, 0x10000);
>  
>  		/* register interrupts */
>  		pdata->irq = irq_of_parse_and_map(op->dev.of_node, 0);
> @@ -573,6 +593,7 @@ err:
>  	edac_mc_free(mci);
>  	return res;
>  }
> +EXPORT_SYMBOL_GPL(fsl_ddr_mc_err_probe);

What's that export for?

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--

Re: [Patch v3 08/11] driver/edac/fsl_ddr: Add support of little endian

[york sun]

On 08/08/2016 01:50 AM, Borislav Petkov wrote:
> On Thu, Aug 04, 2016 at 03:58:33PM -0700, York Sun wrote:
>> Get endianness from device tree. Both big endian and little endian
>> are supported. Default to big endian for backward compatibility to
>> MPC85xx.
>>
>> Signed-off-by: York Sun <york.sun@nxp.com>
>>
>> ---
>> Change log
>>   v3: no change
>>   v2: Separated from "Add support for ARM-based SoCs" patch
>>
>>  .../fsl/ddr.txt}                                   |  6 ++
>>  drivers/edac/fsl_ddr_edac.c                        | 83 ++++++++++++++--------
>>  2 files changed, 58 insertions(+), 31 deletions(-)
>>  rename Documentation/devicetree/bindings/{powerpc/fsl/mem-ctrlr.txt => memory-controllers/fsl/ddr.txt} (74%)
>>
>> diff --git a/Documentation/devicetree/bindings/powerpc/fsl/mem-ctrlr.txt b/Documentation/devicetree/bindings/memory-controllers/fsl/ddr.txt
>> similarity index 74%
>> rename from Documentation/devicetree/bindings/powerpc/fsl/mem-ctrlr.txt
>> rename to Documentation/devicetree/bindings/memory-controllers/fsl/ddr.txt
>> index f87856f..62623f9 100644
>> --- a/Documentation/devicetree/bindings/powerpc/fsl/mem-ctrlr.txt
>> +++ b/Documentation/devicetree/bindings/memory-controllers/fsl/ddr.txt
>> @@ -7,6 +7,10 @@ Properties:
>>  		  "fsl,qoriq-memory-controller".
>>  - reg		: Address and size of DDR controller registers
>>  - interrupts	: Error interrupt of DDR controller
>> +- little-endian	: Specifies little-endian access to registers
>> +- big-endian	: Specifies big-endian access to registers
>> +
>> +If both little-endian and big-endian are omitted, big-endian will be used.
>>
>>  Example 1:
>>
>> @@ -14,6 +18,7 @@ Example 1:
>>  		compatible = "fsl,bsc9132-memory-controller";
>>  		reg = <0x2000 0x1000>;
>>  		interrupts = <16 2 1 8>;
>> +		big-endian;
>>  	};
>>
>>
>> @@ -24,4 +29,5 @@ Example 2:
>>  				"fsl,qoriq-memory-controller";
>>  		reg = <0x8000 0x1000>;
>>  		interrupts = <16 2 1 23>;
>> +		big-endian;
>>  	};
>
> This needs an ACK from DT maintainers. They're on CC.
>
>> diff --git a/drivers/edac/fsl_ddr_edac.c b/drivers/edac/fsl_ddr_edac.c
>> index 88ecf7d..b1b7924 100644
>> --- a/drivers/edac/fsl_ddr_edac.c
>> +++ b/drivers/edac/fsl_ddr_edac.c
>
> ...
>
>> @@ -470,6 +483,13 @@ int fsl_ddr_mc_err_probe(struct platform_device *op)
>>  	mci->ctl_name = pdata->name;
>>  	mci->dev_name = pdata->name;
>>
>> +	if (of_find_property(op->dev.of_node, "little-endian", NULL))
>> +		little_endian = true;
>> +	else if (of_find_property(op->dev.of_node, "big-endian", NULL))
>> +		little_endian = false;
>> +	else
>> +		little_endian = false;
>
> What happened here?
>
> Considering little_endian is static, this should suffice for its
> initialization:
>
> 	if (of_find_property(op->dev.of_node, "little-endian", NULL))
> 		little_endian = true;
>
>

OK.

>
>
>>  	res = of_address_to_resource(op->dev.of_node, 0, &r);
>>  	if (res) {
>>  		pr_err("%s: Unable to get resource for MC err regs\n",
>> @@ -492,7 +512,7 @@ int fsl_ddr_mc_err_probe(struct platform_device *op)
>>  		goto err;
>>  	}
>>
>> -	sdram_ctl = in_be32(pdata->mc_vbase + MC_DDR_SDRAM_CFG);
>> +	sdram_ctl = ddr_in32(pdata->mc_vbase + MC_DDR_SDRAM_CFG);
>>  	if (!(sdram_ctl & DSC_ECC_EN)) {
>>  		/* no ECC */
>>  		pr_warn("%s: No ECC DIMMs discovered\n", __func__);
>> @@ -520,11 +540,11 @@ int fsl_ddr_mc_err_probe(struct platform_device *op)
>>
>>  	/* store the original error disable bits */
>>  	orig_ddr_err_disable =
>> -	    in_be32(pdata->mc_vbase + MC_ERR_DISABLE);
>> -	out_be32(pdata->mc_vbase + MC_ERR_DISABLE, 0);
>> +	    ddr_in32(pdata->mc_vbase + MC_ERR_DISABLE);
>> +	ddr_out32(pdata->mc_vbase + MC_ERR_DISABLE, 0);
>>
>>  	/* clear all error bits */
>> -	out_be32(pdata->mc_vbase + MC_ERR_DETECT, ~0);
>> +	ddr_out32(pdata->mc_vbase + MC_ERR_DETECT, ~0);
>>
>>  	if (edac_mc_add_mc_with_groups(mci, fsl_ddr_dev_groups)) {
>>  		edac_dbg(3, "failed edac_mc_add_mc()\n");
>> @@ -532,15 +552,15 @@ int fsl_ddr_mc_err_probe(struct platform_device *op)
>>  	}
>>
>>  	if (edac_op_state == EDAC_OPSTATE_INT) {
>> -		out_be32(pdata->mc_vbase + MC_ERR_INT_EN,
>> -			 DDR_EIE_MBEE | DDR_EIE_SBEE);
>> +		ddr_out32(pdata->mc_vbase + MC_ERR_INT_EN,
>> +			  DDR_EIE_MBEE | DDR_EIE_SBEE);
>>
>>  		/* store the original error management threshold */
>> -		orig_ddr_err_sbe = in_be32(pdata->mc_vbase +
>> +		orig_ddr_err_sbe = ddr_in32(pdata->mc_vbase +
>>  					   MC_ERR_SBE) & 0xff0000;
>>
>>  		/* set threshold to 1 error per interrupt */
>> -		out_be32(pdata->mc_vbase + MC_ERR_SBE, 0x10000);
>> +		ddr_out32(pdata->mc_vbase + MC_ERR_SBE, 0x10000);
>>
>>  		/* register interrupts */
>>  		pdata->irq = irq_of_parse_and_map(op->dev.of_node, 0);
>> @@ -573,6 +593,7 @@ err:
>>  	edac_mc_free(mci);
>>  	return res;
>>  }
>> +EXPORT_SYMBOL_GPL(fsl_ddr_mc_err_probe);
>
> What's that export for?
>

I thought I need to export this symbol so it can be call from a module 
driver. Actually I don't. Will remove.

York

Re: [Patch v3 08/11] driver/edac/fsl_ddr: Add support of little endian

[Mark Rutland]

On Thu, Aug 04, 2016 at 03:58:33PM -0700, York Sun wrote:
> Get endianness from device tree. Both big endian and little endian
> are supported. Default to big endian for backward compatibility to
> MPC85xx.
> 
> Signed-off-by: York Sun <york.sun@nxp.com>
> 
> ---
> Change log
>   v3: no change
>   v2: Separated from "Add support for ARM-based SoCs" patch
> 
>  .../fsl/ddr.txt}                                   |  6 ++
>  drivers/edac/fsl_ddr_edac.c                        | 83 ++++++++++++++--------
>  2 files changed, 58 insertions(+), 31 deletions(-)
>  rename Documentation/devicetree/bindings/{powerpc/fsl/mem-ctrlr.txt => memory-controllers/fsl/ddr.txt} (74%)
> 
> diff --git a/Documentation/devicetree/bindings/powerpc/fsl/mem-ctrlr.txt b/Documentation/devicetree/bindings/memory-controllers/fsl/ddr.txt
> similarity index 74%
> rename from Documentation/devicetree/bindings/powerpc/fsl/mem-ctrlr.txt
> rename to Documentation/devicetree/bindings/memory-controllers/fsl/ddr.txt
> index f87856f..62623f9 100644
> --- a/Documentation/devicetree/bindings/powerpc/fsl/mem-ctrlr.txt
> +++ b/Documentation/devicetree/bindings/memory-controllers/fsl/ddr.txt
> @@ -7,6 +7,10 @@ Properties:
>  		  "fsl,qoriq-memory-controller".
>  - reg		: Address and size of DDR controller registers
>  - interrupts	: Error interrupt of DDR controller
> +- little-endian	: Specifies little-endian access to registers
> +- big-endian	: Specifies big-endian access to registers
> +
> +If both little-endian and big-endian are omitted, big-endian will be used.

The binding looks fine.

> @@ -470,6 +483,13 @@ int fsl_ddr_mc_err_probe(struct platform_device *op)
>  	mci->ctl_name = pdata->name;
>  	mci->dev_name = pdata->name;
>  
> +	if (of_find_property(op->dev.of_node, "little-endian", NULL))

Use:	if (of_property_read_bool(dev.of_node, "little-endian"))

> +		little_endian = true;
> +	else if (of_find_property(op->dev.of_node, "big-endian", NULL))
	
Use:	else if (of_property_read_bool(dev.of_node, "big-endian"))

> +		little_endian = false;
> +	else
> +		little_endian = false;
> +

With those fixed up:

Acked-by: Mark Rutland <mark.rutland@arm.com>

Thanks,
Mark.

[Patch v3 09/11] driver/edac/fsl_ddr: Fix kernel warning when module is removed

[York Sun]

When compiled as a module, removing this module causes kernel
warnings when irq_dispose_mapping() is called. Instead of calling
irq_of_parse_and_map(), using platform_get_irq() to acquire the
IRQ number.

Signed-off-by: York Sun <york.sun@nxp.com>

---
Change log
  v3: no change
  v2: no change

 drivers/edac/fsl_ddr_edac.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/edac/fsl_ddr_edac.c b/drivers/edac/fsl_ddr_edac.c
index b1b7924..a3fc8fe 100644
--- a/drivers/edac/fsl_ddr_edac.c
+++ b/drivers/edac/fsl_ddr_edac.c
@@ -563,7 +563,7 @@ int fsl_ddr_mc_err_probe(struct platform_device *op)
 		ddr_out32(pdata->mc_vbase + MC_ERR_SBE, 0x10000);
 
 		/* register interrupts */
-		pdata->irq = irq_of_parse_and_map(op->dev.of_node, 0);
+		pdata->irq = platform_get_irq(op, 0);
 		res = devm_request_irq(&op->dev, pdata->irq,
 				       fsl_ddr_mc_isr,
 				       IRQF_SHARED,
@@ -571,7 +571,6 @@ int fsl_ddr_mc_err_probe(struct platform_device *op)
 		if (res < 0) {
 			pr_err("%s: Unable to request irq %d for FSL DDR DRAM ERR\n",
 			       __func__, pdata->irq);
-			irq_dispose_mapping(pdata->irq);
 			res = -ENODEV;
 			goto err2;
 		}
@@ -603,7 +602,6 @@ int fsl_ddr_mc_err_remove(struct platform_device *op)
 	edac_dbg(0, "\n");
 
 	if (edac_op_state == EDAC_OPSTATE_INT) {
-		irq_dispose_mapping(pdata->irq);
 		ddr_out32(pdata->mc_vbase + MC_ERR_INT_EN, 0);
 	}
 
-- 
2.7.4

[Patch v3 10/11] driver/edac/layerscape_edac: Add Layerscape EDAC support

[York Sun]

Add DDR EDAC for ARM-based compatible controllers. Both big-endian
and little-endian are supported.

Signed-off-by: York Sun <york.sun@nxp.com>

---
Change log
  v3: no change
  v2: Create new driver using shared DDR object

 arch/arm64/Kconfig.platforms           |  1 +
 arch/{arm => arm64}/include/asm/edac.h | 21 +++++------
 arch/arm64/include/asm/irq.h           |  4 ++
 drivers/edac/Kconfig                   |  7 ++++
 drivers/edac/Makefile                  |  3 ++
 drivers/edac/fsl_ddr_edac.c            |  1 +
 drivers/edac/layerscape_edac.c         | 67 ++++++++++++++++++++++++++++++++++
 7 files changed, 92 insertions(+), 12 deletions(-)
 copy arch/{arm => arm64}/include/asm/edac.h (79%)
 create mode 100644 drivers/edac/layerscape_edac.c

diff --git a/arch/arm64/Kconfig.platforms b/arch/arm64/Kconfig.platforms
index 7ef1d05..185a215 100644
--- a/arch/arm64/Kconfig.platforms
+++ b/arch/arm64/Kconfig.platforms
@@ -41,6 +41,7 @@ config ARCH_EXYNOS
 
 config ARCH_LAYERSCAPE
 	bool "ARMv8 based Freescale Layerscape SoC family"
+	select EDAC_SUPPORT
 	help
 	  This enables support for the Freescale Layerscape SoC family.
 
diff --git a/arch/arm/include/asm/edac.h b/arch/arm64/include/asm/edac.h
similarity index 79%
copy from arch/arm/include/asm/edac.h
copy to arch/arm64/include/asm/edac.h
index 5189fa8..36a226c 100644
--- a/arch/arm/include/asm/edac.h
+++ b/arch/arm64/include/asm/edac.h
@@ -18,16 +18,15 @@
 #define ASM_EDAC_H
 /*
  * ECC atomic, DMA, SMP and interrupt safe scrub function.
- * Implements the per arch edac_atomic_scrub() that EDAC use for software
+ * Implements the per arch atomic_scrub() that EDAC use for software
  * ECC scrubbing.  It reads memory and then writes back the original
  * value, allowing the hardware to detect and correct memory errors.
  */
-
-static inline void edac_atomic_scrub(void *va, u32 size)
+static inline void atomic_scrub(void *va, u32 size)
 {
-#if __LINUX_ARM_ARCH__ >= 6
-	unsigned int *virt_addr = va;
-	unsigned int temp, temp2;
+	unsigned long *virt_addr = va;
+	unsigned long temp;
+	unsigned int temp2;
 	unsigned int i;
 
 	for (i = 0; i < size / sizeof(*virt_addr); i++, virt_addr++) {
@@ -35,15 +34,13 @@ static inline void edac_atomic_scrub(void *va, u32 size)
 		 * so we are interrupt, DMA and SMP safe.
 		 */
 		__asm__ __volatile__("\n"
-			"1:	ldrex	%0, [%2]\n"
-			"	strex	%1, %0, [%2]\n"
-			"	teq	%1, #0\n"
-			"	bne	1b\n"
+			"1:	ldxr	%0, [%2]\n"
+			"	stxr	%w1, %0, [%2]\n"
+			"	cbnz	%w1, 1b\n"
 			: "=&r"(temp), "=&r"(temp2)
 			: "r"(virt_addr)
-			: "cc");
+			: "memory");
 	}
-#endif
 }
 
 #endif
diff --git a/arch/arm64/include/asm/irq.h b/arch/arm64/include/asm/irq.h
index b77197d..d09c008 100644
--- a/arch/arm64/include/asm/irq.h
+++ b/arch/arm64/include/asm/irq.h
@@ -11,6 +11,10 @@
 #include <asm-generic/irq.h>
 #include <asm/thread_info.h>
 
+#ifndef NO_IRQ
+#define NO_IRQ	((unsigned int)(-1))
+#endif
+
 struct pt_regs;
 
 DECLARE_PER_CPU(unsigned long [IRQ_STACK_SIZE/sizeof(long)], irq_stack);
diff --git a/drivers/edac/Kconfig b/drivers/edac/Kconfig
index 6ca7474..f1ac4e2 100644
--- a/drivers/edac/Kconfig
+++ b/drivers/edac/Kconfig
@@ -258,6 +258,13 @@ config EDAC_MPC85XX
 	  Support for error detection and correction on the Freescale
 	  MPC8349, MPC8560, MPC8540, MPC8548, T4240
 
+config EDAC_LAYERSCAPE
+	tristate "Freescale Layerscape DDR"
+	depends on EDAC_MM_EDAC && ARCH_LAYERSCAPE
+	help
+	  Support for error detection and correction on Freescale memory
+	  controllers on Layerscape SoCs.
+
 config EDAC_MV64X60
 	tristate "Marvell MV64x60"
 	depends on EDAC_MM_EDAC && MV64X60
diff --git a/drivers/edac/Makefile b/drivers/edac/Makefile
index ee047a4..910dc83 100644
--- a/drivers/edac/Makefile
+++ b/drivers/edac/Makefile
@@ -54,6 +54,9 @@ obj-$(CONFIG_EDAC_PASEMI)		+= pasemi_edac.o
 mpc85xx_edac_mod-y			:= fsl_ddr_edac.o mpc85xx_edac.o
 obj-$(CONFIG_EDAC_MPC85XX)		+= mpc85xx_edac_mod.o
 
+layerscape_edac_mod-y			:= fsl_ddr_edac.o layerscape_edac.o
+obj-$(CONFIG_EDAC_LAYERSCAPE)		+= layerscape_edac_mod.o
+
 obj-$(CONFIG_EDAC_MV64X60)		+= mv64x60_edac.o
 obj-$(CONFIG_EDAC_CELL)			+= cell_edac.o
 obj-$(CONFIG_EDAC_PPC4XX)		+= ppc4xx_edac.o
diff --git a/drivers/edac/fsl_ddr_edac.c b/drivers/edac/fsl_ddr_edac.c
index a3fc8fe..d4b910d 100644
--- a/drivers/edac/fsl_ddr_edac.c
+++ b/drivers/edac/fsl_ddr_edac.c
@@ -22,6 +22,7 @@
 
 #include <linux/of_platform.h>
 #include <linux/of_device.h>
+#include <linux/of_address.h>
 #include "edac_module.h"
 #include "edac_core.h"
 #include "fsl_ddr_edac.h"
diff --git a/drivers/edac/layerscape_edac.c b/drivers/edac/layerscape_edac.c
new file mode 100644
index 0000000..6935e22
--- /dev/null
+++ b/drivers/edac/layerscape_edac.c
@@ -0,0 +1,67 @@
+/*
+ * Freescale Memory Controller kernel module
+ *
+ * Derived from mpc85xx_edac.c
+ * Author: Dave Jiang <djiang@mvista.com>
+ *
+ * 2006-2007 (c) MontaVista Software, Inc. This file is licensed under
+ * the terms of the GNU General Public License version 2. This program
+ * is licensed "as is" without any warranty of any kind, whether express
+ * or implied.
+ */
+
+#include "edac_core.h"
+#include "fsl_ddr_edac.h"
+
+static const struct of_device_id fsl_ddr_mc_err_of_match[] = {
+	{ .compatible = "fsl,qoriq-memory-controller", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, fsl_ddr_mc_err_of_match);
+
+static struct platform_driver fsl_ddr_mc_err_driver = {
+	.probe = fsl_ddr_mc_err_probe,
+	.remove = fsl_ddr_mc_err_remove,
+	.driver = {
+		.name = "fsl_ddr_mc_err",
+		.of_match_table = fsl_ddr_mc_err_of_match,
+	},
+};
+
+static int __init fsl_ddr_mc_init(void)
+{
+	int res = 0;
+
+	/* make sure error reporting method is sane */
+	switch (edac_op_state) {
+	case EDAC_OPSTATE_POLL:
+	case EDAC_OPSTATE_INT:
+		break;
+	default:
+		edac_op_state = EDAC_OPSTATE_INT;
+		break;
+	}
+
+	res = platform_driver_register(&fsl_ddr_mc_err_driver);
+	if (res) {
+		pr_err("Layerscape EDAC: MC fails to register\n");
+		return res;
+	}
+
+	return 0;
+}
+
+module_init(fsl_ddr_mc_init);
+
+static void __exit fsl_ddr_mc_exit(void)
+{
+	platform_driver_unregister(&fsl_ddr_mc_err_driver);
+}
+
+module_exit(fsl_ddr_mc_exit);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Montavista Software, Inc.");
+module_param(edac_op_state, int, 0444);
+MODULE_PARM_DESC(edac_op_state,
+		 "EDAC Error Reporting state: 0=Poll, 2=Interrupt");
-- 
2.7.4

Re: [Patch v3 10/11] driver/edac/layerscape_edac: Add Layerscape EDAC support

[Alexander Stein]

On Thursday 04 August 2016 15:58:35, York Sun wrote:
> Add DDR EDAC for ARM-based compatible controllers. Both big-endian
> and little-endian are supported.
> 
> Signed-off-by: York Sun <york.sun@nxp.com>
> 
> ---
> Change log
>   v3: no change
>   v2: Create new driver using shared DDR object
> 
>  arch/arm64/Kconfig.platforms           |  1 +
>  arch/{arm => arm64}/include/asm/edac.h | 21 +++++------
>  arch/arm64/include/asm/irq.h           |  4 ++
>  drivers/edac/Kconfig                   |  7 ++++
>  drivers/edac/Makefile                  |  3 ++
>  drivers/edac/fsl_ddr_edac.c            |  1 +
>  drivers/edac/layerscape_edac.c         | 67
> ++++++++++++++++++++++++++++++++++ 7 files changed, 92 insertions(+), 12
> deletions(-)
>  copy arch/{arm => arm64}/include/asm/edac.h (79%)
>  create mode 100644 drivers/edac/layerscape_edac.c

This only adds EDAC support for ARM64. What would be necessary for ARM 
support, e.g. LS1021A?

Best regards,
Alexander

Re: [Patch v3 10/11] driver/edac/layerscape_edac: Add Layerscape EDAC support

[york sun]

On 08/08/2016 01:57 AM, Alexander Stein wrote:
> On Thursday 04 August 2016 15:58:35, York Sun wrote:
>> Add DDR EDAC for ARM-based compatible controllers. Both big-endian
>> and little-endian are supported.
>>
>> Signed-off-by: York Sun <york.sun@nxp.com>
>>
>> ---
>> Change log
>>   v3: no change
>>   v2: Create new driver using shared DDR object
>>
>>  arch/arm64/Kconfig.platforms           |  1 +
>>  arch/{arm => arm64}/include/asm/edac.h | 21 +++++------
>>  arch/arm64/include/asm/irq.h           |  4 ++
>>  drivers/edac/Kconfig                   |  7 ++++
>>  drivers/edac/Makefile                  |  3 ++
>>  drivers/edac/fsl_ddr_edac.c            |  1 +
>>  drivers/edac/layerscape_edac.c         | 67
>> ++++++++++++++++++++++++++++++++++ 7 files changed, 92 insertions(+), 12
>> deletions(-)
>>  copy arch/{arm => arm64}/include/asm/edac.h (79%)
>>  create mode 100644 drivers/edac/layerscape_edac.c
>
> This only adds EDAC support for ARM64. What would be necessary for ARM
> support, e.g. LS1021A?
>

It should be just one step away. I haven't got a chance to verify/debug 
it on LS1021A.

York

Re: [Patch v3 10/11] driver/edac/layerscape_edac: Add Layerscape EDAC support

[Marc Zyngier]

On Thu, 4 Aug 2016 15:58:35 -0700
York Sun <york.sun@nxp.com> wrote:

> Add DDR EDAC for ARM-based compatible controllers. Both big-endian
> and little-endian are supported.
> 
> Signed-off-by: York Sun <york.sun@nxp.com>
> 
> ---
> Change log
>   v3: no change
>   v2: Create new driver using shared DDR object
> 
>  arch/arm64/Kconfig.platforms           |  1 +
>  arch/{arm => arm64}/include/asm/edac.h | 21 +++++------
>  arch/arm64/include/asm/irq.h           |  4 ++
>  drivers/edac/Kconfig                   |  7 ++++
>  drivers/edac/Makefile                  |  3 ++
>  drivers/edac/fsl_ddr_edac.c            |  1 +
>  drivers/edac/layerscape_edac.c         | 67 ++++++++++++++++++++++++++++++++++
>  7 files changed, 92 insertions(+), 12 deletions(-)
>  copy arch/{arm => arm64}/include/asm/edac.h (79%)
>  create mode 100644 drivers/edac/layerscape_edac.c
> 
> diff --git a/arch/arm64/Kconfig.platforms b/arch/arm64/Kconfig.platforms
> index 7ef1d05..185a215 100644
> --- a/arch/arm64/Kconfig.platforms
> +++ b/arch/arm64/Kconfig.platforms
> @@ -41,6 +41,7 @@ config ARCH_EXYNOS
>  
>  config ARCH_LAYERSCAPE
>  	bool "ARMv8 based Freescale Layerscape SoC family"
> +	select EDAC_SUPPORT
>  	help
>  	  This enables support for the Freescale Layerscape SoC family.
>  
> diff --git a/arch/arm/include/asm/edac.h b/arch/arm64/include/asm/edac.h
> similarity index 79%
> copy from arch/arm/include/asm/edac.h
> copy to arch/arm64/include/asm/edac.h
> index 5189fa8..36a226c 100644
> --- a/arch/arm/include/asm/edac.h
> +++ b/arch/arm64/include/asm/edac.h
> @@ -18,16 +18,15 @@
>  #define ASM_EDAC_H
>  /*
>   * ECC atomic, DMA, SMP and interrupt safe scrub function.
> - * Implements the per arch edac_atomic_scrub() that EDAC use for software
> + * Implements the per arch atomic_scrub() that EDAC use for software
>   * ECC scrubbing.  It reads memory and then writes back the original
>   * value, allowing the hardware to detect and correct memory errors.
>   */
> -
> -static inline void edac_atomic_scrub(void *va, u32 size)
> +static inline void atomic_scrub(void *va, u32 size)
>  {
> -#if __LINUX_ARM_ARCH__ >= 6
> -	unsigned int *virt_addr = va;
> -	unsigned int temp, temp2;
> +	unsigned long *virt_addr = va;
> +	unsigned long temp;
> +	unsigned int temp2;
>  	unsigned int i;
>  
>  	for (i = 0; i < size / sizeof(*virt_addr); i++, virt_addr++) {
> @@ -35,15 +34,13 @@ static inline void edac_atomic_scrub(void *va, u32 size)
>  		 * so we are interrupt, DMA and SMP safe.
>  		 */
>  		__asm__ __volatile__("\n"
> -			"1:	ldrex	%0, [%2]\n"
> -			"	strex	%1, %0, [%2]\n"
> -			"	teq	%1, #0\n"
> -			"	bne	1b\n"
> +			"1:	ldxr	%0, [%2]\n"
> +			"	stxr	%w1, %0, [%2]\n"
> +			"	cbnz	%w1, 1b\n"

Hiding architecture code in a driver, are we? Hmmm.

Also, how can this be safe if you have non cache-coherent DMA going on?

>  			: "=&r"(temp), "=&r"(temp2)
>  			: "r"(virt_addr)
> -			: "cc");
> +			: "memory");
>  	}
> -#endif
>  }
>  
>  #endif
> diff --git a/arch/arm64/include/asm/irq.h b/arch/arm64/include/asm/irq.h
> index b77197d..d09c008 100644
> --- a/arch/arm64/include/asm/irq.h
> +++ b/arch/arm64/include/asm/irq.h
> @@ -11,6 +11,10 @@
>  #include <asm-generic/irq.h>
>  #include <asm/thread_info.h>
>  
> +#ifndef NO_IRQ
> +#define NO_IRQ	((unsigned int)(-1))
> +#endif

NAK.

NO_IRQ shouldn't exist at all, and if it had to exist, it would have
the value 0. This really is a leftover of a distant past that has no
real relevance on recent architectures.

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny.

Re: [Patch v3 10/11] driver/edac/layerscape_edac: Add Layerscape EDAC support

[york sun]

On 08/08/2016 11:07 AM, Marc Zyngier wrote:
> On Thu, 4 Aug 2016 15:58:35 -0700
> York Sun <york.sun@nxp.com> wrote:
>
>> Add DDR EDAC for ARM-based compatible controllers. Both big-endian
>> and little-endian are supported.
>>
>> Signed-off-by: York Sun <york.sun@nxp.com>
>>
>> ---
>> Change log
>>   v3: no change
>>   v2: Create new driver using shared DDR object
>>
>>  arch/arm64/Kconfig.platforms           |  1 +
>>  arch/{arm => arm64}/include/asm/edac.h | 21 +++++------
>>  arch/arm64/include/asm/irq.h           |  4 ++
>>  drivers/edac/Kconfig                   |  7 ++++
>>  drivers/edac/Makefile                  |  3 ++
>>  drivers/edac/fsl_ddr_edac.c            |  1 +
>>  drivers/edac/layerscape_edac.c         | 67 ++++++++++++++++++++++++++++++++++
>>  7 files changed, 92 insertions(+), 12 deletions(-)
>>  copy arch/{arm => arm64}/include/asm/edac.h (79%)
>>  create mode 100644 drivers/edac/layerscape_edac.c
>>
>> diff --git a/arch/arm64/Kconfig.platforms b/arch/arm64/Kconfig.platforms
>> index 7ef1d05..185a215 100644
>> --- a/arch/arm64/Kconfig.platforms
>> +++ b/arch/arm64/Kconfig.platforms
>> @@ -41,6 +41,7 @@ config ARCH_EXYNOS
>>
>>  config ARCH_LAYERSCAPE
>>  	bool "ARMv8 based Freescale Layerscape SoC family"
>> +	select EDAC_SUPPORT
>>  	help
>>  	  This enables support for the Freescale Layerscape SoC family.
>>
>> diff --git a/arch/arm/include/asm/edac.h b/arch/arm64/include/asm/edac.h
>> similarity index 79%
>> copy from arch/arm/include/asm/edac.h
>> copy to arch/arm64/include/asm/edac.h
>> index 5189fa8..36a226c 100644
>> --- a/arch/arm/include/asm/edac.h
>> +++ b/arch/arm64/include/asm/edac.h
>> @@ -18,16 +18,15 @@
>>  #define ASM_EDAC_H
>>  /*
>>   * ECC atomic, DMA, SMP and interrupt safe scrub function.
>> - * Implements the per arch edac_atomic_scrub() that EDAC use for software
>> + * Implements the per arch atomic_scrub() that EDAC use for software
>>   * ECC scrubbing.  It reads memory and then writes back the original
>>   * value, allowing the hardware to detect and correct memory errors.
>>   */
>> -
>> -static inline void edac_atomic_scrub(void *va, u32 size)
>> +static inline void atomic_scrub(void *va, u32 size)
>>  {
>> -#if __LINUX_ARM_ARCH__ >= 6
>> -	unsigned int *virt_addr = va;
>> -	unsigned int temp, temp2;
>> +	unsigned long *virt_addr = va;
>> +	unsigned long temp;
>> +	unsigned int temp2;
>>  	unsigned int i;
>>
>>  	for (i = 0; i < size / sizeof(*virt_addr); i++, virt_addr++) {
>> @@ -35,15 +34,13 @@ static inline void edac_atomic_scrub(void *va, u32 size)
>>  		 * so we are interrupt, DMA and SMP safe.
>>  		 */
>>  		__asm__ __volatile__("\n"
>> -			"1:	ldrex	%0, [%2]\n"
>> -			"	strex	%1, %0, [%2]\n"
>> -			"	teq	%1, #0\n"
>> -			"	bne	1b\n"
>> +			"1:	ldxr	%0, [%2]\n"
>> +			"	stxr	%w1, %0, [%2]\n"
>> +			"	cbnz	%w1, 1b\n"
>
> Hiding architecture code in a driver, are we? Hmmm.

This adds atomic_scrub() for arm64. Arm already has this function. Are 
you suggesting to separate this part as an individual patch?
>
> Also, how can this be safe if you have non cache-coherent DMA going on?

I am clueless on this topic. What's your suggestion.

>
>>  			: "=&r"(temp), "=&r"(temp2)
>>  			: "r"(virt_addr)
>> -			: "cc");
>> +			: "memory");
>>  	}
>> -#endif
>>  }
>>
>>  #endif
>> diff --git a/arch/arm64/include/asm/irq.h b/arch/arm64/include/asm/irq.h
>> index b77197d..d09c008 100644
>> --- a/arch/arm64/include/asm/irq.h
>> +++ b/arch/arm64/include/asm/irq.h
>> @@ -11,6 +11,10 @@
>>  #include <asm-generic/irq.h>
>>  #include <asm/thread_info.h>
>>
>> +#ifndef NO_IRQ
>> +#define NO_IRQ	((unsigned int)(-1))
>> +#endif
>
> NAK.
>
> NO_IRQ shouldn't exist at all, and if it had to exist, it would have
> the value 0. This really is a leftover of a distant past that has no
> real relevance on recent architectures.
>

OK. I will drop the NO_IRQ in the driver.

York

Re: [Patch v3 10/11] driver/edac/layerscape_edac: Add Layerscape EDAC support

[Will Deacon]

On Mon, Aug 08, 2016 at 07:56:04PM +0000, york sun wrote:
> On 08/08/2016 11:07 AM, Marc Zyngier wrote:
> > On Thu, 4 Aug 2016 15:58:35 -0700
> > York Sun <york.sun@nxp.com> wrote:
> >
> >> Add DDR EDAC for ARM-based compatible controllers. Both big-endian
> >> and little-endian are supported.
> >>
> >> Signed-off-by: York Sun <york.sun@nxp.com>
> >>
> >> ---
> >> Change log
> >>   v3: no change
> >>   v2: Create new driver using shared DDR object
> >>
> >>  arch/arm64/Kconfig.platforms           |  1 +
> >>  arch/{arm => arm64}/include/asm/edac.h | 21 +++++------
> >>  arch/arm64/include/asm/irq.h           |  4 ++
> >>  drivers/edac/Kconfig                   |  7 ++++
> >>  drivers/edac/Makefile                  |  3 ++
> >>  drivers/edac/fsl_ddr_edac.c            |  1 +
> >>  drivers/edac/layerscape_edac.c         | 67 ++++++++++++++++++++++++++++++++++
> >>  7 files changed, 92 insertions(+), 12 deletions(-)
> >>  copy arch/{arm => arm64}/include/asm/edac.h (79%)
> >>  create mode 100644 drivers/edac/layerscape_edac.c
> >>
> >> diff --git a/arch/arm64/Kconfig.platforms b/arch/arm64/Kconfig.platforms
> >> index 7ef1d05..185a215 100644
> >> --- a/arch/arm64/Kconfig.platforms
> >> +++ b/arch/arm64/Kconfig.platforms
> >> @@ -41,6 +41,7 @@ config ARCH_EXYNOS
> >>
> >>  config ARCH_LAYERSCAPE
> >>  	bool "ARMv8 based Freescale Layerscape SoC family"
> >> +	select EDAC_SUPPORT
> >>  	help
> >>  	  This enables support for the Freescale Layerscape SoC family.
> >>
> >> diff --git a/arch/arm/include/asm/edac.h b/arch/arm64/include/asm/edac.h
> >> similarity index 79%
> >> copy from arch/arm/include/asm/edac.h
> >> copy to arch/arm64/include/asm/edac.h
> >> index 5189fa8..36a226c 100644
> >> --- a/arch/arm/include/asm/edac.h
> >> +++ b/arch/arm64/include/asm/edac.h
> >> @@ -18,16 +18,15 @@
> >>  #define ASM_EDAC_H
> >>  /*
> >>   * ECC atomic, DMA, SMP and interrupt safe scrub function.
> >> - * Implements the per arch edac_atomic_scrub() that EDAC use for software
> >> + * Implements the per arch atomic_scrub() that EDAC use for software
> >>   * ECC scrubbing.  It reads memory and then writes back the original
> >>   * value, allowing the hardware to detect and correct memory errors.
> >>   */
> >> -
> >> -static inline void edac_atomic_scrub(void *va, u32 size)
> >> +static inline void atomic_scrub(void *va, u32 size)
> >>  {
> >> -#if __LINUX_ARM_ARCH__ >= 6
> >> -	unsigned int *virt_addr = va;
> >> -	unsigned int temp, temp2;
> >> +	unsigned long *virt_addr = va;
> >> +	unsigned long temp;
> >> +	unsigned int temp2;
> >>  	unsigned int i;
> >>
> >>  	for (i = 0; i < size / sizeof(*virt_addr); i++, virt_addr++) {
> >> @@ -35,15 +34,13 @@ static inline void edac_atomic_scrub(void *va, u32 size)
> >>  		 * so we are interrupt, DMA and SMP safe.
> >>  		 */
> >>  		__asm__ __volatile__("\n"
> >> -			"1:	ldrex	%0, [%2]\n"
> >> -			"	strex	%1, %0, [%2]\n"
> >> -			"	teq	%1, #0\n"
> >> -			"	bne	1b\n"
> >> +			"1:	ldxr	%0, [%2]\n"
> >> +			"	stxr	%w1, %0, [%2]\n"
> >> +			"	cbnz	%w1, 1b\n"
> >
> > Hiding architecture code in a driver, are we? Hmmm.
> 
> This adds atomic_scrub() for arm64. Arm already has this function. Are 
> you suggesting to separate this part as an individual patch?
> >
> > Also, how can this be safe if you have non cache-coherent DMA going on?
> 
> I am clueless on this topic. What's your suggestion.

We've discussed this in the past for ARMv7 and the bottom line is that
it's not safe to scrub memory backing non-coherent DMA buffers. You will
likely end up with data corruption if you do that.

The two options are:

  (1) Restrict scrubbing to non-DMA memory, or
  (2) Only enable scrubbing if the system is completely cache-coherent

The problem with (1) is that we can't detect that, particularly when
the streaming DMA API is in use.

The problem with (2) is that you can have a coherent system that decides
dynamically to drop in and out of coherency as a result of, e.g. PCI
NoSnoop transactions.

Will

Re: [Patch v3 10/11] driver/edac/layerscape_edac: Add Layerscape EDAC support

[york sun]

On 08/09/2016 04:12 AM, Will Deacon wrote:
> On Mon, Aug 08, 2016 at 07:56:04PM +0000, york sun wrote:
>> On 08/08/2016 11:07 AM, Marc Zyngier wrote:
>>> On Thu, 4 Aug 2016 15:58:35 -0700
>>> York Sun <york.sun@nxp.com> wrote:
>>>
>>>> Add DDR EDAC for ARM-based compatible controllers. Both big-endian
>>>> and little-endian are supported.
>>>>
>>>> Signed-off-by: York Sun <york.sun@nxp.com>
>>>>
>>>> ---
>>>> Change log
>>>>   v3: no change
>>>>   v2: Create new driver using shared DDR object
>>>>
>>>>  arch/arm64/Kconfig.platforms           |  1 +
>>>>  arch/{arm => arm64}/include/asm/edac.h | 21 +++++------
>>>>  arch/arm64/include/asm/irq.h           |  4 ++
>>>>  drivers/edac/Kconfig                   |  7 ++++
>>>>  drivers/edac/Makefile                  |  3 ++
>>>>  drivers/edac/fsl_ddr_edac.c            |  1 +
>>>>  drivers/edac/layerscape_edac.c         | 67 ++++++++++++++++++++++++++++++++++
>>>>  7 files changed, 92 insertions(+), 12 deletions(-)
>>>>  copy arch/{arm => arm64}/include/asm/edac.h (79%)
>>>>  create mode 100644 drivers/edac/layerscape_edac.c
>>>>
>>>> diff --git a/arch/arm64/Kconfig.platforms b/arch/arm64/Kconfig.platforms
>>>> index 7ef1d05..185a215 100644
>>>> --- a/arch/arm64/Kconfig.platforms
>>>> +++ b/arch/arm64/Kconfig.platforms
>>>> @@ -41,6 +41,7 @@ config ARCH_EXYNOS
>>>>
>>>>  config ARCH_LAYERSCAPE
>>>>  	bool "ARMv8 based Freescale Layerscape SoC family"
>>>> +	select EDAC_SUPPORT
>>>>  	help
>>>>  	  This enables support for the Freescale Layerscape SoC family.
>>>>
>>>> diff --git a/arch/arm/include/asm/edac.h b/arch/arm64/include/asm/edac.h
>>>> similarity index 79%
>>>> copy from arch/arm/include/asm/edac.h
>>>> copy to arch/arm64/include/asm/edac.h
>>>> index 5189fa8..36a226c 100644
>>>> --- a/arch/arm/include/asm/edac.h
>>>> +++ b/arch/arm64/include/asm/edac.h
>>>> @@ -18,16 +18,15 @@
>>>>  #define ASM_EDAC_H
>>>>  /*
>>>>   * ECC atomic, DMA, SMP and interrupt safe scrub function.
>>>> - * Implements the per arch edac_atomic_scrub() that EDAC use for software
>>>> + * Implements the per arch atomic_scrub() that EDAC use for software
>>>>   * ECC scrubbing.  It reads memory and then writes back the original
>>>>   * value, allowing the hardware to detect and correct memory errors.
>>>>   */
>>>> -
>>>> -static inline void edac_atomic_scrub(void *va, u32 size)
>>>> +static inline void atomic_scrub(void *va, u32 size)
>>>>  {
>>>> -#if __LINUX_ARM_ARCH__ >= 6
>>>> -	unsigned int *virt_addr = va;
>>>> -	unsigned int temp, temp2;
>>>> +	unsigned long *virt_addr = va;
>>>> +	unsigned long temp;
>>>> +	unsigned int temp2;
>>>>  	unsigned int i;
>>>>
>>>>  	for (i = 0; i < size / sizeof(*virt_addr); i++, virt_addr++) {
>>>> @@ -35,15 +34,13 @@ static inline void edac_atomic_scrub(void *va, u32 size)
>>>>  		 * so we are interrupt, DMA and SMP safe.
>>>>  		 */
>>>>  		__asm__ __volatile__("\n"
>>>> -			"1:	ldrex	%0, [%2]\n"
>>>> -			"	strex	%1, %0, [%2]\n"
>>>> -			"	teq	%1, #0\n"
>>>> -			"	bne	1b\n"
>>>> +			"1:	ldxr	%0, [%2]\n"
>>>> +			"	stxr	%w1, %0, [%2]\n"
>>>> +			"	cbnz	%w1, 1b\n"
>>>
>>> Hiding architecture code in a driver, are we? Hmmm.
>>
>> This adds atomic_scrub() for arm64. Arm already has this function. Are
>> you suggesting to separate this part as an individual patch?
>>>
>>> Also, how can this be safe if you have non cache-coherent DMA going on?
>>
>> I am clueless on this topic. What's your suggestion.
>
> We've discussed this in the past for ARMv7 and the bottom line is that
> it's not safe to scrub memory backing non-coherent DMA buffers. You will
> likely end up with data corruption if you do that.
>
> The two options are:
>
>   (1) Restrict scrubbing to non-DMA memory, or
>   (2) Only enable scrubbing if the system is completely cache-coherent
>
> The problem with (1) is that we can't detect that, particularly when
> the streaming DMA API is in use.
>
> The problem with (2) is that you can have a coherent system that decides
> dynamically to drop in and out of coherency as a result of, e.g. PCI
> NoSnoop transactions.
>

Good information! I realize I added this code on an older kernel and 
rebased to the latest one. This code is actually not needed, as long as 
CONFIG_EDAC_ATOMIC_SCRUB.

You said this has been discussed for ARMv7, what was the conclusion? I 
see edac_atomic_scrub() is still used for ARM.

York

[Patch v3 11/11] arm64: Update device tree for Layerscape SoCs

[York Sun]

Add DDR memory controller nodes to enable EDAC driver.

Signed-off-by: York Sun <york.sun@nxp.com>

---
Change log
  v3: no change
  v2: no change

 arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi |  7 +++++++
 arch/arm64/boot/dts/freescale/fsl-ls2080a.dtsi | 14 ++++++++++++++
 2 files changed, 21 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi b/arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi
index de0323b..cb33f23 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi
+++ b/arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi
@@ -196,6 +196,13 @@
 			bus-width = <4>;
 		};
 
+		ddr: memory-controller@1080000 {
+			compatible = "fsl,qoriq-memory-controller";
+			reg = <0x0 0x1080000 0x0 0x1000>;
+			interrupts = <0 144 0x4>;
+			big-endian;
+		};
+
 		dspi0: dspi@2100000 {
 			compatible = "fsl,ls1043a-dspi", "fsl,ls1021a-v1.0-dspi";
 			#address-cells = <1>;
diff --git a/arch/arm64/boot/dts/freescale/fsl-ls2080a.dtsi b/arch/arm64/boot/dts/freescale/fsl-ls2080a.dtsi
index 3187c82..3221e5c 100644
--- a/arch/arm64/boot/dts/freescale/fsl-ls2080a.dtsi
+++ b/arch/arm64/boot/dts/freescale/fsl-ls2080a.dtsi
@@ -689,4 +689,18 @@
 			interrupts = <0 12 4>;
 		};
 	};
+
+	ddr1: memory-controller@1080000 {
+		compatible = "fsl,qoriq-memory-controller";
+		reg = <0x0 0x1080000 0x0 0x1000>;
+		interrupts = <0 17 0x4>;
+		little-endian;
+	};
+
+	ddr2: memory-controller@1090000 {
+		compatible = "fsl,qoriq-memory-controller";
+		reg = <0x0 0x1090000 0x0 0x1000>;
+		interrupts = <0 18 0x4>;
+		little-endian;
+	};
 };
-- 
2.7.4