[LinuxBIOS] [LinuxBIOSv2] WIP Support for RCA RM4100 - i82830 Complete

Uwe Hermann uwe at hermann-uwe.de
Tue Sep 18 14:33:26 CEST 2007


On Mon, Sep 17, 2007 at 10:34:35PM -0400, joe at smittys.pointclark.net wrote:
> Here you go Uwe,
> i82830 northbridge code is done. Works great. Note: Checkout the generic 
> spd_get_dimm_size()function in raminit.c, there is a pretty cool way to get 
> each side of a dimm's size only using SPD 5 and 31 (original idea from 
> Corey, Thanks). Works for dimms up to 2GB. Works on SDRAM and DDR, DDR2, 
> maybe more.

Yep, I have a similarly generic SPD function here (not yet submitted),
both should go into a global lib/spd.c file. I'll check if this is
related to your function so we can merge them, if necessary.


> Signed-off-by: Joseph Smith <joe at smittys.pointclark.net>
> Acked-by: Joseph Smith <joe at smittys.pointclark.net>

Only Signed-off-by, please. The Acked-by is added by the reviewer if the
code is ready to be committed.


> RCA RM4100 is still having problems below with PCI to PCI Bridge (i82801DB 
> Southbridge issue?). Please submit as "WIP". Note: this board is classified 
> as a Set-top-box.

I'd like to wait with committing until (a) a few issues are resolved,
and (b) the board boots up to a Linux login prompt, if possible.


> Index: src/mainboard/rca/rm4100/Config.lb

RCA RM4100 is the name of the box as a whole, correct? Can you look at
the mainboard and search for a vendor and mainboard name/ID?
If there is one (printed on the PCB), we should use that as the
target name in LinuxBIOS (as we did with other embedded stuff).


> +chip northbridge/intel/i82830
> +	device pci_domain 0 on 
> +		device pci 0.0 on end # Host bridge: Intel Corporation 82830 830 Chipset Host Bridge
> +		device pci 1.0 off end # Host-AGP Bridge
> +		device pci 2.0 off end # VGA compatible controller: Intel Corporation 82830 CGC
> +		chip southbridge/intel/i82801xx
> +			device pci 1d.0 on end # USB UHCI Controller #1
> +			device pci 1d.1 on end # USB UHCI Controller #2
> +			device pci 1d.2 on end # USB UHCI Controller #3
> +			device pci 1d.7 on end # USB2 EHCI Controller
> +			device pci 1e.0 on end # PCI bridge: Intel Corporation 82801 PCI Bridge
> +			device pci 1f.0 on # ISA bridge: Intel Corporation 82801DB LPC Interface SouthBridge
> +				chip superio/smsc/smscsuperio 

Very good, you're using the generic 'smscsuperio' code. Once this boots
to a Linux prompt, please let me know how well the code works for this
Super I/O. There are still some TODOs in the 'smscsuperio' code.

Which Super I/O model is on your board?


> +					device pnp 2e.0 off # Floppy
> +						io 0x60 = 0x3f0
> +						irq 0x70 = 6
> +						drq 0x74 = 2
> +					end
> +					device pnp 2e.3 off # Parallel Port
> +						io 0x60 = 0x378
> +						irq 0x70 = 7
> +					end
> +					device pnp 2e.4 on # Com1
> +						io 0x60 = 0x3f8
> +						irq 0x70 = 4
> +					end
> +					device pnp 2e.5 off # Com2 / IR
> +						io 0x60 = 0x2f8
> +						irq 0x70 = 3
> +					end
> +					device pnp 2e.7 on # Keyboard
> +						io 0x60 = 0x60
> +						io 0x62 = 0x64
> +						irq 0x70 = 1 # Keyboard interrupt
> +						irq 0x72 = 12 # Mouse interrupt
> +					end
> +					device pnp 2e.9 off end # Game Port
> +					device pnp 2e.a off end # PME
> +					device pnp 2e.b off end # MPU-401

Did you check whether the above values are all correct?
Attach as many devices as possible to the box (PS/2 keyboard, PS/2
mouse, serial, parallel, etc), then boot it in Linux and send the output
of 'lspnp -v', please. lspnp is part of the pnputils (or similar) package.


