Re: [PATCH v6 02/23] PM: EM: Refactor em_cpufreq_update_efficiencies() arguments

[Lukasz Luba]

Hi Rafael,

On 1/4/24 19:07, Rafael J. Wysocki wrote:
> The word "refactor" appears to be quite loaded in your patch
> descriptions, but it is not always the best one to use IMV.

Fair enough, I'll change those patches according to your comments.

> 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 PM Lukasz Luba <lukasz.luba@xxxxxxx> wrote:
> > 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.

I think I got the point, will change it.

> > The function now takes the ptr to the EM table as its argument.
> >
> > No functional impact.
> >
> > Signed-off-by: Lukasz Luba <lukasz.luba@xxxxxxx>
> > ---
> >   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 = dev->em_pd;
> > -       struct em_perf_state *table;
> >          struct cpufreq_policy *policy;
> >          int found = 0;
> >          int i;
> > @@ -254,8 +254,6 @@ static void em_cpufreq_update_efficiencies(struct device *dev)
> >                  return;
> >          }
> >
> > -       table = pd->table;
> > -
> >          for (i = 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, unsigned int nr_states,
> >
> >          dev->em_pd->flags |= 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.

Thanks