From: "Pallipadi, Venkatesh" <venkatesh.pallipadi@intel.com>
To: linux-kernel@vger.kernel.org, akpm@linux-foundation.org
Cc: Nils Carlson <nils.carlson@ericsson.com>,
David Brownell <david-b@pacbell.net>
Subject: [PATCH] hpet: hpet driver periodic timer setup bug fixes
Date: Mon, 10 Aug 2009 15:05:04 -0700 [thread overview]
Message-ID: <20090810220503.GA29239@linux-os.sc.intel.com> (raw)
From: Nils Carlson <nils.carlson@ericsson.com>
The periodic interrupt from drivers/char/hpet.c does not work correctly,
both when using the periodic capability of the hardware and while
emulating the periodic interrupt (when hardware does not support
periodic mode).
With timers capable of periodic interrupts, the comparator field is first
set with the period value followed by set of hidden accumulator,
which has the side effect of overwriting the comparator value. This
results in wrong periodicity for the interrupts. For,
periodic interrupts to work, following steps are necessary, in that order.
* Set config with Tn_VAL_SET_CNF bit
* Write to hidden accumulator, the value written is the time when the
first interrupt should be generated
* Write compartor with period interval for subsequent interrupts
(http://www.intel.com/hardwaredesign/hpetspec_1.pdf )
When emulating periodic timer with timers not capable of periodic
interrupt, driver is adding the period to counter value instead of
comparator value, which causes slow drift when using this emulation.
Also, driver seems to add hpetp->hp_delta both while setting up
periodic interrupt and while emulating periodic interrupts with timers
not capable of doing periodic interrupts. This hp_delta will result in
slower than expected interrupt rate and should not be used while setting
the interval.
Signed-off-by: Venkatesh Pallipadi <venkatesh.pallipadi@intel.com>
Signed-off-by: Nils Carlson <nils.carlson@ericsson.com>
---
drivers/char/hpet.c | 21 ++++++++++++---------
1 files changed, 12 insertions(+), 9 deletions(-)
diff --git a/drivers/char/hpet.c b/drivers/char/hpet.c
index 4a9f349..70a770a 100644
--- a/drivers/char/hpet.c
+++ b/drivers/char/hpet.c
@@ -166,9 +166,8 @@ static irqreturn_t hpet_interrupt(int irq, void *data)
unsigned long m, t;
t = devp->hd_ireqfreq;
- m = read_counter(&devp->hd_hpet->hpet_mc);
- write_counter(t + m + devp->hd_hpets->hp_delta,
- &devp->hd_timer->hpet_compare);
+ m = read_counter(&devp->hd_timer->hpet_compare);
+ write_counter(t + m, &devp->hd_timer->hpet_compare);
}
if (devp->hd_flags & HPET_SHARED_IRQ)
@@ -504,21 +503,25 @@ static int hpet_ioctl_ieon(struct hpet_dev *devp)
g = v | Tn_32MODE_CNF_MASK | Tn_INT_ENB_CNF_MASK;
if (devp->hd_flags & HPET_PERIODIC) {
- write_counter(t, &timer->hpet_compare);
g |= Tn_TYPE_CNF_MASK;
- v |= Tn_TYPE_CNF_MASK;
- writeq(v, &timer->hpet_config);
- v |= Tn_VAL_SET_CNF_MASK;
+ v |= Tn_TYPE_CNF_MASK | Tn_VAL_SET_CNF_MASK;
writeq(v, &timer->hpet_config);
local_irq_save(flags);
- /* NOTE: what we modify here is a hidden accumulator
+ /*
+ * NOTE: First we modify the hidden accumulator
* register supported by periodic-capable comparators.
* We never want to modify the (single) counter; that
- * would affect all the comparators.
+ * would affect all the comparators. The value written
+ * is the counter value when the first interrupt is due.
*/
m = read_counter(&hpet->hpet_mc);
write_counter(t + m + hpetp->hp_delta, &timer->hpet_compare);
+ /*
+ * Then we modify the comparator, indicating the period
+ * for subsequent interrupt.
+ */
+ write_counter(t, &timer->hpet_compare);
} else {
local_irq_save(flags);
m = read_counter(&hpet->hpet_mc);
--
1.6.0.6
next reply other threads:[~2009-08-10 22:05 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-08-10 22:05 Pallipadi, Venkatesh [this message]
2009-08-11 4:03 ` [PATCH] hpet: hpet driver periodic timer setup bug fixes Andrew Morton
2009-08-11 18:07 ` Pallipadi, Venkatesh
2009-08-27 19:17 ` Goldwyn Rodrigues
2009-08-28 6:59 ` Nils Carlson
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20090810220503.GA29239@linux-os.sc.intel.com \
--to=venkatesh.pallipadi@intel.com \
--cc=akpm@linux-foundation.org \
--cc=david-b@pacbell.net \
--cc=linux-kernel@vger.kernel.org \
--cc=nils.carlson@ericsson.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox