Re: [RFC PATCH 7/7] xen/arm: cpufreq: add cpufreq driver

[Julien Grall <julien.grall@linaro.org>]

Hi Oleksandr,

On 10/07/2014 03:20 PM, Oleksandr Dmytryshyn wrote:
> Xen changes frequencies on CPUs using this driver.
> 
> Signed-off-by: Oleksandr Dmytryshyn <oleksandr.dmytryshyn@globallogic.com>
> ---
>  drivers/xen/Kconfig         |  20 +
>  drivers/xen/Makefile        |   1 +
>  drivers/xen/xen-cpufreq.c   | 882 ++++++++++++++++++++++++++++++++++++++++++++
>  include/xen/interface/xen.h |   1 +
>  4 files changed, 904 insertions(+)
>  create mode 100644 drivers/xen/xen-cpufreq.c
> 
> diff --git a/drivers/xen/Kconfig b/drivers/xen/Kconfig
> index 254a5cc..bb2d3d5 100644
> --- a/drivers/xen/Kconfig
> +++ b/drivers/xen/Kconfig
> @@ -220,6 +220,26 @@ config XEN_ACPI_PROCESSOR
>  	  called xen_acpi_processor  If you do not know what to choose, select
>  	  M here. If the CPUFREQ drivers are built in, select Y here.
>  
> +config XEN_CPUFREQ
> +	bool "Xen Cpufreq driver"
> +	depends on XEN_DOM0 && !CPU_FREQ

Why xen-cpufreq is only available when CPU_FREQ is not present?

As said early, a same kernel should be able to run as Xen domain (DOM0
and other guests) and bare metal.


> +	default n
> +	help
> +	  This driver uploads Power Management information to the Xen
> +	  hypervisor and changes CPUs frequency using CPU Frequency scaling
> +	  drivers.
> +
> +	  To do that the driver uses CPU Frequency scaling drivers to parse
> +	  the Power Management data and uploads said information to the Xen
> +	  hypervisor. Then the Xen hypervisor can select the proper Pxx states.
> +
> +	  Then the Xen hypervisor can change CPUs frequency by giving commands
> +	  via this driver to the CPU Frequency scaling driver.
> +
> +	  To compile this driver as a module, choose M here: the module will be
> +	  called xen_acpi_processor  If you do not know what to choose, select

Is guess you copied the config help from xen_acpi_processor?

> +	  M here. If the CPUFREQ drivers are built in, select Y here.
> +
>  config XEN_MCE_LOG
>  	bool "Xen platform mcelog"
>  	depends on XEN_DOM0 && X86_64 && X86_MCE
> diff --git a/drivers/xen/Makefile b/drivers/xen/Makefile
> index b7c835f..0345d65 100644
> --- a/drivers/xen/Makefile
> +++ b/drivers/xen/Makefile
> @@ -33,6 +33,7 @@ obj-$(CONFIG_XEN_PRIVCMD)		+= xen-privcmd.o
>  obj-$(CONFIG_XEN_ACPI_HOTPLUG_MEMORY)	+= xen-acpi-memhotplug.o
>  obj-$(CONFIG_XEN_ACPI_HOTPLUG_CPU)	+= xen-acpi-cpuhotplug.o
>  obj-$(CONFIG_XEN_ACPI_PROCESSOR)	+= xen-acpi-processor.o
> +obj-$(CONFIG_XEN_CPUFREQ)		+= xen-cpufreq.o

I think this new cpufreq driver should live in drivers/cpufreq, not in
drivers/xen.

>  xen-evtchn-y				:= evtchn.o
>  xen-gntdev-y				:= gntdev.o
>  xen-gntalloc-y				:= gntalloc.o
> diff --git a/drivers/xen/xen-cpufreq.c b/drivers/xen/xen-cpufreq.c
> new file mode 100644
> index 0000000..a0d9adc
> --- /dev/null
> +++ b/drivers/xen/xen-cpufreq.c

[..]

> +#ifdef CONFIG_CPUMASK_OFFSTACK
> +#error CONFIG_CPUMASK_OFFSTACK config should not be used with this driver
> +#endif

hmmm. Why?

Regards,

-- 
Julien Grall