[LinuxBIOS] patch: alix1c fake spd

Uwe Hermann uwe at hermann-uwe.de
Fri Oct 19 18:28:17 CEST 2007


On Fri, Oct 19, 2007 at 08:13:20AM -0700, ron minnich wrote:
>  #define POST_CODE(x) outb(x, 0x80)
>  #define SERIAL_DEV PNP_DEV(0x2e, W83627HF_SP1)
>  
> -#include "southbridge/amd/cs5536/cs5536_early_smbus.c"
> +/* no smbus here */

Not on this board on not on CS5536 in general?


> +static u8 spdbytes[] = {
> +	[SPD_ACCEPTABLE_CAS_LATENCIES] = 0x10,
> +	[SPD_BANK_DENSITY] = 0x20,
> +	[SPD_DEVICE_ATTRIBUTES_GENERAL] = 0xff,
> +	[SPD_MEMORY_TYPE] = 7,
> +	[SPD_MIN_CYCLE_TIME_AT_CAS_MAX] = 0xff, /* not used */
> +	[SPD_MODULE_ATTRIBUTES] = 0xff, /* fix me later when we figure out */
> +	[SPD_NUM_BANKS_PER_SDRAM] = 4,
> +	[SPD_NUM_DIMM_BANKS] = 4,
> +	[SPD_NUM_COLUMNS] = 0xa,
> +	[SPD_NUM_ROWS] = 3,
> +	[SPD_REFRESH] = 0x3a,
> +	[SPD_SDRAM_CYCLE_TIME_2ND] = 60,
> +	[SPD_SDRAM_CYCLE_TIME_3RD] = 75,
> +	[SPD_tRAS] = 40,
> +	[SPD_tRCD] = 15,
> +	[SPD_tRFC] = 70,
> +	[SPD_tRP] = 15,
> +	[SPD_tRRD] = 10,
> +};

Yep, nice. I like this approach.


> +
> +static u8 spd_read_byte(unsigned device, unsigned address)
>  {
> -		return smbus_read_byte(device, address);
> +	print_debug("spd_read_byte dev ");print_debug_hex8(device);
> +	if (device != (((0xa << 3) | 1)<<1)){

Can you add a comment here? So this is 0x51 shifted to the left? Why?


> +		print_debug("ENXDEV\r\n");

What is ENXDEV? Can we make the message a bit more readable?


> +		return 0xff;
> +	}
> +
> +	print_debug(" addr ");print_debug_hex8(address);
> +	print_debug(" returns "); print_debug_hex8(spdbytes[address]);

Put them all on extra lines, please. Yes, it's longer, but better
wrt coding-style (IMO).


> +	print_debug("\r\n");
> +	return spdbytes[address];
>  }
>  
>  #define ManualConf 0		/* Do automatic strapped PLL config */
> @@ -83,13 +135,12 @@
>  
>  void cache_as_ram_main(void)
>  {
> +	static const struct mem_controller memctrl[] = {
> +		{.channel0 = {(0xa << 3) | 0, (0xa << 3) | 1}}

I always wondered where this syntax comes from? Why not just

  .channel0 = {0x50, 0x51}

What's the deeper reason for this?


> +	};
>  	extern void RestartCAR();
>  	POST_CODE(0x01);
>  
> -	static const struct mem_controller memctrl [] = {
> -		{.channel0 = {(0xa<<3)|0, (0xa<<3)|1}}
> -	};
> -
>  	SystemPreInit();
>  	msr_init();

Patch looks good apart from a few coding style issues (please
fix those).


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/20071019/a42687f3/attachment.sig>


More information about the coreboot mailing list