[LinuxBIOS] [PATCH] Via CN700

[Corey Osgood]

See attached patch. Unfortunately, I've killed my board, so I can't make
any real code changes because I simply can't test them. No ETA on when
I'll have another board. Jetway J7F2 patch will follow later tonight.

-Corey

[Uwe Hermann]

On Thu, Nov 15, 2007 at 12:38:01PM -0500, Corey Osgood wrote:
> Add support for the Via CN700 northbridge, with C7 CPU and DDR2 RAM.

Nice! Quick review below:
> Signed-off-by: Corey Osgood
> Index: src/include/device/pci_ids.h
>
> --- src/include/device/pci_ids.h	(revision 2974)
> +++ src/include/device/pci_ids.h	(working copy)
> @@ -1153,6 +1153,14 @@
>  #define PCI_DEVICE_ID_VIA_K8T890CE_BR	0xb188
>  #define PCI_DEVICE_ID_VIA_VT6420_SATA	0x3149
>  #define PCI_DEVICE_ID_VIA_VT8237R_LPC	0x3227
> +#define PCI_DEVICE_ID_VIA_CN700_AGP	0x0314
> +#define PCI_DEVICE_ID_VIA_CN700_ERR	0x1314
> +#define PCI_DEVICE_ID_VIA_CN700_HOST	0x2314
> +#define PCI_DEVICE_ID_VIA_CN700_MEMCTRL	0x3208
> +#define PCI_DEVICE_ID_VIA_CN700_PM	0x4314
> +#define PCI_DEVICE_ID_VIA_CN700_VLINK	0x7314
> +#define PCI_DEVICE_ID_VIA_CN700_BRIDGE	0xB198
> +#define PCI_DEVICE_ID_VIA_CN700_VGA	0x3344
>
> #define PCI_VENDOR_ID_SIEMENS           0x110A
>  #define PCI_DEVICE_ID_SIEMENS_DSCC4     0x2102
> Index: src/northbridge/via/cn700/Config.lb
> ===================================================================
> --- src/northbridge/via/cn700/Config.lb	(revision 0)
> +++ src/northbridge/via/cn700/Config.lb	(revision 0)
> @@ -0,0 +1,25 @@
> +##
> +## This file is part of the LinuxBIOS project.
> +##
> +## Copyright (C) 2007 Corey Osgood 
> +##
> +## This program is free software; you can redistribute it and/or modify
> +## it under the terms of the GNU General Public License as published by
> +## the Free Software Foundation; either version 2 of the License, or
> +## (at your option) any later version.
> +##
> +## This program is distributed in the hope that it will be useful,
> +## but WITHOUT ANY WARRANTY; without even the implied warranty of
> +## MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +## GNU General Public License for more details.
> +##
> +## You should have received a copy of the GNU General Public License
> +## along with this program; if not, write to the Free Software
> +## Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301 USA
> +##
>
> +config chip.h
> +object vgabios.o
> +object northbridge.o
> +driver agp.o
> +driver vga.o
> Index: src/northbridge/via/cn700/raminit.c
> ===================================================================
> --- src/northbridge/via/cn700/raminit.c	(revision 0)
> +++ src/northbridge/via/cn700/raminit.c	(revision 0)
> @@ -0,0 +1,331 @@
> +/*
> This file is part of the LinuxBIOS project.
> Copyright (C) 2007 Corey Osgood
> This program is free software; you can redistribute it and/or modify
> it under the terms of the GNU General Public License as published by
> the Free Software Foundation; either version 2 of the License, or
> (at your option) any later version.
> This program is distributed in the hope that it will be useful,
> but WITHOUT ANY WARRANTY; without even the implied warranty of
> MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> GNU General Public License for more details.
> You should have received a copy of the GNU General Public License
> along with this program; if not, write to the Free Software
> Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301 USA
> */
>
> +#include <spd.h>
> +#include <sdram_mode.h>
> +#include <delay.h>
>
> +//#define DEBUG_RAM_SETUP 1
>
> +#ifdef DEBUG_RAM_SETUP
> +#define PRINT_DEBUG_MEM(x)		print_debug(x)
> +#define PRINT_DEBUG_MEM_HEX8(x)		print_debug_hex8(x)
> +#define PRINT_DEBUG_MEM_HEX16(x)	print_debug_hex16(x)
> +#define PRINT_DEBUG_MEM_HEX32(x)	print_debug_hex32(x)
> +#define DUMPNORTH()			dump_pci_device(PCI_DEV(0, 0, 0))
> +#else
> +#define PRINT_DEBUG_MEM(x)
> +#define PRINT_DEBUG_MEM_HEX8(x)
> +#define PRINT_DEBUG_MEM_HEX16(x)
> +#define PRINT_DEBUG_MEM_HEX32(x)
> +#define DUMPNORTH()
> +#endif
>
> +static void do_ram_command(device_t dev, uint8_t command, uint32_t addr_offset)
> +{
> u8 reg;
> /* TODO: Support for multiple DIMMs. */
> reg = pci_read_config8(dev, 0x6b);

0x6b -> #define
> reg &= 0xf8;		/* Clear bits 2-0. */
> reg |= command;
> pci_write_config8(dev, 0x6b, reg);
> PRINT_DEBUG_MEM("    Sending RAM command 0x");
> PRINT_DEBUG_MEM_HEX8(reg);
> PRINT_DEBUG_MEM(" to 0x");
> PRINT_DEBUG_MEM_HEX32(0 + addr_offset);
> PRINT_DEBUG_MEM("\r\n");
> read32(0 + addr_offset);
>
> +}
>
> +/**
> Configure the bus between the cpu and the northbridge. This might be able to
> be moved to post-ram code in the future. This is specific to the C7 CPU, P4
> will need another function.
> TODO: Changing the dram frequency doesn't work
> @param ctrl The onboard northbridge devices and smbus addresses

The @param is wrong, there's no "ctrl" here, only "dev".
> */
>
> +static void c7_cpu_setup(device_t dev)
> +{
> /* Host bus interface registers (D0F2 0x50-0x67) */
> pci_write_config8(dev, 0x50, 0x88);
> pci_write_config8(dev, 0x51, 0x7a);
> pci_write_config8(dev, 0x52, 0x6f);
> pci_write_config8(dev, 0x53, 0x88);
> pci_write_config8(dev, 0x54, 0x10);//was 0x1e, modifried
> pci_write_config8(dev, 0x55, 0x16);
> pci_write_config8(dev, 0x56, 0x01);
> //pci_write_config8(dev, 0x57, 0x61);//set@200MHz dram clock
> pci_write_config8(dev, 0x59, 0x60);
> pci_write_config8(dev, 0x5d, 0xb2);
> pci_write_config8(dev, 0x5e, 0x88);
> pci_write_config8(dev, 0x5f, 0xc7);
> pci_write_config8(dev, 0x60, 0xff);
> pci_write_config8(dev, 0x61, 0xff);
> pci_write_config8(dev, 0x62, 0x0f);
> pci_write_config8(dev, 0x63, 0xff);
> pci_write_config8(dev, 0x64, 0xff);
> pci_write_config8(dev, 0x65, 0x0f);
> pci_write_config8(dev, 0x66, 0xff);
> pci_write_config8(dev, 0x67, 0x70);
> /* Host Bus IO Circuit (D0F2 0x70-0x7d) */
> pci_write_config8(dev, 0x70, 0x33);
> pci_write_config8(dev, 0x71, 0x00);
> pci_write_config8(dev, 0x72, 0x33);
> pci_write_config8(dev, 0x73, 0x00);
> pci_write_config8(dev, 0x74, 0x00);
> pci_write_config8(dev, 0x75, 0x28);
> pci_write_config8(dev, 0x76, 0x74);
> pci_write_config8(dev, 0x77, 0x00);
> pci_write_config8(dev, 0x78, 0x0a);
> pci_write_config8(dev, 0x79, 0xaa);
> pci_write_config8(dev, 0x7a, 0x24);
> pci_write_config8(dev, 0x7b, 0x00);
> pci_write_config8(dev, 0x7c, 0x00);
> pci_write_config8(dev, 0x7d, 0x6d);

Pretty scary for someone without the datasheet. Please add more comments
and/or use self-explanatory #defines or something alike.
> +}
>
> +/**
> Set up various ram and other control registers statically. Some of these may
> not be needed, other should be done with spd info, but that's a project for
> the future
> */
>
> +static void sdram_set_registers(const struct mem_controller *ctrl)
> +{
> /* TODO: Eliminate any duplicates */
> pci_write_config8(ctrl->d0f3, 0xd0, 0x88);
> pci_write_config8(ctrl->d0f3, 0xd1, 0x8b);
> pci_write_config8(ctrl->d0f3, 0xd2, 0x89);
> pci_write_config8(ctrl->d0f3, 0x86, 0x2d);
> /* Driving selection */
> /* DQ / DQS ODT Driving and Range Select */
> pci_write_config8(ctrl->d0f3, 0xd5, 0x8a);
> /* Memory Pads Driving and Range Select */
> pci_write_config8(ctrl->d0f3, 0xd6, 0xaa);
> /* DRAM Driving ??? Group DQS */
> pci_write_config8(ctrl->d0f3, 0xe0, 0xee);
> /* DRAM Driving ??? Group DQ (DQ, MPD, DQM) */
> pci_write_config8(ctrl->d0f3, 0xe2, 0xac);//ba
> /* DRAM Driving ??? Group CS */
> pci_write_config8(ctrl->d0f3, 0xe4, 0x66);
> /* DRAM Driving ??? Group MA */
> pci_write_config8(ctrl->d0f3, 0xe8, 0x86);
> /* DRAM Driving ??? Group MCLK */
> pci_write_config8(ctrl->d0f3, 0xe6, 0xaa);

This is better, at least there are a few comments so we (sort of) know
what might be going on.

It's still problematic if we were to make any changes to the code
without having access to the datasheets, but I don't know a good way
to make that easier (short of reproducing the datasheet as code comment).
> /* ODT (some are set with driving select above) */
> /* Memory Pad ODT Pullup / Pulldown Control */
> pci_write_config8(ctrl->d0f3, 0xd4, 0x0a);
> /* Memory Ranks ODT Lookup Table */
> pci_write_config8(ctrl->d0f3, 0xd8, 0x00);//was 1
> /* Compensation Control */
> pci_write_config8(ctrl->d0f3, 0xd3, 0x89);//enable auto compensation
> /* MCLKO Phase Control */
> pci_write_config8(ctrl->d0f3, 0x91, 0x02);
> /* CS/CKE Clock Phase Control */
> pci_write_config8(ctrl->d0f3, 0x92, 0x06);
> /* SCMD/MA Clock Phase Control */
> pci_write_config8(ctrl->d0f3, 0x93, 0x07);
> /* Channel A DQS Input Capture Range Control */
> pci_write_config8(ctrl->d0f3, 0x78, 0x83);
> /* DQS Input Capture Range Control */
> /* Set in accordance with the BIOS update note */
> pci_write_config8(ctrl->d0f3, 0x7a, 0x00);
> /* DQS Input Delay Offset Control */
> pci_write_config8(ctrl->d0f3, 0x7c, 0x00);
> /* SDRAM ODT Control */
> pci_write_config8(ctrl->d0f3, 0xda, 0x80);
> /* DQ/DQS CKG Output Delay Control - I */
> pci_write_config8(ctrl->d0f3, 0xdc, 0xff);
> /* DQ/DQS CKG Output Delay Control - II */
> pci_write_config8(ctrl->d0f3, 0xdd, 0xff);
> /* DQS / DQ CKG Duty Cycle Control */
> pci_write_config8(ctrl->d0f3, 0xec, 0x88);
> /* MCLK Output Duty Control */
> pci_write_config8(ctrl->d0f3, 0xee, 0x00);
> pci_write_config8(ctrl->d0f3, 0xed, 0x10);
> /* DQS CKG Input Delay Control */
> pci_write_config8(ctrl->d0f3, 0xef, 0x10);
> pci_write_config8(ctrl->d0f3, 0x77, 0x9d);
> pci_write_config8(ctrl->d0f3, 0x79, 0x83);
> pci_write_config16(ctrl->d0f3, 0x88, 0x0020);
> pci_write_config8(ctrl->d0f4, 0xa7, 0x80);
> /* VLink Control */
> pci_write_config8(ctrl->d0f7, 0xb0, 0x05);
> pci_write_config8(ctrl->d0f7, 0xb1, 0x01);
> /* Memory base */
> pci_write_config16(ctrl->d1f0, 0x20, 0xfb00);
> /* Memory limit */
> pci_write_config16(ctrl->d1f0, 0x22, 0xfcf0);
> /* Prefetch memory base */
> pci_write_config16(ctrl->d1f0, 0x24, 0xf400);
> /* Prefetch memory limit */
> pci_write_config16(ctrl->d1f0, 0x26, 0xf7f0);
> /* PCI to PCI bridge control */
> pci_write_config16(ctrl->d1f0, 0x3e, 0x0008);
> /* CPU to PCI flow control 1 */
> pci_write_config8(ctrl->d1f0, 0x40, 0x83);
> pci_write_config8(ctrl->d1f0, 0x41, 0xc3);//clear reset error, set to 43
> pci_write_config8(ctrl->d1f0, 0x42, 0xe2);
> pci_write_config8(ctrl->d1f0, 0x43, 0x44);
> pci_write_config8(ctrl->d1f0, 0x44, 0x34);
> pci_write_config8(ctrl->d1f0, 0x45, 0x72);
> /* Disable cross bank/multi page mode */
> pci_write_config8(ctrl->d0f3, 0x69, 0x80);
> pci_write_config8(ctrl->d0f3, 0x6a, 0x00);
> /* Set WR=5 and RFC */
> pci_write_config8(ctrl->d0f3, 0x61, 0xc7);
> /* Set CAS=5 */
> pci_write_config8(ctrl->d0f3, 0x62, 0xaf);
> pci_write_config8(ctrl->d0f3, 0x63, 0xca);
> /* Set to DDR2 sdram, BL=8 (0xc8, 0xc0 for bl=4) */
> pci_write_config8(ctrl->d0f3, 0x6c, 0xc8);
> /* Allow manual dll reset */
> pci_write_config8(ctrl->d0f3, 0x6b, 0x10);
> pci_write_config8(ctrl->d0f3, 0x6e, 0x89);
> pci_write_config8(ctrl->d0f3, 0x67, 0x50);
> pci_write_config8(ctrl->d0f3, 0x65, 0xd9);
> /* Only enable bank 1, for now */

Add a big TODO here, please.
> pci_write_config8(ctrl->d0f3, 0x54, 0x80);
> pci_write_config8(ctrl->d0f3, 0x55, 0x00);
> /* Set to 2T, MA Map type 1. Needs to become dynamic */

TODO
> pci_write_config16(ctrl->d0f3, 0x50, 0x0020);
> /* BA0-2 Selection. Don't mess with */
> pci_write_config8(ctrl->d0f3, 0x52, 0x33);
> pci_write_config8(ctrl->d0f3, 0x53, 0x3f);
> /* Disable bank interleaving. This feature seems useless anyways */

Why useless?
> pci_write_config32(ctrl->d0f3, 0x58, 0x00000000);
> pci_write_config8(ctrl->d0f3, 0x88, 0x08);
> /* Some DQS control stuffs */
> pci_write_config8(ctrl->d0f3, 0x74, 0x04);
> pci_write_config8(ctrl->d0f3, 0x75, 0x04);
> pci_write_config8(ctrl->d0f3, 0x76, 0x00);
>
> +}
>
> +/**
> Set up dram size according to spd data. Eventually, DRAM timings should be
> done in a similar manner.
> @param ctrl The northbridge devices and spd addresses.
> */
>
> +static void sdram_set_spd_registers(const struct mem_controller *ctrl)
> +{
> u8 spd_data, spd_data2;
> /* DRAM Bank Size */
> spd_data = spd_read_byte(ctrl->channel0[0], 31);

SPD_DENSITY_OF_EACH_ROW_ON_MODULE
> /* I know this seems weird. Blame JEDEC/Via. */
> if(spd_data >= 0x10)
> spd_data = spd_data >> 1;
> else
> spd_data = spd_data << 1;
> /* Check for double sided dimm and adjust size accordingly */
> spd_data2 = spd_read_byte(ctrl->channel0[0], 17);

SPD_NUM_BANKS_PER_SDRAM
> /* There should be 4 banks on a single sided dimm,
> * or 8 on a dual sided one */
> spd_data = spd_data * (spd_data2 / 4);
> pci_write_config8(ctrl->d0f3, 0x40, spd_data);

0x40 -> #define
> +}
>
> +static void sdram_enable(device_t dev)
> +{
> int i;
> /* 1. Apply NOP. */
> PRINT_DEBUG_MEM("RAM Enable 1: Apply NOP\r\n");
> do_ram_command(dev, RAM_COMMAND_NOP, 0);
> udelay(200);
> /* 2. Precharge all. */
> PRINT_DEBUG_MEM("RAM Enable 2: Precharge all\r\n");
> do_ram_command(dev, RAM_COMMAND_PRECHARGE, 0);
> /* 3. Mode register set. */
> PRINT_DEBUG_MEM("RAM Enable 4: Mode register set\r\n");
> do_ram_command(dev, RAM_COMMAND_MRS, 0x2000);//enable dll
> do_ram_command(dev, RAM_COMMAND_MRS, 0x800);//reset dll
> /* 4. Precharge all again. */
> PRINT_DEBUG_MEM("RAM Enable 2: Precharge all\r\n");
> do_ram_command(dev, RAM_COMMAND_PRECHARGE, 0);
> /* 5. Perform 8 refresh cycles. Wait tRC each time. */
> PRINT_DEBUG_MEM("RAM Enable 3: CBR\r\n");
> do_ram_command(dev, RAM_COMMAND_CBR, 0);
> /* First read is actually done by do_ram_command */
> for(i = 0; i < 7; i++) {
> udelay(100);
> read32(0);
> }
> /* 6. Mode register set. */
> PRINT_DEBUG_MEM("RAM Enable 4: Mode register set\r\n");
> //safe value for now, BL=8, WR=5, CAS=5
> do_ram_command(dev, RAM_COMMAND_MRS, 0x0022d8);

Please list/explain the bits and their meaning, 0x0022d8 is unreadable.
> /* 7. Mode register set. */
> PRINT_DEBUG_MEM("RAM Enable 4: Mode register set\r\n");
> do_ram_command(dev, RAM_COMMAND_MRS, 0x21c20);//default OCD calibration
> do_ram_command(dev, RAM_COMMAND_MRS, 0x20020);//exit calibration mode

Ditto.
> /* 8. Normal operation */
> PRINT_DEBUG_MEM("RAM Enable 5: Normal operation\r\n");
> do_ram_command(dev, RAM_COMMAND_NORMAL, 0);
> /* Enable multipage mode. */
> pci_write_config8(dev, 0x69, 0x83);
> /* Enable refresh. */
> pci_write_config8(dev, 0x6a, 0x32);

#defines
> /* DQS Tuning: testing on a couple different boards has shown this is
> * static, or close enough that it can be. Which is good, because the
> * tuning function used too many registers */
> pci_write_config8(dev, 0x70, 0x00);
> pci_write_config8(dev, 0x71, 0x03);

Ditto.
> /* Enable VGA device with no memory, add memory later. We need this
> * here to enable the actual device, otherwise it won't show up until
> * later and LB will have a fit. */
> pci_write_config16(dev, 0xa0, (1 << 15));
> pci_write_config16(dev, 0xa4, 0x0010);

Ditto.
> +}
> Index: src/northbridge/via/cn700/vgachip.h
> ===================================================================
> --- src/northbridge/via/cn700/vgachip.h	(revision 0)
> +++ src/northbridge/via/cn700/vgachip.h	(revision 0)
> @@ -0,0 +1,30 @@
> +/*
> This file is part of the LinuxBIOS project.
> Copyright (C) 2007 Corey Osgood
> This program is free software; you can redistribute it and/or modify
> it under the terms of the GNU General Public License as published by
> the Free Software Foundation; either version 2 of the License, or
> (at your option) any later version.
> This program is distributed in the hope that it will be useful,
> but WITHOUT ANY WARRANTY; without even the implied warranty of
> MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> GNU General Public License for more details.
> You should have received a copy of the GNU General Public License
> along with this program; if not, write to the Free Software
> Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301 USA
> */
>
> +#ifndef _PC80_VGABIOS
> +#define _PC80_VGABIOS
>
> +extern struct chip_control pc80_vgabios_control;
>
> +struct pc80_vgabios_config {
> int nothing;
>
> +};
>
> +#endif /* _PC80_VGABIOS */
> Index: src/northbridge/via/cn700/agp.c
> ===================================================================
> --- src/northbridge/via/cn700/agp.c	(revision 0)
> +++ src/northbridge/via/cn700/agp.c	(revision 0)
> @@ -0,0 +1,128 @@
> +/*
> This file is part of the LinuxBIOS project.
> Copyright (C) 2007 Corey Osgood
> This program is free software; you can redistribute it and/or modify
> it under the terms of the GNU General Public License as published by
> the Free Software Foundation; either version 2 of the License, or
> (at your option) any later version.
> This program is distributed in the hope that it will be useful,
> but WITHOUT ANY WARRANTY; without even the implied warranty of
> MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> GNU General Public License for more details.
> You should have received a copy of the GNU General Public License
> along with this program; if not, write to the Free Software
> Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301 USA
> */
>
> +#include <console/console.h>
> +#include <arch/io.h>
> +#include <stdint.h>
> +#include <device/device.h>
> +#include <device/pci.h>
> +#include <device/pci_ids.h>
> +#include "chip.h"
> +#include "northbridge.h"
> +#include "cn700.h"
>
> +/* This is the main AGP device, and only one used when configured for AGP 2.0 */
> +static void agp_init(device_t dev)
> +{
> printk_debug("Enabling AGP.\n");
> /* Do first */
> pci_write_config8(dev, 0x4d, 0x15);
> pci_write_config8(dev, 0x4, 0x6);
> pci_write_config8(dev, 0xd, 0x8);
> pci_write_config8(dev, 0x84, 0xb);
> pci_write_config8(dev, 0x4a, 0x1f);
> pci_write_config8(dev, 0x4b, 0xc4);
> pci_write_config8(dev, 0xb5, 0x03);

??? No idea what the above stuff does. Please add comments, use #defines.
Always remember that you're probably the only one out there with the
datasheets.
> /* Set up a 32mb AGP Aperture @ 0xf80000 */
> pci_write_config32(dev, 0x94, 0x00010f38);
> pci_write_config32(dev, 0x98, 0x01558000);
> pci_write_config32(dev, 0x10, 0xf8000008);
> /* Enable aperture */
> pci_write_config8(dev, 0xbf, 0x8c);
> pci_write_config32(dev, 0x90, 0x00000180);
> pci_write_config8(dev, 0xbc, 0x21);
> pci_write_config8(dev, 0xbd, 0xd2);
> //pci_write_config8(dev, 0x3e, 0x0c);
> pci_write_config8(dev, 0x40, 0xc7);
> pci_write_config8(dev, 0x41, 0xdb);
> pci_write_config8(dev, 0x42, 0x10);
> pci_write_config8(dev, 0x43, 0xdb);
> pci_write_config8(dev, 0x44, 0x24);
> //pci_write_config8(dev, 0x83, 0x02);
> pci_write_config8(dev, 0x4a, 0x1f);
> pci_write_config8(dev, 0x4b, 0xc4);
> pci_write_config8(dev, 0xc0, 0x02);
> pci_write_config8(dev, 0xc1, 0x02);
>
> +}
>
> +static struct device_operations agp_operations = {

Can be 'static const'.
> .read_resources   = cn700_noop,
> .set_resources    = pci_dev_set_resources,
> .enable_resources = pci_dev_enable_resources,
> .init             = agp_init,
> .ops_pci          = 0,
>
> +};
>
> +static struct pci_driver agp_driver __pci_driver = {

Can be 'static const'.
> .ops = &agp_operations,
> .vendor = PCI_VENDOR_ID_VIA,
> .device = PCI_DEVICE_ID_VIA_CN700_AGP,
>
> +};
>
> +/* This is the AGP 3.0 "bridge" @Bus 0 Device 1 Func 0. When using AGP 3.0, the
> +config in this device takes presidence. We configure both just to be safe. */
> +static void agp_bridge_init(device_t dev)
> +{
> printk_debug("Setting up AGP Bridge device\n");
> pci_write_config16(dev, 0x4, 0x0007);
> pci_write_config8(dev, 0x19, 0x01);
> pci_write_config8(dev, 0x1a, 0x01);
> pci_write_config8(dev, 0x1c, 0xd0);
> pci_write_config8(dev, 0x1d, 0xd0);
> pci_write_config16(dev, 0x1e, 0xe220);
> pci_write_config16(dev, 0x20, 0xfb00);
> pci_write_config16(dev, 0x22, 0xfcf0);
> pci_write_config16(dev, 0x24, 0xf400);
> pci_write_config16(dev, 0x26, 0xf7f0);
> pci_write_config8(dev, 0x3e, 0x08);
> pci_write_config8(dev, 0x40, 0x83);
> pci_write_config8(dev, 0x41, 0x43);
> pci_write_config8(dev, 0x42, 0xe2);
> pci_write_config8(dev, 0x43, 0x44);
> pci_write_config8(dev, 0x44, 0x34);
> pci_write_config8(dev, 0x45, 0x72);

???
> +}
>
> +static struct device_operations agp_bridge_operations = {
> .read_resources   = cn700_noop,
> .set_resources    = pci_dev_set_resources,
> .enable_resources = pci_bus_enable_resources,
> .init             = agp_bridge_init,
> .scan_bus         = pci_scan_bridge,
> .ops_pci          = 0,
>
> +};
>
> +static struct pci_driver agp_bridge_driver __pci_driver = {
> .ops = &agp_bridge_operations,
> .vendor = PCI_VENDOR_ID_VIA,
> .device = PCI_DEVICE_ID_VIA_CN700_BRIDGE,
>
> +};
> Index: src/northbridge/via/cn700/northbridge.c
> ===================================================================
> --- src/northbridge/via/cn700/northbridge.c	(revision 0)
> +++ src/northbridge/via/cn700/northbridge.c	(revision 0)
> @@ -0,0 +1,216 @@
> +/*
> This file is part of the LinuxBIOS project.
> Copyright (C) 2007 Corey Osgood
> This program is free software; you can redistribute it and/or modify
> it under the terms of the GNU General Public License as published by
> the Free Software Foundation; either version 2 of the License, or
> (at your option) any later version.
> This program is distributed in the hope that it will be useful,
> but WITHOUT ANY WARRANTY; without even the implied warranty of
> MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> GNU General Public License for more details.
> You should have received a copy of the GNU General Public License
> along with this program; if not, write to the Free Software
> Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301 USA
> */
>
> +#include <console/console.h>
> +#include <arch/io.h>
> +#include <stdint.h>
> +#include <device/device.h>
> +#include <device/pci.h>
> +#include <device/pci_ids.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <bitops.h>
> +#include <cpu/cpu.h>
> +#include "chip.h"
> +#include "northbridge.h"
> +#include "cn700.h"
>
> +static void memctrl_init(device_t dev)
> +{
> uint16_t reg16;
> pci_write_config8(dev, 0x86, 0x2d);
> /* Set up the vga framebuffer size */
> reg16 = (log2(CONFIG_VIDEO_MB) << 12) | (1 << 15);
> pci_write_config16(dev, 0xa0, reg16);
> /* Set up VGA timers */
> pci_write_config8(dev, 0xa2, 0x44);
> pci_write_config16(dev, 0xb0, 0xaa60);
> pci_write_config8(dev, 0xb8, 0x08);
>
> +}
>
> +static struct device_operations memctrl_operations = {
> .read_resources = cn700_noop,
> .init = memctrl_init,
>
> +};
>
> +static struct pci_driver memctrl_driver __pci_driver = {
> .ops = &memctrl_operations,
> .vendor = PCI_VENDOR_ID_VIA,
> .device = PCI_DEVICE_ID_VIA_CN700_MEMCTRL,
>
> +};
>
> +static void pci_domain_read_resources(device_t dev)
> +{
>    struct resource *resource;
> printk_spew("Entering cn700 pci_domain_read_resources.\n");
>    /* Initialize the system wide io space constraints */
>    resource = new_resource(dev, IOINDEX_SUBTRACTIVE(0,0));
>    resource->limit = 0xffffUL;
>    resource->flags = IORESOURCE_IO | IORESOURCE_SUBTRACTIVE |
> IORESOURCE_ASSIGNED;
>    /* Initialize the system wide memory resources constraints */
>    resource = new_resource(dev, IOINDEX_SUBTRACTIVE(1,0));
>    resource->limit = 0xffffffffULL;
>    resource->flags = IORESOURCE_MEM | IORESOURCE_SUBTRACTIVE |
> IORESOURCE_ASSIGNED;
> printk_spew("Leaving cn700 pci_domain_read_resources.\n");

OK for now, but this is the same on pretty much all northbridges, we
should move it to a common location.
> +}
>
> +static void ram_resource(device_t dev, unsigned long index,
>    unsigned long basek, unsigned long sizek)
>
> +{
>    struct resource *resource;
>    if (!sizek) {
>            return;
>    }
>    resource = new_resource(dev, index);
>    resource->base  = ((resource_t)basek) << 10;
>    resource->size  = ((resource_t)sizek) << 10;
>    resource->flags =  IORESOURCE_MEM | IORESOURCE_CACHEABLE | \
>            IORESOURCE_FIXED | IORESOURCE_STORED | IORESOURCE_ASSIGNED;
>
> +}

