From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751526AbdJRWEJ (ORCPT ); Wed, 18 Oct 2017 18:04:09 -0400 Received: from cloudserver094114.home.net.pl ([79.96.170.134]:47748 "EHLO cloudserver094114.home.net.pl" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750960AbdJRWEH (ORCPT ); Wed, 18 Oct 2017 18:04:07 -0400 From: "Rafael J. Wysocki" To: Ulf Hansson Cc: Linux PM , Bjorn Helgaas , Alan Stern , Greg Kroah-Hartman , LKML , Linux ACPI , Linux PCI , Linux Documentation , Mika Westerberg , Andy Shevchenko , Kevin Hilman , Wolfram Sang , "linux-i2c@vger.kernel.org" , Lee Jones Subject: Re: [PATCH 0/12] PM / sleep: Driver flags for system suspend/resume Date: Wed, 18 Oct 2017 23:54:28 +0200 Message-ID: <20665978.JlZ4Hgb5rF@aspire.rjw.lan> In-Reply-To: References: <3806130.B2KCK0tvef@aspire.rjw.lan> <2745759.OYb9FSKnNE@aspire.rjw.lan> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wednesday, October 18, 2017 2:34:10 PM CEST Ulf Hansson wrote: > [...] > > >> Are there any major reasons why the appended patch (obviously untested) won't > >> work, then? > > > > OK, there is a reason, which is the optimizations bundled into > > pm_runtime_force_*, because (a) the device may be left in runtime suspend > > by them (in which case amba_pm_suspend_early() in my patch should not run) > > and (b) pm_runtime_force_resume() may decide to leave it suspended (in which > > case amba_pm_suspend_late() in my patch should not run). > > Exactly. > > > > > [BTW, the "leave the device suspended" optimization in pm_runtime_force_* > > is potentially problematic too, because it requires the children to do > > the right thing, which effectively means that their drivers need to use > > pm_runtime_force_* too, but what if they don't want to reuse their > > runtime PM callbacks for system-wide PM?] > > Deployment of pm_runtime_force_suspend() should generally be done for > children devices first. > > If some reason that isn't the case, it's expected that the call to > pm_runtime_set_suspended() invoked from pm_runtime_force_suspend(), > for the parent, should fail and thus abort system suspend. Well, generally what about drivers that need to do something significantly different for system suspend and runtime PM? The whole picture seems to be falling apart if one of these is involved. > > > > Honestly, I don't like the way this is designed. IMO, it would be better > > to do the optimizations and all in the bus type middle-layer code instead > > of expecting drivers to use pm_runtime_force_* as their system-wide PM > > callbacks (and that expectation should at least be documented, which I'm > > not sure is the case now). But whatever. > > > > It all should work the way it does now without pm_runtime_force_* if (a) the > > bus type's PM callbacks are changed like in the last patch and the drivers > > (b) point their system suspend callbacks to the runtime PM callback routines > > and (c) set DPM_FLAG_SMART_SUSPEND and DPM_FLAG_LEAVE_SUSPENDED for the > > devices (if they need to do the PM in ->suspend and ->resume, they may set > > DPM_FLAG_AVOID_RPM too). > > > > And if you see a reason why that won't work, please let me know. > > I will have look and try out the series by using my local "runtime PM > test driver". > > I get back to you with an update on this. OK, thanks!