[coreboot] Coreboot for AMD Fusion family 14h: ASRock E350M1

[Peter Stuge]

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