| Message ID | 1380019739-8196-1-git-send-email-X.Xie@freescale.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
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
> -----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 >
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
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
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(+)