From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752979AbcD0Hhk (ORCPT ); Wed, 27 Apr 2016 03:37:40 -0400 Received: from mail.kmu-office.ch ([178.209.48.109]:52535 "EHLO mail.kmu-office.ch" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751988AbcD0Hhj (ORCPT ); Wed, 27 Apr 2016 03:37:39 -0400 MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Date: Wed, 27 Apr 2016 00:34:07 -0700 From: Stefan Agner To: Dong Aisheng Cc: Shawn Guo , Lucas Stach , Michael Turquette , Stephen Boyd , linux-kernel@vger.kernel.org, mingo@redhat.com, kernel@pengutronix.de, tglx@linutronix.de, linux-clk@vger.kernel.org, linux-arm-kernel@lists.infradead.org Subject: Re: [PATCH 1/2] clk: imx: do not sleep if IRQ's are still disabled In-Reply-To: References: <1454107764-19876-1-git-send-email-stefan@agner.ch> <20160421034520.GA19965@shlinux2.ap.freescale.net> <20160426012341.GB8870@tiger> <1461663072.7839.17.camel@pengutronix.de> <20160427015835.GE30692@tiger> Message-ID: User-Agent: Roundcube Webmail/1.1.3 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 2016-04-26 19:57, Dong Aisheng wrote: > On Wed, Apr 27, 2016 at 9:58 AM, Shawn Guo wrote: >> On Tue, Apr 26, 2016 at 07:27:03PM +0800, Dong Aisheng wrote: >>> Shawn, >>> What's your suggestion? >> >> I think this needs more discussion, and I just dropped Stefan's patch >> from my tree. >> >> We need to firstly understand why this is happening. The .prepare hook >> is defined to be non-atomic context, and so that we call sleep function >> in there. We did everything right. Why are we getting the warning? If >> I'm correct, this warning only happens on i.MX7D. Why is that? >> > > Why Stefan's patch works (checking irqs_disabled()) is because during kernel > time init, the irq is still not enabled. It fixes the issue indirectly. > See: > asmlinkage __visible void __init start_kernel(void) > { > /* > * Set up the scheduler prior starting any interrupts (such as the > * timer interrupt). Full topology setup happens at smp_init() > * time - but meanwhile we still have a functioning scheduler. > */ > sched_init(); > ............. > time_init(); > .............. > WARN(!irqs_disabled(), "Interrupts were enabled early\n"); > early_boot_irqs_disabled = false; > local_irq_enable(); > } > > The issue can only happen when PLL enable causes a schedule during > imx_clock_init(). > Not all PLL has this issue. > The issue happens on MX7D pll_audio_main_clk/pll_video_main_clk > which requires more delay time and cause usleep. > Because clk framework does not support MX7D clock types (operation requires > parents on), we simply enable all clocks in imx7d_clocks_init(). > > If apply my this patch series: > https://lkml.org/lkml/2016/4/20/199 > The issue can also be gone. Oh ok, it does make sense to enable as few clocks as possible. However, even if we do not enable lots of clocks at that time, and this helps to avoid the problem for now, it could still be that someone/something requests a clock early during boot, leading to a PLL enable... Shouldn't we make sure that those base clocks can be enabled even during early boot..? -- Stefan From mboxrd@z Thu Jan 1 00:00:00 1970 From: stefan@agner.ch (Stefan Agner) Date: Wed, 27 Apr 2016 00:34:07 -0700 Subject: [PATCH 1/2] clk: imx: do not sleep if IRQ's are still disabled In-Reply-To: References: <1454107764-19876-1-git-send-email-stefan@agner.ch> <20160421034520.GA19965@shlinux2.ap.freescale.net> <20160426012341.GB8870@tiger> <1461663072.7839.17.camel@pengutronix.de> <20160427015835.GE30692@tiger> Message-ID: To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On 2016-04-26 19:57, Dong Aisheng wrote: > On Wed, Apr 27, 2016 at 9:58 AM, Shawn Guo wrote: >> On Tue, Apr 26, 2016 at 07:27:03PM +0800, Dong Aisheng wrote: >>> Shawn, >>> What's your suggestion? >> >> I think this needs more discussion, and I just dropped Stefan's patch >> from my tree. >> >> We need to firstly understand why this is happening. The .prepare hook >> is defined to be non-atomic context, and so that we call sleep function >> in there. We did everything right. Why are we getting the warning? If >> I'm correct, this warning only happens on i.MX7D. Why is that? >> > > Why Stefan's patch works (checking irqs_disabled()) is because during kernel > time init, the irq is still not enabled. It fixes the issue indirectly. > See: > asmlinkage __visible void __init start_kernel(void) > { > /* > * Set up the scheduler prior starting any interrupts (such as the > * timer interrupt). Full topology setup happens at smp_init() > * time - but meanwhile we still have a functioning scheduler. > */ > sched_init(); > ............. > time_init(); > .............. > WARN(!irqs_disabled(), "Interrupts were enabled early\n"); > early_boot_irqs_disabled = false; > local_irq_enable(); > } > > The issue can only happen when PLL enable causes a schedule during > imx_clock_init(). > Not all PLL has this issue. > The issue happens on MX7D pll_audio_main_clk/pll_video_main_clk > which requires more delay time and cause usleep. > Because clk framework does not support MX7D clock types (operation requires > parents on), we simply enable all clocks in imx7d_clocks_init(). > > If apply my this patch series: > https://lkml.org/lkml/2016/4/20/199 > The issue can also be gone. Oh ok, it does make sense to enable as few clocks as possible. However, even if we do not enable lots of clocks at that time, and this helps to avoid the problem for now, it could still be that someone/something requests a clock early during boot, leading to a PLL enable... Shouldn't we make sure that those base clocks can be enabled even during early boot..? -- Stefan