From patchwork Mon Jul 15 10:01:01 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Rajendra Nayak X-Patchwork-Id: 2827345 Return-Path: X-Original-To: patchwork-linux-arm@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 3C3159F967 for ; Mon, 15 Jul 2013 10:01:50 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 9B67220171 for ; Mon, 15 Jul 2013 10:01:48 +0000 (UTC) Received: from casper.infradead.org (casper.infradead.org [85.118.1.10]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 240852016C for ; Mon, 15 Jul 2013 10:01:47 +0000 (UTC) Received: from merlin.infradead.org ([2001:4978:20e::2]) by casper.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1Uyfb3-0006Tu-NS; Mon, 15 Jul 2013 10:01:33 +0000 Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1Uyfb1-0007gv-Al; Mon, 15 Jul 2013 10:01:31 +0000 Received: from arroyo.ext.ti.com ([192.94.94.40]) by merlin.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1Uyfay-0007fv-LG for linux-arm-kernel@lists.infradead.org; Mon, 15 Jul 2013 10:01:29 +0000 Received: from dlelxv90.itg.ti.com ([172.17.2.17]) by arroyo.ext.ti.com (8.13.7/8.13.7) with ESMTP id r6FA151x007460; Mon, 15 Jul 2013 05:01:05 -0500 Received: from DFLE73.ent.ti.com (dfle73.ent.ti.com [128.247.5.110]) by dlelxv90.itg.ti.com (8.14.3/8.13.8) with ESMTP id r6FA15AM001032; Mon, 15 Jul 2013 05:01:05 -0500 Received: from dlelxv22.itg.ti.com (172.17.1.197) by DFLE73.ent.ti.com (128.247.5.110) with Microsoft SMTP Server id 14.2.342.3; Mon, 15 Jul 2013 05:01:05 -0500 Received: from [172.24.145.51] (ula0131687-172024145051.apr.dhcp.ti.com [172.24.145.51]) by dlelxv22.itg.ti.com (8.13.8/8.13.8) with ESMTP id r6FA118n009311; Mon, 15 Jul 2013 05:01:02 -0500 Message-ID: <51E3C85D.2010008@ti.com> Date: Mon, 15 Jul 2013 15:31:01 +0530 From: Rajendra Nayak User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:15.0) Gecko/20120827 Thunderbird/15.0 MIME-Version: 1.0 To: Suman Anna Subject: Re: Boot hang regression 3.10.0-rc4 -> 3.10.0 References: <20130708112553.GU5523@atomide.com> <51DAB394.3050104@ti.com> <20130708131033.GA5523@atomide.com> <51DABC81.3080409@ti.com> <20130708133512.GD31221@arwen.pp.htv.fi> <87mwpuakod.fsf@linaro.org> <20130710142633.GV5523@atomide.com> <20130710160704.GH18966@arwen.pp.htv.fi> <20130710161158.GA19716@arwen.pp.htv.fi> <20130711063209.GZ5523@atomide.com> <51DE8215.5060306@ti.com> <51DF508F.3050703@ti.com> <51E39A3F.6080106@ti.com> In-Reply-To: <51E39A3F.6080106@ti.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20130715_060128_860837_0C932716 X-CRM114-Status: GOOD ( 30.15 ) X-Spam-Score: -7.3 (-------) Cc: Paul Walmsley , Kevin Hilman , Mark Jackson , Tony Lindgren , "Bedia, Vaibhav" , Grygorii Strashko , Felipe Balbi , Sourav Poddar , "linux-omap@vger.kernel.org" , "linux-arm-kernel@lists.infradead.org" X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org X-Spam-Status: No, score=-4.6 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_MED, RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Monday 15 July 2013 12:14 PM, Rajendra Nayak wrote: > On Friday 12 July 2013 06:10 AM, Suman Anna wrote: >> On 07/11/2013 04:59 AM, Grygorii Strashko wrote: >>> Hi, >>> >>> On 07/11/2013 09:32 AM, Tony Lindgren wrote: >>>> * Felipe Balbi [130710 09:18]: >>>>> >>>>> On Wed, Jul 10, 2013 at 07:07:04PM +0300, Felipe Balbi wrote: >>>>>> how about something like below ? It makes omap_device/hwmod and >>>>>> pm_runtime agree on the initial state of the device and will prevent >>>>>> ->runtime_resume() from being called on first pm_runtime_get*() done >>>>>> during probe. >>>>>> >>>>>> This is similar to what PCI bus does (if you look at pci_pm_init()). >>>>>> >>>>>> commit 59108a500b4ab4b1a5102648a3360276dbf7df6f >>>>>> Author: Felipe Balbi >>>>>> Date: Wed Jul 10 18:50:16 2013 +0300 >>>>>> >>>>>> arm: omap2plus: unidle devices which are about to probe >>>>>> >>>>>> in order to make HWMOD and pm_runtime agree on the >>>>>> initial state of the device, we will unidle the device >>>>>> and call pm_runtime_set_active() to tell pm_runtime >>>>>> that the device is really active. >>> Don't think that it's good idea ( >>> I've checked some driver's and think this patch will enable some devices >>> unpredictably: >>> - hwspinlock >>> - mailbox >>> - iommu >>> - ipu >>> All above devices need to be enabled on demand only (no >>> pm_runtime_get*() calls in probe). More over, some of them have very >>> specific enabling sequence - like ipu). >>> >>> May be Summan can say more on that. >> >> Indeed, this is a problem for any of the slave processor devices. >> mailbox and iommu would be slaves to the remoteproc and the drivers have >> a specific sequence of bringing up a processor. The current >> hwmod/omap_device code is such that these devices will be left in reset >> and the driver code use the omap_device_(de)assert_hardreset API >> together with omap_device_enable code to bring up the devices. The >> remoteproc driver also needs to assert the resets (there are other >> problems associated with using omap_device_idle for remoteproc and >> iommu) for bringing up the devices after a suspend sequence. hwspinlock >> and mailbox may get away since they are in CORE domain, but definitely >> an issue for iommu and remoteproc. I would think that this would also >> affect other compute devices like IVAHD, ISS, SGX. > > Today, for these IPs I guess hwmod waits for the resets to be de-asserted, right? > > /* > * If an IP block contains HW reset lines and all of them are > * asserted, we let integration code associated with that > * block handle the enable. We've received very little > * information on what those driver authors need, and until > * detailed information is provided and the driver code is > * posted to the public lists, this is probably the best we > * can do. > */ > if (_are_all_hardreset_lines_asserted(oh)) > return 0; > > What if this information is send back to omap_device() through a return value > so omap_device() knows about this too, so it avoids marking the omap device as > enabled? Wouldn't that fix the issue? I meant something like this.. From 2fbea0dde0f72897089ef2e8e441b5e5bd6ea967 Mon Sep 17 00:00:00 2001 From: Rajendra Nayak Date: Mon, 15 Jul 2013 15:23:07 +0530 Subject: [PATCH] ARM: OMAP2+: Make omap_device aware of hwmod failing to enable/idle/shutdown the hwmods For IP blocks (mainly processors) which have hard reset lines, hwmod avoids enable/idle/shutdown operations as long as all the hard reset lines are kept asserted. However it does not return an error back to the caller (in some cases the omap_device layer) to communicate back the failure to operate on the hwmod. Fix this by making _enable()/_idle()/_shutdown() all return an error in such cases, and also fix the omap_device layer to look at the return values coming from hwmod operations before doing a omap_device level state transition. Signed-off-by: Rajendra Nayak --- arch/arm/mach-omap2/omap_device.c | 20 ++++++++++---------- arch/arm/mach-omap2/omap_hwmod.c | 6 +++--- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/arch/arm/mach-omap2/omap_device.c b/arch/arm/mach-omap2/omap_device.c index 5cc9287..e89244b 100644 --- a/arch/arm/mach-omap2/omap_device.c +++ b/arch/arm/mach-omap2/omap_device.c @@ -210,13 +210,12 @@ static int _omap_device_notifier_call(struct notifier_block *nb, */ static int _omap_device_enable_hwmods(struct omap_device *od) { - int i; + int i, ret = 0; for (i = 0; i < od->hwmods_cnt; i++) - omap_hwmod_enable(od->hwmods[i]); + ret |= omap_hwmod_enable(od->hwmods[i]); - /* XXX pass along return value here? */ - return 0; + return ret; } /** @@ -227,13 +226,12 @@ static int _omap_device_enable_hwmods(struct omap_device *od) */ static int _omap_device_idle_hwmods(struct omap_device *od) { - int i; + int i, ret = 0; for (i = 0; i < od->hwmods_cnt; i++) - omap_hwmod_idle(od->hwmods[i]); + ret |= omap_hwmod_idle(od->hwmods[i]); - /* XXX pass along return value here? */ - return 0; + return ret; } /* Public functions for use by core code */ @@ -697,7 +695,8 @@ int omap_device_enable(struct platform_device *pdev) ret = _omap_device_enable_hwmods(od); - od->_state = OMAP_DEVICE_STATE_ENABLED; + if (!ret) + od->_state = OMAP_DEVICE_STATE_ENABLED; return ret; } @@ -727,7 +726,8 @@ int omap_device_idle(struct platform_device *pdev) ret = _omap_device_idle_hwmods(od); - od->_state = OMAP_DEVICE_STATE_IDLE; + if (!ret) + od->_state = OMAP_DEVICE_STATE_IDLE; return ret; } diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c index 7341eff..07fd2a8 100644 --- a/arch/arm/mach-omap2/omap_hwmod.c +++ b/arch/arm/mach-omap2/omap_hwmod.c @@ -2133,7 +2133,7 @@ static int _enable(struct omap_hwmod *oh) * can do. */ if (_are_all_hardreset_lines_asserted(oh)) - return 0; + return -EPERM; /* Mux pins for device runtime if populated */ if (oh->mux && (!oh->mux->enabled || @@ -2222,7 +2222,7 @@ static int _idle(struct omap_hwmod *oh) } if (_are_all_hardreset_lines_asserted(oh)) - return 0; + return -EPERM; if (oh->class->sysc) _idle_sysc(oh); @@ -2276,7 +2276,7 @@ static int _shutdown(struct omap_hwmod *oh) } if (_are_all_hardreset_lines_asserted(oh)) - return 0; + return -EPERM; pr_debug("omap_hwmod: %s: disabling\n", oh->name);