| Message ID | 1413458321-23880-3-git-send-email-haokexin@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Scott Wood |
| Headers | show |
On Thu, 2014-10-16 at 19:18 +0800, Kevin Hao wrote: > In commit da788acb2838 ("clk: ppc-corenet: Fix Section mismatch > warning"), we put the ppc_corenet_clk_driver struct to init section > in order to fix section mismatch warning. This is definitely wrong > because the kernel would free the memories occupied by > ppc_corenet_clk_driver after boot while this driver is still registered > in the driver core. The kernel would panic when accessing this driver > struct. So choose to use CLK_OF_DECLARE to scan and init the clock devices. > > Signed-off-by: Kevin Hao <haokexin@gmail.com> > --- > arch/powerpc/platforms/85xx/corenet_generic.c | 7 +++++ > drivers/clk/clk-ppc-corenet.c | 43 ++++----------------------- > 2 files changed, 13 insertions(+), 37 deletions(-) > > diff --git a/arch/powerpc/platforms/85xx/corenet_generic.c b/arch/powerpc/platforms/85xx/corenet_generic.c > index e56b89a792ed..7677cfecb787 100644 > --- a/arch/powerpc/platforms/85xx/corenet_generic.c > +++ b/arch/powerpc/platforms/85xx/corenet_generic.c > @@ -16,6 +16,7 @@ > #include <linux/kdev_t.h> > #include <linux/delay.h> > #include <linux/interrupt.h> > +#include <linux/clk-provider.h> > > #include <asm/time.h> > #include <asm/machdep.h> > @@ -188,11 +189,17 @@ static int __init corenet_generic_probe(void) > return 0; > } > > +static void __init corenet_gen_init(void) > +{ > + of_clk_init(NULL); > +} Why is this board-specific? -Scott
On Thu, Oct 16, 2014 at 11:55:23PM +0200, Scott Wood wrote: > On Thu, 2014-10-16 at 19:18 +0800, Kevin Hao wrote: > > diff --git a/arch/powerpc/platforms/85xx/corenet_generic.c b/arch/powerpc/platforms/85xx/corenet_generic.c > > index e56b89a792ed..7677cfecb787 100644 > > --- a/arch/powerpc/platforms/85xx/corenet_generic.c > > +++ b/arch/powerpc/platforms/85xx/corenet_generic.c > > @@ -16,6 +16,7 @@ > > #include <linux/kdev_t.h> > > #include <linux/delay.h> > > #include <linux/interrupt.h> > > +#include <linux/clk-provider.h> > > > > #include <asm/time.h> > > #include <asm/machdep.h> > > @@ -188,11 +189,17 @@ static int __init corenet_generic_probe(void) > > return 0; > > } > > > > +static void __init corenet_gen_init(void) > > +{ > > + of_clk_init(NULL); > > +} > > Why is this board-specific? I have thought about to put it in a more common place such as time_init(), but this will be in conflict with mpc512x board. How about add an arch_initcall(mpc85xx_clk_init) in arch/powerpc/platforms/85xx/common.c? Thanks, Kevin
On Fri, 2014-10-17 at 06:55 +0800, Kevin Hao wrote: > On Thu, Oct 16, 2014 at 11:55:23PM +0200, Scott Wood wrote: > > On Thu, 2014-10-16 at 19:18 +0800, Kevin Hao wrote: > > > diff --git a/arch/powerpc/platforms/85xx/corenet_generic.c b/arch/powerpc/platforms/85xx/corenet_generic.c > > > index e56b89a792ed..7677cfecb787 100644 > > > --- a/arch/powerpc/platforms/85xx/corenet_generic.c > > > +++ b/arch/powerpc/platforms/85xx/corenet_generic.c > > > @@ -16,6 +16,7 @@ > > > #include <linux/kdev_t.h> > > > #include <linux/delay.h> > > > #include <linux/interrupt.h> > > > +#include <linux/clk-provider.h> > > > > > > #include <asm/time.h> > > > #include <asm/machdep.h> > > > @@ -188,11 +189,17 @@ static int __init corenet_generic_probe(void) > > > return 0; > > > } > > > > > > +static void __init corenet_gen_init(void) > > > +{ > > > + of_clk_init(NULL); > > > +} > > > > Why is this board-specific? > > I have thought about to put it in a more common place such as time_init(), > but this will be in conflict with mpc512x board. How about add an > arch_initcall(mpc85xx_clk_init) in arch/powerpc/platforms/85xx/common.c? Gerhard, does 512x really require of_clk_init() to be called at that specific time, or can it be replaced by a common of_clk_init()? -Scott
On Fri, Oct 17, 2014 at 07:58:55AM +0200, Scott Wood wrote: > On Fri, 2014-10-17 at 06:55 +0800, Kevin Hao wrote: > > I have thought about to put it in a more common place such as time_init(), > > but this will be in conflict with mpc512x board. How about add an > > arch_initcall(mpc85xx_clk_init) in arch/powerpc/platforms/85xx/common.c? > > Gerhard, does 512x really require of_clk_init() to be called at that > specific time, or can it be replaced by a common of_clk_init()? I have took a look at the mpc5121_clk_init() and it seems that the of_clk_init() is only used to scan the 'fixed-clock' in the DT. So I think it should be safe to invoke it a bit earlier in a common place. I will create v2 to use this method. Gerhard, please correct me if I am wrong. Thanks, Kevin
diff --git a/arch/powerpc/platforms/85xx/corenet_generic.c b/arch/powerpc/platforms/85xx/corenet_generic.c index e56b89a792ed..7677cfecb787 100644 --- a/arch/powerpc/platforms/85xx/corenet_generic.c +++ b/arch/powerpc/platforms/85xx/corenet_generic.c @@ -16,6 +16,7 @@ #include <linux/kdev_t.h> #include <linux/delay.h> #include <linux/interrupt.h> +#include <linux/clk-provider.h> #include <asm/time.h> #include <asm/machdep.h> @@ -188,11 +189,17 @@ static int __init corenet_generic_probe(void) return 0; } +static void __init corenet_gen_init(void) +{ + of_clk_init(NULL); +} + define_machine(corenet_generic) { .name = "CoreNet Generic", .probe = corenet_generic_probe, .setup_arch = corenet_gen_setup_arch, .init_IRQ = corenet_gen_pic_init, + .init = corenet_gen_init, #ifdef CONFIG_PCI .pcibios_fixup_bus = fsl_pcibios_fixup_bus, .pcibios_fixup_phb = fsl_pcibios_fixup_phb, diff --git a/drivers/clk/clk-ppc-corenet.c b/drivers/clk/clk-ppc-corenet.c index 8e58edfeeb37..bf0fe565ce4e 100644 --- a/drivers/clk/clk-ppc-corenet.c +++ b/drivers/clk/clk-ppc-corenet.c @@ -268,40 +268,9 @@ static void __init sysclk_init(struct device_node *node) of_clk_add_provider(np, of_clk_src_simple_get, clk); } -static const struct of_device_id clk_match[] __initconst = { - { .compatible = "fsl,qoriq-sysclk-1.0", .data = sysclk_init, }, - { .compatible = "fsl,qoriq-sysclk-2.0", .data = sysclk_init, }, - { .compatible = "fsl,qoriq-core-pll-1.0", .data = core_pll_init, }, - { .compatible = "fsl,qoriq-core-pll-2.0", .data = core_pll_init, }, - { .compatible = "fsl,qoriq-core-mux-1.0", .data = core_mux_init, }, - { .compatible = "fsl,qoriq-core-mux-2.0", .data = core_mux_init, }, - {} -}; - -static int __init ppc_corenet_clk_probe(struct platform_device *pdev) -{ - of_clk_init(clk_match); - - return 0; -} - -static const struct of_device_id ppc_clk_ids[] __initconst = { - { .compatible = "fsl,qoriq-clockgen-1.0", }, - { .compatible = "fsl,qoriq-clockgen-2.0", }, - {} -}; - -static struct platform_driver ppc_corenet_clk_driver __initdata = { - .driver = { - .name = "ppc_corenet_clock", - .owner = THIS_MODULE, - .of_match_table = ppc_clk_ids, - }, - .probe = ppc_corenet_clk_probe, -}; - -static int __init ppc_corenet_clk_init(void) -{ - return platform_driver_register(&ppc_corenet_clk_driver); -} -subsys_initcall(ppc_corenet_clk_init); +CLK_OF_DECLARE(qoriq_sysclk_1, "fsl,qoriq-sysclk-1.0", sysclk_init); +CLK_OF_DECLARE(qoriq_sysclk_2, "fsl,qoriq-sysclk-2.0", sysclk_init); +CLK_OF_DECLARE(qoriq_core_pll_1, "fsl,qoriq-core-pll-1.0", core_pll_init); +CLK_OF_DECLARE(qoriq_core_pll_2, "fsl,qoriq-core-pll-2.0", core_pll_init); +CLK_OF_DECLARE(qoriq_core_mux_1, "fsl,qoriq-core-mux-1.0", core_mux_init); +CLK_OF_DECLARE(qoriq_core_mux_2, "fsl,qoriq-core-mux-2.0", core_mux_init);
In commit da788acb2838 ("clk: ppc-corenet: Fix Section mismatch warning"), we put the ppc_corenet_clk_driver struct to init section in order to fix section mismatch warning. This is definitely wrong because the kernel would free the memories occupied by ppc_corenet_clk_driver after boot while this driver is still registered in the driver core. The kernel would panic when accessing this driver struct. So choose to use CLK_OF_DECLARE to scan and init the clock devices. Signed-off-by: Kevin Hao <haokexin@gmail.com> --- arch/powerpc/platforms/85xx/corenet_generic.c | 7 +++++ drivers/clk/clk-ppc-corenet.c | 43 ++++----------------------- 2 files changed, 13 insertions(+), 37 deletions(-)