Change in coreboot[master]: [WIP] fix autoport

[Angel Pons (Code Review)]

Angel Pons has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/37730 )

Change subject: [WIP] fix autoport
......................................................................

[WIP] fix autoport

Change-Id: I8d1a6af6f1d70268f17692bee130c08502082c97
Signed-off-by: Angel Pons 
---
M util/autoport/azalia.go
M util/autoport/bd82x6x.go
2 files changed, 13 insertions(+), 27 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/30/37730/1

diff --git a/util/autoport/azalia.go b/util/autoport/azalia.go
index c525189..e00f457 100644
--- a/util/autoport/azalia.go
+++ b/util/autoport/azalia.go
@@ -20,13 +20,12 @@
 `)

for _, codec := range ctx.InfoSource.GetAzaliaCodecs() {
-		fmt.Fprintf(az, "\t0x%08x, /* Codec Vendor / Device ID: %s */\n",
+		fmt.Fprintf(az, "\t0x%08x,\t/* Codec Vendor / Device ID: %s */\n",
    		codec.VendorID, codec.Name)
-		fmt.Fprintf(az, "\t0x%08x, /* Subsystem ID */\n",
+		fmt.Fprintf(az, "\t0x%08x,\t/* Subsystem ID */\n",
    		codec.SubsystemID)
-		fmt.Fprintf(az, "\n\t0x%08x, /* Number of 4 dword sets */\n",
+		fmt.Fprintf(az, "\n\t%d,\t\t/* Number of 4 dword sets */\n",
    		len(codec.PinConfig)+1)
-		fmt.Fprintf(az, "\t/* NID 0x01: Subsystem ID.  */\n")
    	fmt.Fprintf(az, "\tAZALIA_SUBVENDOR(0x%x, 0x%08x),\n",
    		codec.CodecNo, codec.SubsystemID)

@@ -38,7 +37,6 @@
    	sort.Ints(keys)

for _, nid := range keys {
-			fmt.Fprintf(az, "\n\t/* NID 0x%02x.  */\n", nid)
    		fmt.Fprintf(az, "\tAZALIA_PIN_CFG(0x%x, 0x%02x, 0x%08x),\n",
    			codec.CodecNo, nid, codec.PinConfig[nid])
    	}
diff --git a/util/autoport/bd82x6x.go b/util/autoport/bd82x6x.go
index fbe0c3a..1cee9f2 100644
--- a/util/autoport/bd82x6x.go
+++ b/util/autoport/bd82x6x.go
@@ -276,7 +276,7 @@
    }

PutPCIChip(addr, cur)
-	PutPCIDevParent(addr, "PCI-LPC bridge", "lpc")
+	PutPCIDevParent(addr, "LPC bridge", "lpc")

DSDTIncludes = append(DSDTIncludes, DSDTInclude{
    	File: "southbridge/intel/bd82x6x/acpi/platform.asl",
@@ -286,13 +286,13 @@
    	Comment: "global NVS and variables",
    })
    DSDTIncludes = append(DSDTIncludes, DSDTInclude{
-		File: "southbridge/intel/bd82x6x/acpi/sleepstates.asl",
+		File: "southbridge/intel/common/acpi/sleepstates.asl",
    })
    DSDTPCI0Includes = append(DSDTPCI0Includes, DSDTInclude{
    	File: "southbridge/intel/bd82x6x/acpi/pch.asl",
    })

-	sb := Create(ctx, "romstage.c")
+	sb := Create(ctx, "early_init.c")
    defer sb.Close()
    Add_gpl(sb)
    sb.WriteString(`/* FIXME: Check if all includes are needed. */
@@ -310,20 +310,7 @@
 #include <southbridge/intel/bd82x6x/pch.h>
 #include <southbridge/intel/common/gpio.h>

