2001-11-12 13:14:39

by Jeronimo Pellegrini

[permalink] [raw]
Subject: [PATCH] VIA timer fix was removed?

Hello.

The following patch (introduced by Vojtech Pavlik some time ago) was
removed somewhere between 2.4.14 and 2.4.15-pre3.
Without it, the timer counter is reset to a wrong value and
gettimeofday() starts to return strange values.

Nothing aboutit is mentioned in the changelog, so I suppose it wasn't
supposed to be removed?

J.

--- linux-2.4.15-pre3/arch/i386/kernel/time.c Sun Nov 11 21:33:31 2001
+++ linux-2.4.15-pre3-new/arch/i386/kernel/time.c Mon Nov 12 10:45:57 2001
@@ -501,6 +501,14 @@

count = inb_p(0x40); /* read the latched count */
count |= inb(0x40) << 8;
+
+ if (count > LATCH-1) {
+ outb_p(0x34, 0x43);
+ outb_p(LATCH & 0xff, 0x40);
+ outb(LATCH >> 8, 0x40);
+ count = LATCH - 1;
+ }
+
spin_unlock(&i8253_lock);

count = ((LATCH-1) - count) * TICK_SIZE;


2001-11-12 15:43:32

by Andrzej Krzysztofowicz

[permalink] [raw]
Subject: Re: [PATCH] VIA timer fix was removed?

> The following patch (introduced by Vojtech Pavlik some time ago) was
> removed somewhere between 2.4.14 and 2.4.15-pre3.
> Without it, the timer counter is reset to a wrong value and
> gettimeofday() starts to return strange values
>
> Nothing aboutit is mentioned in the changelog, so I suppose it wasn't
> supposed to be removed?

Maybe, it happens because somebody forgot to comment why this code is
necessary here ?
Just a guess...

> --- linux-2.4.15-pre3/arch/i386/kernel/time.c Sun Nov 11 21:33:31 2001
> +++ linux-2.4.15-pre3-new/arch/i386/kernel/time.c Mon Nov 12 10:45:57 2001
> @@ -501,6 +501,14 @@
>
> count = inb_p(0x40); /* read the latched count */
> count |= inb(0x40) << 8;
> +
> + if (count > LATCH-1) {
> + outb_p(0x34, 0x43);
> + outb_p(LATCH & 0xff, 0x40);
> + outb(LATCH >> 8, 0x40);
> + count = LATCH - 1;
> + }
> +
> spin_unlock(&i8253_lock);

--
=======================================================================
Andrzej M. Krzysztofowicz [email protected]
phone (48)(58) 347 14 61
Faculty of Applied Phys. & Math., Technical University of Gdansk

2001-11-12 16:06:38

by Jeronimo Pellegrini

[permalink] [raw]
Subject: Re: [PATCH] VIA timer fix was removed?

On Mon, Nov 12, 2001 at 03:48:24PM +0100, Andrzej Krzysztofowicz wrote:
> > The following patch (introduced by Vojtech Pavlik some time ago) was
> > removed somewhere between 2.4.14 and 2.4.15-pre3.
> > Without it, the timer counter is reset to a wrong value and
> > gettimeofday() starts to return strange values
> >
> > Nothing aboutit is mentioned in the changelog, so I suppose it wasn't
> > supposed to be removed?
>
> Maybe, it happens because somebody forgot to comment why this code is
> necessary here ?
> Just a guess...

Then perhaps this would be a good idea?

J.

--- linux-2.4.15-pre3/arch/i386/kernel/time.c Sun Nov 11 21:33:31 2001
+++ linux-2.4.15-pre3-new/arch/i386/kernel/time.c Mon Nov 12 14:04:20 2001
@@ -501,6 +501,19 @@

count = inb_p(0x40); /* read the latched count */
count |= inb(0x40) << 8;
+
+ /*
+ * When using some via chipsets (as the vt82c686a, for example)
+ * the system timer counter (i8253) should be reprogrammed in
+ * this case, otherwise it may be reset to a wrong value.
+ */
+ if (count > LATCH-1) {
+ outb_p(0x34, 0x43);
+ outb_p(LATCH & 0xff, 0x40);
+ outb(LATCH >> 8, 0x40);
+ count = LATCH - 1;
+ }
+
spin_unlock(&i8253_lock);

count = ((LATCH-1) - count) * TICK_SIZE;

2001-11-12 18:50:34

by Nils Philippsen

