[patch 2.6.26] /dev/hpet - fixes and cleanup - Kernel

This is a discussion on [patch 2.6.26] /dev/hpet - fixes and cleanup - Kernel ; From: David Brownell 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 + ...

+ Reply to Thread
Results 1 to 17 of 17

Thread: [patch 2.6.26] /dev/hpet - fixes and cleanup

  1. [patch 2.6.26] /dev/hpet - fixes and cleanup

    From: David Brownell

    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
    ---
    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
    -.
    +The High Precision Event Timer (HPET) hardware follows a specification
    +by Intel and Microsoft which can be found at
    +
    + http://www.intel.com/technology/arch...e/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
    #include
    @@ -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 */
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  2. Re: [patch 2.6.26] /dev/hpet - fixes and cleanup

    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
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  3. Re: [patch 2.6.26] /dev/hpet - fixes and cleanup

    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
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  4. Re: [patch 2.6.26] /dev/hpet - fixes and cleanup

    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];
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  5. Re: [patch 2.6.26] /dev/hpet - fixes and cleanup

    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...u-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
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  6. [patch 2.6.26-git] /dev/hpet - fixes and cleanup

    From: David Brownell

    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
    ---
    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
    -.
    +The High Precision Event Timer (HPET) hardware follows a specification
    +by Intel and Microsoft which can be found at
    +
    + http://www.intel.com/technology/arch...e/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
    #include
    @@ -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 */
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  7. Re: [patch 2.6.26] /dev/hpet - fixes and cleanup

    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...
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  8. Re: [patch 2.6.26-git] /dev/hpet - fixes and cleanup

    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 ), which _is_ used.


    Regards,
    Clemens
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  9. Re: [patch 2.6.26-git] /dev/hpet - fixes and cleanup

    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 ), 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

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  10. Re: [patch 2.6.26-git] /dev/hpet - fixes and cleanup

    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_count er_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
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  11. Re: [patch 2.6.26-git] /dev/hpet - fixes and cleanup

    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_count er_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
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  12. [patch 2.6.27-rc1] /dev/hpet - fixes and cleanup

    From: David Brownell

    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
    ---
    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
    -.
    +The High Precision Event Timer (HPET) hardware follows a specification
    +by Intel and Microsoft which can be found at
    +
    + http://www.intel.com/technology/arch...e/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
    #include
    @@ -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 */
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  13. Re: [patch 2.6.26] /dev/hpet - fixes and cleanup

    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
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  14. Re: [patch 2.6.27-rc1] /dev/hpet - fixes and cleanup


    * David Brownell wrote:

    > From: David Brownell
    >
    > 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
    > ---
    > 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
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  15. Re: [patch 2.6.27-rc1] /dev/hpet - fixes and cleanup


    * Ingo Molnar 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..._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
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  16. Re: [patch 2.6.27-rc1] /dev/hpet - fixes and cleanup

    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

    --- 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) {

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  17. Re: [patch 2.6.27-rc1] /dev/hpet - fixes and cleanup


    * David Brownell 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
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

+ Reply to Thread