-void pch_enable_lpc(void)
-{
 `)
-	RestorePCI16Simple(sb, addr, 0x82)
-
-	RestorePCI16Simple(sb, addr, 0x80)
-
-	sb.WriteString(`}
-
-void mainboard_rcba_config(void)
-{
-`)
-	sb.WriteString("}\n\n")
-
    sb.WriteString("const struct southbridge_usb_port mainboard_usb_ports[] = {\n")

currentMap := map[uint32]int{
@@ -360,12 +347,13 @@
    guessedMap := GuessSPDMap(ctx)

sb.WriteString(`
-void mainboard_early_init(int s3resume)
-{
-}
+void bootblock_mainboard_early_init(void)
+{`)
+	RestorePCI16Simple(sb, addr, 0x82)

-void mainboard_config_superio(void)
-{
+	RestorePCI16Simple(sb, addr, 0x80)
+
+	sb.WriteString(`
 }

/* FIXME: Put proper SPD map here. */
@@ -394,7 +382,7 @@
    gnvs->s5u0 = 0;
    gnvs->s5u1 = 0;

-	// the lid is open by default.
+	/* The lid is open by default. */
    gnvs->lids = 1;

gnvs->tcrt = 100;

Change in coreboot[master]: util/autoport: correct build errors of produced files

[Angel Pons (Code Review)]

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37730 )

Change subject: util/autoport: correct build errors of produced files
......................................................................

Patch Set 2:

This change is ready for review.

Change in coreboot[master]: util/autoport: correct build errors of produced files

[Paul Menzel (Code Review)]

Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37730 )

Change subject: util/autoport: correct build errors of produced files
......................................................................

Patch Set 2: Code-Review+1

Change in coreboot[master]: util/autoport: correct build errors of produced files

[Maxim Polyakov (Code Review)]

Maxim Polyakov has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37730 )

Change subject: util/autoport: correct build errors of produced files
......................................................................

Patch Set 2: Code-Review+1

Change in coreboot[master]: util/autoport: correct build errors of produced files

[Nico Huber (Code Review)]

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37730 )

Change subject: util/autoport: correct build errors of produced files
......................................................................

Patch Set 2:

(5 comments)

This is full of unrelated cosmetic changes that need a commit message. Please split.

https://review.coreboot.org/c/coreboot/+/37730/2/util/autoport/azalia.go 
File util/autoport/azalia.go:

https://review.coreboot.org/c/coreboot/+/37730/2/util/autoport/azalia.go@a55 
PS2, Line 55: Audio controller
What is fixed by removing this? Isn't it just an informational string?

https://review.coreboot.org/c/coreboot/+/37730/2/util/autoport/bd82x6x.go 
File util/autoport/bd82x6x.go:

https://review.coreboot.org/c/coreboot/+/37730/2/util/autoport/bd82x6x.go@a2... 
PS2, Line 279: PCI-
Why drop this?

https://review.coreboot.org/c/coreboot/+/37730/2/util/autoport/main.go 
File util/autoport/main.go:

https://review.coreboot.org/c/coreboot/+/37730/2/util/autoport/main.go@821 
PS2, Line 821: 	fmt.Fprintf(bi, "Flashrom support: %s\n", FlashROMSupport)
What is the reason for the changes here?

https://review.coreboot.org/c/coreboot/+/37730/2/util/autoport/sandybridge.g... 
File util/autoport/sandybridge.go:

https://review.coreboot.org/c/coreboot/+/37730/2/util/autoport/sandybridge.g... 
PS2, Line 137: 		RegisterPCI(0x8086, id, GenericVGA{GenericPCI{Comment: "VGA controller"}})
Why?

https://review.coreboot.org/c/coreboot/+/37730/2/util/autoport/sandybridge.g... 
PS2, Line 15: `)
Hmmmm, I wonder if we should drop this. There is no reason to run
the VBIOS within coreboot anymore. (To gain better compatibility,
one would have to run it in SeaBIOS anyway, which has its own
handlers.)

Change in coreboot[master]: util/autoport: correct build errors of produced files

[Angel Pons (Code Review)]

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37730 )

Change subject: util/autoport: correct build errors of produced files
......................................................................

Patch Set 2:

(5 comments)
> Patch Set 2:
>
> (5 comments)
>
> This is full of unrelated cosmetic changes that need a commit message. Please split.

You are right. The cosmetic changes are things I would otherwise have to change or comment on each and every autoport usage.

https://review.coreboot.org/c/coreboot/+/37730/2/util/autoport/azalia.go 
File util/autoport/azalia.go:

https://review.coreboot.org/c/coreboot/+/37730/2/util/autoport/azalia.go@a55 
PS2, Line 55: Audio controller
> What is fixed by removing this? Isn't it just an informational string?

It results in "High Definition Audio Audio Controller"

https://review.coreboot.org/c/coreboot/+/37730/2/util/autoport/bd82x6x.go 
File util/autoport/bd82x6x.go:

https://review.coreboot.org/c/coreboot/+/37730/2/util/autoport/bd82x6x.go@a2... 
PS2, Line 279: PCI-
> Why drop this?

It is kind of redundant, results in "LPC bridge PCI-LPC bridge". I might as well drop it altogether.

https://review.coreboot.org/c/coreboot/+/37730/2/util/autoport/main.go 
File util/autoport/main.go:

https://review.coreboot.org/c/coreboot/+/37730/2/util/autoport/main.go@821 
PS2, Line 821: 	fmt.Fprintf(bi, "Flashrom support: %s\n", FlashROMSupport)
> What is the reason for the changes here?

It's easier to complete the fields that way. Also, I dropped the release year because it is of little use (I find it hard to determine the release year of a board, and it doesn't even appear on board_status anyway).

https://review.coreboot.org/c/coreboot/+/37730/2/util/autoport/sandybridge.g... 
File util/autoport/sandybridge.go:

https://review.coreboot.org/c/coreboot/+/37730/2/util/autoport/sandybridge.g... 
PS2, Line 137: 		RegisterPCI(0x8086, id, GenericVGA{GenericPCI{Comment: "VGA controller"}})
> Why?

Results in "Integrated Graphics VGA Controller", which is rather obnoxious.

https://review.coreboot.org/c/coreboot/+/37730/2/util/autoport/sandybridge.g... 
PS2, Line 15: `)
> Hmmmm, I wonder if we should drop this. There is no reason to run […]