[permalink] [raw]
Subject: Re: [PATCH] VIA timer fix was removed?

On Mon, 2001-11-12 at 17:05, Jeronimo Pellegrini wrote:
> On Mon, Nov 12, 2001 at 03:48:24PM +0100, Andrzej Krzysztofowicz wrote:
> > > The following patch (introduced by Vojtech Pavlik some time ago) was
> > > removed somewhere between 2.4.14 and 2.4.15-pre3.
> > > Without it, the timer counter is reset to a wrong value and
> > > gettimeofday() starts to return strange values
> > >
> > > Nothing aboutit is mentioned in the changelog, so I suppose it wasn't
> > > supposed to be removed?
> >
> > Maybe, it happens because somebody forgot to comment why this code is
> > necessary here ?
> > Just a guess...
>
> Then perhaps this would be a good idea?

I have seen IBM machines (Netfinity 6000R) where this code got triggered
even though they didn't have VIA chipsets/timers. Seems to have caused
some problems there and I removed the code (in the custom kernel running
on those machines).

Nils
--
Nils Philippsen / Berliner Stra?e 39 / D-71229 Leonberg //
+49.7152.209647
[email protected] / [email protected] /
[email protected]
Ever noticed that common sense isn't really all that common?


Attachments:
(No filename) (232.00 B)

2001-11-12 19:00:54

by Jeronimo Pellegrini

[permalink] [raw]
Subject: Re: [PATCH] VIA timer fix was removed?

> I have seen IBM machines (Netfinity 6000R) where this code got triggered
> even though they didn't have VIA chipsets/timers. Seems to have caused
> some problems there and I removed the code (in the custom kernel running
> on those machines).

#ifdefs and a question in config, maybe?

J.

--

2001-11-12 19:22:24

by Nils Philippsen

[permalink] [raw]
Subject: Re: [PATCH] VIA timer fix was removed?

On Mon, 2001-11-12 at 20:00, Jeronimo Pellegrini wrote:
> > I have seen IBM machines (Netfinity 6000R) where this code got triggered
> > even though they didn't have VIA chipsets/timers. Seems to have caused
> > some problems there and I removed the code (in the custom kernel running
> > on those machines).
>
> #ifdefs and a question in config, maybe?

It wasn't something that left those machines. People with more intimate
knowledge of this code and maybe those IBMs should take care.

Nils
--
Nils Philippsen / Berliner Stra?e 39 / D-71229 Leonberg //
+49.7152.209647
[email protected] / [email protected] /
[email protected]
Ever noticed that common sense isn't really all that common?


Attachments:
(No filename) (232.00 B)

2001-11-12 21:17:11

by Neale Banks

[permalink] [raw]
Subject: Re: [PATCH] VIA timer fix was removed?

On Mon, 12 Nov 2001, Jeronimo Pellegrini wrote:

> > I have seen IBM machines (Netfinity 6000R) where this code got triggered
> > even though they didn't have VIA chipsets/timers. Seems to have caused
> > some problems there and I removed the code (in the custom kernel running
> > on those machines).
>
> #ifdefs and a question in config, maybe?

Or a boot-time option to disable this fix?

Attached (untested) patch against 21.2.20 (which still has the $SUBJECT
code) should implement timer=no-via686a option to disable this. Hopefully
I'll get it tested in the next day or two.

Regards,
Neale.


Attachments:
2.2.20-time.diff (2.14 kB)

2001-11-12 21:25:31

by Alan

[permalink] [raw]
Subject: Re: [PATCH] VIA timer fix was removed?

> Attached (untested) patch against 21.2.20 (which still has the $SUBJECT
> code) should implement timer=no-via686a option to disable this. Hopefully
> I'll get it tested in the next day or two.

This isnt the real problem - we are seeing it triggered by cases we dont
underatand that seem to be software. We need to find those

2001-11-12 21:48:33

by Neale Banks

[permalink] [raw]
Subject: Re: [PATCH] VIA timer fix was removed?


> > Attached (untested) patch against 21.2.20 (which still has the $SUBJECT
> > code) should implement timer=no-via686a option to disable this. Hopefully
> > I'll get it tested in the next day or two.

Oops, my typo: s/21.2.20/2.2.20/

On Mon, 12 Nov 2001, Alan Cox wrote:

> This isnt the real problem - we are seeing it triggered by cases we dont
> underatand that seem to be software. We need to find those

