From mboxrd@z Thu Jan  1 00:00:00 1970
From: Julien Grall <julien.grall@linaro.org>
Subject: Re: [RFC PATCH v3 8/8] xen/arm: cpufreq: add
 xen-cpufreq driver
Date: Thu, 23 Oct 2014 17:03:33 +0100
Message-ID: <544926D5.2060007@linaro.org>
References: <1414076916-2231-1-git-send-email-oleksandr.dmytryshyn@globallogic.com>
	<1414076916-2231-9-git-send-email-oleksandr.dmytryshyn@globallogic.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
In-Reply-To: <1414076916-2231-9-git-send-email-oleksandr.dmytryshyn@globallogic.com>
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: Oleksandr Dmytryshyn <oleksandr.dmytryshyn@globallogic.com>, xen-devel@lists.xen.org
Cc: "Rafael J. Wysocki" <rjw@sisk.pl>, Stefano Stabellini <stefano.stabellini@citrix.com>, Tim Deegan <tim@xen.org>, Ian Campbell <ian.campbell@citrix.com>
List-Id: xen-devel@lists.xenproject.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