I dropped the whole mainboard.c on (IIRC) Asus P8H61-M PRO, so it's a good idea

Change in coreboot[master]: util/autoport: correct build errors of produced files

[Nico Huber (Code Review)]

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37730 )

Change subject: util/autoport: correct build errors of produced files
......................................................................

Patch Set 2:

(4 comments)

https://review.coreboot.org/c/coreboot/+/37730/2/util/autoport/azalia.go 
File util/autoport/azalia.go:

https://review.coreboot.org/c/coreboot/+/37730/2/util/autoport/azalia.go@a55 
PS2, Line 55: Audio controller
> It results in "High Definition Audio Audio Controller"

Ack

https://review.coreboot.org/c/coreboot/+/37730/2/util/autoport/bd82x6x.go 
File util/autoport/bd82x6x.go:

https://review.coreboot.org/c/coreboot/+/37730/2/util/autoport/bd82x6x.go@a2... 
PS2, Line 279: PCI-
> It is kind of redundant, results in "LPC bridge PCI-LPC bridge". I might as well drop it altogether.

Well, if I take `PCI-` out of `LPC bridge PCI-LPC bridge` what will be left
is `LPC bridge LPC bridge`?

https://review.coreboot.org/c/coreboot/+/37730/2/util/autoport/main.go 
File util/autoport/main.go:

https://review.coreboot.org/c/coreboot/+/37730/2/util/autoport/main.go@821 
PS2, Line 821: 	fmt.Fprintf(bi, "Flashrom support: %s\n", FlashROMSupport)
> It's easier to complete the fields that way. […]

Just put it in the commit message, please.

https://review.coreboot.org/c/coreboot/+/37730/2/util/autoport/sandybridge.g... 
File util/autoport/sandybridge.go:

https://review.coreboot.org/c/coreboot/+/37730/2/util/autoport/sandybridge.g... 
PS2, Line 137: 		RegisterPCI(0x8086, id, GenericVGA{GenericPCI{Comment: "VGA controller"}})
> Results in "Integrated Graphics VGA Controller", which is rather obnoxious.

Ack

Change in coreboot[master]: util/autoport: correct build errors of produced files

[Nico Huber (Code Review)]

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37730 )

Change subject: util/autoport: correct build errors of produced files
......................................................................

Patch Set 2:

(1 comment)

https://review.coreboot.org/c/coreboot/+/37730/2/util/autoport/bd82x6x.go 
File util/autoport/bd82x6x.go:

https://review.coreboot.org/c/coreboot/+/37730/2/util/autoport/bd82x6x.go@a2... 
PS2, Line 260: 			PCISlot{PCIAddr: PCIAddr{Dev: 0x1f, Func: 0}, writeEmpty: true, additionalComment: "LPC bridge"},
This might be the first "LPC bridge" in "LPC bridge PCI-LPC bridge"?

Change in coreboot[master]: util/autoport: correct build errors of produced files

[Angel Pons (Code Review)]

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37730 )

Change subject: util/autoport: correct build errors of produced files
......................................................................

Patch Set 2:

(2 comments)

https://review.coreboot.org/c/coreboot/+/37730/2/util/autoport/bd82x6x.go 
File util/autoport/bd82x6x.go:

https://review.coreboot.org/c/coreboot/+/37730/2/util/autoport/bd82x6x.go@a2... 
PS2, Line 260: 			PCISlot{PCIAddr: PCIAddr{Dev: 0x1f, Func: 0}, writeEmpty: true, additionalComment: "LPC bridge"},
> This might be the first "LPC bridge" in "LPC bridge PCI-LPC bridge"?

Yep, seems like it. I'll also rename some of these later. For example, IIRC, SATA controller #1 is for AHCI and #2 is for legacy mode.

https://review.coreboot.org/c/coreboot/+/37730/2/util/autoport/bd82x6x.go@a2... 
PS2, Line 279: PCI-
> Well, if I take `PCI-` out of `LPC bridge PCI-LPC bridge` what will be left […]

Yes. I don't know what I was thinking when I did this

Change in coreboot[master]: util/autoport: correct build errors of produced files

[Angel Pons (Code Review)]

Hello Maxim Polyakov, Paul Menzel, build bot (Jenkins),

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/37730

to look at the new patch set (#3).

Change subject: util/autoport: correct build errors of produced files
......................................................................

util/autoport: correct build errors of produced files

Change-Id: I8d1a6af6f1d70268f17692bee130c08502082c97
Signed-off-by: Angel Pons 
---
M util/autoport/azalia.go
M util/autoport/bd82x6x.go
M util/autoport/ec_fixme.go
M util/autoport/main.go
M util/autoport/sandybridge.go
5 files changed, 40 insertions(+), 47 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/30/37730/3

Change in coreboot[master]: util/autoport: correct build errors of produced files

[Angel Pons (Code Review)]

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37730 )

Change subject: util/autoport: correct build errors of produced files
......................................................................

Patch Set 3:

(2 comments)

https://review.coreboot.org/c/coreboot/+/37730/2/util/autoport/bd82x6x.go 
File util/autoport/bd82x6x.go:

https://review.coreboot.org/c/coreboot/+/37730/2/util/autoport/bd82x6x.go@a2... 
PS2, Line 279: PCI-
> Yes. […]

Undid the change, will do properly on a follow-up

https://review.coreboot.org/c/coreboot/+/37730/2/util/autoport/main.go 
File util/autoport/main.go:

https://review.coreboot.org/c/coreboot/+/37730/2/util/autoport/main.go@821 
PS2, Line 821: 	fmt.Fprintf(bi, "Flashrom support: %s\n", FlashROMSupport)
> Just put it in the commit message, please.

Will do in a separate change.

Change in coreboot[master]: util/autoport: correct build errors of produced files

[Angel Pons (Code Review)]

Hello Maxim Polyakov, Paul Menzel, build bot (Jenkins),

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/37730

to look at the new patch set (#4).

Change subject: util/autoport: correct build errors of produced files
......................................................................

util/autoport: correct build errors of produced files

Change-Id: I8d1a6af6f1d70268f17692bee130c08502082c97
Signed-off-by: Angel Pons 
---
M util/autoport/azalia.go
M util/autoport/bd82x6x.go
M util/autoport/ec_fixme.go
M util/autoport/main.go
M util/autoport/sandybridge.go
5 files changed, 40 insertions(+), 46 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/30/37730/4

Change in coreboot[master]: util/autoport: correct build errors of produced files

[Nico Huber (Code Review)]

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37730 )

Change subject: util/autoport: correct build errors of produced files
......................................................................

Patch Set 4: Code-Review+2

(1 comment)

https://review.coreboot.org/c/coreboot/+/37730/4/util/autoport/bd82x6x.go 
File util/autoport/bd82x6x.go:

https://review.coreboot.org/c/coreboot/+/37730/4/util/autoport/bd82x6x.go@36... 
PS4, Line 360: 
For the cosmetic changes: Maybe add a comment here to check if it's necessary
(a lot of LPC routing is set up by default now).

Change in coreboot[master]: util/autoport: correct build errors of produced files

[Nico Huber (Code Review)]

