[PATCH 1/1] Clocksource: Move the Hyper-V clocksource driver out of staging

[PATCH 1/1] Clocksource: Move the Hyper-V clocksource driver out of staging

[K. Y. Srinivasan]

Move the Hyper-V clocksource driver out of staging.

Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Signed-off-by: Hank Janssen <hjanssen@microsoft.com>
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
---
 drivers/clocksource/Makefile        |    1 +
 drivers/clocksource/hv_timesource.c |  102 +++++++++++++++++++++++++++++++++++
 drivers/staging/hv/Makefile         |    2 +-
 drivers/staging/hv/hv_timesource.c  |  102 -----------------------------------
 4 files changed, 104 insertions(+), 103 deletions(-)
 create mode 100644 drivers/clocksource/hv_timesource.c
 delete mode 100644 drivers/staging/hv/hv_timesource.c

diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
index be61ece..ea44327 100644
--- a/drivers/clocksource/Makefile
+++ b/drivers/clocksource/Makefile
@@ -6,3 +6,4 @@ obj-$(CONFIG_CS5535_CLOCK_EVENT_SRC)	+= cs5535-clockevt.o
 obj-$(CONFIG_SH_TIMER_CMT)	+= sh_cmt.o
 obj-$(CONFIG_SH_TIMER_MTU2)	+= sh_mtu2.o
 obj-$(CONFIG_SH_TIMER_TMU)	+= sh_tmu.o
+obj-$(CONFIG_HYPERV)		+= hv_timesource.o
diff --git a/drivers/clocksource/hv_timesource.c b/drivers/clocksource/hv_timesource.c
new file mode 100644
index 0000000..0efb049
--- /dev/null
+++ b/drivers/clocksource/hv_timesource.c
@@ -0,0 +1,102 @@
+/*
+ * A clocksource for Linux running on HyperV.
+ *
+ *
+ * Copyright (C) 2010, Novell, Inc.
+ * Author : K. Y. Srinivasan <ksrinivasan@novell.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY OR FITNESS FOR A PARTICULAR PURPOSE, GOOD TITLE or
+ * NON INFRINGEMENT.  See the GNU General Public License for more
+ * details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ */
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/version.h>
+#include <linux/clocksource.h>
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/pci.h>
+#include <linux/dmi.h>
+#include <asm/hyperv.h>
+#include <asm/mshyperv.h>
+#include <asm/hypervisor.h>
+
+#define HV_CLOCK_SHIFT	22
+
+static cycle_t read_hv_clock(struct clocksource *arg)
+{
+	cycle_t current_tick;
+	/*
+	 * Read the partition counter to get the current tick count. This count
+	 * is set to 0 when the partition is created and is incremented in
+	 * 100 nanosecond units.
+	 */
+	rdmsrl(HV_X64_MSR_TIME_REF_COUNT, current_tick);
+	return current_tick;
+}
+
+static struct clocksource hyperv_cs = {
+	.name           = "hyperv_clocksource",
+	.rating         = 400, /* use this when running on Hyperv*/
+	.read           = read_hv_clock,
+	.mask           = CLOCKSOURCE_MASK(64),
+	/*
+	 * The time ref counter in HyperV is in 100ns units.
+	 * The definition of mult is:
+	 * mult/2^shift = ns/cyc = 100
+	 * mult = (100 << shift)
+	 */
+	.mult           = (100 << HV_CLOCK_SHIFT),
+	.shift          = HV_CLOCK_SHIFT,
+};
+
+static const struct dmi_system_id __initconst
+hv_timesource_dmi_table[] __maybe_unused  = {
+	{
+		.ident = "Hyper-V",
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "Microsoft Corporation"),
+			DMI_MATCH(DMI_PRODUCT_NAME, "Virtual Machine"),
+			DMI_MATCH(DMI_BOARD_NAME, "Virtual Machine"),
+		},
+	},
+	{ },
+};
+MODULE_DEVICE_TABLE(dmi, hv_timesource_dmi_table);
+
+static const struct pci_device_id __initconst
+hv_timesource_pci_table[] __maybe_unused = {
+	{ PCI_DEVICE(0x1414, 0x5353) }, /* VGA compatible controller */
+	{ 0 }
+};
+MODULE_DEVICE_TABLE(pci, hv_timesource_pci_table);
+
+
+static int __init init_hv_clocksource(void)
+{
+	if ((x86_hyper != &x86_hyper_ms_hyperv) ||
+		!(ms_hyperv.features & HV_X64_MSR_TIME_REF_COUNT_AVAILABLE))
+		return -ENODEV;
+
+	if (!dmi_check_system(hv_timesource_dmi_table))
+		return -ENODEV;
+
+	pr_info("Registering HyperV clock source\n");
+	return clocksource_register(&hyperv_cs);
+}
+
+module_init(init_hv_clocksource);
+MODULE_DESCRIPTION("HyperV based clocksource");
+MODULE_AUTHOR("K. Y. Srinivasan <ksrinivasan@novell.com>");
+MODULE_LICENSE("GPL");
diff --git a/drivers/staging/hv/Makefile b/drivers/staging/hv/Makefile
index 3004674..14e3c6a 100644
--- a/drivers/staging/hv/Makefile
+++ b/drivers/staging/hv/Makefile
@@ -1,4 +1,4 @@
-obj-$(CONFIG_HYPERV)		+= hv_vmbus.o hv_timesource.o
+obj-$(CONFIG_HYPERV)		+= hv_vmbus.o
 obj-$(CONFIG_HYPERV_STORAGE)	+= hv_storvsc.o
 obj-$(CONFIG_HYPERV_BLOCK)	+= hv_blkvsc.o
 obj-$(CONFIG_HYPERV_NET)	+= hv_netvsc.o
