From mboxrd@z Thu Jan 1 00:00:00 1970 From: Oleksandr Dmytryshyn Subject: Re: [RFC PATCH v3 10/12] cpufreq: add hwdom-cpufreq driver Date: Mon, 27 Oct 2014 15:29:58 +0200 Message-ID: References: <1414076867-2148-1-git-send-email-oleksandr.dmytryshyn@globallogic.com> <1414076867-2148-11-git-send-email-oleksandr.dmytryshyn@globallogic.com> <54492FDA.9070508@linaro.org> <544A3BD6.5030503@linaro.org> <544A4F59.1040503@linaro.org> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <544A4F59.1040503@linaro.org> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Julien Grall Cc: Stefano Stabellini , Tim Deegan , Ian Campbell , xen-devel List-Id: xen-devel@lists.xenproject.org On Fri, Oct 24, 2014 at 4:08 PM, Julien Grall wrote: > On 10/24/2014 02:05 PM, Oleksandr Dmytryshyn wrote: >> On Fri, Oct 24, 2014 at 2:45 PM, Julien Grall wrote: >>> On 10/24/2014 11:30 AM, Oleksandr Dmytryshyn wrote: >>>> On Thu, Oct 23, 2014 at 7:42 PM, Julien Grall 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 >>>>>> --- >>>>>> 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