public inbox for linux-arm-kernel@lists.infradead.org 
 help / color / mirror / Atom feed
From: rnayak@ti•com (Rajendra Nayak)
To: linux-arm-kernel@lists•infradead.org
Subject: [PATCH 2/2] ARM: OMAP2+: clocks: Pass static parent pointers to common clock core
Date: Tue, 19 Mar 2013 10:46:50 +0530	[thread overview]
Message-ID: <5147F4C2.5050307@ti.com> (raw)
In-Reply-To: <20130319012340.8663.43189@quantum>

On Tuesday 19 March 2013 06:53 AM, Mike Turquette wrote:
> Quoting Tony Lindgren (2013-03-18 09:37:58)
>> * Rajendra Nayak <rnayak@ti•com> [130318 01:25]:
>>> On Friday 15 March 2013 09:50 PM, Tony Lindgren wrote:
>>>> * Rajendra Nayak <rnayak@ti•com> [130315 05:31]:
>>>>> On Thursday 14 March 2013 10:36 PM, Tony Lindgren wrote:
>>>>>> * Rajendra Nayak <rnayak@ti•com> [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 at 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

  reply	other threads:[~2013-03-19  5:16 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-03-14 12:36 [PATCH 0/2] Fix crash on OMAP with CONFIG_DEBUG_SLAB Rajendra Nayak
2013-03-14 12:36 ` [PATCH 2/2] ARM: OMAP2+: clocks: Pass static parent pointers to common clock core Rajendra Nayak
2013-03-14 17:06   ` Tony Lindgren
2013-03-15 12:27     ` Rajendra Nayak
2013-03-15 16:20       ` Tony Lindgren
2013-03-18  8:20         ` Rajendra Nayak
2013-03-18 16:37           ` Tony Lindgren
2013-03-19  1:23             ` Mike Turquette
2013-03-19  5:16               ` Rajendra Nayak [this message]
2013-03-14 12:45 ` [PATCH 0/2] Fix crash on OMAP with CONFIG_DEBUG_SLAB Rajendra Nayak

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5147F4C2.5050307@ti.com \
    --to=rnayak@ti$(echo .)com \
    --cc=linux-arm-kernel@lists$(echo .)infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox