Re: [Xen-devel] [RFC PATCH v2 06/12] cpufreq: make cpufreq driver more generalizable

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 22.10.14 at 11:57, <oleksandr.dmytryshyn@xxxxxxxxxxxxxxx> wrote:
> On Wed, Oct 22, 2014 at 12:51 PM, Oleksandr Dmytryshyn
> <oleksandr.dmytryshyn@xxxxxxxxxxxxxxx> wrote:
>> On Wed, Oct 22, 2014 at 12:21 PM, Jan Beulich <JBeulich@xxxxxxxx> wrote:
>>>>>> On 22.10.14 at 10:39, <oleksandr.dmytryshyn@xxxxxxxxxxxxxxx> wrote:
>>>> On Fri, Oct 17, 2014 at 3:31 PM, Jan Beulich <JBeulich@xxxxxxxx> wrote:
>>>>>>>> On 16.10.14 at 13:27, <oleksandr.dmytryshyn@xxxxxxxxxxxxxxx> wrote:
>>>>>> --- a/xen/drivers/cpufreq/cpufreq.c
>>>>>> +++ b/xen/drivers/cpufreq/cpufreq.c
>>>>>> @@ -43,9 +43,14 @@
>>>>>>  #include <asm/io.h>
>>>>>>  #include <asm/processor.h>
>>>>>>  #include <asm/percpu.h>
>>>>>> -#include <acpi/acpi.h>
>>>>>>  #include <xen/cpufreq.h>
>>>>>
>>>>> I can see this being moved into a #ifdef CONFIG_ACPI section, but
>>>>> removing it altogether seems kind of wrong.
>>>>>
>>>>>> +#ifdef CONFIG_ACPI
>>>>>> +    #define XEN_PX_FULL_INIT (XEN_PX_PCT | XEN_PX_PSS | XEN_PX_PSD |
>>>> XEN_PX_PPC)
>>>>>> +#else
>>>>>> +    #define XEN_PX_FULL_INIT (XEN_PX_PSS | XEN_PX_PSD | XEN_PX_PPC)
>>>>>> +#endif
>>>>>
>>>>> I'm not really understanding what is ACPI-specific about PCT but not
>>>>> any of the other three. Please give some explanation in the commit
>>>>> description.
>>>> XEN_PX_PPC - 'platform_limit' setting is included to the
>>>> processor_performance
>>>> structure which can be used in ARM meaning
>>>>
>>>> XEN_PX_PCT - 'control_register' and 'status_register' settings (MSR and MCR
>>>> registers) are included to the processor_performance structure - this is
>>>> ACPI-specific settings because MSR and MCR registers are ACPI-specific
>>>>
>>>> XEN_PX_PSS - information about P-states is included to the
>>>> processor_performance
>>>> structure which can be used in ARM meaning (frequency in each state,
>>>> state_count etc.)
>>
>> It was a typo. It should be XEN_PX_PSD in last paragraph
>>>> XEN_PX_PPC - inforamtion about pomer domain info and shared_type is 
>>>> included
>>>> to the processor_performance structure which can be used in ARM meaning
>>>> (shared_type and power domain number)
>>>
>>> Not sure why you listed XEN_PX_PPC twice, but ignored PSD (I guess
>>> one of the two was just a typo). In any event I suggest no
>>> overloading ACPI things with ARM non-ACPI ones (re-using interface
>>> structures may be okay if they're truly not ACPI specific, but ACPI
>>> naming shouldn't be applied to non-ACPI items).
> May be it will be better to introduce a new flag (XEN_PX_DATA) for
> non-ACPI case?

Exactly.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel