[{"id":1759465,"web_url":"http://patchwork.ozlabs.org/comment/1759465/","msgid":"<2855684.NY3Ly9PzrE@aspire.rjw.lan>","list_archive_url":null,"date":"2017-08-29T15:27:55","subject":"Re: [PATCH v3 6/8] PM / ACPI: Enable the runtime PM centric\n\tapproach for system sleep","submitter":{"id":26536,"url":"http://patchwork.ozlabs.org/api/people/26536/","name":"Rafael J. Wysocki","email":"rjw@rjwysocki.net"},"content":"On Tuesday, August 29, 2017 4:56:48 PM CEST Ulf Hansson wrote:\n> This change enables the ACPI PM domain to cope with drivers that deploys\n> the runtime PM centric path for system sleep.\n\n[cut]\n\n> @@ -1052,11 +1066,20 @@ EXPORT_SYMBOL_GPL(acpi_subsys_complete);\n> * @dev: Device to handle.\n> *\n> * Follow PCI and resume devices suspended at run time before running their\n> - * system suspend callbacks.\n> + * system suspend callbacks. However, try to avoid it in case the runtime PM\n> + * centric path is used for the device and then trust the driver to do the\n> + * right thing.\n> */\n> int acpi_subsys_suspend(struct device *dev)\n> {\n> -\tpm_runtime_resume(dev);\n> +\tstruct acpi_device *adev = ACPI_COMPANION(dev);\n> +\n> +\tif (!adev)\n> +\t\treturn 0;\n> +\n> +\tif (!dev_pm_is_rpm_sleep(dev) || acpi_dev_needs_resume(dev, adev))\n> +\t\tpm_runtime_resume(dev);\n> +\n> \treturn pm_generic_suspend(dev);\n> }\n> EXPORT_SYMBOL_GPL(acpi_subsys_suspend);\n\nWell, I tried to avoid calling acpi_dev_needs_resume() for multiple times\nand that's why I added the update_state thing.\n\nMoreover, the is_rpm_sleep flag here has to mean not only that\ndirect_complete should not be used with the device, but also that its driver\nis fine with not resuming it.\n\nIMO it is not a good idea to use one flag for these two different things at the\nsame time at all.\n\nThanks,\nRafael","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=linux-i2c-owner@vger.kernel.org;\n\treceiver=)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xhXml0lMzz9t33\n\tfor ;\n\tWed, 30 Aug 2017 01:36:39 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751717AbdH2Pgh (ORCPT );\n\tTue, 29 Aug 2017 11:36:37 -0400","from cloudserver094114.home.net.pl ([79.96.170.134]:42624 \"EHLO\n\tcloudserver094114.home.net.pl\" rhost-flags-OK-OK-OK-OK)\n\tby vger.kernel.org with ESMTP id S1751299AbdH2Pgg (ORCPT\n\t); Tue, 29 Aug 2017 11:36:36 -0400","from 79.184.253.199.ipv4.supernova.orange.pl (79.184.253.199)\n\t(HELO aspire.rjw.lan)\n\tby serwer1319399.home.pl (79.96.170.134) with SMTP (IdeaSmtpServer\n\t0.82) id 944c6b487de22f45; Tue, 29 Aug 2017 17:36:35 +0200"],"From":"\"Rafael J. Wysocki\" ","To":"Ulf Hansson ","Cc":"Wolfram Sang , Len Brown ,\n\tlinux-acpi@vger.kernel.org, linux-pm@vger.kernel.org,\n\tKevin Hilman ,\n\tJarkko Nikula ,\n\tAndy Shevchenko ,\n\tMika Westerberg ,\n\tJisheng Zhang ,\n\tJohn Stultz , Guodong Xu ,\n\tSumit Semwal ,\n\tHaojian Zhuang ,\n\tJohannes Stezenbach ,\n\tlinux-arm-kernel@lists.infradead.org, linux-i2c@vger.kernel.org","Subject":"Re: [PATCH v3 6/8] PM / ACPI: Enable the runtime PM centric\n\tapproach for system sleep","Date":"Tue, 29 Aug 2017 17:27:55 +0200","Message-ID":"<2855684.NY3Ly9PzrE@aspire.rjw.lan>","In-Reply-To":"<1504018610-10822-7-git-send-email-ulf.hansson@linaro.org>","References":"<1504018610-10822-1-git-send-email-ulf.hansson@linaro.org>\n\t<1504018610-10822-7-git-send-email-ulf.hansson@linaro.org>","MIME-Version":"1.0","Content-Transfer-Encoding":"7Bit","Content-Type":"text/plain; charset=\"us-ascii\"","Sender":"linux-i2c-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"linux-i2c@vger.kernel.org"}},{"id":1761515,"web_url":"http://patchwork.ozlabs.org/comment/1761515/","msgid":"","list_archive_url":null,"date":"2017-09-01T08:27:05","subject":"Re: [PATCH v3 6/8] PM / ACPI: Enable the runtime PM centric approach\n\tfor system sleep","submitter":{"id":21036,"url":"http://patchwork.ozlabs.org/api/people/21036/","name":"Ulf Hansson","email":"ulf.hansson@linaro.org"},"content":"On 29 August 2017 at 17:27, Rafael J. Wysocki wrote:\n> On Tuesday, August 29, 2017 4:56:48 PM CEST Ulf Hansson wrote:\n>> This change enables the ACPI PM domain to cope with drivers that deploys\n>> the runtime PM centric path for system sleep.\n>\n> [cut]\n>\n>> @@ -1052,11 +1066,20 @@ EXPORT_SYMBOL_GPL(acpi_subsys_complete);\n>> * @dev: Device to handle.\n>> *\n>> * Follow PCI and resume devices suspended at run time before running their\n>> - * system suspend callbacks.\n>> + * system suspend callbacks. However, try to avoid it in case the runtime PM\n>> + * centric path is used for the device and then trust the driver to do the\n>> + * right thing.\n>> */\n>> int acpi_subsys_suspend(struct device *dev)\n>> {\n>> - pm_runtime_resume(dev);\n>> + struct acpi_device *adev = ACPI_COMPANION(dev);\n>> +\n>> + if (!adev)\n>> + return 0;\n>> +\n>> + if (!dev_pm_is_rpm_sleep(dev) || acpi_dev_needs_resume(dev, adev))\n>> + pm_runtime_resume(dev);\n>> +\n>> return pm_generic_suspend(dev);\n>> }\n>> EXPORT_SYMBOL_GPL(acpi_subsys_suspend);\n>\n> Well, I tried to avoid calling acpi_dev_needs_resume() for multiple times\n> and that's why I added the update_state thing.\n>\n> Moreover, the is_rpm_sleep flag here has to mean not only that\n> direct_complete should not be used with the device, but also that its driver\n> is fine with not resuming it.\n\nLet me try to explain this better. I realize the changelog is\nmisleading around this particular section! Huh, apologize for that!\n\nFirst, patch1 makes the PM core treat the is_rpm_sleep flag as the\ndirect_complete isn't allowed for the device.\n\nFor that reason, when the is_rpm_sleep is set, there is no point\ncalling acpi_dev_needs_resume() from acpi_subsys_prepare(), but\ninstead that can be deferred to acpi_subsys_suspend() - because it\ndoesn't matter if acpi_subsys_prepare() returns 0 or 1, in either case\nthe acpi_subsys_suspend() will be called. That's really what goes on\nhere.\n\nThe end result is the same. If the acpi_dev_needs_resume() thinks that\nthe device needs to be runtime resumed, pm_runtime_resume() is called\nfor the device in acpi_subsys_suspend().\n\nSo, this has nothing to do with whether the driver \"is fine with not\nresuming it\" thing.\n\n>\n> IMO it is not a good idea to use one flag for these two different things at the\n> same time at all.\n\nYeah, I guess my upper comment addresses your immediate concern here?\n\nHowever, there is one other thing the is_rpm_flag means. That is that\nthe driver has informed the ACPI PM domain, to trust the driver to\ndeal with system sleep, via re-using the runtime PM callbacks.\nSo the flag does still have two meanings, but that we can change - of course.\n\n>\n> Thanks,\n> Rafael\n>\n\nKind regards\nUffe","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=linux-i2c-owner@vger.kernel.org;\n\treceiver=)","ozlabs.org; dkim=pass (1024-bit key;\n\tunprotected) header.d=linaro.org header.i=@linaro.org\n\theader.b=\"LhEn1DvD\"; dkim-atps=neutral"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xkC5q3QQKz9t1t\n\tfor ;\n\tFri, 1 Sep 2017 18:27:11 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1751676AbdIAI1J (ORCPT );\n\tFri, 1 Sep 2017 04:27:09 -0400","from mail-io0-f176.google.com ([209.85.223.176]:33935 \"EHLO\n\tmail-io0-f176.google.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1751667AbdIAI1H (ORCPT\n\t); Fri, 1 Sep 2017 04:27:07 -0400","by mail-io0-f176.google.com with SMTP id i200so2392088ioa.1\n\tfor ; Fri, 01 Sep 2017 01:27:06 -0700 (PDT)","by 10.2.96.38 with HTTP; Fri, 1 Sep 2017 01:27:05 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google;\n\th=mime-version:in-reply-to:references:from:date:message-id:subject:to\n\t:cc; bh=5ixv3x7gqEIQVDxuc1U5Pyyn0lgxF/p6HRlCDu6LsiU=;\n\tb=LhEn1DvDO4xLeWbajOMmxCf7AX2qT/s6En+BhcQS03NfjFeVQ1DR/C/80Qby00TQB7\n\tZNLICgFvAEGn1r+yxmSLv1mdM2eGGjo579eHcEx2r2oZ9ohsJ3UBXpF+AZQa1plQiHa9\n\txuL+F7qORz/C0tJSBfMQ0YJz/ZEpA0JUxiwvk=","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:in-reply-to:references:from:date\n\t:message-id:subject:to:cc;\n\tbh=5ixv3x7gqEIQVDxuc1U5Pyyn0lgxF/p6HRlCDu6LsiU=;\n\tb=qE8qOSP2B4+NsbTvywOA5EV8Uibq0uxRvFgb2x/4mJ1pah7gHKrIO6oeF1PQXr4F0W\n\ttYSPWr0hDkEPqWJkHsI5wBamTWnMSfTwl8ktq9dCS85CzVR3reyNS7eeATRQ3AkqiL7o\n\tlbSvM3SU/INJ8Lthjmcr2MIlV0WBqGTKvnBFngeq1gsC2lXUIEwh0FMKjKlRRpwKsMq3\n\tkTDBKYNplaflKMNOVNIRNvQT6jllq5gn4b1d6q5kxfKYKiIEUfvsoBzA8RmJ7K7/LKF3\n\tG5Qf9l0jO8iBbVPeYAPk3gfrYIL2inNi/YGNjyrx5nITkNit637zx4cX93uvS6yt3dKM\n\tzrHQ==","X-Gm-Message-State":"AHPjjUh/On9xcTourU2FrneTM2MdXKNHzS36cBKqR66TYuglynwbinl9\n\thGnXrolC+rc25Bg/KVzOOma6vjUwgFZ2","X-Google-Smtp-Source":"ADKCNb4NSgGmoy41O0tiW2uN7wuXzUMi7/3YAhc5FvNLuSh4hd/ivZbz2K+g3PXU967tBHm6XzCALhAHapjednk37ag=","X-Received":"by 10.107.15.10 with SMTP id x10mr970302ioi.45.1504254426423;\n\tFri, 01 Sep 2017 01:27:06 -0700 (PDT)","MIME-Version":"1.0","In-Reply-To":"<2855684.NY3Ly9PzrE@aspire.rjw.lan>","References":"<1504018610-10822-1-git-send-email-ulf.hansson@linaro.org>\n\t<1504018610-10822-7-git-send-email-ulf.hansson@linaro.org>\n\t<2855684.NY3Ly9PzrE@aspire.rjw.lan>","From":"Ulf Hansson ","Date":"Fri, 1 Sep 2017 10:27:05 +0200","Message-ID":"","Subject":"Re: [PATCH v3 6/8] PM / ACPI: Enable the runtime PM centric approach\n\tfor system sleep","To":"\"Rafael J. Wysocki\" ","Cc":"Wolfram Sang , Len Brown ,\n\tACPI Devel Maling List ,\n\t\"linux-pm@vger.kernel.org\" ,\n\tKevin Hilman ,\n\tJarkko Nikula ,\n\tAndy Shevchenko ,\n\tMika Westerberg ,\n\tJisheng Zhang ,\n\tJohn Stultz , Guodong Xu ,\n\tSumit Semwal ,\n\tHaojian Zhuang ,\n\tJohannes Stezenbach ,\n\t\"linux-arm-kernel@lists.infradead.org\" \n\t,\n\t\"linux-i2c@vger.kernel.org\" ","Content-Type":"text/plain; charset=\"UTF-8\"","Sender":"linux-i2c-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"linux-i2c@vger.kernel.org"}},{"id":1762142,"web_url":"http://patchwork.ozlabs.org/comment/1762142/","msgid":"<2209426.5Z3u1iKN4i@aspire.rjw.lan>","list_archive_url":null,"date":"2017-09-02T15:38:04","subject":"Re: [PATCH v3 6/8] PM / ACPI: Enable the runtime PM centric\n\tapproach for system sleep","submitter":{"id":26536,"url":"http://patchwork.ozlabs.org/api/people/26536/","name":"Rafael J. Wysocki","email":"rjw@rjwysocki.net"},"content":"On Friday, September 1, 2017 10:27:05 AM CEST Ulf Hansson wrote:\n> On 29 August 2017 at 17:27, Rafael J. Wysocki wrote:\n> > On Tuesday, August 29, 2017 4:56:48 PM CEST Ulf Hansson wrote:\n> >> This change enables the ACPI PM domain to cope with drivers that deploys\n> >> the runtime PM centric path for system sleep.\n> >\n> > [cut]\n> >\n> >> @@ -1052,11 +1066,20 @@ EXPORT_SYMBOL_GPL(acpi_subsys_complete);\n> >> * @dev: Device to handle.\n> >> *\n> >> * Follow PCI and resume devices suspended at run time before running their\n> >> - * system suspend callbacks.\n> >> + * system suspend callbacks. However, try to avoid it in case the runtime PM\n> >> + * centric path is used for the device and then trust the driver to do the\n> >> + * right thing.\n> >> */\n> >> int acpi_subsys_suspend(struct device *dev)\n> >> {\n> >> - pm_runtime_resume(dev);\n> >> + struct acpi_device *adev = ACPI_COMPANION(dev);\n> >> +\n> >> + if (!adev)\n> >> + return 0;\n> >> +\n> >> + if (!dev_pm_is_rpm_sleep(dev) || acpi_dev_needs_resume(dev, adev))\n> >> + pm_runtime_resume(dev);\n> >> +\n> >> return pm_generic_suspend(dev);\n> >> }\n> >> EXPORT_SYMBOL_GPL(acpi_subsys_suspend);\n> >\n> > Well, I tried to avoid calling acpi_dev_needs_resume() for multiple times\n> > and that's why I added the update_state thing.\n> >\n> > Moreover, the is_rpm_sleep flag here has to mean not only that\n> > direct_complete should not be used with the device, but also that its driver\n> > is fine with not resuming it.\n> \n> Let me try to explain this better. I realize the changelog is\n> misleading around this particular section! Huh, apologize for that!\n> \n> First, patch1 makes the PM core treat the is_rpm_sleep flag as the\n> direct_complete isn't allowed for the device.\n> \n> For that reason, when the is_rpm_sleep is set, there is no point\n> calling acpi_dev_needs_resume() from acpi_subsys_prepare(), but\n> instead that can be deferred to acpi_subsys_suspend() - because it\n> doesn't matter if acpi_subsys_prepare() returns 0 or 1, in either case\n> the acpi_subsys_suspend() will be called. That's really what goes on\n> here.\n> \n> The end result is the same. If the acpi_dev_needs_resume() thinks that\n> the device needs to be runtime resumed, pm_runtime_resume() is called\n> for the device in acpi_subsys_suspend().\n> \n> So, this has nothing to do with whether the driver \"is fine with not\n> resuming it\" thing.\n\nNo, sorry.\n\nIf is_rpm_sleep was not set, the ACPI PM domain would resume the device in\nacpi_subsys_suspend() regardless of the acpi_dev_needs_resume() return value.\nThat's what's there in the patch. So clearly, setting is_rpm_sleep means\n\"this device does not need to be resumed in acpi_subsys_suspend() unless\nacpi_dev_needs_resume() returns true\". Which clearly means that the driver\n*is* fine with not resuming it, because if is_rpm_sleep is set, the device\nin fact may not be resumed and then the driver will need to cope with that.\n\nAnd note that this meaning of is_rpm_sleep is different from what it is\nexpected to mean to the core.\n\n> >\n> > IMO it is not a good idea to use one flag for these two different things at the\n> > same time at all.\n> \n> Yeah, I guess my upper comment addresses your immediate concern here?\n\nNo, they don't.\n\n> However, there is one other thing the is_rpm_flag means. That is that\n> the driver has informed the ACPI PM domain, to trust the driver to\n> deal with system sleep, via re-using the runtime PM callbacks.\n> So the flag does still have two meanings, but that we can change - of course.\n\nI guess that you are referring to the use of dev_pm_is_rpm_sleep() in\nacpi_subsys_suspend_late()? That's the third thing this flag means ...\n\nThanks,\nRafael","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=linux-i2c-owner@vger.kernel.org;\n\treceiver=)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xl0ph6flGz9sQl\n\tfor ;\n\tSun, 3 Sep 2017 01:46:52 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1752690AbdIBPqu (ORCPT );\n\tSat, 2 Sep 2017 11:46:50 -0400","from cloudserver094114.home.net.pl ([79.96.170.134]:50300 \"EHLO\n\tcloudserver094114.home.net.pl\" rhost-flags-OK-OK-OK-OK)\n\tby vger.kernel.org with ESMTP id S1752684AbdIBPqu (ORCPT\n\t); Sat, 2 Sep 2017 11:46:50 -0400","from 79.184.253.199.ipv4.supernova.orange.pl (79.184.253.199)\n\t(HELO aspire.rjw.lan)\n\tby serwer1319399.home.pl (79.96.170.134) with SMTP (IdeaSmtpServer\n\t0.82) id e2a649d0fff84b71; Sat, 2 Sep 2017 17:46:48 +0200"],"From":"\"Rafael J. Wysocki\" ","To":"Ulf Hansson ","Cc":"Wolfram Sang , Len Brown ,\n\tACPI Devel Maling List ,\n\t\"linux-pm@vger.kernel.org\" ,\n\tKevin Hilman ,\n\tJarkko Nikula ,\n\tAndy Shevchenko ,\n\tMika Westerberg ,\n\tJisheng Zhang ,\n\tJohn Stultz , Guodong Xu ,\n\tSumit Semwal ,\n\tHaojian Zhuang ,\n\tJohannes Stezenbach ,\n\t\"linux-arm-kernel@lists.infradead.org\" \n\t,\n\t\"linux-i2c@vger.kernel.org\" ","Subject":"Re: [PATCH v3 6/8] PM / ACPI: Enable the runtime PM centric\n\tapproach for system sleep","Date":"Sat, 02 Sep 2017 17:38:04 +0200","Message-ID":"<2209426.5Z3u1iKN4i@aspire.rjw.lan>","In-Reply-To":"","References":"<1504018610-10822-1-git-send-email-ulf.hansson@linaro.org>\n\t<2855684.NY3Ly9PzrE@aspire.rjw.lan>\n\t","MIME-Version":"1.0","Content-Transfer-Encoding":"7Bit","Content-Type":"text/plain; charset=\"us-ascii\"","Sender":"linux-i2c-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"linux-i2c@vger.kernel.org"}},{"id":1762631,"web_url":"http://patchwork.ozlabs.org/comment/1762631/","msgid":"","list_archive_url":null,"date":"2017-09-04T13:21:15","subject":"Re: [PATCH v3 6/8] PM / ACPI: Enable the runtime PM centric approach\n\tfor system sleep","submitter":{"id":21036,"url":"http://patchwork.ozlabs.org/api/people/21036/","name":"Ulf Hansson","email":"ulf.hansson@linaro.org"},"content":"On 2 September 2017 at 17:38, Rafael J. Wysocki wrote:\n> On Friday, September 1, 2017 10:27:05 AM CEST Ulf Hansson wrote:\n>> On 29 August 2017 at 17:27, Rafael J. Wysocki wrote:\n>> > On Tuesday, August 29, 2017 4:56:48 PM CEST Ulf Hansson wrote:\n>> >> This change enables the ACPI PM domain to cope with drivers that deploys\n>> >> the runtime PM centric path for system sleep.\n>> >\n>> > [cut]\n>> >\n>> >> @@ -1052,11 +1066,20 @@ EXPORT_SYMBOL_GPL(acpi_subsys_complete);\n>> >> * @dev: Device to handle.\n>> >> *\n>> >> * Follow PCI and resume devices suspended at run time before running their\n>> >> - * system suspend callbacks.\n>> >> + * system suspend callbacks. However, try to avoid it in case the runtime PM\n>> >> + * centric path is used for the device and then trust the driver to do the\n>> >> + * right thing.\n>> >> */\n>> >> int acpi_subsys_suspend(struct device *dev)\n>> >> {\n>> >> - pm_runtime_resume(dev);\n>> >> + struct acpi_device *adev = ACPI_COMPANION(dev);\n>> >> +\n>> >> + if (!adev)\n>> >> + return 0;\n>> >> +\n>> >> + if (!dev_pm_is_rpm_sleep(dev) || acpi_dev_needs_resume(dev, adev))\n>> >> + pm_runtime_resume(dev);\n>> >> +\n>> >> return pm_generic_suspend(dev);\n>> >> }\n>> >> EXPORT_SYMBOL_GPL(acpi_subsys_suspend);\n>> >\n>> > Well, I tried to avoid calling acpi_dev_needs_resume() for multiple times\n>> > and that's why I added the update_state thing.\n>> >\n>> > Moreover, the is_rpm_sleep flag here has to mean not only that\n>> > direct_complete should not be used with the device, but also that its driver\n>> > is fine with not resuming it.\n>>\n>> Let me try to explain this better. I realize the changelog is\n>> misleading around this particular section! Huh, apologize for that!\n>>\n>> First, patch1 makes the PM core treat the is_rpm_sleep flag as the\n>> direct_complete isn't allowed for the device.\n>>\n>> For that reason, when the is_rpm_sleep is set, there is no point\n>> calling acpi_dev_needs_resume() from acpi_subsys_prepare(), but\n>> instead that can be deferred to acpi_subsys_suspend() - because it\n>> doesn't matter if acpi_subsys_prepare() returns 0 or 1, in either case\n>> the acpi_subsys_suspend() will be called. That's really what goes on\n>> here.\n>>\n>> The end result is the same. If the acpi_dev_needs_resume() thinks that\n>> the device needs to be runtime resumed, pm_runtime_resume() is called\n>> for the device in acpi_subsys_suspend().\n>>\n>> So, this has nothing to do with whether the driver \"is fine with not\n>> resuming it\" thing.\n>\n> No, sorry.\n>\n> If is_rpm_sleep was not set, the ACPI PM domain would resume the device in\n> acpi_subsys_suspend() regardless of the acpi_dev_needs_resume() return value.\n\nYes, I believe I forgot about one scenario, when the direct_complete\npath has been abandoned by the PM core, because a child device was\nsuspend before and it couldn't run the direct_complete path for it?\n\nJust to be sure, that's the case you also had in mind?\n\n> That's what's there in the patch. So clearly, setting is_rpm_sleep means\n> \"this device does not need to be resumed in acpi_subsys_suspend() unless\n> acpi_dev_needs_resume() returns true\". Which clearly means that the driver\n> *is* fine with not resuming it, because if is_rpm_sleep is set, the device\n> in fact may not be resumed and then the driver will need to cope with that.\n\nYes, I understand your concern, because we may break the default\nbehavior of the ACPI PM domain.\n\nSo, *if* there will be a next version, I will make sure to be better\nsafe than sorry, and add one flag per use case.\n\n>\n> And note that this meaning of is_rpm_sleep is different from what it is\n> expected to mean to the core.\n>\n>> >\n>> > IMO it is not a good idea to use one flag for these two different things at the\n>> > same time at all.\n>>\n>> Yeah, I guess my upper comment addresses your immediate concern here?\n>\n> No, they don't.\n>\n>> However, there is one other thing the is_rpm_flag means. That is that\n>> the driver has informed the ACPI PM domain, to trust the driver to\n>> deal with system sleep, via re-using the runtime PM callbacks.\n>> So the flag does still have two meanings, but that we can change - of course.\n>\n> I guess that you are referring to the use of dev_pm_is_rpm_sleep() in\n> acpi_subsys_suspend_late()? That's the third thing this flag means ...\n\nYes.\n\nKind regards\nUffe","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=linux-i2c-owner@vger.kernel.org;\n\treceiver=)","ozlabs.org; dkim=pass (1024-bit key;\n\tunprotected) header.d=linaro.org header.i=@linaro.org\n\theader.b=\"C9sX9awi\"; dkim-atps=neutral"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xm9Tr3NcFz9sR9\n\tfor ;\n\tMon, 4 Sep 2017 23:21:20 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1753616AbdIDNVS (ORCPT );\n\tMon, 4 Sep 2017 09:21:18 -0400","from mail-io0-f181.google.com ([209.85.223.181]:37620 \"EHLO\n\tmail-io0-f181.google.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1753515AbdIDNVR (ORCPT\n\t); Mon, 4 Sep 2017 09:21:17 -0400","by mail-io0-f181.google.com with SMTP id 62so1386591iok.4\n\tfor ; Mon, 04 Sep 2017 06:21:17 -0700 (PDT)","by 10.2.96.38 with HTTP; Mon, 4 Sep 2017 06:21:15 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google;\n\th=mime-version:in-reply-to:references:from:date:message-id:subject:to\n\t:cc; bh=bzQvUPQY0HRl5qQZDEazRF6l6srRl4eS0XAryhYBKbc=;\n\tb=C9sX9awi72n8VJGnyIExPun7dBoq9LX9YqyQ8eWpRyE8w6WNLpFX2KEPS9vnjr/uhB\n\tQ7Og01RAxuiKjUXfS3WI2gggJaplaZFipMBD7oCrY+4KbTqIyo0RRgxiOiq1n9ghNSVa\n\tCNdrh0CQzh5+QV8fY2QaNHE0dttjxsStATeb0=","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:in-reply-to:references:from:date\n\t:message-id:subject:to:cc;\n\tbh=bzQvUPQY0HRl5qQZDEazRF6l6srRl4eS0XAryhYBKbc=;\n\tb=qtdH8WSlvNf3WvNRUou2cHBqOYduoZXQ/Ml4+J+0sgYqMzGTuk39OnpFa3SRkUS9N4\n\t7zmh0zpia8dJ8ST9Z//yXyyZfR1ZG0ecefEDNadeb8GhEoSqwFH3nciIbHnaz1QvzxVl\n\tl8ln0cQO5Y7fAgaAL1KfY6KUoTvhK9dzF6U+03CCHEFNuWByoND3gCcPC6wltr4dQgTD\n\tlermQMH7/KZZt1073CGC8qdU2He0b/nk0frBF3WCd/7seDzd3vsRR9e760OhQP4OQcQi\n\tQazLvDQ/mJVrb61KXv/vVxJ+NHkvws+GYzkALubxh4eir1I4D+VsJ8KCyMpkknvVO3Gd\n\tssFA==","X-Gm-Message-State":"AHPjjUgJAQO9D17dUgojUzzkIH5GnGbBIKsoHZlgWZYVOuQjOxpPpREo\n\tHwc58f97YB4qMNsI8panqcwp5A0VbO5U","X-Google-Smtp-Source":"ADKCNb6vf3eoJgYszW8/UUqkUXRFa5aZZNX023es8RmCUDRmiB9PzUOrw6oWkTGbJgx7JaeG90f/R1Ej8F9xKNHaOV8=","X-Received":"by 10.107.26.198 with SMTP id a189mr573809ioa.223.1504531276439; \n\tMon, 04 Sep 2017 06:21:16 -0700 (PDT)","MIME-Version":"1.0","In-Reply-To":"<2209426.5Z3u1iKN4i@aspire.rjw.lan>","References":"<1504018610-10822-1-git-send-email-ulf.hansson@linaro.org>\n\t<2855684.NY3Ly9PzrE@aspire.rjw.lan>\n\t\n\t<2209426.5Z3u1iKN4i@aspire.rjw.lan>","From":"Ulf Hansson ","Date":"Mon, 4 Sep 2017 15:21:15 +0200","Message-ID":"","Subject":"Re: [PATCH v3 6/8] PM / ACPI: Enable the runtime PM centric approach\n\tfor system sleep","To":"\"Rafael J. Wysocki\" ","Cc":"Wolfram Sang , Len Brown ,\n\tACPI Devel Maling List ,\n\t\"linux-pm@vger.kernel.org\" ,\n\tKevin Hilman ,\n\tJarkko Nikula ,\n\tAndy Shevchenko ,\n\tMika Westerberg ,\n\tJisheng Zhang ,\n\tJohn Stultz , Guodong Xu ,\n\tSumit Semwal ,\n\tHaojian Zhuang ,\n\tJohannes Stezenbach ,\n\t\"linux-arm-kernel@lists.infradead.org\" \n\t,\n\t\"linux-i2c@vger.kernel.org\" ","Content-Type":"text/plain; charset=\"UTF-8\"","Sender":"linux-i2c-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"linux-i2c@vger.kernel.org"}},{"id":1763732,"web_url":"http://patchwork.ozlabs.org/comment/1763732/","msgid":"<28965959.lSjY1gHxkl@aspire.rjw.lan>","list_archive_url":null,"date":"2017-09-06T00:02:41","subject":"Re: [PATCH v3 6/8] PM / ACPI: Enable the runtime PM centric\n\tapproach for system sleep","submitter":{"id":26536,"url":"http://patchwork.ozlabs.org/api/people/26536/","name":"Rafael J. Wysocki","email":"rjw@rjwysocki.net"},"content":"On Monday, September 4, 2017 3:21:15 PM CEST Ulf Hansson wrote:\n> On 2 September 2017 at 17:38, Rafael J. Wysocki wrote:\n> > On Friday, September 1, 2017 10:27:05 AM CEST Ulf Hansson wrote:\n> >> On 29 August 2017 at 17:27, Rafael J. Wysocki wrote:\n> >> > On Tuesday, August 29, 2017 4:56:48 PM CEST Ulf Hansson wrote:\n> >> >> This change enables the ACPI PM domain to cope with drivers that deploys\n> >> >> the runtime PM centric path for system sleep.\n> >> >\n> >> > [cut]\n> >> >\n> >> >> @@ -1052,11 +1066,20 @@ EXPORT_SYMBOL_GPL(acpi_subsys_complete);\n> >> >> * @dev: Device to handle.\n> >> >> *\n> >> >> * Follow PCI and resume devices suspended at run time before running their\n> >> >> - * system suspend callbacks.\n> >> >> + * system suspend callbacks. However, try to avoid it in case the runtime PM\n> >> >> + * centric path is used for the device and then trust the driver to do the\n> >> >> + * right thing.\n> >> >> */\n> >> >> int acpi_subsys_suspend(struct device *dev)\n> >> >> {\n> >> >> - pm_runtime_resume(dev);\n> >> >> + struct acpi_device *adev = ACPI_COMPANION(dev);\n> >> >> +\n> >> >> + if (!adev)\n> >> >> + return 0;\n> >> >> +\n> >> >> + if (!dev_pm_is_rpm_sleep(dev) || acpi_dev_needs_resume(dev, adev))\n> >> >> + pm_runtime_resume(dev);\n> >> >> +\n> >> >> return pm_generic_suspend(dev);\n> >> >> }\n> >> >> EXPORT_SYMBOL_GPL(acpi_subsys_suspend);\n> >> >\n> >> > Well, I tried to avoid calling acpi_dev_needs_resume() for multiple times\n> >> > and that's why I added the update_state thing.\n> >> >\n> >> > Moreover, the is_rpm_sleep flag here has to mean not only that\n> >> > direct_complete should not be used with the device, but also that its driver\n> >> > is fine with not resuming it.\n> >>\n> >> Let me try to explain this better. I realize the changelog is\n> >> misleading around this particular section! Huh, apologize for that!\n> >>\n> >> First, patch1 makes the PM core treat the is_rpm_sleep flag as the\n> >> direct_complete isn't allowed for the device.\n> >>\n> >> For that reason, when the is_rpm_sleep is set, there is no point\n> >> calling acpi_dev_needs_resume() from acpi_subsys_prepare(), but\n> >> instead that can be deferred to acpi_subsys_suspend() - because it\n> >> doesn't matter if acpi_subsys_prepare() returns 0 or 1, in either case\n> >> the acpi_subsys_suspend() will be called. That's really what goes on\n> >> here.\n> >>\n> >> The end result is the same. If the acpi_dev_needs_resume() thinks that\n> >> the device needs to be runtime resumed, pm_runtime_resume() is called\n> >> for the device in acpi_subsys_suspend().\n> >>\n> >> So, this has nothing to do with whether the driver \"is fine with not\n> >> resuming it\" thing.\n> >\n> > No, sorry.\n> >\n> > If is_rpm_sleep was not set, the ACPI PM domain would resume the device in\n> > acpi_subsys_suspend() regardless of the acpi_dev_needs_resume() return value.\n> \n> Yes, I believe I forgot about one scenario, when the direct_complete\n> path has been abandoned by the PM core, because a child device was\n> suspend before and it couldn't run the direct_complete path for it?\n> \n> Just to be sure, that's the case you also had in mind?\n\nYes.\n\nThanks,\nRafael","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=vger.kernel.org\n\t(client-ip=209.132.180.67; helo=vger.kernel.org;\n\tenvelope-from=linux-i2c-owner@vger.kernel.org;\n\treceiver=)","Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xn3sf0z6Bz9sPs\n\tfor ;\n\tWed, 6 Sep 2017 10:11:34 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1753811AbdIFALc (ORCPT );\n\tTue, 5 Sep 2017 20:11:32 -0400","from cloudserver094114.home.net.pl ([79.96.170.134]:58241 \"EHLO\n\tcloudserver094114.home.net.pl\" rhost-flags-OK-OK-OK-OK)\n\tby vger.kernel.org with ESMTP id S1753614AbdIFALb (ORCPT\n\t); Tue, 5 Sep 2017 20:11:31 -0400","from 79.184.253.199.ipv4.supernova.orange.pl (79.184.253.199)\n\t(HELO aspire.rjw.lan)\n\tby serwer1319399.home.pl (79.96.170.134) with SMTP (IdeaSmtpServer\n\t0.82) id 8c472b82d51be5ca; Wed, 6 Sep 2017 02:11:29 +0200"],"From":"\"Rafael J. Wysocki\" ","To":"Ulf Hansson ","Cc":"Wolfram Sang , Len Brown ,\n\tACPI Devel Maling List ,\n\t\"linux-pm@vger.kernel.org\" ,\n\tKevin Hilman ,\n\tJarkko Nikula ,\n\tAndy Shevchenko ,\n\tMika Westerberg ,\n\tJisheng Zhang ,\n\tJohn Stultz , Guodong Xu ,\n\tSumit Semwal ,\n\tHaojian Zhuang ,\n\tJohannes Stezenbach ,\n\t\"linux-arm-kernel@lists.infradead.org\" \n\t,\n\t\"linux-i2c@vger.kernel.org\" ","Subject":"Re: [PATCH v3 6/8] PM / ACPI: Enable the runtime PM centric\n\tapproach for system sleep","Date":"Wed, 06 Sep 2017 02:02:41 +0200","Message-ID":"<28965959.lSjY1gHxkl@aspire.rjw.lan>","In-Reply-To":"","References":"<1504018610-10822-1-git-send-email-ulf.hansson@linaro.org>\n\t<2209426.5Z3u1iKN4i@aspire.rjw.lan>\n\t","MIME-Version":"1.0","Content-Transfer-Encoding":"7Bit","Content-Type":"text/plain; charset=\"us-ascii\"","Sender":"linux-i2c-owner@vger.kernel.org","Precedence":"bulk","List-ID":"","X-Mailing-List":"linux-i2c@vger.kernel.org"}}]