diff --git a/drivers/staging/hv/hv_timesource.c b/drivers/staging/hv/hv_timesource.c
deleted file mode 100644
index 0efb049..0000000
--- a/drivers/staging/hv/hv_timesource.c
+++ /dev/null
@@ -1,102 +0,0 @@
-/*
- * A clocksource for Linux running on HyperV.
- *
- *
- * Copyright (C) 2010, Novell, Inc.
- * Author : K. Y. Srinivasan <ksrinivasan@novell.com>
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2 as
- * published by the Free Software Foundation.
- *
- * This program is distributed in the hope that it will be useful, but
- * WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY OR FITNESS FOR A PARTICULAR PURPOSE, GOOD TITLE or
- * NON INFRINGEMENT.  See the GNU General Public License for more
- * details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, write to the Free Software
- * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
- *
- */
-#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
-
-#include <linux/version.h>
-#include <linux/clocksource.h>
-#include <linux/init.h>
-#include <linux/module.h>
-#include <linux/pci.h>
-#include <linux/dmi.h>
-#include <asm/hyperv.h>
-#include <asm/mshyperv.h>
-#include <asm/hypervisor.h>
-
-#define HV_CLOCK_SHIFT	22
-
-static cycle_t read_hv_clock(struct clocksource *arg)
-{
-	cycle_t current_tick;
-	/*
-	 * Read the partition counter to get the current tick count. This count
-	 * is set to 0 when the partition is created and is incremented in
-	 * 100 nanosecond units.
-	 */
-	rdmsrl(HV_X64_MSR_TIME_REF_COUNT, current_tick);
-	return current_tick;
-}
-
-static struct clocksource hyperv_cs = {
-	.name           = "hyperv_clocksource",
-	.rating         = 400, /* use this when running on Hyperv*/
-	.read           = read_hv_clock,
-	.mask           = CLOCKSOURCE_MASK(64),
-	/*
-	 * The time ref counter in HyperV is in 100ns units.
-	 * The definition of mult is:
-	 * mult/2^shift = ns/cyc = 100
-	 * mult = (100 << shift)
-	 */
-	.mult           = (100 << HV_CLOCK_SHIFT),
-	.shift          = HV_CLOCK_SHIFT,
-};
-
-static const struct dmi_system_id __initconst
-hv_timesource_dmi_table[] __maybe_unused  = {
-	{
-		.ident = "Hyper-V",
-		.matches = {
-			DMI_MATCH(DMI_SYS_VENDOR, "Microsoft Corporation"),
-			DMI_MATCH(DMI_PRODUCT_NAME, "Virtual Machine"),
-			DMI_MATCH(DMI_BOARD_NAME, "Virtual Machine"),
-		},
-	},
-	{ },
-};
-MODULE_DEVICE_TABLE(dmi, hv_timesource_dmi_table);
-
-static const struct pci_device_id __initconst
-hv_timesource_pci_table[] __maybe_unused = {
-	{ PCI_DEVICE(0x1414, 0x5353) }, /* VGA compatible controller */
-	{ 0 }
-};
-MODULE_DEVICE_TABLE(pci, hv_timesource_pci_table);
-
-
-static int __init init_hv_clocksource(void)
-{
-	if ((x86_hyper != &x86_hyper_ms_hyperv) ||
-		!(ms_hyperv.features & HV_X64_MSR_TIME_REF_COUNT_AVAILABLE))
-		return -ENODEV;
-
-	if (!dmi_check_system(hv_timesource_dmi_table))
-		return -ENODEV;
-
-	pr_info("Registering HyperV clock source\n");
-	return clocksource_register(&hyperv_cs);
-}
-
-module_init(init_hv_clocksource);
-MODULE_DESCRIPTION("HyperV based clocksource");
-MODULE_AUTHOR("K. Y. Srinivasan <ksrinivasan@novell.com>");
-MODULE_LICENSE("GPL");
-- 
1.7.4.1

Re: [PATCH 1/1] Clocksource: Move the Hyper-V clocksource driver out of staging

[Thomas Gleixner]

On Mon, 23 May 2011, K. Y. Srinivasan wrote:
> +
> +#include <linux/version.h>
> +#include <linux/clocksource.h>
> +#include <linux/init.h>
> +#include <linux/module.h>
> +#include <linux/pci.h>
> +#include <linux/dmi.h>
> +#include <asm/hyperv.h>
> +#include <asm/mshyperv.h>
> +#include <asm/hypervisor.h>
> +
> +#define HV_CLOCK_SHIFT	22

Please do not use hard coded conversion factors. See below

> +static cycle_t read_hv_clock(struct clocksource *arg)
> +{
> +	cycle_t current_tick;
> +	/*
> +	 * Read the partition counter to get the current tick count. This count
> +	 * is set to 0 when the partition is created and is incremented in
> +	 * 100 nanosecond units.
> +	 */
> +	rdmsrl(HV_X64_MSR_TIME_REF_COUNT, current_tick);
> +	return current_tick;
> +}
> +
> +static struct clocksource hyperv_cs = {
> +	.name           = "hyperv_clocksource",
> +	.rating         = 400, /* use this when running on Hyperv*/
> +	.read           = read_hv_clock,
> +	.mask           = CLOCKSOURCE_MASK(64),
> +	/*
> +	 * The time ref counter in HyperV is in 100ns units.
> +	 * The definition of mult is:
> +	 * mult/2^shift = ns/cyc = 100
> +	 * mult = (100 << shift)
> +	 */
> +	.mult           = (100 << HV_CLOCK_SHIFT),
> +	.shift          = HV_CLOCK_SHIFT,
> +};
> +
> +static const struct dmi_system_id __initconst
> +hv_timesource_dmi_table[] __maybe_unused  = {
> +	{
> +		.ident = "Hyper-V",
> +		.matches = {
> +			DMI_MATCH(DMI_SYS_VENDOR, "Microsoft Corporation"),
> +			DMI_MATCH(DMI_PRODUCT_NAME, "Virtual Machine"),
> +			DMI_MATCH(DMI_BOARD_NAME, "Virtual Machine"),
> +		},
> +	},
> +	{ },
> +};
> +MODULE_DEVICE_TABLE(dmi, hv_timesource_dmi_table);
> +
> +static const struct pci_device_id __initconst
> +hv_timesource_pci_table[] __maybe_unused = {
> +	{ PCI_DEVICE(0x1414, 0x5353) }, /* VGA compatible controller */
> +	{ 0 }
> +};
> +MODULE_DEVICE_TABLE(pci, hv_timesource_pci_table);

This pci_id table is unused. What's the purpose ?

> +
> +static int __init init_hv_clocksource(void)
> +{
> +	if ((x86_hyper != &x86_hyper_ms_hyperv) ||
> +		!(ms_hyperv.features & HV_X64_MSR_TIME_REF_COUNT_AVAILABLE))
> +		return -ENODEV;

Isn't this a sufficient indicator ?

> +	if (!dmi_check_system(hv_timesource_dmi_table))
> +		return -ENODEV;

Do we really need this additional check? I'd say if x86_hyper ==
x86_hyper_ms_hyperv then failing the DMI check would be more than
silly.

> +	pr_info("Registering HyperV clock source\n");
> +	return clocksource_register(&hyperv_cs);

Please use clocksource_register_hz/khz and get rid of the hard coded
constants.

> +}
> +
> +module_init(init_hv_clocksource);
> +MODULE_DESCRIPTION("HyperV based clocksource");
> +MODULE_AUTHOR("K. Y. Srinivasan <ksrinivasan@novell.com>");
> +MODULE_LICENSE("GPL");

Why do we need to build this as a module?

Thanks,

	tglx

Re: [PATCH 1/1] Clocksource: Move the Hyper-V clocksource driver out of staging

[john stultz]

On Mon, 2011-05-23 at 10:12 -0700, K. Y. Srinivasan wrote:
> Move the Hyper-V clocksource driver out of staging.
> 
> Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> Signed-off-by: Hank Janssen <hjanssen@microsoft.com>
> Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
> ---
>  drivers/clocksource/Makefile        |    1 +
>  drivers/clocksource/hv_timesource.c |  102 +++++++++++++++++++++++++++++++++++
>  drivers/staging/hv/Makefile         |    2 +-
>  drivers/staging/hv/hv_timesource.c  |  102 -----------------------------------
>  4 files changed, 104 insertions(+), 103 deletions(-)
>  create mode 100644 drivers/clocksource/hv_timesource.c
>  delete mode 100644 drivers/staging/hv/hv_timesource.c
> 
> diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
> index be61ece..ea44327 100644
> --- a/drivers/clocksource/Makefile
> +++ b/drivers/clocksource/Makefile
> @@ -6,3 +6,4 @@ obj-$(CONFIG_CS5535_CLOCK_EVENT_SRC)	+= cs5535-clockevt.o
>  obj-$(CONFIG_SH_TIMER_CMT)	+= sh_cmt.o
>  obj-$(CONFIG_SH_TIMER_MTU2)	+= sh_mtu2.o
>  obj-$(CONFIG_SH_TIMER_TMU)	+= sh_tmu.o
> +obj-$(CONFIG_HYPERV)		+= hv_timesource.o
> diff --git a/drivers/clocksource/hv_timesource.c b/drivers/clocksource/hv_timesource.c
> new file mode 100644
> index 0000000..0efb049
> --- /dev/null
> +++ b/drivers/clocksource/hv_timesource.c
> @@ -0,0 +1,102 @@
> +/*
> + * A clocksource for Linux running on HyperV.
> + *
> + *
> + * Copyright (C) 2010, Novell, Inc.
> + * Author : K. Y. Srinivasan <ksrinivasan@novell.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful, but
> + * WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY OR FITNESS FOR A PARTICULAR PURPOSE, GOOD TITLE or
> + * NON INFRINGEMENT.  See the GNU General Public License for more
> + * details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
> + *
> + */
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +
> +#include <linux/version.h>
> +#include <linux/clocksource.h>
> +#include <linux/init.h>
> +#include <linux/module.h>
> +#include <linux/pci.h>
> +#include <linux/dmi.h>
> +#include <asm/hyperv.h>
> +#include <asm/mshyperv.h>
> +#include <asm/hypervisor.h>
> +
> +#define HV_CLOCK_SHIFT	22
> +
> +static cycle_t read_hv_clock(struct clocksource *arg)
> +{
> +	cycle_t current_tick;
> +	/*
> +	 * Read the partition counter to get the current tick count. This count
> +	 * is set to 0 when the partition is created and is incremented in
> +	 * 100 nanosecond units.
> +	 */
> +	rdmsrl(HV_X64_MSR_TIME_REF_COUNT, current_tick);
> +	return current_tick;
> +}
> +
> +static struct clocksource hyperv_cs = {
> +	.name           = "hyperv_clocksource",
> +	.rating         = 400, /* use this when running on Hyperv*/
> +	.read           = read_hv_clock,
> +	.mask           = CLOCKSOURCE_MASK(64),
> +	/*
> +	 * The time ref counter in HyperV is in 100ns units.
> +	 * The definition of mult is:
> +	 * mult/2^shift = ns/cyc = 100
> +	 * mult = (100 << shift)
> +	 */
> +	.mult           = (100 << HV_CLOCK_SHIFT),
> +	.shift          = HV_CLOCK_SHIFT,

The mult/shift assignments can be dropped. Please use
clocksource_register_hz/khz() which will assign mult/shift for you.

Otherwise it looks pretty straightforward. 


> +module_init(init_hv_clocksource);
> +MODULE_DESCRIPTION("HyperV based clocksource");
> +MODULE_AUTHOR("K. Y. Srinivasan <ksrinivasan@novell.com>");
> +MODULE_LICENSE("GPL");

One other nit: Should this email address be updated to your current one?

thanks
-john

Re: [PATCH 1/1] Clocksource: Move the Hyper-V clocksource driver out of staging

[Christoph Hellwig]

> +#include <linux/version.h>

This one shouldn't be needed.

> +#include <asm/hyperv.h>
> +#include <asm/mshyperv.h>

not really a review for this driver, but what's the purpose if having
these two headers?

Shouldn't the Kconfig entry also move from drivers/staging to
arch/x86 towards the other clocksources?

RE: [PATCH 1/1] Clocksource: Move the Hyper-V clocksource driver out of staging

[KY Srinivasan]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 4707 bytes --]



