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: Fri, 24 Oct 2014 16:05:46 +0300 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> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <544A3BD6.5030503@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 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. > Regards, > > -- > Julien Grall