[PATCH 2/2] ARM: OMAP2+: clocks: Pass static parent pointers to common clock core
OMAP clock inits happen quite early, even before the slab is available.
As part of the clock init, the common clock core tries to cache parent
pointers (if not passed by the caller registering the clock) which
fails in case of OMAP since the slab isn't initied.
Without CONFIG_DEBUG_SLAB enabled, this just results in the common clock core
retrying the caching attempt at some point later.
However with CONFIG_DEBUG_SLAB enabled this results in a BUG() as reported
in the link below by Tony..
http://www.mail-archive.com/linux-omap@vger.kernel.org/msg85932.html
Fix this by passing static parent pointers to the common clock core
while registering clocks.
Reported-by: Piotr Haber
Reported-by: Tony Lindgren
Signed-off-by: Rajendra Nayak
---
arch/arm/mach-omap2/cclock2420_data.c | 56 +++-
arch/arm/mach-omap2/cclock2430_data.c | 56 +++-
arch/arm/mach-omap2/cclock33xx_data.c | 74 ---
arch/arm/mach-omap2/cclock3xxx_data.c | 79 +--
arch/arm/mach-omap2/cclock44xx_data.c | 236 ++---
arch/arm/mach-omap2/clock.h | 15 ++-
6 files changed, 436 insertions(+), 80 deletions(-)
diff --git a/arch/arm/mach-omap2/cclock2420_data.c
b/arch/arm/mach-omap2/cclock2420_data.c
index 4b89559..727a6dd 100644
--- a/arch/arm/mach-omap2/cclock2420_data.c
+++ b/arch/arm/mach-omap2/cclock2420_data.c
@@ -296,6 +296,10 @@ static const char *dsp_fck_parents[] = {
"core_ck",
};
+static struct clk *dsp_fck_parents_ptr[] = {
+ &core_ck,
+};
+
static const struct clk_ops dsp_fck_ops = {
.init = &omap2_init_clk_clkdm,
.enable = &omap2_dflt_clk_enable,
@@ -322,6 +326,10 @@ static const char *dsp_ick_parents[] = {
"dsp_fck",
};
+static struct clk *dsp_ick_parents_ptr[] = {
+ &dsp_fck,
+};
+
DEFINE_CLK_OMAP_MUX_GATE(dsp_ick, "dsp_clkdm", dsp_ick_clksel,
OMAP_CM_REGADDR(OMAP24XX_DSP_MOD, CM_CLKSEL),
OMAP24XX_CLKSEL_DSP_IF_MASK,
@@ -358,6 +366,10 @@ static const char *dss1_fck_parents[] = {
"sys_ck", "core_ck",
};
+static struct clk *dss1_fck_parents_ptr[] = {
+ &sys_ck, &core_ck,
+};
+
static struct clk dss1_fck;
static const struct clk_ops dss1_fck_ops = {
@@ -407,6 +419,10 @@ static const char *func_48m_ck_parents[] = {
"apll96_ck", "alt_ck",
};
+static struct clk *func_48m_ck_parents_ptr[] = {
+ &apll96_ck, &alt_ck,
+};
+
static struct clk func_48m_ck;
static const struct clk_ops func_48m_ck_ops = {
@@ -428,7 +444,7 @@ static struct clk_hw_omap func_48m_ck_hw = {
.clkdm_name = "wkup_clkdm",
};
-DEFINE_STRUCT_CLK(func_48m_ck, func_48m_ck_parents, func_48m_ck_ops);
+DEFINE_STRUCT_CLK_MUX(func_48m_ck, func_48m_ck_parents, func_48m_ck_ops);
static const struct clksel dss2_fck_clksel[] = {
{ .parent = &sys_ck, .rates = dss2_fck_sys_rates },
@@ -440,6 +456,10 @@ static const char *dss2_fck_parents[] = {
"sys_ck", "func_48m_ck",
};
+static struct clk *dss2_fck_parents_ptr[] = {
+ &sys_ck, &func_48m_ck,
+};
+
DEFINE_CLK_OMAP_MUX_GATE(dss2_fck, "dss_clkdm", dss2_fck_clksel,
OMAP_CM_REGADDR(CORE_MOD, CM_CLKSEL1),
OMAP24XX_CLKSEL_DSS2_MASK,
@@ -451,7 +471,11 @@ static const char *func_54m_ck_parents[] = {
"apll54_ck", "alt_ck",
};
-DEFINE_CLK_MUX(func_54m_ck, func_54m_ck_parents, NULL, 0x0,
+static struct clk *func_54m_ck_parents_ptr[] = {
+ &apll54_ck, &alt_ck,
+};
+
+DEFINE_CLK_MUX(func_54m_ck, func_54m_ck_parents, func_54m_ck_parents_ptr, 0x0,
OMAP_CM_REGADDR(PLL_MOD, CM_CLKSEL1),
OMAP24XX_54M_SOURCE_SHIFT, OMAP24XX_54M_SOURCE_WIDTH,
0x0, NULL);
@@ -572,6 +596,10 @@ static const char *gfx_2d_fck_parents[] = {
"core_l3_ck",
};
+static struct clk *gfx_2d_fck_parents_ptr[] = {
+ &core_l3_ck,
+};
+
DEFINE_CLK_OMAP_MUX_GATE(gfx_2d_fck, "gfx_clkdm", gfx_fck_clksel,
OMAP_CM_REGADDR(GFX_MOD, CM_CLKSEL),
OMAP_CLKSEL_GFX_MASK,
@@ -671,6 +699,10 @@ static const char *gpt10_fck_parents[] = {
"func_32k_ck", "sys_ck", "alt_ck",
};
+static struct clk *gpt10_fck_parents_ptr[] = {
+ &func_32k_ck, &sys_ck, &alt_ck,
+};
+
DEFINE_CLK_OMAP_MUX_GATE(gpt10_fck, "core_l4_clkdm", omap24xx_gpt_clksel,
OMAP_CM_REGADDR(CORE_MOD, CM_CLKSEL2),
OMAP24XX_CLKSEL_GPT10_MASK,
@@ -1088,6 +1120,10 @@ static const char *mcbsp1_fck_parents[] = {
"func_96m_ck", "mcbsp_clks",
};
+static struct clk *mcbsp1_fck_parents_ptr[] = {
+ &func_96m_ck, &mcbsp_clks,
+};
+
DEFINE_CLK_OMAP_MUX_GATE(mcbsp1_fck, "core_l4_clkdm", mcbsp_fck_clksel,
OMAP242X_CTRL_REGADDR(OMAP2_CONTROL_DEVCONF0),
OMAP2_MCBSP1_CLKS_MASK,
@@ -1403,6 +1439,10 @@ static const char *ssi_ssr_sst_fck_parents[] = {
"core_ck",
};
+static
Re: [PATCH 2/2] ARM: OMAP2+: clocks: Pass static parent pointers to common clock core
* Rajendra Nayak [130314 05:44]: > OMAP clock inits happen quite early, even before the slab is available. > As part of the clock init, the common clock core tries to cache parent > pointers (if not passed by the caller registering the clock) which > fails in case of OMAP since the slab isn't initied. > Without CONFIG_DEBUG_SLAB enabled, this just results in the common clock core > retrying the caching attempt at some point later. > However with CONFIG_DEBUG_SLAB enabled this results in a BUG() as reported > in the link below by Tony.. > http://www.mail-archive.com/linux-omap@vger.kernel.org/msg85932.html > > Fix this by passing static parent pointers to the common clock core > while registering clocks. I wonder if we could easily fix this by initializing only some of the clocks that early? If we had a two stage initialization we could dynamically allocate the rest of the parent clocks without having to add all the static data. Regards, Tony -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] ARM: OMAP2+: clocks: Pass static parent pointers to common clock core
On Thursday 14 March 2013 10:36 PM, Tony Lindgren wrote: > * Rajendra Nayak [130314 05:44]: >> OMAP clock inits happen quite early, even before the slab is available. >> As part of the clock init, the common clock core tries to cache parent >> pointers (if not passed by the caller registering the clock) which >> fails in case of OMAP since the slab isn't initied. >> Without CONFIG_DEBUG_SLAB enabled, this just results in the common clock core >> retrying the caching attempt at some point later. >> However with CONFIG_DEBUG_SLAB enabled this results in a BUG() as reported >> in the link below by Tony.. >> http://www.mail-archive.com/linux-omap@vger.kernel.org/msg85932.html >> >> Fix this by passing static parent pointers to the common clock core >> while registering clocks. > > I wonder if we could easily fix this by initializing only some of the > clocks that early? We actually don't need any clocks that early (I mean even before slab init) We only need them before timer init. This isn't something specific to OMAP so I started looking at what others in drivers/clk seem to do. I found imx, spear, mvebu all do their clk inits within .init_time callbacks. tegra seems to even do it within .init_irq callback. Either one would work for us too :) > > If we had a two stage initialization we could dynamically allocate the > rest of the parent clocks without having to add all the static data. > > Regards, > > Tony > -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] ARM: OMAP2+: clocks: Pass static parent pointers to common clock core
* Rajendra Nayak [130315 05:31]: > On Thursday 14 March 2013 10:36 PM, Tony Lindgren wrote: > > * Rajendra Nayak [130314 05:44]: > >> OMAP clock inits happen quite early, even before the slab is available. > >> As part of the clock init, the common clock core tries to cache parent > >> pointers (if not passed by the caller registering the clock) which > >> fails in case of OMAP since the slab isn't initied. > >> Without CONFIG_DEBUG_SLAB enabled, this just results in the common clock > >> core > >> retrying the caching attempt at some point later. > >> However with CONFIG_DEBUG_SLAB enabled this results in a BUG() as reported > >> in the link below by Tony.. > >> http://www.mail-archive.com/linux-omap@vger.kernel.org/msg85932.html > >> > >> Fix this by passing static parent pointers to the common clock core > >> while registering clocks. > > > > I wonder if we could easily fix this by initializing only some of the > > clocks that early? > > We actually don't need any clocks that early (I mean even before slab init) > We only need them before timer init. Yes and only the timer clocks at that point. The others could be initialized later on so we get console error messages without having to turn on DEBUG_LL and earlyprintk. > This isn't something specific to OMAP so I started looking at what others > in drivers/clk seem to do. > > I found imx, spear, mvebu all do their clk inits within .init_time callbacks. > tegra seems to even do it within .init_irq callback. > > Either one would work for us too :) Well I suggest init_irq as that's when we need the first clocks for timer. Regards, Tony -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] ARM: OMAP2+: clocks: Pass static parent pointers to common clock core
On Friday 15 March 2013 09:50 PM, Tony Lindgren wrote: > * Rajendra Nayak [130315 05:31]: >> On Thursday 14 March 2013 10:36 PM, Tony Lindgren wrote: >>> * Rajendra Nayak [130314 05:44]: OMAP clock inits happen quite early, even before the slab is available. As part of the clock init, the common clock core tries to cache parent pointers (if not passed by the caller registering the clock) which fails in case of OMAP since the slab isn't initied. Without CONFIG_DEBUG_SLAB enabled, this just results in the common clock core retrying the caching attempt at some point later. However with CONFIG_DEBUG_SLAB enabled this results in a BUG() as reported in the link below by Tony.. http://www.mail-archive.com/linux-omap@vger.kernel.org/msg85932.html Fix this by passing static parent pointers to the common clock core while registering clocks. >>> >>> I wonder if we could easily fix this by initializing only some of the >>> clocks that early? >> >> We actually don't need any clocks that early (I mean even before slab init) >> We only need them before timer init. > > Yes and only the timer clocks at that point. The others could be initialized > later on so we get console error messages without having to turn on DEBUG_LL > and earlyprintk. but that wouldn't be possible with the hwmod dependency on clocks. We would need to move the hwmod inits further down too. > >> This isn't something specific to OMAP so I started looking at what others >> in drivers/clk seem to do. >> >> I found imx, spear, mvebu all do their clk inits within .init_time callbacks. >> tegra seems to even do it within .init_irq callback. >> >> Either one would work for us too :) > > Well I suggest init_irq as that's when we need the first clocks for timer. For which platforms? I instead see them being needed only by init_timer, so was thinking thats a better place instead. For the -rc fix to fix the crash with DEBUG_SLAB, is it fine if I move *all* clock inits down to init_timer (or init_irq). Splitting clocks alone would not work for now, without hwmod being moved down also. > > Regards, > > Tony > -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] ARM: OMAP2+: clocks: Pass static parent pointers to common clock core
* Rajendra Nayak [130318 01:25]: > On Friday 15 March 2013 09:50 PM, Tony Lindgren wrote: > > * Rajendra Nayak [130315 05:31]: > >> On Thursday 14 March 2013 10:36 PM, Tony Lindgren wrote: > >>> * Rajendra Nayak [130314 05:44]: > OMAP clock inits happen quite early, even before the slab is available. > As part of the clock init, the common clock core tries to cache parent > pointers (if not passed by the caller registering the clock) which > fails in case of OMAP since the slab isn't initied. > Without CONFIG_DEBUG_SLAB enabled, this just results in the common clock > core > retrying the caching attempt at some point later. > However with CONFIG_DEBUG_SLAB enabled this results in a BUG() as > reported > in the link below by Tony.. > http://www.mail-archive.com/linux-omap@vger.kernel.org/msg85932.html > > Fix this by passing static parent pointers to the common clock core > while registering clocks. > >>> > >>> I wonder if we could easily fix this by initializing only some of the > >>> clocks that early? > >> > >> We actually don't need any clocks that early (I mean even before slab init) > >> We only need them before timer init. > > > > Yes and only the timer clocks at that point. The others could be initialized > > later on so we get console error messages without having to turn on DEBUG_LL > > and earlyprintk. > > but that wouldn't be possible with the hwmod dependency on clocks. We would > need to move the hwmod inits further down too. > > > > >> This isn't something specific to OMAP so I started looking at what others > >> in drivers/clk seem to do. > >> > >> I found imx, spear, mvebu all do their clk inits within .init_time > >> callbacks. > >> tegra seems to even do it within .init_irq callback. > >> > >> Either one would work for us too :) > > > > Well I suggest init_irq as that's when we need the first clocks for timer. > > For which platforms? I instead see them being needed only by init_timer, so > was thinking thats a better place instead. > > For the -rc fix to fix the crash with DEBUG_SLAB, is it fine if I move *all* > clock inits down to init_timer (or init_irq). Sounds good to me for the -rc series. How about initialize omap_clk_init() function pointer in the SoC specific omap*_init_early() then just call omap_clk_init() in timer_init? > Splitting clocks alone would not work for now, without hwmod being moved down > also. Good point. For the -rc series the sounds like your suggestion is the least intrusive fix and also gets us started removing few more early boot time dependencies. Regards, Tony -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] ARM: OMAP2+: clocks: Pass static parent pointers to common clock core
Quoting Tony Lindgren (2013-03-18 09:37:58) > * Rajendra Nayak [130318 01:25]: > > On Friday 15 March 2013 09:50 PM, Tony Lindgren wrote: > > > * Rajendra Nayak [130315 05:31]: > > >> On Thursday 14 March 2013 10:36 PM, Tony Lindgren wrote: > > >>> * Rajendra Nayak [130314 05:44]: > > OMAP clock inits happen quite early, even before the slab is available. > > As part of the clock init, the common clock core tries to cache parent > > pointers (if not passed by the caller registering the clock) which > > fails in case of OMAP since the slab isn't initied. > > Without CONFIG_DEBUG_SLAB enabled, this just results in the common > > clock core > > retrying the caching attempt at some point later. > > However with CONFIG_DEBUG_SLAB enabled this results in a BUG() as > > reported > > in the link below by Tony.. > > http://www.mail-archive.com/linux-omap@vger.kernel.org/msg85932.html > > > > Fix this by passing static parent pointers to the common clock core > > while registering clocks. > > >>> > > >>> I wonder if we could easily fix this by initializing only some of the > > >>> clocks that early? > > >> > > >> We actually don't need any clocks that early (I mean even before slab > > >> init) > > >> We only need them before timer init. > > > > > > Yes and only the timer clocks at that point. The others could be > > > initialized > > > later on so we get console error messages without having to turn on > > > DEBUG_LL > > > and earlyprintk. > > > > but that wouldn't be possible with the hwmod dependency on clocks. We would > > need to move the hwmod inits further down too. > > > > > > > >> This isn't something specific to OMAP so I started looking at what others > > >> in drivers/clk seem to do. > > >> > > >> I found imx, spear, mvebu all do their clk inits within .init_time > > >> callbacks. > > >> tegra seems to even do it within .init_irq callback. > > >> > > >> Either one would work for us too :) > > > > > > Well I suggest init_irq as that's when we need the first clocks for timer. > > > > For which platforms? I instead see them being needed only by init_timer, so > > was thinking thats a better place instead. > > > > For the -rc fix to fix the crash with DEBUG_SLAB, is it fine if I move *all* > > clock inits down to init_timer (or init_irq). > > Sounds good to me for the -rc series. How about initialize omap_clk_init() > function pointer in the SoC specific omap*_init_early() then just call > omap_clk_init() in timer_init? > > > Splitting clocks alone would not work for now, without hwmod being moved > > down also. > > Good point. For the -rc series the sounds like your suggestion is the > least intrusive fix and also gets us started removing few more early > boot time dependencies. > +1 for removing the early init dependencies. hwmod initialization was the original impetus for all this static struct clk stuff. If the initialization for hwmod and the omap clocks can be moved to a later callback then it is the first step in removing the static data and getting rid of clk-private.h usage. Regards, Mike > Regards, > > Tony -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] ARM: OMAP2+: clocks: Pass static parent pointers to common clock core
On Tuesday 19 March 2013 06:53 AM, Mike Turquette wrote: > Quoting Tony Lindgren (2013-03-18 09:37:58) >> * Rajendra Nayak [130318 01:25]: >>> On Friday 15 March 2013 09:50 PM, Tony Lindgren wrote: * Rajendra Nayak [130315 05:31]: > On Thursday 14 March 2013 10:36 PM, Tony Lindgren wrote: >> * Rajendra Nayak [130314 05:44]: >>> OMAP clock inits happen quite early, even before the slab is available. >>> As part of the clock init, the common clock core tries to cache parent >>> pointers (if not passed by the caller registering the clock) which >>> fails in case of OMAP since the slab isn't initied. >>> Without CONFIG_DEBUG_SLAB enabled, this just results in the common >>> clock core >>> retrying the caching attempt at some point later. >>> However with CONFIG_DEBUG_SLAB enabled this results in a BUG() as >>> reported >>> in the link below by Tony.. >>> http://www.mail-archive.com/linux-omap@vger.kernel.org/msg85932.html >>> >>> Fix this by passing static parent pointers to the common clock core >>> while registering clocks. >> >> I wonder if we could easily fix this by initializing only some of the >> clocks that early? > > We actually don't need any clocks that early (I mean even before slab > init) > We only need them before timer init. Yes and only the timer clocks at that point. The others could be initialized later on so we get console error messages without having to turn on DEBUG_LL and earlyprintk. >>> >>> but that wouldn't be possible with the hwmod dependency on clocks. We would >>> need to move the hwmod inits further down too. >>> > This isn't something specific to OMAP so I started looking at what others > in drivers/clk seem to do. > > I found imx, spear, mvebu all do their clk inits within .init_time > callbacks. > tegra seems to even do it within .init_irq callback. > > Either one would work for us too :) Well I suggest init_irq as that's when we need the first clocks for timer. >>> >>> For which platforms? I instead see them being needed only by init_timer, so >>> was thinking thats a better place instead. >>> >>> For the -rc fix to fix the crash with DEBUG_SLAB, is it fine if I move *all* >>> clock inits down to init_timer (or init_irq). >> >> Sounds good to me for the -rc series. How about initialize omap_clk_init() >> function pointer in the SoC specific omap*_init_early() then just call >> omap_clk_init() in timer_init? >> >>> Splitting clocks alone would not work for now, without hwmod being moved >>> down also. >> >> Good point. For the -rc series the sounds like your suggestion is the >> least intrusive fix and also gets us started removing few more early >> boot time dependencies. >> > > +1 for removing the early init dependencies. hwmod initialization was > the original impetus for all this static struct clk stuff. If the > initialization for hwmod and the omap clocks can be moved to a later > callback then it is the first step in removing the static data and > getting rid of clk-private.h usage. I am working on that stuff to get rid of the dependency on clk-private.h Will post some RFC soon once I have omap4 working and if the approach seems fine, will move over all of omap so you can delete the clk-private.h completely :) > > Regards, > Mike > >> Regards, >> >> Tony -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
