From patchwork Wed Nov 18 10:48:39 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ulf Hansson X-Patchwork-Id: 7647931 X-Patchwork-Delegate: rjw@sisk.pl Return-Path: X-Original-To: patchwork-linux-pm@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork2.web.kernel.org (Postfix) with ESMTP id EFF96BF90C for ; Wed, 18 Nov 2015 10:49:02 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id DA23820603 for ; Wed, 18 Nov 2015 10:49:01 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id B0F272060B for ; Wed, 18 Nov 2015 10:49:00 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753455AbbKRKs7 (ORCPT ); Wed, 18 Nov 2015 05:48:59 -0500 Received: from mail-lf0-f53.google.com ([209.85.215.53]:34553 "EHLO mail-lf0-f53.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751349AbbKRKs6 (ORCPT ); Wed, 18 Nov 2015 05:48:58 -0500 Received: by lffu14 with SMTP id u14so23620789lff.1 for ; Wed, 18 Nov 2015 02:48:56 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro-org.20150623.gappssmtp.com; s=20150623; h=from:to:cc:subject:date:message-id; bh=FAwekRxrkgi62xz+l1sGMWev1f07gY4pndnjOvjY8fo=; b=PAVz8r9W1fkqdImKZtirmf+jNR/L/A9Q8tDM/QVVR+KEGuf+P3ayXOFrAn3brPX0cw UFWv1Kd+DuNT5c77jbEYyT4y+7dJ9XieanqbXbQMMvx4G30Ho8A87FohQusx5bBhHA57 Z4+vVPNwe6sX4cdTmdQNrY62/zfBJ5eYsqB9uj9+3KmvWKB9LBrX0iidBUOGi6By93L6 QzfZYOzu4OadCjP4p2Xjr63H1bnzX69jmw0PNvy+7RXPpzEJm/XGE4Pi09D7jIAW4Os8 LW0J5KOQXxfp7AQuI++VADOcBIuVZSmByj69RLWXrT1mQgq+6yb1jdLjwVl3U+4GOXZe dc3A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:cc:subject:date:message-id; bh=FAwekRxrkgi62xz+l1sGMWev1f07gY4pndnjOvjY8fo=; b=ZJA83YdsMrXMwZJTFZ/6TGDLwuc/MMlHoCCxBOQMOISmzjcR9wybReBc4pW4TevXpw fMLI4WKgBb88uBLg8Ii+B+1Z2J4wFizQ/G3rTEJWRpKr3XHEEsFfKiYC6PZodh0F6BYy yvDy5ezsVBcyDCJNuDPjfOkIf57T+rCZpYX6naAjln6M5DBgssEVymz3uLYsVFy4KjEQ 4QsTZrzc4JLFPZNfF1id1NE6i9PO2ofO1GkurDGIUoalb4nVhcdTMftY5Hm3qnAbxHrL K27Eo9LEWEs4DE1aAyzi4UCE9TqHYiyjtxHkU4HRLpftajkfOSqQB3ZswKRK2Y7gnspn C01w== X-Gm-Message-State: ALoCoQmZZcJbM/HGzKbBoRMSBnAVOhGCKTdqGYz/XhWyISvJWd/Alfh991l8ByhcmGIcGU8CQSjJ X-Received: by 10.25.142.84 with SMTP id q81mr303158lfd.77.1447843736409; Wed, 18 Nov 2015 02:48:56 -0800 (PST) Received: from uffe-Latitude-E6430s.ideon.se ([85.235.10.227]) by smtp.gmail.com with ESMTPSA id 194sm343471lfd.4.2015.11.18.02.48.55 (version=TLSv1/SSLv3 cipher=OTHER); Wed, 18 Nov 2015 02:48:55 -0800 (PST) From: Ulf Hansson To: "Rafael J. Wysocki" , Greg Kroah-Hartman , Alan Stern , linux-pm@vger.kernel.org Cc: Kevin Hilman , Len Brown , Pavel Machek , Geert Uytterhoeven , Kuninori Morimoto , Eduardo Valentin , Ulf Hansson Subject: [PATCH v2] Driver core: Re-init runtime PM states at probe error and driver unbind Date: Wed, 18 Nov 2015 11:48:39 +0100 Message-Id: <1447843719-9501-1-git-send-email-ulf.hansson@linaro.org> X-Mailer: git-send-email 1.9.1 Sender: linux-pm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-pm@vger.kernel.org X-Spam-Status: No, score=-7.3 required=5.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_HI,RP_MATCHES_RCVD,T_DKIM_INVALID,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 There are two common expectations among several subsystems/drivers that deploys runtime PM support, but which isn't met by the driver core. Expectation 1) At ->probe() the subsystem/driver expects the runtime PM status of the device to be RPM_SUSPENDED, which is the initial status being assigned at device registration. This expectation is especially common among some of those subsystems/ drivers that manages devices with an attached PM domain, as those requires the ->runtime_resume() callback at the PM domain level to be invoked during ->probe(). Moreover these subsystems/drivers entirely relies on runtime PM resources being managed at the PM domain level, thus don't implement their own set of runtime PM callbacks. These are two scenarios that suffers from this unmet expectation. i) A failed ->probe() sequence requests probe deferral: ->probe() ... pm_runtime_enable() pm_runtime_get_sync() ... err: pm_runtime_put() pm_runtime_disable() ... As there are no guarantees that such sequence turns the runtime PM status of the device into RPM_SUSPENDED, the re-trying ->probe() may start with the status in RPM_ACTIVE. In such case the runtime PM core won't invoke the ->runtime_resume() callback because of a pm_runtime_get_sync(), as it considers the device to be already runtime resumed. ii) A driver re-bind sequence: At driver unbind, the subsystem/driver's >remove() callback invokes a sequence of runtime PM APIs, to undo actions during ->probe() and to put the device into low power state. ->remove() ... pm_runtime_put() pm_runtime_disable() ... Similar as in the failing ->probe() case, this sequence don't guarantee the runtime PM status of the device to turn into RPM_SUSPENDED. Trying to re-bind the driver thus causes the same issue as when re-trying ->probe(), in the probe deferral scenario. Expectation 2) Drivers that invokes the pm_runtime_irq_safe() API during ->probe(), triggers the runtime PM core to increase the usage count for the device's parent and permanently make it runtime resumed. The usage count is only dropped at device removal, which also allows it to be runtime suspended again. A re-trying ->probe() repeats the call to pm_runtime_irq_safe() and thus once more triggers the usage count of the device's parent to be increased. This leads to not only an imbalance issue of the usage count of the device's parent, but also to keep it runtime resumed permanently even if ->probe() fails. To address these issues, let's change the policy of the driver core to meet these expectations. More precisely, at ->probe() failures and driver unbind, restore the initial states of runtime PM. Although to still allow subsystem's to control PM for devices that doesn't ->probe() successfully, don't restore the initial states unless runtime PM is disabled. Signed-off-by: Ulf Hansson Reviewed-by: Kevin Hilman --- Changes in v2: - Folded patches into one patch, according to Rafael's request. - Add pm_runtime_reinit() and updated code from review by Alan. - Updated commit message header. --- drivers/base/dd.c | 2 ++ drivers/base/power/power.h | 2 ++ drivers/base/power/runtime.c | 26 ++++++++++++++++++++------ 3 files changed, 24 insertions(+), 6 deletions(-) diff --git a/drivers/base/dd.c b/drivers/base/dd.c index a641cf3..cd2d79b 100644 --- a/drivers/base/dd.c +++ b/drivers/base/dd.c @@ -340,6 +340,7 @@ probe_failed: dev_set_drvdata(dev, NULL); if (dev->pm_domain && dev->pm_domain->dismiss) dev->pm_domain->dismiss(dev); + pm_runtime_reinit(dev); switch (ret) { case -EPROBE_DEFER: @@ -695,6 +696,7 @@ static void __device_release_driver(struct device *dev) dev_set_drvdata(dev, NULL); if (dev->pm_domain && dev->pm_domain->dismiss) dev->pm_domain->dismiss(dev); + pm_runtime_reinit(dev); klist_remove(&dev->p->knode_driver); if (dev->bus) diff --git a/drivers/base/power/power.h b/drivers/base/power/power.h index 998fa6b..8b06193 100644 --- a/drivers/base/power/power.h +++ b/drivers/base/power/power.h @@ -18,6 +18,7 @@ static inline void pm_runtime_early_init(struct device *dev) } extern void pm_runtime_init(struct device *dev); +extern void pm_runtime_reinit(struct device *dev); extern void pm_runtime_remove(struct device *dev); struct wake_irq { @@ -84,6 +85,7 @@ static inline void pm_runtime_early_init(struct device *dev) } static inline void pm_runtime_init(struct device *dev) {} +static inline void pm_runtime_reinit(struct device *dev) {} static inline void pm_runtime_remove(struct device *dev) {} static inline int dpm_sysfs_add(struct device *dev) { return 0; } diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c index e1a10a0..ab3fcd9 100644 --- a/drivers/base/power/runtime.c +++ b/drivers/base/power/runtime.c @@ -1390,18 +1390,32 @@ void pm_runtime_init(struct device *dev) } /** + * pm_runtime_reinit - Re-initialize runtime PM fields in given device object. + * @dev: Device object to re-initialize. + */ +void pm_runtime_reinit(struct device *dev) +{ + if (!pm_runtime_enabled(dev)) { + if (dev->power.runtime_status == RPM_ACTIVE) + pm_runtime_set_suspended(dev); + if (dev->power.irq_safe) { + spin_lock_irq(&dev->power.lock); + dev->power.irq_safe = 0; + spin_unlock_irq(&dev->power.lock); + if (dev->parent) + pm_runtime_put(dev->parent); + } + } +} + +/** * pm_runtime_remove - Prepare for removing a device from device hierarchy. * @dev: Device object being removed from device hierarchy. */ void pm_runtime_remove(struct device *dev) { __pm_runtime_disable(dev, false); - - /* Change the status back to 'suspended' to match the initial status. */ - if (dev->power.runtime_status == RPM_ACTIVE) - pm_runtime_set_suspended(dev); - if (dev->power.irq_safe && dev->parent) - pm_runtime_put(dev->parent); + pm_runtime_reinit(dev); } /**