[coreboot] vx800 Example Board Patch
Stefan Reinauer
stepan at coresystems.de
Sat Jul 18 19:34:16 CEST 2009
Dear Bari,
thanks a lot for this patch... Here's a quick first shot at a review..
On 18.07.2009 17:59, bari wrote:
> 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 @@
>
What's the reason for not using the mechanism that's already there?
> 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)
>
> +/* Disable Erratum 343 Workaround, see RevGuide for Fam10h, Pub#41322 Rev 3.33 */
> +
>
This looks wrong.
> 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)
>
This one mostly breaks whitespace.
> 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
>
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.
> 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,
> };
>
I don't think this is an improvement.
> 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
> }
>
Adds plenty of whitespace breakage.
Stopping here with the review, as I believe this needs quite some work
before it can be merged...
--
coresystems GmbH • Brahmsstr. 16 • D-79104 Freiburg i. Br.
Tel.: +49 761 7668825 • Fax: +49 761 7664613
Email: info at coresystems.de • http://www.coresystems.de/
Registergericht: Amtsgericht Freiburg • HRB 7656
Geschäftsführer: Stefan Reinauer • Ust-IdNr.: DE245674866
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.coreboot.org/pipermail/coreboot/attachments/20090718/d885715d/attachment.html>
More information about the coreboot
mailing list