[PATCH 1/2] MPC85xx: add definitions for PCI error detection soc part

[PATCH 1/2] MPC85xx: add definitions for PCI error detection soc part

[Dmitry Eremin-Solenikov]

Add definitions for PCI error detection device to be handled by (already
existing) mpc85xx_edac.c driver.

Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
---
 arch/powerpc/boot/dts/mpc8536ds.dts        |    7 +++++++
 arch/powerpc/boot/dts/mpc8536ds_36b.dts    |    7 +++++++
 arch/powerpc/boot/dts/mpc8540ads.dts       |    8 ++++++++
 arch/powerpc/boot/dts/mpc8541cds.dts       |   14 ++++++++++++++
 arch/powerpc/boot/dts/mpc8544ds.dts        |    7 +++++++
 arch/powerpc/boot/dts/mpc8548cds.dts       |   14 ++++++++++++++
 arch/powerpc/boot/dts/mpc8555cds.dts       |   14 ++++++++++++++
 arch/powerpc/boot/dts/mpc8560ads.dts       |    7 +++++++
 arch/powerpc/boot/dts/mpc8568mds.dts       |    7 +++++++
 arch/powerpc/boot/dts/sbc8548.dts          |    7 +++++++
 arch/powerpc/boot/dts/sbc8560.dts          |    7 +++++++
 arch/powerpc/boot/dts/socrates.dts         |    7 +++++++
 arch/powerpc/boot/dts/stx_gp3_8560.dts     |    7 +++++++
 arch/powerpc/boot/dts/tqm8540.dts          |   10 ++++++++++
 arch/powerpc/boot/dts/tqm8541.dts          |    7 +++++++
 arch/powerpc/boot/dts/tqm8548-bigflash.dts |    7 +++++++
 arch/powerpc/boot/dts/tqm8548.dts          |    7 +++++++
 arch/powerpc/boot/dts/tqm8555.dts          |    7 +++++++
 arch/powerpc/boot/dts/tqm8560.dts          |    7 +++++++
 arch/powerpc/boot/dts/xpedite5200.dts      |    7 +++++++
 arch/powerpc/boot/dts/xpedite5200_xmon.dts |    7 +++++++
 21 files changed, 172 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/boot/dts/mpc8536ds.dts b/arch/powerpc/boot/dts/mpc8536ds.dts
index 815cebb..6c02e5a 100644
--- a/arch/powerpc/boot/dts/mpc8536ds.dts
+++ b/arch/powerpc/boot/dts/mpc8536ds.dts
@@ -278,6 +278,13 @@
 			interrupt-parent = <&mpic>;
 		};
 
+		pci-error@8e00 {
+			compatible = "fsl,mpc8536-pci-error";
+			reg = <0x8e00 0x100>;
+			interrupt-parent = <&mpic>;
+			interrupts = <24 2>;
+		};
+
 		crypto@30000 {
 			compatible = "fsl,sec3.0", "fsl,sec2.4", "fsl,sec2.2",
 				     "fsl,sec2.1", "fsl,sec2.0";
diff --git a/arch/powerpc/boot/dts/mpc8536ds_36b.dts b/arch/powerpc/boot/dts/mpc8536ds_36b.dts
index d95b260..89ef5c1 100644
--- a/arch/powerpc/boot/dts/mpc8536ds_36b.dts
+++ b/arch/powerpc/boot/dts/mpc8536ds_36b.dts
@@ -278,6 +278,13 @@
 			interrupt-parent = <&mpic>;
 		};
 
+		pci-error@8e00 {
+			compatible = "fsl,mpc8536-pci-error";
+			reg = <0x8e00 0x100>;
+			interrupt-parent = <&mpic>;
+			interrupts = <24 2>;
+		};
+
 		crypto@30000 {
 			compatible = "fsl,sec3.0", "fsl,sec2.4", "fsl,sec2.2",
 				     "fsl,sec2.1", "fsl,sec2.0";
diff --git a/arch/powerpc/boot/dts/mpc8540ads.dts b/arch/powerpc/boot/dts/mpc8540ads.dts
index 9dc2929..d89f470 100644
--- a/arch/powerpc/boot/dts/mpc8540ads.dts
+++ b/arch/powerpc/boot/dts/mpc8540ads.dts
@@ -259,6 +259,14 @@
 			interrupts = <42 2>;
 			interrupt-parent = <&mpic>;
 		};
+
+		pci-error@8e00 {
+			compatible = "fsl,mpc8540-pci-error";
+			reg = <0x8e00 0x100>;
+			interrupt-parent = <&mpic>;
+			interrupts = <24 2>;
+		};
+
 		mpic: pic@40000 {
 			interrupt-controller;
 			#address-cells = <0>;
diff --git a/arch/powerpc/boot/dts/mpc8541cds.dts b/arch/powerpc/boot/dts/mpc8541cds.dts
index 9a3ad31..c2c5732 100644
--- a/arch/powerpc/boot/dts/mpc8541cds.dts
+++ b/arch/powerpc/boot/dts/mpc8541cds.dts
@@ -226,6 +226,20 @@
 			interrupt-parent = <&mpic>;
 		};
 
+		pci-error@8e00 {
+			compatible = "fsl,mpc8541-pci-error";
+			reg = <0x8e00 0x100>;
+			interrupt-parent = <&mpic>;
+			interrupts = <24 2>;
+		};
+
+		pci-error@9e00 {
+			compatible = "fsl,mpc8541-pci-error";
+			reg = <0x9e00 0x100>;
+			interrupt-parent = <&mpic>;
+			interrupts = <25 2>;
+		};
+
 		crypto@30000 {
 			compatible = "fsl,sec2.0";
 			reg = <0x30000 0x10000>;
diff --git a/arch/powerpc/boot/dts/mpc8544ds.dts b/arch/powerpc/boot/dts/mpc8544ds.dts
index 98e94b4..387da1f 100644
--- a/arch/powerpc/boot/dts/mpc8544ds.dts
+++ b/arch/powerpc/boot/dts/mpc8544ds.dts
@@ -242,6 +242,13 @@
 			interrupt-parent = <&mpic>;
 		};
 
+		pci-error@8e00 {
+			compatible = "fsl,mpc8544-pci-error";
+			reg = <0x8e00 0x100>;
+			interrupt-parent = <&mpic>;
+			interrupts = <24 2>;
+		};
+
 		global-utilities@e0000 {	//global utilities block
 			compatible = "fsl,mpc8548-guts";
 			reg = <0xe0000 0x1000>;
diff --git a/arch/powerpc/boot/dts/mpc8548cds.dts b/arch/powerpc/boot/dts/mpc8548cds.dts
index 0f52624..973966e 100644
--- a/arch/powerpc/boot/dts/mpc8548cds.dts
+++ b/arch/powerpc/boot/dts/mpc8548cds.dts
@@ -328,6 +328,20 @@
 			interrupt-parent = <&mpic>;
 		};
 
