On Tue, 2014-03-04 at 03:09 -0600, Zhao Qiang-B45475 wrote:

On Mar 3, 2014, at 11:51 PM, Kumar Gala [galak@kernel.crashing.org] wrote:

quoted

-----Original Message-----
From: Kumar Gala [mailto:galak@kernel.crashing.org]
Sent: Monday, March 03, 2014 11:51 PM
To: Zhao Qiang-B45475
Cc: linuxppc-dev@lists.ozlabs.org; Wood Scott-B07421; Xie Xiaobo-R63061
Subject: Re: [PATCH] Corenet: Add QE platform support for Corenet


On Feb 28, 2014, at 2:48 AM, Zhao Qiang [off-list ref] wrote:

quoted

There is QE on platform T104x, add support.
Call funcs qe_ic_init and qe_init if CONFIG_QUICC_ENGINE is defined.

Signed-off-by: Zhao Qiang <redacted>
---
arch/powerpc/platforms/85xx/corenet_generic.c | 32
+++++++++++++++++++++++++++
1 file changed, 32 insertions(+)

Can you use mpc85xx_qe_init() instead?

mpc85xx_qe_init() is for old QE which is different from new QE.
New QE has no par_io, and it is not correct to init 
par_io(par_io_init() called in mpc85xx_qe_init()) for new QE. 

So split that function into mpc85xx_qe_init() and
mpc85xx_qe_par_io_init().

quoted

quoted

diff --git a/arch/powerpc/platforms/85xx/corenet_generic.c

b/arch/powerpc/platforms/85xx/corenet_generic.c
index fbd871e..f8c8e0c 100644

--- a/arch/powerpc/platforms/85xx/corenet_generic.c
+++ b/arch/powerpc/platforms/85xx/corenet_generic.c

/*

@@ -52,11 +68,24 @@ void __init corenet_gen_pic_init(void)  */ void

__init corenet_gen_setup_arch(void) {
+#ifdef CONFIG_QUICC_ENGINE
+	struct device_node *np;
+#endif
	mpc85xx_smp_init();

	swiotlb_detect_4g();

	pr_info("%s board from Freescale Semiconductor\n", ppc_md.name);
+
+#ifdef CONFIG_QUICC_ENGINE
+	np = of_find_compatible_node(NULL, NULL, "fsl,qe");
+	if (!np) {
+		pr_err("%s: Could not find Quicc Engine node\n", __func__);
+		return;

This doesn't seem like an reasonable error message for common corenet
platform.  It seems reasonable to build QE support but boot on a chip w/o
QE.

mpc85xx_qe_init() has a similar problem regarding the error message, but
the above is worse because it does an early return from
corenet_gen_setup_arch() rather than just from mpc85xx_qe_init() -- what
if someone added non-QE things after this point?

-Scott