[{"id":1774933,"web_url":"http://patchwork.ozlabs.org/comment/1774933/","msgid":"","list_archive_url":null,"date":"2017-09-25T19:12:28","subject":"Re: [PATCH v4 1/3] PM: i2c-designware-platdrv: Clean up PM handling\n\tin probe","submitter":{"id":21036,"url":"http://patchwork.ozlabs.org/api/people/21036/","name":"Ulf Hansson","email":"ulf.hansson@linaro.org"},"content":"On 25 September 2017 at 01:17, Rafael J. Wysocki wrote:\n> From: Rafael J. Wysocki \n>\n> The power management handling in dw_i2c_plat_probe() is somewhat\n> messy and it is rather hard to figure out the code intention for\n> the case when pm_disabled is set. In that case, the driver doesn't\n> enable runtime PM at all, but in addition to that it calls\n> pm_runtime_forbid() as though it wasn't sure if runtime PM might\n> be enabled for the device later by someone else.\n>\n> Although that concern doesn't seem to be actually valid, the\n> device is clearly still expected to be PM-capable even in the\n> pm_disabled set case, so a better approach would be to enable\n> runtime PM for it unconditionally and then prevent it from\n> being runtime-suspended by using pm_runtime_forbid().\n\nThis is nice cleanup! However I have one suggestion/comment.\n\nUsing pm_runtime_forbid() to prevent the device from being runtime\nsuspended may be a bit fragile, as userspace can then still change to\n\"allow\" it. Wouldn't it be better to bump the runtime PM usage count\n(pm_runtime_get_noresume()) instead?\n\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=\"YwzRn9Sa\"; dkim-atps=neutral"],"Received":["from vger.kernel.org (vger.kernel.org [209.132.180.67])\n\tby ozlabs.org (Postfix) with ESMTP id 3y1DHV3hCVz9t16\n\tfor ;\n\tTue, 26 Sep 2017 05:12:33 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S935861AbdIYTMb (ORCPT );\n\tMon, 25 Sep 2017 15:12:31 -0400","from mail-it0-f46.google.com ([209.85.214.46]:47204 \"EHLO\n\tmail-it0-f46.google.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n\twith ESMTP id S1752586AbdIYTMa (ORCPT\n\t); Mon, 25 Sep 2017 15:12:30 -0400","by mail-it0-f46.google.com with SMTP id 85so287225ith.2\n\tfor ; Mon, 25 Sep 2017 12:12:30 -0700 (PDT)","by 10.2.181.165 with HTTP; Mon, 25 Sep 2017 12:12: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=vCkG6p0QSmJ12O793X+ZHdvTxiDd6oWqbl0aD7PBh44=;\n\tb=YwzRn9SaxU9ihoqu8n0YxQOQzrYtzqDALgFJMpCIYOdGYm65EmHFhk6FXPErN70uJK\n\tUfkR87Kjm21hNOfXdzsleJoBtrajwjB5tBOY/QSkfvET9cN2RfBjt082ryvx7wFREQ51\n\tD+tqk6FlZzP220cy3EJSls2KrYcnCp7PW8sII=","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=vCkG6p0QSmJ12O793X+ZHdvTxiDd6oWqbl0aD7PBh44=;\n\tb=LKUrfiSpGgrVmka7fxewLLBkKMHCqd/3KG1JP37pEiqbF9yPdFgXRhD/0w+6zqjeNM\n\tODmdBKOwHykZz6CgRYJmZI3ml7fRfAYQPRLtM+eWcNfDKt8KHRTGmMxFq4hsqrnch59/\n\t6ypMitWDXimrxL/T453kP6b5+c5NntYSyYCOs5yuNW7EGQjWWx8v2G63LAhJECUP5P1p\n\tESKfXz59IVEosslrUTXEa180VX+NCeLZ8n2V30l2VzNkXrL6iSNTozDbdBoF6NPHO3bw\n\td6BSfMI+3Px/QBUolkEsCvtsiyupw8tMblSvJayLojEVsvL4XD3d2GRuTfRzOJ8qTc3/\n\tt7qQ==","X-Gm-Message-State":"AHPjjUhE9m8uWak93W8gtF/lTb2NaTRD7l4wgqJtkLn8Kfjrsdeq/jL8\n\tcrsTHDncH/C9KPspzZxlckwubskuFSEFr880HfIz8w==","X-Google-Smtp-Source":"AOwi7QBoO8p160mhisi1cQYRM2gMnmER5o+OlCj/8d0eufZCWhKR6MgueS16PdfHKRw00lA7+gVuxN0ymcb6tttQMUE=","X-Received":"by 10.36.26.87 with SMTP id 84mr2147825iti.38.1506366749647; Mon,\n\t25 Sep 2017 12:12:29 -0700 (PDT)","MIME-Version":"1.0","In-Reply-To":"<2813826.3DA0fVXnVa@aspire.rjw.lan>","References":"<3023226.l5IfJK6GIc@aspire.rjw.lan>\n\t<3958866.l2qnKDbinI@aspire.rjw.lan>\n\t<65494652.vEfz4tCBDb@aspire.rjw.lan>\n\t<2813826.3DA0fVXnVa@aspire.rjw.lan>","From":"Ulf Hansson ","Date":"Mon, 25 Sep 2017 21:12:28 +0200","Message-ID":"","Subject":"Re: [PATCH v4 1/3] PM: i2c-designware-platdrv: Clean up PM handling\n\tin probe","To":"\"Rafael J. Wysocki\" ","Cc":"\"linux-pm@vger.kernel.org\" ,\n\tWolfram Sang ,\n\t\"linux-i2c@vger.kernel.org\" ,\n\tACPI Devel Maling List ,\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 , Lee Jones ,\n\tRajat Jain ","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":1774972,"web_url":"http://patchwork.ozlabs.org/comment/1774972/","msgid":"<3491198.ZaBooUSjR1@aspire.rjw.lan>","list_archive_url":null,"date":"2017-09-25T20:45:31","subject":"Re: [PATCH v4 1/3] PM: i2c-designware-platdrv: Clean up PM handling\n\tin probe","submitter":{"id":26536,"url":"http://patchwork.ozlabs.org/api/people/26536/","name":"Rafael J. Wysocki","email":"rjw@rjwysocki.net"},"content":"On Monday, September 25, 2017 9:12:28 PM CEST Ulf Hansson wrote:\n> On 25 September 2017 at 01:17, Rafael J. Wysocki wrote:\n> > From: Rafael J. Wysocki \n> >\n> > The power management handling in dw_i2c_plat_probe() is somewhat\n> > messy and it is rather hard to figure out the code intention for\n> > the case when pm_disabled is set. In that case, the driver doesn't\n> > enable runtime PM at all, but in addition to that it calls\n> > pm_runtime_forbid() as though it wasn't sure if runtime PM might\n> > be enabled for the device later by someone else.\n> >\n> > Although that concern doesn't seem to be actually valid, the\n> > device is clearly still expected to be PM-capable even in the\n> > pm_disabled set case, so a better approach would be to enable\n> > runtime PM for it unconditionally and then prevent it from\n> > being runtime-suspended by using pm_runtime_forbid().\n> \n> This is nice cleanup! However I have one suggestion/comment.\n> \n> Using pm_runtime_forbid() to prevent the device from being runtime\n> suspended may be a bit fragile, as userspace can then still change to\n> \"allow\" it. Wouldn't it be better to bump the runtime PM usage count\n> (pm_runtime_get_noresume()) instead?\n\nRight, this is not a PCI driver. :-)\n\nI'll send an update of this patch shortly.\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 3y1GYL625Bz9t30\n\tfor ;\n\tTue, 26 Sep 2017 06:54:46 +1000 (AEST)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n\tid S935571AbdIYUyo (ORCPT );\n\tMon, 25 Sep 2017 16:54:44 -0400","from cloudserver094114.home.net.pl ([79.96.170.134]:54000 \"EHLO\n\tcloudserver094114.home.net.pl\" rhost-flags-OK-OK-OK-OK)\n\tby vger.kernel.org with ESMTP id S934254AbdIYUyn (ORCPT\n\t); Mon, 25 Sep 2017 16:54:43 -0400","from 79.184.252.54.ipv4.supernova.orange.pl (79.184.252.54) (HELO\n\taspire.rjw.lan)\n\tby serwer1319399.home.pl (79.96.170.134) with SMTP (IdeaSmtpServer\n\t0.82) id f7a4beeb0f8ba1e7; Mon, 25 Sep 2017 22:54:41 +0200"],"From":"\"Rafael J. Wysocki\" ","To":"Ulf Hansson ","Cc":"\"linux-pm@vger.kernel.org\" ,\n\tWolfram Sang ,\n\t\"linux-i2c@vger.kernel.org\" ,\n\tACPI Devel Maling List ,\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 , Lee Jones ,\n\tRajat Jain ","Subject":"Re: [PATCH v4 1/3] PM: i2c-designware-platdrv: Clean up PM handling\n\tin probe","Date":"Mon, 25 Sep 2017 22:45:31 +0200","Message-ID":"<3491198.ZaBooUSjR1@aspire.rjw.lan>","In-Reply-To":"","References":"<3023226.l5IfJK6GIc@aspire.rjw.lan>\n\t<2813826.3DA0fVXnVa@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"}}]