* [PATCH] edac: xgene: fix cpuid abuse
@ 2015-06-01 22:09 Loc Ho
[not found] ` <1433196575-9455-1-git-send-email-lho-qTEPVZfXA3Y@public.gmane.org>
0 siblings, 1 reply; 5+ messages in thread
From: Loc Ho @ 2015-06-01 22:09 UTC (permalink / raw)
To: dougthompson-aS9lmoZGLiVWk0Htik3J/w, bp-Gina5bIWoIWzQB+pC5nmwQ,
mchehab-JPH+aEBZ4P+UEJcrhfAQsw, arnd-r2nGTMty4D4,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg
Cc: linux-edac-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
jcm-H+wXaHxf7aLQT0dZR+AlfA, patches-qTEPVZfXA3Y, Loc Ho
The new x-gene EDAC driver incorrectly tried to figure out the version of
one of its IP blocks by looking at the version of the CPU core, which is
only vagely related.
This removes the incorrect code and instead uses the version of the IP
block in the compatible string where it belongs.
Found using build testing on x86, which does not provide the arm64
cpuid interface.
This version is based on Arnd original version.
Signed-off-by: Loc Ho <lho-qTEPVZfXA3Y@public.gmane.org>
---
.../devicetree/bindings/edac/apm-xgene-edac.txt | 3 +-
drivers/edac/xgene_edac.c | 55 +++-----------------
2 files changed, 10 insertions(+), 48 deletions(-)
diff --git a/Documentation/devicetree/bindings/edac/apm-xgene-edac.txt b/Documentation/devicetree/bindings/edac/apm-xgene-edac.txt
index 480911c..78edb80 100644
--- a/Documentation/devicetree/bindings/edac/apm-xgene-edac.txt
+++ b/Documentation/devicetree/bindings/edac/apm-xgene-edac.txt
@@ -25,7 +25,8 @@ Required properties for memory controller subnode:
- memory-controller : Instance number of the memory controller.
Required properties for PMD subnode:
-- compatible : Shall be "apm,xgene-edac-pmd".
+- compatible : Shall be "apm,xgene-edac-pmd" or
+ "apm,xgene-edac-pmd-v2".
- reg : First resource shall be the PMD resource.
- pmd-controller : Instance number of the PMD controller.
diff --git a/drivers/edac/xgene_edac.c b/drivers/edac/xgene_edac.c
index b515857..14636e4 100644
--- a/drivers/edac/xgene_edac.c
+++ b/drivers/edac/xgene_edac.c
@@ -523,6 +523,7 @@ struct xgene_edac_pmd_ctx {
struct edac_device_ctl_info *edac_dev;
void __iomem *pmd_csr;
u32 pmd;
+ int version;
};
static void xgene_edac_pmd_l1_check(struct edac_device_ctl_info *edac_dev,
@@ -784,50 +785,6 @@ static void xgene_edac_pmd_cpu_hw_cfg(struct edac_device_ctl_info *edac_dev,
writel(0x00000101, pg_f + MEMERR_CPU_MMUECR_PAGE_OFFSET);
}
-static bool xgene_edac_pmd_l2c_version1(void)
-{
- /* Check all chips with PMD L2C version 1 HW */
- #define REVIDR_MINOR_REV(revidr) ((revidr) & 0x00000007)
-
- switch (MIDR_VARIANT(read_cpuid_id())) {
- case 0:
- switch (MIDR_REVISION(read_cpuid_id())) {
- case 0:
-
- switch (REVIDR_MINOR_REV(read_cpuid(REVIDR_EL1))) {
- case 1:
- case 2:
- return true;
- };
- break;
- case 1:
- if (REVIDR_MINOR_REV(read_cpuid(REVIDR_EL1)) == 1)
- return true;
- break;
- }
- break;
- case 1:
- switch (MIDR_REVISION(read_cpuid_id())) {
- case 0:
- switch (REVIDR_MINOR_REV(read_cpuid(REVIDR_EL1))) {
- case 1:
- return true;
- };
- break;
- case 1:
- switch (REVIDR_MINOR_REV(read_cpuid(REVIDR_EL1))) {
- case 1:
- case 0:
- return true;
- };
- break;
- }
- break;
- }
-
- return false;
-}
-
static void xgene_edac_pmd_hw_cfg(struct edac_device_ctl_info *edac_dev)
{
struct xgene_edac_pmd_ctx *ctx = edac_dev->pvt_info;
@@ -837,7 +794,7 @@ static void xgene_edac_pmd_hw_cfg(struct edac_device_ctl_info *edac_dev)
/* Enable PMD memory error - MEMERR_L2C_L2ECR and L2C_L2RTOCR */
writel(0x00000703, pg_e + MEMERR_L2C_L2ECR_PAGE_OFFSET);
/* Configure L2C HW request time out feature if supported */
- if (!xgene_edac_pmd_l2c_version1())
+ if (ctx->version > 1)
writel(0x00000119, pg_d + CPUX_L2C_L2RTOCR_PAGE_OFFSET);
}
@@ -956,7 +913,8 @@ static int xgene_edac_pmd_available(u32 efuse, int pmd)
return (efuse & (1 << pmd)) ? 0 : 1;
}
-static int xgene_edac_pmd_add(struct xgene_edac *edac, struct device_node *np)
+static int xgene_edac_pmd_add(struct xgene_edac *edac, struct device_node *np,
+ int version)
{
struct edac_device_ctl_info *edac_dev;
struct xgene_edac_pmd_ctx *ctx;
@@ -998,6 +956,7 @@ static int xgene_edac_pmd_add(struct xgene_edac *edac, struct device_node *np)
ctx->edac = edac;
ctx->edac_dev = edac_dev;
ctx->ddev = *edac->dev;
+ ctx->version = version;
edac_dev->dev = &ctx->ddev;
edac_dev->ctl_name = ctx->name;
edac_dev->dev_name = ctx->name;
@@ -1169,7 +1128,9 @@ static int xgene_edac_probe(struct platform_device *pdev)
if (of_device_is_compatible(child, "apm,xgene-edac-mc"))
xgene_edac_mc_add(edac, child);
if (of_device_is_compatible(child, "apm,xgene-edac-pmd"))
- xgene_edac_pmd_add(edac, child);
+ xgene_edac_pmd_add(edac, child, 1);
+ if (of_device_is_compatible(child, "apm,xgene-edac-pmd-v2"))
+ xgene_edac_pmd_add(edac, child, 2);
}
return 0;
--
1.7.1
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH v11 0/4] edac: Add APM X-Gene SoC EDAC driver
@ 2015-05-22 23:32 Loc Ho
2015-06-01 15:11 ` [PATCH v11 4/5] " Catalin Marinas
0 siblings, 1 reply; 5+ messages in thread
From: Loc Ho @ 2015-05-22 23:32 UTC (permalink / raw)
To: dougthompson-aS9lmoZGLiVWk0Htik3J/w, bp-Gina5bIWoIWzQB+pC5nmwQ,
mchehab-JPH+aEBZ4P+UEJcrhfAQsw, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
mark.rutland-5wv7dgnIgG8, ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg
Cc: linux-edac-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
jcm-H+wXaHxf7aLQT0dZR+AlfA, patches-qTEPVZfXA3Y, Loc Ho
This patch adds support for the APM X-Gene SoC EDAC driver for DT.
v11:
* Update DT binding documnetation with additional memory controller and PMD
controller instance number
* Move global MC variables to EDAC top level structure
* Move EDAC support functions to top of file
* Inlined some EDAC support functions
* Remove define of CONFIG_EDAC_DEBUG where it is possible
* Update driver to reflect additional memory controller and PMD controller
properties in DT
* Update DT with additional memory controller and PMD instance number
* Drop the stub scrub function for ARM64. You will need an EDAC code update
to compile.
v10:
* Move regmap for CSW, MCBA, MCBB, and efuse to top driver node and add
accessor support functions
* Change retrieval OF address proerty for subnode to use of_address_to_resource
* Add COMPILE_TEST to Kconfig for EDAC_XGENE
* Update DT binding documentation and DT node accordingly
* Remove DT node naming and number from nodes
v9:
* Update maintainer section
* Update binding documentation to reflect update DT binding
* Restructure the EDAC driver such that its includes subnode for MC, PMD's,
and future L3's and SoC.
* Update DT node to reflect update DT binding
* NOTE: Temporary removed L3 and SoC. They will be added back later on.
v8:
* Change ASM_EDAC_H to __ASM_EDAC_H in file edac.h
* Add WARN_ONCE in stub function atomic_scrub
* Update DTS binding documentation (with only memory controller node)
* Temporary remove L1/L2, L3, and SoC driver code and update memory driver
code accordingly
v7:
* Update binding documentation for memory controller, PMD, L3, and SoC EDAC
* Change resource for PCP, CSW, MCBA, MCBB, and efuse to syscon type nodes
and updae driver accordingly
* Fix clearing L2 RTO register properly
* Fix the MODULE_DEVICE_TABLE for OF
* Update DT accordingly to binding documentation
v6:
* Rebase to 4.0.0-rc3
* Add memory scrub stub function and enable ARM64 EDAC support patch
* Add bit definition defines for L2RTOS registers
* Remove un-necessary clearing of all L1/L2 software generated registers
* Remove wrong notification of LSU un-correctable error
* Change L2 reporting of un-correcable error to correctable error
* Change clearing of the L2C L2RTO registers
* Add support for L2 HW version 1 and version 2 or above
* Add support for L3 HW version 1 and version 2 or above
v5:
* Rebase to 3.17.rc1 (next)
* Update binding documentation for additional SoC node binding resource
* Enable MCU correctable and uncorrectable interrupts if not enabled by
firmware
* Enable top level interrupt only after all MCU registered. Otherwise,
error interrupt will never get cleared by the corresponding MCU.
* Remove clearing of L1 and L2 errors during initialization time. Otherwise,
they will not be captured between firmware booting and error configuration.
* Add capture and clearing SoC register bus errors
* Add register bus resource to SoC DT node
v4:
* Fix PMD l1/l2 error reading address due to wrong variable type
* Fix clearing of software generated and HW errors for l1/l2
v3:
* Update binding documentation for PMD DT node and exampples
* Add binding documentation for SoC DT node
* Change MC, PMD, and L3C driver error injection to use debugfs
* Add missing IRQ for MC correctable error (code and DT)
* Use true/false where appropriate instead 1/0
* Add bit definition for L1 MMUESR register and fully decode this error
* Remove the un-necessary dev variable from xgene_edac_pmd_ctx structure
* Add check for disabled PMD (code and DT)
* Switch to edac_printk instead pr_err
* Some minor comments update
v2:
* Add EDAC entry in MAINTAINERS for APM EDAC driver
* Remove the MC scrub patch
* Remove the word 'Caches' from Kconfig
* Change all MASK defines to use BIT(x)
* Update comment or remove them
* Wrap error injection code around CONFIG_EDAC_DEBUG
* Change function name xgene_edac_mc_hw_init to xgene_edac_mc_irq_ctl
* Change all function XXX_hw_init to XXX_hw_ctl
* Fix typo 'activie'
* Move calling function edac_mc_alloc after resource retrieval
* Check for NULL on platform_get_resource return if reference directly
* Add documentation for struct xgene_edac_pmd_ctx
* Move L1 and L2 check out of function xgene_edac_pmd_check to its own
functions
* Use for loop for configure each CPU of an PMD
* Replace /2 by >> 1
* Remove unnecessary comment on edac_device_add_device failure
* Make mem_err_ip static const
* Unwind EDAC register correctly if failed
---
Loc Ho (5):
arm64: Enable EDAC on ARM64
MAINTAINERS: Add entry for APM X-Gene SoC EDAC driver
Documentation: Add documentation for the APM X-Gene SoC EDAC DTS
binding
edac: Add APM X-Gene SoC EDAC driver
arm64: Add APM X-Gene SoC EDAC DTS entries
.../devicetree/bindings/edac/apm-xgene-edac.txt | 78 ++
MAINTAINERS | 7 +
arch/arm64/Kconfig | 1 +
arch/arm64/boot/dts/apm/apm-storm.dtsi | 83 ++
drivers/edac/Kconfig | 7 +
drivers/edac/Makefile | 1 +
drivers/edac/xgene_edac.c | 1254 ++++++++++++++++++++
7 files changed, 1431 insertions(+), 0 deletions(-)
create mode 100644 Documentation/devicetree/bindings/edac/apm-xgene-edac.txt
create mode 100644 drivers/edac/xgene_edac.c
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH v11 4/5] edac: Add APM X-Gene SoC EDAC driver
@ 2015-06-01 15:11 ` Catalin Marinas
2015-06-01 15:17 ` Mark Rutland
0 siblings, 1 reply; 5+ messages in thread
From: Catalin Marinas @ 2015-06-01 15:11 UTC (permalink / raw)
To: Arnd Bergmann
Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
mark.rutland-5wv7dgnIgG8, devicetree-u79uwXL29TY76Z2rM5mHXA,
mchehab-JPH+aEBZ4P+UEJcrhfAQsw, jcm-H+wXaHxf7aLQT0dZR+AlfA,
ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg, patches-qTEPVZfXA3Y,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, bp-Gina5bIWoIWzQB+pC5nmwQ, Loc Ho,
dougthompson-aS9lmoZGLiVWk0Htik3J/w,
linux-edac-u79uwXL29TY76Z2rM5mHXA
On Mon, Jun 01, 2015 at 04:42:58PM +0200, Arnd Bergmann wrote:
> On Friday 22 May 2015 17:32:59 Loc Ho wrote:
> > +static bool xgene_edac_pmd_l2c_version1(void)
> > +{
> > + /* Check all chips with PMD L2C version 1 HW */
> > + #define REVIDR_MINOR_REV(revidr) ((revidr) & 0x00000007)
> > +
> > + switch (MIDR_VARIANT(read_cpuid_id())) {
> > + case 0:
> > + switch (MIDR_REVISION(read_cpuid_id())) {
> > + case 0:
> > +
> > + switch (REVIDR_MINOR_REV(read_cpuid(REVIDR_EL1))) {
> > + case 1:
> > + case 2:
> > + return true;
> > + };
> > + break;
> > + case 1:
> > + if (REVIDR_MINOR_REV(read_cpuid(REVIDR_EL1)) == 1)
> > + return true;
> > + break;
> > + }
> > + break;
> > + case 1:
> > + switch (MIDR_REVISION(read_cpuid_id())) {
> > + case 0:
> > + switch (REVIDR_MINOR_REV(read_cpuid(REVIDR_EL1))) {
> > + case 1:
> > + return true;
> > + };
> > + break;
> > + case 1:
> > + switch (REVIDR_MINOR_REV(read_cpuid(REVIDR_EL1))) {
> > + case 1:
> > + case 0:
> > + return true;
> > + };
> > + break;
> > + }
> > + break;
> > + }
> > +
>
> As this is causing build errors on other architectures with COMPILE_TEST
> now, I'd suggest removing the function completely.
>
> Please use different compatible strings for IP blocks that are different
> and undetectable, instead of reading the ID of another IP block.
I fully agree, the MIDR_* macros shouldn't be used by anything under
drivers/.
--
Catalin
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH v11 4/5] edac: Add APM X-Gene SoC EDAC driver
@ 2015-06-01 15:17 ` Mark Rutland
2015-06-01 15:27 ` [PATCH] edac: xgene: fix cpuid abuse Arnd Bergmann
0 siblings, 1 reply; 5+ messages in thread
From: Mark Rutland @ 2015-06-01 15:17 UTC (permalink / raw)
To: Catalin Marinas
Cc: Arnd Bergmann,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
mchehab-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org,
jcm-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org,
ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg@public.gmane.org,
patches-qTEPVZfXA3Y@public.gmane.org,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org,
bp-Gina5bIWoIWzQB+pC5nmwQ@public.gmane.org, Loc Ho,
dougthompson-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org,
linux-edac-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
On Mon, Jun 01, 2015 at 04:11:31PM +0100, Catalin Marinas wrote:
> On Mon, Jun 01, 2015 at 04:42:58PM +0200, Arnd Bergmann wrote:
> > On Friday 22 May 2015 17:32:59 Loc Ho wrote:
> > > +static bool xgene_edac_pmd_l2c_version1(void)
> > > +{
> > > + /* Check all chips with PMD L2C version 1 HW */
> > > + #define REVIDR_MINOR_REV(revidr) ((revidr) & 0x00000007)
> > > +
> > > + switch (MIDR_VARIANT(read_cpuid_id())) {
> > > + case 0:
> > > + switch (MIDR_REVISION(read_cpuid_id())) {
> > > + case 0:
> > > +
> > > + switch (REVIDR_MINOR_REV(read_cpuid(REVIDR_EL1))) {
> > > + case 1:
> > > + case 2:
> > > + return true;
> > > + };
> > > + break;
> > > + case 1:
> > > + if (REVIDR_MINOR_REV(read_cpuid(REVIDR_EL1)) == 1)
> > > + return true;
> > > + break;
> > > + }
> > > + break;
> > > + case 1:
> > > + switch (MIDR_REVISION(read_cpuid_id())) {
> > > + case 0:
> > > + switch (REVIDR_MINOR_REV(read_cpuid(REVIDR_EL1))) {
> > > + case 1:
> > > + return true;
> > > + };
> > > + break;
> > > + case 1:
> > > + switch (REVIDR_MINOR_REV(read_cpuid(REVIDR_EL1))) {
> > > + case 1:
> > > + case 0:
> > > + return true;
> > > + };
> > > + break;
> > > + }
> > > + break;
> > > + }
> > > +
> >
> > As this is causing build errors on other architectures with COMPILE_TEST
> > now, I'd suggest removing the function completely.
> >
> > Please use different compatible strings for IP blocks that are different
> > and undetectable, instead of reading the ID of another IP block.
>
> I fully agree, the MIDR_* macros shouldn't be used by anything under
> drivers/.
Likewise.
Mark.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 5+ messages in thread* [PATCH] edac: xgene: fix cpuid abuse
2015-06-01 15:17 ` Mark Rutland
@ 2015-06-01 15:27 ` Arnd Bergmann
2015-06-01 17:04 ` Loc Ho
0 siblings, 1 reply; 5+ messages in thread
From: Arnd Bergmann @ 2015-06-01 15:27 UTC (permalink / raw)
To: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
Cc: Mark Rutland, Catalin Marinas,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg@public.gmane.org,
jcm-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org,
mchehab-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org,
patches-qTEPVZfXA3Y@public.gmane.org,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org,
bp-Gina5bIWoIWzQB+pC5nmwQ@public.gmane.org, Loc Ho,
dougthompson-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org,
linux-edac-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
The new x-gene EDAC driver incorrectly tried to figure out the version of
one of its IP blocks by looking at the version of the CPU core, which is
only vagely related.
This removes the incorrect code and instead uses the version of the IP
block in the compatible string where it belongs.
Found using build testing on x86, which does not provide the arm64
cpuid interface.
Signed-off-by: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
---
How about this one? We should make sure this gets fixed before the binding
gets set in stone with a kernel release.
Loc, do you know if some of the other blocks might also have versions
associated with them? Normally we try to use more specific product names,
but in case of APM there don't seem to be any more specific names
than X-Gene and X-Gene2 any more.
diff --git a/Documentation/devicetree/bindings/edac/apm-xgene-edac.txt b/Documentation/devicetree/bindings/edac/apm-xgene-edac.txt
index 480911c38ff9..e31b696ba939 100644
--- a/Documentation/devicetree/bindings/edac/apm-xgene-edac.txt
+++ b/Documentation/devicetree/bindings/edac/apm-xgene-edac.txt
@@ -25,7 +25,7 @@ Required properties for memory controller subnode:
- memory-controller : Instance number of the memory controller.
Required properties for PMD subnode:
-- compatible : Shall be "apm,xgene-edac-pmd".
+- compatible : Shall be "apm,xgene-edac-pmd" or "apm,xgene-v2-edac-pmd"
- reg : First resource shall be the PMD resource.
- pmd-controller : Instance number of the PMD controller.
diff --git a/drivers/edac/xgene_edac.c b/drivers/edac/xgene_edac.c
index b5158572f6ab..8071c2223f92 100644
--- a/drivers/edac/xgene_edac.c
+++ b/drivers/edac/xgene_edac.c
@@ -523,6 +523,7 @@ struct xgene_edac_pmd_ctx {
struct edac_device_ctl_info *edac_dev;
void __iomem *pmd_csr;
u32 pmd;
+ int version;
};
static void xgene_edac_pmd_l1_check(struct edac_device_ctl_info *edac_dev,
@@ -784,50 +785,6 @@ static void xgene_edac_pmd_cpu_hw_cfg(struct edac_device_ctl_info *edac_dev,
writel(0x00000101, pg_f + MEMERR_CPU_MMUECR_PAGE_OFFSET);
}
-static bool xgene_edac_pmd_l2c_version1(void)
-{
- /* Check all chips with PMD L2C version 1 HW */
- #define REVIDR_MINOR_REV(revidr) ((revidr) & 0x00000007)
-
- switch (MIDR_VARIANT(read_cpuid_id())) {
- case 0:
- switch (MIDR_REVISION(read_cpuid_id())) {
- case 0:
-
- switch (REVIDR_MINOR_REV(read_cpuid(REVIDR_EL1))) {
- case 1:
- case 2:
- return true;
- };
- break;
- case 1:
- if (REVIDR_MINOR_REV(read_cpuid(REVIDR_EL1)) == 1)
- return true;
- break;
- }
- break;
- case 1:
- switch (MIDR_REVISION(read_cpuid_id())) {
- case 0:
- switch (REVIDR_MINOR_REV(read_cpuid(REVIDR_EL1))) {
- case 1:
- return true;
- };
- break;
- case 1:
- switch (REVIDR_MINOR_REV(read_cpuid(REVIDR_EL1))) {
- case 1:
- case 0:
- return true;
- };
- break;
- }
- break;
- }
-
- return false;
-}
-
static void xgene_edac_pmd_hw_cfg(struct edac_device_ctl_info *edac_dev)
{
struct xgene_edac_pmd_ctx *ctx = edac_dev->pvt_info;
@@ -837,7 +794,7 @@ static void xgene_edac_pmd_hw_cfg(struct edac_device_ctl_info *edac_dev)
/* Enable PMD memory error - MEMERR_L2C_L2ECR and L2C_L2RTOCR */
writel(0x00000703, pg_e + MEMERR_L2C_L2ECR_PAGE_OFFSET);
/* Configure L2C HW request time out feature if supported */
- if (!xgene_edac_pmd_l2c_version1())
+ if (ctx->version == 1)
writel(0x00000119, pg_d + CPUX_L2C_L2RTOCR_PAGE_OFFSET);
}
@@ -956,7 +913,7 @@ static int xgene_edac_pmd_available(u32 efuse, int pmd)
return (efuse & (1 << pmd)) ? 0 : 1;
}
-static int xgene_edac_pmd_add(struct xgene_edac *edac, struct device_node *np)
+static int xgene_edac_pmd_add(struct xgene_edac *edac, struct device_node *np, int version)
{
struct edac_device_ctl_info *edac_dev;
struct xgene_edac_pmd_ctx *ctx;
@@ -998,6 +955,7 @@ static int xgene_edac_pmd_add(struct xgene_edac *edac, struct device_node *np)
ctx->edac = edac;
ctx->edac_dev = edac_dev;
ctx->ddev = *edac->dev;
+ ctx->version = version;
edac_dev->dev = &ctx->ddev;
edac_dev->ctl_name = ctx->name;
edac_dev->dev_name = ctx->name;
@@ -1169,7 +1127,9 @@ static int xgene_edac_probe(struct platform_device *pdev)
if (of_device_is_compatible(child, "apm,xgene-edac-mc"))
xgene_edac_mc_add(edac, child);
if (of_device_is_compatible(child, "apm,xgene-edac-pmd"))
- xgene_edac_pmd_add(edac, child);
+ xgene_edac_pmd_add(edac, child, 1);
+ if (of_device_is_compatible(child, "apm,xgene-v2-edac-pmd"))
+ xgene_edac_pmd_add(edac, child, 2);
}
return 0;
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] edac: xgene: fix cpuid abuse
2015-06-01 15:27 ` [PATCH] edac: xgene: fix cpuid abuse Arnd Bergmann
@ 2015-06-01 17:04 ` Loc Ho
[not found] ` <CAPw-ZT=591MT3HZcZ7TsfD_13mEJQ5swsw3kV2BoVxYUaYADCA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
0 siblings, 1 reply; 5+ messages in thread
From: Loc Ho @ 2015-06-01 17:04 UTC (permalink / raw)
To: Arnd Bergmann
Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
Mark Rutland, Catalin Marinas,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg@public.gmane.org,
jcm-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org,
mchehab-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org,
patches-qTEPVZfXA3Y@public.gmane.org,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org,
bp-Gina5bIWoIWzQB+pC5nmwQ@public.gmane.org,
dougthompson-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org,
linux-edac-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Hi Arnd,
Thanks for the changes, some minor change requested below.
> The new x-gene EDAC driver incorrectly tried to figure out the version of
> one of its IP blocks by looking at the version of the CPU core, which is
> only vagely related.
>
> This removes the incorrect code and instead uses the version of the IP
> block in the compatible string where it belongs.
>
> Found using build testing on x86, which does not provide the arm64
> cpuid interface.
>
> Signed-off-by: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
> ---
>
> How about this one? We should make sure this gets fixed before the binding
> gets set in stone with a kernel release.
>
> Loc, do you know if some of the other blocks might also have versions
> associated with them? Normally we try to use more specific product names,
> but in case of APM there don't seem to be any more specific names
> than X-Gene and X-Gene2 any more.
>
I already asked the designer. There is no version register in the PMD block.
> diff --git a/Documentation/devicetree/bindings/edac/apm-xgene-edac.txt b/Documentation/devicetree/bindings/edac/apm-xgene-edac.txt
> index 480911c38ff9..e31b696ba939 100644
> --- a/Documentation/devicetree/bindings/edac/apm-xgene-edac.txt
> +++ b/Documentation/devicetree/bindings/edac/apm-xgene-edac.txt
> @@ -25,7 +25,7 @@ Required properties for memory controller subnode:
> - memory-controller : Instance number of the memory controller.
>
> Required properties for PMD subnode:
> -- compatible : Shall be "apm,xgene-edac-pmd".
> +- compatible : Shall be "apm,xgene-edac-pmd" or "apm,xgene-v2-edac-pmd"
Can we change to "apm,xgene-edac-pmd-v2". I would like to associate
with the PMD instead with the family name. The PMD (processor module)
might be re-use in future chip as is such as Gen3, Gen4, and etc.
> - reg : First resource shall be the PMD resource.
> - pmd-controller : Instance number of the PMD controller.
>
> diff --git a/drivers/edac/xgene_edac.c b/drivers/edac/xgene_edac.c
> index b5158572f6ab..8071c2223f92 100644
> --- a/drivers/edac/xgene_edac.c
> +++ b/drivers/edac/xgene_edac.c
> @@ -523,6 +523,7 @@ struct xgene_edac_pmd_ctx {
> struct edac_device_ctl_info *edac_dev;
> void __iomem *pmd_csr;
> u32 pmd;
> + int version;
> };
>
> static void xgene_edac_pmd_l1_check(struct edac_device_ctl_info *edac_dev,
> @@ -784,50 +785,6 @@ static void xgene_edac_pmd_cpu_hw_cfg(struct edac_device_ctl_info *edac_dev,
> writel(0x00000101, pg_f + MEMERR_CPU_MMUECR_PAGE_OFFSET);
> }
>
> -static bool xgene_edac_pmd_l2c_version1(void)
> -{
> - /* Check all chips with PMD L2C version 1 HW */
> - #define REVIDR_MINOR_REV(revidr) ((revidr) & 0x00000007)
> -
> - switch (MIDR_VARIANT(read_cpuid_id())) {
> - case 0:
> - switch (MIDR_REVISION(read_cpuid_id())) {
> - case 0:
> -
> - switch (REVIDR_MINOR_REV(read_cpuid(REVIDR_EL1))) {
> - case 1:
> - case 2:
> - return true;
> - };
> - break;
> - case 1:
> - if (REVIDR_MINOR_REV(read_cpuid(REVIDR_EL1)) == 1)
> - return true;
> - break;
> - }
> - break;
> - case 1:
> - switch (MIDR_REVISION(read_cpuid_id())) {
> - case 0:
> - switch (REVIDR_MINOR_REV(read_cpuid(REVIDR_EL1))) {
> - case 1:
> - return true;
> - };
> - break;
> - case 1:
> - switch (REVIDR_MINOR_REV(read_cpuid(REVIDR_EL1))) {
> - case 1:
> - case 0:
> - return true;
> - };
> - break;
> - }
> - break;
> - }
> -
> - return false;
> -}
> -
> static void xgene_edac_pmd_hw_cfg(struct edac_device_ctl_info *edac_dev)
> {
> struct xgene_edac_pmd_ctx *ctx = edac_dev->pvt_info;
> @@ -837,7 +794,7 @@ static void xgene_edac_pmd_hw_cfg(struct edac_device_ctl_info *edac_dev)
> /* Enable PMD memory error - MEMERR_L2C_L2ECR and L2C_L2RTOCR */
> writel(0x00000703, pg_e + MEMERR_L2C_L2ECR_PAGE_OFFSET);
> /* Configure L2C HW request time out feature if supported */
> - if (!xgene_edac_pmd_l2c_version1())
> + if (ctx->version == 1)
The correct changes would be ctx->version > 1. So:
if (ctx->version > 1)
> writel(0x00000119, pg_d + CPUX_L2C_L2RTOCR_PAGE_OFFSET);
> }
>
> @@ -956,7 +913,7 @@ static int xgene_edac_pmd_available(u32 efuse, int pmd)
> return (efuse & (1 << pmd)) ? 0 : 1;
> }
>
> -static int xgene_edac_pmd_add(struct xgene_edac *edac, struct device_node *np)
> +static int xgene_edac_pmd_add(struct xgene_edac *edac, struct device_node *np, int version)
> {
> struct edac_device_ctl_info *edac_dev;
> struct xgene_edac_pmd_ctx *ctx;
> @@ -998,6 +955,7 @@ static int xgene_edac_pmd_add(struct xgene_edac *edac, struct device_node *np)
> ctx->edac = edac;
> ctx->edac_dev = edac_dev;
> ctx->ddev = *edac->dev;
> + ctx->version = version;
> edac_dev->dev = &ctx->ddev;
> edac_dev->ctl_name = ctx->name;
> edac_dev->dev_name = ctx->name;
> @@ -1169,7 +1127,9 @@ static int xgene_edac_probe(struct platform_device *pdev)
> if (of_device_is_compatible(child, "apm,xgene-edac-mc"))
> xgene_edac_mc_add(edac, child);
> if (of_device_is_compatible(child, "apm,xgene-edac-pmd"))
> - xgene_edac_pmd_add(edac, child);
> + xgene_edac_pmd_add(edac, child, 1);
> + if (of_device_is_compatible(child, "apm,xgene-v2-edac-pmd"))
Change to "apm,xgene-edac-pmd-v2"
> + xgene_edac_pmd_add(edac, child, 2);
> }
>
> return 0;
Again... Thanks for the changes.
-Loc
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2015-06-02 19:10 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-06-01 22:09 [PATCH] edac: xgene: fix cpuid abuse Loc Ho
[not found] ` <1433196575-9455-1-git-send-email-lho-qTEPVZfXA3Y@public.gmane.org>
2015-06-02 19:10 ` Borislav Petkov
-- strict thread matches above, loose matches on Subject: below --
2015-05-22 23:32 [PATCH v11 0/4] edac: Add APM X-Gene SoC EDAC driver Loc Ho
2015-06-01 15:11 ` [PATCH v11 4/5] " Catalin Marinas
2015-06-01 15:17 ` Mark Rutland
2015-06-01 15:27 ` [PATCH] edac: xgene: fix cpuid abuse Arnd Bergmann
2015-06-01 17:04 ` Loc Ho
[not found] ` <CAPw-ZT=591MT3HZcZ7TsfD_13mEJQ5swsw3kV2BoVxYUaYADCA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2015-06-01 18:09 ` Arnd Bergmann
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).