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

[Julien Grall <julien.grall@xxxxxxxxxx>]

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.

Regards,

-- 
Julien Grall

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