Ditto.
> +static void tolm_test(void *gp, struct device *dev, struct resource *new)
> +{
> struct resource **best_p = gp;
> struct resource *best;
> best = *best_p;
> if (!best || (best->base > new->base)) {
> best = new;
> }
> *best_p = best;
>
> +}

Ditto.
> +static uint32_t find_pci_tolm(struct bus *bus)
> +{
> print_debug("Entering find_pci_tolm\n");
> struct resource *min;
> uint32_t tolm;
> min = 0;
> search_bus_resources(bus, IORESOURCE_MEM, IORESOURCE_MEM, tolm_test, &min);
> tolm = 0xffffffffUL;
> if (min && tolm > min->base) {
> tolm = min->base;
> }
> print_debug("Leaving find_pci_tolm\n");
> return tolm;
>
> +}

Ditto.
> +static void pci_domain_set_resources(device_t dev)
> +{
> static const uint8_t ramregs[] = {0x40, 0x41, 0x42, 0x43};
> device_t mc_dev;
>    uint32_t pci_tolm;

u32.

Let's use the short form for all code in LB. We'll do this in v3 anyway,
and doing the same in v2 will ease porting the code to v3.
> printk_spew("Entering cn700 pci_domain_set_resources.\n");
>    pci_tolm = find_pci_tolm(&dev->link[0]);
> mc_dev = dev_find_device(PCI_VENDOR_ID_VIA,
> 		PCI_DEVICE_ID_VIA_CN700_MEMCTRL, 0);
> if (mc_dev) {
> unsigned long tomk, tolmk;
> unsigned char rambits;
> int i, idx;
> for(rambits = 0, i = 0; i < ARRAY_SIZE(ramregs); i++) {
> 	unsigned char reg;

u8
> 	reg = pci_read_config8(mc_dev, ramregs[i]);
> 	rambits += reg;
> }
> tomk = rambits;//GCC should optimize out the extra variable.

Why do we care?
> /* Compute the Top Of Low Memory, in Kb */
> tolmk = pci_tolm >> 10;
> if (tolmk >= tomk) {
> 	/* The PCI hole does does not overlap the memory. */
> 	tolmk = tomk;
> }
> /* Report the memory regions */
> idx = 10;
> /* TODO: Hole needed? */
> ram_resource(dev, idx++, 0, 640); /* first 640k */
> /* Leave a hole for vga */
> ram_resource(dev, idx++, 768, (tolmk - 768 -
> 				(CONFIG_VIDEO_MB * 1024)));
> }
> assign_resources(&dev->link[0]);
>
> +}
>
> +static unsigned int pci_domain_scan_bus(device_t dev, unsigned int max)
> +{
> printk_spew("Entering cn700 pci_domain_scan_bus.\n");
>    max = pci_scan_bus(&dev->link[0], PCI_DEVFN(0, 0), 0xff, max);
>    return max;
>
> +}
>
> +static struct device_operations pci_domain_ops = {
>    .read_resources   = pci_domain_read_resources,
>    .set_resources    = pci_domain_set_resources,
>    .enable_resources = enable_childrens_resources,
>    .init             = 0,
>    .scan_bus         = pci_domain_scan_bus,
>
> +};
>
> +static void cpu_bus_init(device_t dev)
> +{
>    initialize_cpus(&dev->link[0]);
>
> +}
>
> +static void cpu_bus_noop(device_t dev)
> +{
> +}
>
> +static struct device_operations cpu_bus_ops = {
>    .read_resources   = cpu_bus_noop,
>    .set_resources    = cpu_bus_noop,
>    .enable_resources = cpu_bus_noop,
>    .init             = cpu_bus_init,
>    .scan_bus         = 0,
>
> +};
>
> +static void enable_dev(struct device *dev)
> +{
> printk_spew("In cn700 enable_dev for device %s.\n", dev_path(dev));
>    /* Set the operations if it is a special bus type */
>    if (dev->path.type == DEVICE_PATH_PCI_DOMAIN) {
>            dev->ops = &pci_domain_ops;
> pci_set_method(dev);
>    }
>    else if (dev->path.type == DEVICE_PATH_APIC_CLUSTER) {
>            dev->ops = &cpu_bus_ops;
>    }
>
> +}
>
> +struct chip_operations northbridge_via_cn700_ops = {
> CHIP_NAME("VIA CN700 Northbridge")
> .enable_dev = enable_dev,
>
> +};
> Index: src/northbridge/via/cn700/raminit.h
> ===================================================================
> --- src/northbridge/via/cn700/raminit.h	(revision 0)
> +++ src/northbridge/via/cn700/raminit.h	(revision 0)
> @@ -0,0 +1,35 @@
> +/*
> This file is part of the LinuxBIOS project.
> Copyright (C) 2007 Corey Osgood
> This program is free software; you can redistribute it and/or modify
> it under the terms of the GNU General Public License as published by
> the Free Software Foundation; either version 2 of the License, or
> (at your option) any later version.
> This program is distributed in the hope that it will be useful,
> but WITHOUT ANY WARRANTY; without even the implied warranty of
> MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> GNU General Public License for more details.
> You should have received a copy of the GNU General Public License
> along with this program; if not, write to the Free Software
> Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301 USA
> */
>
> +#ifndef RAMINIT_H
> +#define RAMINIT_H
>
> +#define RAM_COMMAND_NORMAL	0x0
> +#define RAM_COMMAND_NOP		0x1
> +#define RAM_COMMAND_PRECHARGE	0x2
> +#define RAM_COMMAND_MRS		0x3
> +#define RAM_COMMAND_CBR		0x4
>
> +#define DIMM_SOCKETS 1 //Only one on my board.

