From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754673AbcHWQrl (ORCPT ); Tue, 23 Aug 2016 12:47:41 -0400 Received: from mail-ve1eur01on0069.outbound.protection.outlook.com ([104.47.1.69]:18085 "EHLO EUR01-VE1-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1752325AbcHWQrP (ORCPT ); Tue, 23 Aug 2016 12:47:15 -0400 From: york sun To: Borislav Petkov CC: "linux-edac@vger.kernel.org" , "morbidrsa@gmail.com" , "oss@buserror.net" , Stuart Yoder , Catalin Marinas , Will Deacon , Doug Thompson , "mchehab@kernel.org" , "linux-arm-kernel@lists.infradead.org" , "linux-kernel@vger.kernel.org" Subject: Re: [Patch v4 8/9] driver/edac/layerscape_edac: Add Layerscape EDAC support Thread-Topic: [Patch v4 8/9] driver/edac/layerscape_edac: Add Layerscape EDAC support Thread-Index: AQHR8okL/GKUb2eF/UKmItyOozUKJg== Date: Tue, 23 Aug 2016 16:46:10 +0000 Message-ID: References: <1470779760-16483-1-git-send-email-york.sun@nxp.com> <1470779760-16483-9-git-send-email-york.sun@nxp.com> <20160812091248.GA333@nazgul.tnic> Reply-To: york sun Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: spf=none (sender IP is ) smtp.mailfrom=york.sun@nxp.com; x-originating-ip: [192.88.168.49] x-ms-office365-filtering-correlation-id: 737e42ca-8e21-4206-528b-08d3cb74fcb4 x-microsoft-exchange-diagnostics: 1;HE1PR0401MB2635;6:TiCDx0O0ELOYBzoqlrZoTPqex6mCe8jD1rjThKiZNNAtP3PEeliGrgDjfSuwhoq8JNCq/l15T4cGI2uY3MtuJKW/n+rQijLwzoAlFgR02/RRIiwC8s9HTSe/q/kpIM512gp8/IWHXXGLDm2x5+T8b+U7bYgSK2uZUWQZQsvuuy8ylAQ/Pxjd37ydheieanDE9JLwYcdY9kc1F9qsyiPQK3fwETLOcpHjMrVkegfbM1WKOV/mA+6QddWORSq4fL+Pw+ZCG8FmHx/b+yvHJZmzyVAOuniLBvaq3zw2Ywf7f7B1/ZmNPIDaCiYJeikIx+BitZB8q6LacXZLY7uu5Rt2GA==;5:jkfLzIDQ53tLNK1dayV5A4GkSekvdr7/R03nWIqi/U+QvCSUn17t1Y0gUprKNkbhRGmvjSRSHQAT8n5jkcihfJzN2XUFQUbWmtvMN055ldEbdWoSw/dHZmpQDvLHwyB+tFu+ItMawU0+gvP4DJxFCw==;24:/y2ru5bHdbhEZB0cmVtSN7XAEnf/t1uPMOVvYOPrHuqfvojBEjP4Tas3EYR2LH6J7VdLoF5WL8OwveRgsZxRpEFA9QIpJUCAeZgui+POQAA=;7:D1itIjiSfMoGJfMm+gwF3XkrvvXPjw34WGd3gReruBJhRw/uUvadWLzFrlF5icXkMXXYVy35UXNIjRyYr5w0+lYtJzNlB1ckH4XJF7dEHnuK6AVQNvIzBt9PjOo1P9ndxRKIckVcKRvtgrMG0/+xFgrpfXirhmb3s5yqcxjLyUQlmRats2C720rc/1Fbvv0WNaaf6vbPuwXexJx43HgsJz24ULusjXAI7XcLlsLSBuao+fLjZ4zixDGzjf14+teQ x-microsoft-antispam: UriScan:;BCL:0;PCL:0;RULEID:;SRVR:HE1PR0401MB2635; x-nips-smtp-inbound: xFSL-EDGE x-microsoft-antispam-prvs: x-exchange-antispam-report-test: UriScan:(185117386973197)(211171220733660); x-exchange-antispam-report-cfa-test: BCL:0;PCL:0;RULEID:(6040176)(601004)(2401047)(5005006)(8121501046)(3002001)(10201501046)(6055026);SRVR:HE1PR0401MB2635;BCL:0;PCL:0;RULEID:;SRVR:HE1PR0401MB2635; x-forefront-prvs: 004395A01C x-forefront-antispam-report: SFV:NSPM;SFS:(10009020)(6009001)(7916002)(377454003)(189002)(54534003)(24454002)(61684002)(199003)(4326007)(81166006)(8676002)(3660700001)(81156014)(101416001)(50986999)(105586002)(2900100001)(3450700001)(8936002)(76176999)(53806999)(54356999)(87936001)(106356001)(3280700002)(5660300001)(33656002)(106116001)(102836003)(86152002)(3846002)(6116002)(5002640100001)(86362001)(586003)(66066001)(7696003)(7846002)(110136002)(7736002)(77096005)(7416002)(68736007)(76576001)(97736004)(305945005)(2906002)(19580405001)(9686002)(19580395003)(43066003)(74316002)(189998001)(122556002)(10400500002);DIR:OUT;SFP:1101;SCL:1;SRVR:HE1PR0401MB2635;H:AM4PR0401MB1732.eurprd04.prod.outlook.com;FPR:;SPF:None;PTR:InfoNoRecords;MX:1;A:1;LANG:en; spamdiagnosticoutput: 1:99 spamdiagnosticmetadata: NSPM Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 X-OriginatorOrg: nxp.com X-MS-Exchange-CrossTenant-originalarrivaltime: 23 Aug 2016 16:46:10.4501 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 686ea1d3-bc2b-4c6f-a92c-d99c5c301635 X-MS-Exchange-Transport-CrossTenantHeadersStamped: HE1PR0401MB2635 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Transfer-Encoding: 8bit X-MIME-Autoconverted: from quoted-printable to 8bit by mail.home.local id u7NGlkQ2024197 On 08/12/2016 02:13 AM, Borislav Petkov wrote: > On Tue, Aug 09, 2016 at 02:55:45PM -0700, York Sun wrote: >> Add DDR EDAC for ARM-based compatible controllers. Both big-endian >> and little-endian are supported, as specified in device tree. >> >> Signed-off-by: York Sun >> >> --- >> Change log >> v4: Drop adding atomic_scrub() for arm64 >> Drop NO_IRQ >> v3: no change >> v2: Create new driver using shared DDR object >> >> arch/arm64/Kconfig.platforms | 1 + >> drivers/edac/Kconfig | 7 +++++ >> drivers/edac/Makefile | 3 ++ >> drivers/edac/fsl_ddr_edac.c | 2 +- >> drivers/edac/layerscape_edac.c | 67 ++++++++++++++++++++++++++++++++++++++++++ >> 5 files changed, 79 insertions(+), 1 deletion(-) >> 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/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 d8ce1f6..afade14 100644 >> --- a/drivers/edac/fsl_ddr_edac.c >> +++ b/drivers/edac/fsl_ddr_edac.c >> @@ -26,6 +26,7 @@ >> >> #include >> #include >> +#include >> #include "edac_module.h" >> #include "edac_core.h" >> #include "fsl_ddr_edac.h" >> @@ -478,7 +479,6 @@ int fsl_mc_err_probe(struct platform_device *op) >> >> pdata = mci->pvt_info; >> pdata->name = "fsl_mc_err"; >> - pdata->irq = NO_IRQ; >> mci->pdev = &op->dev; >> pdata->edac_idx = edac_mc_idx++; >> dev_set_drvdata(mci->pdev, mci); >> diff --git a/drivers/edac/layerscape_edac.c b/drivers/edac/layerscape_edac.c >> new file mode 100644 >> index 0000000..6ba771d >> --- /dev/null >> +++ b/drivers/edac/layerscape_edac.c >> @@ -0,0 +1,67 @@ >> +/* >> + * Freescale Memory Controller kernel module >> + * >> + * Derived from mpc85xx_edac.c > > Up to here is fine... > >> + * Author: Dave Jiang >> + * >> + * 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. >> + */ > > ... but I'm not sure about this. This is a new driver which you are > adding and not Dave. You probably could say > > "Author: York Sun > (c) NXP ... > > Derived from mpc85xx_edac.c, (c) Montavista ..." > > so that we have both copyright tags in there. > Sorry for the late response. I was out of office last week. Will respin this patch. >> + >> +#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_mc_err_probe, >> + .remove = fsl_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; > > No need to init it since it is going to be overwritten anyway. OK. > >> + >> + /* 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"); > > This is done with pr_fmt(). Grep the tree for examples. > > You could define EDAC_MOD_STR like the rest of the drivers do. > Will fix. > > >> + 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."); > > See above. Will fix. > >> +module_param(edac_op_state, int, 0444); >> +MODULE_PARM_DESC(edac_op_state, >> + "EDAC Error Reporting state: 0=Poll, 2=Interrupt"); >> -- > York