From patchwork Fri Mar 20 00:43:36 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Rafael J. Wysocki" X-Patchwork-Id: 6053531 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.29.136]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 213729F2A9 for ; Fri, 20 Mar 2015 00:24:25 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 1EA2C20519 for ; Fri, 20 Mar 2015 00:24:24 +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 29920204FC for ; Fri, 20 Mar 2015 00:24:23 +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 1YYkff-0001kQ-5b; Fri, 20 Mar 2015 00:20:15 +0000 Received: from v094114.home.net.pl ([79.96.170.134]) by bombadil.infradead.org with smtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1YYkfa-0000XX-Dx for linux-arm-kernel@lists.infradead.org; Fri, 20 Mar 2015 00:20:11 +0000 Received: from afem64.neoplus.adsl.tpnet.pl (95.49.116.64) (HELO vostro.rjw.lan) by serwer1319399.home.pl (79.96.170.134) with SMTP (IdeaSmtpServer v0.80) id cf66d92180daca35; Fri, 20 Mar 2015 01:19:40 +0100 From: "Rafael J. Wysocki" To: Dmitry Torokhov Subject: Re: [PATCH v2] driver core / PM: Add PM domain callbacks for device setup/cleanup Date: Fri, 20 Mar 2015 01:43:36 +0100 Message-ID: <2669979.KjLdG5zoJ9@vostro.rjw.lan> User-Agent: KMail/4.11.5 (Linux/3.19.0+; KDE/4.11.5; x86_64; ; ) In-Reply-To: References: <3533423.QQDErEmeTO@vostro.rjw.lan> <1810625.BTgaNclHRk@vostro.rjw.lan> MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20150319_172010_663906_AEA9435C X-CRM114-Status: GOOD ( 31.59 ) X-Spam-Score: -0.0 (/) Cc: Len Brown , Ulf Hansson , Kevin Hilman , Russell King , Geert Uytterhoeven , "linux-pm@vger.kernel.org" , Greg Kroah-Hartman , Wolfram Sang , Mark Brown , Alan Stern , Pavel Machek , "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-Spam-Status: No, score=-4.2 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_MED, T_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 Thursday, March 19, 2015 03:42:03 PM Dmitry Torokhov wrote: > On Thu, Mar 19, 2015 at 2:51 PM, Rafael J. Wysocki wrote: > > From: Rafael J. Wysocki > > Subject: driver core / PM: Add PM domain callbacks for device setup/cleanup > > > > If PM domains are in use, it may be necessary to prepare the code > > handling a PM domain for driver probing. For example, in some > > cases device drivers rely on the ability to power on the devices > > with the help of the IO runtime PM framework and the PM domain > > code needs to be ready for that. Also, if that code has not been > > fully initialized yet, the driver probing should be deferred. > > > > Moreover, after the probing is complete, it may be necessary to > > put the PM domain in question into the state reflecting the current > > needs of the devices in it, for example, so that power is not drawn > > in vain. The same should be done after removing a driver from > > a device, as the PM domain state may need to be changed to reflect > > the new situation. > > > > For these reasons, introduce new PM domain callbacks, ->activate > > and ->sync, called, respectively, before probing for a device > > driver and after the probing has been completed or the driver > > has been removed. > > > > Signed-off-by: Rafael J. Wysocki > > --- > >> --- > >> > >> This is an update without the additional bus type callbacks. This should > >> be suffucient at this point for the use cases we have. > >> > >> I've decided to also call ->sync on driver removal as that is consistent > >> with what happens for failing probe. > >> > >> --- > > > > One more update. > > > > I've realized that in the PM domain's ->sync callback is called after > > clearing the device's driver field (in the failed probe or driver removal > > case) rather than before it, that could be used to distinguish between the > > two cases (successful probe vs no driver), so I reworked the patch accordingly. > > That is very non-obvious. dev->drv belongs to driver core and maybe > bus, nothing else should really be looking at it. Why can't we add an > explicit argument to the callback indicating device state? Well, if that's a concern, I'd rather introduce a separate callback to be run on driver removal/failing probe, like in the patch below, as it is then clear what the conditions for each callback's execution are (and one can always define them as wrappers around the same routine if need be). I still think that this ordering of code is the right one, though. Rafael --- drivers/base/dd.c | 16 ++++++++++++++++ include/linux/pm.h | 8 ++++++++ 2 files changed, 24 insertions(+) Index: linux-pm/include/linux/pm.h =================================================================== --- linux-pm.orig/include/linux/pm.h +++ linux-pm/include/linux/pm.h @@ -603,10 +603,18 @@ extern void dev_pm_put_subsys_data(struc * Power domains provide callbacks that are executed during system suspend, * hibernation, system resume and during runtime PM transitions along with * subsystem-level and driver-level callbacks. + * + * @detach: Called when removing a device from the domain. + * @activate: Called before executing probe routines for bus types and drivers. + * @sync: Called after successful driver probe. + * @dismiss: Called after unsuccessful driver probe and after driver removal. */ struct dev_pm_domain { struct dev_pm_ops ops; void (*detach)(struct device *dev, bool power_off); + int (*activate)(struct device *dev); + void (*sync)(struct device *dev); + void (*dismiss)(struct device *dev); }; /* Index: linux-pm/drivers/base/dd.c =================================================================== --- linux-pm.orig/drivers/base/dd.c +++ linux-pm/drivers/base/dd.c @@ -279,6 +279,7 @@ static int really_probe(struct device *d { int ret = 0; int local_trigger_count = atomic_read(&deferred_trigger_count); + struct dev_pm_domain *pm_domain = dev->pm_domain; atomic_inc(&probe_count); pr_debug("bus: '%s': %s: probing driver %s with device %s\n", @@ -298,6 +299,12 @@ static int really_probe(struct device *d goto probe_failed; } + if (pm_domain && pm_domain->activate) { + ret = pm_domain->activate(dev); + if (ret) + goto probe_failed; + } + if (dev->bus->probe) { ret = dev->bus->probe(dev); if (ret) @@ -308,6 +315,9 @@ static int really_probe(struct device *d goto probe_failed; } + if (pm_domain && pm_domain->sync) + pm_domain->sync(dev); + driver_bound(dev); ret = 1; pr_debug("bus: '%s': %s: bound device %s to driver %s\n", @@ -319,6 +329,8 @@ probe_failed: driver_sysfs_remove(dev); dev->driver = NULL; dev_set_drvdata(dev, NULL); + if (pm_domain && pm_domain->dismiss) + pm_domain->dismiss(dev); if (ret == -EPROBE_DEFER) { /* Driver requested deferred probing */ @@ -522,9 +534,13 @@ static void __device_release_driver(stru dev->bus->remove(dev); else if (drv->remove) drv->remove(dev); + devres_release_all(dev); dev->driver = NULL; dev_set_drvdata(dev, NULL); + if (dev->pm_domain && dev->pm_domain->dismiss) + dev->pm_domain->dismiss(dev); + klist_remove(&dev->p->knode_driver); if (dev->bus) blocking_notifier_call_chain(&dev->bus->p->bus_notifier,