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 <[EMAIL PROTECTED]> for AMD.
> +##
> +## Copyright (C) 2007 University of Mannheim
> +## Written by Philipp Degler <[EMAIL PROTECTED]> 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
signature.asc
Description: Digital signature
-- linuxbios mailing list linuxbios@linuxbios.org http://www.linuxbios.org/mailman/listinfo/linuxbios