Well, maybe, but the controller supports more than one?

This is src/northbridge/via/cn700/raminit.h, which should not be specific
to your board.
> +struct mem_controller {
> device_t d0f0, d0f2, d0f3, d0f4, d0f7, d1f0;
> uint8_t channel0[DIMM_SOCKETS];
>
> +};
> Index: src/northbridge/via/cn700/chip.h
> ===================================================================
> --- src/northbridge/via/cn700/chip.h	(revision 0)
> +++ src/northbridge/via/cn700/chip.h	(revision 0)
> @@ -0,0 +1,25 @@
> +/*
> This file is part of the LinuxBIOS project.
> Copyright (C) 2007 Corey Osgood
> This program is free software; you can redistribute it and/or modify
> it under the terms of the GNU General Public License as published by
> the Free Software Foundation; either version 2 of the License, or
> (at your option) any later version.
> This program is distributed in the hope that it will be useful,
> but WITHOUT ANY WARRANTY; without even the implied warranty of
> MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> GNU General Public License for more details.
> You should have received a copy of the GNU General Public License
> along with this program; if not, write to the Free Software
> Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301 USA
> */
>
> +struct northbridge_via_cn700_config
> +{
> +};
>
> +extern struct chip_operations northbridge_via_cn700_ops;
> Index: src/northbridge/via/cn700/vgabios.c
>
> --- src/northbridge/via/cn700/vgabios.c	(revision 0)
> +++ src/northbridge/via/cn700/vgabios.c	(revision 0)

