Myles Watson wrote:
This patch adds initial support for mainboard/tyan/s2892. It compiles, but is untested in hardware.
I think this needs a little polishing.
+++ svn/mainboard/tyan/s2892/dts
..
+/{
- device_operations="s2892";
- mainboard_vendor = "AMD";
Tyan, not AMD, right? :)
+++ svn/mainboard/tyan/Kconfig
..
+choice
- prompt "Mainboard model"
- depends on VENDOR_TYAN
+config BOARD_TYAN_S2892
- bool "s2892"
- select ARCH_X86
- select CPU_AMD_K8
- select NORTHBRIDGE_AMD_K8
- select SOUTHBRIDGE_NVIDIA_CK804
- select SOUTHBRIDGE_AMD_AMD8131
- select SUPERIO_WINBOND_W83627HF
- select IOAPIC
- help
Tyan Thunder K8SE (s2892)+endchoice
Does Kconfig allow choices to span multiple files? If so it would be nice to not have to repeat the Mainboard model choice for each vendor.
I would like this option to read "Thunder K8SE (S2892)" as Tyan uses on the web: http://www.tyan.com/product_board_detail.aspx?pid=145
(And not just the help, but the actual user visible option name.)
+++ svn/mainboard/tyan/s2892/mainboard.c
..
+struct device_operations s2892 = {
- .id = {.type = DEVICE_ID_PCI,
{.pci = {.vendor = PCI_VENDOR_ID_TYAN,.device = 0x2892}}},- .constructor = default_device_constructor,
+};
Why is this needed? The board itself isn't a PCI device. It can certainly have a subsystem it belonging to it, but that was already specified in the dts so must not be needed to duplicate here.
+++ svn/mainboard/tyan/s2892/stage1.c
..
+void ck804_enable_rom(void);
This prototype shouldn't be needed here right? What ROM is it anyway? The NIC? See discussion in ck804 thread.
+#define SERIAL_DEV W83627HF_SP1 +#define SERIAL_IOBASE 0x3f8
Isn't at least the iobase in a CONFIG_ already?
+void mainboard_pre_payload(void) +{
- banner(BIOS_DEBUG, "mainboard_pre_payload: done");
+}
Can we remove this function from mainboard sources when it doesn't do anything like here?
+++ svn/mainboard/tyan/s2892/initram.c
..
+/* this code is very mainboard dependent, sadly. */
..
+/**
- read a byte from spd.
- @param device device to read from
- @param address address in the spd ROM
- @return the value of the byte at that address.
- */
+u8 spd_read_byte(u16 device, u8 address) +{
- int smbus_read_byte(u16 device, u16 address);
- return smbus_read_byte(device, address);
+}
I hope you see the irony above.
+/**
- main for initram for the AMD Serengeti
No, this is for s2892. If this code is copypaste, it can not go in. I would like us to reuse more.
//Peter