Skip to contentSkip to conversation

more HPET fixes and enhancements

PATCH In review Clemens Ladisch 4 participants 12 messages Oct 4, 2005 – Oct 18, 2005

Subsystems (2)
lore
Share & tools
lore

Clemens Ladisch

author maintainer
#0 lore raw

cover more HPET fixes and enhancements

Another round of HPET bugfixes and cleanups.

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

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

From: Keir Fraser <[email protected]>

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.

drivers/char/hpet.c +0 -1
Index: linux-2.6.13/drivers/char/hpet.c
===================================================================
@@ -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)) {
author maintainer
#2 lore raw

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

In reply to Clemens Ladisch

From: Clemens Ladisch <[email protected]>

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().

drivers/char/hpet.c +6 -2
Index: linux-2.6.13/drivers/char/hpet.c
===================================================================
@@ -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:
author maintainer
#3 lore raw

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

In reply to Clemens Ladisch

From: Clemens Ladisch <[email protected]>

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

drivers/char/hpet.c +5 -2
Index: linux-2.6.13/drivers/char/hpet.c
===================================================================
@@ -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;
author maintainer
#4 lore raw

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

In reply to Clemens Ladisch

From: Clemens Ladisch <[email protected]>

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.

drivers/char/hpet.c +2 -0
Index: linux-2.6.13/drivers/char/hpet.c
===================================================================
@@ -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);
 
author maintainer
#5 lore raw

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

In reply to Clemens Ladisch

From: Clemens Ladisch <[email protected]>

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

drivers/char/hpet.c +2 -2
Index: linux-2.6.13/drivers/char/hpet.c
===================================================================
@@ -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;
author maintainer
#6 lore raw

PATCH 6/7 HPET: remove superfluous indirections

In reply to Clemens Ladisch

From: Clemens Ladisch <[email protected]>

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

drivers/char/hpet.c +2 -2
Index: linux-2.6.13/drivers/char/hpet.c
===================================================================
@@ -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
author maintainer
#7 lore raw

PATCH 7/7 HPET: simplify initialization message

In reply to Clemens Ladisch

From: Clemens Ladisch <[email protected]>

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.

drivers/char/hpet.c +4 -5
Index: linux-2.6.13/drivers/char/hpet.c
===================================================================
@@ -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) {
#8 lore raw
In reply to Clemens Ladisch

Clemens Ladisch wrote: [Tue Oct 04 2005, 08:41:26AM EDT]

Quoted 4 lines

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

#9 lore raw
In reply to Clemens Ladisch
Quoted 4 lines · 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

author maintainer
#10 lore raw
Quoted 4 lines · Randy.Dunlap
Quoted 1 line · On Tue, 04 Oct 2005 14:41:26 +0200 (MEST) Clemens Ladisch wrote:

Another round of HPET bugfixes and cleanups.

Quoted 8 lines

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

#11 lore raw

At Mon, 17 Oct 2005 18:30:40 +0200 (METDST),

Quoted 12 lines · Clemens Ladisch
Quoted 9 lines · Randy.Dunlap
Quoted 1 line · 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. :)

Quoted 8 lines
Quoted 3 lines

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