Re: [PATCH 03/13] cpufreq: Export intel_feature_detect

[Jason Andryuk <jandryuk@xxxxxxxxx>]

On Wed, May 26, 2021 at 9:27 AM Jan Beulich <jbeulich@xxxxxxxx> wrote:
>
> On 03.05.2021 21:28, Jason Andryuk wrote:
> > Export feature_detect as intel_feature_detect so it can be re-used by
> > HWP.
> >
> > Signed-off-by: Jason Andryuk <jandryuk@xxxxxxxxx>
>
> Acked-by: Jan Beulich <jbeulich@xxxxxxxx>
> albeit ...
>
> > --- a/xen/arch/x86/acpi/cpufreq/cpufreq.c
> > +++ b/xen/arch/x86/acpi/cpufreq/cpufreq.c
> > @@ -340,7 +340,7 @@ static unsigned int get_cur_freq_on_cpu(unsigned int 
> > cpu)
> >      return extract_freq(get_cur_val(cpumask_of(cpu)), data);
> >  }
> >
> > -static void feature_detect(void *info)
> > +void intel_feature_detect(void *info)
> >  {
> >      struct cpufreq_policy *policy = info;
>
> ... because of this (requiring the hwp code to stay in sync with
> possible changes here, without the compiler being able to point
> out inconsistencies) I'm not overly happy with such a change. Yet
> I guess this isn't the first case we have in the code base.

For acpi-cpufreq, this is called by on_selected_cpus(), but hwp calls
this directly.  You could do something like:

void intel_feature_detect(struct cpufreq_policy *policy)
{
    /* current feature_detect() */
}

static void feature_detect(void *info)
    struct cpufreq_policy *policy = info;

    intel_feature_detect(policy);
}

Would you prefer that?

Regards,
Jason