[LinuxBIOS] new Target ASUS A8NE

[Uwe Hermann]

Hi,

On Fri, May 18, 2007 at 04:39:38PM +0200, Philipp Degler wrote:
> - RAM olny runs with small modification of amdk8/raminit.c !!!!!!!!
> 	=> comment out debug for athlon 64 in line 1204
> 	-----------------------------------------------------------------------------------------------------------------
> 	-#if 1
> 	+#if 0
>         	//By yhlu for debug Athlon64 939 can do dual channel, but it .....
>         	if (unbuffered && is_opteron(ctrl)) {
>                 	die("Unbuffered Dimms not supported on Opteron");
> 	-----------------------------------------------------------------------------------------------------------------

Hm, why is that? Would this change break other targets?


> - ps2 keyboard and mouse are not working yet (use usb counterparts instead)
> 	=> seems to be an interrupt issue?? maybe someone with ck804 datasheets or 
> 		board schematics able to help out here ?

The IT8712F code is incomplete, that might be part of the problem.


> The patch also targets pc80/keyboard.c. I added an error message in case of a 
> selftest failure.

Please submit an extra patch for that.

 
> The superio ite it8712f implementation was modified too but as far as these 
> changes did not really solve my interrupt issue for keyboard and mouse i 
> would provide these changes in a separate patch if it is of any interest.  

Yes, please send an extra patch.

 
> Index: src/mainboard/asus/a8ne/Config.lb
> ===================================================================
> --- src/mainboard/asus/a8ne/Config.lb	(revision 0)
> +++ src/mainboard/asus/a8ne/Config.lb	(revision 0)

Can we move this to src/mainboard/asus/a8n_e/? We should stay as close
to the original vendor name as possible, i.e. A8N-E (not A8NE) as string
in mainboard.c. For directory names or file names this means 'a8n_e'.


> @@ -0,0 +1,360 @@
> +##
> +## This file is part of the LinuxBIOS project.
> +##
> +## Copyright (C) 2007 AMD 
> +## Written by Yinghai Lu <yinghailu at amd.com> for AMD.
> +##
> +## Copyright (C) 2007 University of Mannheim
> +## Written by Philipp Degler <pdegler at rumms.uni-mannheim.de> for Uni Ma.

Just curious -- are you sure the university owns the copyright? AFAIK
this is not possible in Germany, you as the author always retain the
copyright (but you can give away _usage_ rights).

Do you work at the university? Did you sign some contract which says
something about copyright of code you write?


> +			/* Initialize interrupt mapping */
> +			//dword = 0x0000d218;
> +			dword = 0x01200000;	//a8ne
> +			pci_write_config32(dev, 0x7c, dword);
> +
> +			//dword = 0x12008a00;
> +			dword = 0x12008009;	//a8ne
> +			pci_write_config32(dev, 0x80, dword);
> +
> +			//dword = 0x0000007d;
> +			dword = 0x0002010d;	//a8ne
> +			pci_write_config32(dev, 0x84, dword);

What are the commented values? Not for the A8N-E? Then we should probably
drop them.


> +static void memreset_setup(void)
> +{
> +	/*FIXME: nothing to do?? */
> +}
> +
> +static void memreset(int controllers, const struct mem_controller *ctrl)
> +{
> +	/*FIXME: nothing to do?? */
> +}
> +
> +static inline void activate_spd_rom(const struct mem_controller *ctrl)
> +{
> +	/*FIXME: nothing to do?? */
> +}

I think we can drop these functions for now. If they're needed, we'll
re-add them with some content... (does the code still build without them?)


> +	option LINUXBIOS_EXTRA_VERSION="$(shell cat ../../VERSION)_Normal"
[...]
> Index: targets/asus/a8ne/VERSION
> ===================================================================
> --- targets/asus/a8ne/VERSION	(revision 0)
> +++ targets/asus/a8ne/VERSION	(revision 0)
> @@ -0,0 +1 @@
> +_a8ne

Is this really needed? Unless there's a good reason for an extra file,
it would be better to put this string into Config.lb directly.


Otherwise your patch looks really great! With the above comments
addressed I think we can commit this.


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/20070518/2e9a0715/attachment.sig>