Re: [LinuxBIOS] new Target ASUS A8NE

[Philipp Degler]

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  for AMD.
> > +##
> > +## Copyright (C) 2007 University of Mannheim
> > +## Written by Philipp Degler  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