There must be a better way than cloning the whole file. Did you change
any of the contents?
> Index: src/northbridge/via/cn700/northbridge.h
>
> --- src/northbridge/via/cn700/northbridge.h	(revision 0)
> +++ src/northbridge/via/cn700/northbridge.h	(revision 0)
> @@ -0,0 +1,26 @@
> +/*
> This file is part of the LinuxBIOS project.
> Copyright (C) 2007 Corey Osgood
> This program is free software; you can redistribute it and/or modify
> it under the terms of the GNU General Public License as published by
> the Free Software Foundation; either version 2 of the License, or
> (at your option) any later version.
> This program is distributed in the hope that it will be useful,
> but WITHOUT ANY WARRANTY; without even the implied warranty of
> MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> GNU General Public License for more details.
> You should have received a copy of the GNU General Public License
> along with this program; if not, write to the Free Software
> Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301 USA
> */
>
> +#ifndef NORTHBRIDGE_VIA_CN700_H
> +#define NORTHBRIDGE_VIA_CN700_H
>
> +extern unsigned int cn700_scan_root_bus(device_t root, unsigned int max);
>
> +#endif /* NORTHBRIDGE_VIA_CN700_H */
> Index: src/northbridge/via/cn700/cn700.h
> ===================================================================
> --- src/northbridge/via/cn700/cn700.h	(revision 0)
> +++ src/northbridge/via/cn700/cn700.h	(revision 0)
> @@ -0,0 +1,30 @@
> +/*
> This file is part of the LinuxBIOS project.
> Copyright (C) 2007 Corey Osgood
> This program is free software; you can redistribute it and/or modify
> it under the terms of the GNU General Public License as published by
> the Free Software Foundation; either version 2 of the License, or
> (at your option) any later version.
> This program is distributed in the hope that it will be useful,
> but WITHOUT ANY WARRANTY; without even the implied warranty of
> MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> GNU General Public License for more details.
> You should have received a copy of the GNU General Public License
> along with this program; if not, write to the Free Software
> Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301 USA
> */
>
> +/* TODO: Kill or fill this file */