+		pci-error@8e00 {
+			compatible = "fsl,mpc8548-pci-error";
+			reg = <0x8e00 0x100>;
+			interrupt-parent = <&mpic>;
+			interrupts = <24 2>;
+		};
+
+		pci-error@9e00 {
+			compatible = "fsl,mpc8548-pci-error";
+			reg = <0x9e00 0x100>;
+			interrupt-parent = <&mpic>;
+			interrupts = <25 2>;
+		};
+
 		global-utilities@e0000 {	//global utilities reg
 			compatible = "fsl,mpc8548-guts";
 			reg = <0xe0000 0x1000>;
diff --git a/arch/powerpc/boot/dts/mpc8555cds.dts b/arch/powerpc/boot/dts/mpc8555cds.dts
index 065b2f0..bcdc344 100644
--- a/arch/powerpc/boot/dts/mpc8555cds.dts
+++ b/arch/powerpc/boot/dts/mpc8555cds.dts
@@ -226,6 +226,20 @@
 			interrupt-parent = <&mpic>;
 		};
 
+		pci-error@8e00 {
+			compatible = "fsl,mpc8555-pci-error";
+			reg = <0x8e00 0x100>;
+			interrupt-parent = <&mpic>;
+			interrupts = <24 2>;
+		};
+
+		pci-error@9e00 {
+			compatible = "fsl,mpc8555-pci-error";
+			reg = <0x9e00 0x100>;
+			interrupt-parent = <&mpic>;
+			interrupts = <25 2>;
+		};
+
 		crypto@30000 {
 			compatible = "fsl,sec2.0";
 			reg = <0x30000 0x10000>;
diff --git a/arch/powerpc/boot/dts/mpc8560ads.dts b/arch/powerpc/boot/dts/mpc8560ads.dts
index a5bb1ec..04bc095 100644
--- a/arch/powerpc/boot/dts/mpc8560ads.dts
+++ b/arch/powerpc/boot/dts/mpc8560ads.dts
@@ -216,6 +216,13 @@
 			device_type = "open-pic";
 		};
 
+		pci-error@8e00 {
+			compatible = "fsl,mpc8560-pci-error";
+			reg = <0x8e00 0x100>;
+			interrupt-parent = <&mpic>;
+			interrupts = <24 2>;
+		};
+
 		cpm@919c0 {
 			#address-cells = <1>;
 			#size-cells = <1>;
diff --git a/arch/powerpc/boot/dts/mpc8568mds.dts b/arch/powerpc/boot/dts/mpc8568mds.dts
index 92fb178..66af3ab 100644
--- a/arch/powerpc/boot/dts/mpc8568mds.dts
+++ b/arch/powerpc/boot/dts/mpc8568mds.dts
@@ -329,6 +329,13 @@
 			};
 		};
 
+		pci-error@8e00 {
+			compatible = "fsl,mpc8568-pci-error";
+			reg = <0x8e00 0x100>;
+			interrupt-parent = <&mpic>;
+			interrupts = <24 2>;
+		};
+
 		global-utilities@e0000 {
 			#address-cells = <1>;
 			#size-cells = <1>;
diff --git a/arch/powerpc/boot/dts/sbc8548.dts b/arch/powerpc/boot/dts/sbc8548.dts
index 94a3322..002428c 100644
--- a/arch/powerpc/boot/dts/sbc8548.dts
+++ b/arch/powerpc/boot/dts/sbc8548.dts
@@ -333,6 +333,13 @@
 			interrupt-parent = <&mpic>;
 		};
 
+		pci-error@8e00 {
+			compatible = "fsl,mpc8548-pci-error";
+			reg = <0x8e00 0x100>;
+			interrupt-parent = <&mpic>;
+			interrupts = <24 2>;
+		};
+
 		global-utilities@e0000 {	//global utilities reg
 			compatible = "fsl,mpc8548-guts";
 			reg = <0xe0000 0x1000>;
diff --git a/arch/powerpc/boot/dts/sbc8560.dts b/arch/powerpc/boot/dts/sbc8560.dts
index 9e13ed8..1dc389a 100644
--- a/arch/powerpc/boot/dts/sbc8560.dts
+++ b/arch/powerpc/boot/dts/sbc8560.dts
@@ -239,6 +239,13 @@
 			device_type = "open-pic";
 		};
 
+		pci-error@8e00 {
+			compatible = "fsl,mpc8560-pci-error";
+			reg = <0x8e00 0x100>;
+			interrupt-parent = <&mpic>;
+			interrupts = <24 2>;
+		};
+
 		cpm@919c0 {
 			#address-cells = <1>;
 			#size-cells = <1>;
diff --git a/arch/powerpc/boot/dts/socrates.dts b/arch/powerpc/boot/dts/socrates.dts
index feb4ef6..06b08f5 100644
--- a/arch/powerpc/boot/dts/socrates.dts
+++ b/arch/powerpc/boot/dts/socrates.dts
@@ -216,6 +216,13 @@
 			interrupt-parent = <&mpic>;
 		};
 
+		pci-error@8e00 {
+			compatible = "fsl,mpc8544-pci-error";
+			reg = <0x8e00 0x100>;
+			interrupt-parent = <&mpic>;
+			interrupts = <24 2>;
+		};
+
 		global-utilities@e0000 {	//global utilities block
 			compatible = "fsl,mpc8548-guts";
 			reg = <0xe0000 0x1000>;
diff --git a/arch/powerpc/boot/dts/stx_gp3_8560.dts b/arch/powerpc/boot/dts/stx_gp3_8560.dts
index b670d03..78e9e22 100644
--- a/arch/powerpc/boot/dts/stx_gp3_8560.dts
+++ b/arch/powerpc/boot/dts/stx_gp3_8560.dts
@@ -213,6 +213,13 @@
 			device_type = "open-pic";
 		};
 
