[coreboot] r636 - in coreboot-v3: include mainboard/artecgroup/dbe61 mainboard/artecgroup/dbe62 mainboard/pcengines/alix1c northbridge/amd/geodelx southbridge/amd/cs5536

[Uwe Hermann]

On Fri, Mar 07, 2008 at 12:33:59AM +0100, svn at coreboot.org wrote:
> Modified: coreboot-v3/mainboard/artecgroup/dbe61/initram.c
> ===================================================================
> --- coreboot-v3/mainboard/artecgroup/dbe61/initram.c	2008-03-06 23:18:13 UTC (rev 635)
> +++ coreboot-v3/mainboard/artecgroup/dbe61/initram.c	2008-03-06 23:33:59 UTC (rev 636)
> @@ -97,13 +97,17 @@
>  	/* returns 0xFF on any failures */
>  	u8 ret = 0xff;
>  
> -	printk(BIOS_DEBUG, "spd_read_byte dev %04x\n", device);
> +	printk(BIOS_DEBUG, "spd_read_byte dev %04x", device);
>  	if (device == DIMM0) {
>  		for (i = 0; i < ARRAY_SIZE(spd_table); i++) {
>  			if (spd_table[i].address == address) {
>  				ret = spd_table[i].data;

No break here?


>  			}
>  		}
> +
> +		if (i == ARRAY_SIZE(spd_table))
> +			printk(BIOS_DEBUG, " addr %02x does not exist in SPD table",
> +				address);
>  	}
>  
>  	printk(BIOS_DEBUG, " addr %02x returns %02x\n", address, ret);
> 


> Modified: coreboot-v3/northbridge/amd/geodelx/raminit.c
> ===================================================================
> --- coreboot-v3/northbridge/amd/geodelx/raminit.c	2008-03-06 23:18:13 UTC (rev 635)
> +++ coreboot-v3/northbridge/amd/geodelx/raminit.c	2008-03-06 23:33:59 UTC (rev 636)
> @@ -130,6 +130,7 @@
>  	/* Module Density * Module Banks */
>  	/* Shift to multiply by the number of DIMM banks. */
>  	dimm_size <<= (dimm_setting >> CF07_UPPER_D0_MB_SHIFT) & 1;
> +	printk(BIOS_DEBUG, "DIMM size is %x\n", dimm_size);
>  	banner(BIOS_DEBUG, "BEFORT CTZ");
>  	dimm_size = __builtin_ctz(dimm_size);
>  	banner(BIOS_DEBUG, "TEST DIMM SIZE>8");
> @@ -183,6 +184,7 @@
>  	banner(BIOS_DEBUG, "RDMSR CF07");
>  	msr = rdmsr(MC_CF07_DATA);
>  	banner(BIOS_DEBUG, "WRMSR CF07");
> +	printk(BIOS_DEBUG, "CF07(%x): %08x.%08x\n", MC_CF07_DATA, msr.hi, msr.lo);
>  	if (dimm == dimm0) {
>  		msr.hi &= 0xFFFF0000;
>  		msr.hi |= dimm_setting;
> @@ -223,6 +225,7 @@
>  	/* Turn SPD ns time into MHz. Check what the asm does to this math. */
>  	speed = 2 * ((10000 / (((spd_byte0 >> 4) * 10) + (spd_byte0 & 0x0F))));
>  
> +	printk(BIOS_DEBUG, "ddr max speed is %d\n", speed);

ddr -> DDR


>  	/* Current speed > max speed? */
>  	if (geode_link_speed() > speed) {
>  		printk(BIOS_EMERG, "DIMM overclocked. Check GeodeLink speed\n");
> @@ -266,6 +269,7 @@
>  	msr = rdmsr(MC_CF07_DATA);
>  	msr.lo |= ((rate0 * (geode_link_speed() / 2)) / 16)
>  		   << CF07_LOWER_REF_INT_SHIFT;
> +	printk(BIOS_DEBUG, "Refresh rate set to %x\n", rate0);
>  	wrmsr(MC_CF07_DATA, msr);
>  }
>  
> @@ -385,6 +389,7 @@
>  		hlt();
>  	}
>  
> +	printk(BIOS_DEBUG, "Set cas latency to %x\n", spd_byte);

cas -> CAS


> Modified: coreboot-v3/southbridge/amd/cs5536/cs5536.h
> ===================================================================
> --- coreboot-v3/southbridge/amd/cs5536/cs5536.h	2008-03-06 23:18:13 UTC (rev 635)
> +++ coreboot-v3/southbridge/amd/cs5536/cs5536.h	2008-03-06 23:33:59 UTC (rev 636)
> @@ -444,6 +444,7 @@
>  /* Function prototypes */
>  void cs5536_disable_internal_uart(void);
>  void cs5536_setup_onchipuart(void);
> +void cs5536_setup_onchipuart2(void);

Is there some more descriptive suffix than "2" for this function?


>  void cs5536_stage1(void);
>  
>  #endif /* SOUTHBRIDGE_AMD_CS5536_CS5536_H */
> 
> @@ -239,6 +222,40 @@
>  	wrmsr(MDD_UART1_CONF, msr);
>  }
>  
> +void cs5536_setup_onchipuart2(void)

Please add a (doxygen-)comment for this function, which should also
mention what is different to cs5536_setup_onchipuart() and why we
need two such functions.


> +{
> +	struct msr msr;
> +
> +	/* GPIO4 - UART2_TX */
> +	/* Set: Output Enable  (0x4) */
> +	outl(GPIOL_4_SET, GPIO_IO_BASE + GPIOL_OUTPUT_ENABLE);
> +	/* Set: OUTAUX1 Select (0x10) */
> +	outl(GPIOL_4_SET, GPIO_IO_BASE + GPIOL_OUT_AUX1_SELECT);
> +	/* GPIO4 - UART2_RX */
> +	/* Set: Input Enable   (0x20) */
> +	outl(GPIOL_3_SET, GPIO_IO_BASE + GPIOL_INPUT_ENABLE);
> +	/* Set: INAUX1 Select  (0x34) */

The (0x4), (0x10), (0x20), (0x34) etc. should be dropped, that's what the
defines are used for. We don't _care_ about the actual numbers here.


Uwe.
-- 
http://www.hermann-uwe.de  | http://www.holsham-traders.de
http://www.crazy-hacks.org | http://www.unmaintained-free-software.org