diff mbox series

opp: Allow opp-level to be set to 0

Message ID 40b676a1e261d1cfd12788d88e0d2026dbbe226b.1599817789.git.viresh.kumar@linaro.org
State Accepted
Commit a5663c9b1e31c00e0bdfaf4d92eb51358fc3950f
Headers show
Series opp: Allow opp-level to be set to 0 | expand

Commit Message

Viresh Kumar Sept. 11, 2020, 9:49 a.m. UTC
The DT bindings don't put such a constraint, nor should the kernel. It
is perfectly fine for opp-level to be set to 0, if we need to put the
performance state votes for a domain for a particular OPP.

Reported-by: Stephan Gerhold <stephan@gerhold.net>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>

---
 drivers/opp/core.c |  3 ---
 drivers/opp/of.c   | 20 +++++++-------------
 2 files changed, 7 insertions(+), 16 deletions(-)

-- 
2.25.0.rc1.19.g042ed3e048af

Comments

Stephan Gerhold Sept. 14, 2020, 8:11 p.m. UTC | #1
On Fri, Sep 11, 2020 at 03:19:53PM +0530, Viresh Kumar wrote:
> The DT bindings don't put such a constraint, nor should the kernel. It
> is perfectly fine for opp-level to be set to 0, if we need to put the
> performance state votes for a domain for a particular OPP.
> 
> Reported-by: Stephan Gerhold <stephan@gerhold.net>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>

I tried adding an opp-level = <0> and voting for it using required-opps
for some of the OPPs in the CPU OPP table and it seems to work fine now.

Tested-by: Stephan Gerhold <stephan@gerhold.net>

Just need to solve the "enable power domains" problem now :)

Thanks!
Stephan

> ---
>  drivers/opp/core.c |  3 ---
>  drivers/opp/of.c   | 20 +++++++-------------
>  2 files changed, 7 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/opp/core.c b/drivers/opp/core.c
> index 214c1619b445..2483e765318a 100644
> --- a/drivers/opp/core.c
> +++ b/drivers/opp/core.c
> @@ -2117,9 +2117,6 @@ int dev_pm_opp_xlate_performance_state(struct opp_table *src_table,
>  	int dest_pstate = -EINVAL;
>  	int i;
>  
> -	if (!pstate)
> -		return 0;
> -
>  	/*
>  	 * Normally the src_table will have the "required_opps" property set to
>  	 * point to one of the OPPs in the dst_table, but in some cases the
> diff --git a/drivers/opp/of.c b/drivers/opp/of.c
> index e72753be7dc7..87f839a1899a 100644
> --- a/drivers/opp/of.c
> +++ b/drivers/opp/of.c
> @@ -842,7 +842,7 @@ static struct dev_pm_opp *_opp_add_static_v2(struct opp_table *opp_table,
>  static int _of_add_opp_table_v2(struct device *dev, struct opp_table *opp_table)
>  {
>  	struct device_node *np;
> -	int ret, count = 0, pstate_count = 0;
> +	int ret, count = 0;
>  	struct dev_pm_opp *opp;
>  
>  	/* OPP table is already initialized for the device */
> @@ -876,20 +876,14 @@ static int _of_add_opp_table_v2(struct device *dev, struct opp_table *opp_table)
>  		goto remove_static_opp;
>  	}
>  
> -	list_for_each_entry(opp, &opp_table->opp_list, node)
> -		pstate_count += !!opp->pstate;
> -
> -	/* Either all or none of the nodes shall have performance state set */
> -	if (pstate_count && pstate_count != count) {
> -		dev_err(dev, "Not all nodes have performance state set (%d: %d)\n",
> -			count, pstate_count);
> -		ret = -ENOENT;
> -		goto remove_static_opp;
> +	list_for_each_entry(opp, &opp_table->opp_list, node) {
> +		/* Any non-zero performance state would enable the feature */
> +		if (opp->pstate) {
> +			opp_table->genpd_performance_state = true;
> +			break;
> +		}
>  	}
>  
> -	if (pstate_count)
> -		opp_table->genpd_performance_state = true;
> -
>  	return 0;
>  
>  remove_static_opp:
> -- 
> 2.25.0.rc1.19.g042ed3e048af
>
diff mbox series

Patch

diff --git a/drivers/opp/core.c b/drivers/opp/core.c
index 214c1619b445..2483e765318a 100644
--- a/drivers/opp/core.c
+++ b/drivers/opp/core.c
@@ -2117,9 +2117,6 @@  int dev_pm_opp_xlate_performance_state(struct opp_table *src_table,
 	int dest_pstate = -EINVAL;
 	int i;
 
-	if (!pstate)
-		return 0;
-
 	/*
 	 * Normally the src_table will have the "required_opps" property set to
 	 * point to one of the OPPs in the dst_table, but in some cases the
diff --git a/drivers/opp/of.c b/drivers/opp/of.c
index e72753be7dc7..87f839a1899a 100644
--- a/drivers/opp/of.c
+++ b/drivers/opp/of.c
@@ -842,7 +842,7 @@  static struct dev_pm_opp *_opp_add_static_v2(struct opp_table *opp_table,
 static int _of_add_opp_table_v2(struct device *dev, struct opp_table *opp_table)
 {
 	struct device_node *np;
-	int ret, count = 0, pstate_count = 0;
+	int ret, count = 0;
 	struct dev_pm_opp *opp;
 
 	/* OPP table is already initialized for the device */
@@ -876,20 +876,14 @@  static int _of_add_opp_table_v2(struct device *dev, struct opp_table *opp_table)
 		goto remove_static_opp;
 	}
 
-	list_for_each_entry(opp, &opp_table->opp_list, node)
-		pstate_count += !!opp->pstate;
-
-	/* Either all or none of the nodes shall have performance state set */
-	if (pstate_count && pstate_count != count) {
-		dev_err(dev, "Not all nodes have performance state set (%d: %d)\n",
-			count, pstate_count);
-		ret = -ENOENT;
-		goto remove_static_opp;
+	list_for_each_entry(opp, &opp_table->opp_list, node) {
+		/* Any non-zero performance state would enable the feature */
+		if (opp->pstate) {
+			opp_table->genpd_performance_state = true;
+			break;
+		}
 	}
 
-	if (pstate_count)
-		opp_table->genpd_performance_state = true;
-
 	return 0;
 
 remove_static_opp: