From patchwork Mon Oct 24 04:26:59 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Viresh Kumar X-Patchwork-Id: 78845 Delivered-To: patch@linaro.org Received: by 10.140.97.247 with SMTP id m110csp2382786qge; Sun, 23 Oct 2016 21:27:07 -0700 (PDT) X-Received: by 10.99.115.65 with SMTP id d1mr20203361pgn.56.1477283227719; Sun, 23 Oct 2016 21:27:07 -0700 (PDT) Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id j128si13517849pgc.174.2016.10.23.21.27.06; Sun, 23 Oct 2016 21:27:07 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of linux-pm-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; dkim=neutral (body hash did not verify) header.i=@linaro.org; spf=pass (google.com: best guess record for domain of linux-pm-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-pm-owner@vger.kernel.org; dmarc=fail (p=NONE dis=NONE) header.from=linaro.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751512AbcJXE1F (ORCPT + 14 others); Mon, 24 Oct 2016 00:27:05 -0400 Received: from mail-pf0-f174.google.com ([209.85.192.174]:33319 "EHLO mail-pf0-f174.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751438AbcJXE1E (ORCPT ); Mon, 24 Oct 2016 00:27:04 -0400 Received: by mail-pf0-f174.google.com with SMTP id 128so90893479pfz.0 for ; Sun, 23 Oct 2016 21:27:04 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=mycE3QZh6XtX1pa8HuTj5y8jC4dFS5YKd04Rhw3eeWA=; b=fnCkQoQqlHtGIHPuUrLWtSSpkY3R4PAOq41fGexL80It7w5C4H7oRJaAHY2YeSw9BI 2AfPMtWb57hWMpR4M/L4IyodT83vFHyKcTRsNowdbun2JcuhMAeqvbENJCJB285Y1jo0 83rYTuYYVE5QBL3Wd5YMAZAiBlDReknW7KV6k= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=mycE3QZh6XtX1pa8HuTj5y8jC4dFS5YKd04Rhw3eeWA=; b=BaW2koEOQQjcJmkf07PuToXwp4YIW31WtRSbIOyyiAG8OZrjFqWRoNK5zKrGum8SqN Um02bqxJ/+ky/P7QEcY+eMom5fTmoXQWbyTTZIYXcdQuWFYRaOsyNUsHEF3JrZI/qLhE BugBtsHtlLzrCslhxM2GMwDoOuYXTcVJbO9ED4mA4qS5PhwPfapS5huBdWL8MWDFjoKK om7RMdiHOdKqBwHAUEkb/b1ZmeMRRQjfp0YbkVQacZO93LwVi0Oe1A8X/m3bYKVSAwaA i6U7ZB0f5RZpik90hFX8atM1P0CeIOdkLJOppBLbu9Mwhx5hQEdqUH8v4XVM3QFgb3ix 3x2g== X-Gm-Message-State: ABUngvcAggAcwBYpTvL02PBcGQCq5UG0B6tojgZ7ZrYRrYQ4199+8AcG/WH8QzquYxaicrP/ X-Received: by 10.98.16.203 with SMTP id 72mr25066640pfq.57.1477283223624; Sun, 23 Oct 2016 21:27:03 -0700 (PDT) Received: from localhost ([171.61.116.10]) by smtp.gmail.com with ESMTPSA id yx8sm20981923pac.29.2016.10.23.21.27.01 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Sun, 23 Oct 2016 21:27:02 -0700 (PDT) Date: Mon, 24 Oct 2016 09:56:59 +0530 From: Viresh Kumar To: Dave Gerlach Cc: "Rafael J. Wysocki" , Rafael Wysocki , Nishanth Menon , Stephen Boyd , Lists linaro-kernel , Linux PM , Linux Kernel Mailing List , Vincent Guittot , Rob Herring , Mark Brown Subject: Re: [PATCH V2 0/8] PM / OPP: Multiple regulator support Message-ID: <20161024042659.GD24749@vireshk-i7> References: <41a8b4a1-94d3-e086-a328-495f69815d1c@ti.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <41a8b4a1-94d3-e086-a328-495f69815d1c@ti.com> User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-pm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-pm@vger.kernel.org On 23-10-16, 20:08, Dave Gerlach wrote: > Overall this series looks good to me apart from a few small things. Most > importantly I was able to get a working implementation using two regulators > on ti dra7xx platform with proper sequencing built on top of this series. We > have cpu regulator and Adaptive body bias (abb) regulator that must be > scaled in a certain order before or after clock scaling and I was able to > implement a rough custom set_rate to perform this and ran some dvfs stress > tests that all worked fine. Thanks for testing it buddy. > First comment, I think the platform specific set_rate is a good place to > hook in for adaptive voltage scaling as well. I was able to implement TI > Class0 AVS in the same code by using the requested transition voltage as a > reference and programming AVS voltage using that, along with scaling the > additional regulators in sequence (the original multi regulator > functionality). Hmm, interesting.. > I would think some people would want to use this even with > single regulator platforms, no? Maybe, but I would like to see such user code first. It may be possible to handle much of AVS stuff in core so that everyone isn't required to do it. > This raises some concerns about dependencies/probe sequencing. Right now we > just need to make sure the cpufreq-dt driver probes after we have called > _set_regulators, but if our platform code fails cpufreq-dt currently will > treat this as no regulator needed for the platform and operate without one, > which will likely hang the system. Is there a good way to to guarantee this > doesn't happen? My main concern is that if we plan to provide a platform > specific set-rate function, we should have a way to indicate this and > prevent things from progressing if it isn't yet ready. > > Again, overall I think it solves the multi regulator problem, and it works > well for AVS as well. For the series: > > Tested-by: Dave Gerlach Thanks. For the concern you shared about, does the below patch fix it ? I will include that in V3 then. -------------------------8<------------------------- From: Viresh Kumar Date: Mon, 24 Oct 2016 09:45:30 +0530 Subject: [PATCH] PM / OPP: Don't assume platform doesn't have regulators If the regulators aren't set explicitly by the platform, the OPP core assumes that the platform doesn't have any regulator and uses the clk-only callback. If the platform failed to register a regulator with the core, then this can turn out to be a dangerous assumption as the OPP core will try to change clk without changing regulators. Handle that properly by making sure that the DT didn't had any entries for supply voltages as well. Signed-off-by: Viresh Kumar --- drivers/base/power/opp/core.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) -- viresh -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/drivers/base/power/opp/core.c b/drivers/base/power/opp/core.c index b69908b74ed6..fb4250532180 100644 --- a/drivers/base/power/opp/core.c +++ b/drivers/base/power/opp/core.c @@ -737,7 +737,17 @@ int dev_pm_opp_set_rate(struct device *dev, unsigned long target_freq) /* Only frequency scaling */ if (!regulators) { - rcu_read_unlock(); + /* + * DT contained supply ratings? Consider platform failed to set + * regulators. + */ + if (unlikely(opp->supplies[0].u_volt)) { + rcu_read_unlock(); + dev_err(dev, "%s: Regulator not registered with OPP core\n", + __func__); + return -EINVAL; + } + return _generic_opp_set_rate_clk_only(dev, clk, old_freq, freq); }