diff mbox

[v11,3/5] Documentation: Add documentation for the APM X-Gene SoC EDAC DTS binding

Message ID 1432337580-3750-4-git-send-email-lho@apm.com
State Accepted, archived
Commit ee66e6a24bd2f8a294f018ab90f660f23d8a7ff5
Headers show

Commit Message

Loc Ho May 22, 2015, 11:32 p.m. UTC 
This patch adds documentation for the APM X-Gene SoC EDAC DTS binding.

Signed-off-by: Loc Ho <lho@apm.com>
Acked-by: Arnd Bergmann <arnd@arndb.de>
---
 .../devicetree/bindings/edac/apm-xgene-edac.txt    |   78 ++++++++++++++++++++
 1 files changed, 78 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/edac/apm-xgene-edac.txt

Comments

Arnd Bergmann June 1, 2015, 2:42 p.m. UTC | #1 
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.

	Arnd

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Catalin Marinas June 1, 2015, 3:11 p.m. UTC | #2 
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/.
Mark Rutland June 1, 2015, 3:17 p.m. UTC | #3 
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@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/edac/apm-xgene-edac.txt b/Documentation/devicetree/bindings/edac/apm-xgene-edac.txt
new file mode 100644
index 0000000..480911c
--- /dev/null
+++ b/Documentation/devicetree/bindings/edac/apm-xgene-edac.txt
@@ -0,0 +1,78 @@ 
+* APM X-Gene SoC EDAC node
+
+EDAC node is defined to describe on-chip error detection and correction.
+The follow error types are supported:
+
+  memory controller	- Memory controller
+  PMD (L1/L2)		- Processor module unit (PMD) L1/L2 cache
+
+The following section describes the EDAC DT node binding.
+
+Required properties:
+- compatible		: Shall be "apm,xgene-edac".
+- regmap-csw		: Regmap of the CPU switch fabric (CSW) resource.
+- regmap-mcba		: Regmap of the MCB-A (memory bridge) resource.
+- regmap-mcbb		: Regmap of the MCB-B (memory bridge) resource.
+- regmap-efuse		: Regmap of the PMD efuse resource.
+- reg			: First resource shall be the CPU bus (PCP) resource.
+- interrupts            : Interrupt-specifier for MCU, PMD, L3, or SoC error
+			  IRQ(s).
+
+Required properties for memory controller subnode:
+- compatible		: Shall be "apm,xgene-edac-mc".
+- reg			: First resource shall be the memory controller unit
+                          (MCU) resource.
+- memory-controller	: Instance number of the memory controller.
+
+Required properties for PMD subnode:
+- compatible		: Shall be "apm,xgene-edac-pmd".
+- reg			: First resource shall be the PMD resource.
+- pmd-controller	: Instance number of the PMD controller.
+
+Example:
+	csw: csw@7e200000 {
+		compatible = "apm,xgene-csw", "syscon";
+		reg = <0x0 0x7e200000 0x0 0x1000>;
+	};
+
+	mcba: mcba@7e700000 {
+		compatible = "apm,xgene-mcb", "syscon";
+		reg = <0x0 0x7e700000 0x0 0x1000>;
+	};
+
+	mcbb: mcbb@7e720000 {
+		compatible = "apm,xgene-mcb", "syscon";
+		reg = <0x0 0x7e720000 0x0 0x1000>;
+	};
+
+	efuse: efuse@1054a000 {
+		compatible = "apm,xgene-efuse", "syscon";
+		reg = <0x0 0x1054a000 0x0 0x20>;
+	};
+
+	edac@78800000 {
+		compatible = "apm,xgene-edac";
+		#address-cells = <2>;
+		#size-cells = <2>;
+		ranges;
+		regmap-csw = <&csw>;
+		regmap-mcba = <&mcba>;
+		regmap-mcbb = <&mcbb>;
+		regmap-efuse = <&efuse>;
+		reg = <0x0 0x78800000 0x0 0x100>;
+		interrupts = <0x0 0x20 0x4>,
+			     <0x0 0x21 0x4>,
+			     <0x0 0x27 0x4>;
+
+		edacmc@7e800000 {
+			compatible = "apm,xgene-edac-mc";
+			reg = <0x0 0x7e800000 0x0 0x1000>;
+			memory-controller = <0>;
+		};
+
+		edacpmd@7c000000 {
+			compatible = "apm,xgene-edac-pmd";
+			reg = <0x0 0x7c000000 0x0 0x200000>;
+			pmd-controller = <0>;
+		};
+	};