From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1759485Ab3KMO46 (ORCPT ); Wed, 13 Nov 2013 09:56:58 -0500 Received: from bear.ext.ti.com ([192.94.94.41]:40153 "EHLO bear.ext.ti.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755676Ab3KMO4z (ORCPT ); Wed, 13 Nov 2013 09:56:55 -0500 Message-ID: <52839306.7050701@ti.com> Date: Wed, 13 Nov 2013 08:56:06 -0600 From: Nishanth Menon User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.0 MIME-Version: 1.0 To: CC: Tony Lindgren , , , , , , Subject: Re: [PATCH V2] ARM: OMAP2+: omap_device: maintain sane runtime pm status around suspend/resume References: <1384297710-29694-1-git-send-email-nm@ti.com> <20131113125149.GB28192@saruman.home> In-Reply-To: <20131113125149.GB28192@saruman.home> Content-Type: text/plain; charset="ISO-8859-1" Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 11/13/2013 06:51 AM, Felipe Balbi wrote: > Hi, > > On Tue, Nov 12, 2013 at 05:08:30PM -0600, Nishanth Menon wrote: >> diff --git a/arch/arm/mach-omap2/omap_device.c b/arch/arm/mach-omap2/omap_device.c >> index b69dd9a..f97b34b 100644 >> --- a/arch/arm/mach-omap2/omap_device.c >> +++ b/arch/arm/mach-omap2/omap_device.c >> @@ -621,6 +621,7 @@ static int _od_suspend_noirq(struct device *dev) >> >> if (!ret && !pm_runtime_status_suspended(dev)) { >> if (pm_generic_runtime_suspend(dev) == 0) { >> + pm_runtime_set_suspended(dev); > > don't you have to disable pm_runtime around status changes ? Or is > pm_runtime already disabled by the time we get here ? pm_runtime is already disabled by the time no_irq suspend is invoked. > >> @@ -634,10 +635,10 @@ static int _od_resume_noirq(struct device *dev) >> struct platform_device *pdev = to_platform_device(dev); >> struct omap_device *od = to_omap_device(pdev); >> >> - if ((od->flags & OMAP_DEVICE_SUSPENDED) && >> - !pm_runtime_status_suspended(dev)) { >> + if (od->flags & OMAP_DEVICE_SUSPENDED) { >> od->flags &= ~OMAP_DEVICE_SUSPENDED; >> omap_device_enable(pdev); >> + pm_runtime_set_active(dev); > > ditto, also pm_runtime_set_active() may fail. > again, pm_runtime is not yet active here yet - we just restore the pm runtime state with which we went down with -> and that is not expected to fail either - So, how about just adding a WARN if our expectation of balanced operation was somehow broken in the future with changes to runtime framework? -- Regards, Nishanth Menon