From mboxrd@z Thu Jan 1 00:00:00 1970 From: Rajendra Nayak Subject: Re: [PATCH 2/2] ARM: OMAP2+: clocks: Pass static parent pointers to common clock core Date: Mon, 18 Mar 2013 13:50:43 +0530 Message-ID: <5146CE5B.6090705@ti.com> References: <1363264618-5071-1-git-send-email-rnayak@ti.com> <1363264618-5071-3-git-send-email-rnayak@ti.com> <20130314170601.GH26093@atomide.com> <514313B6.4050905@ti.com> <20130315162049.GB9370@atomide.com> Mime-Version: 1.0 Content-Type: text/plain; charset="ISO-8859-1" Content-Transfer-Encoding: 7bit Return-path: Received: from arroyo.ext.ti.com ([192.94.94.40]:37400 "EHLO arroyo.ext.ti.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751561Ab3CRIU6 (ORCPT ); Mon, 18 Mar 2013 04:20:58 -0400 In-Reply-To: <20130315162049.GB9370@atomide.com> Sender: linux-omap-owner@vger.kernel.org List-Id: linux-omap@vger.kernel.org To: Tony Lindgren Cc: paul@pwsan.com, linux-omap@vger.kernel.org, linux-arm-kernel@lists.infradead.org, phaber@broadcom.com, Mike Turquette , "Shilimkar, Santosh" 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 > From mboxrd@z Thu Jan 1 00:00:00 1970 From: rnayak@ti.com (Rajendra Nayak) Date: Mon, 18 Mar 2013 13:50:43 +0530 Subject: [PATCH 2/2] ARM: OMAP2+: clocks: Pass static parent pointers to common clock core In-Reply-To: <20130315162049.GB9370@atomide.com> References: <1363264618-5071-1-git-send-email-rnayak@ti.com> <1363264618-5071-3-git-send-email-rnayak@ti.com> <20130314170601.GH26093@atomide.com> <514313B6.4050905@ti.com> <20130315162049.GB9370@atomide.com> Message-ID: <5146CE5B.6090705@ti.com> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org 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 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). Splitting clocks alone would not work for now, without hwmod being moved down also. > > Regards, > > Tony >