> +				end
> +			end
> +			device pci 1f.1 on end # IDE interface: Intel Corporation 82801DB IDE Controller
> +			device pci 1f.3 on end # SMBus: Intel Corporation 82801DB SMBus Controller
> +			device pci 1f.5 on end # Multimedia audio controller: AC'97 Audio Controller
> +			device pci 1f.6 on end # Modem: AC'97 Modem Controller
> +		end
> +	end
> +	chip cpu/intel/socket_PGA370

Is this a Socket 370 board?


> +	end
> +end
> +
> 
> Property changes on: src/mainboard/rca/rm4100/Config.lb
> ___________________________________________________________________
> Name: svn:executable
>    + *

Should be dropped, there's not reason to make any of the files
executable.


> Index: src/mainboard/rca/rm4100/spd_table.h
> ===================================================================
> --- src/mainboard/rca/rm4100/spd_table.h	(revision 0)
> +++ src/mainboard/rca/rm4100/spd_table.h	(revision 0)
> @@ -0,0 +1,42 @@
> +/*
> + * This file is part of the LinuxBIOS project.
> + *
> + * Copyright (C) 2007 Joseph Smith <joe at smittys.pointclark.net>
> + *
> + * 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>
> +
> +struct spd_entry {
> +	unsigned int address;
> +	unsigned int data;
> +	};
> +
> +/* The onboard 128MB PC133 memory does not have a SPD eeprom so the
> + * values have to be set manually, 
> + * the onboard memory is located in socket1 (0x51).
> +*/
> +
> +const struct spd_entry spd_table [] = 
> +{
> +{2,	0x04}, /* (Fundamental) memory type */
> +{4,	0x09}, /* Number of column address bits */
> +{6,	0x40}, /* Module data width (LSB) */
> +{5,	0x01}, /* Number of module rows (banks) */
> +{9,	0x75}, /* SDRAM cycle time (highest CAS latency), RAS access time (tRAC) */
> +{10,	0x54}, /* SDRAM access time from clock (highest CAS latency), CAS access time (Tac, tCAC) */
> +{31,	0x20}, /* Density of each row on module */

Don't use magic constants, please use the #defines from spd.h.

Also, the coding style is wrong is some parts, please check
http://linuxbios.org/index.php/Development_Guidelines#Coding_Style
http://lxr.linux.no/source/Documentation/CodingStyle


> Index: src/mainboard/rca/rm4100/irq_tables.c
> ===================================================================
> --- src/mainboard/rca/rm4100/irq_tables.c	(revision 0)
> +++ src/mainboard/rca/rm4100/irq_tables.c	(revision 0)
> @@ -0,0 +1,42 @@
> +/* This file was generated by getpir.c, do not modify! 
> +   (but if you do, please run checkpir on it to verify)
> + * Contains the IRQ Routing Table dumped directly from your memory, which BIOS sets up
> + *
> + * Documentation at : http://www.microsoft.com/hwdev/busbios/PCIIRQ.HTM
> +*/
> +
> +#include <arch/pirq_routing.h>
> +
> +const struct irq_routing_table intel_irq_routing_table = {
> +	PIRQ_SIGNATURE,  /* u32 signature */
> +	PIRQ_VERSION,    /* u16 version   */
> +	32+16*12,	 /* there can be total 12 devices on the bus */

32*16*IRQ_SLOT_COUNT


> +	0x00,		 /* Where the interrupt router lies (bus) */
> +	(0x1f<<3)|0x0,   /* Where the interrupt router lies (dev) */
> +	0,		 /* IRQs devoted exclusively to PCI usage */
> +	0x8086,		 /* Vendor */
> +	0x24cc,		 /* Device */
> +	0,		 /* Crap (miniport) */
> +	{ 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 }, /* u8 rfu[11] */
> +	0x50,         /*  u8 checksum , this hase to set to some value that would give 0 after the sum of all bytes for this structure (including checksum) */
> +	{
> +		/* bus,     dev|fn,   {link, bitmap}, {link, bitmap}, {link, bitmap}, {link, bitmap},  slot, rfu */
> +		{0x01,(0x08<<3)|0x0, {{0x68, 0xdef8}, {0x00, 0x0000}, {0x00, 0x0000}, {0x00, 0x00000}}, 0x0, 0x0},
> +		{0x00,(0x1f<<3)|0x0, {{0x62, 0xdef8}, {0x61, 0xdef8}, {0x00, 0x0000}, {0x00, 0x00000}}, 0x0, 0x0},
> +		{0x00,(0x1d<<3)|0x0, {{0x60, 0xdef8}, {0x63, 0xdef8}, {0x62, 0xdef8}, {0x6b, 0x0def8}}, 0x0, 0x0},
> +		{0x00,(0x00<<3)|0x0, {{0x60, 0xdef8}, {0x61, 0xdef8}, {0x62, 0xdef8}, {0x63, 0x0def8}}, 0x0, 0x0},
> +		{0x00,(0x01<<3)|0x0, {{0x60, 0xdef8}, {0x61, 0xdef8}, {0x00, 0x0000}, {0x00, 0x00000}}, 0x0, 0x0},
> +		{0x01,(0x00<<3)|0x0, {{0x60, 0xdef8}, {0x61, 0xdef8}, {0x62, 0xdef8}, {0x63, 0x0def8}}, 0x0, 0x0},
> +		{0x01,(0x01<<3)|0x0, {{0x63, 0xdef8}, {0x60, 0xdef8}, {0x61, 0xdef8}, {0x62, 0x0def8}}, 0x1, 0x0},
> +		{0x01,(0x02<<3)|0x0, {{0x62, 0xdef8}, {0x63, 0xdef8}, {0x60, 0xdef8}, {0x61, 0x0def8}}, 0x2, 0x0},
> +		{0x01,(0x09<<3)|0x0, {{0x63, 0xdef8}, {0x00, 0x0000}, {0x00, 0x0000}, {0x00, 0x00000}}, 0x0, 0x0},
> +		{0x01,(0x06<<3)|0x0, {{0x61, 0xdef8}, {0x00, 0x0000}, {0x00, 0x0000}, {0x00, 0x00000}}, 0x0, 0x0},
> +		{0x01,(0x07<<3)|0x0, {{0x63, 0xdef8}, {0x62, 0xdef8}, {0x00, 0x0000}, {0x00, 0x00000}}, 0x0, 0x0},
> +		{0x00,(0x02<<3)|0x0, {{0x60, 0xdef8}, {0x61, 0xdef8}, {0x00, 0x0000}, {0x00, 0x00000}}, 0x0, 0x0},
> +	}
> +};

Is this copied from another target, or written manually, or based on
getpir output?

Once the board boots, please check if it's correct, i.e. if all devices
work as expected.

Either way, this file needs a license header, too. You can drop the
"generated by getpir" blurb at the top, IMHO.


> Index: src/mainboard/rca/rm4100/debug.c
> ===================================================================
> --- src/mainboard/rca/rm4100/debug.c	(revision 0)
> +++ src/mainboard/rca/rm4100/debug.c	(revision 0)

Please drop this file completely. If you need it, please add it locally
when testing or use a global version of it (I think there is one global
version, or even two; if not, we should add it).


> Index: src/mainboard/rca/rm4100/Options.lb
[...]
> +uses DEBUG
> +
> +## ROM_SIZE is the size of boot ROM that this board will use.
> +default ROM_SIZE  = 524288

default ROM_SIZE = 512 * 1024

Let's not make this more complicated than it is already.


> +## IDE Support
> +default CONFIG_IDE = 1

Needed?

This should rather be a "register" setting in Config.lb as with other
boards (if at all), I think.


> +## Request this level of debugging output
> +default  DEFAULT_CONSOLE_LOGLEVEL = 9
> +## At a maximum only compile in this level of debugging
> +default  MAXIMUM_CONSOLE_LOGLEVEL = 9
> +
> +default DEBUG = 0

Where is DEBUG used? Why not just use DEFAULT_CONSOLE_LOGLEVEL?


> Index: src/mainboard/rca/rm4100/failover.c
> ===================================================================
> --- src/mainboard/rca/rm4100/failover.c	(revision 0)
> +++ src/mainboard/rca/rm4100/failover.c	(revision 0)

Nope, please drop this. There's a generic version somewhere in arch/i386/*,
please use that. You'll need to adapt Config.lb accordingly, see
http://tracker.linuxbios.org/trac/LinuxBIOS/changeset/2772
for an example.


> +#include "spd_table.h"
> +
> +#define SERIAL_DEV PNP_DEV(0x2e, SMSCSUPERIO_SP1)
> +
> +#include "southbridge/intel/i82801xx/i82801xx_early_smbus.c"
> +
> +/* The onboard 128MB PC133 memory does not have a SPD eeprom so the
> + * values have to be set manually, 
> + * the onboard memory is located in socket1 (0x51).
> +*/
> +static inline int spd_read_byte(unsigned device, unsigned address)
> +{
> +	int i;
> +
> +	if (device == 0x50){
> +		return smbus_read_byte(device, address);

If 0x51 is RAM, what is 0x50 then?


> +	} else if (device == 0x51){
> +		for (i=0; i < (sizeof spd_table/sizeof spd_table[0]); i++){
> +			if (spd_table[i].address == address){
> +				return spd_table[i].data;
> +			}
> +		}
> +		return 0xFF; /* This line returns 0xFF when address not found */
> +	} else {
> +		return 0xFF; /* returns 0xFF on any failures */
> +	}
> +}
> +
> +static void ac97_io_enable(void)
> +{
> +	device_t dev;
> +
> +	/* Set the ac97 audio device staticly. */
> +	dev = PCI_DEV(0x0, 0x1f, 0x5);
> +
> +	/* Enable access to the IO space. */
> +	pci_write_config8(dev, 0x41, 0x01);

Is this really needed? The lines in Config.lb should instruct LinuxBIOS
to properly enable the device, no?


> +/* TODO: Not needed? */
> +void udelay(int usecs) 
> +{
> +	int i;
> +	for (i = 0; i < usecs; i++)
> +		outb(i&0xff, 0x80);
> +}

Nope, not needed, please drop it. If you get build errors, you must
include udelay_io.c (or so).


> Index: src/northbridge/intel/i82830/i82830.h
[...]
> +#define VID 0X00     /* Vendor Identification Register - Default Value 0X8086 - 16 bits Read Only */
> +#define DID 0X02     /* Device Identification Register - Default Value 0X3575 - 16 bits Read Only */
> +#define RID 0X08     /* Revision Identification Register - Default Value 0X04 - 8 bits Read Only */
> +#define SUBC 0X0A     /* SUB-Class Code Register - Default Value 0X00 - 8 bits Read Only */
> +#define BCC 0X0B     /* Base Class Code Register - Default Value 0X06 - 8 bits Read Only */
> +#define MLT 0X0D     /* Master Latency Timer Register - Default Value 0X00 - 8 bits Read Only */
> +#define HDR 0X0E     /* Header Type Register - Default Value 0X00 - 8 bits Read Only */
> +#define CAPPTR 0X34     /* Capablities Pointer - Default Value 0X40 - 8 bits Read Only */
> +
> +#define PCICMD0 0X04  /* PCI Command Register - Default Value 0X0006 - 16 bits */
> +#define PCISTS 0X06  /* PCI Status Register - Default Value 0X0010 - 16 bits */

I think you can drop all of the above (and maybe also SVID/SID), they're
generic PCI registers, not i82830 specific at all.


> +#define SVID 0X2C    /* Subsystem Vendor ID - Default Value 0X0000 - 16 bits */
> +#define SID 0X2E    /* Subsystem ID - Default Value 0X0000 - 16 bits */
> +
> +#define RRBAR 0X48    /* Register Range Base Address Register - Default Value 0X00000000 - 32 bits */
> +#define GCC0 0X50    /* GMCH Control Register #0 - Default Value 0XA072 - 16 bits */
> +#define GCC1 0X52    /* GMCH Control Register #1 - Default Value 0X0000 - 16 bits */
> +#define FDHC 0X58    /* Fixed DRAM Hole Control Register - Default Value 0X00 - 8 bits */
> +#define PAM0 0X59     /* Programable Attribute Map Register #0 - Default Value 0X00 - 8 bits */
> +#define PAM1 0X5A     /* Programable Attribute Map Register #1 - Default Value 0X00 - 8 bits */
> +#define PAM2 0X5B     /* Programable Attribute Map Register #2 - Default Value 0X00 - 8 bits */
> +#define PAM3 0X5C     /* Programable Attribute Map Register #3 - Default Value 0X00 - 8 bits */
> +#define PAM4 0X5D     /* Programable Attribute Map Register #4 - Default Value 0X00 - 8 bits */
> +#define PAM5 0X5E     /* Programable Attribute Map Register #5 - Default Value 0X00 - 8 bits */
> +#define PAM6 0X5F     /* Programable Attribute Map Register #6 - Default Value 0X00 - 8 bits */
> +#define DRB 0X60     /* DRAM Row Boundary Register #0 - Default Value 0X00 - 8 bits */
> +#define DRB1 0X61     /* DRAM Row Boundary Register #1 - Default Value 0X00 - 8 bits */
> +#define DRB2 0X62     /* DRAM Row Boundary Register #2 - Default Value 0X00 - 8 bits */
> +#define DRB3 0X63     /* DRAM Row Boundary Register #3 - Default Value 0X00 - 8 bits */
> +#define DRA 0X70     /* DRAM Row Attribute Register #0 - Default Value 0XFF - 8 bits */
> +#define DRA1 0X71     /* DRAM Row Attribute Register #1 - Default Value 0XFF - 8 bits */
> +#define DRT	0X78     /* DRAM Timing Register - Default Value 0X00000010 - 32 bits */
> +#define DRC  0X7C     /* DRAM Controller Mode Register #0 - Default Value 0X00000000 - 32 bits */
> +#define DRC1 0X7D     /* DRAM Controller Mode Register #1 - Default Value 0X00000000 - 32 bits */
> +#define DRC2 0X7E     /* DRAM Controller Mode Register #2 - Default Value 0X00000000 - 32 bits */
> +#define DRC3 0X7F     /* DRAM Controller Mode Register #3 - Default Value 0X00000000 - 32 bits */
> +#define DTC	0X8C     /* DRAM Throttling Control Register - Default Value 0X00000000 - 32 bits */
> +#define SMRAM 0X90   /* System Management RAM Control Register - Default Value 0X02 - 8 bits */
> +#define ESMRAMC 0X91 /* Extended System Management RAM Control Register - Default Value 0X38 - 8 bits */
> +
> +#define ERRSTS 0X92    /* Error Status Register - Default Value 0X0000 - 16 bits */
> +#define ERRCMD 0X94    /* Error Command Register - Default Value 0X0000 - 16 bits */
> +
> +#define BUFF_SC 0XEC     /* System Memory Buffer Strength Control Register - Default Value 0X00000000 - 32 bits */
> +
> +#define APBASE 0X10    /* Aperture Base Configuration Register - Default Value 0X00000008 - 32 bits */
> +#define APSIZE 0XB4     /* Apterture Size - Default Value 0X00 - 8 bits */
> +#define ATTBASE 0XB8     /* Aperture Translation Table Base Register - Default Value 0X00000000 - 32 bits */

Not _too_ different to i815, it seems. Any chance this code could be
merged into the i815 code, with some i830 specifics special-cased?
If that's possible I'd really prefer it over creating another
near-duplicate code base.


> Index: src/northbridge/intel/i82830/raminit.c
> ===================================================================
> --- src/northbridge/intel/i82830/raminit.c	(revision 0)
> +++ src/northbridge/intel/i82830/raminit.c	(revision 0)
> @@ -0,0 +1,489 @@
> +/*
> + * This file is part of the LinuxBIOS project.
> + *
> + * Copyright (C) 2007 Joseph Smith <joe at smittys.pointclark.net>
> + *
> + * 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
> + */


> +static void do_ram_command(const struct mem_controller *ctrl, uint32_t command, uint32_t addr_offset)
> +{
> +	uint8_t reg;
> +
> +	/* Configure the RAM command. */
> +	reg = pci_read_config32(ctrl->d0, DRC);
> +	/* Clear bits 29, 10-8, 6-4. */
> +	reg &= 0xdffff88f;
> +	reg |= command << 4;
> +      /* If RAM_COMMAND_NORMAL set the refresh mode and IC bit. */
> +	if (command == RAM_COMMAND_NORMAL) {
> +	reg |= ((RAM_COMMAND_REFRESH << 8) | (1 << 29));

Is (1 << 29) the IC bit? If so, please use a #define for it. Easily
readable #defines for magic constants are always a good thing.


> +		/* First check if a DIMM is actually present. */
> +		if (spd_read_byte(device, 2) == 0x4) {
> +			print_debug("Found DIMM in slot ");
> +			print_debug_hex8(i);
> +			print_debug(", setting DRA...\r\n");
> +
> +			dra = 0x00;
> +
> +			/* columns */
> +			col = spd_read_byte(device, 4);
> +
> +			/* data width */
> +			width = spd_read_byte(device, 6);
> +
> +			/* calculate page size in bits */
> +			value = ((1 << col) * width);
> +
> +			/* convert to Kilobytes */
> +			dra = ((value / 8) >> 10);
> +
> +			/* # of banks of DIMM (single or double sided) */
> +			value = spd_read_byte(device, 5);
> +
> +			if (value == 1) {
> +				/* 2KB */
> +				if (dra == 0x2) {
> +					dra = 0xF0;
> +				/* 4KB */
> +				} else if (dra == 0x4) {
> +					dra = 0xF1;
> +				/* 8KB */
> +				} else if (dra == 0x8) {
> +					dra = 0xF2;
> +				/* 16KB */
> +				} else if (dra == 0x10) {
> +					dra = 0xF3;
> +				} else {
> +					print_err("Page size not supported\r\n");
> +					die("HALT\r\n");
> +				}
> +			} else if (value == 2) {
> +				/* 2KB */
> +				if (dra == 0x2) {
> +					dra = 0x00;
> +				/* 4KB */
> +				} else if (dra == 0x4) {
> +					dra = 0x11;
> +				/* 8KB */
> +				} else if (dra == 0x8) {
> +					dra = 0x22;
> +				/* 16KB */
> +				} else if (dra == 0x10) {
> +					dra = 0x33;
> +				} else {
> +					print_err("Page size not supported\r\n");
> +					die("HALT\r\n");
> +				}

This code can be made a lot shorter using a table and a loop, I think.


> +static void sdram_set_registers(const struct mem_controller *ctrl)
> +{
> +	unsigned value;
> +	value = 0;
> +
> +	PRINT_DEBUG("Setting initial registers....\r\n");
> +
> +	/* Set the value for PCI Command Register */
> +	pci_write_config16(ctrl->d0, PCICMD0, 0x0006);
> +
> +	/* Set the value for PCI Status Register */
> +	pci_write_config16(ctrl->d0, PCISTS, 0x0010);

Are you sure you need the two above?


> +      /* Set the value for GMCH Control Register #0 */
> +      pci_write_config16(ctrl->d0, GCC0, 0xA072);
> +
> +	/* Set the value for GMCH Control Register #1
> +	 * Only set bits 6-0, bits 15-7 are reserved.
> +	 */
> +	value = pci_read_config16(ctrl->d0, GCC1);
> +	value |= 0x20;
> +	pci_write_config16(ctrl->d0, GCC1, value);
> +
> +	/* Set the value for Fixed DRAM Hole Control Register */
> +	pci_write_config8(ctrl->d0, FDHC, 0x00);
> +
> +	/* Set the value for Aperture Base Configuration Register  */
> +	pci_write_config32(ctrl->d0, APBASE, 0x00000008);
> +
> +	/* Set the value for Register Range Base Address Register */
> +	pci_write_config32(ctrl->d0, RRBAR, 0x00000000);
> +
> +	/* Set the value for Programable Attribute Map Registers
> +	 * Ideally, this should be R/W for as many ranges as possible.
> +	 */
> +	pci_write_config8(ctrl->d0, PAM0, 0x30);
> +	pci_write_config8(ctrl->d0, PAM1, 0x33);
> +	pci_write_config8(ctrl->d0, PAM2, 0x33);
> +	pci_write_config8(ctrl->d0, PAM3, 0x33);
> +	pci_write_config8(ctrl->d0, PAM4, 0x33);
> +	pci_write_config8(ctrl->d0, PAM5, 0x33);
> +	pci_write_config8(ctrl->d0, PAM6, 0x33);
> +
> +	/* Set the value for DRAM Throttling Control Register */
> +	pci_write_config32(ctrl->d0, DTC, 0x00000000);
> +
> +	/* Set the value for System Management RAM Control Register */
> +	pci_write_config8(ctrl->d0, SMRAM, 0x02);
> +
> +	/* Set the value for Extended System Management RAM Control Register */
> +	pci_write_config8(ctrl->d0, ESMRAMC, 0x38);

This is all hardcoded for your board, and needs to be more generic on
the long run, I think.


> Index: src/northbridge/intel/i82830/northbridge.c
> ===================================================================
> --- src/northbridge/intel/i82830/northbridge.c	(revision 0)
> +++ src/northbridge/intel/i82830/northbridge.c	(revision 0)

I don't really like this file (way too much duplicated code).

We need a global northbridge.c file in lib/ which contains all the
common code. Only the mainboard-specifics should be in this file here.


> +static void northbridge_init(device_t dev)
> +{
> +	printk_spew("Northbridge init\n");
> +}
> +
> +static struct device_operations northbridge_operations = {
> +	.read_resources	= pci_dev_read_resources,
> +	.set_resources	= pci_dev_set_resources,
> +	.enable_resources	= pci_dev_enable_resources,
> +	.init			= northbridge_init,
> +	.enable		= 0,
> +	.ops_pci		= 0,
> +};
> +
> +static struct pci_driver northbridge_driver __pci_driver = {
> +	.ops	= &northbridge_operations,
> +	.vendor	= PCI_VENDOR_ID_INTEL,
> +	.device	= 0x3575,

Should be a #define, probably in pci_ids.h.


> +#define BRIDGE_IO_MASK (IORESOURCE_IO | IORESOURCE_MEM)
> +
> +static void pci_domain_read_resources(device_t dev)
> +{
> +	struct resource *resource;
> +
> +	/* 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;
> +}

Common.


> +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;
> +}

Common.


> +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;
> +}

Common.


> +static uint32_t find_pci_tolm(struct bus *bus)
> +{
> +	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;
> +	}
> +	return tolm;
> +}

Common.


> +static void pci_domain_set_resources(device_t dev)
> +{
> +	device_t mc_dev;
> +        uint32_t pci_tolm;
> +
> +        pci_tolm = find_pci_tolm(&dev->link[0]);
> +	mc_dev = dev->link[0].children;
> +	if (mc_dev) {
> +		/* Figure out which areas are/should be occupied by RAM.
> +		 * This is all computed in kilobytes and converted to/from
> +		 * the memory controller right at the edges.
> +		 * Having different variables in different units is
> +		 * too confusing to get right.  Kilobytes are good up to
> +		 * 4 Terabytes of RAM...
> +		 */
> +
> +		unsigned long tomk, tolmk;
> +		int idx;
> +
> +		/* Get the value of the highest DRB. This tells the end of
> +		 * the physical memory.  The units are ticks of 32MB
> +		 * i.e. 1 means 32MB.
> +		 */
> +		tomk = ((unsigned long)pci_read_config8(mc_dev, DRB+3)) << 15;
> +
> +		printk_debug("Setting RAM size to %d\n", tomk);
> +
> +		/* Compute the top of Low memory */
> +		tolmk = pci_tolm >> 10;
> +		if (tolmk >= tomk) {
> +			/* The PCI hole does does not overlap the memory.
> +			 */
> +			tolmk = tomk;
> +		}
> +
> +		/* Report the memory regions */
> +		idx = 10;
> +		ram_resource(dev, idx++, 0, 640);
> +		ram_resource(dev, idx++, 1024, tolmk - 1024);
> +	}
> +	assign_resources(&dev->link[0]);
> +}

This one contains chipset-specific stuff, but even that can be made
generic enough to be in a global location, and take some parameters
to handle the chipset specifics (e.g. the max. DRB register value).


> Index: src/northbridge/intel/i82830/northbridge.h
[...]
> +#ifndef NORTHBRIDGE_INTEL_I82830_NORTHBRIDGE_H
> +#define NORTHBRIDGE_INTEL_I82830_NORTHBRIDGE_H
> +
> +extern unsigned int i82830_scan_root_bus(device_t root, unsigned int max);

Is this needed? Is the whole file needed?


> Index: targets/rca/rm4100/Config.lb
> ===================================================================
> --- targets/rca/rm4100/Config.lb	(revision 0)
> +++ targets/rca/rm4100/Config.lb	(revision 0)
[...]
> +## Total number of bytes allocated for LinuxBIOS use
> +## (normal AND fallback images and payloads).
> +option ROM_SIZE = 524288

option ROM_SIZE = 512 * 1024


> +## Debugging
> +## option DEBUG = 1

Please use the standard DEFAULT_CONSOLE_LOGLEVEL, if possible.


> +romimage "normal"
> +	option USE_FALLBACK_IMAGE = 0
> +	option ROM_IMAGE_SIZE = 131072

option ROM_IMAGE_SIZE = 128 * 1024


> +	option LINUXBIOS_EXTRA_VERSION = ".0Normal"
> +	payload /etc/hosts
> +#	payload /home/amp/filo-0.5/filo.elf
> +end
> +
> +romimage "fallback" 
> +	option USE_FALLBACK_IMAGE = 1
> +	option ROM_IMAGE_SIZE = 131072

option ROM_IMAGE_SIZE = 128 * 1024


> LinuxBIOS-2.0.0.0Fallback Wed Sep 12 07:11:03 EDT 2007 starting...
> Setting initial registers....
> Initial registers have been set.
> No DIMM found in slot 00
> DRB 0x60 has been set to 0x00
> DRB1 0x61 has been set to 0x00
> Found DIMM in slot 01
> DIMM is 0x0080 on side 1
> DIMM is 0x0000 on side 2
> DRB2 0x62 has been set to 0x04
> DRB3 0x63 has been set to 0x04
> No DIMM found in slot 00, setting DRA to 0xFF
> DRA 0x70 has been set to 0xff
> Found DIMM in slot 01, setting DRA...
> DRA 0x71 has been set to 0xf1
> RAM Enable 1: Apply NOP
>     Sending RAM command 0x00000010 to 0x00000000
> RAM Enable 2: Precharge all
>     Sending RAM command 0x00000020 to 0x00000000
> RAM Enable 3: CBR
>     Sending RAM command 0x00000060 to 0x00000000
> RAM Enable 4: Mode register set
>     Sending RAM command 0x00000030 to 0x000001d0
> RAM Enable 5: Normal operation
>     Sending RAM command 0x20000170 to 0x00000000
> Northbridge following SDRAM init:
> PCI: 00:00.00
> 00: 86 80 75 35 06 00 10 00 04 00 00 06 00 00 00 00
> 10: 08 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> 20: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> 30: 00 00 00 00 40 00 00 00 00 00 00 00 00 00 00 00
> 40: 09 00 05 01 00 00 00 00 00 00 00 00 02 28 00 0e
> 50: 72 a0 20 00 00 00 00 00 00 30 33 33 33 33 33 33
> 60: 00 00 04 04 00 00 00 00 00 00 00 00 00 00 00 00
> 70: ff f1 ff ff 00 00 00 00 10 00 00 00 70 01 00 20
> 80: 00 00 00 00 00 00 00 00 00 90 00 40 00 00 00 00
> 90: 02 38 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> a0: 02 00 20 00 17 02 00 1f 00 00 00 00 00 00 00 00
> b0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> c0: 00 54 0e 41 a2 99 01 00 c0 00 00 00 00 00 00 00
> d0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> e0: 00 00 00 00 00 00 00 00 00 00 00 00 09 c9 9f fc
> f0: 11 11 01 00 00 00 0b 05 37 d6 30 d0 22 d0 23 cf
> Copying LinuxBIOS to RAM.

Did you enable the ram_check() function and did you verify
that the RAM init actually works?


After the above fixes, the code is in good shape IMO, so we should be
able to commit it as soon as a Linux boot works.

You can split out the northbridge code in an extra patch if you want,
I think we can commit that even before your board works, so that others
who have i830 boards can work with it.
But please consider merging the i830 code into the i815 code, if at all
possible.


Uwe.
-- 
http://www.hermann-uwe.de  | http://www.holsham-traders.de
http://www.crazy-hacks.org | http://www.unmaintained-free-software.org
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://www.coreboot.org/pipermail/coreboot/attachments/20070918/5748d065/attachment.sig>


More information about the coreboot mailing list