Re: [LinuxBIOS] patch: alix1c support for v3

[Uwe Hermann]

On Sun, Nov 25, 2007 at 12:45:29PM -0800, ron minnich wrote:
> This builds.

Not yet:

HOSTCC  build/util/kconfig/zconf.tab.o
  HOSTCC  build/util/kconfig/mconf
mainboard/Kconfig:61: can't open file "mainboard/pcengines/Kconfig"
make: *** [menuconfig] Error 1

The fix should be easy though. You probably forgot to 'svn add' the
Kconfig file mainboard/pcengines/Kconfig.

Please repost the full patch.
> It also includes some additional posts for geode setup which ought to be useful.

This is unrelated and should probably be in an extra patch.
> Index: mainboard/Kconfig
>
> --- mainboard/Kconfig	(revision 518)
> +++ mainboard/Kconfig	(working copy)
> @@ -47,12 +47,18 @@
>    help
>      Select this option for various system emulators, such as QEMU.
>
> +config VENDOR_PCENGINES
> bool "PC Engines"
> help
>  Select this option for PC Engines systems

Add a full stop to the help text, please.
> endchoice
>
> source "mainboard/adl/Kconfig"
>  source "mainboard/amd/Kconfig"
>  source "mainboard/artecgroup/Kconfig"
>  source "mainboard/emulation/Kconfig"
> +source "mainboard/pcengines/Kconfig"
>
> choice
>    prompt "ROM chip size"
> Index: mainboard/pcengines/alix1c/Kconfig
> ===================================================================
> --- mainboard/pcengines/alix1c/Kconfig	(revision 0)
> +++ mainboard/pcengines/alix1c/Kconfig	(revision 0)
> @@ -0,0 +1,46 @@
> +##
> +## This file is part of the LinuxBIOS project.
> +##
> +## Copyright (C) 2007 coresystems GmbH
> +## (Written by Stefan Reinauer  for coresystems GmbH)

Ron Minnich?
> +##
> +## This program is free software; you can redistribute it and/or modify
> +## it under the terms of the GNU General Public License as published by
> +## the Free Software Foundation; either version 2 of the License, or
> +## (at your option) any later version.
> +##
> +## This program is distributed in the hope that it will be useful,
> +## but WITHOUT ANY WARRANTY; without even the implied warranty of
> +## MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +## GNU General Public License for more details.
> +##
> +## You should have received a copy of the GNU General Public License
> +## along with this program; if not, write to the Free Software
> +## Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301 USA
> +##
>
> +config MAINBOARD_NAME
> string
> default pcengines/alix1c
> depends BOARD_PCENGINES_ALIX1C
> help
>  This is the default mainboard name.
>
> +config MAINBOARD_PCI_SUBSYSTEM_VENDOR_ID
> hex
> # TODO: Fix PCI ID.
> # PC Engines has no PCI vendor ID. Now what?

Good point. I think we need to improve this in LinuxBIOS anyway.
In v2 we currently have only one such ID in Options.lb which is then
used for _all_ devices in the system (if it's used at all).

The correct thing would be to allow an override of the ID for each
PCI device, per-board (via entries in the dts, IMHO).
> default 0x1022
> depends BOARD_AMD_NORWICH
> help
>  Mainboard specific PCI subsystem vendor ID.
>
> +config MAINBOARD_PCI_SUBSYSTEM_DEVICE_ID
> hex
> # TODO: Fix PCI ID.
> # PC Engines has no PCI vendor ID. Now what?
> default 0x2323
> depends BOARD_AMD_NORWICH
> help
>  Mainboard specific PCI subsystem device ID.

For now I'd say just drop the entries here, the code checks via #ifdef
if they're defined and does nothing if they're not...
> +int main(void)
> +{
> u8 smb_devices[] =  {
> DIMM0, DIMM1
> };
> post_code(POST_START_OF_MAIN);

POST_START_OF_MAIN, and...
> +void hardware_stage1(void)
> +{
> post_code(POST_START_OF_MAIN);

... POST_START_OF_MAIN again.

Shouldn't those be different?
> +/{
> enabled;
> mainboard-vendor = "PC Engines";
> mainboard-name = "Alix 1C";

ALIX1.C
> Index: mainboard/pcengines/alix1c/cmos.layout
>
> --- mainboard/pcengines/alix1c/cmos.layout	(revision 0)
> +++ mainboard/pcengines/alix1c/cmos.layout	(revision 0)

Add the common license header to this file, please.
> Index: arch/x86/geodelx/stage0.S
>
> --- arch/x86/geodelx/stage0.S	(revision 518)
> +++ arch/x86/geodelx/stage0.S	(working copy)
> @@ -183,6 +183,7 @@
>    movw	%ax, %fs
>    movw	%ax, %gs
> port80_post(0x02)

Please don't hardcode POST codes, add them aѕ #defines in post_code.h
and document them (i.e. in which situation it is supposed to be used).
> @@ -364,6 +379,8 @@
>    outb	%al, $0x80
>
> DCacheSetupBad:
>    movb    $0xFF, %al
>    outb    %al, $0x80

Why this?

Uwe.