Hey Venkatesh,
I've been looking into a bug where i386 2.6 kernels do not boot on IBM
e325s if HPET_TIMER is enabled (hpet=disable works around the issue).
When running x86-64 kernels, the issue isn't seen. It appears that after
the hpet is enabled, we stop receiving timer ticks. I've not played on
any other HPET enabled systems, nor have I looked at the HPET spec, so
I'm not sure if this is a hardware issue or not.
The following patch, which uses the x86-64 code for
hpet_timer_stop_set_go() seems to fix the issue.
Your thoughts?
thanks
-john
===== arch/i386/kernel/time_hpet.c 1.10 vs edited =====
--- 1.10/arch/i386/kernel/time_hpet.c 2004-11-02 06:40:42 -08:00
+++ edited/arch/i386/kernel/time_hpet.c 2005-02-02 17:59:27 -08:00
@@ -64,29 +64,30 @@
{
unsigned int cfg;
- /*
- * Stop the timers and reset the main counter.
- */
+/*
+ * Stop the timers and reset the main counter.
+ */
+
cfg = hpet_readl(HPET_CFG);
- cfg &= ~HPET_CFG_ENABLE;
+ cfg &= ~(HPET_CFG_ENABLE | HPET_CFG_LEGACY);
hpet_writel(cfg, HPET_CFG);
hpet_writel(0, HPET_COUNTER);
hpet_writel(0, HPET_COUNTER + 4);
- /*
- * Set up timer 0, as periodic with first interrupt to happen at
- * hpet_tick, and period also hpet_tick.
- */
- cfg = hpet_readl(HPET_T0_CFG);
- cfg |= HPET_TN_ENABLE | HPET_TN_PERIODIC |
- HPET_TN_SETVAL | HPET_TN_32BIT;
- hpet_writel(cfg, HPET_T0_CFG);
- hpet_writel(tick, HPET_T0_CMP);
+/*
+ * Set up timer 0, as periodic with first interrupt to happen at hpet_tick,
+ * and period also hpet_tick.
+ */
+
+ hpet_writel(HPET_TN_ENABLE | HPET_TN_PERIODIC | HPET_TN_SETVAL |
+ HPET_TN_32BIT, HPET_T0_CFG);
+ hpet_writel(hpet_tick, HPET_T0_CMP);
+ hpet_writel(hpet_tick, HPET_T0_CMP); /* AK: why twice? */
+
+/*
+ * Go!
+ */
- /*
- * Go!
- */
- cfg = hpet_readl(HPET_CFG);
cfg |= HPET_CFG_ENABLE | HPET_CFG_LEGACY;
hpet_writel(cfg, HPET_CFG);
On Thursday 03 February 2005 02:05, john stultz wrote:
> Hey Venkatesh,
> I've been looking into a bug where i386 2.6 kernels do not boot on IBM
> e325s if HPET_TIMER is enabled (hpet=disable works around the issue).
> When running x86-64 kernels, the issue isn't seen. It appears that after
FWIW The problem is not limited to the IBM e325. I cannot boot a HPET_TIMER
enabled x86 kernel on any of the various Tyan and MSI opteron boards I have
here without hpet=disable. x86_64 kernels work fine.
Andrew Walrond
Hi John, Andrew,
Can you check whether only the following change makes the problem go
away. If yes, then it looks like a hardware issue.
> hpet_writel(hpet_tick, HPET_T0_CMP);
>+ hpet_writel(hpet_tick, HPET_T0_CMP); /* AK: why twice? */
>
Thanks,
Venki
>-----Original Message-----
>From: john stultz [mailto:[email protected]]
>Sent: Wednesday, February 02, 2005 6:05 PM
>To: Pallipadi, Venkatesh
>Cc: Andi Kleen; lkml; keith maanthey; Max Asbock; Chris McDermott
>Subject: i386 HPET code
>
>Hey Venkatesh,
> I've been looking into a bug where i386 2.6 kernels do
>not boot on IBM
>e325s if HPET_TIMER is enabled (hpet=disable works around the issue).
>When running x86-64 kernels, the issue isn't seen. It appears
>that after
>the hpet is enabled, we stop receiving timer ticks. I've not played on
>any other HPET enabled systems, nor have I looked at the HPET spec, so
>I'm not sure if this is a hardware issue or not.
>
>The following patch, which uses the x86-64 code for
>hpet_timer_stop_set_go() seems to fix the issue.
>
>Your thoughts?
>
>thanks
>-john
>
>
>===== arch/i386/kernel/time_hpet.c 1.10 vs edited =====
>--- 1.10/arch/i386/kernel/time_hpet.c 2004-11-02 06:40:42 -08:00
>+++ edited/arch/i386/kernel/time_hpet.c 2005-02-02
>17:59:27 -08:00
>@@ -64,29 +64,30 @@
> {
> unsigned int cfg;
>
>- /*
>- * Stop the timers and reset the main counter.
>- */
>+/*
>+ * Stop the timers and reset the main counter.
>+ */
>+
> cfg = hpet_readl(HPET_CFG);
>- cfg &= ~HPET_CFG_ENABLE;
>+ cfg &= ~(HPET_CFG_ENABLE | HPET_CFG_LEGACY);
> hpet_writel(cfg, HPET_CFG);
> hpet_writel(0, HPET_COUNTER);
> hpet_writel(0, HPET_COUNTER + 4);
>
>- /*
>- * Set up timer 0, as periodic with first interrupt to happen at
>- * hpet_tick, and period also hpet_tick.
>- */
>- cfg = hpet_readl(HPET_T0_CFG);
>- cfg |= HPET_TN_ENABLE | HPET_TN_PERIODIC |
>- HPET_TN_SETVAL | HPET_TN_32BIT;
>- hpet_writel(cfg, HPET_T0_CFG);
>- hpet_writel(tick, HPET_T0_CMP);
>+/*
>+ * Set up timer 0, as periodic with first interrupt to happen
>at hpet_tick,
>+ * and period also hpet_tick.
>+ */
>+
>+ hpet_writel(HPET_TN_ENABLE | HPET_TN_PERIODIC | HPET_TN_SETVAL |
>+ HPET_TN_32BIT, HPET_T0_CFG);
>+ hpet_writel(hpet_tick, HPET_T0_CMP);
>+ hpet_writel(hpet_tick, HPET_T0_CMP); /* AK: why twice? */
>+
>+/*
>+ * Go!
>+ */
>
>- /*
>- * Go!
>- */
>- cfg = hpet_readl(HPET_CFG);
> cfg |= HPET_CFG_ENABLE | HPET_CFG_LEGACY;
> hpet_writel(cfg, HPET_CFG);
>
>
>
>
On Thu, 2005-02-03 at 06:28 -0800, Pallipadi, Venkatesh wrote:
> Can you check whether only the following change makes the problem go
> away. If yes, then it looks like a hardware issue.
>
> > hpet_writel(hpet_tick, HPET_T0_CMP);
> >+ hpet_writel(hpet_tick, HPET_T0_CMP); /* AK: why twice? */
> >
Yep. Adding only the second write seems to make the box boot.
Since this isn't just affecting our hardware (see Andrew Walrond's
comment in the thread), would doing two writes like x86-64 does be
acceptable?
thanks
-john
On Thu, Feb 03, 2005 at 11:30:56AM -0800, john stultz wrote:
> On Thu, 2005-02-03 at 06:28 -0800, Pallipadi, Venkatesh wrote:
> > Can you check whether only the following change makes the problem go
> > away. If yes, then it looks like a hardware issue.
> >
> > > hpet_writel(hpet_tick, HPET_T0_CMP);
> > >+ hpet_writel(hpet_tick, HPET_T0_CMP); /* AK: why twice? */
> > >
>
> Yep. Adding only the second write seems to make the box boot.
>
> Since this isn't just affecting our hardware (see Andrew Walrond's
> comment in the thread), would doing two writes like x86-64 does be
> acceptable?
>
Yes. As this is just the initialization code, I think adding second write
is OK. But, I am not sure why two writes are required and will the two write
be sufficient for all systems. I don't seem to remember anything about this
in HPET specs. I will double check it.
Basically I am thinking of something like this will be a good generic solution
in place of simple two writes.
for (i = 0 ; i <some number for max retries>; i++) {
hpet_writel(hpet_tick, HPET_T0_CMP);
if (hpet_tick == hpet_readl(hpet_tick, HPET_T0_CMP))
break;
}
I think we can wait for result from Andrew's system and chose either one
of the above approaches.
Thanks,
Venki
> Basically I am thinking of something like this will be a good generic solution
> in place of simple two writes.
>
> for (i = 0 ; i <some number for max retries>; i++) {
> hpet_writel(hpet_tick, HPET_T0_CMP);
> if (hpet_tick == hpet_readl(hpet_tick, HPET_T0_CMP))
> break;
> }
Makes sense. There were so many bugs in PIT timer access over time,
it would be probably a miracle if the hardware engineers got all
the HPET implementations right ;-)
If you do a fix like this please change x86-64 too.
-Andi
On Thu, Feb 03, 2005 at 06:28:27AM -0800, Pallipadi, Venkatesh wrote:
>
> Hi John, Andrew,
>
>
> Can you check whether only the following change makes the problem go
> away. If yes, then it looks like a hardware issue.
>
> > hpet_writel(hpet_tick, HPET_T0_CMP);
> >+ hpet_writel(hpet_tick, HPET_T0_CMP); /* AK: why twice? */
Ask Vojtech (cced), he wrote the x86-64 HPET code.
-Andi
On Thursday 03 February 2005 20:02, Venkatesh Pallipadi wrote:
> On Thu, Feb 03, 2005 at 11:30:56AM -0800, john stultz wrote:
> > On Thu, 2005-02-03 at 06:28 -0800, Pallipadi, Venkatesh wrote:
> > > Can you check whether only the following change makes the problem go
> > > away. If yes, then it looks like a hardware issue.
> > >
> > > > hpet_writel(hpet_tick, HPET_T0_CMP);
> > > >+ hpet_writel(hpet_tick, HPET_T0_CMP); /* AK: why twice? */
> >
> > Yep. Adding only the second write seems to make the box boot.
> >
Just to confirm that this also fixes the problem for two types of MSI dual
opteron motherboards. (MSI K8D Master 2/3)
Andrew Walrond
On Thu, Feb 03, 2005 at 10:30:26PM +0100, Andi Kleen wrote:
> On Thu, Feb 03, 2005 at 06:28:27AM -0800, Pallipadi, Venkatesh wrote:
> >
> > Hi John, Andrew,
> >
> >
> > Can you check whether only the following change makes the problem go
> > away. If yes, then it looks like a hardware issue.
> >
> > > hpet_writel(hpet_tick, HPET_T0_CMP);
> > >+ hpet_writel(hpet_tick, HPET_T0_CMP); /* AK: why twice? */
>
>
> Ask Vojtech (cced), he wrote the x86-64 HPET code.
Can you add some background of the thread?
--
Vojtech Pavlik
SuSE Labs, SuSE CR
On Fri, Feb 04, 2005 at 09:02:38PM +0100, Vojtech Pavlik wrote:
> On Thu, Feb 03, 2005 at 10:30:26PM +0100, Andi Kleen wrote:
> > On Thu, Feb 03, 2005 at 06:28:27AM -0800, Pallipadi, Venkatesh wrote:
> > >
> > > Hi John, Andrew,
> > >
> > >
> > > Can you check whether only the following change makes the problem go
> > > away. If yes, then it looks like a hardware issue.
> > >
> > > > hpet_writel(hpet_tick, HPET_T0_CMP);
> > > >+ hpet_writel(hpet_tick, HPET_T0_CMP); /* AK: why twice? */
> >
> >
> > Ask Vojtech (cced), he wrote the x86-64 HPET code.
>
> It took me a while to remember, but:
>
> The first write after writing TN_SETVAL to the config register sets the
> counter value, the second write sets the threshold.
>
> When you only do the first write you never set the threshold and
> interrupts won't be generated properly.
That means it's not a bug, but a (documented) feature.
--
Vojtech Pavlik
SuSE Labs, SuSE CR
On Thu, Feb 03, 2005 at 10:30:26PM +0100, Andi Kleen wrote:
> On Thu, Feb 03, 2005 at 06:28:27AM -0800, Pallipadi, Venkatesh wrote:
> >
> > Hi John, Andrew,
> >
> >
> > Can you check whether only the following change makes the problem go
> > away. If yes, then it looks like a hardware issue.
> >
> > > hpet_writel(hpet_tick, HPET_T0_CMP);
> > >+ hpet_writel(hpet_tick, HPET_T0_CMP); /* AK: why twice? */
>
>
> Ask Vojtech (cced), he wrote the x86-64 HPET code.
It took me a while to remember, but:
The first write after writing TN_SETVAL to the config register sets the
counter value, the second write sets the threshold.
When you only do the first write you never set the threshold and
interrupts won't be generated properly.
--
Vojtech Pavlik
SuSE Labs, SuSE CR
This patch fixes the issue with HPET on some platforms.
According to Vojtech Pavlik:
The first write after writing TN_SETVAL to the config register sets the
counter value, the second write sets the threshold.
When you only do the first write you never set the threshold and
interrupts won't be generated properly.
Thanks to John Stultz and Andrew Walrond for reporting, root causing
the issue and verifying this fix.
Signed-off-by: Venkatesh Pallipadi <[email protected]>
--- linux-2.6.10/arch/i386/kernel/time_hpet.c.org 2005-02-04 12:04:09.000000000 -0800
+++ linux-2.6.10/arch/i386/kernel/time_hpet.c 2005-02-04 18:01:25.000000000 -0800
@@ -81,6 +81,11 @@ static int hpet_timer_stop_set_go(unsign
cfg |= HPET_TN_ENABLE | HPET_TN_PERIODIC |
HPET_TN_SETVAL | HPET_TN_32BIT;
hpet_writel(cfg, HPET_T0_CFG);
+ /*
+ * Some systems seems to need two writes to HPET_T0_CMP,
+ * to get interrupts working
+ */
+ hpet_writel(tick, HPET_T0_CMP);
hpet_writel(tick, HPET_T0_CMP);
/*
On Friday 04 February 2005 23:41, Venkatesh Pallipadi wrote:
> + /*
> + * Some systems seems to need two writes to HPET_T0_CMP,
> + * to get interrupts working
> + */
I think you should update this comment in light of the information from
Vojtech:
/*
* The first write after writing TN_SETVAL to the config register sets the
* counter value, the second write sets the threshold.
*/
Andrew Walrond
Venkatesh Pallipadi wrote:
> + /*
> + * Some systems seems to need two writes to HPET_T0_CMP,
> + * to get interrupts working
> + */
> + hpet_writel(tick, HPET_T0_CMP);
> hpet_writel(tick, HPET_T0_CMP);
Is it known which platforms require two, and which ones require
one write? Is it cost-effective to #if CONFIG_ the second
write?
--
Giuseppe "Oblomov" Bilotta
Can't you see
It all makes perfect sense
Expressed in dollar and cents
Pounds shillings and pence
(Roger Waters)
>-----Original Message-----
>From: [email protected]
>[mailto:[email protected]] On Behalf Of
>Giuseppe Bilotta
>Sent: Sunday, February 06, 2005 7:59 AM
>To: [email protected]
>Subject: Re: [PATCH][i386] HPET setup, duplicate HPET_T0_CMP
>needed for some platforms
>
>Venkatesh Pallipadi wrote:
>> + /*
>> + * Some systems seems to need two writes to HPET_T0_CMP,
>> + * to get interrupts working
>> + */
>> + hpet_writel(tick, HPET_T0_CMP);
>> hpet_writel(tick, HPET_T0_CMP);
>
>Is it known which platforms require two, and which ones require
>one write? Is it cost-effective to #if CONFIG_ the second
>write?
Additional write should not be performace critical, as this code is
called
only once, during boot up (and again during system suspend-resume).
Thanks,
Venki