> -----Original Message-----
> From: john stultz [mailto:johnstul@us.ibm.com]
> Sent: Monday, May 23, 2011 2:40 PM
> To: KY Srinivasan
> Cc: gregkh@suse.de; linux-kernel@vger.kernel.org;
> devel@linuxdriverproject.org; tglx@linutronix.de; hch@infradead.org; Hank
> Janssen; Haiyang Zhang
> Subject: Re: [PATCH 1/1] Clocksource: Move the Hyper-V clocksource driver out
> of staging
> 
> On Mon, 2011-05-23 at 10:12 -0700, K. Y. Srinivasan wrote:
> > Move the Hyper-V clocksource driver out of staging.
> >
> > Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> > Signed-off-by: Hank Janssen <hjanssen@microsoft.com>
> > Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
> > ---
> >  drivers/clocksource/Makefile        |    1 +
> >  drivers/clocksource/hv_timesource.c |  102
> +++++++++++++++++++++++++++++++++++
> >  drivers/staging/hv/Makefile         |    2 +-
> >  drivers/staging/hv/hv_timesource.c  |  102 -----------------------------------
> >  4 files changed, 104 insertions(+), 103 deletions(-)
> >  create mode 100644 drivers/clocksource/hv_timesource.c
> >  delete mode 100644 drivers/staging/hv/hv_timesource.c
> >
> > diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
> > index be61ece..ea44327 100644
> > --- a/drivers/clocksource/Makefile
> > +++ b/drivers/clocksource/Makefile
> > @@ -6,3 +6,4 @@ obj-$(CONFIG_CS5535_CLOCK_EVENT_SRC)	+= cs5535-
> clockevt.o
> >  obj-$(CONFIG_SH_TIMER_CMT)	+= sh_cmt.o
> >  obj-$(CONFIG_SH_TIMER_MTU2)	+= sh_mtu2.o
> >  obj-$(CONFIG_SH_TIMER_TMU)	+= sh_tmu.o
> > +obj-$(CONFIG_HYPERV)		+= hv_timesource.o
> > diff --git a/drivers/clocksource/hv_timesource.c
> b/drivers/clocksource/hv_timesource.c
> > new file mode 100644
> > index 0000000..0efb049
> > --- /dev/null
> > +++ b/drivers/clocksource/hv_timesource.c
> > @@ -0,0 +1,102 @@
> > +/*
> > + * A clocksource for Linux running on HyperV.
> > + *
> > + *
> > + * Copyright (C) 2010, Novell, Inc.
> > + * Author : K. Y. Srinivasan <ksrinivasan@novell.com>
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License version 2 as
> > + * published by the Free Software Foundation.
> > + *
> > + * This program is distributed in the hope that it will be useful, but
> > + * WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY OR FITNESS FOR A PARTICULAR PURPOSE, GOOD TITLE
> or
> > + * NON INFRINGEMENT.  See the GNU General Public License for more
> > + * details.
> > + *
> > + * You should have received a copy of the GNU General Public License
> > + * along with this program; if not, write to the Free Software
> > + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
> > + *
> > + */
> > +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> > +
> > +#include <linux/version.h>
> > +#include <linux/clocksource.h>
> > +#include <linux/init.h>
> > +#include <linux/module.h>
> > +#include <linux/pci.h>
> > +#include <linux/dmi.h>
> > +#include <asm/hyperv.h>
> > +#include <asm/mshyperv.h>
> > +#include <asm/hypervisor.h>
> > +
> > +#define HV_CLOCK_SHIFT	22
> > +
> > +static cycle_t read_hv_clock(struct clocksource *arg)
> > +{
> > +	cycle_t current_tick;
> > +	/*
> > +	 * Read the partition counter to get the current tick count. This count
> > +	 * is set to 0 when the partition is created and is incremented in
> > +	 * 100 nanosecond units.
> > +	 */
> > +	rdmsrl(HV_X64_MSR_TIME_REF_COUNT, current_tick);
> > +	return current_tick;
> > +}
> > +
> > +static struct clocksource hyperv_cs = {
> > +	.name           = "hyperv_clocksource",
> > +	.rating         = 400, /* use this when running on Hyperv*/
> > +	.read           = read_hv_clock,
> > +	.mask           = CLOCKSOURCE_MASK(64),
> > +	/*
> > +	 * The time ref counter in HyperV is in 100ns units.
> > +	 * The definition of mult is:
> > +	 * mult/2^shift = ns/cyc = 100
> > +	 * mult = (100 << shift)
> > +	 */
> > +	.mult           = (100 << HV_CLOCK_SHIFT),
> > +	.shift          = HV_CLOCK_SHIFT,
> 
> The mult/shift assignments can be dropped. Please use
> clocksource_register_hz/khz() which will assign mult/shift for you.
> 
> Otherwise it looks pretty straightforward.

Will do; thanks.
> 
> 
> > +module_init(init_hv_clocksource);
> > +MODULE_DESCRIPTION("HyperV based clocksource");
> > +MODULE_AUTHOR("K. Y. Srinivasan <ksrinivasan@novell.com>");
> > +MODULE_LICENSE("GPL");
> 
> One other nit: Should this email address be updated to your current one?

I will update it.

Regards,

K. Y

ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

RE: [PATCH 1/1] Clocksource: Move the Hyper-V clocksource driver out of staging

[KY Srinivasan]

> -----Original Message-----
> From: Thomas Gleixner [mailto:tglx@linutronix.de]
> Sent: Monday, May 23, 2011 2:16 PM
> To: KY Srinivasan
> Cc: gregkh@suse.de; linux-kernel@vger.kernel.org;
> devel@linuxdriverproject.org; johnstul@us.ibm.com; hch@infradead.org; Hank
> Janssen; Haiyang Zhang
> Subject: Re: [PATCH 1/1] Clocksource: Move the Hyper-V clocksource driver out
> of staging
> 
> On Mon, 23 May 2011, K. Y. Srinivasan wrote:
> > +
> > +#include <linux/version.h>
> > +#include <linux/clocksource.h>
> > +#include <linux/init.h>
> > +#include <linux/module.h>
> > +#include <linux/pci.h>
> > +#include <linux/dmi.h>
> > +#include <asm/hyperv.h>
> > +#include <asm/mshyperv.h>
> > +#include <asm/hypervisor.h>
> > +
> > +#define HV_CLOCK_SHIFT	22
> 
> Please do not use hard coded conversion factors. See below

I will fix this.
> 
> > +static cycle_t read_hv_clock(struct clocksource *arg)
> > +{
> > +	cycle_t current_tick;
> > +	/*
> > +	 * Read the partition counter to get the current tick count. This count
> > +	 * is set to 0 when the partition is created and is incremented in
> > +	 * 100 nanosecond units.
> > +	 */
> > +	rdmsrl(HV_X64_MSR_TIME_REF_COUNT, current_tick);
> > +	return current_tick;
> > +}
> > +
> > +static struct clocksource hyperv_cs = {
> > +	.name           = "hyperv_clocksource",
> > +	.rating         = 400, /* use this when running on Hyperv*/
> > +	.read           = read_hv_clock,
> > +	.mask           = CLOCKSOURCE_MASK(64),
> > +	/*
> > +	 * The time ref counter in HyperV is in 100ns units.
> > +	 * The definition of mult is:
> > +	 * mult/2^shift = ns/cyc = 100
> > +	 * mult = (100 << shift)
> > +	 */
> > +	.mult           = (100 << HV_CLOCK_SHIFT),
> > +	.shift          = HV_CLOCK_SHIFT,
> > +};
> > +
> > +static const struct dmi_system_id __initconst
> > +hv_timesource_dmi_table[] __maybe_unused  = {
> > +	{
> > +		.ident = "Hyper-V",
> > +		.matches = {
> > +			DMI_MATCH(DMI_SYS_VENDOR, "Microsoft
> Corporation"),
> > +			DMI_MATCH(DMI_PRODUCT_NAME, "Virtual Machine"),
> > +			DMI_MATCH(DMI_BOARD_NAME, "Virtual Machine"),
> > +		},
> > +	},
> > +	{ },
> > +};
> > +MODULE_DEVICE_TABLE(dmi, hv_timesource_dmi_table);
> > +
> > +static const struct pci_device_id __initconst
> > +hv_timesource_pci_table[] __maybe_unused = {
> > +	{ PCI_DEVICE(0x1414, 0x5353) }, /* VGA compatible controller */
> > +	{ 0 }
> > +};
> > +MODULE_DEVICE_TABLE(pci, hv_timesource_pci_table);
> 
> This pci_id table is unused. What's the purpose ?

Both the PCI and DMI tables were defined to ensure autoloading
> 
> > +
> > +static int __init init_hv_clocksource(void)
> > +{
> > +	if ((x86_hyper != &x86_hyper_ms_hyperv) ||
> > +		!(ms_hyperv.features &
> HV_X64_MSR_TIME_REF_COUNT_AVAILABLE))
> > +		return -ENODEV;
> 
> Isn't this a sufficient indicator ?

This ensures we are running on Hyper-V and for that this check is sufficient.

> 
> > +	if (!dmi_check_system(hv_timesource_dmi_table))
> > +		return -ENODEV;
> 
> Do we really need this additional check? I'd say if x86_hyper ==
> x86_hyper_ms_hyperv then failing the DMI check would be more than
> silly.

Both the PCI and DMI tables were introduced by Greg to support autoloading.
I will see if I can clean this up.

> 
> > +	pr_info("Registering HyperV clock source\n");
> > +	return clocksource_register(&hyperv_cs);
> 
> Please use clocksource_register_hz/khz and get rid of the hard coded
> constants.

Will do.
> 
> > +}
> > +
> > +module_init(init_hv_clocksource);
> > +MODULE_DESCRIPTION("HyperV based clocksource");
> > +MODULE_AUTHOR("K. Y. Srinivasan <ksrinivasan@novell.com>");
> > +MODULE_LICENSE("GPL");
> 
> Why do we need to build this as a module?

No particular reason. This is the way, I had it in the staging directory.
Do you want me to build this as part of the kernel? I would then not have
to worry about auto-loading issues.

Regards,

K. Y 

RE: [PATCH 1/1] Clocksource: Move the Hyper-V clocksource driver out of staging

[KY Srinivasan]

> -----Original Message-----
> From: Christoph Hellwig [mailto:hch@infradead.org]
> Sent: Monday, May 23, 2011 2:43 PM
> To: KY Srinivasan
> Cc: gregkh@suse.de; linux-kernel@vger.kernel.org;
> devel@linuxdriverproject.org; tglx@linutronix.de; johnstul@us.ibm.com;
> hch@infradead.org; Hank Janssen; Haiyang Zhang
> Subject: Re: [PATCH 1/1] Clocksource: Move the Hyper-V clocksource driver out
> of staging
> 
> > +#include <linux/version.h>
> 
> This one shouldn't be needed.

Correct; I will fix this.
> 
> > +#include <asm/hyperv.h>
> > +#include <asm/mshyperv.h>
> 
> not really a review for this driver, but what's the purpose if having
> these two headers?

These header files have the defines for checking if we are running on 
Hyper-V.

> 
> Shouldn't the Kconfig entry also move from drivers/staging to
> arch/x86 towards the other clocksources?

Currently this driver is built if  HYPERV is defined. I agree with you
that I should have Kconfig entry in arch/x86. Since the rest of the 
Hyper-V drivers are still in staging, I am thinking that perhaps we should 
have a different config switch - HYPERV_CLKSRC. I will go ahead and 
spin a new version of this patch with this change.

Regards,

K. Y

RE: [PATCH 1/1] Clocksource: Move the Hyper-V clocksource driver out of staging

[Thomas Gleixner]

On Mon, 23 May 2011, KY Srinivasan wrote:
> > > +module_init(init_hv_clocksource);
> > > +MODULE_DESCRIPTION("HyperV based clocksource");
> > > +MODULE_AUTHOR("K. Y. Srinivasan <ksrinivasan@novell.com>");
> > > +MODULE_LICENSE("GPL");
> > 
> > Why do we need to build this as a module?
> 
> No particular reason. This is the way, I had it in the staging directory.
> Do you want me to build this as part of the kernel? I would then not have
> to worry about auto-loading issues.

If we get rid of the DMI/PCI stuff then the whole module code is
larger than the real code. So no point in building it modular, just
make it depend on CONFIG_WHATEVER_MEANS_HYPERV

Thanks,

	tglx

[PATCH 1/1] Clocksource: Move the Hyper-V clocksource driver out of staging

[K. Y. Srinivasan]

Move the Hyper-V clocksource driver out of the staging area. Given the size of
this driver, this driver is being built as part of the kernel.

Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Signed-off-by: Hank Janssen <hjanssen@microsoft.com>
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
---
 arch/x86/Kconfig                    |    3 +
 drivers/clocksource/Makefile        |    1 +
 drivers/clocksource/hv_timesource.c |   65 ++++++++++++++++++++++
 drivers/staging/hv/Makefile         |    2 +-
 drivers/staging/hv/hv_timesource.c  |  102 -----------------------------------
 5 files changed, 70 insertions(+), 103 deletions(-)
 create mode 100644 drivers/clocksource/hv_timesource.c
 delete mode 100644 drivers/staging/hv/hv_timesource.c

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index cc6c53a..85e9644 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -595,6 +595,9 @@ config X86_CYCLONE_TIMER
 	def_bool y
 	depends on X86_32_NON_STANDARD
 
+config HYPERV_CLKSRC
+	def_bool y
+
 source "arch/x86/Kconfig.cpu"
 
 config HPET_TIMER
diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
index be61ece..e398746 100644
--- a/drivers/clocksource/Makefile
+++ b/drivers/clocksource/Makefile
@@ -6,3 +6,4 @@ obj-$(CONFIG_CS5535_CLOCK_EVENT_SRC)	+= cs5535-clockevt.o
 obj-$(CONFIG_SH_TIMER_CMT)	+= sh_cmt.o
 obj-$(CONFIG_SH_TIMER_MTU2)	+= sh_mtu2.o
 obj-$(CONFIG_SH_TIMER_TMU)	+= sh_tmu.o
+obj-$(CONFIG_HYPERV_CLKSRC)		+= hv_timesource.o
diff --git a/drivers/clocksource/hv_timesource.c b/drivers/clocksource/hv_timesource.c
new file mode 100644
index 0000000..bcf3dc7
--- /dev/null
+++ b/drivers/clocksource/hv_timesource.c
@@ -0,0 +1,65 @@
+/*
+ * A clocksource for Linux running on Hyper-V.
+ *
+ *
+ * Copyright (C) 2010, Novell, Inc.
+ * Author : K. Y. Srinivasan <kys@microsoft.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY OR FITNESS FOR A PARTICULAR PURPOSE, GOOD TITLE or
+ * NON INFRINGEMENT.  See the GNU General Public License for more
+ * details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ */
+
+#include <linux/clocksource.h>
+#include <linux/time.h>
+#include <asm/hyperv.h>
+#include <asm/mshyperv.h>
+#include <asm/hypervisor.h>
+
+
+static cycle_t read_hv_clock(struct clocksource *arg)
+{
+	cycle_t current_tick;
+	/*
+	 * Read the partition counter to get the current tick count. This count
+	 * is set to 0 when the partition is created and is incremented in
+	 * 100 nanosecond units.
+	 */
+	rdmsrl(HV_X64_MSR_TIME_REF_COUNT, current_tick);
+	return current_tick;
+}
+
+static struct clocksource hyperv_cs = {
+	.name           = "hyperv_clocksource",
+	.rating         = 400, /* use this when running on Hyperv*/
+	.read           = read_hv_clock,
+	.mask           = CLOCKSOURCE_MASK(64),
+};
+
+static int __init init_hv_clocksource(void)
+{
+	unsigned long	hv_period = 100; /* 100 ns granularity clocksource */
+	u32		hv_freq;
+	if ((x86_hyper != &x86_hyper_ms_hyperv) ||
+		!(ms_hyperv.features & HV_X64_MSR_TIME_REF_COUNT_AVAILABLE))
+		return -ENODEV;
+
+	hv_freq = NSEC_PER_SEC;
+	do_div(hv_freq, hv_period);
+
+	printk(KERN_INFO "Registering Hyper-V clock source\n");
+	return clocksource_register_hz(&hyperv_cs, hv_freq);
+}
+
+core_initcall(init_hv_clocksource);
diff --git a/drivers/staging/hv/Makefile b/drivers/staging/hv/Makefile
index 3004674..14e3c6a 100644
--- a/drivers/staging/hv/Makefile
+++ b/drivers/staging/hv/Makefile
@@ -1,4 +1,4 @@
-obj-$(CONFIG_HYPERV)		+= hv_vmbus.o hv_timesource.o
+obj-$(CONFIG_HYPERV)		+= hv_vmbus.o
 obj-$(CONFIG_HYPERV_STORAGE)	+= hv_storvsc.o
 obj-$(CONFIG_HYPERV_BLOCK)	+= hv_blkvsc.o
 obj-$(CONFIG_HYPERV_NET)	+= hv_netvsc.o
diff --git a/drivers/staging/hv/hv_timesource.c b/drivers/staging/hv/hv_timesource.c
deleted file mode 100644
index 0efb049..0000000
--- a/drivers/staging/hv/hv_timesource.c
+++ /dev/null
@@ -1,102 +0,0 @@
-/*
- * A clocksource for Linux running on HyperV.
- *
- *
- * Copyright (C) 2010, Novell, Inc.
- * Author : K. Y. Srinivasan <ksrinivasan@novell.com>
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2 as
- * published by the Free Software Foundation.
- *
- * This program is distributed in the hope that it will be useful, but
- * WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY OR FITNESS FOR A PARTICULAR PURPOSE, GOOD TITLE or
- * NON INFRINGEMENT.  See the GNU General Public License for more
- * details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, write to the Free Software
- * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
- *
- */
-#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
-
-#include <linux/version.h>
-#include <linux/clocksource.h>
-#include <linux/init.h>
-#include <linux/module.h>
-#include <linux/pci.h>
-#include <linux/dmi.h>
-#include <asm/hyperv.h>
-#include <asm/mshyperv.h>
-#include <asm/hypervisor.h>
-
-#define HV_CLOCK_SHIFT	22
-
-static cycle_t read_hv_clock(struct clocksource *arg)
-{
-	cycle_t current_tick;
-	/*
-	 * Read the partition counter to get the current tick count. This count
-	 * is set to 0 when the partition is created and is incremented in
-	 * 100 nanosecond units.
-	 */
-	rdmsrl(HV_X64_MSR_TIME_REF_COUNT, current_tick);
-	return current_tick;
-}
-
-static struct clocksource hyperv_cs = {
-	.name           = "hyperv_clocksource",
-	.rating         = 400, /* use this when running on Hyperv*/
-	.read           = read_hv_clock,
-	.mask           = CLOCKSOURCE_MASK(64),
-	/*
-	 * The time ref counter in HyperV is in 100ns units.
-	 * The definition of mult is:
-	 * mult/2^shift = ns/cyc = 100
-	 * mult = (100 << shift)
-	 */
-	.mult           = (100 << HV_CLOCK_SHIFT),
-	.shift          = HV_CLOCK_SHIFT,
-};
-
-static const struct dmi_system_id __initconst
-hv_timesource_dmi_table[] __maybe_unused  = {
-	{
-		.ident = "Hyper-V",
-		.matches = {
-			DMI_MATCH(DMI_SYS_VENDOR, "Microsoft Corporation"),
-			DMI_MATCH(DMI_PRODUCT_NAME, "Virtual Machine"),
-			DMI_MATCH(DMI_BOARD_NAME, "Virtual Machine"),
-		},
-	},
-	{ },
-};
-MODULE_DEVICE_TABLE(dmi, hv_timesource_dmi_table);
-
-static const struct pci_device_id __initconst
-hv_timesource_pci_table[] __maybe_unused = {
-	{ PCI_DEVICE(0x1414, 0x5353) }, /* VGA compatible controller */
-	{ 0 }
-};
-MODULE_DEVICE_TABLE(pci, hv_timesource_pci_table);
-
-
-static int __init init_hv_clocksource(void)
-{
-	if ((x86_hyper != &x86_hyper_ms_hyperv) ||
-		!(ms_hyperv.features & HV_X64_MSR_TIME_REF_COUNT_AVAILABLE))
-		return -ENODEV;
-
-	if (!dmi_check_system(hv_timesource_dmi_table))
-		return -ENODEV;
-
-	pr_info("Registering HyperV clock source\n");
-	return clocksource_register(&hyperv_cs);
-}
-
-module_init(init_hv_clocksource);
-MODULE_DESCRIPTION("HyperV based clocksource");
-MODULE_AUTHOR("K. Y. Srinivasan <ksrinivasan@novell.com>");
-MODULE_LICENSE("GPL");
-- 
1.7.4.1

Re: [PATCH 1/1] Clocksource: Move the Hyper-V clocksource driver out of staging

[Thomas Gleixner]

On Tue, 24 May 2011, K. Y. Srinivasan wrote:
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -595,6 +595,9 @@ config X86_CYCLONE_TIMER
>  	def_bool y
>  	depends on X86_32_NON_STANDARD
>  
> +config HYPERV_CLKSRC
> +	def_bool y

Errm, why do we need another random config switch for every other
feature of hyperv?

> +#include <linux/clocksource.h>
> +#include <linux/time.h>

Why do you need time.h?

> +#include <asm/hyperv.h>
> +#include <asm/mshyperv.h>
> +#include <asm/hypervisor.h>

Can we please have one sensible asm/hyperwtf.h include for all of this ?

> +
> +static cycle_t read_hv_clock(struct clocksource *arg)
> +{
> +	cycle_t current_tick;
> +	/*
> +	 * Read the partition counter to get the current tick count. This count
> +	 * is set to 0 when the partition is created and is incremented in
> +	 * 100 nanosecond units.
> +	 */

Please move that comment above the function. That's really irritating
to read.

> +	rdmsrl(HV_X64_MSR_TIME_REF_COUNT, current_tick);
> +	return current_tick;
> +}
> +
> +static struct clocksource hyperv_cs = {
> +	.name           = "hyperv_clocksource",
> +	.rating         = 400, /* use this when running on Hyperv*/

That tail comment is pretty useless.

> +	.read           = read_hv_clock,
> +	.mask           = CLOCKSOURCE_MASK(64),
> +};
> +
> +static int __init init_hv_clocksource(void)
> +{
> +	unsigned long	hv_period = 100; /* 100 ns granularity clocksource */

  unsigned long period_ns = 100;

would make the horrible tail comment go away and make it obvious to
the reader what the variable is for. Also please stop adding extra
useless noise to local variables by adding hv_ or whatever the heck to
them.

> +	u32		hv_freq;

Newline between declarations and code please.

> +	if ((x86_hyper != &x86_hyper_ms_hyperv) ||
> +		!(ms_hyperv.features & HV_X64_MSR_TIME_REF_COUNT_AVAILABLE))
> +		return -ENODEV;
> +
> +	hv_freq = NSEC_PER_SEC;
> +	do_div(hv_freq, hv_period);

ROTFL. Do you really need runtime evaluation of 10^9/10^2 ?

#define HV_CLK_FREQ	(NSEC_PER_SEC/100)

would solve that problem with 5 lines less source code and a even
larger reduction of binary size.

> +
> +	printk(KERN_INFO "Registering Hyper-V clock source\n");

How is that interesting ?

> +	return clocksource_register_hz(&hyperv_cs, hv_freq);
> +}

And if you remove that useless stuff then the ten remaining lines
should go to arch/x86/ into a file which will contain more than those
ten lines. It's pretty unlikely that anything else than MSHV will
reuse that code.

Thanks,

	tglx

RE: [PATCH 1/1] Clocksource: Move the Hyper-V clocksource driver out of staging

[KY Srinivasan]

> -----Original Message-----
> From: Thomas Gleixner [mailto:tglx@linutronix.de]
> Sent: Tuesday, May 24, 2011 6:43 PM
> To: KY Srinivasan
> Cc: gregkh@suse.de; linux-kernel@vger.kernel.org;
> devel@linuxdriverproject.org; johnstul@us.ibm.com; hch@infradead.org; Hank
> Janssen; Haiyang Zhang
> Subject: Re: [PATCH 1/1] Clocksource: Move the Hyper-V clocksource driver out
> of staging
> 
> On Tue, 24 May 2011, K. Y. Srinivasan wrote:
> > --- a/arch/x86/Kconfig
> > +++ b/arch/x86/Kconfig
> > @@ -595,6 +595,9 @@ config X86_CYCLONE_TIMER
> >  	def_bool y
> >  	depends on X86_32_NON_STANDARD
> >
> > +config HYPERV_CLKSRC
> > +	def_bool y
> 
> Errm, why do we need another random config switch for every other
> feature of hyperv?
> 
> > +#include <linux/clocksource.h>
> > +#include <linux/time.h>
> 
> Why do you need time.h?

For the definition of NSEC_PER_SEC
 
> 
> > +#include <asm/hyperv.h>
> > +#include <asm/mshyperv.h>
> > +#include <asm/hypervisor.h>
> 
> Can we please have one sensible asm/hyperwtf.h include for all of this ?

Well,  hyperv.h has all the Hyper-V specific defines. When mshyperv.h was 
introduced, I tried very hard to merge it with hyperv.h and I was voted down.
With your help, maybe we can merge mshyperv.h and hyperv.h. Greg may remember
the arguments that kept these two related files separate.
 
> 
> > +
> > +static cycle_t read_hv_clock(struct clocksource *arg)
> > +{
> > +	cycle_t current_tick;
> > +	/*
> > +	 * Read the partition counter to get the current tick count. This count
> > +	 * is set to 0 when the partition is created and is incremented in
> > +	 * 100 nanosecond units.
> > +	 */
> 
> Please move that comment above the function. That's really irritating
> to read.
> 
> > +	rdmsrl(HV_X64_MSR_TIME_REF_COUNT, current_tick);
> > +	return current_tick;
> > +}
> > +
> > +static struct clocksource hyperv_cs = {
> > +	.name           = "hyperv_clocksource",
> > +	.rating         = 400, /* use this when running on Hyperv*/
> 
> That tail comment is pretty useless.
> 
> > +	.read           = read_hv_clock,
> > +	.mask           = CLOCKSOURCE_MASK(64),
> > +};
> > +
> > +static int __init init_hv_clocksource(void)
> > +{
> > +	unsigned long	hv_period = 100; /* 100 ns granularity clocksource */
> 
>   unsigned long period_ns = 100;
> 
> would make the horrible tail comment go away and make it obvious to
> the reader what the variable is for. Also please stop adding extra
> useless noise to local variables by adding hv_ or whatever the heck to
> them.
> 
> > +	u32		hv_freq;
> 
> Newline between declarations and code please.
> 
> > +	if ((x86_hyper != &x86_hyper_ms_hyperv) ||
> > +		!(ms_hyperv.features &
> HV_X64_MSR_TIME_REF_COUNT_AVAILABLE))
> > +		return -ENODEV;
> > +
> > +	hv_freq = NSEC_PER_SEC;
> > +	do_div(hv_freq, hv_period);
> 
> ROTFL. Do you really need runtime evaluation of 10^9/10^2 ?
> 
> #define HV_CLK_FREQ	(NSEC_PER_SEC/100)
> 
> would solve that problem with 5 lines less source code and a even
> larger reduction of binary size.
> 
> > +
> > +	printk(KERN_INFO "Registering Hyper-V clock source\n");
> 
> How is that interesting ?
> 
> > +	return clocksource_register_hz(&hyperv_cs, hv_freq);
> > +}
> 
> And if you remove that useless stuff then the ten remaining lines
> should go to arch/x86/ into a file which will contain more than those
> ten lines. It's pretty unlikely that anything else than MSHV will
> reuse that code.

I like the idea of merging this code with some other file under arch/x86/.
I could merge this code into mshyperv.c file that already has hyperv 
specific code. Who would take this patch if I were to merge this 
cleaned up Hyper-V clocksource code into mshyperv.c file under
arch/X86/kernel/cpu.

Thomas, thank you for your patience here.

Regards,

K. Y

RE: [PATCH 1/1] Clocksource: Move the Hyper-V clocksource driver out of staging

[Thomas Gleixner]

On Wed, 25 May 2011, KY Srinivasan wrote:
> I like the idea of merging this code with some other file under arch/x86/.
> I could merge this code into mshyperv.c file that already has hyperv 
> specific code. Who would take this patch if I were to merge this 
> cleaned up Hyper-V clocksource code into mshyperv.c file under
> arch/X86/kernel/cpu.

x86 maintainers perhaps :)