Nico Huber has submitted this change. ( https://review.coreboot.org/c/coreboot/+/37730 )

Change subject: util/autoport: correct build errors of produced files
......................................................................

util/autoport: correct build errors of produced files

Change-Id: I8d1a6af6f1d70268f17692bee130c08502082c97
Signed-off-by: Angel Pons 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/37730
Tested-by: build bot (Jenkins) 
Reviewed-by: Nico Huber 
---
M util/autoport/azalia.go
M util/autoport/bd82x6x.go
M util/autoport/ec_fixme.go
M util/autoport/main.go
M util/autoport/sandybridge.go
5 files changed, 40 insertions(+), 46 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Nico Huber: Looks good to me, approved

diff --git a/util/autoport/azalia.go b/util/autoport/azalia.go
index c525189..d94441b 100644
--- a/util/autoport/azalia.go
+++ b/util/autoport/azalia.go
@@ -20,13 +20,12 @@
 `)

for _, codec := range ctx.InfoSource.GetAzaliaCodecs() {
-		fmt.Fprintf(az, "\t0x%08x, /* Codec Vendor / Device ID: %s */\n",
+		fmt.Fprintf(az, "\t0x%08x,\t/* Codec Vendor / Device ID: %s */\n",
    		codec.VendorID, codec.Name)
-		fmt.Fprintf(az, "\t0x%08x, /* Subsystem ID */\n",
+		fmt.Fprintf(az, "\t0x%08x,\t/* Subsystem ID */\n",
    		codec.SubsystemID)
-		fmt.Fprintf(az, "\n\t0x%08x, /* Number of 4 dword sets */\n",
+		fmt.Fprintf(az, "\t%d,\t\t/* Number of 4 dword sets */\n",
    		len(codec.PinConfig)+1)
-		fmt.Fprintf(az, "\t/* NID 0x01: Subsystem ID.  */\n")
    	fmt.Fprintf(az, "\tAZALIA_SUBVENDOR(0x%x, 0x%08x),\n",
    		codec.CodecNo, codec.SubsystemID)

@@ -38,10 +37,10 @@
    	sort.Ints(keys)

for _, nid := range keys {
-			fmt.Fprintf(az, "\n\t/* NID 0x%02x.  */\n", nid)
    		fmt.Fprintf(az, "\tAZALIA_PIN_CFG(0x%x, 0x%02x, 0x%08x),\n",
    			codec.CodecNo, nid, codec.PinConfig[nid])
    	}
+		az.WriteString("\n");
    }

az.WriteString(
@@ -52,7 +51,7 @@
 AZALIA_ARRAY_SIZES;
 `)

-	PutPCIDev(addr, "Audio controller")
+	PutPCIDev(addr, "")
 }

