<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN">
<html>
<head>
<meta content="text/html; charset=UTF-8" http-equiv="Content-Type">
</head>
<body bgcolor="#ffffff" text="#000000">
Dear Bari,<br>
<br>
thanks a lot for this patch... Here's a quick first shot at a review..<br>
<br>
<br>
On 18.07.2009 17:59, bari wrote:<br>
<br>
<blockquote cite="mid:4A61F174.4070200@onelabs.com" type="cite">
<div class="moz-text-plain" wrap="true" graphical-quote="true"
style="font-family: -moz-fixed; font-size: 13px;" lang="x-unicode">
<pre wrap="">Index: src/cpu/via/car/disable_cache_as_ram.c
===================================================================
--- src/cpu/via/car/disable_cache_as_ram.c (Revision 0)
+++ src/cpu/via/car/disable_cache_as_ram.c (Revision 0)
@@ -0,0 +1,101 @@
</pre>
</div>
</blockquote>
What's the reason for not using the mechanism that's already there?<br>
<br>
<br>
<blockquote cite="mid:4A61F174.4070200@onelabs.com" type="cite">
<div class="moz-text-plain" wrap="true" graphical-quote="true"
style="font-family: -moz-fixed; font-size: 13px;" lang="x-unicode">
<pre wrap="">Index: src/cpu/via/car/post_cache_as_ram.c
===================================================================
--- src/cpu/via/car/post_cache_as_ram.c (Revision 0)
+++ src/cpu/via/car/post_cache_as_ram.c (Revision 0)
</pre>
</div>
</blockquote>
<br>
<blockquote cite="mid:4A61F174.4070200@onelabs.com" type="cite">
<div class="moz-text-plain" wrap="true" graphical-quote="true"
style="font-family: -moz-fixed; font-size: 13px;" lang="x-unicode">
<pre wrap="">+/* Disable Erratum 343 Workaround, see RevGuide for Fam10h, Pub#41322 Rev 3.33 */
+
</pre>
</div>
</blockquote>
<br>
This looks wrong.<br>
<br>
<blockquote cite="mid:4A61F174.4070200@onelabs.com" type="cite">
<div class="moz-text-plain" wrap="true" graphical-quote="true"
style="font-family: -moz-fixed; font-size: 13px;" lang="x-unicode">
<pre wrap="">Index: src/cpu/via/car/cache_as_ram.inc
===================================================================
--- src/cpu/via/car/cache_as_ram.inc (Revision 4427)
+++ src/cpu/via/car/cache_as_ram.inc (Arbeitskopie)
</pre>
</div>
</blockquote>
<br>
This one mostly breaks whitespace.<br>
<br>
<blockquote cite="mid:4A61F174.4070200@onelabs.com" type="cite">
<div class="moz-text-plain" wrap="true" graphical-quote="true"
style="font-family: -moz-fixed; font-size: 13px;" lang="x-unicode">
<pre wrap=""> fixed_mtrr_msr:
Index: src/cpu/via/model_c7/model_c7_init.c
===================================================================
--- src/cpu/via/model_c7/model_c7_init.c (Revision 4427)
+++ src/cpu/via/model_c7/model_c7_init.c (Arbeitskopie)
@@ -60,6 +60,7 @@
0x0406, 0x0806, // 400MHz, 796mV --> 800MHz, 796mV C7-M ULV
0x0406, 0x0a06, // 400MHz, 796mV --> 1000MHz, 796mV
0x0406, 0x0c09, // 400MHz, 796mV --> 1200MHz, 844mV
+ 0x0406, 0x0609, //
0x0806, 0x0c09, // 800MHz, 796mV --> 1200MHz, 844mV
0x0406, 0x0f10, // 400MHz, 796mV --> 1500MHz, 956mV
0x0806, 0x1010, // 800MHz, 796mV --> 1600MHz, 956mV
@@ -100,6 +101,13 @@
}
}
+#if 1
+ /* Some value may not be included in c7d_speed_translation,
+ so a more general way is needed:*/
+ current=msr.hi&0xffff;
+ msr.lo=msr.lo&0xffff0000;
+ msr.lo=msr.lo|current;
+#else
</pre>
</div>
</blockquote>
Please do not use this. It does fail on C7 CPUs we've encountered.
Either only execute it if the values are not in the list, or drop it
completely.<br>
<br>
<br>
<blockquote cite="mid:4A61F174.4070200@onelabs.com" type="cite">
<div class="moz-text-plain" wrap="true" graphical-quote="true"
style="font-family: -moz-fixed; font-size: 13px;" lang="x-unicode">
<pre wrap="">Index: src/northbridge/via/vx800/vx800_ide.c
===================================================================
--- src/northbridge/via/vx800/vx800_ide.c (Revision 4427)
+++ src/northbridge/via/vx800/vx800_ide.c (Arbeitskopie)
@@ -25,244 +25,78 @@
#include "chip.h"
#include <arch/io.h>
#include "vx800.h"
+#include "vx800_pci_io_modify_ops.h"
-static const idedevicepcitable[16 * 12] = {
- /*
- 0x02, 0x00, 0x00, 0x00, 0x00, 0x82, 0x00, 0x00,
- 0x00, 0x00, 0xA8, 0xA8, 0xF0, 0x00, 0x00, 0xB6,
- 0x00, 0x00, 0x01, 0x21, 0x00, 0x00, 0x00, 0x00,
- 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
- 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
- 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
- 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
- 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
- 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
- 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
- 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
- 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
- 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
- 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
- 0x01, 0x00, 0x02, 0x00, 0x00, 0x00, 0x00, 0x00,
- 0x00, 0x01, 0x09, 0xC4, 0x06, 0x11, 0x09, 0xC4,
- 0x00, 0xC2, 0xF9, 0x01, 0x10, 0x00, 0x00, 0x00,
- 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
- 0x00, 0x00, 0x00, 0x00, 0x0C, 0x00, 0x00, 0x00,
- 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
- 0x00, 0x02, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
- 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
- 0x02, 0x01, 0x24, 0x00, 0x00, 0x00, 0x00, 0x00,
- 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
- */
-
- 0x02, 0x00, 0x00, 0x00, 0x00, 0x82, 0x00, 0x00,
- 0x00, 0x00, 0x99, 0x20, 0xf0, 0x00, 0x00, 0x20,
- 0x00, 0x00, 0x17, 0xF1, 0x00, 0x00, 0x00, 0x00,
- 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
- 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
- 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
- 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
- 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
- 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
- 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
- 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
- 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
- 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
- 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
- 0x01, 0x00, 0x02, 0x00, 0x00, 0x00, 0x00, 0x00,
- 0x00, 0x01, 0x09, 0xC4, 0x06, 0x11, 0x09, 0xC4,
- 0x00, 0xc2, 0x09, 0x01, 0x10, 0x00, 0x00, 0x00,
- 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
- 0x00, 0x00, 0x00, 0x00, 0x0c, 0x00, 0x00, 0x00,
- 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
- 0x00, 0x02, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
- 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
- 0x02, 0x01, 0x24, 0x00, 0x00, 0x00, 0x00, 0x00,
- 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
-
- /* Legacy BIOS XP PCI value */
- /*
- 0x02, 0x00, 0x00, 0x00, 0x00, 0x82, 0x00, 0x00,
- 0x00, 0x00, 0xa8, 0x20, 0x00, 0x00, 0x00, 0xb6,
- 0x00, 0x00, 0x16, 0xF1, 0x00, 0x00, 0x00, 0x00,
- 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
- 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
- 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
- 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
- 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
- 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
- 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
- 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
- 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
- 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
- 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
- 0x01, 0x00, 0x02, 0x00, 0x00, 0x00, 0x00, 0x00,
- 0x00, 0x01, 0x09, 0xC4, 0x06, 0x11, 0x09, 0xC4,
- 0x00, 0x02, 0x09, 0x00, 0x18, 0x00, 0x00, 0x00,
- 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
- 0x00, 0x00, 0x00, 0x00, 0x34, 0x00, 0x00, 0x00,
- 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
- 0x00, 0x02, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
- 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
- 0x02, 0x01, 0x24, 0x00, 0x00, 0x00, 0x00, 0x00,
- 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
- */
-
- /* ROM legacy BIOS on cn_8562b */
- /*
- 0x03, 0x00, 0x00, 0x00, 0x00, 0x82, 0x00, 0x00,
- 0x00, 0x00, 0x99, 0x20, 0x60, 0x00, 0x00, 0x20,
- 0x00, 0x00, 0x1E, 0xF1, 0x00, 0x00, 0x00, 0x00,
- 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
- 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
- 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
- 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
- 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
- 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
- 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
- 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
- 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
- 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
- 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
- 0x01, 0x00, 0x02, 0x00, 0x00, 0x00, 0x00, 0x00,
- 0x00, 0x01, 0x09, 0xC4, 0x06, 0x11, 0x09, 0xC4,
- 0x00, 0x02, 0x09, 0x01, 0x18, 0x0C, 0x00, 0x00,
- 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
- 0x00, 0x00, 0x00, 0x00, 0x34, 0x00, 0x00, 0x00,
- 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
- 0x00, 0x02, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
- 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
- 0x02, 0x01, 0x24, 0x00, 0x00, 0x00, 0x00, 0x00,
- 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
- */
-
- /* From legacy BIOS on c7_8562b */
- /*
- 0x03, 0x00, 0x00, 0x00, 0x00, 0x82, 0x00, 0x00,
- 0x00, 0x00, 0x5E, 0x20, 0x60, 0x00, 0x00, 0xB6,
- 0x00, 0x00, 0x1E, 0xF1, 0x00, 0x00, 0x00, 0x00,
- 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
- 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
- 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
- 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
- 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
- 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
- 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
- 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
- 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
- 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
- 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
- 0x01, 0x00, 0x02, 0x00, 0x00, 0x00, 0x00, 0x00,
- 0x00, 0x01, 0x09, 0xC4, 0x06, 0x11, 0x09, 0xC4,
- 0x00, 0x02, 0x09, 0x01, 0x18, 0x0C, 0x00, 0x00,
- 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
- 0x00, 0x00, 0x00, 0x00, 0x34, 0x00, 0x00, 0x00,
- 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
- 0x00, 0x02, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
- 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
- 0x02, 0x01, 0x24, 0x00, 0x00, 0x00, 0x00, 0x00,
- 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
- */
+static const idedevicepcitable [16*12]={
+0x02, 0x00, 0x00, 0x00, 0x00, 0x82, 0x00, 0x00, 0x00, 0x00, 0x99, 0x20, 0xf0, 0x00, 0x00, 0x20,
+0x00, 0x00, 0x17, 0xF1, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+0x01, 0x00, 0x02, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x09, 0xC4, 0x06, 0x11, 0x09, 0xC4,
+0x00, 0xc2, 0x09, 0x01, 0x10, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+0x00, 0x00, 0x00, 0x00, 0x0c, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+0x00, 0x02, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+0x02, 0x01, 0x24, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
};
</pre>
</div>
</blockquote>
<br>
I don't think this is an improvement.<br>
<br>
<blockquote cite="mid:4A61F174.4070200@onelabs.com" type="cite">
<div class="moz-text-plain" wrap="true" graphical-quote="true"
style="font-family: -moz-fixed; font-size: 13px;" lang="x-unicode">
<pre wrap="">Index: src/northbridge/via/vx800/northbridge.c
===================================================================
--- src/northbridge/via/vx800/northbridge.c (Revision 4427)
+++ src/northbridge/via/vx800/northbridge.c (Arbeitskopie)
@@ -41,26 +41,26 @@
u8 acpi_sleep_type = 0;
static void memctrl_init(device_t dev)
-{
+{
/*
- set VGA in uma_ram_setting.c, not in this function.
+ set VGA in uma_ram_setting.c, not in this function.
*/
#if 0
pci_write_config8(dev, 0x85, 0x20);
pci_write_config8(dev, 0x86, 0x2d);
-
+
/* Set up VGA timers */
pci_write_config8(dev, 0xa2, 0x44);
-
+
/* Enable VGA with a 32mb framebuffer */
pci_write_config16(dev, 0xa0, 0xd000);
-
+
pci_write_config16(dev, 0xa4, 0x0010);
-
+
//b0: 60 aa aa 5a 0f 00 00 00 08
pci_write_config16(dev, 0xb0, 0xaa00);
pci_write_config8(dev, 0xb8, 0x08);
-#endif
+#endif
}
</pre>
</div>
</blockquote>
<br>
Adds plenty of whitespace breakage.<br>
<br>
<br>
Stopping here with the review, as I believe this needs quite some work
before it can be merged...<br>
<br>
<pre class="moz-signature" cols="72">--
coresystems GmbH • Brahmsstr. 16 • D-79104 Freiburg i. Br.
Tel.: +49 761 7668825 • Fax: +49 761 7664613
Email: <a class="moz-txt-link-abbreviated" href="mailto:info@coresystems.de">info@coresystems.de</a> • <a class="moz-txt-link-freetext" href="http://www.coresystems.de/">http://www.coresystems.de/</a>
Registergericht: Amtsgericht Freiburg • HRB 7656
Geschäftsführer: Stefan Reinauer • Ust-IdNr.: DE245674866
</pre>
</body>
</html>