Patchwork [V4,1/3] powerpc/85xx: Add QE common init functions

login
register
mail settings
Submitter Xiaobo Xie
Date Sept. 24, 2013, 10:48 a.m.
Message ID <1380019739-8196-1-git-send-email-X.Xie@freescale.com>
Download mbox | patch
Permalink /patch/277430/
State Superseded
Headers show

Comments

Xiaobo Xie - Sept. 24, 2013, 10:48 a.m.
Define two QE init functions in common file, and avoid
the same codes being duplicated in board files.

Signed-off-by: Xie Xiaobo <X.Xie@freescale.com>
---
V4 -> V3: Nochange

 arch/powerpc/platforms/85xx/common.c  | 51 +++++++++++++++++++++++++++++++++++
 arch/powerpc/platforms/85xx/mpc85xx.h |  8 ++++++
 2 files changed, 59 insertions(+)
Scott Wood - Sept. 24, 2013, 11:13 p.m.
On Tue, 2013-09-24 at 18:48 +0800, Xie Xiaobo wrote:
> Define two QE init functions in common file, and avoid
> the same codes being duplicated in board files.
> 
> Signed-off-by: Xie Xiaobo <X.Xie@freescale.com>
> ---
> V4 -> V3: Nochange
> 
>  arch/powerpc/platforms/85xx/common.c  | 51 +++++++++++++++++++++++++++++++++++
>  arch/powerpc/platforms/85xx/mpc85xx.h |  8 ++++++
>  2 files changed, 59 insertions(+)
> 
> diff --git a/arch/powerpc/platforms/85xx/common.c b/arch/powerpc/platforms/85xx/common.c
> index d0861a0..08fff48 100644
> --- a/arch/powerpc/platforms/85xx/common.c
> +++ b/arch/powerpc/platforms/85xx/common.c
> @@ -7,6 +7,9 @@
>   */
>  #include <linux/of_platform.h>
>  
> +#include <asm/machdep.h>
> +#include <asm/qe.h>
> +#include <asm/qe_ic.h>
>  #include <sysdev/cpm2_pic.h>
>  
>  #include "mpc85xx.h"
> @@ -80,3 +83,51 @@ void __init mpc85xx_cpm2_pic_init(void)
>  	irq_set_chained_handler(irq, cpm2_cascade);
>  }
>  #endif
> +
> +#ifdef CONFIG_QUICC_ENGINE
> +void __init mpc85xx_qe_pic_init(void)
> +{
> +	struct device_node *np;
> +
> +	np = of_find_compatible_node(NULL, NULL, "fsl,qe-ic");
> +	if (np) {
> +		if (machine_is(mpc8568_mds) || machine_is(mpc8569_mds))
> +			qe_ic_init(np, 0, qe_ic_cascade_muxed_mpic, NULL);
> +		else
> +			qe_ic_init(np, 0, qe_ic_cascade_low_mpic,
> +					qe_ic_cascade_high_mpic);
> +		of_node_put(np);
> +	} else
> +		pr_err("%s: Could not find qe-ic node\n", __func__);
> +}

Have the caller pass in a flag indicating the type of cascade.  Or,
perhaps this function isn't worth factoring out.  Where is the check for
p1021_mds?  Where did 8568/9 MDS come from?  I don't see those checks
removed in patch 2.

BTW, when you move code from one place to another, do it in one patch.
Don't add it in one patch and then remove it in another.  A more useful
split would have been one patch handling qe_init and another handling
qe_pic_init.

