[PATCH 0/7] more HPET fixes and enhancements

[PATCH 0/7] more HPET fixes and enhancements

[Clemens Ladisch]

Another round of HPET bugfixes and cleanups.

 drivers/char/hpet.c |   35 +++++++++++++++++++++--------------
 1 file changed, 21 insertions(+), 14 deletions(-)

[PATCH 1/7] HPET: Fix mmap() of /dev/hpet

[Clemens Ladisch]

From: Keir Fraser <keir@xensource.com>

The address passed to io_remap_pfn_range() in hpet_mmap() does not
need to be converted using __pa(): it is already a physical
address. This bug was found and the patch suggested by Clay Harris.

I introduced this particular bug when making io_remap_pfn_range
changes a few months ago. In fact mmap()ing /dev/hpet has *never*
previously worked: before my changes __pa() was being executed on an
ioremap()ed virtual address, which is also invalid.

Signed-off-by: Keir Fraser <keir@xensource.com>

Index: linux-2.6.13/drivers/char/hpet.c
===================================================================
--- linux-2.6.13.orig/drivers/char/hpet.c	2005-10-03 22:52:30.000000000 +0200
+++ linux-2.6.13/drivers/char/hpet.c	2005-10-03 22:53:09.000000000 +0200
@@ -279,7 +279,6 @@ static int hpet_mmap(struct file *file, 
 
 	vma->vm_flags |= VM_IO;
 	vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot);
