From patchwork Sun Feb 12 16:07:45 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Lukas Wunner X-Patchwork-Id: 9568443 X-Patchwork-Delegate: bhelgaas@google.com Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id A780060476 for ; Sun, 12 Feb 2017 16:09:48 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 974C828394 for ; Sun, 12 Feb 2017 16:09:48 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 8AE1A28399; Sun, 12 Feb 2017 16:09:48 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.9 required=2.0 tests=BAYES_00,RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id F1D9628394 for ; Sun, 12 Feb 2017 16:09:47 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751238AbdBLQJr (ORCPT ); Sun, 12 Feb 2017 11:09:47 -0500 Received: from mailout3.hostsharing.net ([176.9.242.54]:52689 "EHLO mailout3.hostsharing.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751226AbdBLQJq (ORCPT ); Sun, 12 Feb 2017 11:09:46 -0500 Received: from h08.hostsharing.net (h08.hostsharing.net [83.223.95.28]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mailout3.hostsharing.net (Postfix) with ESMTPS id 9218F101E6922; Sun, 12 Feb 2017 17:09:44 +0100 (CET) Received: from localhost (3-38-90-81.adsl.cmo.de [81.90.38.3]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-SHA (128/128 bits)) (No client certificate requested) by h08.hostsharing.net (Postfix) with ESMTPSA id BB79E603E044; Sun, 12 Feb 2017 17:09:42 +0100 (CET) X-Mailbox-Line: From 5a2cea3d2fd2f3cdaa30d3a78016c3e5800a32a6 Mon Sep 17 00:00:00 2001 Message-Id: <5a2cea3d2fd2f3cdaa30d3a78016c3e5800a32a6.1486913733.git.lukas@wunner.de> In-Reply-To: References: From: Lukas Wunner Date: Sun, 12 Feb 2017 17:07:45 +0100 Subject: [PATCH v6 5/8] PM: Make requirements of dev_pm_domain_set() more precise To: Bjorn Helgaas Cc: Andreas Noever , linux-pci@vger.kernel.org, "Rafael J. Wysocki" Sender: linux-pci-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-pci@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Since commit 989561de9b51 ("PM / Domains: add setter for dev.pm_domain") a PM domain may only be assigned to unbound devices. The motivation was not made explicit in the changelog other than "in the general case that can cause problems and also [...] we can simplify code quite a bit if we can always assume that". Rafael J. Wysocki elaborated in a mailing list conversation that "setting a PM domain generally changes the set of PM callbacks for the device and it may not be safe to call it after the driver has been bound". The concern seems to be that if a device is put to sleep and its PM callbacks are changed, the device may end up in an undefined state or not resume at all. The real underlying requirement is thus to ensure that the device is awake and execution of its PM callbacks is prevented while the PM domain is assigned. Unbound devices happen to fulfill this requirement, but bound devices can be made to satisfy it as well: The caller can prevent execution of PM ops with lock_system_sleep() and by holding a runtime PM reference to the device. Accordingly, adjust dev_pm_domain_set() to WARN only if the device is in the midst of a system sleep transition, or runtime PM is enabled and the device is either not active or may become inactive imminently (because it has no active children or its refcount is zero). The change is required to support runtime PM for Thunderbolt on the Mac, which poses the unique issue that a child device (the NHI) needs to assign a PM domain to its grandparent (the upstream bridge). Because the grandparent's driver is built-in and the child's driver is a module, the grandparent is usually already bound when the child probes, resulting in a WARN splat when calling dev_pm_domain_set(). However the PM core guarantees both that the grandparent is active and that system sleep is not commenced until the child has finished probing. So in this case it is safe to call dev_pm_domain_set() from the child's ->probe hook and the WARN splat is entirely gratuitous. Note that commit e79aee49bcf9 ("PM: Avoid false-positive warnings in dev_pm_domain_set()") modified the WARN to not apply if a PM domain is removed. This is unsafe as it allows removal of the PM domain while the device is asleep. The present commit rectifies this. Cc: Ulf Hansson Cc: Tomeu Vizoso Cc: Rafael J. Wysocki Signed-off-by: Lukas Wunner --- drivers/base/power/common.c | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/drivers/base/power/common.c b/drivers/base/power/common.c index f6a9ad52cbbf..d02c1e08a7ed 100644 --- a/drivers/base/power/common.c +++ b/drivers/base/power/common.c @@ -13,6 +13,7 @@ #include #include #include +#include #include "power.h" @@ -136,8 +137,10 @@ EXPORT_SYMBOL_GPL(dev_pm_domain_detach); * @dev: Device whose PM domain is to be set. * @pd: PM domain to be set, or NULL. * - * Sets the PM domain the device belongs to. The PM domain of a device needs - * to be set before its probe finishes (it's bound to a driver). + * Sets the PM domain the device belongs to. The PM domain of a device needs + * to be set while the device is awake. This is guaranteed during ->probe. + * Otherwise the caller is responsible for ensuring wakefulness, e.g. by + * holding a runtime PM reference as well as invoking lock_system_sleep(). * * This function must be called with the device lock held. */ @@ -146,8 +149,12 @@ void dev_pm_domain_set(struct device *dev, struct dev_pm_domain *pd) if (dev->pm_domain == pd) return; - WARN(pd && device_is_bound(dev), - "PM domains can only be changed for unbound devices\n"); + WARN(dev->power.is_prepared || dev->power.is_suspended || + (pm_runtime_enabled(dev) && + (dev->power.runtime_status != RPM_ACTIVE || + (pm_children_suspended(dev) && + !atomic_read(&dev->power.usage_count)))), + "PM domains can only be changed for awake devices\n"); dev->pm_domain = pd; device_pm_check_callbacks(dev); }