+		pci-error@8e00 {
+			compatible = "fsl,mpc8560-pci-error";
+			reg = <0x8e00 0x100>;
+			interrupt-parent = <&mpic>;
+			interrupts = <24 2>;
+		};
+
 		cpm@919c0 {
 			#address-cells = <1>;
 			#size-cells = <1>;
diff --git a/arch/powerpc/boot/dts/tqm8540.dts b/arch/powerpc/boot/dts/tqm8540.dts
index b5c0940..3e73e35 100644
--- a/arch/powerpc/boot/dts/tqm8540.dts
+++ b/arch/powerpc/boot/dts/tqm8540.dts
@@ -160,6 +160,7 @@
 			local-mac-address = [ 00 00 00 00 00 00 ];
 			interrupts = <29 2 30 2 34 2>;
 			interrupt-parent = <&mpic>;
+			tbi-handle = <&tbi0>;
 			phy-handle = <&phy2>;
 
 			mdio@520 {
@@ -205,6 +206,7 @@
 			local-mac-address = [ 00 00 00 00 00 00 ];
 			interrupts = <35 2 36 2 40 2>;
 			interrupt-parent = <&mpic>;
+			tbi-handle = <&tbi1>;
 			phy-handle = <&phy1>;
 
 			mdio@520 {
@@ -232,6 +234,7 @@
 			local-mac-address = [ 00 00 00 00 00 00 ];
 			interrupts = <41 2>;
 			interrupt-parent = <&mpic>;
+			tbi-handle = <&tbi2>;
 			phy-handle = <&phy3>;
 
 			mdio@520 {
@@ -276,6 +279,13 @@
 			device_type = "open-pic";
 			compatible = "chrp,open-pic";
 		};
+
+		pci-error@8e00 {
+			compatible = "fsl,mpc8540-pci-error";
+			reg = <0x8e00 0x100>;
+			interrupt-parent = <&mpic>;
+			interrupts = <24 2>;
+		};
 	};
 
 	pci0: pci@e0008000 {
diff --git a/arch/powerpc/boot/dts/tqm8541.dts b/arch/powerpc/boot/dts/tqm8541.dts
index f49d091..92106fc 100644
--- a/arch/powerpc/boot/dts/tqm8541.dts
+++ b/arch/powerpc/boot/dts/tqm8541.dts
@@ -261,6 +261,13 @@
 			compatible = "chrp,open-pic";
 		};
 
+		pci-error@8e00 {
+			compatible = "fsl,mpc8541-pci-error";
+			reg = <0x8e00 0x100>;
+			interrupt-parent = <&mpic>;
+			interrupts = <24 2>;
+		};
+
 		cpm@919c0 {
 			#address-cells = <1>;
 			#size-cells = <1>;
diff --git a/arch/powerpc/boot/dts/tqm8548-bigflash.dts b/arch/powerpc/boot/dts/tqm8548-bigflash.dts
index 5dbb36e..b1944a1 100644
--- a/arch/powerpc/boot/dts/tqm8548-bigflash.dts
+++ b/arch/powerpc/boot/dts/tqm8548-bigflash.dts
@@ -338,6 +338,13 @@
 			compatible = "chrp,open-pic";
 			device_type = "open-pic";
 		};
+
+		pci-error@8e00 {
+			compatible = "fsl,mpc8548-pci-error";
+			reg = <0x8e00 0x100>;
+			interrupt-parent = <&mpic>;
+			interrupts = <24 2>;
+		};
 	};
 
 	localbus@a0005000 {
diff --git a/arch/powerpc/boot/dts/tqm8548.dts b/arch/powerpc/boot/dts/tqm8548.dts
index a050ae4..be2b1de 100644
--- a/arch/powerpc/boot/dts/tqm8548.dts
+++ b/arch/powerpc/boot/dts/tqm8548.dts
@@ -338,6 +338,13 @@
 			compatible = "chrp,open-pic";
 			device_type = "open-pic";
 		};
+
+		pci-error@8e00 {
+			compatible = "fsl,mpc8548-pci-error";
+			reg = <0x8e00 0x100>;
+			interrupt-parent = <&mpic>;
+			interrupts = <24 2>;
+		};
 	};
 
 	localbus@e0005000 {
diff --git a/arch/powerpc/boot/dts/tqm8555.dts b/arch/powerpc/boot/dts/tqm8555.dts
index 81bad8c..7b64c71 100644
--- a/arch/powerpc/boot/dts/tqm8555.dts
+++ b/arch/powerpc/boot/dts/tqm8555.dts
@@ -261,6 +261,13 @@
 			compatible = "chrp,open-pic";
 		};
 
+		pci-error@8e00 {
+			compatible = "fsl,mpc8555-pci-error";
+			reg = <0x8e00 0x100>;
+			interrupt-parent = <&mpic>;
+			interrupts = <24 2>;
+		};
+
 		cpm@919c0 {
 			#address-cells = <1>;
 			#size-cells = <1>;
diff --git a/arch/powerpc/boot/dts/tqm8560.dts b/arch/powerpc/boot/dts/tqm8560.dts
index 22ec39b..82408d5 100644
--- a/arch/powerpc/boot/dts/tqm8560.dts
+++ b/arch/powerpc/boot/dts/tqm8560.dts
@@ -232,6 +232,13 @@
 			compatible = "chrp,open-pic";
 		};
 
+		pci-error@8e00 {
+			compatible = "fsl,mpc8560-pci-error";
+			reg = <0x8e00 0x100>;
+			interrupt-parent = <&mpic>;
+			interrupts = <24 2>;
+		};
+
 		cpm@919c0 {
 			#address-cells = <1>;
 			#size-cells = <1>;
diff --git a/arch/powerpc/boot/dts/xpedite5200.dts b/arch/powerpc/boot/dts/xpedite5200.dts
index a0cf53f..211dbbf 100644
--- a/arch/powerpc/boot/dts/xpedite5200.dts
+++ b/arch/powerpc/boot/dts/xpedite5200.dts
@@ -352,6 +352,13 @@
 			interrupt-parent = <&mpic>;
 		};
 
+		pci-error@8e00 {
+			compatible = "fsl,mpc8548-pci-error";
+			reg = <0x8e00 0x100>;
+			interrupt-parent = <&mpic>;
+			interrupts = <24 2>;
+		};
+
 		global-utilities@e0000 {	// global utilities reg
 			compatible = "fsl,mpc8548-guts";
 			reg = <0xe0000 0x1000>;
diff --git a/arch/powerpc/boot/dts/xpedite5200_xmon.dts b/arch/powerpc/boot/dts/xpedite5200_xmon.dts
index c5b2975..ebe9365 100644
--- a/arch/powerpc/boot/dts/xpedite5200_xmon.dts
+++ b/arch/powerpc/boot/dts/xpedite5200_xmon.dts
@@ -356,6 +356,13 @@
 			interrupt-parent = <&mpic>;
 		};
 
