Skip to contentSkip to conversation

VIA timer fix was removed?

PATCH In review Jeronimo Pellegrini 6 participants 11 messages Nov 12, 2001

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 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.

arch/i386/kernel/time.c +8 -0
@@ -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;
#1 lore raw
Quoted 7 lines

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...

Quoted 15 lines
arch/i386/kernel/time.c +8 -0
@@ -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);
Signature · 4 lines
=======================================================================
  Andrzej M. Krzysztofowicz               [email protected]
  phone (48)(58) 347 14 61
Faculty of Applied Phys. & Math.,   Technical University of Gdansk
#2 lore raw
In reply to Jeronimo Pellegrini
Quoted 11 lines · Andrzej Krzysztofowicz
Quoted 7 lines

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.

arch/i386/kernel/time.c +13 -0
@@ -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;
#3 lore raw
Quoted 6 lines · Jeronimo Pellegrini
Quoted 5 lines · Andrzej Krzysztofowicz
Quoted 5 lines

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

Quoted 8 lines
Quoted 6 lines
Quoted 2 lines

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

Signature · 5 lines
 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?
#4 lore raw
In reply to Jeronimo Pellegrini
Quoted 4 lines

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.

#5 lore raw
Quoted 6 lines · Jeronimo Pellegrini
Quoted 4 lines

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

Signature · 5 lines
 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?
#6 lore raw
In reply to Jeronimo Pellegrini
Quoted 6 lines · Jeronimo Pellegrini
Quoted 4 lines

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.

arch/i386/kernel/time.c +38 -9
@@ -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");
@@ -177,7 +179,7 @@
 	count |= inb_p(0x40) << 8;
 
 	/* VIA686a test code... reset the latch if count > max */
- 	if (count > LATCH-1) {
+ 	if ((via686a_hacks) && (count > LATCH-1)) {
 		outb_p(0x34, 0x43);
 		outb_p(LATCH & 0xff, 0x40);
 		outb(LATCH >> 8, 0x40);
@@ -480,16 +482,21 @@
 		/* 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))
-			{
+			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");
+				if (via686a_hacks) {
+					printk(KERN_WARNING "probable hardware bug: restoring chip configuration.\n");
+				} else {
+					printk(KERN_WARNING "probable hardware bug: but VIA686a workaround disabled at boot.\n");
+				}
 				last_whine = jiffies + HZ;
-			}			
+			}
+			if (via686a_hacks) {
+				outb_p(0x34, 0x43);
+				outb_p(LATCH & 0xff, 0x40);
+				outb(LATCH >> 8, 0x40);
+				count = LATCH - 1;
+			}
 		}	
 
 #if 0
@@ -737,3 +744,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);
#7 lore raw
Quoted 3 lines

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

#8 lore raw
Quoted 4 lines
Quoted 4 lines

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/

Quoted 2 lines · Alan Cox

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.

#9 lore raw
Quoted 5 lines

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

#10 lore raw
In reply to Neale Banks
Quoted 6 lines · Alan Cox
Quoted 3 lines

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.

Signature · 2 lines
Vojtech Pavlik
SuSE Labs
arch/i386/kernel/time.c +50 -6
diff -urN linux-2.4.15-pre4/arch/i386/kernel/time.c linux/arch/i386/kernel/time.c
@@ -112,6 +112,50 @@
 	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. "
+				"Resetting PIT timer.\n");
+			new_whine = jiffies + HZ;
+		}
+		outb_p(0x34, 0x43);
+		outb_p(LATCH & 0xff, 0x40);
+		outb(LATCH >> 8, 0x40);
+		*count = LATCH - 1;
+	}
+
+	last_whine = new_whine;
+}
+
 #define TICK_SIZE tick
 
 spinlock_t i8253_lock = SPIN_LOCK_UNLOCKED;
@@ -180,12 +224,8 @@
 	count |= inb_p(0x40) << 8;
 	
         /* VIA686a test code... reset the latch if count > max + 1 */
-        if (count > LATCH) {
-                outb_p(0x34, 0x43);
-                outb_p(LATCH & 0xff, 0x40);
-                outb(LATCH >> 8, 0x40);
-                count = LATCH - 1;
-        }
+        if (count > LATCH) 
+		via_reset_and_whine(&count);
 	
 	spin_unlock(&i8253_lock);
 
@@ -501,6 +541,10 @@
 
 		count = inb_p(0x40);    /* read the latched count */
 		count |= inb(0x40) << 8;
+
+		if (count > LATCH)
+			via_reset_and_whine(&count);
+
 		spin_unlock(&i8253_lock);
 
 		count = ((LATCH-1) - count) * TICK_SIZE;