-Scott
Xie Xiaobo-R63061 - Sept. 25, 2013, 9:51 a.m.
> -----Original Message-----
> From: Wood Scott-B07421
> Sent: Wednesday, September 25, 2013 7:13 AM
> To: Xie Xiaobo-R63061
> Cc: linuxppc-dev@lists.ozlabs.org
> Subject: Re: [PATCH V4 1/3] powerpc/85xx: Add QE common init functions
> 
> On Tue, 2013-09-24 at 18:48 +0800, Xie Xiaobo wrote:
> > Define two QE init functions in common file, and avoid the same codes
> > being duplicated in board files.
> >
> > Signed-off-by: Xie Xiaobo <X.Xie@freescale.com>
> > ---
> > V4 -> V3: Nochange
> >
> >  arch/powerpc/platforms/85xx/common.c  | 51
> > +++++++++++++++++++++++++++++++++++
> >  arch/powerpc/platforms/85xx/mpc85xx.h |  8 ++++++
> >  2 files changed, 59 insertions(+)
> >
> > diff --git a/arch/powerpc/platforms/85xx/common.c
> > b/arch/powerpc/platforms/85xx/common.c
> > index d0861a0..08fff48 100644
> > --- a/arch/powerpc/platforms/85xx/common.c
> > +++ b/arch/powerpc/platforms/85xx/common.c
> > @@ -7,6 +7,9 @@
> >   */
> >  #include <linux/of_platform.h>
> >
> > +#include <asm/machdep.h>
> > +#include <asm/qe.h>
> > +#include <asm/qe_ic.h>
> >  #include <sysdev/cpm2_pic.h>
> >
> >  #include "mpc85xx.h"
> > @@ -80,3 +83,51 @@ void __init mpc85xx_cpm2_pic_init(void)
> >  	irq_set_chained_handler(irq, cpm2_cascade);  }  #endif
> > +
> > +#ifdef CONFIG_QUICC_ENGINE
> > +void __init mpc85xx_qe_pic_init(void) {
> > +	struct device_node *np;
> > +
> > +	np = of_find_compatible_node(NULL, NULL, "fsl,qe-ic");
> > +	if (np) {
> > +		if (machine_is(mpc8568_mds) || machine_is(mpc8569_mds))
> > +			qe_ic_init(np, 0, qe_ic_cascade_muxed_mpic, NULL);
> > +		else
> > +			qe_ic_init(np, 0, qe_ic_cascade_low_mpic,
> > +					qe_ic_cascade_high_mpic);
> > +		of_node_put(np);
> > +	} else
> > +		pr_err("%s: Could not find qe-ic node\n", __func__); }
> 
> Have the caller pass in a flag indicating the type of cascade.  Or,
> perhaps this function isn't worth factoring out.  Where is the check for
> p1021_mds?  Where did 8568/9 MDS come from?  I don't see those checks
> removed in patch 2.

[Xie] The qe_pic_init just call one function qe_ic_init(), So I just need factor out the qe_init function, Is it feasible?
 
> 
> BTW, when you move code from one place to another, do it in one patch.
> Don't add it in one patch and then remove it in another.  A more useful
> split would have been one patch handling qe_init and another handling
> qe_pic_init.

[Xie] I will place these change into a patch. 

> 
> -Scott
>
Scott Wood - Sept. 25, 2013, 6:01 p.m.
On Wed, 2013-09-25 at 04:51 -0500, Xie Xiaobo-R63061 wrote:
> > -----Original Message-----
> > From: Wood Scott-B07421
> > Sent: Wednesday, September 25, 2013 7:13 AM
> > To: Xie Xiaobo-R63061
> > Cc: linuxppc-dev@lists.ozlabs.org
> > Subject: Re: [PATCH V4 1/3] powerpc/85xx: Add QE common init functions
> > 
> > On Tue, 2013-09-24 at 18:48 +0800, Xie Xiaobo wrote:
> > > Define two QE init functions in common file, and avoid the same codes
> > > being duplicated in board files.
> > >
> > > Signed-off-by: Xie Xiaobo <X.Xie@freescale.com>
> > > ---
> > > V4 -> V3: Nochange
> > >
> > >  arch/powerpc/platforms/85xx/common.c  | 51
> > > +++++++++++++++++++++++++++++++++++
> > >  arch/powerpc/platforms/85xx/mpc85xx.h |  8 ++++++
> > >  2 files changed, 59 insertions(+)
> > >
> > > diff --git a/arch/powerpc/platforms/85xx/common.c
> > > b/arch/powerpc/platforms/85xx/common.c
> > > index d0861a0..08fff48 100644
> > > --- a/arch/powerpc/platforms/85xx/common.c
> > > +++ b/arch/powerpc/platforms/85xx/common.c
> > > @@ -7,6 +7,9 @@
> > >   */
> > >  #include <linux/of_platform.h>
> > >
> > > +#include <asm/machdep.h>
> > > +#include <asm/qe.h>
> > > +#include <asm/qe_ic.h>
> > >  #include <sysdev/cpm2_pic.h>
> > >
> > >  #include "mpc85xx.h"
> > > @@ -80,3 +83,51 @@ void __init mpc85xx_cpm2_pic_init(void)
> > >  	irq_set_chained_handler(irq, cpm2_cascade);  }  #endif
> > > +
> > > +#ifdef CONFIG_QUICC_ENGINE
> > > +void __init mpc85xx_qe_pic_init(void) {
> > > +	struct device_node *np;
> > > +
> > > +	np = of_find_compatible_node(NULL, NULL, "fsl,qe-ic");
> > > +	if (np) {
> > > +		if (machine_is(mpc8568_mds) || machine_is(mpc8569_mds))
> > > +			qe_ic_init(np, 0, qe_ic_cascade_muxed_mpic, NULL);
> > > +		else
> > > +			qe_ic_init(np, 0, qe_ic_cascade_low_mpic,
> > > +					qe_ic_cascade_high_mpic);
> > > +		of_node_put(np);
> > > +	} else
> > > +		pr_err("%s: Could not find qe-ic node\n", __func__); }
> > 
> > Have the caller pass in a flag indicating the type of cascade.  Or,
> > perhaps this function isn't worth factoring out.  Where is the check for
> > p1021_mds?  Where did 8568/9 MDS come from?  I don't see those checks
> > removed in patch 2.
> 
> [Xie] The qe_pic_init just call one function qe_ic_init(), So I just need factor out the qe_init function, Is it feasible?

