Re: [Xen-devel] [RFC PATCH v3 10/12] cpufreq: add hwdom-cpufreq driver

[Oleksandr Dmytryshyn <oleksandr.dmytryshyn@xxxxxxxxxxxxxxx>]

On Fri, Oct 24, 2014 at 4:08 PM, Julien Grall <julien.grall@xxxxxxxxxx> wrote:
> On 10/24/2014 02:05 PM, Oleksandr Dmytryshyn wrote:
>> On Fri, Oct 24, 2014 at 2:45 PM, Julien Grall <julien.grall@xxxxxxxxxx> 
>> wrote:
>>> On 10/24/2014 11:30 AM, Oleksandr Dmytryshyn wrote:
>>>> On Thu, Oct 23, 2014 at 7:42 PM, Julien Grall <julien.grall@xxxxxxxxxx> 
>>>> wrote:
>>>>> Hi Oleksandr,
>>>>>
>>>>> On 10/23/2014 04:07 PM, Oleksandr Dmytryshyn wrote:
>>>>>> This driver uses hwdom to change frequencies on CPUs
>>>>>>
>>>>>> Signed-off-by: Oleksandr Dmytryshyn 
>>>>>> <oleksandr.dmytryshyn@xxxxxxxxxxxxxxx>
>>>>>> ---
>>>>>>  xen/Rules.mk                        |   1 +
>>>>>>  xen/drivers/cpufreq/Makefile        |   1 +
>>>>>>  xen/drivers/cpufreq/hwdom-cpufreq.c | 220 
>>>>>> ++++++++++++++++++++++++++++++++++++
>>>>>>  xen/include/public/xen.h            |   1 +
>>>>>>  4 files changed, 223 insertions(+)
>>>>>>  create mode 100644 xen/drivers/cpufreq/hwdom-cpufreq.c
>>>>>>
>>>>>> diff --git a/xen/Rules.mk b/xen/Rules.mk
>>>>>> index 3b0b89b..cccbc72 100644
>>>>>> --- a/xen/Rules.mk
>>>>>> +++ b/xen/Rules.mk
>>>>>> @@ -56,6 +56,7 @@ CFLAGS-$(perfc_arrays)  += -DPERF_ARRAYS
>>>>>>  CFLAGS-$(lock_profile)  += -DLOCK_PROFILE
>>>>>>  CFLAGS-$(HAS_ACPI)      += -DHAS_ACPI
>>>>>>  CFLAGS-$(HAS_CPUFREQ)   += -DHAS_CPUFREQ
>>>>>> +CFLAGS-$(HAS_HWDOM_CPUFREQ) += -DHAS_HWDOM_CPUFREQ
>>>>>>  CFLAGS-$(HAS_PM)        += -DHAS_PM
>>>>>>  CFLAGS-$(HAS_CPU_TURBO) += -DHAS_CPU_TURBO
>>>>>>  CFLAGS-$(HAS_GDBSX)     += -DHAS_GDBSX
>>>>>> diff --git a/xen/drivers/cpufreq/Makefile b/xen/drivers/cpufreq/Makefile
>>>>>> index b87d127..891997c 100644
>>>>>> --- a/xen/drivers/cpufreq/Makefile
>>>>>> +++ b/xen/drivers/cpufreq/Makefile
>>>>>> @@ -2,3 +2,4 @@ obj-y += cpufreq.o
>>>>>>  obj-y += cpufreq_ondemand.o
>>>>>>  obj-y += cpufreq_misc_governors.o
>>>>>>  obj-y += utility.o
>>>>>> +obj-$(HAS_HWDOM_CPUFREQ) += hwdom-cpufreq.o
>>>>>> diff --git a/xen/drivers/cpufreq/hwdom-cpufreq.c 
>>>>>> b/xen/drivers/cpufreq/hwdom-cpufreq.c
>>>>>> new file mode 100644
>>>>>> index 0000000..67c9e1d
>>>>>> --- /dev/null
>>>>>> +++ b/xen/drivers/cpufreq/hwdom-cpufreq.c
>>>>>> @@ -0,0 +1,220 @@
>>>>>> +/*
>>>>>> + *  Copyright (C) 2014 GlobalLogic Inc.
>>>>>
>>>>> A part of this file has been copied from xen/arch/x86/acpi/cpufreq.c. I
>>>>> would keep the copyright from this file and add yours.
>>>> I'll do this in the next patch-set.
>>>>
>>>>> Maybe we could share the initialization code (and others parts?) with
>>>>> this file? For instance the structure looks the same...
>>>> I don't think that we could simple share the initialization code and
>>>> others parts.
>>>> A lot of code looks the same. But I've introduced a new structure
>>>> hwdom_cpufreq_data which has different field names (non-ACPI meaning).
>>>
>>> Except the name, the type of each fields are the same (except the third
>>> one which doesn't exist here).
>>>
>>> IHMO, those names could be renamed if it could avoid to duplicate tens
>>> lines of code.
>>>
>>> It would be easier for maintaining the code later.
>> In this case I'll back to the 'acpi_cpufreq_data' structure and I'll try
>> to create an additional file with common code for those drivers.
>
> I'm not the maintainers, so I would wait any input from maintainers
> before doing a such big change :).
In this case, I'll leave a separate file (without sharing of the code).
If there will be any input from maintainers I'll rework this patch.

> Regards,
>
> --
> Julien Grall

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