[LinuxBIOS] [PATCH][v3] geodelx: Code cleanups

Marc Jones marc.jones at amd.com
Mon Jul 23 22:32:01 CEST 2007



Uwe Hermann wrote:
> See patch.
> 
> Uwe.
> 
> 
> ------------------------------------------------------------------------
> 
> Various code cleanups:
> 
>  - Simplify lots of code, especially msr-related code.
> 
>  - Move struct msrinit declaration into msr.h as we use it quite often
>    and there's no use to duplicate it again and again in each file.
> 
>  - Remove unrequired variable usage (e.g. numEnabled, msrnum, val, port).
> 
>  - Remove useless comments.
> 
> Signed-off-by: Uwe Hermann <uwe at hermann-uwe.de>
> 

The changes look good. I had some problems patching 5536.c so you might 
want to double check it before you submit it to svn.

Acked-by: Marc Jones <marc.jones at amd.com>







> Index: southbridge/amd/cs5536/stage1.c
> ===================================================================
> --- southbridge/amd/cs5536/stage1.c	(Revision 460)
> +++ southbridge/amd/cs5536/stage1.c	(Arbeitskopie)
> @@ -19,6 +19,7 @@
>  
>  #include <types.h>
>  #include <msr.h>
> +#include <lib.h>
>  #include <amd_geodelx.h>
>  #include "cs5536.h"
>  
> @@ -43,7 +44,7 @@
>  	struct msr msr;
>  
>  	/* Forward MSR access to CS5536_GLINK_PORT_NUM to CS5536_DEV_NUM. */
> -	msr.hi = msr.lo = 0x00000000;
> +	msr.hi = msr.lo = 0;
>  
>  	/* TODO: unsigned char -> u8? */
>  #if CS5536_GLINK_PORT_NUM <= 4
> @@ -88,6 +89,14 @@
>  	}
>  }
>  
> +static const struct msrinit msr_table[] = {
> +	{MDD_LBAR_SMB,   {.hi = 0x0000f001, .lo = SMBUS_IO_BASE}},
> +	{MDD_LBAR_GPIO,  {.hi = 0x0000f001, .lo = GPIO_IO_BASE}},
> +	{MDD_LBAR_MFGPT, {.hi = 0x0000f001, .lo = MFGPT_IO_BASE}},
> +	{MDD_LBAR_ACPI,  {.hi = 0x0000f001, .lo = ACPI_IO_BASE}},
> +	{MDD_LBAR_PMS,   {.hi = 0x0000f001, .lo = PMS_IO_BASE}},
> +};
> +
>  /**
>   * Set up I/O bases for SMBus, GPIO, MFGPT, ACPI, and PM.
>   *
> @@ -97,32 +106,10 @@
>   */
>  static void cs5536_setup_iobase(void)
>  {
> -	struct msr msr;
> +	int i;
>  
> -	/* Setup LBAR for SMBus controller. */
> -	msr.hi = 0x0000f001;
> -	msr.lo = SMBUS_IO_BASE;
> -	wrmsr(MDD_LBAR_SMB, msr);
> -
> -	/* Setup LBAR for GPIO. */
> -	msr.hi = 0x0000f001;
> -	msr.lo = GPIO_IO_BASE;
> -	wrmsr(MDD_LBAR_GPIO, msr);
> -
> -	/* Setup LBAR for MFGPT. */
> -	msr.hi = 0x0000f001;
> -	msr.lo = MFGPT_IO_BASE;
> -	wrmsr(MDD_LBAR_MFGPT, msr);
> -
> -	/* Setup LBAR for ACPI. */
> -	msr.hi = 0x0000f001;
> -	msr.lo = ACPI_IO_BASE;
> -	wrmsr(MDD_LBAR_ACPI, msr);
> -
> -	/* Setup LBAR for PM support. */
> -	msr.hi = 0x0000f001;
> -	msr.lo = PMS_IO_BASE;
> -	wrmsr(MDD_LBAR_PMS, msr);
> +	for (i = 0; i < ARRAY_SIZE(msr_table); i++)
> +		wrmsr(msr_table[i].msrnum, msr_table[i].msr);
>  }
>  
>  /**
> Index: southbridge/amd/cs5536/cs5536.c
> ===================================================================
> --- southbridge/amd/cs5536/cs5536.c	(Revision 460)
> +++ southbridge/amd/cs5536/cs5536.c	(Arbeitskopie)
> @@ -29,13 +29,8 @@
>  #include <statictree.h>
>  #include "cs5536.h"
>  
> -struct msrinit {
> -	u32 msrnum;
> -	struct msr msr;
> -};
> -
>  /* Master configuration register for bus masters */
> -static struct msrinit SB_MASTER_CONF_TABLE[] = {
> +static const struct msrinit SB_MASTER_CONF_TABLE[] = {
>  	{USB2_SB_GLD_MSR_CONF, {.hi = 0,.lo = 0x00008f000}},
>  	{ATA_SB_GLD_MSR_CONF,  {.hi = 0,.lo = 0x00048f000}},
>  	{AC97_SB_GLD_MSR_CONF, {.hi = 0,.lo = 0x00008f000}},
> @@ -44,7 +39,7 @@
>  };
>  
>  /* CS5536 clock gating */
> -static struct msrinit CS5536_CLOCK_GATING_TABLE[] = {
> +static const struct msrinit CS5536_CLOCK_GATING_TABLE[] = {
>  	{GLIU_SB_GLD_MSR_PM,  {.hi = 0,.lo = 0x000000004}},
>  	{GLPCI_SB_GLD_MSR_PM, {.hi = 0,.lo = 0x000000005}},
>  	{GLCP_SB_GLD_MSR_PM,  {.hi = 0,.lo = 0x000000004}},
> @@ -99,32 +94,18 @@
>   */
>  static void pm_chipset_init(void)
>  {
> -	u32 val = 0;
> -	u16 port;
> +	outl(0x0E00, PMS_IO_BASE + 0x010);	/* 1ms */
>  
> -	port = (PMS_IO_BASE + 0x010);
> -	val = 0x0E00;		/*  1ms */
> -	outl(val, port);
> -
> -	/* PM_WKXD */
>  	/* Make sure bits[3:0]=0000b to clear the saved Sx state. */
> -	port = (PMS_IO_BASE + PM_WKXD);
> -	val = 0x0A0;		/*  5ms */
> -	outl(val, port);
> +	outl(0x00A0, PMS_IO_BASE + PM_WKXD);	/* 5ms */
>  
> -	/* PM_WKD */
> -	port = (PMS_IO_BASE + PM_WKD);
> -	outl(val, port);
> +	outl(0x00A0, PMS_IO_BASE + PM_WKD);
>  
> -	/* PM_SED */
> -	port = (PMS_IO_BASE + PM_SED);
> -	val = 0x04601;		/*  5ms, # of 3.57954MHz clock edges */
> -	outl(val, port);
> +	/* 5ms, # of 3.57954MHz clock edges */
> +	outl(0x4601, PMS_IO_BASE + PM_SED);
>  
> -	/* PM_SIDD */
> -	port = (PMS_IO_BASE + PM_SIDD);
> -	val = 0x08C02;		/*  10ms, # of 3.57954MHz clock edges */
> -	outl(val, port);
> +	/* 10ms, # of 3.57954MHz clock edges */
> +	outl(0x8C02, PMS_IO_BASE + PM_SIDD);
>  }
>  
>  /**
> @@ -136,7 +117,6 @@
>  {
>  	int i;
>  	struct msr msr;
> -	int numEnabled = 0;
>  
>  	printk(BIOS_DEBUG, "chipset_flash_setup: Start\n");
>  	for (i = 0; i < ARRAY_SIZE(FlashInitTable); i++) {
> @@ -164,9 +144,6 @@
>  			printk(BIOS_DEBUG, "MSR(0x%08X, %08X_%08X)\n",
>  			       MDD_NORF_CNTRL, msr.hi, msr.lo);
>  			wrmsr(MDD_NORF_CNTRL, msr);
> -
> -			/* Update the number enabled. */
> -			numEnabled++;
>  		}
>  	}
>  	printk(BIOS_DEBUG, "chipset_flash_setup: Finish\n");
> @@ -253,6 +230,7 @@
>  
>  	dev = dev_find_device(PCI_VENDOR_ID_AMD,
>  			      PCI_DEVICE_ID_AMD_CS5536_ISA, 0);
> +
>  	gpio_addr = pci_read_config32(dev, PCI_BASE_ADDRESS_1);
>  	gpio_addr &= ~1;	/* Clear I/O bit */
>  	printk(BIOS_DEBUG, "GPIO_ADDR: %08X\n", gpio_addr);
> @@ -440,16 +418,14 @@
>  		*(bar + UOCMUX) &= PUEN_SET;
>  
>  		/* Host or Device? */
> -		if (sb->enable_USBP4_device) {
> +		if (sb->enable_USBP4_device)
>  			*(bar + UOCMUX) |= PMUX_DEVICE;
> -		} else {
> +		else
>  			*(bar + UOCMUX) |= PMUX_HOST;
> -		}
>  
>  		/* Overcurrent configuration */
> -		if (sb->enable_USBP4_overcurrent) {
> +		if (sb->enable_USBP4_overcurrent)
>  			*(bar + UOCCAP) |= sb->enable_USBP4_overcurrent;
> -		}
>  	}
>  
>  	/* PBz#6466: If the UOC(OTG) device, port 4, is configured as a
> @@ -478,15 +454,13 @@
>  	/* Disable virtual PCI UDC and OTG headers. */
>  	dev = dev_find_device(PCI_VENDOR_ID_AMD,
>  			      PCI_DEVICE_ID_AMD_CS5536_UDC, 0);
> -	if (dev) {
> +	if (dev)
>  		pci_write_config32(dev, 0x7C, 0xDEADBEEF);
> -	}
>  
>  	dev = dev_find_device(PCI_VENDOR_ID_AMD,
>  			      PCI_DEVICE_ID_AMD_CS5536_OTG, 0);
> -	if (dev) {
> +	if (dev)
>  		pci_write_config32(dev, 0x7C, 0xDEADBEEF);
> -	}
>  }
>  
>  /** 
> @@ -502,9 +476,8 @@
>  {
>  	struct device *dev;
>  	struct msr msr;
> -	u32 msrnum;
>  	struct southbridge_amd_cs5536_config *sb;
> -	struct msrinit *csi;
> +	const struct msrinit *csi;
>  
>  	post_code(P80_CHIPSET_INIT);
>  	dev = dev_find_device(PCI_VENDOR_ID_AMD,
> @@ -520,7 +493,7 @@
>  	if (!IsS3Resume())
>  	{
>  		struct acpi_init *aci = acpi_init_table;
> -		for (; aci->ioreg; aci++) {
> +		for (/* Nothing */; aci->ioreg; aci++) {
>  			outl(aci->regdata, aci->ioreg);
>  			inl(aci->ioreg);
>  		}
> @@ -535,22 +508,20 @@
>  	/* Allow I/O reads and writes during a ATA DMA operation. This could
>  	 * be done in the HD ROM but do it here for easier debugging.
>  	 */
> -	msrnum = ATA_SB_GLD_MSR_ERR;
> -	msr = rdmsr(msrnum);
> +	msr = rdmsr(ATA_SB_GLD_MSR_ERR);
>  	msr.lo &= ~0x100;
> -	wrmsr(msrnum, msr);
> +	wrmsr(ATA_SB_GLD_MSR_ERR, msr);
>  
>  	/* Enable post primary IDE. */
> -	msrnum = GLPCI_SB_CTRL;
> -	msr = rdmsr(msrnum);
> +	msr = rdmsr(GLPCI_SB_CTRL);
>  	msr.lo |= GLPCI_CRTL_PPIDE_SET;
> -	wrmsr(msrnum, msr);
> +	wrmsr(GLPCI_SB_CTRL, msr);
>  
>  	csi = SB_MASTER_CONF_TABLE;
>  	for (/* Nothing */; csi->msrnum; csi++) {
>  		msr.lo = csi->msr.lo;
> 
>  		msr.hi = csi->msr.hi;
> -		wrmsr(csi->msrnum, msr);	/* MSR - see table above */
> +		wrmsr(csi->msrnum, msr);
>  	}
>  
>  	/* Flash BAR size setup. */
> @@ -566,7 +537,7 @@
>  		for (/* Nothing */; csi->msrnum; csi++) {
>  			msr.lo = csi->msr.lo;
>  			msr.hi = csi->msr.hi;
> -			wrmsr(csi->msrnum, msr); /* MSR - see table above */
> +			wrmsr(csi->msrnum, msr);
>  		}
>  	}
>  }
> @@ -601,9 +572,8 @@
>  
>  	printk(BIOS_ERR, "cs5536: %s: enable_ide_nand_flash is %d\n",
>  	       __FUNCTION__, sb->enable_ide_nand_flash);
> -	if (sb->enable_ide_nand_flash == 1) {
> +	if (sb->enable_ide_nand_flash == 1)
>  		enable_ide_nand_flash_header();
> -	}
>  
>  	enable_USB_port4(sb);
>  
> @@ -633,8 +603,7 @@
>   */
>  static void cs5536_pci_dev_enable_resources(struct device *dev)
>  {
> -	/* TODO: Shouldn't this be BIOS_SPEW? */
> -	printk(BIOS_ERR, "cs5536: %s()\n", __FUNCTION__);
> +	printk(BIOS_SPEW, "cs5536: %s()\n", __FUNCTION__);
>  	pci_dev_enable_resources(dev);
>  	enable_childrens_resources(dev);
>  }
> Index: include/arch/x86/msr.h
> ===================================================================
> --- include/arch/x86/msr.h	(Revision 460)
> +++ include/arch/x86/msr.h	(Arbeitskopie)
> @@ -21,12 +21,16 @@
>  #define CPU_X86_MSR_H
>  
>  /* standard MSR operations, everyone has written these one hundred times */
> -struct msr 
> -{
> +struct msr {
>  	u32 lo;
>  	u32 hi;
>  };
>  
> +struct msrinit {
> +        u32 msrnum;
> +        struct msr msr;
> +};
> +
>  static inline struct msr rdmsr(u32 index)
>  {
>  	struct msr  result;
> Index: northbridge/amd/geodelx/raminit.c
> ===================================================================
> --- northbridge/amd/geodelx/raminit.c	(Revision 460)
> +++ northbridge/amd/geodelx/raminit.c	(Arbeitskopie)
> @@ -80,14 +80,17 @@
>  
>  	/* Size = Module Density * Module Banks */
>  	dimm_size = smbus_read_byte(dimm, SPD_BANK_DENSITY);
> +
>  	/* Align so 1 GB (bit 0) is bit 8. This is a little weird to get gcc
>  	 * to not optimize this out.
>  	 */
>  	dimm_size |= (dimm_size << 8);
> +
>  	/* And off 2 GB DIMM size: not supported and the 1 GB size we just
>  	 * moved up to bit 8 as well as all the extra on top.
>  	 */
>  	dimm_size &= 0x01FC;
> +
>  	/* Module Density * Module Banks */
>  	/* Shift to multiply by the number of DIMM banks. */
>  	dimm_size <<= (dimm_setting >> CF07_UPPER_D0_MB_SHIFT) & 1;
> @@ -130,11 +133,11 @@
>  		post_code(ERROR_SET_PAGE);
>  		hlt();
>  	}
> +
>  	spd_byte -= 7;
>  	/* If the value is above 6 it means >12 address lines... */
> -	if (spd_byte > 5) {
> +	if (spd_byte > 5)
>  		spd_byte = 7;	/* ...which means >32k so set to disabled. */
> -	}
>  	/* 0 = 1k, 1 = 2k, 2 = 4k, etc. */
>  	dimm_setting |= spd_byte << CF07_UPPER_D0_PSZ_SHIFT;
>  
> @@ -181,9 +184,8 @@
>  #endif
>  
>  	/* Use the slowest DIMM. */
> -	if (spd_byte0 < spd_byte1) {
> +	if (spd_byte0 < spd_byte1)
>  		spd_byte0 = spd_byte1;
> -	}
>  
>  	/* Turn SPD ns time into MHz. Check what the asm does to this math. */
>  	speed = 2 * ((10000 / (((spd_byte0 >> 4) * 10) + (spd_byte0 & 0x0F))));
> @@ -214,22 +216,19 @@
>  
>  	spd_byte0 = smbus_read_byte(dimm0, SPD_REFRESH);
>  	spd_byte0 &= 0xF;
> -	if (spd_byte0 > 5) {
> +	if (spd_byte0 > 5)
>  		spd_byte0 = 5;
> -	}
>  	rate0 = REFRESH_RATE[spd_byte0];
>  
>  	spd_byte1 = smbus_read_byte(dimm1, SPD_REFRESH);
>  	spd_byte1 &= 0xF;
> -	if (spd_byte1 > 5) {
> +	if (spd_byte1 > 5)
>  		spd_byte1 = 5;
> -	}
>  	rate1 = REFRESH_RATE[spd_byte1];
>  
>  	/* Use the faster rate (lowest number). */
> -	if (rate0 > rate1) {
> +	if (rate0 > rate1)
>  		rate0 = rate1;
> -	}
>  
>  	msr = rdmsr(MC_CF07_DATA);
>  	msr.lo |= ((rate0 * (geode_link_speed() / 2)) / 16)
> @@ -488,14 +487,10 @@
>  	spd_byte1 &= spd_byte0;
>  
>  	msr = rdmsr(MC_CF07_DATA);
> -	if (spd_byte1 & 1) {
> -		/* Drive Strength Control */
> -		msr.lo |= CF07_LOWER_EMR_DRV_SET;
> -	}
> -	if (spd_byte1 & 2) {
> -		/* FET Control */
> -		msr.lo |= CF07_LOWER_EMR_QFC_SET;
> -	}
> +	if (spd_byte1 & 1)
> +		msr.lo |= CF07_LOWER_EMR_DRV_SET; /* Drive Strength Control */
> +	if (spd_byte1 & 2)
> +		msr.lo |= CF07_LOWER_EMR_QFC_SET; /* FET Control */
>  	wrmsr(MC_CF07_DATA, msr);
>  }
>  
> @@ -508,15 +503,13 @@
>  
>  	msr = rdmsr(GLCP_DELAY_CONTROLS);
>  	msr.hi &= ~(7 << 20);	/* Clear bits 54:52. */
> -	if (geode_link_speed() < 200) {
> +	if (geode_link_speed() < 200)
>  		msr.hi |= 2 << 20;
> -	}
>  	wrmsr(GLCP_DELAY_CONTROLS, msr);
>  
>  	msr = rdmsr(MC_CFCLK_DBUG);
> -	msr.hi |=
> -	    CFCLK_UPPER_MTST_B2B_DIS_SET | CFCLK_UPPER_MTEST_EN_SET |
> -	    CFCLK_UPPER_MTST_RBEX_EN_SET;
> +	msr.hi |= CFCLK_UPPER_MTST_B2B_DIS_SET | CFCLK_UPPER_MTEST_EN_SET |
> +		  CFCLK_UPPER_MTST_RBEX_EN_SET;
>  	msr.lo |= CFCLK_LOWER_TRISTATE_DIS_SET;
>  	wrmsr(MC_CFCLK_DBUG, msr);
>  
> @@ -530,32 +523,27 @@
>  void sdram_set_registers(void)
>  {
>  	struct msr msr;
> -	u32 msrnum;
>  
>  	/* Set Timing Control */
> -	msrnum = MC_CF1017_DATA;
> -	msr = rdmsr(msrnum);
> +	msr = rdmsr(MC_CF1017_DATA);
>  	msr.lo &= ~(7 << CF1017_LOWER_RD_TMG_CTL_SHIFT);
> -	if (geode_link_speed() < 334) {
> +	if (geode_link_speed() < 334)
>  		msr.lo |= (3 << CF1017_LOWER_RD_TMG_CTL_SHIFT);
> -	} else {
> +	else
>  		msr.lo |= (4 << CF1017_LOWER_RD_TMG_CTL_SHIFT);
> -	}
> -	wrmsr(msrnum, msr);
> +	wrmsr(MC_CF1017_DATA, msr);
>  
>  	/* Set Refresh Staggering */
> -	msrnum = MC_CF07_DATA;
> -	msr = rdmsr(msrnum);
> +	msr = rdmsr(MC_CF07_DATA);
>  	msr.lo &= ~0xF0;
>  	msr.lo |= 0x40;		/* Set refresh to 4 SDRAM clocks. */
> -	wrmsr(msrnum, msr);
> +	wrmsr(MC_CF07_DATA, msr);
>  
>  	/* Memory Interleave: Set HOI here otherwise default is LOI. */
>  #if 0
> -	msrnum = MC_CF8F_DATA;
> -	msr = rdmsr(msrnum);
> +	msr = rdmsr(MC_CF8F_DATA);
>  	msr.hi |= CF8F_UPPER_HOI_LOI_SET;
> -	wrmsr(msrnum, msr);
> +	wrmsr(MC_CF8F_DATA, msr);
>  #endif
>  }
>  
> @@ -636,7 +624,7 @@
>   */
>  void sdram_enable(u8 dimm0, u8 dimm1)
>  {
> -	u32 i, msrnum;
> +	u32 i;
>  	struct msr msr;
>  
>  	post_code(POST_MEM_ENABLE);
> @@ -658,38 +646,33 @@
>  	}
>  
>  	/* Set CKEs. */
> -	msrnum = MC_CFCLK_DBUG;
> -	msr = rdmsr(msrnum);
> +	msr = rdmsr(MC_CFCLK_DBUG);
>  	msr.lo &= ~(CFCLK_LOWER_MASK_CKE_SET0 | CFCLK_LOWER_MASK_CKE_SET1);
> -	wrmsr(msrnum, msr);
> +	wrmsr(MC_CFCLK_DBUG, msr);
>  
>  	/* Force Precharge All on next command, EMRS. */
> -	msrnum = MC_CFCLK_DBUG;
> -	msr = rdmsr(msrnum);
> +	msr = rdmsr(MC_CFCLK_DBUG);
>  	msr.lo |= CFCLK_LOWER_FORCE_PRE_SET;
> -	wrmsr(msrnum, msr);
> +	wrmsr(MC_CFCLK_DBUG, msr);
>  
>  	/* EMRS to enable DLL (pre-setup done in setExtendedModeRegisters). */
> -	msrnum = MC_CF07_DATA;
> -	msr = rdmsr(msrnum);
> +	msr = rdmsr(MC_CF07_DATA);
>  	msr.lo |= CF07_LOWER_PROG_DRAM_SET | CF07_LOWER_LOAD_MODE_DDR_SET;
> -	wrmsr(msrnum, msr);
> +	wrmsr(MC_CF07_DATA, msr);
>  	msr.lo &= ~(CF07_LOWER_PROG_DRAM_SET | CF07_LOWER_LOAD_MODE_DDR_SET);
> -	wrmsr(msrnum, msr);
> +	wrmsr(MC_CF07_DATA, msr);
>  
>  	/* Clear Force Precharge All. */
> -	msrnum = MC_CFCLK_DBUG;
> -	msr = rdmsr(msrnum);
> +	msr = rdmsr(MC_CFCLK_DBUG);
>  	msr.lo &= ~CFCLK_LOWER_FORCE_PRE_SET;
> -	wrmsr(msrnum, msr);
> +	wrmsr(MC_CFCLK_DBUG, msr);
>  
>  	/* MRS Reset DLL - set. */
> -	msrnum = MC_CF07_DATA;
> -	msr = rdmsr(msrnum);
> +	msr = rdmsr(MC_CF07_DATA);
>  	msr.lo |= CF07_LOWER_PROG_DRAM_SET | CF07_LOWER_LOAD_MODE_DLL_RESET;
> -	wrmsr(msrnum, msr);
> +	wrmsr(MC_CF07_DATA, msr);
>  	msr.lo &= ~(CF07_LOWER_PROG_DRAM_SET | CF07_LOWER_LOAD_MODE_DLL_RESET);
> -	wrmsr(msrnum, msr);
> +	wrmsr(MC_CF07_DATA, msr);
>  
>  	/* 2us delay (200 clocks @ 200Mhz). We probably really don't need
>  	 * this but... better safe.
> @@ -701,59 +684,52 @@
>  	while (!(~inb(0x61)));
>  
>  	/* Force Precharge All on the next command, auto-refresh. */
> -	msrnum = MC_CFCLK_DBUG;
> -	msr = rdmsr(msrnum);
> +	msr = rdmsr(MC_CFCLK_DBUG);
>  	msr.lo |= CFCLK_LOWER_FORCE_PRE_SET;
> -	wrmsr(msrnum, msr);
> +	wrmsr(MC_CFCLK_DBUG, msr);
>  
>  	/* Manually AUTO refresh #1. If auto refresh was not enabled above we
>  	 * would need to do 8 refreshes to prime the pump before these 2.
>  	 */
> -	msrnum = MC_CF07_DATA;
> -	msr = rdmsr(msrnum);
> +	msr = rdmsr(MC_CF07_DATA);
>  	msr.lo |= CF07_LOWER_REF_TEST_SET;
> -	wrmsr(msrnum, msr);
> +	wrmsr(MC_CF07_DATA, msr);
>  	msr.lo &= ~CF07_LOWER_REF_TEST_SET;
> -	wrmsr(msrnum, msr);
> +	wrmsr(MC_CF07_DATA, msr);
>  
>  	/* Clear Force Precharge All. */
> -	msrnum = MC_CFCLK_DBUG;
> -	msr = rdmsr(msrnum);
> +	msr = rdmsr(MC_CFCLK_DBUG);
>  	msr.lo &= ~CFCLK_LOWER_FORCE_PRE_SET;
> -	wrmsr(msrnum, msr);
> +	wrmsr(MC_CFCLK_DBUG, msr);
>  
>  	/* Manually AUTO refresh.
>  	 * The MC should insert the right delay between the refreshes.
>  	 */
> -	msrnum = MC_CF07_DATA;
> -	msr = rdmsr(msrnum);
> +	msr = rdmsr(MC_CF07_DATA);
>  	msr.lo |= CF07_LOWER_REF_TEST_SET;
> -	wrmsr(msrnum, msr);
> +	wrmsr(MC_CF07_DATA, msr);
>  	msr.lo &= ~CF07_LOWER_REF_TEST_SET;
> -	wrmsr(msrnum, msr);
> +	wrmsr(MC_CF07_DATA, msr);
>  
>  	/* MRS Reset DLL - clear. */
> -	msrnum = MC_CF07_DATA;
> -	msr = rdmsr(msrnum);
> +	msr = rdmsr(MC_CF07_DATA);
>  	msr.lo |= CF07_LOWER_PROG_DRAM_SET;
> -	wrmsr(msrnum, msr);
> +	wrmsr(MC_CF07_DATA, msr);
>  	msr.lo &= ~CF07_LOWER_PROG_DRAM_SET;
> -	wrmsr(msrnum, msr);
> +	wrmsr(MC_CF07_DATA, msr);
>  
>  	/* Allow MC to tristate during idle cycles with MTEST OFF. */
> -	msrnum = MC_CFCLK_DBUG;
> -	msr = rdmsr(msrnum);
> +	msr = rdmsr(MC_CFCLK_DBUG);
>  	msr.lo &= ~CFCLK_LOWER_TRISTATE_DIS_SET;
> -	wrmsr(msrnum, msr);
> +	wrmsr(MC_CFCLK_DBUG, msr);
>  
>  	/* Disable SDCLK DIMM 1 slot if no DIMM installed (to save power). */
>  	msr = rdmsr(MC_CF07_DATA);
>  	if ((msr.hi & (7 << CF07_UPPER_D1_PSZ_SHIFT)) ==
>  	    (7 << CF07_UPPER_D1_PSZ_SHIFT)) {
> -		msrnum = GLCP_DELAY_CONTROLS;
> -		msr = rdmsr(msrnum);
> +		msr = rdmsr(GLCP_DELAY_CONTROLS);
>  		msr.hi |= (1 << 23);	/* SDCLK bit for 2.0 */
> -		wrmsr(msrnum, msr);
> +		wrmsr(GLCP_DELAY_CONTROLS, msr);
>  	}
>  
>  	/* Set PMode0 Sensitivity Counter. */
> @@ -762,10 +738,9 @@
>  	wrmsr(MC_CF_PMCTR, msr);
>  
>  	/* Set PMode1 Up delay enable. */
> -	msrnum = MC_CF1017_DATA;
> -	msr = rdmsr(msrnum);
> +	msr = rdmsr(MC_CF1017_DATA);
>  	msr.lo |= (209 << 8);	/* bits[15:8] = 209 */
> -	wrmsr(msrnum, msr);
> +	wrmsr(MC_CF1017_DATA, msr);
>  
>  	printk(BIOS_DEBUG, "DRAM controller init done.\n");
>  	post_code(POST_MEM_SETUP_GOOD);
> @@ -786,8 +761,7 @@
>  	 * Check for failed DLL settings now that we have done a
>  	 * memory write.
>  	 */
> -	msrnum = GLCP_DELAY_CONTROLS;
> -	msr = rdmsr(msrnum);
> +	msr = rdmsr(GLCP_DELAY_CONTROLS);
>  	if ((msr.lo & 0x7FF) == 0x104) {
>  		/* If you had it you would need to clear out the fail boot
>  		 * count flag (depending on where it counts from etc).
> @@ -799,10 +773,9 @@
>  		 */
>  
>  		/* Reset the system. */
> -		msrnum = MDD_SOFT_RESET;
> -		msr = rdmsr(msrnum);
> +		msr = rdmsr(MDD_SOFT_RESET);
>  		msr.lo |= 1;
> -		wrmsr(msrnum, msr);
> +		wrmsr(MDD_SOFT_RESET, msr);
>  	}
>  
>  	printk(BIOS_DEBUG, "RAM DLL lock\n");
> Index: northbridge/amd/geodelx/geodelx.c
> ===================================================================
> --- northbridge/amd/geodelx/geodelx.c	(Revision 460)
> +++ northbridge/amd/geodelx/geodelx.c	(Arbeitskopie)
> @@ -100,10 +100,7 @@
>  #define BRIDGE_IO_MASK (IORESOURCE_IO | IORESOURCE_MEM)
>  
>  /* TODO: Not used!? */
> -static const struct msr_defaults {
> -	u32 msr_no;
> -	struct msr msr;
> -} msr_defaults[] = {
> +static const struct msrinit msr_defaults[] = {
>  	{ 0x1700, {.hi = 0,.lo = IM_QWAIT}},
>  	{ 0x1800, {.hi = DMCF_WRITE_SERIALIZE_REQUEST,
>  		   .lo = DMCF_SERIAL_LOAD_MISSES}},
> @@ -235,6 +232,7 @@
>  {
>  	struct resource *resource, *last;
>  	unsigned int link;
> +	struct bus *bus;
>  	u8 line;
>  
>  	last = &dev->resource[dev->resources];
> @@ -248,7 +246,6 @@
>  	}
>  
>  	for (link = 0; link < dev->links; link++) {
> -		struct bus *bus;
>  		bus = &dev->link[link];
>  		if (bus->children) {
>  			printk(BIOS_DEBUG,
> @@ -402,9 +399,7 @@
>  						unsigned int max)
>  {
>  	printk(BIOS_SPEW, ">> Entering northbridge.c: %s\n", __FUNCTION__);
> -
> -	max = pci_scan_bus(&dev->link[0], PCI_DEVFN(0, 0), 0xff, max);
> -	return max;
> +	return pci_scan_bus(&dev->link[0], PCI_DEVFN(0, 0), 0xff, max);
>  }
>  
>  /**
> Index: northbridge/amd/geodelx/geodelxinit.c
> ===================================================================
> --- northbridge/amd/geodelx/geodelxinit.c	(Revision 460)
> +++ northbridge/amd/geodelx/geodelxinit.c	(Arbeitskopie)
> @@ -25,6 +25,9 @@
>  #include <cpu.h>
>  #include <amd_geodelx.h>
>  
> +/* Function prototypes */
> +extern int sizeram(void);
> +
>  struct gliutable {
>  	unsigned long desc_name;
>  	unsigned short desc_type;
> @@ -80,11 +83,6 @@
>  
>  static struct gliutable *gliutables[] = { gliu0table, gliu1table, 0 };
>  
> -struct msrinit {
> -	unsigned long msrnum;
> -	struct msr msr;
> -};
> -
>  static struct msrinit clock_gating_default[] = {
>  	{GLIU0_GLD_MSR_PM,	{.hi = 0x00,.lo = 0x0005}},
>  	{MC_GLD_MSR_PM,		{.hi = 0x00,.lo = 0x0001}},
> @@ -113,8 +111,6 @@
>  	{0x0FFFFFFFF,			{0x0FFFFFFFF, 0x0FFFFFFFF}},
>  };
>  
> -extern int sizeram(void);
> -
>  /**
>   * Write a GeodeLink MSR.
>   *
> @@ -126,9 +122,8 @@
>  
>  	msr.lo = gl->lo;
>  	msr.hi = gl->hi;
> -	wrmsr(gl->desc_name, msr);	/* MSR - see table above. */
> -	printk(BIOS_SPEW,
> -	       "%s: MSR 0x%08x, val 0x%08x:0x%08x\n",
> +	wrmsr(gl->desc_name, msr);
> +	printk(BIOS_SPEW, "%s: MSR 0x%08x, val 0x%08x:0x%08x\n",
>  	       __FUNCTION__, gl->desc_name, msr.hi, msr.lo);
>  }
>  
> @@ -179,7 +174,7 @@
>  	sizebytes |= 0x100;	/* Start at 1 MB. */
>  	msr.lo = sizebytes;
>  
> -	wrmsr(gl->desc_name, msr);	/* MSR - see table above. */
> +	wrmsr(gl->desc_name, msr);
>  	printk(BIOS_DEBUG, "%s: MSR 0x%08x, val 0x%08x:0x%08x\n", __FUNCTION__,
>  	       gl->desc_name, msr.hi, msr.lo);
>  }
> @@ -210,7 +205,7 @@
>  	msr.lo = SMM_OFFSET << 8;
>  	msr.lo |= ((~(SMM_SIZE * 1024) + 1) >> 12) & 0xfffff;
>  
> -	wrmsr(gl->desc_name, msr);	/* MSR - See table above. */
> +	wrmsr(gl->desc_name, msr);
>  	printk(BIOS_DEBUG, "%s: MSR 0x%08x, val 0x%08x:0x%08x\n", __FUNCTION__,
>  	       gl->desc_name, msr.hi, msr.lo);
>  }
> @@ -232,7 +227,7 @@
>  	msr.lo = (SMM_OFFSET << 8) & 0xFFF00000;
>  	msr.lo |= ((~(SMM_SIZE * 1024) + 1) >> 12) & 0xfffff;
>  
> -	wrmsr(gl->desc_name, msr);	/* MSR - See table above. */
> +	wrmsr(gl->desc_name, msr);
>  	printk(BIOS_DEBUG, "%s: MSR 0x%08x, val 0x%08x:0x%08x\n", __FUNCTION__,
>  	       gl->desc_name, msr.hi, msr.lo);
>  }
> @@ -280,16 +275,16 @@
>  {
>  	struct gliutable *gl = NULL;
>  	struct msr msr;
> -	int i, msrnum, enable_preempt, enable_cpu_override;
> +	int i, enable_preempt, enable_cpu_override;
>  	int nic_grants_control, enable_bus_parking;
> +	unsigned long pah, pal;
>  
>  	/* R0 - GLPCI settings for Conventional Memory space. */
>  	msr.hi = (0x09F000 >> 12) << GLPCI_RC_UPPER_TOP_SHIFT;	/* 640 */
>  	msr.lo = 0;						/* 0 */
>  	msr.lo |= GLPCI_RC_LOWER_EN_SET + GLPCI_RC_LOWER_PF_SET +
>  		  GLPCI_RC_LOWER_WC_SET;
> -	msrnum = GLPCI_RC0;
> -	wrmsr(msrnum, msr);
> +	wrmsr(GLPCI_RC0, msr);
>  
>  	/* R1 - GLPCI settings for SysMem space. */
>  	/* Get systop from GLIU0 SYSTOP Descriptor. */
> @@ -299,10 +294,9 @@
>  			break;
>  		}
>  	}
> +
>  	if (gl) {
> -		unsigned long pah, pal;
> -		msrnum = gl->desc_name;
> -		msr = rdmsr(msrnum);
> +		msr = rdmsr(gl->desc_name);
>  
>  		/* Example: R_SYSMEM value 20:00:00:0f:fb:f0:01:00
>  		 * translates to a base of 0x00100000 and top of 0xffbf0000
> @@ -327,8 +321,7 @@
>  		printk(BIOS_DEBUG,
>  		       "GLPCI R1: system msr.lo 0x%08x msr.hi 0x%08x\n",
>  		       msr.lo, msr.hi);
> -		msrnum = GLPCI_RC1;
> -		wrmsr(msrnum, msr);
> +		wrmsr(GLPCI_RC1, msr);
>  	}
>  
>  	/* R2 - GLPCI settings for SMM space. */
> @@ -338,8 +331,7 @@
>  	msr.lo |= GLPCI_RC_LOWER_EN_SET | GLPCI_RC_LOWER_PF_SET;
>  	printk(BIOS_DEBUG, "GLPCI R2: system msr.lo 0x%08x msr.hi 0x%08x\n",
>  	       msr.lo, msr.hi);
> -	msrnum = GLPCI_RC2;
> -	wrmsr(msrnum, msr);
> +	wrmsr(GLPCI_RC2, msr);
>  
>  	/* This is done elsewhere already, but it does no harm to do
>  	 * it more than once.
> @@ -349,41 +341,25 @@
>  	 */
>  	msr.lo = 0x021212121;	/* Cache disabled and write serialized. */
>  	msr.hi = 0x021212121;	/* Cache disabled and write serialized. */
> +	wrmsr(CPU_RCONF_A0_BF, msr);
> +	wrmsr(CPU_RCONF_C0_DF, msr);
> +	wrmsr(CPU_RCONF_E0_FF, msr);
>  
> -	msrnum = CPU_RCONF_A0_BF;
> -	wrmsr(msrnum, msr);
> -
> -	msrnum = CPU_RCONF_C0_DF;
> -	wrmsr(msrnum, msr);
> -
> -	msrnum = CPU_RCONF_E0_FF;
> -	wrmsr(msrnum, msr);
> -
>  	/* Set Non-Cacheable Read Only for NorthBound Transactions to
>  	 * Memory. The Enable bit is handled in the Shadow setup.
>  	 */
> -	msrnum = GLPCI_A0_BF;
> -	msr.hi = 0x35353535;
>  	msr.lo = 0x35353535;
> -	wrmsr(msrnum, msr);
> -
> -	msrnum = GLPCI_C0_DF;
>  	msr.hi = 0x35353535;
> -	msr.lo = 0x35353535;
> -	wrmsr(msrnum, msr);
> +	wrmsr(GLPCI_A0_BF, msr);
> +	wrmsr(GLPCI_C0_DF, msr);
> +	wrmsr(GLPCI_E0_FF, msr);
>  
> -	msrnum = GLPCI_E0_FF;
> -	msr.hi = 0x35353535;
> -	msr.lo = 0x35353535;
> -	wrmsr(msrnum, msr);
> -
>  	/* Set WSREQ. */
> -	msrnum = CPU_DM_CONFIG0;
> -	msr = rdmsr(msrnum);
> +	msr = rdmsr(CPU_DM_CONFIG0);
>  	msr.hi &= ~(7 << DM_CONFIG0_UPPER_WSREQ_SHIFT);
>  	/* Reduce to 1 for safe mode. */
>  	msr.hi |= 2 << DM_CONFIG0_UPPER_WSREQ_SHIFT;
> -	wrmsr(msrnum, msr);
> +	wrmsr(CPU_DM_CONFIG0, msr);
>  
>  	/* The following settings will not work with a CS5530 southbridge.
>  	 * We are ignoring the CS5530 case for now, and perhaps forever.
> @@ -392,28 +368,22 @@
>  	/* 553x NB Init */
>  
>  	/* Arbiter setup */
> -	enable_preempt =
> -	    GLPCI_ARB_LOWER_PRE0_SET | GLPCI_ARB_LOWER_PRE1_SET |
> -	    GLPCI_ARB_LOWER_PRE2_SET | GLPCI_ARB_LOWER_CPRE_SET;
> +	enable_preempt = GLPCI_ARB_LOWER_PRE0_SET | GLPCI_ARB_LOWER_PRE1_SET |
> +			 GLPCI_ARB_LOWER_PRE2_SET | GLPCI_ARB_LOWER_CPRE_SET;
>  	enable_cpu_override = GLPCI_ARB_LOWER_COV_SET;
>  	enable_bus_parking = GLPCI_ARB_LOWER_PARK_SET;
> -	nic_grants_control =
> -	    (0x4 << GLPCI_ARB_UPPER_R2_SHIFT) | (0x3 <<
> -						 GLPCI_ARB_UPPER_H2_SHIFT);
> +	nic_grants_control = (0x4 << GLPCI_ARB_UPPER_R2_SHIFT) |
> +			     (0x3 << GLPCI_ARB_UPPER_H2_SHIFT);
>  
> -	msrnum = GLPCI_ARB;
> -	msr = rdmsr(msrnum);
> -
> +	msr = rdmsr(GLPCI_ARB);
>  	msr.hi |= nic_grants_control;
>  	msr.lo |= enable_cpu_override | enable_preempt | enable_bus_parking;
> -	wrmsr(msrnum, msr);
> +	wrmsr(GLPCI_ARB, msr);
>  
> -	msrnum = GLPCI_CTRL;
> -	msr = rdmsr(msrnum);
> +	msr = rdmsr(GLPCI_CTRL);
>  	/* Out will be disabled in CPUBUG649 for < 2.0 parts. */
>  	msr.lo |= GLPCI_CTRL_LOWER_ME_SET | GLPCI_CTRL_LOWER_OWC_SET |
> -		  GLPCI_CTRL_LOWER_PCD_SET;
> -	msr.lo |= GLPCI_CTRL_LOWER_LDE_SET;
> +		  GLPCI_CTRL_LOWER_PCD_SET | GLPCI_CTRL_LOWER_LDE_SET;
>  
>  	msr.lo &= ~(0x03 << GLPCI_CTRL_LOWER_IRFC_SHIFT);
>  	msr.lo |= 0x02 << GLPCI_CTRL_LOWER_IRFC_SHIFT;
> @@ -435,24 +405,21 @@
>  
>  	msr.hi &= ~(0x03 << GLPCI_CTRL_UPPER_ILTO_SHIFT);
>  	msr.hi |= 0x00 << GLPCI_CTRL_UPPER_ILTO_SHIFT;
> -	wrmsr(msrnum, msr);
> +	wrmsr(GLPCI_CTRL, msr);
>  
>  	/* Set GLPCI Latency Timer */
> -	msrnum = GLPCI_CTRL;
> -	msr = rdmsr(msrnum);
> +	msr = rdmsr(GLPCI_CTRL);
>  	/* Change once 1.x is gone. */
>  	msr.hi |= 0x1F << GLPCI_CTRL_UPPER_LAT_SHIFT;
> -	wrmsr(msrnum, msr);
> +	wrmsr(GLPCI_CTRL, msr);
>  
>  	/* GLPCI_SPARE */
> -	msrnum = GLPCI_SPARE;
> -	msr = rdmsr(msrnum);
> +	msr = rdmsr(GLPCI_SPARE);
>  	msr.lo &= ~0x7;
> -	msr.lo |=
> -	    GLPCI_SPARE_LOWER_AILTO_SET | GLPCI_SPARE_LOWER_PPD_SET |
> -	    GLPCI_SPARE_LOWER_PPC_SET | GLPCI_SPARE_LOWER_MPC_SET |
> -	    GLPCI_SPARE_LOWER_NSE_SET | GLPCI_SPARE_LOWER_SUPO_SET;
> -	wrmsr(msrnum, msr);
> +	msr.lo |= GLPCI_SPARE_LOWER_AILTO_SET | GLPCI_SPARE_LOWER_PPD_SET |
> +		  GLPCI_SPARE_LOWER_PPC_SET | GLPCI_SPARE_LOWER_MPC_SET |
> +		  GLPCI_SPARE_LOWER_NSE_SET | GLPCI_SPARE_LOWER_SUPO_SET;
> +	wrmsr(GLPCI_SPARE, msr);
>  }
>  
>  /**
> @@ -462,14 +429,12 @@
>  {
>  	struct msr msr;
>  	struct msrinit *gating = clock_gating_default;
> -	int i;
>  
> -	for (i = 0; gating->msrnum != 0xffffffff; i++) {
> +	for (/* Nothing */; gating->msrnum != 0xffffffff; gating++) {
>  		msr = rdmsr(gating->msrnum);
>  		msr.hi |= gating->msr.hi;
>  		msr.lo |= gating->msr.lo;
> -		wrmsr(gating->msrnum, msr);	/* MSR - See table above. */
> -		gating += 1;
> +		wrmsr(gating->msrnum, msr);
>  	}
>  }
>  
> @@ -480,15 +445,13 @@
>  {
>  	struct msr msr;
>  	struct msrinit *prio = geode_link_priority_table;
> -	int i;
>  
> -	for (i = 0; prio->msrnum != 0xffffffff; i++) {
> +	for (/* Nothing */; prio->msrnum != 0xffffffff; prio++) {
>  		msr = rdmsr(prio->msrnum);
>  		msr.hi |= prio->msr.hi;
>  		msr.lo &= ~0xfff;
>  		msr.lo |= prio->msr.lo;
> -		wrmsr(prio->msrnum, msr);	/* MSR - See table above. */
> -		prio += 1;
> +		wrmsr(prio->msrnum, msr);
>  	}
>  }
>  
> @@ -523,6 +486,7 @@
>  	int bit;
>  	u8 shadowByte;
>  	struct msr msr = { 0, 0 };
> +
>  	shadowByte = (u8) (shadowLo >> 16);
>  
>  	/* Load up D000 settings in edx. */
> @@ -614,7 +578,6 @@
>  				msr.hi &= 0xFFFF0000;
>  				msr.hi |=
>  				    ((u32) (shadowSettings >> 32)) & 0x0000FFFF;
> -				/* MSR - See the table above. */
>  				wrmsr(pTable->desc_name, msr);
>  			}
>  		}
> @@ -661,8 +624,8 @@
>   */
>  static void enable_L1_cache(void)
>  {
> -	struct gliutable *gl = NULL;
>  	int i;
> +	struct gliutable *gl = NULL;
>  	struct msr msr;
>  	u8 SysMemCacheProp;
>  
> @@ -678,7 +641,6 @@
>  		while (1);	/* TODO: Should be hlt()? */
>  	}
>  
> -// sysdescfound:
>  	msr = rdmsr(gl->desc_name);
>  
>  	/* 20 bit address - The bottom 12 bits go into bits 20-31 in eax, the
> @@ -806,7 +768,7 @@
>  	if (gl) {
>  		msr = rdmsr(gl->desc_name);
>  		systop = ((msr.hi & 0xFF) << 24) | ((msr.lo & 0xFFF00000) >> 8);
> -		systop += 0x1000;	/* 4K */
> +		systop += 4 * 1024;	/* 4K */
>  	} else {
>  		systop =
>  		    ((sizeram() - CONFIG_VIDEO_MB) * 1024) - SMM_SIZE - 1024;
> @@ -824,7 +786,6 @@
>  void northbridge_init_early(void)
>  {
>  	int i;
> -	struct msr msr;
>  
>  	printk(BIOS_DEBUG, "Enter %s\n", __FUNCTION__);
>  
> Index: arch/x86/geodelx/stage1.c
> ===================================================================
> --- arch/x86/geodelx/stage1.c	(Revision 460)
> +++ arch/x86/geodelx/stage1.c	(Arbeitskopie)
> @@ -23,10 +23,7 @@
>  #include <msr.h>
>  #include <amd_geodelx.h>
>  
> -static const struct msrinit {
> -	u32 msrnum;
> -	struct msr msr;
> -} msr_table[] = {
> +static const struct msrinit msr_table[] = {
>    /* Setup access to cache under 1MB. */
>    {CPU_RCONF_DEFAULT, {.hi = 0x24fffc02,.lo = 0x1000A000}}, // 0x00000-0xA0000
>    {CPU_RCONF_A0_BF,   {.hi = 0x00000000,.lo = 0x00000000}}, // 0xA0000-0xBFFFF
> @@ -43,17 +40,18 @@
>  };
>  
>  /**
> -  * Set up Geode LX registers for sane behaviour.
> -  */
> + * Set up Geode LX registers for sane behaviour.
> + */
>  void geodelx_msr_init(void)
>  {
>  }
>  
>  /**
> -  * Disable Cache As RAM(CAR) after memory is setup.
> -  *  Geode can write back the cache contents to RAM and continue on.
> -  *  Assumes that anything in the cache was under 1MB.
> -  */
> + * Disable Cache As RAM (CAR) after memory is setup.
> + *
> + * Geode can write back the cache contents to RAM and continue on.
> + * Assumes that anything in the cache was under 1MB.
> + */
>  void disable_car(void)
>  {
>  	int i;
> Index: arch/x86/geodelx/geodelx.c
> ===================================================================
> --- arch/x86/geodelx/geodelx.c	(Revision 460)
> +++ arch/x86/geodelx/geodelx.c	(Arbeitskopie)
> @@ -255,6 +255,12 @@
>  		return 33;
>  }
>  
> +static const struct msrinit msr_table[] = {
> +	{CPU_BC_MSS_ARRAY_CTL0, {.hi = 0x00000000, .lo = 0x2814D352}},
> +	{CPU_BC_MSS_ARRAY_CTL1, {.hi = 0x00000000, .lo = 0x1068334D}},
> +	{CPU_BC_MSS_ARRAY_CTL2, {.hi = 0x00000106, .lo = 0x83104104}},
> +};
> +
>  /**
>   * Delay Control Settings table from AMD (MCP 0x4C00000F).
>   */
> @@ -308,44 +314,29 @@
>   */
>  static void set_delay_control(u8 dimm0, u8 dimm1)
>  {
> -	u32 msrnum, glspeed;
> +	u32 glspeed;
>  	u8 spdbyte0, spdbyte1, dimms, i;
>  	struct msr msr;
>  
>  	glspeed = geode_link_speed();
>  
>  	/* Fix delay controls for DM and IM arrays. */
> -	msrnum = CPU_BC_MSS_ARRAY_CTL0;
> -	msr.hi = 0;
> -	msr.lo = 0x2814D352;
> -	wrmsr(msrnum, msr);
> +	for (i = 0; i < ARRAY_SIZE(msr_table); i++)
> +		wrmsr(msr_table[i].msrnum, msr_table[i].msr);
>  
> -	msrnum = CPU_BC_MSS_ARRAY_CTL1;
> -	msr.hi = 0;
> -	msr.lo = 0x1068334D;
> -	wrmsr(msrnum, msr);
> -
> -	msrnum = CPU_BC_MSS_ARRAY_CTL2;
> -	msr.hi = 0x00000106;
> -	msr.lo = 0x83104104;
> -	wrmsr(msrnum, msr);
> -
> -	msrnum = GLCP_FIFOCTL;
> -	msr = rdmsr(msrnum);
> +	msr = rdmsr(GLCP_FIFOCTL);
>  	msr.hi = 0x00000005;
> -	wrmsr(msrnum, msr);
> +	wrmsr(GLCP_FIFOCTL, msr);
>  
>  	/* Enable setting. */
> -	msrnum = CPU_BC_MSS_ARRAY_CTL_ENA;
>  	msr.hi = 0;
>  	msr.lo = 0x00000001;
> -	wrmsr(msrnum, msr);
> +	wrmsr(CPU_BC_MSS_ARRAY_CTL_ENA, msr);
>  
>  	/* Debug Delay Control setup check.
>  	 * Leave it alone if it has been setup. FS2 or something is here.
>  	 */
> -	msrnum = GLCP_DELAY_CONTROLS;
> -	msr = rdmsr(msrnum);
> +	msr = rdmsr(GLCP_DELAY_CONTROLS);
>  	if (msr.lo & ~(DELAY_LOWER_STATUS_MASK))
>  		return;
>  
> @@ -409,85 +400,74 @@
>   */
>  void cpu_reg_init(int debug_clock_disable, u8 dimm0, u8 dimm1)
>  {
> -	int msrnum;
>  	struct msr msr;
>  
>  	/* Castle 2.0 BTM periodic sync period. */
>  	/* [40:37] 1 sync record per 256 bytes. */
> -	msrnum = CPU_PF_CONF;
> -	msr = rdmsr(msrnum);
> +	msr = rdmsr(CPU_PF_CONF);
>  	msr.hi |= (0x8 << 5);
> -	wrmsr(msrnum, msr);
> +	wrmsr(CPU_PF_CONF, msr);
>  
>  	/* Castle performance setting.
>  	 * Enable Quack for fewer re-RAS on the MC.
>  	 */
> -	msrnum = GLIU0_ARB;
> -	msr = rdmsr(msrnum);
> +	msr = rdmsr(GLIU0_ARB);
>  	msr.hi &= ~ARB_UPPER_DACK_EN_SET;
>  	msr.hi |= ARB_UPPER_QUACK_EN_SET;
> -	wrmsr(msrnum, msr);
> +	wrmsr(GLIU0_ARB, msr);
>  
> -	msrnum = GLIU1_ARB;
> -	msr = rdmsr(msrnum);
> +	msr = rdmsr(GLIU1_ARB);
>  	msr.hi &= ~ARB_UPPER_DACK_EN_SET;
>  	msr.hi |= ARB_UPPER_QUACK_EN_SET;
> -	wrmsr(msrnum, msr);
> +	wrmsr(GLIU1_ARB, msr);
>  
>  	/* GLIU port active enable, limit south pole masters (AES and PCI) to
>  	 * one outstanding transaction.
>  	 */
> -	msrnum = GLIU1_PORT_ACTIVE;
> -	msr = rdmsr(msrnum);
> +	msr = rdmsr(GLIU1_PORT_ACTIVE);
>  	msr.lo &= ~0x880;
> -	wrmsr(msrnum, msr);
> +	wrmsr(GLIU1_PORT_ACTIVE, msr);
>  
>  	/* Set the Delay Control in GLCP. */
>  	set_delay_control(dimm0, dimm1);
>  
>  	/* Enable RSDC. */
> -	msrnum = CPU_AC_SMM_CTL;
> -	msr = rdmsr(msrnum);
> +	msr = rdmsr(CPU_AC_SMM_CTL);
>  	msr.lo |= SMM_INST_EN_SET;
> -	wrmsr(msrnum, msr);
> +	wrmsr(CPU_AC_SMM_CTL, msr);
>  
>  	/* FPU imprecise exceptions bit. */
> -	msrnum = CPU_FPU_MSR_MODE;
> -	msr = rdmsr(msrnum);
> +	msr = rdmsr(CPU_FPU_MSR_MODE);
>  	msr.lo |= FPU_IE_SET;
> -	wrmsr(msrnum, msr);
> +	wrmsr(CPU_FPU_MSR_MODE, msr);
>  
>  	/* Power savers (do after BIST). */
>  	/* Enable Suspend on HLT & PAUSE instructions. */
> -	msrnum = CPU_XC_CONFIG;
> -	msr = rdmsr(msrnum);
> +	msr = rdmsr(CPU_XC_CONFIG);
>  	msr.lo |= XC_CONFIG_SUSP_ON_HLT | XC_CONFIG_SUSP_ON_PAUSE;
> -	wrmsr(msrnum, msr);
> +	wrmsr(CPU_XC_CONFIG, msr);
>  
>  	/* Enable SUSP and allow TSC to run in Suspend (keep speed
>  	 * detection happy).
>  	 */
> -	msrnum = CPU_BC_CONF_0;
> -	msr = rdmsr(msrnum);
> +	msr = rdmsr(CPU_BC_CONF_0);
>  	msr.lo |= TSC_SUSP_SET | SUSP_EN_SET;
>  	msr.lo &= 0x0F0FFFFFF;
>  	msr.lo |= 0x002000000;	/* PBZ213: Set PAUSEDLY = 2. */
> -	wrmsr(msrnum, msr);
> +	wrmsr(CPU_BC_CONF_0, msr);
>  
>  	/* Disable the debug clock to save power. */
>  	/* Note: Leave it enabled for FS2 debug. */
>  	if (debug_clock_disable && 0) {
> -		msrnum = GLCP_DBGCLKCTL;
>  		msr.hi = 0;
>  		msr.lo = 0;
> -		wrmsr(msrnum, msr);
> +		wrmsr(GLCP_DBGCLKCTL, msr);
>  	}
>  
>  	/* Setup throttling delays to proper mode if it is ever enabled. */
> -	msrnum = GLCP_TH_OD;
>  	msr.hi = 0;
>  	msr.lo = 0x00000603C;
> -	wrmsr(msrnum, msr);
> +	wrmsr(GLCP_TH_OD, msr);
>  
>  	/* Fix CPU bugs. */
>  #warning testing fixing bugs in initram
> 

-- 
Marc Jones
Senior Firmware Engineer
(970) 226-9684 Office
mailto:Marc.Jones at amd.com
http://www.amd.com/embeddedprocessors






More information about the coreboot mailing list