On Tue, Oct 09, 2007 at 07:51:48PM +0200, Juergen Beisert wrote:
this patch adds main support for BCOM's Winnet100 Geode GX1 based terminal. It activates the VGA support so it would be possible to run this target as a small (and silent) X terminal. With faster SDRAM timing (not yet supported in mainline) it can run graphical resolutions up to SXGA (1280x1024) at 64k colours.
I'm not sure if all files I add to src/mainboard/bcom/winnet100 are required. Most of them are copies from other GX1 based mainboards.
Nope, not all are required. And most miss a GPL header which we should add.
Index: src/mainboard/bcom/winnet100/Config.lb
--- src/mainboard/bcom/winnet100/Config.lb (Revision 0) +++ src/mainboard/bcom/winnet100/Config.lb (Revision 0)
Missing GPL header. This is just (relatively small) config file (which should have a common part somewhere global btw, but that's for another patch) so I'm not sure if we have to track down all of this to the first author of this code (which is used in basically _every_ board).
I'd just slap a '(C) Juergen Beisert' on it and that's it. Counter-opinions welcome, but only with a patch to add the original author to all Config.lb files then ;-)
+## +## Romcc output +## +makerule ./failover.E
- depends "$(MAINBOARD)/failover.c ./romcc"
- action "./romcc -E -O --label-prefix=failover -I$(TOP)/src -I. $(CPPFLAGS) $(MAINBOARD)/failover.c -o $@"
+end
+makerule ./failover.inc
- depends "$(MAINBOARD)/failover.c ./romcc"
- action "./romcc -O --label-prefix=failover -I$(TOP)/src -I. $(CPPFLAGS) $(MAINBOARD)/failover.c -o $@"
+end
Please use the global src/arch/i386/lib/failover.c instead of a local duplicate of the file (i.e. drop failover.c).
You'll have to modify this part a bit, see http://tracker.linuxbios.org/trac/LinuxBIOS/changeset/2772
Index: src/mainboard/bcom/winnet100/irq_tables.c
--- src/mainboard/bcom/winnet100/irq_tables.c (Revision 0) +++ src/mainboard/bcom/winnet100/irq_tables.c (Revision 0)
Please add the usual GPL header, see http://linuxbios.org/Development_Guidelines#Common_License_Header
@@ -0,0 +1,116 @@ +/**
- @file
- @brief Interrupt routing description for BCOM's Winnet100 board
You can drop the @brief, we use the JavaDoc-like autobrief config option in Doxygen (in v3 at least, but also here I think).
Not sure if you can/want to drop the @file.
+/**
- Routing desciption.
- Documentation at : http://www.microsoft.com/whdc/archive/pciirq.mspx
- */
+const struct irq_routing_table intel_irq_routing_table = {
- .signature = PIRQ_SIGNATURE, /* u32 signature */
- .version = PIRQ_VERSION, /* u16 version */
- .size = 32+16*IRQ_SLOT_COUNT, /* there can be total 2 devices on the bus */
2 -> IRQ_SLOT_COUNT, otherwise it's confusing
- .rtr_bus = 0x00, /* Where the interrupt router lies (bus) */
- .rtr_devfn = (0x12<<3)|0x0, /* Where the interrupt router lies (dev) */
- .exclusive_irqs = 0x8800, /* IRQs devoted exclusively to PCI usage */
Where does this value come from?
+/**
- copy the IRQ routing table to memory
Start all sentences with capital letter and end them with full stop, please (I'm not kidding ;-))
- @param[in] addr destination address (between 0xF0000...0x100000)
Capital letters and full-stop here, too, please. Looks better in the Doxygen output, IMO.
* * @param[in] addr Destination address for the IRQ routing table * (between 0xF0000...0x100000). *
Missing @return line, btw.
- **/
One '*' too many.
+unsigned long write_pirq_routing_table(unsigned long addr) +{
- return copy_pirq_routing_table(addr);
+}
This should actually be some global function, but in this case that would be really overkill, I guess ;-)
+/* end of file irq_tables.c */
Not needed.
Index: src/mainboard/bcom/winnet100/Options.lb
--- src/mainboard/bcom/winnet100/Options.lb (Revision 0) +++ src/mainboard/bcom/winnet100/Options.lb (Revision 0)
Missing GPL header.
+## +## Enable VGA with a splash screen +## but only 640x480 to run on nearly all monitors +##
+default CONFIG_GX1_VIDEO=1 +default CONFIG_GX1_VIDEOMODE=0 +default CONFIG_SPLASH_GRAPHIC=1
[...]
+## +## We want to support up to 1024x768@16 so we need +## 2MiB video memory. Note: Higher resolutions might +## need faster SDRAM speed. +##
+default CONFIG_VIDEO_MB=2
Please also add these as comments to targets/.../Config.lb, as users are likely to want to modify them.
# option CONFIG_GX1_VIDEO=1 # option CONFIG_GX1_VIDEOMODE=0 # option CONFIG_SPLASH_GRAPHIC=1 # option CONFIG_VIDEO_MB=2
+## +## Build code to export a CMOS option table +## +default HAVE_OPTION_TABLE=0
[...]
+## +## Only use the option table in a normal image +## +default USE_OPTION_TABLE = 0
If you're not using the CMOS option table, you can drop the cmos.layout file.
+### +### LinuxBIOS layout values +###
+## ROM_IMAGE_SIZE is the amount of space to allow linuxBIOS to occupy. +default ROM_IMAGE_SIZE = 65536 +default FALLBACK_SIZE = 131072
Please make those x*1024 format too, for consistency (_all_ sizes should use that format, IMO).
Index: src/mainboard/bcom/winnet100/failover.c
Should be dropped, see above.
Index: src/mainboard/bcom/winnet100/auto.c
--- src/mainboard/bcom/winnet100/auto.c (Revision 0) +++ src/mainboard/bcom/winnet100/auto.c (Revision 0) @@ -0,0 +1,54 @@ +/*
- This file is part of the LinuxBIOS project.
- Copyright (C) 2007 Uwe Hermann uwe@hermann-uwe.de
- Modyfied by Juergen Beisert juergen@kreuzholzen.de
Nah, drop me, this is way too trivial to list my name here. Just use
Copyright (C) 2007 Juergen Beisert juergen@kreuzholzen.de
+static void main(unsigned long bist) +{
- /* Initialize the serial console. */
- pc97317_enable_serial(SERIAL_DEV, TTYS0_BASE);
- uart_init();
- console_init();
- /* Halt if there was a built in self test failure. */
- report_bist_failure(bist);
- /* Initialize RAM. */
- sdram_init();
- /* Check whether RAM works. */
- /* ram_check(0x00000000, 0x4000); */
ram_check(0x00000000, 640 * 1024);
(just in case anybody want to uncomment it; I think it'll fail in the current form)
Index: src/mainboard/bcom/winnet100/chip.h
Missing GPL header (yes, trivial file, but please add it anyway).
Index: src/mainboard/bcom/winnet100/cmos.layout
--- src/mainboard/bcom/winnet100/cmos.layout (Revision 0) +++ src/mainboard/bcom/winnet100/cmos.layout (Revision 0)
Unused, so can be dropped (or, alternatively, it should be tested and the CMOS config option above be enabled).
Index: src/mainboard/bcom/winnet100/mainboard.c
--- src/mainboard/bcom/winnet100/mainboard.c (Revision 0) +++ src/mainboard/bcom/winnet100/mainboard.c (Revision 0) @@ -0,0 +1,11 @@ +#include <console/console.h> +#include <device/device.h> +#include <device/pci.h> +#include <device/pci_ids.h> +#include <device/pci_ops.h> +#include <arch/io.h> +#include "chip.h"
All except #include <device/device.h> #include "chip.h" can be dropped, I think.
+struct chip_operations mainboard_bcom_winnet100_ops = {
- CHIP_NAME("BCOM winterm100 mainboard ")
winterm100 -> winnet100?
What's the exact spelling in the manual or on the PCB? We should try to use the "correct" name, if possible.
Index: targets/bcom/winnet100/Config.lb
--- targets/bcom/winnet100/Config.lb (Revision 0) +++ targets/bcom/winnet100/Config.lb (Revision 0)
Missing GPL header.
@@ -0,0 +1,24 @@ +# Config file for the BCOM WINNET100 motherboard +# (available as IGEL-316 for example) +# Refer: http://www.linuxbios.org/index.php/BCOM_WINNET100_Build_Tutorial
Drop the "index.php/" from the URL, it'll work too, and the ugly index.php is only a temporary fix, will be gone soon.
+# +target winnet100 +mainboard bcom/winnet100
+option ROM_SIZE=1024*256
Hm, I'd do it the other way around (but it doesn't matter much):
option ROM_SIZE = 256 * 1024
+romimage "normal"
- option USE_FALLBACK_IMAGE=0
- option ROM_IMAGE_SIZE=0x10000
option ROM_IMAGE_SIZE = 64 * 1024
- option LINUXBIOS_EXTRA_VERSION=".0Normal"
- payload ../../../../../../../images/etherboot.elf
+end
+romimage "fallback"
- option USE_FALLBACK_IMAGE=1
- option ROM_IMAGE_SIZE=0x10000
Same here.
Patch looks good, with the above fixes I think we can commit it.
Uwe.