Fill it with #defines.
> +static void cn700_noop(){}
>
> +/* VGA stuff */
> +#define SR_INDEX	0x3c4
> +#define SR_DATA		0x3c5
> +#define CRTM_INDEX	0x3b4
> +#define CRTM_DATA	0x3b5
> +#define CRTC_INDEX	0x3d4
> +#define CRTC_DATA	0x3d5
> Index: src/northbridge/via/cn700/vga.c
> ===================================================================
> --- src/northbridge/via/cn700/vga.c	(revision 0)
> +++ src/northbridge/via/cn700/vga.c	(revision 0)
> @@ -0,0 +1,125 @@
> +/*
> This file is part of the LinuxBIOS project.
> Copyright (C) 2007 Corey Osgood
> This program is free software; you can redistribute it and/or modify
> it under the terms of the GNU General Public License as published by
> the Free Software Foundation; either version 2 of the License, or
> (at your option) any later version.
> This program is distributed in the hope that it will be useful,
> but WITHOUT ANY WARRANTY; without even the implied warranty of
> MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> GNU General Public License for more details.
> You should have received a copy of the GNU General Public License
> along with this program; if not, write to the Free Software
> Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301 USA
> */
>
> +/* Note: Some of the VGA control registers are located on the memory controller.
> Registers are set both in raminit.c and northbridge.c */
>
> +#include <console/console.h>
> +#include <arch/io.h>
> +#include <stdint.h>
> +#include <device/device.h>
> +#include <device/pci.h>
> +#include <device/pci_ids.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <bitops.h>
> +#include <cpu/cpu.h>
> +#include "chip.h"
> +#include "northbridge.h"
> +#include "cn700.h"
>
> +void write_protect_vgabios(void)
> +{
> +//Don't bother for now
> +}
>
> +static void vga_init(device_t dev)
> +{
> uint8_t reg8;
> print_debug("Copying BOCHS Bios to 0xf000\n");
>
> +/* Copy the BOCHs BIOS from 0xFFFFFFFF - ROM_SIZE - BOCHs size (64k) to 0xf0000
> This is for compatibility with the VGA ROM's BIOS callbacks */
> memcpy(0xf0000, (0xFFFFFFFF - ROM_SIZE - 0x10000), 0x10000);
> print_debug("Initiailizing VGA\n");

Typo.
> pci_write_config8(dev, 0x3c, 0xb);
> /* Set memory rate to 200MHz */
> outb(0x3d, CRTM_INDEX);
> reg8 = inb(CRTM_DATA);
> reg8 &= 0x0f;
> reg8 |= (0x1 << 4);
> outb(0x3d, CRTM_INDEX);
> outb(reg8, CRTM_DATA);
> /* Set framebuffer size */
> reg8 = (CONFIG_VIDEO_MB / 4);
> outb(0x39, SR_INDEX);
> outb(reg8, SR_DATA);
> pci_write_config8(dev, 0x04, 0x07);
> pci_write_config8(dev, 0x0d, 0x20);
> pci_write_config32(dev,0x10, 0xf4000008);
> pci_write_config32(dev,0x14, 0xfb000000);
> pci_write_config8(dev, 0x3e, 0x02);
> pci_write_config8(dev, 0x3c, 0x0a);
> printk_debug("INSTALL REAL-MODE IDT\n");
> setup_realmode_idt();
> printk_debug("DO THE VGA BIOS\n");
> do_vgabios();
> /* VGA seems to work without this, but crash & burn with it */
> //printk_debug("Enable VGA console\n");
> //vga_enable_console();
> /* It's not clear if these need to be programmed before or after
> * the VGA bios runs. Try both, clean up later */
> /* Set memory rate to 200MHz */
> outb(0x3d, CRTM_INDEX);
> reg8 = inb(CRTM_DATA);
> reg8 &= 0x0f;
> reg8 |= (0x1 << 4);
> outb(0x3d, CRTM_INDEX);
> outb(reg8, CRTM_DATA);
> /* Set framebuffer size (again) */
> reg8 = (CONFIG_VIDEO_MB / 4);
> outb(0x39, SR_INDEX);
> outb(reg8, SR_DATA);
> /* Clear the BOCHs Bios out of memory, so it doesn't confuse linux */
> memset(0xf0000, 0, 0x10000);
>
> +}
>
> +static void vga_read_resources(device_t dev)
> +{
> dev->rom_address = 0xfff80000;
> dev->on_mainboard=1;
> pci_dev_read_resources(dev);
>
> +}
>
> +static struct device_operations vga_operations = {
> .read_resources   = vga_read_resources,
> .set_resources    = pci_dev_set_resources,
> .enable_resources = pci_dev_enable_resources,
> .init             = vga_init,
> .ops_pci          = 0,
>
> +};
>
> +static struct pci_driver vga_driver __pci_driver = {
> .ops = &vga_operations,
> .vendor = PCI_VENDOR_ID_VIA,
> .device = PCI_DEVICE_ID_VIA_CN700_VGA,
>
> +};