-	addr = __pa(addr);
 
 	if (io_remap_pfn_range(vma, vma->vm_start, addr >> PAGE_SHIFT,
 					PAGE_SIZE, vma->vm_page_prot)) {

[PATCH 2/7] HPET: fix HPET_INFO calls from kernel space

[Clemens Ladisch]

From: Clemens Ladisch <clemens@ladisch.de>

Fix a wrong memory access in hpet_ioctl_common().  It was not possible
to use the HPET_INFO ioctl from kernel space because it always called
copy_to_user().

Signed-off-by: Clemens Ladisch <clemens@ladisch.de>

Index: linux-2.6.13/drivers/char/hpet.c
===================================================================
--- linux-2.6.13.orig/drivers/char/hpet.c	2005-10-03 22:53:09.000000000 +0200
+++ linux-2.6.13/drivers/char/hpet.c	2005-10-03 22:53:12.000000000 +0200
@@ -500,8 +500,12 @@ hpet_ioctl_common(struct hpet_dev *devp,
 			    readq(&timer->hpet_config) & Tn_PER_INT_CAP_MASK;
 			info.hi_hpet = devp->hd_hpets->hp_which;
 			info.hi_timer = devp - devp->hd_hpets->hp_dev;
-			if (copy_to_user((void __user *)arg, &info, sizeof(info)))
-				err = -EFAULT;
+			if (kernel)
+				memcpy((void *)arg, &info, sizeof(info));
+			else
+				if (copy_to_user((void __user *)arg, &info,
+						 sizeof(info)))
+					err = -EFAULT;
 			break;
 		}
 	case HPET_EPI:

[PATCH 3/7] HPET: fix division by zero in HPET_INFO

[Clemens Ladisch]

From: Clemens Ladisch <clemens@ladisch.de>

Fix a division by zero that happened when the HPET_INFO ioctl was
called before a timer frequency had been set.

Signed-off-by: Clemens Ladisch <clemens@ladisch.de>

Index: linux-2.6.13/drivers/char/hpet.c
===================================================================
--- linux-2.6.13.orig/drivers/char/hpet.c	2005-10-03 22:53:12.000000000 +0200
+++ linux-2.6.13/drivers/char/hpet.c	2005-10-03 22:53:15.000000000 +0200
@@ -494,8 +494,11 @@ hpet_ioctl_common(struct hpet_dev *devp,
 		{
 			struct hpet_info info;
 
-			info.hi_ireqfreq = hpet_time_div(hpetp,
-							 devp->hd_ireqfreq);
+			if (devp->hd_ireqfreq)
+				info.hi_ireqfreq =
+					hpet_time_div(hpetp, devp->hd_ireqfreq);
+			else
+				info.hi_ireqfreq = 0;
 			info.hi_flags =
 			    readq(&timer->hpet_config) & Tn_PER_INT_CAP_MASK;
 			info.hi_hpet = devp->hd_hpets->hp_which;

[PATCH 4/7] HPET: fix uninitialized variable in hpet_register()

[Clemens Ladisch]

From: Clemens Ladisch <clemens@ladisch.de>

Clear the ht_opaque field in the hpet_register() function before
searching for a free timer to prevent the function from incorrectly
assuming that the search succeeded afterwards.

Signed-off-by: Clemens Ladisch <clemens@ladisch.de>

Index: linux-2.6.13/drivers/char/hpet.c
===================================================================
--- linux-2.6.13.orig/drivers/char/hpet.c	2005-10-03 22:53:15.000000000 +0200
+++ linux-2.6.13/drivers/char/hpet.c	2005-10-03 22:53:18.000000000 +0200
@@ -587,6 +587,8 @@ int hpet_register(struct hpet_task *tp, 
 		return -EINVAL;
 	}
 
+	tp->ht_opaque = NULL;
+
 	spin_lock_irq(&hpet_task_lock);
 	spin_lock(&hpet_lock);
 

[PATCH 5/7] HPET: fix access to multiple HPET devices

[Clemens Ladisch]

From: Clemens Ladisch <clemens@ladisch.de>

Fix two instances where a function would access the first HPET device
instead of the current one.

Signed-off-by: Clemens Ladisch <clemens@ladisch.de>

Index: linux-2.6.13/drivers/char/hpet.c
===================================================================
--- linux-2.6.13.orig/drivers/char/hpet.c	2005-10-03 22:53:18.000000000 +0200
+++ linux-2.6.13/drivers/char/hpet.c	2005-10-03 22:53:21.000000000 +0200
@@ -430,7 +430,7 @@ static int hpet_ioctl_ieon(struct hpet_d
 	}
 
 	if (devp->hd_flags & HPET_SHARED_IRQ) {
-		isr = 1 << (devp - hpets->hp_dev);
+		isr = 1 << (devp - devp->hd_hpets->hp_dev);
 		writel(isr, &hpet->hpet_isr);
 	}
 	writeq(g, &timer->hpet_config);
@@ -769,7 +769,7 @@ static unsigned long hpet_calibrate(stru
 	if (!timer)
 		return 0;
 
-	hpet = hpets->hp_hpet;
+	hpet = hpetp->hp_hpet;
 	t = read_counter(&timer->hpet_compare);
 
 	i = 0;

[PATCH 6/7] HPET: remove superfluous indirections

[Clemens Ladisch]

From: Clemens Ladisch <clemens@ladisch.de>

In the hpet_ioctl_common() function, devp->hd_hpets is already cached
in the hpetp variable, so we can use just that.

Signed-off-by: Clemens Ladisch <clemens@ladisch.de>

Index: linux-2.6.13/drivers/char/hpet.c
===================================================================
--- linux-2.6.13.orig/drivers/char/hpet.c	2005-10-03 22:53:21.000000000 +0200
+++ linux-2.6.13/drivers/char/hpet.c	2005-10-03 22:53:24.000000000 +0200
@@ -501,8 +501,8 @@ hpet_ioctl_common(struct hpet_dev *devp,
 				info.hi_ireqfreq = 0;
 			info.hi_flags =
 			    readq(&timer->hpet_config) & Tn_PER_INT_CAP_MASK;
-			info.hi_hpet = devp->hd_hpets->hp_which;
-			info.hi_timer = devp - devp->hd_hpets->hp_dev;
+			info.hi_hpet = hpetp->hp_which;
+			info.hi_timer = devp - hpetp->hp_dev;
 			if (kernel)
 				memcpy((void *)arg, &info, sizeof(info));
 			else

[PATCH 7/7] HPET: simplify initialization message

[Clemens Ladisch]

From: Clemens Ladisch <clemens@ladisch.de>

When booting, display the timer frequency in Hertz instead of as tick
length in nanoseconds.  Apart from saving a local variable, this makes
the message more easily comprehensible.

Signed-off-by: Clemens Ladisch <clemens@ladisch.de>

Index: linux-2.6.13/drivers/char/hpet.c
===================================================================
--- linux-2.6.13.orig/drivers/char/hpet.c	2005-10-03 22:53:24.000000000 +0200
+++ linux-2.6.13/drivers/char/hpet.c	2005-10-03 22:53:28.000000000 +0200
@@ -798,7 +798,7 @@ int hpet_alloc(struct hpet_data *hdp)
 	size_t siz;
 	struct hpet __iomem *hpet;
 	static struct hpets *last = (struct hpets *)0;
-	unsigned long ns, period;
+	unsigned long period;
 	unsigned long long temp;
 
 	/*
@@ -863,10 +863,9 @@ int hpet_alloc(struct hpet_data *hdp)
 		printk("%s %d", i > 0 ? "," : "", hdp->hd_irq[i]);
 	printk("\n");
 
-	ns = period / 1000000;	/* convert to nanoseconds, 10^-9 */
-	printk(KERN_INFO "hpet%d: %ldns tick, %d %d-bit timers\n",
-		hpetp->hp_which, ns, hpetp->hp_ntimer,
-		cap & HPET_COUNTER_SIZE_MASK ? 64 : 32);
+	printk(KERN_INFO "hpet%u: %u %d-bit timers, %Lu Hz\n",
+	       hpetp->hp_which, hpetp->hp_ntimer,
+	       cap & HPET_COUNTER_SIZE_MASK ? 64 : 32, hpetp->hp_tick_freq);
 
 	mcfg = readq(&hpet->hpet_config);
 	if ((mcfg & HPET_ENABLE_CNF_MASK) == 0) {

Re: [PATCH 0/7] more HPET fixes and enhancements

[Bob Picco]

Clemens Ladisch wrote:	[Tue Oct 04 2005, 08:41:26AM EDT]
> Another round of HPET bugfixes and cleanups.
> 
>  drivers/char/hpet.c |   35 +++++++++++++++++++++--------------
>  1 file changed, 21 insertions(+), 14 deletions(-)

Clemens:

These improvements and fixes look good to me.  Thanks for your work.

bob

Re: [PATCH 0/7] more HPET fixes and enhancements

[Randy.Dunlap]

On Tue, 04 Oct 2005 14:41:26 +0200 (MEST) Clemens Ladisch wrote:

> Another round of HPET bugfixes and cleanups.
> 
>  drivers/char/hpet.c |   35 +++++++++++++++++++++--------------
>  1 file changed, 21 insertions(+), 14 deletions(-)

Hi,

I've applied and tested all of these along with what is
currently in -mm (only -mm hpet + timer patches).

By "tested" I mean that I booted the kernel.  :)

What kind of testing have you done?
Do you have any timer test tools that you use to verify that
timers are actually working as expected?

Or maybe I could/should ask one or all of:
- John Stultz
- Thomas Gleixner
- George Anzinger (HRT project does have some tests)

Thanks,
---
~Randy

Re: [PATCH 0/7] more HPET fixes and enhancements

[Clemens Ladisch]

Randy.Dunlap wrote:

> On Tue, 04 Oct 2005 14:41:26 +0200 (MEST) Clemens Ladisch wrote:
>
> > Another round of HPET bugfixes and cleanups.
>
> I've applied and tested all of these along with what is
> currently in -mm (only -mm hpet + timer patches).
>
> By "tested" I mean that I booted the kernel.  :)
>
> What kind of testing have you done?
> Do you have any timer test tools that you use to verify that
> timers are actually working as expected?

Apart from the test program in hpet.txt, I'm using the ALSA HPET
driver (contained in the ALSA 1.0.9 package, but not yet in the kernel
tree) and then just test it using the ALSA API and/or use it as the
MIDI sequencer timer.


HTH
Clemens

Re: [PATCH 0/7] more HPET fixes and enhancements

[Takashi Iwai]

At Mon, 17 Oct 2005 18:30:40 +0200 (METDST),
Clemens Ladisch wrote:
> 
> Randy.Dunlap wrote:
> 
> > On Tue, 04 Oct 2005 14:41:26 +0200 (MEST) Clemens Ladisch wrote:
> >
> > > Another round of HPET bugfixes and cleanups.
> >
> > I've applied and tested all of these along with what is
> > currently in -mm (only -mm hpet + timer patches).
> >
> > By "tested" I mean that I booted the kernel.  :)
> >
> > What kind of testing have you done?
> > Do you have any timer test tools that you use to verify that
> > timers are actually working as expected?
> 
> Apart from the test program in hpet.txt, I'm using the ALSA HPET
> driver (contained in the ALSA 1.0.9 package, but not yet in the kernel
> tree) and then just test it using the ALSA API and/or use it as the
> MIDI sequencer timer.

Let's push it to kernel tree :)


Takashi