Fair enough (and I make no pretence to knowing where to start to address
the real problem here).

Meanwhile, for those of us that have either suspected or confirmed
problems with the VIA686a workaround (<sigh> possibly due to entirely
different hardware/BIOS bugs? </sigh>), a method of nobbling the VIA686a
fix at compile or boot time could be somewhat useful? Mmm... boot-time is
probably best as it allows easiet experimentation?

Thanks,
Neale.

2001-11-12 21:59:13

by Vojtech Pavlik

[permalink] [raw]
Subject: Re: [PATCH] VIA timer fix was removed?

On Mon, Nov 12, 2001 at 09:31:35PM +0000, Alan Cox wrote:

> > Attached (untested) patch against 21.2.20 (which still has the $SUBJECT
> > code) should implement timer=no-via686a option to disable this. Hopefully
> > I'll get it tested in the next day or two.
>
> This isnt the real problem - we are seeing it triggered by cases we dont
> underatand that seem to be software. We need to find those

I don't think it's software. At least it's definitely not locking. It's
happening on machines where none of the drivers missing the locks are
used (only ftape and analog joystick).

I think it's the old Neptune bug biting us again. I'd like to verify
this theory - do you have any list of people who've seen the VIA bugfix
triggered on non-VIA hardware? Some might be willing to test
experimental patches ...

... like the one attached. It doesn't add anything that already hasn't
been there, but:

1) Should be safe for machines that have
a different bug than VIA. It'll print a message
but won't reset the timer if the > LATCH reading
isn't persistent.

2) Should printk enough data to shed some light
on what is triggering the VIA check.

Patch against 2.4.15-pre4.

--
Vojtech Pavlik
SuSE Labs


Attachments:
(No filename) (1.19 kB)
new-via-check.diff (2.06 kB)
Download all attachments

2001-11-12 22:36:53

by Alan

[permalink] [raw]
Subject: Re: [PATCH] VIA timer fix was removed?

> Meanwhile, for those of us that have either suspected or confirmed
> problems with the VIA686a workaround (<sigh> possibly due to entirely
> different hardware/BIOS bugs? </sigh>), a method of nobbling the VIA686a
> fix at compile or boot time could be somewhat useful? Mmm... boot-time is
> probably best as it allows easiet experimentation?

No need - the workaround is 100% harmless if misapplied

2002-04-08 05:56:58

by Neale Banks

[permalink] [raw]
Subject: Re: [PATCH] VIA timer fix was removed?

Hi Vojtech,

Appended patch:

(a) merges your patch of November 2001 into 2.2.21rc3
(b) adds a boot-time option to explicitly disable the via-timer hacks.

I've been using this patch for a while, but haven't subjected it to more
thorough testing than that it boots OK and doesn't appear to complicate
anything (I might be able to trigger the relevant condition by running the
battery right down on my (old) AcerNote-950, but one of the last times
this happened I also got some pretty nasty file system corruption - so I'm
not too keen to try that one again :-| ).

Anyway, does this patch and my merge of it look correct?

Thanks,
Neale.

--- linux-2.2.21-rc3-orig/arch/i386/kernel/time.c Mon Mar 26 02:37:30 2001
+++ linux-2.2.21-rc3-ntb/arch/i386/kernel/time.c Fri Apr 5 23:04:13 2002
@@ -81,6 +81,8 @@

spinlock_t rtc_lock = SPIN_LOCK_UNLOCKED;

+static int via686a_hacks = 1; /* default to enabled */
+
static inline unsigned long do_fast_gettimeoffset(void)
{
register unsigned long eax asm("ax");
@@ -111,6 +113,54 @@
return delay_at_last_interrupt + edx;
}