+		pci-error@8e00 {
+			compatible = "fsl,mpc8548-pci-error";
+			reg = <0x8e00 0x100>;
+			interrupt-parent = <&mpic>;
+			interrupts = <24 2>;
+		};
+
 		global-utilities@e0000 {	// global utilities reg
 			compatible = "fsl,mpc8548-guts";
 			reg = <0xe0000 0x1000>;
-- 
1.7.1

[PATCH 2/2] mpc85xx_edac: change to use new definitions for PCI EDAC regspace

[Dmitry Eremin-Solenikov]

Currently (as mpc8540-pci) devices are not created on of_platform bus,
mpc85xx_edac can't probe to them. Follow the change to dts trees to bind
not to the main mpc8540-pci node but to special mpc85xx-pci-error nodes,
present on soc bus.

Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
---
 drivers/edac/mpc85xx_edac.c |   18 +++++++++---------
 1 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/edac/mpc85xx_edac.c b/drivers/edac/mpc85xx_edac.c
index 52ca09b..d5a61b8 100644
--- a/drivers/edac/mpc85xx_edac.c
+++ b/drivers/edac/mpc85xx_edac.c
@@ -236,9 +236,6 @@ static int __devinit mpc85xx_pci_err_probe(struct of_device *op,
 		goto err;
 	}
 
-	/* we only need the error registers */
-	r.start += 0xe00;
-
 	if (!devm_request_mem_region(&op->dev, r.start, resource_size(&r),
 					pdata->name)) {
 		printk(KERN_ERR "%s: Error while requesting mem region\n",
@@ -328,12 +325,15 @@ static int mpc85xx_pci_err_remove(struct of_device *op)
 }
 
 static struct of_device_id mpc85xx_pci_err_of_match[] = {
-	{
-	 .compatible = "fsl,mpc8540-pcix",
-	 },
-	{
-	 .compatible = "fsl,mpc8540-pci",
-	},
+	{ .compatible = "fsl,mpc8536-pci-error", },
+	{ .compatible = "fsl,mpc8540-pci-error", },
+	{ .compatible = "fsl,mpc8541-pci-error", },
+	{ .compatible = "fsl,mpc8544-pci-error", },
+	{ .compatible = "fsl,mpc8548-pci-error", },
+	{ .compatible = "fsl,mpc8555-pci-error", },
+	{ .compatible = "fsl,mpc8560-pci-error", },
+	{ .compatible = "fsl,mpc8568-pci-error", },
+	{ .compatible = "fsl,mpc8572-pci-error", },
 	{},
 };
 
-- 
1.7.1

Re: [PATCH 1/2] MPC85xx: add definitions for PCI error detection soc part

[Peter Tyser]

Hi Dmitry,

On Thu, 2010-07-22 at 04:03 +0400, Dmitry Eremin-Solenikov wrote:
> Add definitions for PCI error detection device to be handled by (already
> existing) mpc85xx_edac.c driver.
> 
> Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
> ---
>  arch/powerpc/boot/dts/mpc8536ds.dts        |    7 +++++++
>  arch/powerpc/boot/dts/mpc8536ds_36b.dts    |    7 +++++++
>  arch/powerpc/boot/dts/mpc8540ads.dts       |    8 ++++++++
>  arch/powerpc/boot/dts/mpc8541cds.dts       |   14 ++++++++++++++
>  arch/powerpc/boot/dts/mpc8544ds.dts        |    7 +++++++
>  arch/powerpc/boot/dts/mpc8548cds.dts       |   14 ++++++++++++++
>  arch/powerpc/boot/dts/mpc8555cds.dts       |   14 ++++++++++++++
>  arch/powerpc/boot/dts/mpc8560ads.dts       |    7 +++++++
>  arch/powerpc/boot/dts/mpc8568mds.dts       |    7 +++++++
>  arch/powerpc/boot/dts/sbc8548.dts          |    7 +++++++
>  arch/powerpc/boot/dts/sbc8560.dts          |    7 +++++++
>  arch/powerpc/boot/dts/socrates.dts         |    7 +++++++
>  arch/powerpc/boot/dts/stx_gp3_8560.dts     |    7 +++++++
>  arch/powerpc/boot/dts/tqm8540.dts          |   10 ++++++++++
>  arch/powerpc/boot/dts/tqm8541.dts          |    7 +++++++
>  arch/powerpc/boot/dts/tqm8548-bigflash.dts |    7 +++++++
>  arch/powerpc/boot/dts/tqm8548.dts          |    7 +++++++
>  arch/powerpc/boot/dts/tqm8555.dts          |    7 +++++++
>  arch/powerpc/boot/dts/tqm8560.dts          |    7 +++++++
>  arch/powerpc/boot/dts/xpedite5200.dts      |    7 +++++++
>  arch/powerpc/boot/dts/xpedite5200_xmon.dts |    7 +++++++
>  21 files changed, 172 insertions(+), 0 deletions(-)

It looks like the dts files for the MPC8572-based boards weren't
included in this change despite patch 2/2 adding support for them.  I'd
guess some other Freescale CPUs (eg P1020, P2020, etc) could be
supported by the same driver if you are inclined to add them to this
patch series.

Best,
Peter

Re: [PATCH 2/2] mpc85xx_edac: change to use new definitions for PCI EDAC regspace

[Kumar Gala]

On Jul 21, 2010, at 7:03 PM, Dmitry Eremin-Solenikov wrote:

> Currently (as mpc8540-pci) devices are not created on of_platform bus,
> mpc85xx_edac can't probe to them. Follow the change to dts trees to =
bind
> not to the main mpc8540-pci node but to special mpc85xx-pci-error =
nodes,
> present on soc bus.
>=20
> Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
> ---
> drivers/edac/mpc85xx_edac.c |   18 +++++++++---------
> 1 files changed, 9 insertions(+), 9 deletions(-)

Nak.

We already have a node in the dts for the PCI controller.  Lets update =
the platform code to add the pci controller to the of_platform_bus_probe =
list.

- k=

Re: [PATCH 2/2] mpc85xx_edac: change to use new definitions for PCI EDAC regspace

[Dmitry Eremin-Solenikov]

Hello,

On Thu, Jul 22, 2010 at 7:38 PM, Kumar Gala <galak@kernel.crashing.org> wro=
te:
>
> On Jul 21, 2010, at 7:03 PM, Dmitry Eremin-Solenikov wrote:
>
>> Currently (as mpc8540-pci) devices are not created on of_platform bus,
>> mpc85xx_edac can't probe to them. Follow the change to dts trees to bind
>> not to the main mpc8540-pci node but to special mpc85xx-pci-error nodes,
>> present on soc bus.
>>
>> Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
>> ---
>> drivers/edac/mpc85xx_edac.c | =A0 18 +++++++++---------
>> 1 files changed, 9 insertions(+), 9 deletions(-)
>
> Nak.
>
> We already have a node in the dts for the PCI controller. =A0Lets update =
the platform code to add the pci controller to the of_platform_bus_probe li=
st.

I've had that idea. However it's really look strange to me to call
of_platform_bus_probe() on the bus node, for which we (IMO) explicitly
won't like for
child devices (PCI devices) to be added to of_platform bus. Would it
be suitable to just call of_platform_device_create for it (Or do i
miss someth<ing)?

BTW: While I'm at it, should I change all mpc8540-pci/-pcix device
names to include respective SoC name?

--=20
With best wishes
Dmitry

Re: [PATCH 1/2] MPC85xx: add definitions for PCI error detection soc part

[Dmitry Eremin-Solenikov]

Hello,

On Thu, Jul 22, 2010 at 7:19 PM, Peter Tyser <ptyser@xes-inc.com> wrote:
> Hi Dmitry,
>
> On Thu, 2010-07-22 at 04:03 +0400, Dmitry Eremin-Solenikov wrote:
>> Add definitions for PCI error detection device to be handled by (already
>> existing) mpc85xx_edac.c driver.
>>
>> Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
>> ---
>> =A0arch/powerpc/boot/dts/mpc8536ds.dts =A0 =A0 =A0 =A0| =A0 =A07 +++++++
>> =A0arch/powerpc/boot/dts/mpc8536ds_36b.dts =A0 =A0| =A0 =A07 +++++++
>> =A0arch/powerpc/boot/dts/mpc8540ads.dts =A0 =A0 =A0 | =A0 =A08 ++++++++
>> =A0arch/powerpc/boot/dts/mpc8541cds.dts =A0 =A0 =A0 | =A0 14 +++++++++++=
+++
>> =A0arch/powerpc/boot/dts/mpc8544ds.dts =A0 =A0 =A0 =A0| =A0 =A07 +++++++
>> =A0arch/powerpc/boot/dts/mpc8548cds.dts =A0 =A0 =A0 | =A0 14 +++++++++++=
+++
>> =A0arch/powerpc/boot/dts/mpc8555cds.dts =A0 =A0 =A0 | =A0 14 +++++++++++=
+++
>> =A0arch/powerpc/boot/dts/mpc8560ads.dts =A0 =A0 =A0 | =A0 =A07 +++++++
>> =A0arch/powerpc/boot/dts/mpc8568mds.dts =A0 =A0 =A0 | =A0 =A07 +++++++
>> =A0arch/powerpc/boot/dts/sbc8548.dts =A0 =A0 =A0 =A0 =A0| =A0 =A07 +++++=
++
>> =A0arch/powerpc/boot/dts/sbc8560.dts =A0 =A0 =A0 =A0 =A0| =A0 =A07 +++++=
++
>> =A0arch/powerpc/boot/dts/socrates.dts =A0 =A0 =A0 =A0 | =A0 =A07 +++++++
>> =A0arch/powerpc/boot/dts/stx_gp3_8560.dts =A0 =A0 | =A0 =A07 +++++++
>> =A0arch/powerpc/boot/dts/tqm8540.dts =A0 =A0 =A0 =A0 =A0| =A0 10 +++++++=
+++
>> =A0arch/powerpc/boot/dts/tqm8541.dts =A0 =A0 =A0 =A0 =A0| =A0 =A07 +++++=
++
>> =A0arch/powerpc/boot/dts/tqm8548-bigflash.dts | =A0 =A07 +++++++
>> =A0arch/powerpc/boot/dts/tqm8548.dts =A0 =A0 =A0 =A0 =A0| =A0 =A07 +++++=
++
>> =A0arch/powerpc/boot/dts/tqm8555.dts =A0 =A0 =A0 =A0 =A0| =A0 =A07 +++++=
++
>> =A0arch/powerpc/boot/dts/tqm8560.dts =A0 =A0 =A0 =A0 =A0| =A0 =A07 +++++=
++
>> =A0arch/powerpc/boot/dts/xpedite5200.dts =A0 =A0 =A0| =A0 =A07 +++++++
>> =A0arch/powerpc/boot/dts/xpedite5200_xmon.dts | =A0 =A07 +++++++
>> =A021 files changed, 172 insertions(+), 0 deletions(-)
>
> It looks like the dts files for the MPC8572-based boards weren't
> included in this change despite patch 2/2 adding support for them. =A0I'd
> guess some other Freescale CPUs (eg P1020, P2020, etc) could be
> supported by the same driver if you are inclined to add them to this
> patch series.

I just did a quick search for all dts including mpc8540-pci node and added
respective mpc85xx-pci-error node. Current MPC85xx EDAC driver doesn't
support error handling on PCI-E busses (which MPC8572 and other CPUs
you mentioned have). I'll maybe look into PCI-E EDAC later. Also I don't ha=
ve
access to P10xx/P20xx manuals, so support for them may require some
more time.

--=20
With best wishes
Dmitry

Re: [PATCH 1/2] MPC85xx: add definitions for PCI error detection soc part

[Peter Tyser]

> > On Thu, 2010-07-22 at 04:03 +0400, Dmitry Eremin-Solenikov wrote:
> >> Add definitions for PCI error detection device to be handled by (already
> >> existing) mpc85xx_edac.c driver.
> >>
> >> Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
> >> ---
> >>  arch/powerpc/boot/dts/mpc8536ds.dts        |    7 +++++++
> >>  arch/powerpc/boot/dts/mpc8536ds_36b.dts    |    7 +++++++
> >>  arch/powerpc/boot/dts/mpc8540ads.dts       |    8 ++++++++
> >>  arch/powerpc/boot/dts/mpc8541cds.dts       |   14 ++++++++++++++
> >>  arch/powerpc/boot/dts/mpc8544ds.dts        |    7 +++++++
> >>  arch/powerpc/boot/dts/mpc8548cds.dts       |   14 ++++++++++++++
> >>  arch/powerpc/boot/dts/mpc8555cds.dts       |   14 ++++++++++++++
> >>  arch/powerpc/boot/dts/mpc8560ads.dts       |    7 +++++++
> >>  arch/powerpc/boot/dts/mpc8568mds.dts       |    7 +++++++
> >>  arch/powerpc/boot/dts/sbc8548.dts          |    7 +++++++
> >>  arch/powerpc/boot/dts/sbc8560.dts          |    7 +++++++
> >>  arch/powerpc/boot/dts/socrates.dts         |    7 +++++++
> >>  arch/powerpc/boot/dts/stx_gp3_8560.dts     |    7 +++++++
> >>  arch/powerpc/boot/dts/tqm8540.dts          |   10 ++++++++++
> >>  arch/powerpc/boot/dts/tqm8541.dts          |    7 +++++++
> >>  arch/powerpc/boot/dts/tqm8548-bigflash.dts |    7 +++++++
> >>  arch/powerpc/boot/dts/tqm8548.dts          |    7 +++++++
> >>  arch/powerpc/boot/dts/tqm8555.dts          |    7 +++++++
> >>  arch/powerpc/boot/dts/tqm8560.dts          |    7 +++++++
> >>  arch/powerpc/boot/dts/xpedite5200.dts      |    7 +++++++
> >>  arch/powerpc/boot/dts/xpedite5200_xmon.dts |    7 +++++++
> >>  21 files changed, 172 insertions(+), 0 deletions(-)
> >
> > It looks like the dts files for the MPC8572-based boards weren't
> > included in this change despite patch 2/2 adding support for them.  I'd
> > guess some other Freescale CPUs (eg P1020, P2020, etc) could be
> > supported by the same driver if you are inclined to add them to this
> > patch series.
> 
> I just did a quick search for all dts including mpc8540-pci node and added
> respective mpc85xx-pci-error node. Current MPC85xx EDAC driver doesn't
> support error handling on PCI-E busses (which MPC8572 and other CPUs
> you mentioned have). I'll maybe look into PCI-E EDAC later. Also I don't have
> access to P10xx/P20xx manuals, so support for them may require some
> more time.

Thanks for the explanation.  I noticed this because patch 2/2 adds
support for the 8572 to the mpc85xx_pci_err_of_match[] table.  Sounds
like that was the bug instead of my comment above.

Best,
Peter

Re: [PATCH 2/2] mpc85xx_edac: change to use new definitions for PCI EDAC regspace

[Scott Wood]

On Thu, 22 Jul 2010 20:48:15 +0400
Dmitry Eremin-Solenikov <dbaryshkov@gmail.com> wrote:

> Hello,
>=20
> On Thu, Jul 22, 2010 at 7:38 PM, Kumar Gala <galak@kernel.crashing.org> w=
rote:
> >
> > On Jul 21, 2010, at 7:03 PM, Dmitry Eremin-Solenikov wrote:
> >
> >> Currently (as mpc8540-pci) devices are not created on of_platform bus,
> >> mpc85xx_edac can't probe to them. Follow the change to dts trees to bi=
nd
> >> not to the main mpc8540-pci node but to special mpc85xx-pci-error node=
s,
> >> present on soc bus.
> >>
> >> Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
> >> ---
> >> drivers/edac/mpc85xx_edac.c | =A0 18 +++++++++---------
> >> 1 files changed, 9 insertions(+), 9 deletions(-)
> >
> > Nak.
> >
> > We already have a node in the dts for the PCI controller. =A0Lets updat=
e the platform code to add the pci controller to the of_platform_bus_probe =
list.
>=20
> I've had that idea. However it's really look strange to me to call
> of_platform_bus_probe() on the bus node, for which we (IMO) explicitly
> won't like for
> child devices (PCI devices) to be added to of_platform bus.

Right, and it's also not great for a driver for one aspect of PCI to
claim to be the driver for the whole thing.

But changing the device tree because of this Linux-internal concern is
also not good.

How about keeping the error stuff as a separate device from Linux's
perspective, but have the main Freescale PCI code create it as a
platform device instead of putting it in the device tree?

-Scott

Re: [PATCH 2/2] mpc85xx_edac: change to use new definitions for PCI EDAC regspace

[Kumar Gala]

On Jul 22, 2010, at 1:25 PM, Scott Wood wrote:

> On Thu, 22 Jul 2010 20:48:15 +0400
> Dmitry Eremin-Solenikov <dbaryshkov@gmail.com> wrote:
>=20
>> Hello,
>>=20
>> On Thu, Jul 22, 2010 at 7:38 PM, Kumar Gala =
<galak@kernel.crashing.org> wrote:
>>>=20
>>> On Jul 21, 2010, at 7:03 PM, Dmitry Eremin-Solenikov wrote:
>>>=20
>>>> Currently (as mpc8540-pci) devices are not created on of_platform =
bus,
>>>> mpc85xx_edac can't probe to them. Follow the change to dts trees to =
bind
>>>> not to the main mpc8540-pci node but to special mpc85xx-pci-error =
nodes,
>>>> present on soc bus.
>>>>=20
>>>> Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
>>>> ---
>>>> drivers/edac/mpc85xx_edac.c |   18 +++++++++---------
>>>> 1 files changed, 9 insertions(+), 9 deletions(-)
>>>=20
>>> Nak.
>>>=20
>>> We already have a node in the dts for the PCI controller.  Lets =
update the platform code to add the pci controller to the =
of_platform_bus_probe list.
>>=20
>> I've had that idea. However it's really look strange to me to call
>> of_platform_bus_probe() on the bus node, for which we (IMO) =
explicitly
>> won't like for
>> child devices (PCI devices) to be added to of_platform bus.
>=20
> Right, and it's also not great for a driver for one aspect of PCI to
> claim to be the driver for the whole thing.
>=20
> But changing the device tree because of this Linux-internal concern is
> also not good.
>=20
> How about keeping the error stuff as a separate device from Linux's
> perspective, but have the main Freescale PCI code create it as a
> platform device instead of putting it in the device tree?

I'd be good with that solution.

- k=

Re: [PATCH 2/2] mpc85xx_edac: change to use new definitions for PCI EDAC regspace

[Dmitry Eremin-Solenikov]

Hello,

On Thu, Jul 22, 2010 at 10:40 PM, Kumar Gala <galak@kernel.crashing.org> wr=
ote:
>
> On Jul 22, 2010, at 1:25 PM, Scott Wood wrote:
>
>> On Thu, 22 Jul 2010 20:48:15 +0400
>> Dmitry Eremin-Solenikov <dbaryshkov@gmail.com> wrote:
>>
>>> Hello,
>>>
>>> On Thu, Jul 22, 2010 at 7:38 PM, Kumar Gala <galak@kernel.crashing.org>=
 wrote:
>>>>
>>>> On Jul 21, 2010, at 7:03 PM, Dmitry Eremin-Solenikov wrote:
>>>>
>>>>> Currently (as mpc8540-pci) devices are not created on of_platform bus=
,
>>>>> mpc85xx_edac can't probe to them. Follow the change to dts trees to b=
ind
>>>>> not to the main mpc8540-pci node but to special mpc85xx-pci-error nod=
es,
>>>>> present on soc bus.
>>>>>
>>>>> Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
>>>>> ---
>>>>> drivers/edac/mpc85xx_edac.c | =A0 18 +++++++++---------
>>>>> 1 files changed, 9 insertions(+), 9 deletions(-)
>>>>
>>>> Nak.
>>>>
>>>> We already have a node in the dts for the PCI controller. =A0Lets upda=
te the platform code to add the pci controller to the of_platform_bus_probe=
 list.
>>>
>>> I've had that idea. However it's really look strange to me to call
>>> of_platform_bus_probe() on the bus node, for which we (IMO) explicitly
>>> won't like for
>>> child devices (PCI devices) to be added to of_platform bus.
>>
>> Right, and it's also not great for a driver for one aspect of PCI to
>> claim to be the driver for the whole thing.
>>
>> But changing the device tree because of this Linux-internal concern is
>> also not good.
>>
>> How about keeping the error stuff as a separate device from Linux's
>> perspective, but have the main Freescale PCI code create it as a
>> platform device instead of putting it in the device tree?
>
> I'd be good with that solution.

Then we come back to the question that was raised before (during initial
review of edac driver): as PCI code is probbed long before other parts
of the kernel and mpc85xx_edac code can be compiled as module,
it's not possible to directly call mpc85xx_edac code from fsl_pci.c

Two initial suggestions were:
1) creating special platform device
2) creating special of_platform device from dts

