Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752052AbdIUAje (ORCPT ); Wed, 20 Sep 2017 20:39:34 -0400 Received: from mail-io0-f173.google.com ([209.85.223.173]:48105 "EHLO mail-io0-f173.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751652AbdIUAjc (ORCPT ); Wed, 20 Sep 2017 20:39:32 -0400 X-Google-Smtp-Source: AOwi7QA0TVZvcpeCEep8wqe6YIH6eyZEKqVeyp9Lta0ivQnJpuyMo08Bc3dwi3zzgAscbWCsEbDpiFeVOTN/ZSeGbao= MIME-Version: 1.0 In-Reply-To: <20170920162731.wopbh7iwggu2xouk@sig21.net> References: <1849600.vI30ccvhCD@aspire.rjw.lan> <20170920162731.wopbh7iwggu2xouk@sig21.net> From: "Rafael J. Wysocki" Date: Thu, 21 Sep 2017 02:39:30 +0200 X-Google-Sender-Auth: a5h1wVNtF6vQk6-64es8awvFtHw Message-ID: Subject: Re: [PATCH] PM: Document rules on using pm_runtime_resume() in system suspend callbacks To: Johannes Stezenbach Cc: "Rafael J. Wysocki" , Ulf Hansson , "Rafael J. Wysocki" , Linux PM , Alan Stern , Mika Westerberg , Andy Shevchenko , Kevin Hilman , LKML Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4178 Lines: 88 On Wed, Sep 20, 2017 at 6:27 PM, Johannes Stezenbach wrote: > On Wed, Sep 20, 2017 at 04:01:32PM +0200, Rafael J. Wysocki wrote: >> On Wed, Sep 20, 2017 at 2:28 PM, Ulf Hansson wrote: >> > On 20 September 2017 at 02:26, Rafael J. Wysocki wrote: >> >> >> >> Second, leaving devices in runtime suspend in the "suspend" phase of system >> >> suspend is fishy even when their runtime PM is disabled, because that doesn't >> >> guarantee anything regarding their children or possible consumers. Runtime >> >> PM may still be enabled for those devices at that time and runtime resume may >> >> be triggered for them later, in which case it all quickly falls apart. >> > >> > This is true, although to me this is a about a different problem and >> > has very little to do with pm_runtime_force_suspend(). >> > >> > More precisely, whether runtime PM becomes disabled in the suspend >> > phase or suspend_late phase, really doesn't matter. Because in the end >> > this is about suspending/resuming devices in the correct order. >> >> Yes, it is, but this is not my point (I didn't make it clear enough I guess). >> >> At the time you make the decision to disable runtime PM for a parent >> (say) and leave it in runtime suspend, all of its children are >> suspended just fine (otherwise the parent wouldn't have been suspended >> too). However, you *also* need to make sure that there will be no >> attempts to resume any of them *after* that point, which practically >> means that either runtime PM has to have been disabled already for all >> of them at the time it is disabled for the parent, or there has to be >> another guarantee in place. >> >> That's why the core tries to enforce the "runtime PM disabled for the >> entire hierarchy below" guarantee for the devices with direct_complete >> set, but that may just be overkill in many cases. I guess it may be >> better to use WARN_ON() to catch the cases in which things may really >> go wrong. > > I read this half a dozen times and I'm still confused. > Moreover, Documentation/driver-api/pm/devices.rst says: > > Runtime Power Management model: > > Devices may also be put into low-power states while the system is > running, independently of other power management activity in principle. > However, devices are not generally independent of each other (for > example, a parent device cannot be suspended unless all of its child > devices have been suspended). ... > > However, isn't this a fundamental difference of runtime suspend > vs. system suspend that parent devices *can* be runtime suspended > before their children? No, it isn't. > E.g. an audio codec could keep running > while the i2c bus used to program its registers can be runtime suspended. > If this is correct I think it would be useful to spell it out explicitly > in the documentation. That's because the i2c bus uses the ignore_children flag that allows it to override the general rules. :-) > During system suspend, pm core will suspend children first, > and if the child's ->suspend hook uses the i2c bus to access registers, > it will implicitly runtime resume the i2c bus (e.g. due to pm_runtime_get_sync() > in i2c_dw_xfer()). Later pm core will ->suspend the i2c bus. Yup. > I have a hunch the root of the problem is that ->prepare walks the tree > in top-down order, and its return value is used to decide about > direct-complete. Why does it do that? Shouldn't pm core check > the direct_complete flag during ->suspend if the device > is in runtime suspend, to decide whether to skip runtime resume + ->suspend > for *this* device? direct_complete has nothing to do with this. First off, the PM core does check the direct_complete flag in __device_suspend() and does more-or-less what you are saying. However, that flag is initialized in device_prepare() with the help of the ->suspend() return value, because whether or not it makes sense to set that flag depends on some conditions that may change between consecutive system suspend-resume cycles in general and need to be checked in advance before setting it. HTH Rafael