Uwe.

[Corey Osgood]

New patch attached, I think I've cleared everything up that can be.
Board patch will have to wait until tomorrow, I need to be up in <5 hours.

Uwe Hermann wrote:
> On Thu, Nov 15, 2007 at 12:38:01PM -0500, Corey Osgood wrote:
> > Add support for the Via CN700 northbridge, with C7 CPU and DDR2 RAM.
>
> Nice! Quick review below:

Thanks! Couple responses, tried to shorten it up a bit too.
> > */
> >
> > +static void c7_cpu_setup(device_t dev)
> > +{
> > /* Host bus interface registers (D0F2 0x50-0x67) */
> > pci_write_config8(dev, 0x50, 0x88);
> > pci_write_config8(dev, 0x51, 0x7a);
> > pci_write_config8(dev, 0x52, 0x6f);
> > pci_write_config8(dev, 0x53, 0x88);
> > pci_write_config8(dev, 0x54, 0x10);//was 0x1e, modifried
> > pci_write_config8(dev, 0x55, 0x16);
> > pci_write_config8(dev, 0x56, 0x01);
> > //pci_write_config8(dev, 0x57, 0x61);//set@200MHz dram clock
> > pci_write_config8(dev, 0x59, 0x60);
> > pci_write_config8(dev, 0x5d, 0xb2);
> > pci_write_config8(dev, 0x5e, 0x88);
> > pci_write_config8(dev, 0x5f, 0xc7);
> > pci_write_config8(dev, 0x60, 0xff);
> > pci_write_config8(dev, 0x61, 0xff);
> > pci_write_config8(dev, 0x62, 0x0f);
> > pci_write_config8(dev, 0x63, 0xff);
> > pci_write_config8(dev, 0x64, 0xff);
> > pci_write_config8(dev, 0x65, 0x0f);
> > pci_write_config8(dev, 0x66, 0xff);
> > pci_write_config8(dev, 0x67, 0x70);
> > /* Host Bus IO Circuit (D0F2 0x70-0x7d) */
> > pci_write_config8(dev, 0x70, 0x33);
> > pci_write_config8(dev, 0x71, 0x00);
> > pci_write_config8(dev, 0x72, 0x33);
> > pci_write_config8(dev, 0x73, 0x00);
> > pci_write_config8(dev, 0x74, 0x00);
> > pci_write_config8(dev, 0x75, 0x28);
> > pci_write_config8(dev, 0x76, 0x74);
> > pci_write_config8(dev, 0x77, 0x00);
> > pci_write_config8(dev, 0x78, 0x0a);
> > pci_write_config8(dev, 0x79, 0xaa);
> > pci_write_config8(dev, 0x7a, 0x24);
> > pci_write_config8(dev, 0x7b, 0x00);
> > pci_write_config8(dev, 0x7c, 0x00);
> > pci_write_config8(dev, 0x7d, 0x6d);
>
> Pretty scary for someone without the datasheet. Please add more comments
> and/or use self-explanatory #defines or something alike.

For the most part these shouldn't be messed with, the values are from
the porting guide and should work fine with any board (YMMV though).
I've added some comments, but to do any real work on this would
absolutely require access to the datasheets.
> > +}
> >
> > +/**
> > Set up various ram and other control registers statically. Some of these may
> > not be needed, other should be done with spd info, but that's a project for
> > the future
> > */
> >
> > +static void sdram_set_registers(const struct mem_controller *ctrl)
> > +{
> > /* TODO: Eliminate any duplicates */
> > pci_write_config8(ctrl->d0f3, 0xd0, 0x88);
> > pci_write_config8(ctrl->d0f3, 0xd1, 0x8b);
> > pci_write_config8(ctrl->d0f3, 0xd2, 0x89);
> > pci_write_config8(ctrl->d0f3, 0x86, 0x2d);
> > /* Driving selection */
> > /* DQ / DQS ODT Driving and Range Select */
> > pci_write_config8(ctrl->d0f3, 0xd5, 0x8a);
> > /* Memory Pads Driving and Range Select */
> > pci_write_config8(ctrl->d0f3, 0xd6, 0xaa);
> > /* DRAM Driving ??? Group DQS */
> > pci_write_config8(ctrl->d0f3, 0xe0, 0xee);
> > /* DRAM Driving ??? Group DQ (DQ, MPD, DQM) */
> > pci_write_config8(ctrl->d0f3, 0xe2, 0xac);//ba
> > /* DRAM Driving ??? Group CS */
> > pci_write_config8(ctrl->d0f3, 0xe4, 0x66);
> > /* DRAM Driving ??? Group MA */
> > pci_write_config8(ctrl->d0f3, 0xe8, 0x86);
> > /* DRAM Driving ??? Group MCLK */
> > pci_write_config8(ctrl->d0f3, 0xe6, 0xaa);
>
> This is better, at least there are a few comments so we (sort of) know
> what might be going on.
>
> It's still problematic if we were to make any changes to the code
> without having access to the datasheets, but I don't know a good way
> to make that easier (short of reproducing the datasheet as code comment).

And since the datasheets are NDA'd, completely reproducing them in
comments could cause some problems. Most of these registers have
different functions for every bit, so this file could easily become very
large very fast.
> > pci_write_config16(ctrl->d0f3, 0x50, 0x0020);
> > /* BA0-2 Selection. Don't mess with */
> > pci_write_config8(ctrl->d0f3, 0x52, 0x33);
> > pci_write_config8(ctrl->d0f3, 0x53, 0x3f);
> > /* Disable bank interleaving. This feature seems useless anyways */
>
> Why useless?

It's the most messed up form of interleaving I've ever heard of. It
allows the northbridge to trick /itself/ into thinking 2 dimms are a
single dimm, via a virtual bank routine and this register. I can't come
up with any useful purpose, since the number of virtual and physical
banks supported is the same. You can also (heh) map any physical bank
(or combination of physical banks) to any virtual bank, to really mess
with someone's head. It makes me wonder if Via is taking bribes from the
makers of Excedrin or Heineken.
> > /* 6. Mode register set. */
> > PRINT_DEBUG_MEM("RAM Enable 4: Mode register set\r\n");
> > //safe value for now, BL=8, WR=5, CAS=5
> > do_ram_command(dev, RAM_COMMAND_MRS, 0x0022d8);
>
> Please list/explain the bits and their meaning, 0x0022d8 is unreadable.

It's from the porting guide, there's a comment now. It somehow conforms
to JEDEC specs, but don't ask me how it works out, the address bits seem
to be scrambled. If someone can figure it out, please, let me know.
> > Index: src/northbridge/via/cn700/raminit.h
> >
> > --- src/northbridge/via/cn700/raminit.h	(revision 0)
> > +++ src/northbridge/via/cn700/raminit.h	(revision 0)
> > @@ -0,0 +1,35 @@
> > +/*
> > This file is part of the LinuxBIOS project.
> > Copyright (C) 2007 Corey Osgood
> > This program is free software; you can redistribute it and/or modify
> > it under the terms of the GNU General Public License as published by
> > the Free Software Foundation; either version 2 of the License, or
> > (at your option) any later version.
> > This program is distributed in the hope that it will be useful,
> > but WITHOUT ANY WARRANTY; without even the implied warranty of
> > MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > GNU General Public License for more details.
> > You should have received a copy of the GNU General Public License
> > along with this program; if not, write to the Free Software
> > Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301 USA
> > */
> >
> > +#ifndef RAMINIT_H
> > +#define RAMINIT_H
> >
> > +#define RAM_COMMAND_NORMAL	0x0
> > +#define RAM_COMMAND_NOP		0x1
> > +#define RAM_COMMAND_PRECHARGE	0x2
> > +#define RAM_COMMAND_MRS		0x3
> > +#define RAM_COMMAND_CBR		0x4
> >
> > +#define DIMM_SOCKETS 1 //Only one on my board.
>
> Well, maybe, but the controller supports more than one?
>
> This is src/northbridge/via/cn700/raminit.h, which should not be specific
> to your board.

The controller supports maybe 4, maybe 8, all depends on if you believe
the datasheet or the porting guide. I set it at 1 for now so that if
anyone tries to use more then 1, they'll have to hunt down why it's not
compiling, and hopefully find that note and figure out that only 1 dimm
will work. Note has been changed, too.
> > Index: src/northbridge/via/cn700/vgabios.c
> >
> > --- src/northbridge/via/cn700/vgabios.c	(revision 0)
> > +++ src/northbridge/via/cn700/vgabios.c	(revision 0)
>
> There must be a better way than cloning the whole file. Did you change
> any of the contents?

Yes, the vga rom address, which is hardcoded (2Mb rom on epia-m, 4Mb on
j7f2). Once I can get another board I plan to see if I can get
BOCHS+x86emu to work, if that works this file can be dropped. If not,
I'll do some cleanup work to make it work on both epia-m and here, and
give it a nice home somewhere. It might be a while before I do though,
and I figure other people probably want to use/play with this in the
meantime.

Thanks,
Corey

[ron minnich]

Corey, your responses to these answers are good and I wonder if they
might go as comments in the code.

In general, if a register setting is so confidential we can't talk
about it, we could just
tag the block with
/* NDA required */
/* Done NDA required */
or some such couldn't we?

On Nov 16, 2007 12:19 AM, Corey Osgood  wrote:
> New patch attached, I think I've cleared everything up that can be.
> Board patch will have to wait until tomorrow, I need to be up in <5 hours.
>
> Uwe Hermann wrote:
> > On Thu, Nov 15, 2007 at 12:38:01PM -0500, Corey Osgood wrote:
> > Pretty scary for someone without the datasheet. Please add more comments
> > and/or use self-explanatory #defines or something alike.
>
> For the most part these shouldn't be messed with, the values are from
> the porting guide and should work fine with any board (YMMV though).
> I've added some comments, but to do any real work on this would
> absolutely require access to the datasheets.

So just tag them as magic values and we are fine.
> And since the datasheets are NDA'd, completely reproducing them in
> comments could cause some problems. Most of these registers have
> different functions for every bit, so this file could easily become very
> large very fast.

Ollie once pointed out to me that we don't want to replicate the data
book in the code, I agree with you on this. Past a point, people can
go to the data book.
> It's the most messed up form of interleaving I've ever heard of. It
> allows the northbridge to trick /itself/ into thinking 2 dimms are a
> single dimm, via a virtual bank routine and this register. I can't come
> up with any useful purpose, since the number of virtual and physical
> banks supported is the same. You can also (heh) map any physical bank
> (or combination of physical banks) to any virtual bank, to really mess
> with someone's head. It makes me wonder if Via is taking bribes from the
> makers of Excedrin or Heineken.

Maybe without the more extreme comments, this might be useful in the code :-)

Anyway, I'm looking forward to the commit message :-)

thanks

ron