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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 26.05.2021 16:44, Jason Andryuk wrote:
> On Wed, May 26, 2021 at 9:27 AM Jan Beulich <jbeulich@xxxxxxxx> wrote:
>> On 03.05.2021 21:28, Jason Andryuk wrote:
>>> --- 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?

Would feel less fragile, yes. And no need for the intermediate "policy"
variable, afaics.

Jan