Received-SPF: pass (google.com: domain of linux-kernel+bounces-17123-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Precedence: bulk
MIME-Version: 1.0
References: <20240104171553.2080674-1-lukasz.luba@arm.com> <20240104171553.2080674-3-lukasz.luba@arm.com>
In-Reply-To: <20240104171553.2080674-3-lukasz.luba@arm.com>
From: "Rafael J. Wysocki" <rafael@kernel.org>
Date: Thu, 4 Jan 2024 20:07:59 +0100
Message-ID: <CAJZ5v0jCtS1XHczORnZFhwO7N=FommFNbQC7NdaM5SHKXUnTog@mail.gmail.com>
Subject: Re: [PATCH v6 02/23] PM: EM: Refactor em_cpufreq_update_efficiencies()
 arguments
To: Lukasz Luba <lukasz.luba@arm.com>
Cc: linux-kernel@vger.kernel.org, linux-pm@vger.kernel.org, rafael@kernel.org, 
	dietmar.eggemann@arm.com, rui.zhang@intel.com, amit.kucheria@verdurent.com, 
	amit.kachhap@gmail.com, daniel.lezcano@linaro.org, viresh.kumar@linaro.org, 
	len.brown@intel.com, pavel@ucw.cz, mhiramat@kernel.org, qyousef@layalina.io, 
	wvw@google.com
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

The word "refactor" appears to be quite loaded in your patch
descriptions, but it is not always the best one to use IMV.

For instance, this patch simply extends the argument list of
em_cpufreq_update_efficiencies(), so I would say just that in the
subject: "Extend em_cpufreq_update_efficiencies() argument list"

On Thu, Jan 4, 2024 at 6:14=E2=80=AFPM Lukasz Luba <lukasz.luba@arm.com> wr=
ote:
>
> In order to prepare the code for the modifiable EM perf_state table,
> refactor existing function em_cpufreq_update_efficiencies().

"make em_cpufreq_update_efficiencies() take a pointer to the EM table
as its second argument and modify it to use that new argument instead
of the "table" member of dev->em_pd"

or something like this.

> The function now takes the ptr to the EM table as its argument.
>
> No functional impact.
>
> Signed-off-by: Lukasz Luba <lukasz.luba@arm.com>
> ---
>  kernel/power/energy_model.c | 8 +++-----
>  1 file changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/kernel/power/energy_model.c b/kernel/power/energy_model.c
> index 8b9dd4a39f63..42486674b834 100644
> --- a/kernel/power/energy_model.c
> +++ b/kernel/power/energy_model.c
> @@ -237,10 +237,10 @@ static int em_create_pd(struct device *dev, int nr_=
states,
>         return 0;
>  }
>
> -static void em_cpufreq_update_efficiencies(struct device *dev)
> +static void
> +em_cpufreq_update_efficiencies(struct device *dev, struct em_perf_state =
*table)
>  {
>         struct em_perf_domain *pd =3D dev->em_pd;
> -       struct em_perf_state *table;
>         struct cpufreq_policy *policy;
>         int found =3D 0;
>         int i;
> @@ -254,8 +254,6 @@ static void em_cpufreq_update_efficiencies(struct dev=
ice *dev)
>                 return;
>         }
>
> -       table =3D pd->table;
> -
>         for (i =3D 0; i < pd->nr_perf_states; i++) {
>                 if (!(table[i].flags & EM_PERF_STATE_INEFFICIENT))
>                         continue;
> @@ -397,7 +395,7 @@ int em_dev_register_perf_domain(struct device *dev, u=
nsigned int nr_states,
>
>         dev->em_pd->flags |=3D flags;
>
> -       em_cpufreq_update_efficiencies(dev);
> +       em_cpufreq_update_efficiencies(dev, dev->em_pd->table);
>
>         em_debug_create_pd(dev);
>         dev_info(dev, "EM: created perf domain\n");
> --

The code change itself LGTM.