From: David Brownell <[email protected]>
Minor /dev/hpet updates and bugfixes:
* Remove dead code, mostly remnants of an incomplete/unusable
kernel interface ... noted when addressing "sparse" warnings:
+ hpet_unregister() and a routine it calls
+ hpet_task and all references, including hpet_task_lock
+ hpet_data.hd_flags (and HPET_DATA_PLATFORM)
* Correct and improve boot message:
+ displays *counter* (shared between comparators) bitwidth,
not *timer* bitwidths (which are often mixed)
+ relabel "timers" as "comparators"; this is less confusing,
they are not independent like normal timers are (sigh)
+ display MHz not Hz; it's never less than 10 MHz.
* Tighten and correct the userspace interface code
+ don't accidentally program comparators in 64-bit mode using
32-bit values ... always force comparators into 32-bit mode
+ only open comparators that have an interrupt, and can thus
perform "real work"
+ provide the correct bit definition flagging comparators with
periodic capability ... the ABI is unchanged
* Update Documentation/hpet.txt
+ be more correct and current
+ expand description a bit
+ don't mention that now-gone kernel interface
Plus, add a FIXME comment for something that could cause big trouble
on systems with more capable HPETs than at least Intel seems to ship.
I get the feeling few folk use this userspace interface, else those
nonfunctional IRQs would be causing more trouble.
Signed-off-by: David Brownell <[email protected]>
---
I CC'd everyone who MAINTAINERS says maintains HPET. Odd to have
four entries!!
And re that kernel interface: IMO, worth having a relatively
generic interface for hardware timers instead of inventing a new
interface for each new bit of hardware. Embedded systems tend to
have at least half a dozen SOC timers available...
Documentation/hpet.txt | 43 ++++++++++++-------------
arch/x86/kernel/hpet.c | 6 ++-
drivers/char/hpet.c | 82 +++++++++++--------------------------------------
include/linux/hpet.h | 16 ++-------
4 files changed, 50 insertions(+), 97 deletions(-)
--- a/Documentation/hpet.txt 2008-07-21 15:38:28.000000000 -0700
+++ b/Documentation/hpet.txt 2008-07-21 21:40:58.000000000 -0700
@@ -1,21 +1,32 @@
High Precision Event Timer Driver for Linux
-The High Precision Event Timer (HPET) hardware is the future replacement
-for the 8254 and Real Time Clock (RTC) periodic timer functionality.
-Each HPET can have up to 32 timers. It is possible to configure the
-first two timers as legacy replacements for 8254 and RTC periodic timers.
-A specification done by Intel and Microsoft can be found at
-<http://www.intel.com/technology/architecture/hpetspec.htm>.
+The High Precision Event Timer (HPET) hardware follows a specification
+by Intel and Microsoft which can be found at
+
+ http://www.intel.com/technology/architecture/hpetspec.htm
+
+Each HPET has one fixed-rate counter (at 10+ MHz, hence "High Precision")
+and up to 32 comparators. Normally three or more comparators are provided,
+each of which can generate oneshot interupts and at least one of which has
+additional hardware to support periodic interrupts. The comparators are
+also called "timers", which can be misleading since usually timers are
+independent of each other ... these share a counter, complicating resets.
+
+HPET devices can support two interrupt routing modes. In one mode, the
+comparators are additional interrupt sources with no particular system
+role. Many x86 BIOS writers don't route HPET interrupts at all, which
+prevents use of that mode. They support the other "legacy replacement"
+mode where the first two comparators block interrupts from 8254 timers
+and from the RTC.
The driver supports detection of HPET driver allocation and initialization
of the HPET before the driver module_init routine is called. This enables
platform code which uses timer 0 or 1 as the main timer to intercept HPET
initialization. An example of this initialization can be found in
-arch/i386/kernel/time_hpet.c.
+arch/x86/kernel/hpet.c.
-The driver provides two APIs which are very similar to the API found in
-the rtc.c driver. There is a user space API and a kernel space API.
-An example user space program is provided below.
+The driver provides a userspace API which resembles the API found in the
+RTC driver framework. An example user space program is provided below.
#include <stdio.h>
#include <stdlib.h>
@@ -286,15 +297,3 @@ out:
return;
}
-
-The kernel API has three interfaces exported from the driver:
-
- hpet_register(struct hpet_task *tp, int periodic)
- hpet_unregister(struct hpet_task *tp)
- hpet_control(struct hpet_task *tp, unsigned int cmd, unsigned long arg)
-
-The kernel module using this interface fills in the ht_func and ht_data
-members of the hpet_task structure before calling hpet_register.
-hpet_control simply vectors to the hpet_ioctl routine and has the same
-commands and respective arguments as the user API. hpet_unregister
-is used to terminate usage of the HPET timer reserved by hpet_register.
--- a/arch/x86/kernel/hpet.c 2008-07-21 18:59:49.000000000 -0700
+++ b/arch/x86/kernel/hpet.c 2008-07-21 19:43:04.000000000 -0700
@@ -127,13 +127,17 @@ static void hpet_reserve_platform_timers
hd.hd_phys_address = hpet_address;
hd.hd_address = hpet;
hd.hd_nirqs = nrtimers;
- hd.hd_flags = HPET_DATA_PLATFORM;
hpet_reserve_timer(&hd, 0);
#ifdef CONFIG_HPET_EMULATE_RTC
hpet_reserve_timer(&hd, 1);
#endif
+ /*
+ * NOTE that hd_irq[] reflects IOAPIC input pins (LEGACY_8254
+ * is wrong for i8259!) not the output IRQ. Many BIOS writers
+ * don't bother configuring *any* comparator interrupts.
+ */
hd.hd_irq[0] = HPET_LEGACY_8254;
hd.hd_irq[1] = HPET_LEGACY_RTC;
--- a/drivers/char/hpet.c 2008-07-21 15:38:28.000000000 -0700
+++ b/drivers/char/hpet.c 2008-07-21 21:41:16.000000000 -0700
@@ -76,7 +76,7 @@ static struct clocksource clocksource_hp
.rating = 250,
.read = read_hpet,
.mask = CLOCKSOURCE_MASK(64),
- .mult = 0, /*to be caluclated*/
+ .mult = 0, /*to be calculated*/
.shift = 10,
.flags = CLOCK_SOURCE_IS_CONTINUOUS,
};
@@ -85,8 +85,6 @@ static struct clocksource *hpet_clocksou
/* A lock for concurrent access by app and isr hpet activity. */
static DEFINE_SPINLOCK(hpet_lock);
-/* A lock for concurrent intermodule access to hpet and isr hpet activity. */
-static DEFINE_SPINLOCK(hpet_task_lock);
#define HPET_DEV_NAME (7)
@@ -98,7 +96,6 @@ struct hpet_dev {
unsigned long hd_irqdata;
wait_queue_head_t hd_waitqueue;
struct fasync_struct *hd_async_queue;
- struct hpet_task *hd_task;
unsigned int hd_flags;
unsigned int hd_irq;
unsigned int hd_hdwirq;
@@ -172,11 +169,6 @@ static irqreturn_t hpet_interrupt(int ir
writel(isr, &devp->hd_hpet->hpet_isr);
spin_unlock(&hpet_lock);
- spin_lock(&hpet_task_lock);
- if (devp->hd_task)
- devp->hd_task->ht_func(devp->hd_task->ht_data);
- spin_unlock(&hpet_task_lock);
-
wake_up_interruptible(&devp->hd_waitqueue);
kill_fasync(&devp->hd_async_queue, SIGIO, POLL_IN);
@@ -198,7 +190,7 @@ static int hpet_open(struct inode *inode
for (devp = NULL, hpetp = hpets; hpetp && !devp; hpetp = hpetp->hp_next)
for (i = 0; i < hpetp->hp_ntimer; i++)
if (hpetp->hp_dev[i].hd_flags & HPET_OPEN
- || hpetp->hp_dev[i].hd_task)
+ || !hpetp->hp_dev[i].hd_hdwirq)
continue;
else {
devp = &hpetp->hp_dev[i];
@@ -437,7 +429,11 @@ static int hpet_ioctl_ieon(struct hpet_d
devp->hd_irq = irq;
t = devp->hd_ireqfreq;
v = readq(&timer->hpet_config);
- g = v | Tn_INT_ENB_CNF_MASK;
+
+ /* 64-bit comparators are not yet supported through the ioctls,
+ * so force this into 32-bit mode if it supports both modes
+ */
+ g = v | Tn_32MODE_CNF_MASK | Tn_INT_ENB_CNF_MASK;
if (devp->hd_flags & HPET_PERIODIC) {
write_counter(t, &timer->hpet_compare);
@@ -447,6 +443,12 @@ static int hpet_ioctl_ieon(struct hpet_d
v |= Tn_VAL_SET_CNF_MASK;
writeq(v, &timer->hpet_config);
local_irq_save(flags);
+
+ /* FIXME this may trash both the system clocksource and
+ * the current clock event device! Use HPET_TN_SETVAL
+ * instead, like arch/x86/kernel/hpet.c does ... never
+ * modify the counter, ever.
+ */
m = read_counter(&hpet->hpet_mc);
write_counter(t + m + hpetp->hp_delta, &timer->hpet_compare);
} else {
@@ -600,55 +602,6 @@ static int hpet_is_known(struct hpet_dat
return 0;
}
-static inline int hpet_tpcheck(struct hpet_task *tp)
-{
- struct hpet_dev *devp;
- struct hpets *hpetp;
-
- devp = tp->ht_opaque;
-
- if (!devp)
- return -ENXIO;
-
- for (hpetp = hpets; hpetp; hpetp = hpetp->hp_next)
- if (devp >= hpetp->hp_dev
- && devp < (hpetp->hp_dev + hpetp->hp_ntimer)
- && devp->hd_hpet == hpetp->hp_hpet)
- return 0;
-
- return -ENXIO;
-}
-
-int hpet_unregister(struct hpet_task *tp)
-{
- struct hpet_dev *devp;
- struct hpet_timer __iomem *timer;
- int err;
-
- if ((err = hpet_tpcheck(tp)))
- return err;
-
- spin_lock_irq(&hpet_task_lock);
- spin_lock(&hpet_lock);
-
- devp = tp->ht_opaque;
- if (devp->hd_task != tp) {
- spin_unlock(&hpet_lock);
- spin_unlock_irq(&hpet_task_lock);
- return -ENXIO;
- }
-
- timer = devp->hd_timer;
- writeq((readq(&timer->hpet_config) & ~Tn_INT_ENB_CNF_MASK),
- &timer->hpet_config);
- devp->hd_flags &= ~(HPET_IE | HPET_PERIODIC);
- devp->hd_task = NULL;
- spin_unlock(&hpet_lock);
- spin_unlock_irq(&hpet_task_lock);
-
- return 0;
-}
-
static ctl_table hpet_table[] = {
{
.ctl_name = CTL_UNNUMBERED,
@@ -803,9 +756,12 @@ int hpet_alloc(struct hpet_data *hdp)
printk("%s %d", i > 0 ? "," : "", hdp->hd_irq[i]);
printk("\n");
- 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);
+ printk(KERN_INFO
+ "hpet%u: %u comparators, %d-bit %u.%06u MHz counter\n",
+ hpetp->hp_which, hpetp->hp_ntimer,
+ cap & HPET_COUNTER_SIZE_MASK ? 64 : 32,
+ (unsigned) (hpetp->hp_tick_freq / 1000000),
+ (unsigned) (hpetp->hp_tick_freq % 1000000));
mcfg = readq(&hpet->hpet_config);
if ((mcfg & HPET_ENABLE_CNF_MASK) == 0) {
--- a/include/linux/hpet.h 2008-07-21 15:38:28.000000000 -0700
+++ b/include/linux/hpet.h 2008-07-21 19:00:06.000000000 -0700
@@ -91,23 +91,14 @@ struct hpet {
* exported interfaces
*/
-struct hpet_task {
- void (*ht_func) (void *);
- void *ht_data;
- void *ht_opaque;
-};
-
struct hpet_data {
unsigned long hd_phys_address;
void __iomem *hd_address;
unsigned short hd_nirqs;
- unsigned short hd_flags;
unsigned int hd_state; /* timer allocated */
unsigned int hd_irq[HPET_MAX_TIMERS];
};
-#define HPET_DATA_PLATFORM 0x0001 /* platform call to hpet_alloc */
-
static inline void hpet_reserve_timer(struct hpet_data *hd, int timer)
{
hd->hd_state |= (1 << timer);
@@ -125,7 +119,7 @@ struct hpet_info {
unsigned short hi_timer;
};
-#define HPET_INFO_PERIODIC 0x0001 /* timer is periodic */
+#define HPET_INFO_PERIODIC 0x0010 /* periodic-capable comparator */
#define HPET_IE_ON _IO('h', 0x01) /* interrupt on */
#define HPET_IE_OFF _IO('h', 0x02) /* interrupt off */
David Brownell wrote:
> * Tighten and correct the userspace interface code
> ...
> + only open comparators that have an interrupt, and can thus
> perform "real work"
This prevents userspace applications from doing mmap() on /dev/hpet
even if there is no interrupt.
This seems to be the only part of the userspace interface that is
used in practice. Because of the availability of POSIX timers, it might
make sense to deprecate the HPET ioctl interface.
> And re that kernel interface: IMO, worth having a relatively
> generic interface for hardware timers instead of inventing a new
> interface for each new bit of hardware.
AFAIK it was intended to be similar to the RTC callback interface, but
that was before the generic high-precision timer stuff was introduced.
Regards,
Clemens
On Wednesday 23 July 2008, Clemens Ladisch wrote:
> David Brownell wrote:
> > * Tighten and correct the userspace interface code
> > ...
> > + only open comparators that have an interrupt, and can thus
> > perform "real work"
>
> This prevents userspace applications from doing mmap() on /dev/hpet
> even if there is no interrupt.
OK, I removed that ... HPET_IE_ON will already fail.
I didn't find any software using /dev/hpet, except for
the example in Documentation/hpet.txt ... presumably I
was looking in the wrong place. I'd surely have retched
at anything mmapping hardware registers though. ;)
> This seems to be the only part of the userspace interface that is
> used in practice. Because of the availability of POSIX timers, it might
> make sense to deprecate the HPET ioctl interface.
I'll leave that part up to someone else. If POSIX timers
are a sufficient userspace interface, great ... then that
mmap son't really be needed either! In fact, would the
/dev/hpet support even be needed? It's not very portable...
> > And re that kernel interface: IMO, worth having a relatively
> > generic interface for hardware timers instead of inventing a new
> > interface for each new bit of hardware.
>
> AFAIK it was intended to be similar to the RTC callback interface, but
> that was before the generic high-precision timer stuff was introduced.
People seem to want access to the hardware backed timers too, and
not necessarily coupled to a task like the RTC stuff assumes. Folk
who want to drive that issue will need to sort out requirements...
the requests I've heard seem to focus on talking to kernel code.
But since HPET "timers" are really weak compared to what most embedded
platforms offer, I'm quite sure any API designed to its capabilities
would be underfeatured! Example, there are no external inputs (like
clocks or event pulses) or outputs (PWM or whatever) with HPET, and
likewise no flexibility about inputs (which internal clock, or maybe
external clocks).
- Dave
On Wednesday 23 July 2008, David Brownell wrote:
> On Wednesday 23 July 2008, Clemens Ladisch wrote:
> > David Brownell wrote:
> > > ? * Tighten and correct the userspace interface code
> > > ? ? ? ...
> > > ? ? ? + only open comparators that have an interrupt, and can thus
> > > ? ? ? ? perform "real work"
> >
> > This prevents userspace applications from doing mmap() on /dev/hpet
> > even if there is no interrupt.
>
> OK, I removed that ... HPET_IE_ON will already fail.
This change will be included in the next version of this patch, which
I'll send after other responses get more of a chance to trickle in.
(That line already changed because of the hd_task elimination.)
--- g26.orig/drivers/char/hpet.c 2008-07-23 09:43:59.000000000 -0700
+++ g26/drivers/char/hpet.c 2008-07-23 09:03:34.000000000 -0700
@@ -189,8 +189,7 @@ static int hpet_open(struct inode *inode
for (devp = NULL, hpetp = hpets; hpetp && !devp; hpetp = hpetp->hp_next)
for (i = 0; i < hpetp->hp_ntimer; i++)
- if (hpetp->hp_dev[i].hd_flags & HPET_OPEN
- || !hpetp->hp_dev[i].hd_hdwirq)
+ if (hpetp->hp_dev[i].hd_flags & HPET_OPEN)
continue;
else {
devp = &hpetp->hp_dev[i];
David Brownell wrote:
> I didn't find any software using /dev/hpet, except for
> the example in Documentation/hpet.txt ... presumably I
> was looking in the wrong place. I'd surely have retched
> at anything mmapping hardware registers though. ;)
http://subversion.jackaudio.org/jack/trunk/jack/config/os/gnu-linux/time.c
> > This seems to be the only part of the userspace interface that is
> > used in practice. Because of the availability of POSIX timers, it might
> > make sense to deprecate the HPET ioctl interface.
>
> I'll leave that part up to someone else. If POSIX timers
> are a sufficient userspace interface, great ... then that
> mmap son't really be needed either!
The idea is to be able to get a high-precision timer value without doing
a syscall. (Whether the syscall overhead actually matters in a specific
application is another question.)
Regards,
Clemens
On Thursday 24 July 2008, Clemens Ladisch wrote:
> David Brownell wrote:
> > > This seems to be the only part of the userspace interface that is
> > > used in practice. Because of the availability of POSIX timers, it might
> > > make sense to deprecate the HPET ioctl interface.
> >
> > I'll leave that part up to someone else. If POSIX timers
> > are a sufficient userspace interface, great ... then that
> > mmap son't really be needed either!
>
> The idea is to be able to get a high-precision timer value without doing
> a syscall. (Whether the syscall overhead actually matters in a specific
> application is another question.)
On x86_64 the vsyscall stuff should kick in for gettimeofday()
when HPET is in use, eliminating the need for /dev/hpet mmapping.
I don't really know vsyscalls ... but if that could be done on
i386 systems, the last argument for /dev/hpet would seem gone...
From: David Brownell <[email protected]>
Minor /dev/hpet updates and bugfixes:
* Remove dead code, mostly remnants of an incomplete/unusable
kernel interface ... noted when addressing "sparse" warnings:
+ hpet_unregister() and a routine it calls
+ hpet_task and all references, including hpet_task_lock
+ hpet_data.hd_flags (and HPET_DATA_PLATFORM)
* Correct and improve boot message:
+ displays *counter* (shared between comparators) bitwidth,
not *timer* bitwidths (which are often mixed)
+ relabel "timers" as "comparators"; this is less confusing,
they are not independent like normal timers are (sigh)
+ display MHz not Hz; it's never less than 10 MHz.
* Tighten and correct the userspace interface code
+ don't accidentally program comparators in 64-bit mode using
32-bit values ... always force comparators into 32-bit mode
+ provide the correct bit definition flagging comparators with
periodic capability ... the ABI is unchanged
* Update Documentation/hpet.txt
+ be more correct and current
+ expand description a bit
+ don't mention that now-gone kernel interface
Plus, add a FIXME comment for something that could cause big trouble
on systems with more capable HPETs than at least Intel seems to ship.
I get the feeling few folk use this userspace interface, else those
nonfunctional IRQs would be causing more trouble. (And I'm told the
main current user is probably Jack, through mmap, for what it seems
the gettimeofday vsyscall will do on x86_64.)
Signed-off-by: David Brownell <[email protected]>
---
Refreshed to address the issue Clemens noted, and to cope with a
recent partial fix for the dead code issue.
Documentation/hpet.txt | 43 ++++++++++++------------
arch/x86/kernel/hpet.c | 6 ++-
drivers/char/hpet.c | 85 ++++++++++---------------------------------------
include/linux/hpet.h | 11 ------
4 files changed, 46 insertions(+), 99 deletions(-)
--- a/Documentation/hpet.txt 2008-07-25 12:24:41.000000000 -0700
+++ b/Documentation/hpet.txt 2008-07-25 12:43:50.000000000 -0700
@@ -1,21 +1,32 @@
High Precision Event Timer Driver for Linux
-The High Precision Event Timer (HPET) hardware is the future replacement
-for the 8254 and Real Time Clock (RTC) periodic timer functionality.
-Each HPET can have up to 32 timers. It is possible to configure the
-first two timers as legacy replacements for 8254 and RTC periodic timers.
-A specification done by Intel and Microsoft can be found at
-<http://www.intel.com/technology/architecture/hpetspec.htm>.
+The High Precision Event Timer (HPET) hardware follows a specification
+by Intel and Microsoft which can be found at
+
+ http://www.intel.com/technology/architecture/hpetspec.htm
+
+Each HPET has one fixed-rate counter (at 10+ MHz, hence "High Precision")
+and up to 32 comparators. Normally three or more comparators are provided,
+each of which can generate oneshot interupts and at least one of which has
+additional hardware to support periodic interrupts. The comparators are
+also called "timers", which can be misleading since usually timers are
+independent of each other ... these share a counter, complicating resets.
+
+HPET devices can support two interrupt routing modes. In one mode, the
+comparators are additional interrupt sources with no particular system
+role. Many x86 BIOS writers don't route HPET interrupts at all, which
+prevents use of that mode. They support the other "legacy replacement"
+mode where the first two comparators block interrupts from 8254 timers
+and from the RTC.
The driver supports detection of HPET driver allocation and initialization
of the HPET before the driver module_init routine is called. This enables
platform code which uses timer 0 or 1 as the main timer to intercept HPET
initialization. An example of this initialization can be found in
-arch/i386/kernel/time_hpet.c.
+arch/x86/kernel/hpet.c.
-The driver provides two APIs which are very similar to the API found in
-the rtc.c driver. There is a user space API and a kernel space API.
-An example user space program is provided below.
+The driver provides a userspace API which resembles the API found in the
+RTC driver framework. An example user space program is provided below.
#include <stdio.h>
#include <stdlib.h>
@@ -286,15 +297,3 @@ out:
return;
}
-
-The kernel API has three interfaces exported from the driver:
-
- hpet_register(struct hpet_task *tp, int periodic)
- hpet_unregister(struct hpet_task *tp)
- hpet_control(struct hpet_task *tp, unsigned int cmd, unsigned long arg)
-
-The kernel module using this interface fills in the ht_func and ht_data
-members of the hpet_task structure before calling hpet_register.
-hpet_control simply vectors to the hpet_ioctl routine and has the same
-commands and respective arguments as the user API. hpet_unregister
-is used to terminate usage of the HPET timer reserved by hpet_register.
--- a/arch/x86/kernel/hpet.c 2008-07-25 12:24:41.000000000 -0700
+++ b/arch/x86/kernel/hpet.c 2008-07-25 12:43:50.000000000 -0700
@@ -115,13 +115,17 @@ static void hpet_reserve_platform_timers
hd.hd_phys_address = hpet_address;
hd.hd_address = hpet;
hd.hd_nirqs = nrtimers;
- hd.hd_flags = HPET_DATA_PLATFORM;
hpet_reserve_timer(&hd, 0);
#ifdef CONFIG_HPET_EMULATE_RTC
hpet_reserve_timer(&hd, 1);
#endif
+ /*
+ * NOTE that hd_irq[] reflects IOAPIC input pins (LEGACY_8254
+ * is wrong for i8259!) not the output IRQ. Many BIOS writers
+ * don't bother configuring *any* comparator interrupts.
+ */
hd.hd_irq[0] = HPET_LEGACY_8254;
hd.hd_irq[1] = HPET_LEGACY_RTC;
--- a/drivers/char/hpet.c 2008-07-25 12:28:08.000000000 -0700
+++ b/drivers/char/hpet.c 2008-07-25 12:43:50.000000000 -0700
@@ -77,7 +77,7 @@ static struct clocksource clocksource_hp
.rating = 250,
.read = read_hpet,
.mask = CLOCKSOURCE_MASK(64),
- .mult = 0, /*to be caluclated*/
+ .mult = 0, /*to be calculated*/
.shift = 10,
.flags = CLOCK_SOURCE_IS_CONTINUOUS,
};
@@ -86,8 +86,6 @@ static struct clocksource *hpet_clocksou
/* A lock for concurrent access by app and isr hpet activity. */
static DEFINE_SPINLOCK(hpet_lock);
-/* A lock for concurrent intermodule access to hpet and isr hpet activity. */
-static DEFINE_SPINLOCK(hpet_task_lock);
#define HPET_DEV_NAME (7)
@@ -99,7 +97,6 @@ struct hpet_dev {
unsigned long hd_irqdata;
wait_queue_head_t hd_waitqueue;
struct fasync_struct *hd_async_queue;
- struct hpet_task *hd_task;
unsigned int hd_flags;
unsigned int hd_irq;
unsigned int hd_hdwirq;
@@ -173,11 +170,6 @@ static irqreturn_t hpet_interrupt(int ir
writel(isr, &devp->hd_hpet->hpet_isr);
spin_unlock(&hpet_lock);
- spin_lock(&hpet_task_lock);
- if (devp->hd_task)
- devp->hd_task->ht_func(devp->hd_task->ht_data);
- spin_unlock(&hpet_task_lock);
-
wake_up_interruptible(&devp->hd_waitqueue);
kill_fasync(&devp->hd_async_queue, SIGIO, POLL_IN);
@@ -199,8 +191,7 @@ static int hpet_open(struct inode *inode
for (devp = NULL, hpetp = hpets; hpetp && !devp; hpetp = hpetp->hp_next)
for (i = 0; i < hpetp->hp_ntimer; i++)
- if (hpetp->hp_dev[i].hd_flags & HPET_OPEN
- || hpetp->hp_dev[i].hd_task)
+ if (hpetp->hp_dev[i].hd_flags & HPET_OPEN)
continue;
else {
devp = &hpetp->hp_dev[i];
@@ -441,7 +432,11 @@ static int hpet_ioctl_ieon(struct hpet_d
devp->hd_irq = irq;
t = devp->hd_ireqfreq;
v = readq(&timer->hpet_config);
- g = v | Tn_INT_ENB_CNF_MASK;
+
+ /* 64-bit comparators are not yet supported through the ioctls,
+ * so force this into 32-bit mode if it supports both modes
+ */
+ g = v | Tn_32MODE_CNF_MASK | Tn_INT_ENB_CNF_MASK;
if (devp->hd_flags & HPET_PERIODIC) {
write_counter(t, &timer->hpet_compare);
@@ -451,6 +446,12 @@ static int hpet_ioctl_ieon(struct hpet_d
v |= Tn_VAL_SET_CNF_MASK;
writeq(v, &timer->hpet_config);
local_irq_save(flags);
+
+ /* FIXME this may trash both the system clocksource and
+ * the current clock event device! Use HPET_TN_SETVAL
+ * instead, like arch/x86/kernel/hpet.c does ... never
+ * modify the counter, ever.
+ */
m = read_counter(&hpet->hpet_mc);
write_counter(t + m + hpetp->hp_delta, &timer->hpet_compare);
} else {
@@ -604,57 +605,6 @@ static int hpet_is_known(struct hpet_dat
return 0;
}
-static inline int hpet_tpcheck(struct hpet_task *tp)
-{
- struct hpet_dev *devp;
- struct hpets *hpetp;
-
- devp = tp->ht_opaque;
-
- if (!devp)
- return -ENXIO;
-
- for (hpetp = hpets; hpetp; hpetp = hpetp->hp_next)
- if (devp >= hpetp->hp_dev
- && devp < (hpetp->hp_dev + hpetp->hp_ntimer)
- && devp->hd_hpet == hpetp->hp_hpet)
- return 0;
-
- return -ENXIO;
-}
-
-#if 0
-int hpet_unregister(struct hpet_task *tp)
-{
- struct hpet_dev *devp;
- struct hpet_timer __iomem *timer;
- int err;
-
- if ((err = hpet_tpcheck(tp)))
- return err;
-
- spin_lock_irq(&hpet_task_lock);
- spin_lock(&hpet_lock);
-
- devp = tp->ht_opaque;
- if (devp->hd_task != tp) {
- spin_unlock(&hpet_lock);
- spin_unlock_irq(&hpet_task_lock);
- return -ENXIO;
- }
-
- timer = devp->hd_timer;
- writeq((readq(&timer->hpet_config) & ~Tn_INT_ENB_CNF_MASK),
- &timer->hpet_config);
- devp->hd_flags &= ~(HPET_IE | HPET_PERIODIC);
- devp->hd_task = NULL;
- spin_unlock(&hpet_lock);
- spin_unlock_irq(&hpet_task_lock);
-
- return 0;
-}
-#endif /* 0 */
-
static ctl_table hpet_table[] = {
{
.ctl_name = CTL_UNNUMBERED,
@@ -809,9 +759,12 @@ int hpet_alloc(struct hpet_data *hdp)
printk("%s %d", i > 0 ? "," : "", hdp->hd_irq[i]);
printk("\n");
- 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);
+ printk(KERN_INFO
+ "hpet%u: %u comparators, %d-bit %u.%06u MHz counter\n",
+ hpetp->hp_which, hpetp->hp_ntimer,
+ cap & HPET_COUNTER_SIZE_MASK ? 64 : 32,
+ (unsigned) (hpetp->hp_tick_freq / 1000000),
+ (unsigned) (hpetp->hp_tick_freq % 1000000));
mcfg = readq(&hpet->hpet_config);
if ((mcfg & HPET_ENABLE_CNF_MASK) == 0) {
--- a/include/linux/hpet.h 2008-07-25 12:24:41.000000000 -0700
+++ b/include/linux/hpet.h 2008-07-25 12:43:50.000000000 -0700
@@ -91,23 +91,14 @@ struct hpet {
* exported interfaces
*/
-struct hpet_task {
- void (*ht_func) (void *);
- void *ht_data;
- void *ht_opaque;
-};
-
struct hpet_data {
unsigned long hd_phys_address;
void __iomem *hd_address;
unsigned short hd_nirqs;
- unsigned short hd_flags;
unsigned int hd_state; /* timer allocated */
unsigned int hd_irq[HPET_MAX_TIMERS];
};
-#define HPET_DATA_PLATFORM 0x0001 /* platform call to hpet_alloc */
-
static inline void hpet_reserve_timer(struct hpet_data *hd, int timer)
{
hd->hd_state |= (1 << timer);
@@ -125,7 +116,7 @@ struct hpet_info {
unsigned short hi_timer;
};
-#define HPET_INFO_PERIODIC 0x0001 /* timer is periodic */
+#define HPET_INFO_PERIODIC 0x0010 /* periodic-capable comparator */
#define HPET_IE_ON _IO('h', 0x01) /* interrupt on */
#define HPET_IE_OFF _IO('h', 0x02) /* interrupt off */
David Brownell wrote:
> ...
> Plus, add a FIXME comment for something that could cause big trouble
> on systems with more capable HPETs than at least Intel seems to ship.
> ...
> +
> + /* FIXME this may trash both the system clocksource and
> + * the current clock event device! Use HPET_TN_SETVAL
> + * instead, like arch/x86/kernel/hpet.c does ... never
> + * modify the counter, ever.
> + */
> m = read_counter(&hpet->hpet_mc);
> write_counter(t + m + hpetp->hp_delta, &timer->hpet_compare);
This comment seems to assume that the code below modifies the main
counter, which it doesn't. Additionally, HPET_TN_SETVAL has the same
value as Tn_VAL_SET_CNF_MASK (from <linux/hpet.h>), which _is_ used.
Regards,
Clemens
On Monday 28 July 2008, Clemens Ladisch wrote:
> > +?????????????/* FIXME this may trash both the system clocksource and
> > +????????????? * the current clock event device! ?Use HPET_TN_SETVAL
> > +????????????? * instead, like arch/x86/kernel/hpet.c does ... never
> > +????????????? * modify the counter, ever.
> > +????????????? */
> > ??????????????m = read_counter(&hpet->hpet_mc);
> > ??????????????write_counter(t + m + hpetp->hp_delta, &timer->hpet_compare);
>
> This comment seems to assume that the code below modifies the main
> counter, which it doesn't.
There's only one counter. How could it not modify that?
Oh ... I see. It's called write_counter() but doesn't
actually write the counter. Likewise, read_counter() is
not actually reading the counter. Gaack ...
So that's not really an issue (good!). I'll strike that
comment, except to comment that it's explicitly not modifying
any counter (just a hidden write-only accumulator) ... but
those silly function names should really be changed so they
have a less tenuous connection with reality. In fact, most
places would be better off just hard-wiring 32-bit access...
> Additionally, HPET_TN_SETVAL has the same
> value as Tn_VAL_SET_CNF_MASK (from <linux/hpet.h>), which _is_ used.
OK, I can see that too. This HPET stuff is really a lot
dirtier than I had expected ... there's no reason at all
to have two separate headers with two incompatible sets
of definitions for the same registers!!
Any comments on the rest?
- Dave
David Brownell wrote:
> Oh ... I see. It's called write_counter() but doesn't
> actually write the counter. Likewise, read_counter() is
> not actually reading the counter. Gaack ...
A more correct name would be something like
write_a_register_that_has_the_same_size_as_a_counter_register().
> In fact, most places would be better off just hard-wiring 32-bit
> access...
... and the definition of read/write_counter() depends on the word size
of the CPU architecture and has nothing to do with the actual size of
the HPET registers ...
I'm hoping that I can deprecate and delete much of this code without
having to clean it up first.
> Any comments on the rest?
Looks fine. I would have preferred a series of smaller patches, but my
feelings on this are not strong enough to split it myself or to ask you
to do it.
Regards,
Clemens
On Monday 28 July 2008, Clemens Ladisch wrote:
> David Brownell wrote:
> > Oh ... I see. It's called write_counter() but doesn't
> > actually write the counter. Likewise, read_counter() is
> > not actually reading the counter. Gaack ...
>
> A more correct name would be something like
> write_a_register_that_has_the_same_size_as_a_counter_register().
writel() would be my choice ... :)
I don't think anything in that code really needs to modify the
upper 32 bits of those registers. And nothing except the ia64
HPET clocksource needs to read them either.
> I'm hoping that I can deprecate and delete much of this code without
> having to clean it up first.
I'm not sure how practical that goal is, but I agree that there
seems to be no point to /dev/hpet ... certainly, you could start
by deprecating it and making sure distroes don't enable it.
> > Any comments on the rest?
>
> Looks fine. I would have preferred a series of smaller patches, but my
> feelings on this are not strong enough to split it myself or to ask you
> to do it.
OK, I'll send along the updated patch (against 2.6.27-rc1).
- Dave
From: David Brownell <[email protected]>
Minor /dev/hpet updates and bugfixes:
* Remove dead code, mostly remnants of an incomplete/unusable
kernel interface ... noted when addressing "sparse" warnings:
+ hpet_unregister() and a routine it calls
+ hpet_task and all references, including hpet_task_lock
+ hpet_data.hd_flags (and HPET_DATA_PLATFORM)
* Correct and improve boot message:
+ displays *counter* (shared between comparators) bit width,
not *timer* bit widths (which are often mixed)
+ relabel "timers" as "comparators"; this is less confusing,
they are not independent like normal timers are (sigh)
+ display MHz not Hz; it's never less than 10 MHz.
* Tighten and correct the userspace interface code
+ don't accidentally program comparators in 64-bit mode using
32-bit values ... always force comparators into 32-bit mode
+ provide the correct bit definition flagging comparators with
periodic capability ... the ABI is unchanged
* Update Documentation/hpet.txt
+ be more correct and current
+ expand description a bit
+ don't mention that now-gone kernel interface
Plus, add a FIXME comment for something that could cause big trouble
on systems with more capable HPETs than at least Intel seems to ship.
It seems that few folk use this userspace interface; it's not very
usable given the general lack of HPET IRQ routing. I'm told that
the only real point of it any more is to mmap for fast timestamps;
IMO that's handled better through the gettimeofday() vsyscall.
Signed-off-by: David Brownell <[email protected]>
---
I CC'd everyone who MAINTAINERS says maintains HPET. Odd to have
four entries!!
And re that kernel interface ... IMO, worth having a relatively
generic interface for hardware timers instead of inventing a new
interface for each new bit of hardware. Embedded systems tend to
have at least half a dozen timers to spare.
Documentation/hpet.txt | 43 +++++++++++------------
arch/x86/kernel/hpet.c | 6 ++-
drivers/char/hpet.c | 90 +++++++++++++------------------------------------
include/linux/hpet.h | 11 -----
4 files changed, 51 insertions(+), 99 deletions(-)
--- a/Documentation/hpet.txt 2008-07-27 15:48:04.000000000 -0700
+++ b/Documentation/hpet.txt 2008-07-28 00:37:15.000000000 -0700
@@ -1,21 +1,32 @@
High Precision Event Timer Driver for Linux
-The High Precision Event Timer (HPET) hardware is the future replacement
-for the 8254 and Real Time Clock (RTC) periodic timer functionality.
-Each HPET can have up to 32 timers. It is possible to configure the
-first two timers as legacy replacements for 8254 and RTC periodic timers.
-A specification done by Intel and Microsoft can be found at
-<http://www.intel.com/technology/architecture/hpetspec.htm>.
+The High Precision Event Timer (HPET) hardware follows a specification
+by Intel and Microsoft which can be found at
+
+ http://www.intel.com/technology/architecture/hpetspec.htm
+
+Each HPET has one fixed-rate counter (at 10+ MHz, hence "High Precision")
+and up to 32 comparators. Normally three or more comparators are provided,
+each of which can generate oneshot interupts and at least one of which has
+additional hardware to support periodic interrupts. The comparators are
+also called "timers", which can be misleading since usually timers are
+independent of each other ... these share a counter, complicating resets.
+
+HPET devices can support two interrupt routing modes. In one mode, the
+comparators are additional interrupt sources with no particular system
+role. Many x86 BIOS writers don't route HPET interrupts at all, which
+prevents use of that mode. They support the other "legacy replacement"
+mode where the first two comparators block interrupts from 8254 timers
+and from the RTC.
The driver supports detection of HPET driver allocation and initialization
of the HPET before the driver module_init routine is called. This enables
platform code which uses timer 0 or 1 as the main timer to intercept HPET
initialization. An example of this initialization can be found in
-arch/i386/kernel/time_hpet.c.
+arch/x86/kernel/hpet.c.
-The driver provides two APIs which are very similar to the API found in
-the rtc.c driver. There is a user space API and a kernel space API.
-An example user space program is provided below.
+The driver provides a userspace API which resembles the API found in the
+RTC driver framework. An example user space program is provided below.
#include <stdio.h>
#include <stdlib.h>
@@ -286,15 +297,3 @@ out:
return;
}
-
-The kernel API has three interfaces exported from the driver:
-
- hpet_register(struct hpet_task *tp, int periodic)
- hpet_unregister(struct hpet_task *tp)
- hpet_control(struct hpet_task *tp, unsigned int cmd, unsigned long arg)
-
-The kernel module using this interface fills in the ht_func and ht_data
-members of the hpet_task structure before calling hpet_register.
-hpet_control simply vectors to the hpet_ioctl routine and has the same
-commands and respective arguments as the user API. hpet_unregister
-is used to terminate usage of the HPET timer reserved by hpet_register.
--- a/arch/x86/kernel/hpet.c 2008-07-27 15:48:04.000000000 -0700
+++ b/arch/x86/kernel/hpet.c 2008-07-28 00:37:15.000000000 -0700
@@ -115,13 +115,17 @@ static void hpet_reserve_platform_timers
hd.hd_phys_address = hpet_address;
hd.hd_address = hpet;
hd.hd_nirqs = nrtimers;
- hd.hd_flags = HPET_DATA_PLATFORM;
hpet_reserve_timer(&hd, 0);
#ifdef CONFIG_HPET_EMULATE_RTC
hpet_reserve_timer(&hd, 1);
#endif
+ /*
+ * NOTE that hd_irq[] reflects IOAPIC input pins (LEGACY_8254
+ * is wrong for i8259!) not the output IRQ. Many BIOS writers
+ * don't bother configuring *any* comparator interrupts.
+ */
hd.hd_irq[0] = HPET_LEGACY_8254;
hd.hd_irq[1] = HPET_LEGACY_RTC;
--- a/drivers/char/hpet.c 2008-07-27 15:48:04.000000000 -0700
+++ b/drivers/char/hpet.c 2008-07-28 01:22:01.000000000 -0700
@@ -53,6 +53,11 @@
#define HPET_RANGE_SIZE 1024 /* from HPET spec */
+
+/* WARNING -- don't get confused. These macros are never used
+ * to write the (single) counter, and rarely to read it.
+ * They're badly named; to fix, someday.
+ */
#if BITS_PER_LONG == 64
#define write_counter(V, MC) writeq(V, MC)
#define read_counter(MC) readq(MC)
@@ -77,7 +82,7 @@ static struct clocksource clocksource_hp
.rating = 250,
.read = read_hpet,
.mask = CLOCKSOURCE_MASK(64),
- .mult = 0, /*to be caluclated*/
+ .mult = 0, /*to be calculated*/
.shift = 10,
.flags = CLOCK_SOURCE_IS_CONTINUOUS,
};
@@ -86,8 +91,6 @@ static struct clocksource *hpet_clocksou
/* A lock for concurrent access by app and isr hpet activity. */
static DEFINE_SPINLOCK(hpet_lock);
-/* A lock for concurrent intermodule access to hpet and isr hpet activity. */
-static DEFINE_SPINLOCK(hpet_task_lock);
#define HPET_DEV_NAME (7)
@@ -99,7 +102,6 @@ struct hpet_dev {
unsigned long hd_irqdata;
wait_queue_head_t hd_waitqueue;
struct fasync_struct *hd_async_queue;
- struct hpet_task *hd_task;
unsigned int hd_flags;
unsigned int hd_irq;
unsigned int hd_hdwirq;
@@ -173,11 +175,6 @@ static irqreturn_t hpet_interrupt(int ir
writel(isr, &devp->hd_hpet->hpet_isr);
spin_unlock(&hpet_lock);
- spin_lock(&hpet_task_lock);
- if (devp->hd_task)
- devp->hd_task->ht_func(devp->hd_task->ht_data);
- spin_unlock(&hpet_task_lock);
-
wake_up_interruptible(&devp->hd_waitqueue);
kill_fasync(&devp->hd_async_queue, SIGIO, POLL_IN);
@@ -199,8 +196,7 @@ static int hpet_open(struct inode *inode
for (devp = NULL, hpetp = hpets; hpetp && !devp; hpetp = hpetp->hp_next)
for (i = 0; i < hpetp->hp_ntimer; i++)
- if (hpetp->hp_dev[i].hd_flags & HPET_OPEN
- || hpetp->hp_dev[i].hd_task)
+ if (hpetp->hp_dev[i].hd_flags & HPET_OPEN)
continue;
else {
devp = &hpetp->hp_dev[i];
@@ -441,7 +437,11 @@ static int hpet_ioctl_ieon(struct hpet_d
devp->hd_irq = irq;
t = devp->hd_ireqfreq;
v = readq(&timer->hpet_config);
- g = v | Tn_INT_ENB_CNF_MASK;
+
+ /* 64-bit comparators are not yet supported through the ioctls,
+ * so force this into 32-bit mode if it supports both modes
+ */
+ g = v | Tn_32MODE_CNF_MASK | Tn_INT_ENB_CNF_MASK;
if (devp->hd_flags & HPET_PERIODIC) {
write_counter(t, &timer->hpet_compare);
@@ -451,6 +451,12 @@ static int hpet_ioctl_ieon(struct hpet_d
v |= Tn_VAL_SET_CNF_MASK;
writeq(v, &timer->hpet_config);
local_irq_save(flags);
+
+ /* NOTE: what we modify here is a hidden accumulator
+ * register supported by periodic-capable comparators.
+ * We never want to modify the (single) counter; that
+ * would affect all the comparators.
+ */
m = read_counter(&hpet->hpet_mc);
write_counter(t + m + hpetp->hp_delta, &timer->hpet_compare);
} else {
@@ -604,57 +610,6 @@ static int hpet_is_known(struct hpet_dat
return 0;
}
-static inline int hpet_tpcheck(struct hpet_task *tp)
-{
- struct hpet_dev *devp;
- struct hpets *hpetp;
-
- devp = tp->ht_opaque;
-
- if (!devp)
- return -ENXIO;
-
- for (hpetp = hpets; hpetp; hpetp = hpetp->hp_next)
- if (devp >= hpetp->hp_dev
- && devp < (hpetp->hp_dev + hpetp->hp_ntimer)
- && devp->hd_hpet == hpetp->hp_hpet)
- return 0;
-
- return -ENXIO;
-}
-
-#if 0
-int hpet_unregister(struct hpet_task *tp)
-{
- struct hpet_dev *devp;
- struct hpet_timer __iomem *timer;
- int err;
-
- if ((err = hpet_tpcheck(tp)))
- return err;
-
- spin_lock_irq(&hpet_task_lock);
- spin_lock(&hpet_lock);
-
- devp = tp->ht_opaque;
- if (devp->hd_task != tp) {
- spin_unlock(&hpet_lock);
- spin_unlock_irq(&hpet_task_lock);
- return -ENXIO;
- }
-
- timer = devp->hd_timer;
- writeq((readq(&timer->hpet_config) & ~Tn_INT_ENB_CNF_MASK),
- &timer->hpet_config);
- devp->hd_flags &= ~(HPET_IE | HPET_PERIODIC);
- devp->hd_task = NULL;
- spin_unlock(&hpet_lock);
- spin_unlock_irq(&hpet_task_lock);
-
- return 0;
-}
-#endif /* 0 */
-
static ctl_table hpet_table[] = {
{
.ctl_name = CTL_UNNUMBERED,
@@ -809,9 +764,12 @@ int hpet_alloc(struct hpet_data *hdp)
printk("%s %d", i > 0 ? "," : "", hdp->hd_irq[i]);
printk("\n");
- 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);
+ printk(KERN_INFO
+ "hpet%u: %u comparators, %d-bit %u.%06u MHz counter\n",
+ hpetp->hp_which, hpetp->hp_ntimer,
+ cap & HPET_COUNTER_SIZE_MASK ? 64 : 32,
+ (unsigned) (hpetp->hp_tick_freq / 1000000),
+ (unsigned) (hpetp->hp_tick_freq % 1000000));
mcfg = readq(&hpet->hpet_config);
if ((mcfg & HPET_ENABLE_CNF_MASK) == 0) {
--- a/include/linux/hpet.h 2008-07-27 15:48:04.000000000 -0700
+++ b/include/linux/hpet.h 2008-07-28 00:37:15.000000000 -0700
@@ -91,23 +91,14 @@ struct hpet {
* exported interfaces
*/
-struct hpet_task {
- void (*ht_func) (void *);
- void *ht_data;
- void *ht_opaque;
-};
-
struct hpet_data {
unsigned long hd_phys_address;
void __iomem *hd_address;
unsigned short hd_nirqs;
- unsigned short hd_flags;
unsigned int hd_state; /* timer allocated */
unsigned int hd_irq[HPET_MAX_TIMERS];
};
-#define HPET_DATA_PLATFORM 0x0001 /* platform call to hpet_alloc */
-
static inline void hpet_reserve_timer(struct hpet_data *hd, int timer)
{
hd->hd_state |= (1 << timer);
@@ -125,7 +116,7 @@ struct hpet_info {
unsigned short hi_timer;
};
-#define HPET_INFO_PERIODIC 0x0001 /* timer is periodic */
+#define HPET_INFO_PERIODIC 0x0010 /* periodic-capable comparator */
#define HPET_IE_ON _IO('h', 0x01) /* interrupt on */
#define HPET_IE_OFF _IO('h', 0x02) /* interrupt off */
David Brownell wrote:
>
> On x86_64 the vsyscall stuff should kick in for gettimeofday()
> when HPET is in use, eliminating the need for /dev/hpet mmapping.
>
> I don't really know vsyscalls ... but if that could be done on
> i386 systems, the last argument for /dev/hpet would seem gone...
>
i386 already uses vsyscalls.
-hpa
* David Brownell <[email protected]> wrote:
> From: David Brownell <[email protected]>
>
> Minor /dev/hpet updates and bugfixes:
>
> * Remove dead code, mostly remnants of an incomplete/unusable
> kernel interface ... noted when addressing "sparse" warnings:
> + hpet_unregister() and a routine it calls
> + hpet_task and all references, including hpet_task_lock
> + hpet_data.hd_flags (and HPET_DATA_PLATFORM)
>
> * Correct and improve boot message:
> + displays *counter* (shared between comparators) bit width,
> not *timer* bit widths (which are often mixed)
> + relabel "timers" as "comparators"; this is less confusing,
> they are not independent like normal timers are (sigh)
> + display MHz not Hz; it's never less than 10 MHz.
>
> * Tighten and correct the userspace interface code
> + don't accidentally program comparators in 64-bit mode using
> 32-bit values ... always force comparators into 32-bit mode
> + provide the correct bit definition flagging comparators with
> periodic capability ... the ABI is unchanged
>
> * Update Documentation/hpet.txt
> + be more correct and current
> + expand description a bit
> + don't mention that now-gone kernel interface
>
> Plus, add a FIXME comment for something that could cause big trouble
> on systems with more capable HPETs than at least Intel seems to ship.
>
> It seems that few folk use this userspace interface; it's not very
> usable given the general lack of HPET IRQ routing. I'm told that
> the only real point of it any more is to mmap for fast timestamps;
> IMO that's handled better through the gettimeofday() vsyscall.
>
> Signed-off-by: David Brownell <[email protected]>
> ---
> I CC'd everyone who MAINTAINERS says maintains HPET. Odd to have
> four entries!!
>
> And re that kernel interface ... IMO, worth having a relatively
> generic interface for hardware timers instead of inventing a new
> interface for each new bit of hardware. Embedded systems tend to have
> at least half a dozen timers to spare.
i've asked Clemens of how to merge this and we'll do it via
tip/timers/hpet - so i queued it up there with the ACK of Clemens.
I also merged it up to latest -git. (the documentation bits already
moved, etc.)
I suspect this is a 2.6.27 change, given its fix nature, but it will
need some cooking in linux-next (via auto-timers-next) before this can
be sent to Linus.
Ingo
* Ingo Molnar <[email protected]> wrote:
> I suspect this is a 2.6.27 change, given its fix nature, but it will
> need some cooking in linux-next (via auto-timers-next) before this can
> be sent to Linus.
-tip testing found a build failure caused by this patch:
drivers/built-in.o: In function `hpet_alloc':
: undefined reference to `__udivdi3'
drivers/built-in.o: In function `hpet_alloc':
: undefined reference to `__umoddi3'
with:
http://redhat.com/~mingo/misc/config-Thu_Jul_31_18_46_31_CEST_2008.bad
Please send a delta fix, i've already applied your previous patch and
pushed it out, you can get that tree via:
http://people.redhat.com/mingo/tip.git/README
... and do something like:
git-checkout tip/timers/hpet
The last commit is yours.
Thanks,
Ingo
On Thursday 31 July 2008, Ingo Molnar wrote:
> ? drivers/built-in.o: In function `hpet_alloc':
> ? : undefined reference to `__udivdi3'
> ? drivers/built-in.o: In function `hpet_alloc':
> ? : undefined reference to `__umoddi3'
Verified on 64 and 32 bit builds.
Signed-off-by: David Brownell <[email protected]>
--- a/drivers/char/hpet.c 2008-07-31 11:22:07.000000000 -0700
+++ b/drivers/char/hpet.c 2008-07-31 11:21:15.000000000 -0700
@@ -701,6 +701,7 @@ int hpet_alloc(struct hpet_data *hdp)
static struct hpets *last = NULL;
unsigned long period;
unsigned long long temp;
+ u32 remainder;
/*
* hpet_alloc can be called by platform dependent code.
@@ -764,12 +765,13 @@ int hpet_alloc(struct hpet_data *hdp)
printk("%s %d", i > 0 ? "," : "", hdp->hd_irq[i]);
printk("\n");
+ temp = hpetp->hp_tick_freq;
+ remainder = do_div(temp, 1000000);
printk(KERN_INFO
"hpet%u: %u comparators, %d-bit %u.%06u MHz counter\n",
hpetp->hp_which, hpetp->hp_ntimer,
cap & HPET_COUNTER_SIZE_MASK ? 64 : 32,
- (unsigned) (hpetp->hp_tick_freq / 1000000),
- (unsigned) (hpetp->hp_tick_freq % 1000000));
+ (unsigned) temp, remainder);
mcfg = readq(&hpet->hpet_config);
if ((mcfg & HPET_ENABLE_CNF_MASK) == 0) {
* David Brownell <[email protected]> wrote:
> On Thursday 31 July 2008, Ingo Molnar wrote:
> > ? drivers/built-in.o: In function `hpet_alloc':
> > ? : undefined reference to `__udivdi3'
> > ? drivers/built-in.o: In function `hpet_alloc':
> > ? : undefined reference to `__umoddi3'
>
> Verified on 64 and 32 bit builds.
applied to tip/timers/hpet - thanks David!
Ingo