[PATCH] powerpc: Add EDAC platform devices for 85xx

[PATCH] powerpc: Add EDAC platform devices for 85xx

[Dave Jiang]

Add memory controller and l2-cache controller entries in the dts files for 85xx
platforms. Also adding code to create the platform devices used by EDAC drivers
to claim the resources in order to access the error registers and interrupts. 

Signed-off-by: Dave Jiang <djiang@mvista.com>

---

 arch/powerpc/boot/dts/mpc8540ads.dts |   21 ++++++
 arch/powerpc/boot/dts/mpc8548cds.dts |   21 ++++++
 arch/powerpc/boot/dts/mpc8560ads.dts |   23 +++++++-
 arch/powerpc/sysdev/fsl_soc.c        |  115 ++++++++++++++++++++++++++++++++++
 4 files changed, 179 insertions(+), 1 deletions(-)

diff --git a/arch/powerpc/boot/dts/mpc8540ads.dts b/arch/powerpc/boot/dts/mpc8540ads.dts
index f261d64..8bb7593 100644
--- a/arch/powerpc/boot/dts/mpc8540ads.dts
+++ b/arch/powerpc/boot/dts/mpc8540ads.dts
@@ -48,6 +48,27 @@
 		reg = <e0000000 00100000>;	// CCSRBAR 1M
 		bus-frequency = <0>;
 
