Scott Duplichan wrote:
I accidentally did the 'svn cp' step interactively instead of by patch. The attached patch completes the work of converting AMD Persimmon into ASRock E350M1.
Good stuff. Some simple comments, then I'll ack.
A video option rom needs to be added to support the built-in uma graphics.
Does the default filename match what was extracted from factory EFI?
+++ src/mainboard/asrock/e350m1/dsdt.asl (working copy) @@ -23,7 +23,7 @@ "DSDT", /* Signature */ 0x02, /* DSDT Revision, needs to be 2 for 64bit */ "AMD ", /* OEMID */
- "PERSIMMO", /* TABLE ID */
- "E350M1 ", /* TABLE ID */
Also change AMD?
+++ src/mainboard/asrock/e350m1/Kconfig (working copy)
..
@@ -64,7 +64,7 @@
config MAINBOARD_PART_NUMBER string
default "Persimmon"
default "e350m1"
I think this should be uppercase.
-#define CONFIG_VGA_BIOS_ID "1002,9804" +#define CONFIG_VGA_BIOS_ID "1002,9802"
Why have this comment at all? Better just remove it.
+++ src/mainboard/asrock/e350m1/mainboard.c (working copy)
..
- printk(BIOS_INFO, "Mainboard Persimmon Enable. dev=0x%p\n", dev);
- printk(BIOS_INFO, "Mainboard E350M1 Enable. dev=0x%p\n", dev);
Please change to: "Mainboard " CONFIG_MAINBOARD_PART_NUMBER " Enable." ..
@@ -110,6 +110,6 @@ return 0; } struct chip_operations mainboard_ops = {
- CHIP_NAME("AMD PERSIMMON Mainboard")
- .enable_dev = persimmon_enable,
- CHIP_NAME("ASRock E350M1 Mainboard")
- .enable_dev = e350m1_enable,
Same here. CHIP_NAME(CONFIG_MAINBOARD_VENDOR " " CONFIG_MAINBOARD_PART_NUMBER " Mainboard")
+++ src/mainboard/asrock/e350m1/romstage.c (working copy)
..
@@ -52,7 +52,7 @@ sb_poweron_init();
post_code(0x31);
- f81865f_enable_serial(SERIAL_DEV, CONFIG_TTYS0_BASE);
- w83627hf_enable_serial(SERIAL_DEV, CONFIG_TTYS0_BASE);
Whitespace?
//Peter