"Or, perhaps this function isn't worth factoring out." :-)

-Scott

Patch

diff --git a/arch/powerpc/platforms/85xx/common.c b/arch/powerpc/platforms/85xx/common.c
index d0861a0..08fff48 100644
--- a/arch/powerpc/platforms/85xx/common.c
+++ b/arch/powerpc/platforms/85xx/common.c
@@ -7,6 +7,9 @@ 
  */
 #include <linux/of_platform.h>
 
+#include <asm/machdep.h>
+#include <asm/qe.h>
+#include <asm/qe_ic.h>
 #include <sysdev/cpm2_pic.h>
 
 #include "mpc85xx.h"
@@ -80,3 +83,51 @@  void __init mpc85xx_cpm2_pic_init(void)
 	irq_set_chained_handler(irq, cpm2_cascade);
 }
 #endif
+
+#ifdef CONFIG_QUICC_ENGINE
+void __init mpc85xx_qe_pic_init(void)
+{
+	struct device_node *np;
+
+	np = of_find_compatible_node(NULL, NULL, "fsl,qe-ic");
+	if (np) {
+		if (machine_is(mpc8568_mds) || machine_is(mpc8569_mds))
+			qe_ic_init(np, 0, qe_ic_cascade_muxed_mpic, NULL);
+		else
+			qe_ic_init(np, 0, qe_ic_cascade_low_mpic,
+					qe_ic_cascade_high_mpic);
+		of_node_put(np);
+	} else
+		pr_err("%s: Could not find qe-ic node\n", __func__);
+}
+
+void __init mpc85xx_qe_init(void)
+{
+	struct device_node *np;
+
+	np = of_find_compatible_node(NULL, NULL, "fsl,qe");
+	if (!np) {
+		np = of_find_node_by_name(NULL, "qe");
+		if (!np) {
+			pr_err("%s: Could not find Quicc Engine node\n",
+					__func__);
+			return;
+		}
+	}
+
+	qe_reset();
+	of_node_put(np);
+
+	np = of_find_node_by_name(NULL, "par_io");
+	if (np) {
+		struct device_node *ucc;
+
+		par_io_init(np);
+		of_node_put(np);
+
+		for_each_node_by_name(ucc, "ucc")
+			par_io_of_config(ucc);
+
+	}
+}
+#endif
diff --git a/arch/powerpc/platforms/85xx/mpc85xx.h b/arch/powerpc/platforms/85xx/mpc85xx.h
index 2aa7c5d..1d39095 100644
--- a/arch/powerpc/platforms/85xx/mpc85xx.h
+++ b/arch/powerpc/platforms/85xx/mpc85xx.h
@@ -8,4 +8,12 @@  extern void mpc85xx_cpm2_pic_init(void);
 static inline void __init mpc85xx_cpm2_pic_init(void) {}
 #endif /* CONFIG_CPM2 */
 
+#ifdef CONFIG_QUICC_ENGINE
+extern void mpc85xx_qe_pic_init(void);
+extern void mpc85xx_qe_init(void);
+#else
+static inline void __init mpc85xx_qe_pic_init(void) {}
+static inline void __init mpc85xx_qe_init(void) {}
+#endif
+
 #endif