Re: [XEN PATCH v1] x86/cpufreq: separate powernow/hwp cpufreq code

[Jan Beulich <jbeulich@xxxxxxxx>]

On 07.06.2024 11:14, Sergiy Kibrik wrote:
> 06.06.24 10:54, Jan Beulich:
>> On 06.06.2024 09:30, Sergiy Kibrik wrote:
>>> 06.06.24 10:08, Jan Beulich:
>>>> On 04.06.2024 11:34, Sergiy Kibrik wrote:
>>>>> --- a/xen/arch/x86/acpi/cpufreq/cpufreq.c
>>>>> +++ b/xen/arch/x86/acpi/cpufreq/cpufreq.c
>>>>> @@ -657,7 +657,7 @@ static int __init cf_check cpufreq_driver_init(void)
>>>>>    
>>>>>            case X86_VENDOR_AMD:
>>>>>            case X86_VENDOR_HYGON:
>>>>> -            ret = powernow_register_driver();
>>>>> +            ret = IS_ENABLED(CONFIG_AMD) ? powernow_register_driver() : 
>>>>> -ENODEV;
>>>>>                break;
>>>>>            }
>>>>
>>>> What about the Intel-specific code immediately up from here?
>>>> Dealing with that as well may likely permit to reduce ...
>>>
>>> you mean to guard a call to hwp_register_driver() the same way as for
>>> powernow_register_driver(), and save one stub? ?
>>
>> Yes, and perhaps more. Maybe more stubs can be avoided? And
>> acpi_cpufreq_driver doesn't need registering either, and hence
>> would presumably be left unreferenced when !INTEL?
>>
> 
> {get,set}_hwp_para() can be avoided, as they're being called just once 
> and may be guarded by IS_ENABLED(CONFIG_INTEL).
> The same for hwp_cmdline_parse().
> As for hwp_active() it's being used many times by generic cpufreq code 
> and even outside of cpufreq, so probably it has to be either a stub, or 
> be moved outside of hwp.c and become smth, like this:
> 
>   bool hwp_active(void)
>   {
>      return IS_ENABLED(CONFIG_INTEL) && hwp_in_use;
>   }
> 
> Though I'm not sure such movement would be any better than a stub.
> 
> acpi_cpufreq_driver, i.e. the most of code in cpufreq.c file, can 
> probably be separated into acpi.c and put under CONFIG_INTEL as well. 
> What you think of this?

Sounds like the direction I think we want to be following.

Jan