From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932348AbdLTGm5 (ORCPT ); Wed, 20 Dec 2017 01:42:57 -0500 Received: from lelnx194.ext.ti.com ([198.47.27.80]:16581 "EHLO lelnx194.ext.ti.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932216AbdLTGmy (ORCPT ); Wed, 20 Dec 2017 01:42:54 -0500 Subject: Re: [PATCH 1/3] phy: core: Move runtime PM reference counting to the parent device To: Ulf Hansson , References: <1513718551-28624-1-git-send-email-ulf.hansson@linaro.org> <1513718551-28624-2-git-send-email-ulf.hansson@linaro.org> CC: "Rafael J . Wysocki" , , Yoshihiro Shimoda , Geert Uytterhoeven , From: Kishon Vijay Abraham I Message-ID: Date: Wed, 20 Dec 2017 12:12:42 +0530 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.7.0 MIME-Version: 1.0 In-Reply-To: <1513718551-28624-2-git-send-email-ulf.hansson@linaro.org> Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: 7bit X-EXCLAIMER-MD-CONFIG: e1e8a2fd-e40a-4ac6-ac9b-f7e9cc9ee180 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Ulf, On Wednesday 20 December 2017 02:52 AM, Ulf Hansson wrote: > The runtime PM deployment in the phy core is a bit unnecessary complicated > and the main reason is because it operates on the phy device, which is > created by the phy core and assigned as a child device of the phy provider > device. > > Let's simplify the code, by replacing the existing calls to > phy_pm_runtime_get_sync() and phy_pm_runtime_put(), with regular calls to > pm_runtime_get_sync() and pm_runtime_put(). While doing that, let's also > change to give the phy provider device as the parameter to the runtime PM > calls. This together with adding error paths, that allows the phy > provider device to be runtime PM disabled, enables further clean up the > code. More precisely, we can simply avoid to enable runtime PM for the phy > device altogether, so let's do that as well. > > More importantly, this change also fixes an issue for system suspend. > Especially in those cases when the phy provider device gets put into a low > power state via calling the pm_runtime_force_suspend() helper, as is the > case for a Renesas SoC, which has the phy provider device attached to the > generic PM domain. > > The problem in this case, is that pm_runtime_force_suspend() expects the > child device of the provider device to be runtime suspended, else this will > trigger a WARN splat (correctly) when runtime PM gets re-enabled at system > resume. > > In the current case, even if phy_power_off() triggers a pm_runtime_put() > during system suspend the phy device (child) doesn't get runtime suspended, > because that is prevented in the system suspend phases. However, by > avoiding to enable runtime PM, this problem goes away. > > Signed-off-by: Ulf Hansson > --- > drivers/phy/phy-core.c | 33 +++++++++++++-------------------- > 1 file changed, 13 insertions(+), 20 deletions(-) > > diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c > index b4964b0..9fa3f13 100644 > --- a/drivers/phy/phy-core.c > +++ b/drivers/phy/phy-core.c > @@ -222,10 +222,10 @@ int phy_init(struct phy *phy) > if (!phy) > return 0; > > - ret = phy_pm_runtime_get_sync(phy); > - if (ret < 0 && ret != -ENOTSUPP) > + ret = pm_runtime_get_sync(phy->dev.parent); Won't this make phy-core manage pm_runtime of phy_provider even though the phy_provider might not intend it? Thanks Kishon