[LinuxBIOS] new Target ASUS A8NE

Philipp Degler pdegler at rumms.uni-mannheim.de
Sat May 19 17:55:29 CEST 2007


On Friday 18 May 2007 23:29:45 Uwe Hermann wrote:
> 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?
To be honest I don't know. Maybe yinghai can comment on this.

>
> > - 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'.
i will rename the target, test it on hardware and generate three new patches. 

>
> > @@ -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).
hm, didn't know that.

>
> Do you work at the university? Did you sign some contract which says
> something about copyright of code you write?
I'am a student of the university of mannheim. I was able to use their hardware 
for the port as far as I don't own an ASUS A8N-E myself, not yet :).

>
> > +			/* 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.
yes they are a remainder of s2891 :). i will 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?)
no, code did not 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.
i will check / test it - shouldn't be a big issue to drop it.

>
>
> Otherwise your patch looks really great! With the above comments
> addressed I think we can commit this.
that would be great :)

phil
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 481 bytes
Desc: not available
URL: <http://www.coreboot.org/pipermail/coreboot/attachments/20070519/0b76e818/attachment.sig>


More information about the coreboot mailing list