Which approach should I choose? Did i miss any other opportunities?

--=20
With best wishes
Dmitry

Re: [PATCH 2/2] mpc85xx_edac: change to use new definitions for PCI EDAC regspace

[Grant Likely]

[-- Attachment #1: Type: text/plain, Size: 1545 bytes --]

On Thu, Jul 22, 2010 at 10:48 AM, Dmitry Eremin-Solenikov
<dbaryshkov@gmail.com> wrote:
> Hello,
>
> On Thu, Jul 22, 2010 at 7:38 PM, Kumar Gala <galak@kernel.crashing.org> wrote:
>>
>> On Jul 21, 2010, at 7:03 PM, Dmitry Eremin-Solenikov wrote:
>>
>>> Currently (as mpc8540-pci) devices are not created on of_platform bus,
>>> mpc85xx_edac can't probe to them. Follow the change to dts trees to bind
>>> not to the main mpc8540-pci node but to special mpc85xx-pci-error nodes,
>>> present on soc bus.
>>>
>>> Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
>>> ---
>>> drivers/edac/mpc85xx_edac.c |   18 +++++++++---------
>>> 1 files changed, 9 insertions(+), 9 deletions(-)
>>
>> Nak.
>>
>> We already have a node in the dts for the PCI controller.  Lets update the platform code to add the pci controller to the of_platform_bus_probe list.
>
> I've had that idea. However it's really look strange to me to call
> of_platform_bus_probe() on the bus node, for which we (IMO) explicitly
> won't like for
> child devices (PCI devices) to be added to of_platform bus. Would it
> be suitable to just call of_platform_device_create for it (Or do i
> miss someth<ing)?

Try the attached patch (lightly tested).  If it works for you then
I'll post it for wider review.

> BTW: While I'm at it, should I change all mpc8540-pci/-pcix device
> names to include respective SoC name?

It is good practice to include both the specific name, and the name of
the device it is backwards compatible to.

g.

[-- Attachment #2: 0001-of-device-Register-children-with-a-compatible-value-.patch --]
[-- Type: text/x-diff, Size: 2708 bytes --]

From d84af195dbcd99ce172bf639538231141176d402 Mon Sep 17 00:00:00 2001
From: Grant Likely <grant.likely@secretlab.ca>
Date: Thu, 22 Jul 2010 13:01:11 -0600
Subject: [PATCH] of/device: Register children with a compatible value in of_platform_bus_probe()

Currently, of_platform_bus_probe() completely skips nodes which do not
explicitly match the 'matches' table passed in.  Or, if the root node
matches, then it registers all the children unconditionally.  However,
there are situations, such as registering devices from the root node,
when it is desirable to register child nodes, but only if they actually
represent devices.  For example, the root node may contain both a local
bus and a PCI device, but it also contains the chosen, aliases and cpus
nodes which don't represent real devices.

This patch changes of_platform_bus_probe() to register all nodes at the
top level if they either match the matches table (the current behaviour),
or if they have a 'compatible' value (indicating it represents a device).
---
 drivers/of/platform.c |   31 ++++++++++++++++++++++++++-----
 1 files changed, 26 insertions(+), 5 deletions(-)

diff --git a/drivers/of/platform.c b/drivers/of/platform.c
index f3f1ec8..2ead562 100644
--- a/drivers/of/platform.c
+++ b/drivers/of/platform.c
@@ -709,16 +709,37 @@ int of_platform_bus_probe(struct device_node *root,
 		rc = of_platform_bus_create(root, matches, &dev->dev);
 		goto bail;
 	}
+
+	/* Register each child node if either:
+	 *  a) it has a 'compatible' value indicating they are a device, or
+	 *  b) it is specified by the 'matches' table (by name or device_type)
+	 * If a node is specified in the matches table, then all its children
+	 * also get registered.
+	 */
 	for_each_child_of_node(root, child) {
-		if (!of_match_node(matches, child))
+		void *compat = of_get_property(child, "compatible", NULL);
+		struct of_device_id *match = of_match_node(matches, child);
+
+		/* Skip if node neither matches nor has a compatible property */
+		if (!compat && !match)
 			continue;
 
-		pr_debug("  match: %s\n", child->full_name);
+		pr_debug("  register device: %s\n", child->full_name);
+
+		/* Passed the first test, register node as a platform device */
 		dev = of_platform_device_create(child, NULL, parent);
-		if (dev == NULL)
+		if (!dev) {
 			rc = -ENOMEM;
-		else
-			rc = of_platform_bus_create(child, matches, &dev->dev);
+			of_node_put(child);
+			break;
+		}
+
+		/* Only register child nodes if specified by matches table */
+		if (!match)
+			continue;
+
+		pr_debug("  register children of: %s\n", child->full_name);
+		rc = of_platform_bus_create(child, matches, &dev->dev);
 		if (rc) {
 			of_node_put(child);
 			break;
-- 
1.7.0.4

Re: [PATCH 2/2] mpc85xx_edac: change to use new definitions for PCI EDAC regspace

[Grant Likely]

On Thu, Jul 22, 2010 at 1:03 PM, Dmitry Eremin-Solenikov
<dbaryshkov@gmail.com> wrote:
> Hello,
>
> On Thu, Jul 22, 2010 at 10:40 PM, Kumar Gala <galak@kernel.crashing.org> =
wrote:
>>
>> On Jul 22, 2010, at 1:25 PM, Scott Wood wrote:
>>
>>> On Thu, 22 Jul 2010 20:48:15 +0400
>>> Dmitry Eremin-Solenikov <dbaryshkov@gmail.com> wrote:
>>>
>>>> Hello,
>>>>
>>>> On Thu, Jul 22, 2010 at 7:38 PM, Kumar Gala <galak@kernel.crashing.org=
> wrote:
>>>>>
>>>>> On Jul 21, 2010, at 7:03 PM, Dmitry Eremin-Solenikov wrote:
>>>>>
>>>>>> Currently (as mpc8540-pci) devices are not created on of_platform bu=
s,
>>>>>> mpc85xx_edac can't probe to them. Follow the change to dts trees to =
bind
>>>>>> not to the main mpc8540-pci node but to special mpc85xx-pci-error no=
des,
>>>>>> present on soc bus.
>>>>>>
>>>>>> Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
>>>>>> ---
>>>>>> drivers/edac/mpc85xx_edac.c | =A0 18 +++++++++---------
>>>>>> 1 files changed, 9 insertions(+), 9 deletions(-)
>>>>>
>>>>> Nak.
>>>>>
>>>>> We already have a node in the dts for the PCI controller. =A0Lets upd=
ate the platform code to add the pci controller to the of_platform_bus_prob=
e list.
>>>>
>>>> I've had that idea. However it's really look strange to me to call
>>>> of_platform_bus_probe() on the bus node, for which we (IMO) explicitly
>>>> won't like for
>>>> child devices (PCI devices) to be added to of_platform bus.
>>>
>>> Right, and it's also not great for a driver for one aspect of PCI to
>>> claim to be the driver for the whole thing.
>>>
>>> But changing the device tree because of this Linux-internal concern is
>>> also not good.
>>>
>>> How about keeping the error stuff as a separate device from Linux's
>>> perspective, but have the main Freescale PCI code create it as a
>>> platform device instead of putting it in the device tree?
>>
>> I'd be good with that solution.
>
> Then we come back to the question that was raised before (during initial
> review of edac driver): as PCI code is probbed long before other parts
> of the kernel and mpc85xx_edac code can be compiled as module,
> it's not possible to directly call mpc85xx_edac code from fsl_pci.c

Not sure what you mean here.  If the driver is compiled as a module,
then it must be handled the same way all other drivers are handled.
The platform code registers an of_platform_device at boot time, and
the driver gets bound to it whenever it happens to show up.  None of
the children of that bus can be available until after the driver is
probed.

> Two initial suggestions were:
> 1) creating special platform device
> 2) creating special of_platform device from dts

g.

--=20
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

Re: [PATCH 2/2] mpc85xx_edac: change to use new definitions for PCI EDAC regspace

[Scott Wood]

On Thu, 22 Jul 2010 23:03:03 +0400
Dmitry Eremin-Solenikov <dbaryshkov@gmail.com> wrote:

> Hello,
> 
> On Thu, Jul 22, 2010 at 10:40 PM, Kumar Gala <galak@kernel.crashing.org> wrote:
> >
> > On Jul 22, 2010, at 1:25 PM, Scott Wood wrote:
> >> How about keeping the error stuff as a separate device from Linux's
> >> perspective, but have the main Freescale PCI code create it as a
> >> platform device instead of putting it in the device tree?
> >
> > I'd be good with that solution.
> 
> Then we come back to the question that was raised before (during initial
> review of edac driver): as PCI code is probbed long before other parts
> of the kernel and mpc85xx_edac code can be compiled as module,
> it's not possible to directly call mpc85xx_edac code from fsl_pci.c

Right, that's why I suggested creating a platform device rather than
just a function call.

> Two initial suggestions were:
> 1) creating special platform device
> 2) creating special of_platform device from dts
> 
> Which approach should I choose? Did i miss any other opportunities?