func init() {
diff --git a/util/autoport/bd82x6x.go b/util/autoport/bd82x6x.go
index fbe0c3a..3ad212b 100644
--- a/util/autoport/bd82x6x.go
+++ b/util/autoport/bd82x6x.go
@@ -41,6 +41,7 @@
    gpio := Create(ctx, "gpio.c")
    defer gpio.Close()

+	AddBootBlockFile("gpio.c", "")
    AddROMStageFile("gpio.c", "")

Add_gpl(gpio)
@@ -279,20 +280,22 @@
    PutPCIDevParent(addr, "PCI-LPC bridge", "lpc")

DSDTIncludes = append(DSDTIncludes, DSDTInclude{
-		File: "southbridge/intel/bd82x6x/acpi/platform.asl",
+		File: "southbridge/intel/common/acpi/platform.asl",
    })
    DSDTIncludes = append(DSDTIncludes, DSDTInclude{
-		File:    "southbridge/intel/bd82x6x/acpi/globalnvs.asl",
-		Comment: "global NVS and variables",
+		File:  "southbridge/intel/bd82x6x/acpi/globalnvs.asl",
    })
    DSDTIncludes = append(DSDTIncludes, DSDTInclude{
-		File: "southbridge/intel/bd82x6x/acpi/sleepstates.asl",
+		File: "southbridge/intel/common/acpi/sleepstates.asl",
    })
    DSDTPCI0Includes = append(DSDTPCI0Includes, DSDTInclude{
    	File: "southbridge/intel/bd82x6x/acpi/pch.asl",
    })

-	sb := Create(ctx, "romstage.c")
+	AddBootBlockFile("early_init.c", "")
+	AddROMStageFile("early_init.c", "")
+
+	sb := Create(ctx, "early_init.c")
    defer sb.Close()
    Add_gpl(sb)
    sb.WriteString(`/* FIXME: Check if all includes are needed. */
@@ -305,25 +308,13 @@
 #include <device/pci_ops.h>
 #include <device/pnp_ops.h>
 #include <console/console.h>
+#include <bootblock_common.h>
 #include <northbridge/intel/sandybridge/sandybridge.h>
 #include <northbridge/intel/sandybridge/raminit_native.h>
 #include <southbridge/intel/bd82x6x/pch.h>
 #include <southbridge/intel/common/gpio.h>

-void pch_enable_lpc(void)
-{
 `)
-	RestorePCI16Simple(sb, addr, 0x82)
-
-	RestorePCI16Simple(sb, addr, 0x80)
-
-	sb.WriteString(`}
-
-void mainboard_rcba_config(void)
-{
-`)
-	sb.WriteString("}\n\n")
-
    sb.WriteString("const struct southbridge_usb_port mainboard_usb_ports[] = {\n")

currentMap := map[uint32]int{
@@ -360,13 +351,14 @@
    guessedMap := GuessSPDMap(ctx)

sb.WriteString(`
-void mainboard_early_init(int s3resume)
+void bootblock_mainboard_early_init(void)
 {
-}
+`)
+	RestorePCI16Simple(sb, addr, 0x82)

-void mainboard_config_superio(void)
-{
-}
+	RestorePCI16Simple(sb, addr, 0x80)
+
+	sb.WriteString(`}

/* FIXME: Put proper SPD map here. */
 void mainboard_get_spd(spd_raw_data *spd, bool id_only)
@@ -394,7 +386,7 @@
    gnvs->s5u0 = 0;
    gnvs->s5u1 = 0;

-	// the lid is open by default.
+	/* The lid is open by default. */
    gnvs->lids = 1;

gnvs->tcrt = 100;
diff --git a/util/autoport/ec_fixme.go b/util/autoport/ec_fixme.go
index f06d457..f54bb1a 100644
--- a/util/autoport/ec_fixme.go
+++ b/util/autoport/ec_fixme.go
@@ -22,13 +22,13 @@
    ap.WriteString(
    	`Method(_WAK,1)
 {
-        /* FIXME: EC support  */
+	/* FIXME: EC support  */
    Return(Package(){0,0})
 }

Method(_PTS,1)
 {
-        /* FIXME: EC support  */
+	/* FIXME: EC support  */
 }
 `)

diff --git a/util/autoport/main.go b/util/autoport/main.go
index 03523a2..dbd8913 100644
--- a/util/autoport/main.go
+++ b/util/autoport/main.go
@@ -77,6 +77,7 @@
 }

var SouthBridge SouthBridger
+var BootBlockFiles map[string]string = map[string]string{}
 var ROMStageFiles map[string]string = map[string]string{}
 var RAMStageFiles map[string]string = map[string]string{}
 var SMMFiles map[string]string = map[string]string{}
@@ -154,6 +155,10 @@
    return result
 }

+func AddBootBlockFile(Name string, Condition string) {
+	BootBlockFiles[Name] = Condition
+}
+
 func AddROMStageFile(Name string, Condition string) {
    ROMStageFiles[Name] = Condition
 }
@@ -190,8 +195,7 @@
    	if condition == "" {
    		fmt.Fprintf(mf, "%s-y += %s\n", category, file)
    	} else {
-			fmt.Fprintf(mf, "%s-$(%s) += %s\n", category,
-				condition, file)
+			fmt.Fprintf(mf, "%s-$(%s) += %s\n", category, condition, file)
    	}
    }
 }
@@ -757,9 +761,10 @@
    	AddRAMStageFile("gma-mainboard.ads", "CONFIG_MAINBOARD_USE_LIBGFXINIT")
    }

-	if len(ROMStageFiles) > 0 || len(RAMStageFiles) > 0 || len(SMMFiles) > 0 {
+	if len(BootBlockFiles) > 0 || len(ROMStageFiles) > 0 || len(RAMStageFiles) > 0 || len(SMMFiles) > 0 {
    	mf := Create(ctx, "Makefile.inc")
    	defer mf.Close()
+		writeMF(mf, BootBlockFiles, "bootblock")
    	writeMF(mf, ROMStageFiles, "romstage")
    	writeMF(mf, RAMStageFiles, "ramstage")
    	writeMF(mf, SMMFiles, "smm")
@@ -854,17 +859,18 @@

dsdt.WriteString(
    	`
+
 #include <arch/acpi.h>
+
 DefinitionBlock(
    "dsdt.aml",
    "DSDT",
-	0x02,		// DSDT revision: ACPI 2.0 and up
+	0x02,		/* DSDT revision: ACPI 2.0 and up */
    OEM_ID,
    ACPI_TABLE_CREATOR,
-	0x20141018	// OEM revision
+	0x20141018	/* OEM revision */
 )
 {
-	/* Some generic macros */
    #include "acpi/platform.asl"
 `)

diff --git a/util/autoport/sandybridge.go b/util/autoport/sandybridge.go
index 170d197..718fbe8 100644
--- a/util/autoport/sandybridge.go
+++ b/util/autoport/sandybridge.go
@@ -8,7 +8,7 @@

/* FIXME:XX Move this somewhere else.  */
    MainboardIncludes = append(MainboardIncludes, "drivers/intel/gma/int15.h")
-	MainboardEnable += (`	/* FIXME: fix those values*/
+	MainboardEnable += (`	/* FIXME: fix these values. */
    install_intel_vga_int15_handler(GMA_INT15_ACTIVE_LFP_INT_LVDS,
    				GMA_INT15_PANEL_FIT_DEFAULT,
    				GMA_INT15_BOOT_DISPLAY_DEFAULT, 0);
@@ -37,7 +37,7 @@
    DevTree = DevTreeNode{
    	Chip:          "northbridge/intel/sandybridge",
    	MissingParent: "northbridge",
-		Comment:       "FIXME: check gfx.ndid and gfx.did",
+		Comment:       "FIXME: GPU registers may not always apply.",
    	Registers: map[string]string{
    		"gpu_dp_b_hotplug":                    FormatInt32((inteltool.IGD[0xc4030] >> 2) & 7),
    		"gpu_dp_c_hotplug":                    FormatInt32((inteltool.IGD[0xc4030] >> 10) & 7),
@@ -52,9 +52,6 @@
    		"gpu_pch_backlight":                   FormatHex32((inteltool.IGD[0xc8254] >> 16) * 0x10001),
    		"gfx.use_spread_spectrum_clock":       FormatBool((inteltool.IGD[0xc6200]>>12)&1 != 0),
    		"gfx.link_frequency_270_mhz":          FormatBool(link_frequency > 200000),
-			/* FIXME:XX hardcoded.  */
-			"gfx.ndid": "3",
-			"gfx.did":  "{ 0x80000100, 0x80000240, 0x80000410 }",
    	},
    	Children: []DevTreeNode{
    		{
@@ -95,8 +92,8 @@
    			ChildPCIBus:   0,
    			PCISlots: []PCISlot{
    				PCISlot{PCIAddr: PCIAddr{Dev: 0x0, Func: 0}, writeEmpty: true, additionalComment: "Host bridge"},
-					PCISlot{PCIAddr: PCIAddr{Dev: 0x1, Func: 0}, writeEmpty: true, additionalComment: "PCIe Bridge for discrete graphics"},
-					PCISlot{PCIAddr: PCIAddr{Dev: 0x2, Func: 0}, writeEmpty: true, additionalComment: "Internal graphics"},
+					PCISlot{PCIAddr: PCIAddr{Dev: 0x1, Func: 0}, writeEmpty: true, additionalComment: "PEG"},
+					PCISlot{PCIAddr: PCIAddr{Dev: 0x2, Func: 0}, writeEmpty: true, additionalComment: "iGPU"},
    			},
    		},
    	},
@@ -134,7 +131,7 @@
    	0x0112, 0x0116, 0x0122, 0x0126,
    	0x0152, 0x0156, 0x0162, 0x0166,
    } {
-		RegisterPCI(0x8086, id, GenericVGA{GenericPCI{Comment: "VGA controller"}})
+		RegisterPCI(0x8086, id, GenericVGA{GenericPCI{}})
    }

/* PCIe bridge */

Change in coreboot[master]: util/autoport: correct build errors of produced files

[Angel Pons (Code Review)]

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37730 )

Change subject: util/autoport: correct build errors of produced files
......................................................................

Patch Set 5:

(1 comment)

https://review.coreboot.org/c/coreboot/+/37730/4/util/autoport/bd82x6x.go 
File util/autoport/bd82x6x.go:

https://review.coreboot.org/c/coreboot/+/37730/4/util/autoport/bd82x6x.go@36... 
PS4, Line 360:
> For the cosmetic changes: Maybe add a comment here to check if it's necessary […]

Ack