+/*
+ * VIA hardware bug workaround with check if it is really needed and
+ * a printk that could tell us what's exactly happening on machines which
+ * trigger the check, but are not VIA-based.
+ *
+ * Must be called with the i8253_spinlock held.
+ */
+
+static void via_reset_and_whine(int *count)
+{
+ static unsigned long last_whine = 0;
+ unsigned long new_whine;
+ int count2;
+
+ new_whine = last_whine;
+
+ outb_p(0x00, 0x43); /* Re-read the timer */
+ count2 = inb_p(0x40);
+ count2 |= inb(0x40) << 8;
+
+ if (time_after(jiffies, last_whine)) {
+ printk(KERN_WARNING "timer.c: VIA bug check triggered. "
+ "Value read %d [%#x], re-read %d [%#x]\n",
+ *count, *count, count2, count2);
+ new_whine = jiffies + HZ;
+ }
+
+ *count = count2;
+
+ if (count2 > LATCH) { /* Still bad */
+ if (time_after(jiffies, last_whine)) {
+ printk(KERN_WARNING "timer.c VIA bug really present. ");
+ new_whine = jiffies + HZ;
+ }
+ if (via686a_hacks) {
+ printk(KERN_WARNING "Resetting PIT timer.\n");
+ outb_p(0x34, 0x43);
+ outb_p(LATCH & 0xff, 0x40);
+ outb(LATCH >> 8, 0x40);
+ } else {
+ printk(KERN_WARNING "But VIA hacks disabled.\n");
+ }
+ *count = LATCH - 1;
+ }
+
+ last_whine = new_whine;
+}
+
#define TICK_SIZE tick

#ifndef CONFIG_X86_TSC
@@ -177,12 +227,8 @@
count |= inb_p(0x40) << 8;

/* VIA686a test code... reset the latch if count > max */
- if (count > LATCH-1) {
- outb_p(0x34, 0x43);
- outb_p(LATCH & 0xff, 0x40);
- outb(LATCH >> 8, 0x40);
- count = LATCH - 1;
- }
+ if (count > LATCH)
+ via_reset_and_whine(&count);

/*
* avoiding timer inconsistencies (they are rare, but they happen)...
@@ -478,19 +524,8 @@
count |= inb(0x40) << 8;

/* VIA686a test code... reset the latch if count > max */
- if (count > LATCH-1) {
- static int last_whine;
- outb_p(0x34, 0x43);
- outb_p(LATCH & 0xff, 0x40);
- outb(LATCH >> 8, 0x40);
- count = LATCH - 1;
- if(time_after(jiffies, last_whine))
- {
- printk(KERN_WARNING "probable hardware bug: clock timer configuration lost - probably a VIA686a.\n");
- printk(KERN_WARNING "probable hardware bug: restoring chip configuration.\n");
- last_whine = jiffies + HZ;
- }
- }
+ if (count > LATCH)
+ via_reset_and_whine(&count);

#if 0
spin_unlock(&i8253_lock);
@@ -737,3 +772,25 @@
setup_x86_irq(0, &irq0);
#endif
}
+
+static int __init timer_setup(char *str)
+{
+ int invert;
+
+ while ((str != NULL) && (*str != '\0')) {
+ invert = (strncmp(str, "no-", 3) == 0);
+ if (invert)
+ str += 3;
+ if (strncmp(str, "via686a", 7) == 0) {
+ via686a_hacks = !invert;
+ if (invert)
+ printk(KERN_INFO "timer: VIA686a workaround disabled.\n");
+ }
+ str = strchr(str, ',');
+ if (str != NULL)
+ str += strspn(str, ", \t");
+ }
+ return 1;
+}
+
+__setup("timer=", timer_setup);

2002-04-08 06:13:19

by Robert Wang

[permalink] [raw]
Subject: develope driver for free

Hey,all,
I try to help develop driver for free. I need
hardware and document.
Any info are welcome.
regards,


__________________________________________________
Do You Yahoo!?
Yahoo! Tax Center - online filing with TurboTax
http://taxes.yahoo.com/

2002-04-08 07:24:35

by Vojtech Pavlik

[permalink] [raw]
Subject: Re: [PATCH] VIA timer fix was removed?

On Mon, Apr 08, 2002 at 04:33:49PM +1000, Neale Banks wrote:

> Hi Vojtech,
>
> Appended patch:
>
> (a) merges your patch of November 2001 into 2.2.21rc3
> (b) adds a boot-time option to explicitly disable the via-timer hacks.
>
> I've been using this patch for a while, but haven't subjected it to more
> thorough testing than that it boots OK and doesn't appear to complicate
> anything (I might be able to trigger the relevant condition by running the
> battery right down on my (old) AcerNote-950, but one of the last times
> this happened I also got some pretty nasty file system corruption - so I'm
> not too keen to try that one again :-| ).
>
> Anyway, does this patch and my merge of it look correct?

Looks OK.

