Re: [PATCH 2/4] EDAC, skx_edac: Delete the duplicated codes

[Borislav Petkov]

On Fri, Jan 25, 2019 at 11:59:00AM -0800, Tony Luck wrote:
> From: Qiuxu Zhuo <qiuxu.zhuo@xxxxxxxxx>
> 
> Part of the skx_edac driver codes are carved to the skx_comm_edac.c
> file, which are used by both skx_edac driver and i10nm_edac driver.
> Delete the duplicated codes from the skx_edac.c file and rename
> skx_edac.c to skx_base.c. Update the Makefile to build the skx_edac
> driver from skx_base.c plux skx_comm_edac.c
> 
> Co-developed-by: Tony Luck <tony.luck@xxxxxxxxx>
> Signed-off-by: Qiuxu Zhuo <qiuxu.zhuo@xxxxxxxxx>
> Signed-off-by: Tony Luck <tony.luck@xxxxxxxxx>
> ---
>  drivers/edac/Makefile   |    4 +-
>  drivers/edac/skx_base.c |  673 +++++++++++++++++++
>  drivers/edac/skx_edac.c | 1358 ---------------------------------------
>  3 files changed, 676 insertions(+), 1359 deletions(-)
>  create mode 100644 drivers/edac/skx_base.c
>  delete mode 100644 drivers/edac/skx_edac.c

Please integrate scripts/checkpatch.pl into your patch creation
workflow. Some of the warnings/errors *actually* make sense.

Sensible warnings:

WARNING: Missing or malformed SPDX-License-Identifier tag in line 1
#65: FILE: drivers/edac/skx_base.c:1:
+/*

WARNING: Use #include <linux/processor.h> instead of <asm/processor.h>
#97: FILE: drivers/edac/skx_base.c:33:
+#include <asm/processor.h>

> diff --git a/drivers/edac/Makefile b/drivers/edac/Makefile
> index 716096d08ea0..1764b7ed05db 100644
> --- a/drivers/edac/Makefile
> +++ b/drivers/edac/Makefile
> @@ -30,7 +30,6 @@ obj-$(CONFIG_EDAC_I5400)		+= i5400_edac.o
>  obj-$(CONFIG_EDAC_I7300)		+= i7300_edac.o
>  obj-$(CONFIG_EDAC_I7CORE)		+= i7core_edac.o
>  obj-$(CONFIG_EDAC_SBRIDGE)		+= sb_edac.o
> -obj-$(CONFIG_EDAC_SKX)			+= skx_edac.o
>  obj-$(CONFIG_EDAC_PND2)			+= pnd2_edac.o
>  obj-$(CONFIG_EDAC_E7XXX)		+= e7xxx_edac.o
>  obj-$(CONFIG_EDAC_E752X)		+= e752x_edac.o
> @@ -58,6 +57,9 @@ 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
>  
> +skx_edac-y				:= skx_comm_edac.o skx_base.o
> +obj-$(CONFIG_EDAC_SKX)			+= skx_edac.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/skx_base.c b/drivers/edac/skx_base.c
> new file mode 100644
> index 000000000000..4aa53c15b89b
> --- /dev/null
> +++ b/drivers/edac/skx_base.c
> @@ -0,0 +1,673 @@
> +/*
> + * EDAC driver for Intel(R) Xeon(R) Skylake processors
> + * Copyright (c) 2016, Intel Corporation.
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
> + * more details.
> + */

First patch adds SPDX headers - this one has the old licensing thing. Pls fix.


> +
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/acpi.h>
> +#include <linux/dmi.h>
> +#include <linux/pci.h>
> +#include <linux/pci_ids.h>
> +#include <linux/slab.h>
> +#include <linux/delay.h>
> +#include <linux/edac.h>
> +#include <linux/mmzone.h>
> +#include <linux/smp.h>
> +#include <linux/bitmap.h>
> +#include <linux/math64.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/adxl.h>
> +#include <acpi/nfit.h>
> +#include <asm/cpu_device_id.h>
> +#include <asm/intel-family.h>
> +#include <asm/processor.h>
> +#include <asm/mce.h>

*all* those includes are needed?!

> +
> +#include "edac_module.h"
> +#include "skx_comm_edac.h"

This one has some of the above includes too.

...

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.