From patchwork Fri Oct 3 01:14:42 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kevin Hilman X-Patchwork-Id: 5020851 Return-Path: X-Original-To: patchwork-linux-arm@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork2.web.kernel.org (Postfix) with ESMTP id 1BF10C11AB for ; Fri, 3 Oct 2014 01:18:33 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 3400A2024D for ; Fri, 3 Oct 2014 01:18:32 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.9]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 4FE20201F5 for ; Fri, 3 Oct 2014 01:18:31 +0000 (UTC) Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1XZrSm-0003ZN-K2; Fri, 03 Oct 2014 01:15:16 +0000 Received: from mail.kernel.org ([198.145.19.201]) by bombadil.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1XZrSj-0002RC-KQ for linux-arm-kernel@lists.infradead.org; Fri, 03 Oct 2014 01:15:14 +0000 Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id AD0A22024D; Fri, 3 Oct 2014 01:14:47 +0000 (UTC) Received: from localhost (c-67-183-17-239.hsd1.wa.comcast.net [67.183.17.239]) (using TLSv1.2 with cipher DHE-RSA-AES128-SHA (128/128 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id 182CA201F5; Fri, 3 Oct 2014 01:14:43 +0000 (UTC) From: Kevin Hilman To: Ulf Hansson , "Rafael J. Wysocki" Subject: Re: [PATCH v2 0/4] PM / Domains: Fix race conditions during boot References: <1412174494-15346-1-git-send-email-ulf.hansson@linaro.org> Date: Thu, 02 Oct 2014 18:14:42 -0700 In-Reply-To: <1412174494-15346-1-git-send-email-ulf.hansson@linaro.org> (Ulf Hansson's message of "Wed, 1 Oct 2014 16:41:30 +0200") Message-ID: <7h38b6rm2l.fsf@deeprootsystems.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux) MIME-Version: 1.0 X-Spam-Status: No, score=-2.9 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_NONE, 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 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20141002_181513_721329_313B5A8C X-CRM114-Status: GOOD ( 23.73 ) X-Spam-Score: -6.0 (------) Cc: Len Brown , Philipp Zabel , linux-samsung-soc@vger.kernel.org, Russell King , Ben Dooks , Geert Uytterhoeven , linux-pm@vger.kernel.org, Greg Kroah-Hartman , Mark Brown , Wolfram Sang , Magnus Damm , Tomasz Figa , Simon Horman , Alan Stern , Pavel Machek , Jinkun Hong , Kukjin Kim , Dmitry Torokhov , Jack Dai , linux-arm-kernel@lists.infradead.org X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.18-1 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-Virus-Scanned: ClamAV using ClamSMTP Ulf, Rafael, Ulf Hansson writes: > When there are more than one device in a PM domain these will obviously > be probed at different times. Depending on timing and the implemented > support for runtime PM in a driver/subsystem, genpd may be advised to > power off a PM domain after a successful probe sequence. > > Ideally we should have relied on the driver/subsystem, through runtime > PM, to bring their device's PM domain into powered state prior doing > probing if such requirement exist. I think I've stumbled on a related problem, or maybe the same one. Even if platform-specific init code has initialized a device with pm_runtime_set_active(), it seems that the genpd domain can still power off before before all of its devices are probed. This is because pm_genpd_poweroff() requires there to be a driver when it's checking if a device is pm_runtime_suspended() which will not be the case if the driver has not been probed yet. Consider this case: There are several devices in the domain that haven't been probed yet (dev->driver == NULL), but have been marked with pm_runtime_set_active() + _get_noresume(), so pm_runtime_suspended() == false. Then, one of devices is in the domain is probed, and during the probe it does a _get_sync(), sets some stuff up, and then does _put_sync(). After the probe, because of the _put_sync(), the genpd ->runtime_suspend() will be triggered, causing it to attempt a _genpd_poweroff(). Since the rest of the devices in the domain haven't (yet) been probed, their dev->driver pointers are all still NULL, so the pm_runtime_suspended() check will not be attempted for them. The result is that the genpd will poweroff after the first device is probed, but before the others have had a chance to probe, which is not exactly desired behavior for a genpd that has been initialized as powered on. With the hack below[1], I'm able to avoid that problem, but am not completely sure yet if this is safe in general. Rafael, do you remember why that check for dev->driver is needed? Without digging deeper (which I'll do tomorrow), seems to me that checking pm_runtime_suspended() on devices without drivers is a reasonable thing to do since they can be initailzed by platform code before they are probed. If you think this is OK, I'll cook up a real patch with a changelog. Ulf, I'm not sure if this is the same problem you're having, but do you think this would solve your problem if the drivers are properly initialized? Kevin [1] } diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c index 568bf3172bef..17b0d9466d98 100644 --- a/drivers/base/power/domain.c +++ b/drivers/base/power/domain.c @@ -471,7 +471,7 @@ static int pm_genpd_poweroff(struct generic_pm_domain *genpd) if (stat > PM_QOS_FLAGS_NONE) return -EBUSY; - if (pdd->dev->driver && (!pm_runtime_suspended(pdd->dev) + if ((!pm_runtime_suspended(pdd->dev) || pdd->dev->power.irq_safe)) not_suspended++;