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

[Oleksandr Dmytryshyn <oleksandr.dmytryshyn@xxxxxxxxxxxxxxx>]

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.

> Regards,
>
> --
> Julien Grall

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