From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Rafael J. Wysocki" Subject: Re: [PATCH 1/3] PM / core: Add SAFE_SUSPEND driver flag Date: Tue, 29 Aug 2017 17:02:53 +0200 Message-ID: <2539179.02z2N9fVNH@aspire.rjw.lan> References: <1503499329-28834-1-git-send-email-ulf.hansson@linaro.org> <35841101.LqGbCjJMGH@aspire.rjw.lan> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7Bit Return-path: Received: from cloudserver094114.home.net.pl ([79.96.170.134]:63808 "EHLO cloudserver094114.home.net.pl" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751464AbdH2PLe (ORCPT ); Tue, 29 Aug 2017 11:11:34 -0400 In-Reply-To: Sender: linux-acpi-owner@vger.kernel.org List-Id: linux-acpi@vger.kernel.org To: Ulf Hansson Cc: Wolfram Sang , Len Brown , ACPI Devel Maling List , "linux-pm@vger.kernel.org" , Kevin Hilman , Jarkko Nikula , Andy Shevchenko , Mika Westerberg , Jisheng Zhang , John Stultz , Guodong Xu , Sumit Semwal , Haojian Zhuang , "linux-arm-kernel@lists.infradead.org" , "linux-i2c@vger.kernel.org" , Greg Kroah-Hartman On Tuesday, August 29, 2017 4:57:28 PM CEST Ulf Hansson wrote: > On 29 August 2017 at 02:20, Rafael J. Wysocki wrote: > > From: Rafael J. Wysocki > > > > Add a driver_flags field to struct dev_pm_info for flags that can be > > set by device drivers at the probe time to inform the PM core and/or > > bus types, PM domains and so on on the capabilities and/or > > preferences of device drivers. It is anticipated that more than one > > flag of this kind will be necessary going forward. > > > > Define and document a SAFE_SUSPEND flag to instruct bus types and PM > > domains that the system suspend callbacks provided by the driver can > > cope with runtime suspended devices, so from the driver's perspective > > it should be safe to leave devices in runtime suspend during system > > suspend. > > This changelog is a bit too vague to me. Wouldn't it be more clear if > also adding something along the lines that this also means that > runtime resuming a device isn't needed by the subsystem/PM domain > during system sleep? No. > Because ideally that is what you want to avoid, right? Not really. The driver doesn't know what the needs of the higher level are. It may only say what it can do and the bus type can use this information to make a decision. > Moreover I am also not convinced that this solution really is the > right path. It seems like we might end up adding more bits for the > "driver_flag" field and it gets complicated. Do we really need to > distinguish between all different cases in such detail? Yes, we do. Every time we try to address two different problems with one mechanism, it backfires later. > I will continue to review this tomorrow, however in the meantime I > have finalized a re-spin of my v3 series so I decided to post it > anyway. I am adding only one new flag to the PM core, perhaps I am > over-simplifying things, but please have look once more. I think I > have addressed all your concerns you have raised so far. I'll have a look, but I really don't want to conflate the "I'm fine with not resuming the device" case with the "I don't want to use direct_complete with it" one. To me, they are fundamentally different and I'm not going to apply any patches conflating them. Thanks, Rafael From mboxrd@z Thu Jan 1 00:00:00 1970 From: rjw@rjwysocki.net (Rafael J. Wysocki) Date: Tue, 29 Aug 2017 17:02:53 +0200 Subject: [PATCH 1/3] PM / core: Add SAFE_SUSPEND driver flag In-Reply-To: References: <1503499329-28834-1-git-send-email-ulf.hansson@linaro.org> <35841101.LqGbCjJMGH@aspire.rjw.lan> Message-ID: <2539179.02z2N9fVNH@aspire.rjw.lan> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On Tuesday, August 29, 2017 4:57:28 PM CEST Ulf Hansson wrote: > On 29 August 2017 at 02:20, Rafael J. Wysocki wrote: > > From: Rafael J. Wysocki > > > > Add a driver_flags field to struct dev_pm_info for flags that can be > > set by device drivers at the probe time to inform the PM core and/or > > bus types, PM domains and so on on the capabilities and/or > > preferences of device drivers. It is anticipated that more than one > > flag of this kind will be necessary going forward. > > > > Define and document a SAFE_SUSPEND flag to instruct bus types and PM > > domains that the system suspend callbacks provided by the driver can > > cope with runtime suspended devices, so from the driver's perspective > > it should be safe to leave devices in runtime suspend during system > > suspend. > > This changelog is a bit too vague to me. Wouldn't it be more clear if > also adding something along the lines that this also means that > runtime resuming a device isn't needed by the subsystem/PM domain > during system sleep? No. > Because ideally that is what you want to avoid, right? Not really. The driver doesn't know what the needs of the higher level are. It may only say what it can do and the bus type can use this information to make a decision. > Moreover I am also not convinced that this solution really is the > right path. It seems like we might end up adding more bits for the > "driver_flag" field and it gets complicated. Do we really need to > distinguish between all different cases in such detail? Yes, we do. Every time we try to address two different problems with one mechanism, it backfires later. > I will continue to review this tomorrow, however in the meantime I > have finalized a re-spin of my v3 series so I decided to post it > anyway. I am adding only one new flag to the PM core, perhaps I am > over-simplifying things, but please have look once more. I think I > have addressed all your concerns you have raised so far. I'll have a look, but I really don't want to conflate the "I'm fine with not resuming the device" case with the "I don't want to use direct_complete with it" one. To me, they are fundamentally different and I'm not going to apply any patches conflating them. Thanks, Rafael