#1, as it keeps the split out of the device tree.

Besides the theoretical/aesthetic issues of putting Linux
implementation concerns into the device tree, #2 would mean that the
edac driver wouldn't work when Linux is booted with an old device tree.

-Scott

Re: [PATCH 2/2] mpc85xx_edac: change to use new definitions for PCI EDAC regspace

[Dmitry Eremin-Solenikov]

Hello,

On 7/22/10, Grant Likely <grant.likely@secretlab.ca> wrote:
> On Thu, Jul 22, 2010 at 10:48 AM, Dmitry Eremin-Solenikov
> <dbaryshkov@gmail.com> wrote:
>> Hello,
>>
>> On Thu, Jul 22, 2010 at 7:38 PM, Kumar Gala <galak@kernel.crashing.org>
>> wrote:
>>>
>>> On Jul 21, 2010, at 7:03 PM, Dmitry Eremin-Solenikov wrote:
>>>
>>>> Currently (as mpc8540-pci) devices are not created on of_platform bus,
>>>> mpc85xx_edac can't probe to them. Follow the change to dts trees to bind
>>>> not to the main mpc8540-pci node but to special mpc85xx-pci-error nodes,
>>>> present on soc bus.
>>>>
>>>> Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
>>>> ---
>>>> drivers/edac/mpc85xx_edac.c |   18 +++++++++---------
>>>> 1 files changed, 9 insertions(+), 9 deletions(-)
>>>
>>> Nak.
>>>
>>> We already have a node in the dts for the PCI controller.  Lets update
>>> the platform code to add the pci controller to the of_platform_bus_probe
>>> list.
>>
>> I've had that idea. However it's really look strange to me to call
>> of_platform_bus_probe() on the bus node, for which we (IMO) explicitly
>> won't like for
>> child devices (PCI devices) to be added to of_platform bus. Would it
>> be suitable to just call of_platform_device_create for it (Or do i
>> miss someth<ing)?
>
> Try the attached patch (lightly tested).  If it works for you then
> I'll post it for wider review.

