Re: [RFC PATCH v3 8/8] xen/arm: cpufreq: add xen-cpufreq driver

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

Hi Oleksandr,

On 10/23/2014 04:08 PM, Oleksandr Dmytryshyn wrote:
> diff --git a/drivers/cpufreq/xen-cpufreq.c b/drivers/cpufreq/xen-cpufreq.c
> new file mode 100644
> index 0000000..a7f0977
> --- /dev/null
> +++ b/drivers/cpufreq/xen-cpufreq.c
> @@ -0,0 +1,889 @@
> +/*
> + *  Copyright (C) 2001 Russell King
> + *            (C) 2002 - 2003 Dominik Brodowski <linux@brodo.de>
> + *
> + *  Oct 2005 - Ashok Raj <ashok.raj@intel.com>
> + *	Added handling for CPU hotplug
> + *  Feb 2006 - Jacob Shin <jacob.shin@amd.com>
> + *	Fix handling for CPU hotplug -- affected CPUs
> + *
> + *           (C) 2014 GlobalLogic Inc.
> + *
> + * Based on drivers/cpufreq/cpufreq.c

It would be interesting to explain roughly what are the major changes
between drivers/cpufreq/cpufreq.c and this drivers.

> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
> + * more details.
> + */
> +
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/notifier.h>
> +#include <linux/types.h>
> +#include <linux/slab.h>
> +#include <linux/mutex.h>
> +#include <linux/irq.h>
> +#include <linux/workqueue.h>
> +#include <linux/cpufreq.h>
> +
> +#include <trace/events/power.h>
> +
> +#include <xen/xen.h>
> +#include <xen/events.h>
> +#include <xen/interface/xen.h>
> +#include <xen/interface/platform.h>
> +#include <xen/interface/sysctl.h>
> +#include <asm/xen/hypercall.h>
> +
> +#include "cpufreq_drv_ops.h"
> +
> +#ifdef CONFIG_CPUMASK_OFFSTACK
> +#error CONFIG_CPUMASK_OFFSTACK config should not be used with this driver
> +#endif

I remembered we talk about this check on a previous version of this
patch. Can't we disable the option in Kconfig rather than throwing a
compilation error?

Or maybe...

> +static int __init xen_cpufreq_init(void)

[..]

> +	xen_nr_cpus = op.u.physinfo.nr_cpus;
> +	if (xen_nr_cpus == 0 || xen_nr_cpus > NR_CPUS) {

Improving this check by also testing nr_cpumask_bits here.

Regards,

-- 
Julien Grall