+		mem-ctrl@2000 {
+			device_type = "mem-ctrl";
+			compatible = "85xx";
+			reg = <2000 1000>;
+			linux,phandle = <2000>;
+			interrupt-parent = <40000>;
+			interrupts = <2 2>;
+		};
+
+		l2-cache@20000 {
+			device_type = "l2-cache";
+			compatible = "85xx";
+			reg = <20000 1000>;
+			cache-line-size = <20>;	// 32 bytes
+			cache-size = <40000>;	// L2, 256K
+			32-bit;
+			linux,phandle = <20000>;
+			interrupt-parent = <40000>;
+			interrupts = <0 2>;
+		};
+
 		i2c@3000 {
 			device_type = "i2c";
 			compatible = "fsl-i2c";
diff --git a/arch/powerpc/boot/dts/mpc8548cds.dts b/arch/powerpc/boot/dts/mpc8548cds.dts
index b2b2200..26c740a 100644
--- a/arch/powerpc/boot/dts/mpc8548cds.dts
+++ b/arch/powerpc/boot/dts/mpc8548cds.dts
@@ -48,6 +48,27 @@
 		reg = <e0000000 00100000>;	// CCSRBAR 1M
 		bus-frequency = <0>;
 
+		mem-ctrl@2000 {
+			device_type = "mem-ctrl";
+			compatible = "85xx";
+			reg = <2000 1000>;
+			linux,phandle = <2000>;
+			interrupt-parent = <40000>;
+			interrupts = <2 2>;
+		};
+
+		l2-cache@20000 {
+			device_type = "l2-cache";
+			compatible = "85xx";
+			reg = <20000 1000>;
+			cache-line-size = <20>;	// 32 bytes
+			cache-size = <40000>;	// L2, 256K
+			32-bit;
+			linux,phandle = <20000>;
+			interrupt-parent = <40000>;
+			interrupts = <0 2>;
+		};
+
 		i2c@3000 {
 			device_type = "i2c";
 			compatible = "fsl-i2c";
diff --git a/arch/powerpc/boot/dts/mpc8560ads.dts b/arch/powerpc/boot/dts/mpc8560ads.dts
index 1f2afe9..1bbb54b 100644
--- a/arch/powerpc/boot/dts/mpc8560ads.dts
+++ b/arch/powerpc/boot/dts/mpc8560ads.dts
@@ -48,6 +48,27 @@
 		reg = <e0000000 00000200>;
 		bus-frequency = <13ab6680>;
 
+		mem-ctrl@2000 {
+			device_type = "mem-ctrl";
+			compatible = "85xx";
+			reg = <2000 1000>;
+			linux,phandle = <2000>;
+			interrupt-parent = <40000>;
+			interrupts = <2 2>;
+		};
+
+		l2-cache@20000 {
+			device_type = "l2-cache";
+			compatible = "85xx";
+			reg = <20000 1000>;
+			cache-line-size = <20>;	// 32 bytes
+			cache-size = <40000>;	// L2, 256K
+			32-bit;
+			linux,phandle = <20000>;
+			interrupt-parent = <40000>;
+			interrupts = <0 2>;
+		};
+
 		mdio@24520 {
 			device_type = "mdio";
 			compatible = "gianfar";
@@ -110,7 +131,7 @@
 			#address-cells = <3>;
 			compatible = "85xx";
 			device_type = "pci";
-			reg = <8000 400>;
+			reg = <8000 1000>;
 			clock-frequency = <3f940aa>;
 			interrupt-map-mask = <f800 0 0 7>;
 			interrupt-map = <
diff --git a/arch/powerpc/sysdev/fsl_soc.c b/arch/powerpc/sysdev/fsl_soc.c
index 8a123c7..a0beb8b 100644
--- a/arch/powerpc/sysdev/fsl_soc.c
+++ b/arch/powerpc/sysdev/fsl_soc.c
@@ -1103,3 +1103,118 @@ err:
 arch_initcall(cpm_smc_uart_of_init);
 
 #endif /* CONFIG_8xx */
+
+/* platform device setup for EDAC */
+#ifdef CONFIG_PPC_85xx
+static int __init mpc85xx_mc_err_init(void)
+{
+	struct resource r[2];
+	struct device_node *np;
+	struct platform_device *pdev;
+	int ret = 0;
+
+	memset(r, 0, sizeof(r));
+
+	np = of_find_compatible_node(NULL, "mem-ctrl", "85xx");
+	if (!np)
+		return 0;
+
+	ret = of_address_to_resource(np, 0, &r[0]);
+	if (ret)
+		goto err;
+
+	of_irq_to_resource(np, 0, &r[1]);
+
+	of_node_put(np);
+
+	pdev = platform_device_register_simple("mpc85xx_mc_err", 0, r, 2);
+	if (IS_ERR(pdev))
+		return PTR_ERR(pdev);
+
+	return 0;
+
+err:
+	of_node_put(np);
+	printk(KERN_WARNING "mpc85xx-mem-ctrl setup failed\n");
+	return 0;
+}
+arch_initcall(mpc85xx_mc_err_init);
+
+static int __init mpc85xx_l2_err_init(void)
+{
+	struct resource r[2];
+	struct device_node *np;
+	struct platform_device *pdev;
+	int ret = 0;
+
+	memset(r, 0, sizeof(r));
+
+	np = of_find_compatible_node(NULL, "l2-cache", "85xx");
+	if (!np)
+		return 0;
+
+	ret = of_address_to_resource(np, 0, &r[0]);
+	if (ret)
+		goto err;
+	
+	/* we only need access to the error registers */
+	r[0].start += 0xe00;
+
+	of_irq_to_resource(np, 0, &r[1]);
+
+	of_node_put(np);
+
+	pdev = platform_device_register_simple("mpc85xx_l2_err", 0, r, 2);
+	if (IS_ERR(pdev))
+		return PTR_ERR(pdev);
+
+	return 0;
+
+err:
+	of_node_put(np);
+	printk(KERN_WARNING "mpc85xx-l2 setup failed\n");
+	return 0;
+}
+arch_initcall(mpc85xx_l2_err_init);
+
+static int __init mpc85xx_pci_err_init(void)
+{
+	struct resource r[2];
+	struct device_node *np = NULL;
+	struct platform_device *pdev;
+	int i;
+	int ret = 0;
+
+	for (i = 0;
+		(np = of_find_compatible_node(np, "pci", "85xx"));
+		i++) {
+		memset(r, 0, sizeof(r));
+
+		ret = of_address_to_resource(np, 0, &r[0]);
+		if (ret)
+			goto err;
+	
+		/* we only need access to the error registers */
+		r[0].start += 0xe00;
+
+		of_irq_to_resource(np, 0, &r[1]);
+
+		pdev = platform_device_register_simple("mpc85xx_pci_err", 
+				i, r, 2);
+		if (IS_ERR(pdev)) {
+			ret = PTR_ERR(pdev);
+			goto err;
+		}
+
+	}
+
+	return 0;
+
+err:
+	of_node_put(np);
+	printk(KERN_WARNING "mpc85xx-pci setup failed\n");
+	return ret;
+}
+arch_initcall(mpc85xx_pci_err_init);
+
+#endif /* CONFIG_PPC_85xx */

Re: [PATCH] powerpc: Add EDAC platform devices for 85xx

[David Gibson]

On Wed, Apr 25, 2007 at 02:37:00PM -0700, Dave Jiang wrote:
> Add memory controller and l2-cache controller entries in the dts files for 85xx
> platforms. Also adding code to create the platform devices used by EDAC drivers
> to claim the resources in order to access the error registers and interrupts. 
> 
> Signed-off-by: Dave Jiang <djiang@mvista.com>
> 
> ---
> 
>  arch/powerpc/boot/dts/mpc8540ads.dts |   21 ++++++
>  arch/powerpc/boot/dts/mpc8548cds.dts |   21 ++++++
>  arch/powerpc/boot/dts/mpc8560ads.dts |   23 +++++++-
>  arch/powerpc/sysdev/fsl_soc.c        |  115 ++++++++++++++++++++++++++++++++++
>  4 files changed, 179 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/powerpc/boot/dts/mpc8540ads.dts b/arch/powerpc/boot/dts/mpc8540ads.dts
> index f261d64..8bb7593 100644
> --- a/arch/powerpc/boot/dts/mpc8540ads.dts
> +++ b/arch/powerpc/boot/dts/mpc8540ads.dts
> @@ -48,6 +48,27 @@
>  		reg = <e0000000 00100000>;	// CCSRBAR 1M
>  		bus-frequency = <0>;
>  
> +		mem-ctrl@2000 {
> +			device_type = "mem-ctrl";
> +			compatible = "85xx";
> +			reg = <2000 1000>;
> +			linux,phandle = <2000>;
> +			interrupt-parent = <40000>;

Please don't use explicit phandles.  Use dtc's label feature where you
need to put phandles into properties.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

Re: [PATCH] powerpc: Add EDAC platform devices for 85xx

[Dave Jiang]

Add memory controller and l2-cache controller entries in the dts files for 85xx
platforms. Also adding code to create the platform devices used by EDAC drivers
to claim the resources in order to access the error registers and interrupts. 

Signed-off-by: Dave Jiang <djiang@mvista.com>

---

Removed explicit phandles as suggested by David Gibson. 

 arch/powerpc/boot/dts/mpc8540ads.dts |   19 ++++++
 arch/powerpc/boot/dts/mpc8548cds.dts |   19 ++++++
 arch/powerpc/boot/dts/mpc8560ads.dts |   21 ++++++-
 arch/powerpc/sysdev/fsl_soc.c        |  115 ++++++++++++++++++++++++++++++++++
 4 files changed, 173 insertions(+), 1 deletions(-)

diff --git a/arch/powerpc/boot/dts/mpc8540ads.dts b/arch/powerpc/boot/dts/mpc8540ads.dts
index f261d64..63c274d 100644
--- a/arch/powerpc/boot/dts/mpc8540ads.dts
+++ b/arch/powerpc/boot/dts/mpc8540ads.dts
@@ -48,6 +48,25 @@
 		reg = <e0000000 00100000>;	// CCSRBAR 1M
 		bus-frequency = <0>;
 
+		mem-ctrl@2000 {
+			device_type = "mem-ctrl";
+			compatible = "85xx";
+			reg = <2000 1000>;
+			interrupt-parent = <&mpic>;
+			interrupts = <2 2>;
+		};
+
+		l2-cache@20000 {
+			device_type = "l2-cache";
+			compatible = "85xx";
+			reg = <20000 1000>;
+			cache-line-size = <20>;	// 32 bytes
+			cache-size = <40000>;	// L2, 256K
+			32-bit;
+			interrupt-parent = <&mpic>;
+			interrupts = <0 2>;
+		};
+
 		i2c@3000 {
 			device_type = "i2c";
 			compatible = "fsl-i2c";
diff --git a/arch/powerpc/boot/dts/mpc8548cds.dts b/arch/powerpc/boot/dts/mpc8548cds.dts
index b2b2200..f8a9f3f 100644
--- a/arch/powerpc/boot/dts/mpc8548cds.dts
+++ b/arch/powerpc/boot/dts/mpc8548cds.dts
@@ -48,6 +48,25 @@
 		reg = <e0000000 00100000>;	// CCSRBAR 1M
 		bus-frequency = <0>;
 
+		mem-ctrl@2000 {
+			device_type = "mem-ctrl";
+			compatible = "85xx";
+			reg = <2000 1000>;
+			interrupt-parent = <&mpic>;
+			interrupts = <2 2>;
+		};
+
+		l2-cache@20000 {
+			device_type = "l2-cache";
+			compatible = "85xx";
+			reg = <20000 1000>;
+			cache-line-size = <20>;	// 32 bytes
+			cache-size = <40000>;	// L2, 256K
+			32-bit;
+			interrupt-parent = <&mpic>;
+			interrupts = <0 2>;
+		};
+
 		i2c@3000 {
 			device_type = "i2c";
 			compatible = "fsl-i2c";
diff --git a/arch/powerpc/boot/dts/mpc8560ads.dts b/arch/powerpc/boot/dts/mpc8560ads.dts
index 1f2afe9..43ee847 100644
--- a/arch/powerpc/boot/dts/mpc8560ads.dts
+++ b/arch/powerpc/boot/dts/mpc8560ads.dts
@@ -48,6 +48,25 @@
 		reg = <e0000000 00000200>;
 		bus-frequency = <13ab6680>;
 
+		mem-ctrl@2000 {
+			device_type = "mem-ctrl";
+			compatible = "85xx";
+			reg = <2000 1000>;
+			interrupt-parent = <&mpic>;
+			interrupts = <2 2>;
+		};
+
+		l2-cache@20000 {
+			device_type = "l2-cache";
+			compatible = "85xx";
+			reg = <20000 1000>;
+			cache-line-size = <20>;	// 32 bytes
+			cache-size = <40000>;	// L2, 256K
+			32-bit;
+			interrupt-parent = <&mpic>;
+			interrupts = <0 2>;
+		};
+
 		mdio@24520 {
 			device_type = "mdio";
 			compatible = "gianfar";
@@ -110,7 +129,7 @@
 			#address-cells = <3>;
 			compatible = "85xx";
 			device_type = "pci";
-			reg = <8000 400>;
+			reg = <8000 1000>;
 			clock-frequency = <3f940aa>;
 			interrupt-map-mask = <f800 0 0 7>;
 			interrupt-map = <
diff --git a/arch/powerpc/sysdev/fsl_soc.c b/arch/powerpc/sysdev/fsl_soc.c
index 8a123c7..a0beb8b 100644
--- a/arch/powerpc/sysdev/fsl_soc.c
+++ b/arch/powerpc/sysdev/fsl_soc.c
@@ -1103,3 +1103,118 @@ err:
 arch_initcall(cpm_smc_uart_of_init);
 
 #endif /* CONFIG_8xx */
+
+/* platform device setup for EDAC */
+#ifdef CONFIG_PPC_85xx
+static int __init mpc85xx_mc_err_init(void)
+{
+	struct resource r[2];
+	struct device_node *np;
+	struct platform_device *pdev;
+	int ret = 0;
+
+	memset(r, 0, sizeof(r));
+
+	np = of_find_compatible_node(NULL, "mem-ctrl", "85xx");
+	if (!np)
+		return 0;
+
+	ret = of_address_to_resource(np, 0, &r[0]);
+	if (ret)
+		goto err;
+
+	of_irq_to_resource(np, 0, &r[1]);
+
+	of_node_put(np);
+
+	pdev = platform_device_register_simple("mpc85xx_mc_err", 0, r, 2);
+	if (IS_ERR(pdev))
+		return PTR_ERR(pdev);
+
+	return 0;
+
+err:
+	of_node_put(np);
+	printk(KERN_WARNING "mpc85xx-mem-ctrl setup failed\n");
+	return 0;
+}
+arch_initcall(mpc85xx_mc_err_init);
+
+static int __init mpc85xx_l2_err_init(void)
+{
+	struct resource r[2];
+	struct device_node *np;
+	struct platform_device *pdev;
+	int ret = 0;
+
+	memset(r, 0, sizeof(r));
+
+	np = of_find_compatible_node(NULL, "l2-cache", "85xx");
+	if (!np)
+		return 0;
+
+	ret = of_address_to_resource(np, 0, &r[0]);
+	if (ret)
+		goto err;
+	
+	/* we only need access to the error registers */
+	r[0].start += 0xe00;
+
+	of_irq_to_resource(np, 0, &r[1]);
+
+	of_node_put(np);
+
+	pdev = platform_device_register_simple("mpc85xx_l2_err", 0, r, 2);
+	if (IS_ERR(pdev))
+		return PTR_ERR(pdev);
+
+	return 0;
+
+err:
+	of_node_put(np);
+	printk(KERN_WARNING "mpc85xx-l2 setup failed\n");
+	return 0;
+}
+arch_initcall(mpc85xx_l2_err_init);
+
+static int __init mpc85xx_pci_err_init(void)
+{
+	struct resource r[2];
+	struct device_node *np = NULL;
+	struct platform_device *pdev;
+	int i;
+	int ret = 0;
+
+	for (i = 0;
+		(np = of_find_compatible_node(np, "pci", "85xx"));
+		i++) {
+		memset(r, 0, sizeof(r));
+
+		ret = of_address_to_resource(np, 0, &r[0]);
+		if (ret)
+			goto err;
+	
+		/* we only need access to the error registers */
+		r[0].start += 0xe00;
+
+		of_irq_to_resource(np, 0, &r[1]);
+
+		pdev = platform_device_register_simple("mpc85xx_pci_err", 
+				i, r, 2);
+		if (IS_ERR(pdev)) {
+			ret = PTR_ERR(pdev);
+			goto err;
+		}
+
+	}
+
+	return 0;
+
+err:
+	of_node_put(np);
+	printk(KERN_WARNING "mpc85xx-pci setup failed\n");
+	return ret;
+}
+arch_initcall(mpc85xx_pci_err_init);
+
+#endif /* CONFIG_PPC_85xx */

Re: [PATCH] powerpc: Add EDAC platform devices for 85xx

[Kumar Gala]

On Apr 25, 2007, at 7:37 PM, Dave Jiang wrote:

> Add memory controller and l2-cache controller entries in the dts  
> files for 85xx
> platforms. Also adding code to create the platform devices used by  
> EDAC drivers
> to claim the resources in order to access the error registers and  
> interrupts.
>
> Signed-off-by: Dave Jiang <djiang@mvista.com>
>
> ---
>
> Removed explicit phandles as suggested by David Gibson.
>
>  arch/powerpc/boot/dts/mpc8540ads.dts |   19 ++++++
>  arch/powerpc/boot/dts/mpc8548cds.dts |   19 ++++++
>  arch/powerpc/boot/dts/mpc8560ads.dts |   21 ++++++-
>  arch/powerpc/sysdev/fsl_soc.c        |  115 +++++++++++++++++++++++ 
> +++++++++++
>  4 files changed, 173 insertions(+), 1 deletions(-)
>
> diff --git a/arch/powerpc/boot/dts/mpc8540ads.dts b/arch/powerpc/ 
> boot/dts/mpc8540ads.dts
> index f261d64..63c274d 100644
> --- a/arch/powerpc/boot/dts/mpc8540ads.dts
> +++ b/arch/powerpc/boot/dts/mpc8540ads.dts
> @@ -48,6 +48,25 @@
>  		reg = <e0000000 00100000>;	// CCSRBAR 1M
>  		bus-frequency = <0>;
>
> +		mem-ctrl@2000 {
> +			device_type = "mem-ctrl";
> +			compatible = "85xx";

I'm concerned this is too generic.

> +			reg = <2000 1000>;
> +			interrupt-parent = <&mpic>;
> +			interrupts = <2 2>;
> +		};
> +
> +		l2-cache@20000 {
> +			device_type = "l2-cache";
> +			compatible = "85xx";
> +			reg = <20000 1000>;
> +			cache-line-size = <20>;	// 32 bytes
> +			cache-size = <40000>;	// L2, 256K
> +			32-bit;

Why the 32-bit?

> +			interrupt-parent = <&mpic>;
> +			interrupts = <0 2>;
> +		};
> +
>  		i2c@3000 {
>  			device_type = "i2c";
>  			compatible = "fsl-i2c";
> diff --git a/arch/powerpc/boot/dts/mpc8548cds.dts b/arch/powerpc/ 
> boot/dts/mpc8548cds.dts
> index b2b2200..f8a9f3f 100644
> --- a/arch/powerpc/boot/dts/mpc8548cds.dts
> +++ b/arch/powerpc/boot/dts/mpc8548cds.dts
> @@ -48,6 +48,25 @@
>  		reg = <e0000000 00100000>;	// CCSRBAR 1M
>  		bus-frequency = <0>;
>
> +		mem-ctrl@2000 {
> +			device_type = "mem-ctrl";
> +			compatible = "85xx";
> +			reg = <2000 1000>;
> +			interrupt-parent = <&mpic>;
> +			interrupts = <2 2>;
> +		};
> +
> +		l2-cache@20000 {
> +			device_type = "l2-cache";
> +			compatible = "85xx";
> +			reg = <20000 1000>;
> +			cache-line-size = <20>;	// 32 bytes
> +			cache-size = <40000>;	// L2, 256K
> +			32-bit;
> +			interrupt-parent = <&mpic>;
> +			interrupts = <0 2>;
> +		};
> +
>  		i2c@3000 {
>  			device_type = "i2c";
>  			compatible = "fsl-i2c";
> diff --git a/arch/powerpc/boot/dts/mpc8560ads.dts b/arch/powerpc/ 
> boot/dts/mpc8560ads.dts
> index 1f2afe9..43ee847 100644
> --- a/arch/powerpc/boot/dts/mpc8560ads.dts
> +++ b/arch/powerpc/boot/dts/mpc8560ads.dts
> @@ -48,6 +48,25 @@
>  		reg = <e0000000 00000200>;
>  		bus-frequency = <13ab6680>;
>
> +		mem-ctrl@2000 {
> +			device_type = "mem-ctrl";
> +			compatible = "85xx";
> +			reg = <2000 1000>;
> +			interrupt-parent = <&mpic>;
> +			interrupts = <2 2>;
> +		};
> +
> +		l2-cache@20000 {
> +			device_type = "l2-cache";
> +			compatible = "85xx";
> +			reg = <20000 1000>;
> +			cache-line-size = <20>;	// 32 bytes
> +			cache-size = <40000>;	// L2, 256K
> +			32-bit;
> +			interrupt-parent = <&mpic>;
> +			interrupts = <0 2>;
> +		};
> +
>  		mdio@24520 {
>  			device_type = "mdio";
>  			compatible = "gianfar";
> @@ -110,7 +129,7 @@
>  			#address-cells = <3>;
>  			compatible = "85xx";
>  			device_type = "pci";
> -			reg = <8000 400>;
> +			reg = <8000 1000>;
>  			clock-frequency = <3f940aa>;
>  			interrupt-map-mask = <f800 0 0 7>;
>  			interrupt-map = <
> diff --git a/arch/powerpc/sysdev/fsl_soc.c b/arch/powerpc/sysdev/ 
> fsl_soc.c
> index 8a123c7..a0beb8b 100644
> --- a/arch/powerpc/sysdev/fsl_soc.c
> +++ b/arch/powerpc/sysdev/fsl_soc.c
> @@ -1103,3 +1103,118 @@ err:
>  arch_initcall(cpm_smc_uart_of_init);
>
>  #endif /* CONFIG_8xx */
> +
> +/* platform device setup for EDAC */

Why not have the EDAC code for 85xx use of_device?

> +#ifdef CONFIG_PPC_85xx
> +static int __init mpc85xx_mc_err_init(void)
> +{
> +	struct resource r[2];
> +	struct device_node *np;
> +	struct platform_device *pdev;
> +	int ret = 0;
> +
> +	memset(r, 0, sizeof(r));
> +
> +	np = of_find_compatible_node(NULL, "mem-ctrl", "85xx");
> +	if (!np)
> +		return 0;
> +
> +	ret = of_address_to_resource(np, 0, &r[0]);
> +	if (ret)
> +		goto err;
> +
> +	of_irq_to_resource(np, 0, &r[1]);
> +
> +	of_node_put(np);
> +
> +	pdev = platform_device_register_simple("mpc85xx_mc_err", 0, r, 2);
> +	if (IS_ERR(pdev))
> +		return PTR_ERR(pdev);
> +
> +	return 0;
> +
> +err:
> +	of_node_put(np);
> +	printk(KERN_WARNING "mpc85xx-mem-ctrl setup failed\n");
> +	return 0;
> +}
> +arch_initcall(mpc85xx_mc_err_init);
> +
> +static int __init mpc85xx_l2_err_init(void)
> +{
> +	struct resource r[2];
> +	struct device_node *np;
> +	struct platform_device *pdev;
> +	int ret = 0;
> +
> +	memset(r, 0, sizeof(r));
> +
> +	np = of_find_compatible_node(NULL, "l2-cache", "85xx");
> +	if (!np)
> +		return 0;
> +
> +	ret = of_address_to_resource(np, 0, &r[0]);
> +	if (ret)
> +		goto err;
> +	
> +	/* we only need access to the error registers */
> +	r[0].start += 0xe00;
> +
> +	of_irq_to_resource(np, 0, &r[1]);
> +
> +	of_node_put(np);
> +
> +	pdev = platform_device_register_simple("mpc85xx_l2_err", 0, r, 2);
> +	if (IS_ERR(pdev))
> +		return PTR_ERR(pdev);
> +
> +	return 0;
> +
> +err:
> +	of_node_put(np);
> +	printk(KERN_WARNING "mpc85xx-l2 setup failed\n");
> +	return 0;
> +}
> +arch_initcall(mpc85xx_l2_err_init);
> +
> +static int __init mpc85xx_pci_err_init(void)
> +{
> +	struct resource r[2];
> +	struct device_node *np = NULL;
> +	struct platform_device *pdev;
> +	int i;
> +	int ret = 0;
> +
> +	for (i = 0;
> +		(np = of_find_compatible_node(np, "pci", "85xx"));
> +		i++) {
> +		memset(r, 0, sizeof(r));
> +
> +		ret = of_address_to_resource(np, 0, &r[0]);
> +		if (ret)
> +			goto err;
> +	
> +		/* we only need access to the error registers */
> +		r[0].start += 0xe00;
> +
> +		of_irq_to_resource(np, 0, &r[1]);
> +
> +		pdev = platform_device_register_simple("mpc85xx_pci_err",
> +				i, r, 2);
> +		if (IS_ERR(pdev)) {
> +			ret = PTR_ERR(pdev);
> +			goto err;
> +		}
> +
> +	}
> +
> +	return 0;
> +
> +err:
> +	of_node_put(np);
> +	printk(KERN_WARNING "mpc85xx-pci setup failed\n");
> +	return ret;
> +}
> +arch_initcall(mpc85xx_pci_err_init);
> +
> +#endif /* CONFIG_PPC_85xx */

Re: [PATCH] powerpc: Add EDAC platform devices for 85xx

[Dave Jiang]

Kumar Gala wrote:
> On Apr 25, 2007, at 7:37 PM, Dave Jiang wrote:
>
>   
>> +		mem-ctrl@2000 {
>> +			device_type = "mem-ctrl";
>> +			compatible = "85xx";
>>     
> I'm concerned this is too generic.
>   
I'm supposing not all 85xx uses the same soc? Is there something more
appropriate you can suggest? Thx!

>> +			reg = <2000 1000>;
>> +			interrupt-parent = <&mpic>;
>> +			interrupts = <2 2>;
>> +		};
>> +
>> +		l2-cache@20000 {
>> +			device_type = "l2-cache";
>> +			compatible = "85xx";
>> +			reg = <20000 1000>;
>> +			cache-line-size = <20>;	// 32 bytes
>> +			cache-size = <40000>;	// L2, 256K
>> +			32-bit;
>>     
>
> Why the 32-bit?
>   
No reason particular. Bad assumption. I will remove it.
>> diff --git a/arch/powerpc/sysdev/fsl_soc.c b/arch/powerpc/sysdev/ 
>> fsl_soc.c
>> index 8a123c7..a0beb8b 100644
>> --- a/arch/powerpc/sysdev/fsl_soc.c
>> +++ b/arch/powerpc/sysdev/fsl_soc.c
>> @@ -1103,3 +1103,118 @@ err:
>>  arch_initcall(cpm_smc_uart_of_init);
>>
>>  #endif /* CONFIG_8xx */
>> +
>> +/* platform device setup for EDAC */
>>     
>
> Why not have the EDAC code for 85xx use of_device?
>
>   
I will give that a try.

Re: [PATCH] powerpc: Add EDAC platform devices for 85xx

[Segher Boessenkool]

>>> +		mem-ctrl@2000 {
>>> +			device_type = "mem-ctrl";
>>> +			compatible = "85xx";
>>>
>> I'm concerned this is too generic.
>>
> I'm supposing not all 85xx uses the same soc? Is there something more
> appropriate you can suggest? Thx!

"name" = "memory-controller"
"compatible" = "fsl,85xx-memory-controller"
(or a more specific 85xx model if the controller
isn't identical across those chips)
No "device_type" at all, since there is no binding
for this kind of device.


Segher

RFC: new device types in the device tree (RE: [PATCH] powerpc: Add EDAC platform devices for 85xx)

[Yoder Stuart-B08248]

=20

> -----Original Message-----
> From: linuxppc-dev-bounces+b08248=3Dfreescale.com@ozlabs.org=20
> [mailto:linuxppc-dev-bounces+b08248=3Dfreescale.com@ozlabs.org]=20
> On Behalf Of Segher Boessenkool
> Sent: Thursday, April 26, 2007 1:57 PM
> To: Dave Jiang
> Cc: linuxppc-dev@ozlabs.org; david@gibson.dropbear.id.au;=20
> bluesmoke-devel@lists.sourceforge.net
> Subject: Re: [PATCH] powerpc: Add EDAC platform devices for 85xx
>=20
> >>> +		mem-ctrl@2000 {
> >>> +			device_type =3D "mem-ctrl";
> >>> +			compatible =3D "85xx";
> >>>
> >> I'm concerned this is too generic.
> >>
> > I'm supposing not all 85xx uses the same soc? Is there=20
> something more
> > appropriate you can suggest? Thx!
>=20
> "name" =3D "memory-controller"
> "compatible" =3D "fsl,85xx-memory-controller"
> (or a more specific 85xx model if the controller
> isn't identical across those chips)
> No "device_type" at all, since there is no binding
> for this kind of device.

Is "no device_type" really the approach that should be
taken? =20

booting-without-of.txt currently reads:

   Every node which actually represents an actual device
   (that is, a node which isn't only a virtual "container"
   for more nodes, like "/cpus" is) is also required to
   have a "device_type" property indicating the type of
   node

The 1275 spec is 'Open Firmware centric' in that it says
you don't need a device_type if the node is not used
by Open Firmware.

What should the approach be for new device types that
keep popping up?   If the device type is generally useful
I think it makes sense to create a binding and add it to
booting-without-of.txt-- essentially documenting the
required properties, their values, and what they mean.

If it is vendor specific, that vendor should create some
vendors specific doc for their bindings--
Documentation/powerpc/fsl-of-dev-bindings.txt.

Comments?

Thanks,
Stuart Yoder

Re: RFC: new device types in the device tree (RE: [PATCH] powerpc: Add EDAC platform devices for 85xx)

[Segher Boessenkool]

>> "name" = "memory-controller"
>> "compatible" = "fsl,85xx-memory-controller"
>> (or a more specific 85xx model if the controller
>> isn't identical across those chips)
>> No "device_type" at all, since there is no binding
>> for this kind of device.
>
> Is "no device_type" really the approach that should be
> taken?

Yes.

> booting-without-of.txt currently reads:
>
>    Every node which actually represents an actual device
>    (that is, a node which isn't only a virtual "container"
>    for more nodes, like "/cpus" is) is also required to
>    have a "device_type" property indicating the type of
>    node

That is wrong, IMNSHO.

> The 1275 spec is 'Open Firmware centric' in that it says
> you don't need a device_type if the node is not used
> by Open Firmware.

It is "Open firmware centric" in every way; it is
the Open firmware definition after all.

"device_type" specifies what firmware interfaces a
node implements.  "name" and "compatible" are for
the client (i.e., OS, bootloader, etc.) to use for
matching drivers to device nodes.

It typically makes no sense to create a new "device_type"
value for anything else than very generic classes of
device, where you can drive the device without knowing
anything else than what is defined in the binding for
that "device_type".

> What should the approach be for new device types that
> keep popping up?   If the device type is generally useful
> I think it makes sense to create a binding and add it to
> booting-without-of.txt-- essentially documenting the
> required properties, their values, and what they mean.

And the associated methods and anything else that is
needed.  It would be a big mistake to create incompatibilities
with "real OF" where those can be avoided just as easily.

> If it is vendor specific, that vendor should create some
> vendors specific doc for their bindings--
> Documentation/powerpc/fsl-of-dev-bindings.txt.
>
> Comments?

In short: having a "device_type" in your node doesn't
help at all if all users need to know the "compatible"
property as well; and it _does_ hurt.


Segher

Re: RFC: new device types in the device tree (RE: [PATCH] powerpc: Add EDAC platform devices for 85xx)

[David Gibson]

On Wed, May 02, 2007 at 02:34:45AM +0200, Segher Boessenkool wrote:
> >> "name" = "memory-controller"
> >> "compatible" = "fsl,85xx-memory-controller"
> >> (or a more specific 85xx model if the controller
> >> isn't identical across those chips)
> >> No "device_type" at all, since there is no binding
> >> for this kind of device.
> >
> > Is "no device_type" really the approach that should be
> > taken?
> 
> Yes.
> 
> > booting-without-of.txt currently reads:
> >
> >    Every node which actually represents an actual device
> >    (that is, a node which isn't only a virtual "container"
> >    for more nodes, like "/cpus" is) is also required to
> >    have a "device_type" property indicating the type of
> >    node
> 
> That is wrong, IMNSHO.

I tend to agree. Device drivers should generally be searching on the
"compatible" property, not "device_type".  Defining new device_type
values isn't really of any use to the kernel, so we should just avoid
it.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

RE: RFC: new device types in the device tree (RE: [PATCH] powerpc: Add EDAC platform devices for 85xx)

[Yoder Stuart-B08248]

=20

> -----Original Message-----
> From: David Gibson [mailto:david@gibson.dropbear.id.au]=20
> Sent: Tuesday, May 01, 2007 8:20 PM
> To: Segher Boessenkool
> Cc: Yoder Stuart-B08248; linuxppc-dev@ozlabs.org;=20
> bluesmoke-devel@lists.sourceforge.net
> Subject: Re: RFC: new device types in the device tree (RE:=20
> [PATCH] powerpc: Add EDAC platform devices for 85xx)
>=20
> On Wed, May 02, 2007 at 02:34:45AM +0200, Segher Boessenkool wrote:
> > >> "name" =3D "memory-controller"
> > >> "compatible" =3D "fsl,85xx-memory-controller"
> > >> (or a more specific 85xx model if the controller
> > >> isn't identical across those chips)
> > >> No "device_type" at all, since there is no binding
> > >> for this kind of device.
> > >
> > > Is "no device_type" really the approach that should be
> > > taken?
> >=20
> > Yes.
> >=20
> > > booting-without-of.txt currently reads:
> > >
> > >    Every node which actually represents an actual device
> > >    (that is, a node which isn't only a virtual "container"
> > >    for more nodes, like "/cpus" is) is also required to
> > >    have a "device_type" property indicating the type of
> > >    node
> >=20
> > That is wrong, IMNSHO.
>=20
> I tend to agree. Device drivers should generally be searching on the
> "compatible" property, not "device_type".  Defining new device_type
> values isn't really of any use to the kernel, so we should just avoid
> it.

Right-- drivers search on "compatible".

But, don't we want to keep standardized sets of properties for
certain classes/types of devices?  Defining a standardized,
required set of properties for a "network", "rom", or "i2c"
class of device is helpful.  Without a standardized 'template'
of properties, developers may make up whatever properties they
want and things will work fine as long as the device tree and
driver are in sync.  It works, but you wind up with a plethora
of properties each describing the same thing.

Stuart



If we do away with device_type,
what is it that defines a particular node to be a certain class
of device.

Re: RFC: new device types in the device tree (RE: [PATCH] powerpc: Add EDAC platform devices for 85xx)

[David Gibson]

On Wed, May 02, 2007 at 12:04:11PM -0700, Yoder Stuart-B08248 wrote:
>  
> 
> > -----Original Message-----
> > From: David Gibson [mailto:david@gibson.dropbear.id.au] 
> > Sent: Tuesday, May 01, 2007 8:20 PM
> > To: Segher Boessenkool
> > Cc: Yoder Stuart-B08248; linuxppc-dev@ozlabs.org; 
> > bluesmoke-devel@lists.sourceforge.net
> > Subject: Re: RFC: new device types in the device tree (RE: 
> > [PATCH] powerpc: Add EDAC platform devices for 85xx)
> > 
> > On Wed, May 02, 2007 at 02:34:45AM +0200, Segher Boessenkool wrote:
> > > >> "name" = "memory-controller"
> > > >> "compatible" = "fsl,85xx-memory-controller"
> > > >> (or a more specific 85xx model if the controller
> > > >> isn't identical across those chips)
> > > >> No "device_type" at all, since there is no binding
> > > >> for this kind of device.
> > > >
> > > > Is "no device_type" really the approach that should be
> > > > taken?
> > > 
> > > Yes.
> > > 
> > > > booting-without-of.txt currently reads:
> > > >
> > > >    Every node which actually represents an actual device
> > > >    (that is, a node which isn't only a virtual "container"
> > > >    for more nodes, like "/cpus" is) is also required to
> > > >    have a "device_type" property indicating the type of
> > > >    node
> > > 
> > > That is wrong, IMNSHO.
> > 
> > I tend to agree. Device drivers should generally be searching on the
> > "compatible" property, not "device_type".  Defining new device_type
> > values isn't really of any use to the kernel, so we should just avoid
> > it.
> 
> Right-- drivers search on "compatible".
> 
> But, don't we want to keep standardized sets of properties for
> certain classes/types of devices?  Defining a standardized,
> required set of properties for a "network", "rom", or "i2c"
> class of device is helpful.  Without a standardized 'template'
> of properties, developers may make up whatever properties they
> want and things will work fine as long as the device tree and
> driver are in sync.  It works, but you wind up with a plethora
> of properties each describing the same thing.

If there's an obvious new class, with common properties, then yes,
sure.  But I don't think we need to feel impelled to think up new
classes (and therefore a device_type value) for each new device.  

And even in the case of new classes, I think we might be best off
waiting for a few devices to appear so we can tell what's really
common information before we define the class's device_type and
required properties.

> If we do away with device_type,
> what is it that defines a particular node to be a certain class
> of device.

I'm not suggesting doing away with device_type, just making it
optional.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

Re: RFC: new device types in the device tree (RE: [PATCH] powerpc: Add EDAC platform devices for 85xx)

[Segher Boessenkool]

> If there's an obvious new class, with common properties, then yes,
> sure.  But I don't think we need to feel impelled to think up new
> classes (and therefore a device_type value) for each new device.
>
> And even in the case of new classes, I think we might be best off
> waiting for a few devices to appear so we can tell what's really
> common information before we define the class's device_type and
> required properties.

Nicely put, thanks.


Segher

RE: RFC: new device types in the device tree (RE: [PATCH] powerpc: Add EDAC platform devices for 85xx)

[Yoder Stuart-B08248]

=20

> -----Original Message-----
> From: David Gibson [mailto:david@gibson.dropbear.id.au]=20
> Sent: Wednesday, May 02, 2007 7:17 PM
> To: Yoder Stuart-B08248
> Cc: Segher Boessenkool; linuxppc-dev@ozlabs.org;=20
> bluesmoke-devel@lists.sourceforge.net
> Subject: Re: RFC: new device types in the device tree (RE:=20
> [PATCH] powerpc: Add EDAC platform devices for 85xx)
>=20
> On Wed, May 02, 2007 at 12:04:11PM -0700, Yoder Stuart-B08248 wrote:
> > =20
> >=20
> > > -----Original Message-----
> > > From: David Gibson [mailto:david@gibson.dropbear.id.au]=20
> > > Sent: Tuesday, May 01, 2007 8:20 PM
> > > To: Segher Boessenkool
> > > Cc: Yoder Stuart-B08248; linuxppc-dev@ozlabs.org;=20
> > > bluesmoke-devel@lists.sourceforge.net
> > > Subject: Re: RFC: new device types in the device tree (RE:=20
> > > [PATCH] powerpc: Add EDAC platform devices for 85xx)
> > >=20
> > > On Wed, May 02, 2007 at 02:34:45AM +0200, Segher=20
> Boessenkool wrote:
> > > > >> "name" =3D "memory-controller"
> > > > >> "compatible" =3D "fsl,85xx-memory-controller"
> > > > >> (or a more specific 85xx model if the controller
> > > > >> isn't identical across those chips)
> > > > >> No "device_type" at all, since there is no binding
> > > > >> for this kind of device.
> > > > >
> > > > > Is "no device_type" really the approach that should be
> > > > > taken?
> > > >=20
> > > > Yes.
> > > >=20
> > > > > booting-without-of.txt currently reads:
> > > > >
> > > > >    Every node which actually represents an actual device
> > > > >    (that is, a node which isn't only a virtual "container"
> > > > >    for more nodes, like "/cpus" is) is also required to
> > > > >    have a "device_type" property indicating the type of
> > > > >    node
> > > >=20
> > > > That is wrong, IMNSHO.
> > >=20
> > > I tend to agree. Device drivers should generally be=20
> searching on the
> > > "compatible" property, not "device_type".  Defining new=20
> device_type
> > > values isn't really of any use to the kernel, so we=20
> should just avoid
> > > it.
> >=20
> > Right-- drivers search on "compatible".
> >=20
> > But, don't we want to keep standardized sets of properties for
> > certain classes/types of devices?  Defining a standardized,
> > required set of properties for a "network", "rom", or "i2c"
> > class of device is helpful.  Without a standardized 'template'
> > of properties, developers may make up whatever properties they
> > want and things will work fine as long as the device tree and
> > driver are in sync.  It works, but you wind up with a plethora
> > of properties each describing the same thing.
>=20
> If there's an obvious new class, with common properties, then yes,
> sure.  But I don't think we need to feel impelled to think up new
> classes (and therefore a device_type value) for each new device. =20
>=20
> And even in the case of new classes, I think we might be best off
> waiting for a few devices to appear so we can tell what's really
> common information before we define the class's device_type and
> required properties.

I think this makes sense.  I have seen various new device_type=20
properties popping up that seem very generic.  They simply have
the standard list of "reg",  "compatible", "interrupts",
"interrupt-parent".

I think one thing that would be very helpful would be a=20
crisp list of what the standardized device_types are so
when people create new device trees they have a guideline.
I'll take a stab at adding a section to booting-without-of.txt
for this.

Stuart

Re: RFC: new device types in the device tree (RE: [PATCH] powerpc: Add EDAC platform devices for 85xx)

[Segher Boessenkool]

> But, don't we want to keep standardized sets of properties for
> certain classes/types of devices?

Sure.

> Defining a standardized,
> required set of properties for a "network", "rom", or "i2c"
> class of device is helpful.

Only if such a device binding is generic enough to actually
handle all such devices.  Sure a specific device may have
some extra properties, but all the basic stuff should be
in standard properties.

> Without a standardized 'template'
> of properties, developers may make up whatever properties they
> want and things will work fine as long as the device tree and
> driver are in sync.  It works, but you wind up with a plethora
> of properties each describing the same thing.

You might think this is bad, but IMO it is better to
have several drivers doing similar things in different
ways, than to force all drivers to use a binding that
doesn't fit them well.

> If we do away with device_type,
> what is it that defines a particular node to be a certain class
> of device.

The matching device driver knows (and it knows a great
deal more about the device, hopefully ;-) )


Segher

RE: RFC: new device types in the device tree (RE: [PATCH] powerpc: Add EDAC platform devices for 85xx)

[Yoder Stuart-B08248]

=20

> -----Original Message-----
> From: Segher Boessenkool [mailto:segher@kernel.crashing.org]=20
> Sent: Tuesday, May 01, 2007 7:35 PM
> To: Yoder Stuart-B08248
> Cc: Dave Jiang; linuxppc-dev@ozlabs.org;=20
> david@gibson.dropbear.id.au; bluesmoke-devel@lists.sourceforge.net
> Subject: Re: RFC: new device types in the device tree (RE:=20
> [PATCH] powerpc: Add EDAC platform devices for 85xx)
>=20
> >> "name" =3D "memory-controller"
> >> "compatible" =3D "fsl,85xx-memory-controller"
> >> (or a more specific 85xx model if the controller
> >> isn't identical across those chips)
> >> No "device_type" at all, since there is no binding
> >> for this kind of device.
> >
> > Is "no device_type" really the approach that should be
> > taken?
>=20
> Yes.
>=20
> > booting-without-of.txt currently reads:
> >
> >    Every node which actually represents an actual device
> >    (that is, a node which isn't only a virtual "container"
> >    for more nodes, like "/cpus" is) is also required to
> >    have a "device_type" property indicating the type of
> >    node
>=20
> That is wrong, IMNSHO.
>=20
> > The 1275 spec is 'Open Firmware centric' in that it says
> > you don't need a device_type if the node is not used
> > by Open Firmware.
>=20
> It is "Open firmware centric" in every way; it is
> the Open firmware definition after all.
>=20
> "device_type" specifies what firmware interfaces a
> node implements.  "name" and "compatible" are for
> the client (i.e., OS, bootloader, etc.) to use for
> matching drivers to device nodes.

The flat device tree standard (i.e. booting-without-of.txt)
certainly inherited stuff from IEEE-1275, but can diverge
if need be.

The document currently defines the device_type property
to specify the class of device.  And it's required for
nodes describing a device.

name is always optional (as defined by b-w-of.txt)

There are no interfaces or methods with the device trees
defined in DTS files.  Given that, does the the device_type
property have any value at all?

There are a slew of device_types defined in the DTS files
and in booting-without-of.txt.  Should all these be
removed?


> It typically makes no sense to create a new "device_type"
> value for anything else than very generic classes of
> device, where you can drive the device without knowing
> anything else than what is defined in the binding for
> that "device_type".

What types devices get a device_type and which don't?  Linux
drives the devices based on "compatible".

> > What should the approach be for new device types that
> > keep popping up?   If the device type is generally useful
> > I think it makes sense to create a binding and add it to
> > booting-without-of.txt-- essentially documenting the
> > required properties, their values, and what they mean.
>=20
> And the associated methods and anything else that is
> needed.  It would be a big mistake to create incompatibilities
> with "real OF" where those can be avoided just as easily.

But there are no methods.

Even if the kernel does not use 'device_type', the property
still seems to have some value in that it specifies a
required set of properties and their values for various
classes of devices.

Take a flash device for instance--

        flash@ff000000 {
                device_type =3D "rom";
                compatible =3D "direct-mapped";
                probe-type =3D "CFI";
                reg =3D <ff000000 01000000>;
                bank-width =3D <4>;
                partitions =3D <00000000 00f80000
                              00f80000 00080001>;
                partition-names =3D "fs\0firmware";
        };

I suppose we can delete the device_type property and the
kernel will still be happy, but it's nice to have a standard
set of properties that everyone uses when describing a given
class of device.  The device_type is just a convenient
identifer to refer to this device class and set of properties.

Stuart

Re: RFC: new device types in the device tree (RE: [PATCH] powerpc: Add EDAC platform devices for 85xx)

[Segher Boessenkool]

>>> The 1275 spec is 'Open Firmware centric' in that it says
>>> you don't need a device_type if the node is not used
>>> by Open Firmware.
>>
>> It is "Open firmware centric" in every way; it is
>> the Open firmware definition after all.
>>
>> "device_type" specifies what firmware interfaces a
>> node implements.  "name" and "compatible" are for
>> the client (i.e., OS, bootloader, etc.) to use for
>> matching drivers to device nodes.
>
> The flat device tree standard (i.e. booting-without-of.txt)
> certainly inherited stuff from IEEE-1275, but can diverge
> if need be.

It shares the kernel interface.  You cannot go diverge
on fundamental things.

> name is always optional (as defined by b-w-of.txt)

Where does it say that?  "name" is always required!  In
the newer flat tree format, it's even impossible to
create a node without "name".

> There are no interfaces or methods with the device trees
> defined in DTS files.  Given that, does the the device_type
> property have any value at all?

Not much, but still a bit, like you can find all
"serial" devices if you want to.  And of course,
many bindings require a specific "device_type",
so you should have one.

> There are a slew of device_types defined in the DTS files
> and in booting-without-of.txt.  Should all these be
> removed?

Most should yes.

>> It typically makes no sense to create a new "device_type"
>> value for anything else than very generic classes of
>> device, where you can drive the device without knowing
>> anything else than what is defined in the binding for
>> that "device_type".
>
> What types devices get a device_type and which don't?  Linux
> drives the devices based on "compatible".

No, Linux selects which driver to use based on "compatible".
That's a crucial difference.

>> And the associated methods and anything else that is
>> needed.  It would be a big mistake to create incompatibilities
>> with "real OF" where those can be avoided just as easily.
>
> But there are no methods.

Again, you should not create incompatibilities where
not needed.  Since "device_type" is primarily useful
for (talking to / using) the firmware, and hardly
useful at all for the flat device tree, the flat tree
"standard" shouldn't define new "device_type"s without
making it a real OF binding.

> Even if the kernel does not use 'device_type', the property
> still seems to have some value in that it specifies a
> required set of properties and their values for various
> classes of devices.

Sure.  "network", "block", "serial", etc.

> Take a flash device for instance--

Yes, "flash" could be made into a nice generic binding
(except that no one has succeeded so far, there are just
way too many different kinds of flash/rom/nvram, and
many ways to hook them up).

> I suppose we can delete the device_type property and the
> kernel will still be happy, but it's nice to have a standard
> set of properties that everyone uses when describing a given
> class of device.  The device_type is just a convenient
> identifer to refer to this device class and set of properties.

Unfortunately, there is no standard "device_type" for
flash yet.


Segher

RE: RFC: new device types in the device tree (RE: [PATCH] powerpc: Add EDAC platform devices for 85xx)

[Yoder Stuart-B08248]

=20

> -----Original Message-----
> From: Segher Boessenkool [mailto:segher@kernel.crashing.org]=20
> Sent: Wednesday, May 02, 2007 7:48 PM
> To: Yoder Stuart-B08248
> Cc: Dave Jiang; linuxppc-dev@ozlabs.org;=20
> david@gibson.dropbear.id.au; bluesmoke-devel@lists.sourceforge.net
> Subject: Re: RFC: new device types in the device tree (RE:=20
> [PATCH] powerpc: Add EDAC platform devices for 85xx)
>=20
> >>> The 1275 spec is 'Open Firmware centric' in that it says
> >>> you don't need a device_type if the node is not used
> >>> by Open Firmware.
> >>
> >> It is "Open firmware centric" in every way; it is
> >> the Open firmware definition after all.
> >>
> >> "device_type" specifies what firmware interfaces a
> >> node implements.  "name" and "compatible" are for
> >> the client (i.e., OS, bootloader, etc.) to use for
> >> matching drivers to device nodes.
> >
> > The flat device tree standard (i.e. booting-without-of.txt)
> > certainly inherited stuff from IEEE-1275, but can diverge
> > if need be.
>=20
> It shares the kernel interface.  You cannot go diverge
> on fundamental things.
>=20
> > name is always optional (as defined by b-w-of.txt)
>=20
> Where does it say that?  "name" is always required!  In
> the newer flat tree format, it's even impossible to
> create a node without "name".

I'm specifically referring to the "name" property.  Here is
what booting-without-of.txt says:

   While earlier users of Open Firmware like OldWorld
   macintoshes tended to use the actual device name for
   the "name" property, it's nowadays considered a good
   practice to use a name that is closer to the device
   class (often equal to device_type)...[snip]... However,
   the kernel doesn't generally put any restriction on the
   "name" property; it is simply considered good practice
   to follow the standard and its evolutions as closely as
   possible.

   Note also that the new format version 16 makes the
   "name" property optional. If it's absent for a node, then
   the node's unit name is then used to reconstruct the name.
   That is, the part of the unit name before the "@" sign
   is used (or the entire unit name if no "@" sign is present).

If the name property is missing the kernel contructs it from
the unit name.  I guess my point is the name property implies
nothing and the kernel doesn't seem to care what name is used.
It certainly doesn't imply a required set of properties.

Stuart

Re: RFC: new device types in the device tree (RE: [PATCH] powerpc: Add EDAC platform devices for 85xx)

[Segher Boessenkool]

>>> name is always optional (as defined by b-w-of.txt)
>>
>> Where does it say that?  "name" is always required!  In
>> the newer flat tree format, it's even impossible to
>> create a node without "name".
>
> I'm specifically referring to the "name" property.  Here is
> what booting-without-of.txt says:

Okay, a few separate issues here...

>    While earlier users of Open Firmware like OldWorld
>    macintoshes tended to use the actual device name for
>    the "name" property, it's nowadays considered a good
>    practice to use a name that is closer to the device
>    class (often equal to device_type)...

Yes, this is known as "generic names" -- it makes device
trees more readable and easier to use (on real OF).  See
the "generic names" recommended practice document.

> [snip]... However,
>    the kernel doesn't generally put any restriction on the
>    "name" property; it is simply considered good practice
>    to follow the standard and its evolutions as closely as
>    possible.

When the kernel looks for a driver for a device node,
it should first consider "name" and after that the
entries in "compatible", in order.

Yes I know this is not what the kernel does most of
the time.

>    Note also that the new format version 16 makes the
>    "name" property optional. If it's absent for a node, then
>    the node's unit name is then used to reconstruct the name.
>    That is, the part of the unit name before the "@" sign
>    is used (or the entire unit name if no "@" sign is present).
>
> If the name property is missing the kernel contructs it from
> the unit name.

So there _always_ is a "name" property.  Who cares how
it is encoded in the DTB.

> I guess my point is the name property implies
> nothing and the kernel doesn't seem to care what name is used.

It does, in some cases.  See the bindings for "isa" and "pci"
busses for example (for the bus nodes, not the device nodes
under those).

> It certainly doesn't imply a required set of properties.

That depends on the specific binding for the type of
device; in most cases, some binding specific properties
_are_ required.


Segher

Re: [PATCH] powerpc: Add EDAC platform devices for 85xx

[Dave Jiang]

Kumar Gala wrote:
> On Apr 25, 2007, at 7:37 PM, Dave Jiang wrote:
>> +
>> +/* platform device setup for EDAC */
> 
> Why not have the EDAC code for 85xx use of_device?
-- 

Kumar,
After doing some testing and looking around, it seems that for the 85xx only
the mds code calls the mpc85xx_publish_devices() which pulls in the soc devices
from the dts and creates them as of_devices. I do not see similar code for cds
and ads. Should that be added for ads and cds?

------------------------------------------------------
Dave Jiang
Software Engineer
MontaVista Software, Inc.
http://www.mvista.com
------------------------------------------------------

[PATCH] powerpc: publish 85xx soc devices as of_device on cds and ads

[Dave Jiang]

Publish the devices in dts under soc as of_device just like what 
mpc85xx_mds platforms do. 

Signed-off-by: Dave Jiang <djiang@mvista.com>

---
 arch/powerpc/platforms/85xx/mpc85xx_ads.c |   19 +++++++++++++++++++
 arch/powerpc/platforms/85xx/mpc85xx_cds.c |   19 +++++++++++++++++++
 2 files changed, 38 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/platforms/85xx/mpc85xx_ads.c b/arch/powerpc/platforms/85xx/mpc85xx_ads.c
index 5d27621..217cc13 100644
--- a/arch/powerpc/platforms/85xx/mpc85xx_ads.c
+++ b/arch/powerpc/platforms/85xx/mpc85xx_ads.c
@@ -18,6 +18,8 @@
 #include <linux/delay.h>
 #include <linux/seq_file.h>
 
+#include <asm/of_device.h>
+#include <asm/of_platform.h>
 #include <asm/system.h>
 #include <asm/time.h>
 #include <asm/machdep.h>
@@ -277,6 +279,23 @@ static int __init mpc85xx_ads_probe(void)
         return of_flat_dt_is_compatible(root, "MPC85xxADS");
 }
 
+static struct of_device_id mpc85xx_ids[] = {
+	{ .type = "soc", },
+	{ .compatible = "soc", },
+	{},
+};
+
+static int __init mpc85xx_publish_devices(void)
+{
+	if (!machine_is(mpc85xx_ads))
+		return 0;
+
+	of_platform_bus_probe(NULL, mpc85xx_ids, NULL);
+
+	return 0;
+}
+device_initcall(mpc85xx_publish_devices);
+
 define_machine(mpc85xx_ads) {
 	.name			= "MPC85xx ADS",
 	.probe			= mpc85xx_ads_probe,
diff --git a/arch/powerpc/platforms/85xx/mpc85xx_cds.c b/arch/powerpc/platforms/85xx/mpc85xx_cds.c
index 7e71636..e783d77 100644
--- a/arch/powerpc/platforms/85xx/mpc85xx_cds.c
+++ b/arch/powerpc/platforms/85xx/mpc85xx_cds.c
@@ -26,6 +26,8 @@
 #include <linux/module.h>
 #include <linux/fsl_devices.h>
 
+#include <asm/of_device.h>
+#include <asm/of_platform.h>
 #include <asm/system.h>
 #include <asm/pgtable.h>
 #include <asm/page.h>
@@ -296,6 +298,23 @@ static int __init mpc85xx_cds_probe(void)
         return of_flat_dt_is_compatible(root, "MPC85xxCDS");
 }
 
+static struct of_device_id mpc85xx_ids[] = {
+	{ .type = "soc", },
+	{ .compatible = "soc", },
+	{},
+};
+
+static int __init mpc85xx_publish_devices(void)
+{
+	if (!machine_is(mpc85xx_cds))
+		return 0;
+
+	of_platform_bus_probe(NULL, mpc85xx_ids, NULL);
+
+	return 0;
+}
+device_initcall(mpc85xx_publish_devices);
+
 define_machine(mpc85xx_cds) {
 	.name		= "MPC85xx CDS",
 	.probe		= mpc85xx_cds_probe,

[PATCH] powerpc: add dts entries to 85xx for EDAC

[Dave Jiang]

Adding memory-controller and l2-cache-controller entries to be used by EDAC as
of_devices.

Signed-off-by: Dave Jiang <djiang@mvista.com>

---

 arch/powerpc/boot/dts/mpc8540ads.dts |   16 ++++++++++++++++
 arch/powerpc/boot/dts/mpc8548cds.dts |   16 ++++++++++++++++
 arch/powerpc/boot/dts/mpc8560ads.dts |   18 +++++++++++++++++-
 3 files changed, 49 insertions(+), 1 deletions(-)

diff --git a/arch/powerpc/boot/dts/mpc8540ads.dts b/arch/powerpc/boot/dts/mpc8540ads.dts
index f261d64..f411bc1 100644
--- a/arch/powerpc/boot/dts/mpc8540ads.dts
+++ b/arch/powerpc/boot/dts/mpc8540ads.dts
@@ -48,6 +48,22 @@
 		reg = <e0000000 00100000>;	// CCSRBAR 1M
 		bus-frequency = <0>;
 
+		memory-controller@2000 {
+			compatible = "fsl,85xx-memory-controller";
+			reg = <2000 1000>;
+			interrupt-parent = <&mpic>;
+			interrupts = <2 2>;
+		};
+
+		l2-cache-controller@20000 {
+			compatible = "fsl,85xx-memory-controller";
+			reg = <20000 1000>;
+			cache-line-size = <20>;	// 32 bytes
+			cache-size = <40000>;	// L2, 256K
+			interrupt-parent = <&mpic>;
+			interrupts = <0 2>;
+		};
+
 		i2c@3000 {
 			device_type = "i2c";
 			compatible = "fsl-i2c";
diff --git a/arch/powerpc/boot/dts/mpc8548cds.dts b/arch/powerpc/boot/dts/mpc8548cds.dts
index b2b2200..9de5c3a 100644
--- a/arch/powerpc/boot/dts/mpc8548cds.dts
+++ b/arch/powerpc/boot/dts/mpc8548cds.dts
@@ -48,6 +48,22 @@
 		reg = <e0000000 00100000>;	// CCSRBAR 1M
 		bus-frequency = <0>;
 
+		memory-controller@2000 {
+			compatible = "fsl,85xx-memory-controller";
+			reg = <2000 1000>;
+			interrupt-parent = <&mpic>;
+			interrupts = <2 2>;
+		};
+
+		l2-cache-controller@20000 {
+			compatible = "fsl,85xx-l2-cache-controller";
+			reg = <20000 1000>;
+			cache-line-size = <20>;	// 32 bytes
+			cache-size = <40000>;	// L2, 256K
+			interrupt-parent = <&mpic>;
+			interrupts = <0 2>;
+		};
+
 		i2c@3000 {
 			device_type = "i2c";
 			compatible = "fsl-i2c";
diff --git a/arch/powerpc/boot/dts/mpc8560ads.dts b/arch/powerpc/boot/dts/mpc8560ads.dts
index 1f2afe9..6e59364 100644
--- a/arch/powerpc/boot/dts/mpc8560ads.dts
+++ b/arch/powerpc/boot/dts/mpc8560ads.dts
@@ -48,6 +48,22 @@
 		reg = <e0000000 00000200>;
 		bus-frequency = <13ab6680>;
 
+		memory-controller@2000 {
+			compatible = "fsl,85xx-memory-controller";
+			reg = <2000 1000>;
+			interrupt-parent = <&mpic>;
+			interrupts = <2 2>;
+		};
+
+		l2-cache-controller@20000 {
+			compatible = "fsl,85xx-l2-cache-controller";
+			reg = <20000 1000>;
+			cache-line-size = <20>;	// 32 bytes
+			cache-size = <40000>;	// L2, 256K
+			interrupt-parent = <&mpic>;
+			interrupts = <0 2>;
+		};
+
 		mdio@24520 {
 			device_type = "mdio";
 			compatible = "gianfar";
@@ -110,7 +126,7 @@
 			#address-cells = <3>;
 			compatible = "85xx";
 			device_type = "pci";
-			reg = <8000 400>;
+			reg = <8000 1000>;
 			clock-frequency = <3f940aa>;
 			interrupt-map-mask = <f800 0 0 7>;
 			interrupt-map = <

Re: [PATCH] powerpc: add dts entries to 85xx for EDAC

[Olof Johansson]

Hi,

On Mon, May 07, 2007 at 04:26:34PM -0700, Dave Jiang wrote:

> Adding memory-controller and l2-cache-controller entries to be used by EDAC as
> of_devices.
> 
> Signed-off-by: Dave Jiang <djiang@mvista.com>
> 
> ---
> 
>  arch/powerpc/boot/dts/mpc8540ads.dts |   16 ++++++++++++++++
>  arch/powerpc/boot/dts/mpc8548cds.dts |   16 ++++++++++++++++
>  arch/powerpc/boot/dts/mpc8560ads.dts |   18 +++++++++++++++++-
>  3 files changed, 49 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/powerpc/boot/dts/mpc8540ads.dts b/arch/powerpc/boot/dts/mpc8540ads.dts
> index f261d64..f411bc1 100644
> --- a/arch/powerpc/boot/dts/mpc8540ads.dts
> +++ b/arch/powerpc/boot/dts/mpc8540ads.dts
> @@ -48,6 +48,22 @@
>  		reg = <e0000000 00100000>;	// CCSRBAR 1M
>  		bus-frequency = <0>;
>  
> +		memory-controller@2000 {
> +			compatible = "fsl,85xx-memory-controller";
> +			reg = <2000 1000>;
> +			interrupt-parent = <&mpic>;
> +			interrupts = <2 2>;
> +		};
> +
> +		l2-cache-controller@20000 {
> +			compatible = "fsl,85xx-memory-controller";

Interesting. Is the programming model really the same for the memory and cache controller?


-Olof

Re: [PATCH] powerpc: add dts entries to 85xx for EDAC

[Dave Jiang]

Olof Johansson wrote:
> Hi,
> 
>> diff --git a/arch/powerpc/boot/dts/mpc8540ads.dts b/arch/powerpc/boot/dts/mpc8540ads.dts
>> index f261d64..f411bc1 100644
>> --- a/arch/powerpc/boot/dts/mpc8540ads.dts
>> +++ b/arch/powerpc/boot/dts/mpc8540ads.dts
>> @@ -48,6 +48,22 @@
>>  		reg = <e0000000 00100000>;	// CCSRBAR 1M
>>  		bus-frequency = <0>;
>>  
>> +		memory-controller@2000 {
>> +			compatible = "fsl,85xx-memory-controller";
>> +			reg = <2000 1000>;
>> +			interrupt-parent = <&mpic>;
>> +			interrupts = <2 2>;
>> +		};
>> +
>> +		l2-cache-controller@20000 {
>> +			compatible = "fsl,85xx-memory-controller";
> 
> Interesting. Is the programming model really the same for the memory and cache controller?
> 
> 
> -Olof

That's an oops on my part. Thanks for pointing it out.


---

-= Dave =-

Re: [PATCH] powerpc: add dts entries to 85xx for EDAC

[Kumar Gala]

On May 7, 2007, at 6:26 PM, Dave Jiang wrote:

> Adding memory-controller and l2-cache-controller entries to be used  
> by EDAC as
> of_devices.

we should distinguish the memory controllers on 8540/8560 (DDR1) from  
the one on 8548 (DDR1 & DDR2).

- k


>
> Signed-off-by: Dave Jiang <djiang@mvista.com>
>
> ---
>
>  arch/powerpc/boot/dts/mpc8540ads.dts |   16 ++++++++++++++++
>  arch/powerpc/boot/dts/mpc8548cds.dts |   16 ++++++++++++++++
>  arch/powerpc/boot/dts/mpc8560ads.dts |   18 +++++++++++++++++-
>  3 files changed, 49 insertions(+), 1 deletions(-)
>
> diff --git a/arch/powerpc/boot/dts/mpc8540ads.dts b/arch/powerpc/ 
> boot/dts/mpc8540ads.dts
> index f261d64..f411bc1 100644
> --- a/arch/powerpc/boot/dts/mpc8540ads.dts
> +++ b/arch/powerpc/boot/dts/mpc8540ads.dts
> @@ -48,6 +48,22 @@
>  		reg = <e0000000 00100000>;	// CCSRBAR 1M
>  		bus-frequency = <0>;
>
> +		memory-controller@2000 {
> +			compatible = "fsl,85xx-memory-controller";
> +			reg = <2000 1000>;
> +			interrupt-parent = <&mpic>;
> +			interrupts = <2 2>;
> +		};
> +
> +		l2-cache-controller@20000 {
> +			compatible = "fsl,85xx-memory-controller";
> +			reg = <20000 1000>;
> +			cache-line-size = <20>;	// 32 bytes
> +			cache-size = <40000>;	// L2, 256K
> +			interrupt-parent = <&mpic>;
> +			interrupts = <0 2>;
> +		};
> +
>  		i2c@3000 {
>  			device_type = "i2c";
>  			compatible = "fsl-i2c";
> diff --git a/arch/powerpc/boot/dts/mpc8548cds.dts b/arch/powerpc/ 
> boot/dts/mpc8548cds.dts
> index b2b2200..9de5c3a 100644
> --- a/arch/powerpc/boot/dts/mpc8548cds.dts
> +++ b/arch/powerpc/boot/dts/mpc8548cds.dts
> @@ -48,6 +48,22 @@
>  		reg = <e0000000 00100000>;	// CCSRBAR 1M
>  		bus-frequency = <0>;
>
> +		memory-controller@2000 {
> +			compatible = "fsl,85xx-memory-controller";
> +			reg = <2000 1000>;
> +			interrupt-parent = <&mpic>;
> +			interrupts = <2 2>;
> +		};
> +
> +		l2-cache-controller@20000 {
> +			compatible = "fsl,85xx-l2-cache-controller";
> +			reg = <20000 1000>;
> +			cache-line-size = <20>;	// 32 bytes
> +			cache-size = <40000>;	// L2, 256K
> +			interrupt-parent = <&mpic>;
> +			interrupts = <0 2>;
> +		};
> +
>  		i2c@3000 {
>  			device_type = "i2c";
>  			compatible = "fsl-i2c";
> diff --git a/arch/powerpc/boot/dts/mpc8560ads.dts b/arch/powerpc/ 
> boot/dts/mpc8560ads.dts
> index 1f2afe9..6e59364 100644
> --- a/arch/powerpc/boot/dts/mpc8560ads.dts
> +++ b/arch/powerpc/boot/dts/mpc8560ads.dts
> @@ -48,6 +48,22 @@
>  		reg = <e0000000 00000200>;
>  		bus-frequency = <13ab6680>;
>
> +		memory-controller@2000 {
> +			compatible = "fsl,85xx-memory-controller";
> +			reg = <2000 1000>;
> +			interrupt-parent = <&mpic>;
> +			interrupts = <2 2>;
> +		};
> +
> +		l2-cache-controller@20000 {
> +			compatible = "fsl,85xx-l2-cache-controller";
> +			reg = <20000 1000>;
> +			cache-line-size = <20>;	// 32 bytes
> +			cache-size = <40000>;	// L2, 256K
> +			interrupt-parent = <&mpic>;
> +			interrupts = <0 2>;
> +		};
> +
>  		mdio@24520 {
>  			device_type = "mdio";
>  			compatible = "gianfar";
> @@ -110,7 +126,7 @@
>  			#address-cells = <3>;
>  			compatible = "85xx";
>  			device_type = "pci";
> -			reg = <8000 400>;
> +			reg = <8000 1000>;
>  			clock-frequency = <3f940aa>;
>  			interrupt-map-mask = <f800 0 0 7>;
>  			interrupt-map = <

Re: [PATCH] powerpc: add dts entries to 85xx for EDAC

[Dave Jiang]

Kumar Gala wrote:
> On May 7, 2007, at 6:26 PM, Dave Jiang wrote:
> 
>> Adding memory-controller and l2-cache-controller entries to be used  
>> by EDAC as
>> of_devices.
> 
> we should distinguish the memory controllers on 8540/8560 (DDR1) from  
> the one on 8548 (DDR1 & DDR2).

Do you mean something like:

8540/8560
compatible = "fsl,85xx-ddr-controller"

8548
compatible = "fsl,85xx-ddr2-controller"

--
-= Dave =-

Re: [PATCH] powerpc: add dts entries to 85xx for EDAC

[Segher Boessenkool]

>> we should distinguish the memory controllers on 8540/8560 (DDR1) from
>> the one on 8548 (DDR1 & DDR2).
>
> Do you mean something like:
>
> 8540/8560
> compatible = "fsl,85xx-ddr-controller"

fsl,8540-memory-controller

> 8548
> compatible = "fsl,85xx-ddr2-controller"

fsl,8548-memory-controller

This shows once again why "Nxx" is not such a great
plan; it states that all Nxx devices are compatible,
while some might not even exist yet.


Segher

[PATCH] powerpc: add dts entries to 85xx for EDAC

[Dave Jiang]

Adding memory-controller and l2-cache-controller entries to be used by EDAC as
of_devices.

Signed-off-by: Dave Jiang <djiang@mvista.com>

---
 arch/powerpc/boot/dts/mpc8540ads.dts |   16 ++++++++++++++++
 arch/powerpc/boot/dts/mpc8548cds.dts |   16 ++++++++++++++++
 arch/powerpc/boot/dts/mpc8560ads.dts |   18 +++++++++++++++++-
 3 files changed, 49 insertions(+), 1 deletions(-)

diff --git a/arch/powerpc/boot/dts/mpc8540ads.dts b/arch/powerpc/boot/dts/mpc8540ads.dts
index f261d64..5471227 100644
--- a/arch/powerpc/boot/dts/mpc8540ads.dts
+++ b/arch/powerpc/boot/dts/mpc8540ads.dts
@@ -48,6 +48,22 @@
 		reg = <e0000000 00100000>;	// CCSRBAR 1M
 		bus-frequency = <0>;
 
+		memory-controller@2000 {
+			compatible = "fsl,8540-memory-controller";
+			reg = <2000 1000>;
+			interrupt-parent = <&mpic>;
+			interrupts = <2 2>;
+		};
+
+		l2-cache-controller@20000 {
+			compatible = "fsl,85xx-l2-cache-controller";
+			reg = <20000 1000>;
+			cache-line-size = <20>;	// 32 bytes
+			cache-size = <40000>;	// L2, 256K
+			interrupt-parent = <&mpic>;
+			interrupts = <0 2>;
+		};
+
 		i2c@3000 {
 			device_type = "i2c";
 			compatible = "fsl-i2c";
diff --git a/arch/powerpc/boot/dts/mpc8548cds.dts b/arch/powerpc/boot/dts/mpc8548cds.dts
index b2b2200..f0c5256 100644
--- a/arch/powerpc/boot/dts/mpc8548cds.dts
+++ b/arch/powerpc/boot/dts/mpc8548cds.dts
@@ -48,6 +48,22 @@
 		reg = <e0000000 00100000>;	// CCSRBAR 1M
 		bus-frequency = <0>;
 
+		memory-controller@2000 {
+			compatible = "fsl,8548-memory-controller";
+			reg = <2000 1000>;
+			interrupt-parent = <&mpic>;
+			interrupts = <2 2>;
+		};
+
+		l2-cache-controller@20000 {
+			compatible = "fsl,85xx-l2-cache-controller";
+			reg = <20000 1000>;
+			cache-line-size = <20>;	// 32 bytes
+			cache-size = <40000>;	// L2, 256K
+			interrupt-parent = <&mpic>;
+			interrupts = <0 2>;
+		};
+
 		i2c@3000 {
 			device_type = "i2c";
 			compatible = "fsl-i2c";
diff --git a/arch/powerpc/boot/dts/mpc8560ads.dts b/arch/powerpc/boot/dts/mpc8560ads.dts
index 1f2afe9..144fc65 100644
--- a/arch/powerpc/boot/dts/mpc8560ads.dts
+++ b/arch/powerpc/boot/dts/mpc8560ads.dts
@@ -48,6 +48,22 @@
 		reg = <e0000000 00000200>;
 		bus-frequency = <13ab6680>;
 
+		memory-controller@2000 {
+			compatible = "fsl,8540-memory-controller";
+			reg = <2000 1000>;
+			interrupt-parent = <&mpic>;
+			interrupts = <2 2>;
+		};
+
+		l2-cache-controller@20000 {
+			compatible = "fsl,85xx-l2-cache-controller";
+			reg = <20000 1000>;
+			cache-line-size = <20>;	// 32 bytes
+			cache-size = <40000>;	// L2, 256K
+			interrupt-parent = <&mpic>;
+			interrupts = <0 2>;
+		};
+
 		mdio@24520 {
 			device_type = "mdio";
 			compatible = "gianfar";
@@ -110,7 +126,7 @@
 			#address-cells = <3>;
 			compatible = "85xx";
 			device_type = "pci";
-			reg = <8000 400>;
+			reg = <8000 1000>;
 			clock-frequency = <3f940aa>;
 			interrupt-map-mask = <f800 0 0 7>;
 			interrupt-map = <

Re: [PATCH] powerpc: add dts entries to 85xx for EDAC

[Kumar Gala]

On May 9, 2007, at 11:53 AM, Dave Jiang wrote:

>
> Adding memory-controller and l2-cache-controller entries to be used  
> by EDAC as
> of_devices.
>
> Signed-off-by: Dave Jiang <djiang@mvista.com>
>
> ---
>  arch/powerpc/boot/dts/mpc8540ads.dts |   16 ++++++++++++++++
>  arch/powerpc/boot/dts/mpc8548cds.dts |   16 ++++++++++++++++
>  arch/powerpc/boot/dts/mpc8560ads.dts |   18 +++++++++++++++++-
>  3 files changed, 49 insertions(+), 1 deletions(-)
>
> diff --git a/arch/powerpc/boot/dts/mpc8540ads.dts b/arch/powerpc/ 
> boot/dts/mpc8540ads.dts
> index f261d64..5471227 100644
> --- a/arch/powerpc/boot/dts/mpc8540ads.dts
> +++ b/arch/powerpc/boot/dts/mpc8540ads.dts
> @@ -48,6 +48,22 @@
>  		reg = <e0000000 00100000>;	// CCSRBAR 1M
>  		bus-frequency = <0>;
>
> +		memory-controller@2000 {
> +			compatible = "fsl,8540-memory-controller";
> +			reg = <2000 1000>;
> +			interrupt-parent = <&mpic>;
> +			interrupts = <2 2>;
> +		};
> +
> +		l2-cache-controller@20000 {
> +			compatible = "fsl,85xx-l2-cache-controller";

we should probably do the same fsl,8540-l2-cache-controller, fsl,8548- 
l2-cache-controller, ...

> +			reg = <20000 1000>;
> +			cache-line-size = <20>;	// 32 bytes
> +			cache-size = <40000>;	// L2, 256K
> +			interrupt-parent = <&mpic>;
> +			interrupts = <0 2>;
> +		};
> +
>  		i2c@3000 {
>  			device_type = "i2c";
>  			compatible = "fsl-i2c";
> diff --git a/arch/powerpc/boot/dts/mpc8548cds.dts b/arch/powerpc/ 
> boot/dts/mpc8548cds.dts
> index b2b2200..f0c5256 100644
> --- a/arch/powerpc/boot/dts/mpc8548cds.dts
> +++ b/arch/powerpc/boot/dts/mpc8548cds.dts
> @@ -48,6 +48,22 @@
>  		reg = <e0000000 00100000>;	// CCSRBAR 1M
>  		bus-frequency = <0>;
>
> +		memory-controller@2000 {
> +			compatible = "fsl,8548-memory-controller";
> +			reg = <2000 1000>;
> +			interrupt-parent = <&mpic>;
> +			interrupts = <2 2>;
> +		};
> +
> +		l2-cache-controller@20000 {
> +			compatible = "fsl,85xx-l2-cache-controller";
> +			reg = <20000 1000>;
> +			cache-line-size = <20>;	// 32 bytes
> +			cache-size = <40000>;	// L2, 256K

Cache size 512k on 8548.

> +			interrupt-parent = <&mpic>;
> +			interrupts = <0 2>;
> +		};
> +
>  		i2c@3000 {
>  			device_type = "i2c";
>  			compatible = "fsl-i2c";
> diff --git a/arch/powerpc/boot/dts/mpc8560ads.dts b/arch/powerpc/ 
> boot/dts/mpc8560ads.dts
> index 1f2afe9..144fc65 100644
> --- a/arch/powerpc/boot/dts/mpc8560ads.dts
> +++ b/arch/powerpc/boot/dts/mpc8560ads.dts
> @@ -48,6 +48,22 @@
>  		reg = <e0000000 00000200>;
>  		bus-frequency = <13ab6680>;
>
> +		memory-controller@2000 {
> +			compatible = "fsl,8540-memory-controller";
> +			reg = <2000 1000>;
> +			interrupt-parent = <&mpic>;
> +			interrupts = <2 2>;
> +		};
> +
> +		l2-cache-controller@20000 {
> +			compatible = "fsl,85xx-l2-cache-controller";
> +			reg = <20000 1000>;
> +			cache-line-size = <20>;	// 32 bytes
> +			cache-size = <40000>;	// L2, 256K
> +			interrupt-parent = <&mpic>;
> +			interrupts = <0 2>;
> +		};
> +
>  		mdio@24520 {
>  			device_type = "mdio";
>  			compatible = "gianfar";
> @@ -110,7 +126,7 @@
>  			#address-cells = <3>;
>  			compatible = "85xx";
>  			device_type = "pci";
> -			reg = <8000 400>;
> +			reg = <8000 1000>;
>  			clock-frequency = <3f940aa>;
>  			interrupt-map-mask = <f800 0 0 7>;
>  			interrupt-map = <

[PATCH] powerpc: add dts entries to 85xx for EDAC

[Dave Jiang]

Adding memory-controller and l2-cache-controller entries to be used by EDAC 
as of_devices.

Signed-off-by: Dave Jiang <djiang@mvista.com>

---

Fixed up additional changes per Kumar.

 arch/powerpc/boot/dts/mpc8540ads.dts |   16 ++++++++++++++++
 arch/powerpc/boot/dts/mpc8548cds.dts |   16 ++++++++++++++++
 arch/powerpc/boot/dts/mpc8560ads.dts |   18 +++++++++++++++++-
 3 files changed, 49 insertions(+), 1 deletions(-)

diff --git a/arch/powerpc/boot/dts/mpc8540ads.dts b/arch/powerpc/boot/dts/mpc8540ads.dts
index f261d64..d91e81c 100644
--- a/arch/powerpc/boot/dts/mpc8540ads.dts
+++ b/arch/powerpc/boot/dts/mpc8540ads.dts
@@ -48,6 +48,22 @@
 		reg = <e0000000 00100000>;	// CCSRBAR 1M
 		bus-frequency = <0>;
 
+		memory-controller@2000 {
+			compatible = "fsl,8540-memory-controller";
+			reg = <2000 1000>;
+			interrupt-parent = <&mpic>;
+			interrupts = <2 2>;
+		};
+
+		l2-cache-controller@20000 {
+			compatible = "fsl,8540-l2-cache-controller";
+			reg = <20000 1000>;
+			cache-line-size = <20>;	// 32 bytes
+			cache-size = <40000>;	// L2, 256K
+			interrupt-parent = <&mpic>;
+			interrupts = <0 2>;
+		};
+
 		i2c@3000 {
 			device_type = "i2c";
 			compatible = "fsl-i2c";
diff --git a/arch/powerpc/boot/dts/mpc8548cds.dts b/arch/powerpc/boot/dts/mpc8548cds.dts
index b2b2200..ad96381 100644
--- a/arch/powerpc/boot/dts/mpc8548cds.dts
+++ b/arch/powerpc/boot/dts/mpc8548cds.dts
@@ -48,6 +48,22 @@
 		reg = <e0000000 00100000>;	// CCSRBAR 1M
 		bus-frequency = <0>;
 
+		memory-controller@2000 {
+			compatible = "fsl,8548-memory-controller";
+			reg = <2000 1000>;
+			interrupt-parent = <&mpic>;
+			interrupts = <2 2>;
+		};
+
+		l2-cache-controller@20000 {
+			compatible = "fsl,8548-l2-cache-controller";
+			reg = <20000 1000>;
+			cache-line-size = <20>;	// 32 bytes
+			cache-size = <80000>;	// L2, 512K
+			interrupt-parent = <&mpic>;
+			interrupts = <0 2>;
+		};
+
 		i2c@3000 {
 			device_type = "i2c";
 			compatible = "fsl-i2c";
diff --git a/arch/powerpc/boot/dts/mpc8560ads.dts b/arch/powerpc/boot/dts/mpc8560ads.dts
index 1f2afe9..8068215 100644
--- a/arch/powerpc/boot/dts/mpc8560ads.dts
+++ b/arch/powerpc/boot/dts/mpc8560ads.dts
@@ -48,6 +48,22 @@
 		reg = <e0000000 00000200>;
 		bus-frequency = <13ab6680>;
 
+		memory-controller@2000 {
+			compatible = "fsl,8540-memory-controller";
+			reg = <2000 1000>;
+			interrupt-parent = <&mpic>;
+			interrupts = <2 2>;
+		};
+
+		l2-cache-controller@20000 {
+			compatible = "fsl,8540-l2-cache-controller";
+			reg = <20000 1000>;
+			cache-line-size = <20>;	// 32 bytes
+			cache-size = <40000>;	// L2, 256K
+			interrupt-parent = <&mpic>;
+			interrupts = <0 2>;
+		};
+
 		mdio@24520 {
 			device_type = "mdio";
 			compatible = "gianfar";
@@ -110,7 +126,7 @@
 			#address-cells = <3>;
 			compatible = "85xx";
 			device_type = "pci";
-			reg = <8000 400>;
+			reg = <8000 1000>;
 			clock-frequency = <3f940aa>;
 			interrupt-map-mask = <f800 0 0 7>;
 			interrupt-map = <

Re: [PATCH] powerpc: add dts entries to 85xx for EDAC

[Kumar Gala]

On Thu, 10 May 2007, Dave Jiang wrote:

> Adding memory-controller and l2-cache-controller entries to be used by EDAC
> as of_devices.
>
> Signed-off-by: Dave Jiang <djiang@mvista.com>
>
> ---
>
> Fixed up additional changes per Kumar.
>
>  arch/powerpc/boot/dts/mpc8540ads.dts |   16 ++++++++++++++++
>  arch/powerpc/boot/dts/mpc8548cds.dts |   16 ++++++++++++++++
>  arch/powerpc/boot/dts/mpc8560ads.dts |   18 +++++++++++++++++-
>  3 files changed, 49 insertions(+), 1 deletions(-)

applied.

- k