Yes, this patch worked for me. However it looks a bit like a hack for me.

-- 
With best wishes
Dmitry

Re: [PATCH 2/2] mpc85xx_edac: change to use new definitions for PCI EDAC regspace

[Grant Likely]

On Fri, Jul 23, 2010 at 6:20 PM, Dmitry Eremin-Solenikov
<dbaryshkov@gmail.com> wrote:
> Hello,
>
> On 7/22/10, Grant Likely <grant.likely@secretlab.ca> wrote:
>> On Thu, Jul 22, 2010 at 10:48 AM, Dmitry Eremin-Solenikov
>> <dbaryshkov@gmail.com> wrote:
>>> Hello,
>>>
>>> On Thu, Jul 22, 2010 at 7:38 PM, Kumar Gala <galak@kernel.crashing.org>
>>> wrote:
>>>>
>>>> On Jul 21, 2010, at 7:03 PM, Dmitry Eremin-Solenikov wrote:
>>>>
>>>>> Currently (as mpc8540-pci) devices are not created on of_platform bus=
,
>>>>> mpc85xx_edac can't probe to them. Follow the change to dts trees to b=
ind
>>>>> not to the main mpc8540-pci node but to special mpc85xx-pci-error nod=
es,
>>>>> present on soc bus.
>>>>>
>>>>> Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
>>>>> ---
>>>>> drivers/edac/mpc85xx_edac.c | =A0 18 +++++++++---------
>>>>> 1 files changed, 9 insertions(+), 9 deletions(-)
>>>>
>>>> Nak.
>>>>
>>>> We already have a node in the dts for the PCI controller. =A0Lets upda=
te
>>>> the platform code to add the pci controller to the of_platform_bus_pro=
be
>>>> list.
>>>
>>> I've had that idea. However it's really look strange to me to call
>>> of_platform_bus_probe() on the bus node, for which we (IMO) explicitly
>>> won't like for
>>> child devices (PCI devices) to be added to of_platform bus. Would it
>>> be suitable to just call of_platform_device_create for it (Or do i
>>> miss someth<ing)?
>>
>> Try the attached patch (lightly tested). =A0If it works for you then
>> I'll post it for wider review.
>
> Yes, this patch worked for me. However it looks a bit like a hack for me.