>
> Thanks,
> Neale.
>
> --- linux-2.2.21-rc3-orig/arch/i386/kernel/time.c Mon Mar 26 02:37:30 2001
> +++ linux-2.2.21-rc3-ntb/arch/i386/kernel/time.c Fri Apr 5 23:04:13 2002
> @@ -81,6 +81,8 @@
>
> spinlock_t rtc_lock = SPIN_LOCK_UNLOCKED;
>
> +static int via686a_hacks = 1; /* default to enabled */
> +
> static inline unsigned long do_fast_gettimeoffset(void)
> {
> register unsigned long eax asm("ax");
> @@ -111,6 +113,54 @@
> return delay_at_last_interrupt + edx;
> }
>
> +/*
> + * VIA hardware bug workaround with check if it is really needed and
> + * a printk that could tell us what's exactly happening on machines which
> + * trigger the check, but are not VIA-based.
> + *
> + * Must be called with the i8253_spinlock held.
> + */
> +
> +static void via_reset_and_whine(int *count)
> +{
> + static unsigned long last_whine = 0;
> + unsigned long new_whine;
> + int count2;
> +
> + new_whine = last_whine;
> +
> + outb_p(0x00, 0x43); /* Re-read the timer */
> + count2 = inb_p(0x40);
> + count2 |= inb(0x40) << 8;
> +
> + if (time_after(jiffies, last_whine)) {
> + printk(KERN_WARNING "timer.c: VIA bug check triggered. "
> + "Value read %d [%#x], re-read %d [%#x]\n",
> + *count, *count, count2, count2);
> + new_whine = jiffies + HZ;
> + }
> +
> + *count = count2;
> +
> + if (count2 > LATCH) { /* Still bad */
> + if (time_after(jiffies, last_whine)) {
> + printk(KERN_WARNING "timer.c VIA bug really present. ");
> + new_whine = jiffies + HZ;
> + }
> + if (via686a_hacks) {
> + printk(KERN_WARNING "Resetting PIT timer.\n");
> + outb_p(0x34, 0x43);
> + outb_p(LATCH & 0xff, 0x40);
> + outb(LATCH >> 8, 0x40);
> + } else {
> + printk(KERN_WARNING "But VIA hacks disabled.\n");
> + }
> + *count = LATCH - 1;
> + }
> +
> + last_whine = new_whine;
> +}
> +
> #define TICK_SIZE tick
>
> #ifndef CONFIG_X86_TSC
> @@ -177,12 +227,8 @@
> count |= inb_p(0x40) << 8;
>
> /* VIA686a test code... reset the latch if count > max */
> - if (count > LATCH-1) {
> - outb_p(0x34, 0x43);
> - outb_p(LATCH & 0xff, 0x40);
> - outb(LATCH >> 8, 0x40);
> - count = LATCH - 1;
> - }
> + if (count > LATCH)
> + via_reset_and_whine(&count);
>
> /*
> * avoiding timer inconsistencies (they are rare, but they happen)...
> @@ -478,19 +524,8 @@
> count |= inb(0x40) << 8;
>
> /* VIA686a test code... reset the latch if count > max */
> - if (count > LATCH-1) {
> - static int last_whine;
> - outb_p(0x34, 0x43);
> - outb_p(LATCH & 0xff, 0x40);
> - outb(LATCH >> 8, 0x40);
> - count = LATCH - 1;
> - if(time_after(jiffies, last_whine))
> - {
> - printk(KERN_WARNING "probable hardware bug: clock timer configuration lost - probably a VIA686a.\n");
> - printk(KERN_WARNING "probable hardware bug: restoring chip configuration.\n");
> - last_whine = jiffies + HZ;
> - }
> - }
> + if (count > LATCH)
> + via_reset_and_whine(&count);
>
> #if 0
> spin_unlock(&i8253_lock);
> @@ -737,3 +772,25 @@
> setup_x86_irq(0, &irq0);
> #endif
> }
> +
> +static int __init timer_setup(char *str)
> +{
> + int invert;
> +
> + while ((str != NULL) && (*str != '\0')) {
> + invert = (strncmp(str, "no-", 3) == 0);
> + if (invert)
> + str += 3;
> + if (strncmp(str, "via686a", 7) == 0) {
> + via686a_hacks = !invert;
> + if (invert)
> + printk(KERN_INFO "timer: VIA686a workaround disabled.\n");
> + }
> + str = strchr(str, ',');
> + if (str != NULL)
> + str += strspn(str, ", \t");
> + }
> + return 1;
> +}
> +
> +__setup("timer=", timer_setup);

--
Vojtech Pavlik
SuSE Labs