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@xxxxxxx>
>>
>> ---
>> 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