I'll probably refine it a bit before merging, but I don't think it is
a hack.  It reflects the behaviour that makes sense when registering
devices hanging off the root node.  If a device node is a child of the
root, then we know it isn't hanging off an i2c or pci bus, or anything
else.  It is essentially a system device.

The troublesome bit is that the root node also has memory, cpus,
chosen and aliases nodes which are not devices.  In the vast majority
of cases, we want all the device nodes that are children of the root
to be registered, but we don't want to register the special nodes.
Checking for the presence of a compatible property is a pretty good
test for determining whether or not a node actually represents a
device, especially because all users of of_platform_bus_probe() seem
to be FDT users where we've been very strict about enforcing that
drivers must use the compatible property for matching to device nodes.

Cheers,
g.

Re: [PATCH 2/2] mpc85xx_edac: change to use new definitions for PCI EDAC regspace

[Dmitry Eremin-Solenikov]

On 7/24/10, Grant Likely <grant.likely@secretlab.ca> wrote:
> On Fri, Jul 23, 2010 at 6:20 PM, Dmitry Eremin-Solenikov
> <dbaryshkov@gmail.com> wrote:
>> Hello,
>>
>> On 7/22/10, Grant Likely <grant.likely@secretlab.ca> wrote:
>>> On Thu, Jul 22, 2010 at 10:48 AM, Dmitry Eremin-Solenikov
>>> <dbaryshkov@gmail.com> wrote:
>>>> Hello,
>>>>
>>>> On Thu, Jul 22, 2010 at 7:38 PM, Kumar Gala <galak@kernel.crashing.org>
>>>> wrote:
>>>>>
>>>>> On Jul 21, 2010, at 7:03 PM, Dmitry Eremin-Solenikov wrote:
>>>>>
>>>>>> Currently (as mpc8540-pci) devices are not created on of_platform bus,
>>>>>> mpc85xx_edac can't probe to them. Follow the change to dts trees to
>>>>>> bind
>>>>>> not to the main mpc8540-pci node but to special mpc85xx-pci-error
>>>>>> nodes,
>>>>>> present on soc bus.
>>>>>>
>>>>>> Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
>>>>>> ---
>>>>>> drivers/edac/mpc85xx_edac.c |   18 +++++++++---------
>>>>>> 1 files changed, 9 insertions(+), 9 deletions(-)
>>>>>
>>>>> Nak.
>>>>>
>>>>> We already have a node in the dts for the PCI controller.  Lets update
>>>>> the platform code to add the pci controller to the
>>>>> of_platform_bus_probe
>>>>> list.
>>>>
>>>> I've had that idea. However it's really look strange to me to call
>>>> of_platform_bus_probe() on the bus node, for which we (IMO) explicitly
>>>> won't like for
>>>> child devices (PCI devices) to be added to of_platform bus. Would it
>>>> be suitable to just call of_platform_device_create for it (Or do i
>>>> miss someth<ing)?
>>>
>>> Try the attached patch (lightly tested).  If it works for you then
>>> I'll post it for wider review.
>>
>> Yes, this patch worked for me. However it looks a bit like a hack for me.
>
> I'll probably refine it a bit before merging, but I don't think it is
> a hack.  It reflects the behaviour that makes sense when registering
> devices hanging off the root node.  If a device node is a child of the
> root, then we know it isn't hanging off an i2c or pci bus, or anything
> else.  It is essentially a system device.
>
> The troublesome bit is that the root node also has memory, cpus,
> chosen and aliases nodes which are not devices.  In the vast majority
> of cases, we want all the device nodes that are children of the root
> to be registered, but we don't want to register the special nodes.
> Checking for the presence of a compatible property is a pretty good
> test for determining whether or not a node actually represents a
> device, especially because all users of of_platform_bus_probe() seem
> to be FDT users where we've been very strict about enforcing that
> drivers must use the compatible property for matching to device nodes.


Now it's clear to me, thanks for the explanation.

BTW: On 2.6.35-rc6 I had to make 'compat' and 'match' variables const.

-- 
With best wishes
Dmitry