Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756933AbdLWPJi (ORCPT ); Sat, 23 Dec 2017 10:09:38 -0500 Received: from mail-io0-f175.google.com ([209.85.223.175]:34222 "EHLO mail-io0-f175.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752583AbdLWPJf (ORCPT ); Sat, 23 Dec 2017 10:09:35 -0500 X-Google-Smtp-Source: ACJfBoszWEPG0PogYHnowCJq79l8Hs/iB8BdysUuKh2m3L5DW+VYPubaLBdh3CwzTR76cRntvsvbnTMObEHtun2fRz0= MIME-Version: 1.0 In-Reply-To: References: <1513778960-10073-1-git-send-email-ulf.hansson@linaro.org> <1513778960-10073-2-git-send-email-ulf.hansson@linaro.org> From: Ulf Hansson Date: Sat, 23 Dec 2017 16:09:33 +0100 Message-ID: Subject: Re: [PATCH v2 1/3] phy: core: Move runtime PM reference counting to the parent device To: "Rafael J. Wysocki" Cc: Kishon Vijay Abraham I , Linux Kernel Mailing List , "Rafael J . Wysocki" , Linux PM , Yoshihiro Shimoda , Geert Uytterhoeven , Linux-Renesas Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1977 Lines: 47 [...] > > So IMO the changes you are proposing make sense regardless of the > genpd issue, because they generally simplify the phy code, but the > additional use_runtime_pm field in struct phy represents redundant > information (manipulating reference counters shouldn't matter if > runtime PM is disabled), so it doesn't appear to be necessary. > Actually, the first version I posted treated the return codes from pm_runtime_get_sync() according to your suggestion above. However, Kishon pointed out that it didn't work. That's because, there are phy provider drivers that enables runtime PM *after* calling phy_create(). And in those cases, that is because they want to treat runtime PM themselves. I think that's probably something we should look into to change, but I find it being a separate issue, that I didn't want to investigate as part of this series. See more about the thread here: https://www.spinics.net/lists/linux-renesas-soc/msg21711.html > [On a related note, I'm not sure why phy tries to intercept runtime PM > errors and "fix up" the reference counters. That doesn't look right > to me at all.] > > That said, the current phy code is not strictly invalid. While it > looks more complicated than necessary, it doesn't do things documented > as invalid in principle, so saying "The behaviour around the runtime > PM deployment cause some issues during system suspend" in the > changelog is describing the problem from a very specific angle. > Simply put, pm_runtime_force_suspend() and the current phy code cannot > work together and so using them together is a bug. None of them > individually is at fault, but combining them is incorrect. > > Fortunately enough, the phy code can be modified so that it can be > used with pm_runtime_force_suspend() without problems, but picturing > it as "problematic", because it cannot do that today is not entirely > fair IMO. Right, this makes sense. Let me clarify this in the changelog. Kind regards Uffe