diff mbox series

[3/5] OPP: Reorder code in _opp_set_required_opps_genpd()

Message ID a5bd698a7a899fb63b4c5caec7068bf5a395165c.1697186772.git.viresh.kumar@linaro.org
State Accepted
Commit c2bebf98045f05b3ff596e060c8777b5356e4826
Headers show
Series OPP: Minor cleanups | expand

Commit Message

Viresh Kumar Oct. 13, 2023, 8:48 a.m. UTC
Reorder code in _opp_set_required_opps_genpd() to reduce duplicate code.

Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/opp/core.c | 34 +++++++++++++++++++---------------
 1 file changed, 19 insertions(+), 15 deletions(-)

Comments

Ulf Hansson Oct. 16, 2023, 10:11 a.m. UTC | #1
On Fri, 13 Oct 2023 at 10:48, Viresh Kumar <viresh.kumar@linaro.org> wrote:
>
> Reorder code in _opp_set_required_opps_genpd() to reduce duplicate code.
>
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
>  drivers/opp/core.c | 34 +++++++++++++++++++---------------
>  1 file changed, 19 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/opp/core.c b/drivers/opp/core.c
> index f42b663a4d8b..3516e79cf743 100644
> --- a/drivers/opp/core.c
> +++ b/drivers/opp/core.c
> @@ -1076,7 +1076,18 @@ static int _opp_set_required_opps_genpd(struct device *dev,
>  {
>         struct device **genpd_virt_devs =
>                 opp_table->genpd_virt_devs ? opp_table->genpd_virt_devs : &dev;
> -       int i, ret = 0;
> +       int index, target, delta, ret;
> +
> +       /* Scaling up? Set required OPPs in normal order, else reverse */
> +       if (!scaling_down) {
> +               index = 0;
> +               target = opp_table->required_opp_count;
> +               delta = 1;
> +       } else {
> +               index = opp_table->required_opp_count - 1;
> +               target = -1;
> +               delta = -1;
> +       }
>
>         /*
>          * Acquire genpd_virt_dev_lock to make sure we don't use a genpd_dev
> @@ -1084,24 +1095,17 @@ static int _opp_set_required_opps_genpd(struct device *dev,
>          */
>         mutex_lock(&opp_table->genpd_virt_dev_lock);
>
> -       /* Scaling up? Set required OPPs in normal order, else reverse */
> -       if (!scaling_down) {
> -               for (i = 0; i < opp_table->required_opp_count; i++) {
> -                       ret = _set_performance_state(dev, genpd_virt_devs[i], opp, i);
> -                       if (ret)
> -                               break;
> -               }
> -       } else {
> -               for (i = opp_table->required_opp_count - 1; i >= 0; i--) {
> -                       ret = _set_performance_state(dev, genpd_virt_devs[i], opp, i);
> -                       if (ret)
> -                               break;
> -               }
> +       while (index != target) {
> +               ret = _set_performance_state(dev, genpd_virt_devs[index], opp, index);
> +               if (ret)
> +                       break;
> +
> +               index += delta;
>         }
>
>         mutex_unlock(&opp_table->genpd_virt_dev_lock);
>
> -       return ret;
> +       return 0;

Why always return 0 and not the error code anymore?

[...]

Kind regards
Uffe
Ulf Hansson Oct. 16, 2023, 2:50 p.m. UTC | #2
On Mon, 16 Oct 2023 at 12:38, Viresh Kumar <viresh.kumar@linaro.org> wrote:
>
> On 16-10-23, 12:11, Ulf Hansson wrote:
> > Why always return 0 and not the error code anymore?
>
> Oops, fixed with:
>
> diff --git a/drivers/opp/core.c b/drivers/opp/core.c
> index 3516e79cf743..42ca52fbe210 100644
> --- a/drivers/opp/core.c
> +++ b/drivers/opp/core.c
> @@ -1076,7 +1076,7 @@ static int _opp_set_required_opps_genpd(struct device *dev,
>  {
>         struct device **genpd_virt_devs =
>                 opp_table->genpd_virt_devs ? opp_table->genpd_virt_devs : &dev;
> -       int index, target, delta, ret;
> +       int index, target, delta, ret = 0;
>
>         /* Scaling up? Set required OPPs in normal order, else reverse */
>         if (!scaling_down) {
> @@ -1105,7 +1105,7 @@ static int _opp_set_required_opps_genpd(struct device *dev,
>
>         mutex_unlock(&opp_table->genpd_virt_dev_lock);
>
> -       return 0;
> +       return ret;
>  }
>
>  /* This is only called for PM domain for now */
>

Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org>

Kind regards
Uffe
diff mbox series

Patch

diff --git a/drivers/opp/core.c b/drivers/opp/core.c
index f42b663a4d8b..3516e79cf743 100644
--- a/drivers/opp/core.c
+++ b/drivers/opp/core.c
@@ -1076,7 +1076,18 @@  static int _opp_set_required_opps_genpd(struct device *dev,
 {
 	struct device **genpd_virt_devs =
 		opp_table->genpd_virt_devs ? opp_table->genpd_virt_devs : &dev;
-	int i, ret = 0;
+	int index, target, delta, ret;
+
+	/* Scaling up? Set required OPPs in normal order, else reverse */
+	if (!scaling_down) {
+		index = 0;
+		target = opp_table->required_opp_count;
+		delta = 1;
+	} else {
+		index = opp_table->required_opp_count - 1;
+		target = -1;
+		delta = -1;
+	}
 
 	/*
 	 * Acquire genpd_virt_dev_lock to make sure we don't use a genpd_dev
@@ -1084,24 +1095,17 @@  static int _opp_set_required_opps_genpd(struct device *dev,
 	 */
 	mutex_lock(&opp_table->genpd_virt_dev_lock);
 
-	/* Scaling up? Set required OPPs in normal order, else reverse */
-	if (!scaling_down) {
-		for (i = 0; i < opp_table->required_opp_count; i++) {
-			ret = _set_performance_state(dev, genpd_virt_devs[i], opp, i);
-			if (ret)
-				break;
-		}
-	} else {
-		for (i = opp_table->required_opp_count - 1; i >= 0; i--) {
-			ret = _set_performance_state(dev, genpd_virt_devs[i], opp, i);
-			if (ret)
-				break;
-		}
+	while (index != target) {
+		ret = _set_performance_state(dev, genpd_virt_devs[index], opp, index);
+		if (ret)
+			break;
+
+		index += delta;
 	}
 
 	mutex_unlock(&opp_table->genpd_virt_dev_lock);
 
-	return ret;
+	return 0;
 }
 
 /* This is only called for PM domain for now */