[{"id":1759417,"web_url":"http://patchwork.ozlabs.org/comment/1759417/","msgid":"","list_archive_url":null,"date":"2017-08-29T14:57:28","subject":"Re: [PATCH 1/3] PM / core: Add SAFE_SUSPEND driver flag","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 02:20, Rafael J. Wysocki wrote:\n> From: Rafael J. Wysocki \n>\n> Add a driver_flags field to struct dev_pm_info for flags that can be\n> set by device drivers at the probe time to inform the PM core and/or\n> bus types, PM domains and so on on the capabilities and/or\n> preferences of device drivers. It is anticipated that more than one\n> flag of this kind will be necessary going forward.\n>\n> Define and document a SAFE_SUSPEND flag to instruct bus types and PM\n> domains that the system suspend callbacks provided by the driver can\n> cope with runtime suspended devices, so from the driver's perspective\n> it should be safe to leave devices in runtime suspend during system\n> suspend.\n\nThis changelog is a bit too vague to me. Wouldn't it be more clear if\nalso adding something along the lines that this also means that\nruntime resuming a device isn't needed by the subsystem/PM domain\nduring system sleep? Because ideally that is what you want to avoid,\nright?\n\nMoreover I am also not convinced that this solution really is the\nright path. It seems like we might end up adding more bits for the\n\"driver_flag\" field and it gets complicated. Do we really need to\ndistinguish between all different cases in such detail?\n\nI will continue to review this tomorrow, however in the meantime I\nhave finalized a re-spin of my v3 series so I decided to post it\nanyway. I am adding only one new flag to the PM core, perhaps I am\nover-simplifying things, but please have look once more. I think I\nhave addressed all your concerns you have raised so far.\n\nKind regards\nUffe\n\n>\n> Signed-off-by: Rafael J. Wysocki \n> ---\n> Documentation/driver-api/pm/devices.rst | 7 +++++++\n> drivers/base/dd.c | 2 ++\n> include/linux/pm.h | 16 ++++++++++++++++\n> 3 files changed, 25 insertions(+)\n>\n> Index: linux-pm/include/linux/pm.h\n> ===================================================================\n> --- linux-pm.orig/include/linux/pm.h\n> +++ linux-pm/include/linux/pm.h\n> @@ -550,6 +550,21 @@ struct pm_subsys_data {\n> #endif\n> };\n>\n> +/*\n> + * Driver flags to control system suspend/resume behavior.\n> + *\n> + * These flags can be set by device drivers at the probe time. They need not be\n> + * cleared by the drivers as the driver core will take care of that.\n> + *\n> + * SAFE_SUSPEND: No need to runtime resume the device during system suspend.\n> + *\n> + * Setting SAFE_SUSPEND instructs bus types and PM domains which may want to\n> + * runtime resume the device upfront during system suspend that doing so is not\n> + * necessary from the driver's perspective, because the system suspend callbacks\n> + * provided by it can cope with a runtime suspended device.\n> + */\n> +#define DPM_FLAG_SAFE_SUSPEND BIT(0)\n> +\n> struct dev_pm_info {\n> pm_message_t power_state;\n> unsigned int can_wakeup:1;\n> @@ -561,6 +576,7 @@ struct dev_pm_info {\n> bool is_late_suspended:1;\n> bool early_init:1; /* Owned by the PM core */\n> bool direct_complete:1; /* Owned by the PM core */\n> + unsigned int driver_flags;\n> spinlock_t lock;\n> #ifdef CONFIG_PM_SLEEP\n> struct list_head entry;\n> Index: linux-pm/drivers/base/dd.c\n> ===================================================================\n> --- linux-pm.orig/drivers/base/dd.c\n> +++ linux-pm/drivers/base/dd.c\n> @@ -436,6 +436,7 @@ pinctrl_bind_failed:\n> if (dev->pm_domain && dev->pm_domain->dismiss)\n> dev->pm_domain->dismiss(dev);\n> pm_runtime_reinit(dev);\n> + dev->power.driver_flags = 0;\n>\n> switch (ret) {\n> case -EPROBE_DEFER:\n> @@ -841,6 +842,7 @@ static void __device_release_driver(stru\n> if (dev->pm_domain && dev->pm_domain->dismiss)\n> dev->pm_domain->dismiss(dev);\n> pm_runtime_reinit(dev);\n> + dev->power.driver_flags = 0;\n>\n> klist_remove(&dev->p->knode_driver);\n> device_pm_check_callbacks(dev);\n> Index: linux-pm/Documentation/driver-api/pm/devices.rst\n> ===================================================================\n> --- linux-pm.orig/Documentation/driver-api/pm/devices.rst\n> +++ linux-pm/Documentation/driver-api/pm/devices.rst\n> @@ -729,6 +729,13 @@ state temporarily, for example so that i\n> disabled. This all depends on the hardware and the design of the subsystem and\n> device driver in question.\n>\n> +Some bus types and PM domains have a policy to runtime resume all\n> +devices upfront in their ``->suspend`` callbacks, but that may not be really\n> +necessary if the system suspend-resume callbacks provided by the device's\n> +driver can cope with a runtime-suspended device. The driver can indicate that\n> +by setting ``DPM_FLAG_SAFE_SUSPEND`` in :c:member:`power.driver_flags` at the\n> +probe time.\n> +\n> During system-wide resume from a sleep state it's easiest to put devices into\n> the full-power state, as explained in :file:`Documentation/power/runtime_pm.txt`.\n> Refer to that document for more information regarding this particular issue as\n>\n>","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=\"UpMfiWGO\"; dkim-atps=neutral"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3xhWvg5Fxnz9sR9\n\tfor ;\n\tWed, 30 Aug 2017 00:57:35 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1754683AbdH2O5c (ORCPT );\n\tTue, 29 Aug 2017 10:57:32 -0400","from mail-io0-f178.google.com ([209.85.223.178]:38211 \"EHLO\n\tmail-io0-f178.google.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1754676AbdH2O53 (ORCPT\n\t); Tue, 29 Aug 2017 10:57:29 -0400","by mail-io0-f178.google.com with SMTP id 81so20891995ioj.5\n\tfor ; Tue, 29 Aug 2017 07:57:29 -0700 (PDT)","by 10.2.96.38 with HTTP; Tue, 29 Aug 2017 07:57:28 -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=1aklyd1w72SMChA/x8H1p2q5KewjKhBdQ/kLlWtzrD0=;\n\tb=UpMfiWGOubJbR03icOMlkqvDTb8aVhESjg9jKz93Y9fS96ia5RLeMjSA2+C/td9pzQ\n\ttOSVuudBGKm+Vusk130eomFviNVYktVdl52t8lmfcJgGL2BplDNLsOmjU5cTVyvCfw+Q\n\t6otMieoowstOVs729bDMLpFXI3KBFVCtWIPiI=","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=1aklyd1w72SMChA/x8H1p2q5KewjKhBdQ/kLlWtzrD0=;\n\tb=mHADpn679j2WCGFDAm+74n1j1iv8NuDosO/zw9olfp2wgd28FzMwWONXLDmZDzr24J\n\t8yBFxFB/5iykMLAoM2vbXC0diLInkPKNWSNalQQu9ScxMTtDp6xac91sxxXSXp03k65y\n\t2e8gy6KGwENx+NeszbYodoxAMzIwpwV7rcabYD7xpLOX/cBQWCVu4fo/UZHmWXb1ojN8\n\t54q1ocN3alL/qgzRadBpBqZ8oBgr5iDNw/DWJfglyJY+jTDpxDxrsTJEB9oAMRt5Vy0Y\n\tvminO5ocLbN7cQ8qoxnEargIrdKNdWl+zGV83SQ2CpFABAi/r/LmdQ0PqHlYuvD+vZ5Z\n\tHlHQ==","X-Gm-Message-State":"AHYfb5h5Wae5KsQgQnsDfI1CIFrHqbBmOP8tlpe4NsUhU4pXZd88CrSK\n\tOpdNXdgrcvWRUsYJ7cLcS8pl2trBYASx","X-Received":"by 10.107.5.2 with SMTP id 2mr4673799iof.34.1504018649059;\n\tTue, 29 Aug 2017 07:57:29 -0700 (PDT)","MIME-Version":"1.0","In-Reply-To":"<35841101.LqGbCjJMGH@aspire.rjw.lan>","References":"<1503499329-28834-1-git-send-email-ulf.hansson@linaro.org>\n\t<4245176.X6JjkhnUAM@aspire.rjw.lan>\n\t<35841101.LqGbCjJMGH@aspire.rjw.lan>","From":"Ulf Hansson ","Date":"Tue, 29 Aug 2017 16:57:28 +0200","Message-ID":"","Subject":"Re: [PATCH 1/3] PM / core: Add SAFE_SUSPEND driver flag","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\t\"linux-arm-kernel@lists.infradead.org\" \n\t,\n\t\"linux-i2c@vger.kernel.org\" ,\n\tGreg Kroah-Hartman ","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":1759437,"web_url":"http://patchwork.ozlabs.org/comment/1759437/","msgid":"<2539179.02z2N9fVNH@aspire.rjw.lan>","list_archive_url":null,"date":"2017-08-29T15:02:53","subject":"Re: [PATCH 1/3] PM / core: Add SAFE_SUSPEND driver flag","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:57:28 PM CEST Ulf Hansson wrote:\n> On 29 August 2017 at 02:20, Rafael J. Wysocki wrote:\n> > From: Rafael J. Wysocki \n> >\n> > Add a driver_flags field to struct dev_pm_info for flags that can be\n> > set by device drivers at the probe time to inform the PM core and/or\n> > bus types, PM domains and so on on the capabilities and/or\n> > preferences of device drivers. It is anticipated that more than one\n> > flag of this kind will be necessary going forward.\n> >\n> > Define and document a SAFE_SUSPEND flag to instruct bus types and PM\n> > domains that the system suspend callbacks provided by the driver can\n> > cope with runtime suspended devices, so from the driver's perspective\n> > it should be safe to leave devices in runtime suspend during system\n> > suspend.\n> \n> This changelog is a bit too vague to me. Wouldn't it be more clear if\n> also adding something along the lines that this also means that\n> runtime resuming a device isn't needed by the subsystem/PM domain\n> during system sleep?\n\nNo.\n\n> Because ideally that is what you want to avoid, right?\n\nNot really. The driver doesn't know what the needs of the higher level\nare. It may only say what it can do and the bus type can use this\ninformation to make a decision.\n\n> Moreover I am also not convinced that this solution really is the\n> right path. It seems like we might end up adding more bits for the\n> \"driver_flag\" field and it gets complicated. Do we really need to\n> distinguish between all different cases in such detail?\n\nYes, we do.\n\nEvery time we try to address two different problems with one mechanism,\nit backfires later.\n\n> I will continue to review this tomorrow, however in the meantime I\n> have finalized a re-spin of my v3 series so I decided to post it\n> anyway. I am adding only one new flag to the PM core, perhaps I am\n> over-simplifying things, but please have look once more. I think I\n> have addressed all your concerns you have raised so far.\n\nI'll have a look, but I really don't want to conflate the \"I'm fine\nwith not resuming the device\" case with the \"I don't want to use\ndirect_complete with it\" one. To me, they are fundamentally different\nand I'm not going to apply any patches conflating them.\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 3xhXCs5mC3z9t2Q\n\tfor ;\n\tWed, 30 Aug 2017 01:11:37 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S1752054AbdH2PLf (ORCPT );\n\tTue, 29 Aug 2017 11:11:35 -0400","from cloudserver094114.home.net.pl ([79.96.170.134]:63808 \"EHLO\n\tcloudserver094114.home.net.pl\" rhost-flags-OK-OK-OK-OK)\n\tby vger.kernel.org with ESMTP id S1751464AbdH2PLe (ORCPT\n\t); Tue, 29 Aug 2017 11:11:34 -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 d648335d251501eb; Tue, 29 Aug 2017 17:11:32 +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\t\"linux-arm-kernel@lists.infradead.org\" \n\t,\n\t\"linux-i2c@vger.kernel.org\" ,\n\tGreg Kroah-Hartman ","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>","In-Reply-To":"","References":"<1503499329-28834-1-git-send-email-ulf.hansson@linaro.org>\n\t<35841101.LqGbCjJMGH@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"}}]