HAOUAS Elyes has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/31267
Change subject: mb/{asrock,intel,kontron}: Include missing <arch/acpi.h> ......................................................................
mb/{asrock,intel,kontron}: Include missing <arch/acpi.h>
Also includes lines sorted
Change-Id: Idf2b41f471f531b2a9c3e620563e3c658dea4729 Signed-off-by: Elyes HAOUAS ehaouas@noos.fr --- M src/mainboard/asrock/g41c-gs/romstage.c M src/mainboard/asus/p5qpl-am/romstage.c M src/mainboard/intel/dg41wv/romstage.c M src/mainboard/kontron/ktqm77/romstage.c 4 files changed, 34 insertions(+), 29 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/67/31267/1
diff --git a/src/mainboard/asrock/g41c-gs/romstage.c b/src/mainboard/asrock/g41c-gs/romstage.c index 0605ef1..ab6b1d8 100644 --- a/src/mainboard/asrock/g41c-gs/romstage.c +++ b/src/mainboard/asrock/g41c-gs/romstage.c @@ -15,17 +15,18 @@ * GNU General Public License for more details. */
+#include <arch/io.h> #include <console/console.h> -#include <southbridge/intel/i82801gx/i82801gx.h> -#include <southbridge/intel/common/gpio.h> -#include <northbridge/intel/x4x/x4x.h> -#include <cpu/x86/bist.h> #include <cpu/intel/romstage.h> -#include <superio/nuvoton/nct6776/nct6776.h> -#include <superio/winbond/w83627dhg/w83627dhg.h> -#include <superio/winbond/common/winbond.h> -#include <superio/nuvoton/common/nuvoton.h> +#include <cpu/x86/bist.h> #include <northbridge/intel/x4x/iomap.h> +#include <northbridge/intel/x4x/x4x.h> +#include <southbridge/intel/common/gpio.h> +#include <southbridge/intel/i82801gx/i82801gx.h> +#include <superio/nuvoton/common/nuvoton.h> +#include <superio/nuvoton/nct6776/nct6776.h> +#include <superio/winbond/common/winbond.h> +#include <superio/winbond/w83627dhg/w83627dhg.h>
#define SERIAL_DEV_R2 PNP_DEV(0x2e, NCT6776_SP1) #define SERIAL_DEV_R1 PNP_DEV(0x2e, W83627DHG_SP1) diff --git a/src/mainboard/asus/p5qpl-am/romstage.c b/src/mainboard/asus/p5qpl-am/romstage.c index abe3b67..7f84b6a 100644 --- a/src/mainboard/asus/p5qpl-am/romstage.c +++ b/src/mainboard/asus/p5qpl-am/romstage.c @@ -15,18 +15,19 @@ * GNU General Public License for more details. */
+#include <arch/io.h> #include <console/console.h> -#include <southbridge/intel/i82801gx/i82801gx.h> -#include <southbridge/intel/common/gpio.h> -#include <northbridge/intel/x4x/x4x.h> -#include <cpu/x86/bist.h> #include <cpu/intel/romstage.h> -#include <superio/winbond/w83627dhg/w83627dhg.h> -#include <superio/winbond/common/winbond.h> -#include <northbridge/intel/x4x/iomap.h> -#include <halt.h> #include <cpu/intel/speedstep.h> +#include <cpu/x86/bist.h> #include <cpu/x86/msr.h> +#include <halt.h> +#include <northbridge/intel/x4x/iomap.h> +#include <northbridge/intel/x4x/x4x.h> +#include <southbridge/intel/common/gpio.h> +#include <southbridge/intel/i82801gx/i82801gx.h> +#include <superio/winbond/common/winbond.h> +#include <superio/winbond/w83627dhg/w83627dhg.h>
#define SERIAL_DEV PNP_DEV(0x2e, W83627DHG_SP1) #define GPIO_DEV PNP_DEV(0x2e, W83627DHG_GPIO2345_V) diff --git a/src/mainboard/intel/dg41wv/romstage.c b/src/mainboard/intel/dg41wv/romstage.c index 7038538..2caf5ec 100644 --- a/src/mainboard/intel/dg41wv/romstage.c +++ b/src/mainboard/intel/dg41wv/romstage.c @@ -15,15 +15,16 @@ * GNU General Public License for more details. */
+#include <arch/io.h> #include <console/console.h> -#include <southbridge/intel/i82801gx/i82801gx.h> -#include <southbridge/intel/common/gpio.h> -#include <northbridge/intel/x4x/x4x.h> -#include <cpu/x86/bist.h> #include <cpu/intel/romstage.h> -#include <superio/winbond/w83627dhg/w83627dhg.h> -#include <superio/winbond/common/winbond.h> +#include <cpu/x86/bist.h> #include <northbridge/intel/x4x/iomap.h> +#include <northbridge/intel/x4x/x4x.h> +#include <southbridge/intel/common/gpio.h> +#include <southbridge/intel/i82801gx/i82801gx.h> +#include <superio/winbond/common/winbond.h> +#include <superio/winbond/w83627dhg/w83627dhg.h>
#define SERIAL_DEV PNP_DEV(0x2e, W83627DHG_SP1) #define LPC_DEV PCI_DEV(0, 0x1f, 0) diff --git a/src/mainboard/kontron/ktqm77/romstage.c b/src/mainboard/kontron/ktqm77/romstage.c index cd6f3b9..c4d26d3 100644 --- a/src/mainboard/kontron/ktqm77/romstage.c +++ b/src/mainboard/kontron/ktqm77/romstage.c @@ -16,18 +16,20 @@
#include <stdint.h> #include <string.h> -#include <timestamp.h> -#include <device/pci_def.h> -#include <cpu/x86/lapic.h> #include <arch/acpi.h> -#include <northbridge/intel/sandybridge/sandybridge.h> -#include <northbridge/intel/sandybridge/raminit.h> +#include <arch/io.h> +#include <cpu/x86/lapic.h> +#include <cpu/x86/msr.h> +#include <device/pci_def.h> +#include <halt.h> #include <northbridge/intel/sandybridge/raminit_native.h> +#include <northbridge/intel/sandybridge/raminit.h> +#include <northbridge/intel/sandybridge/sandybridge.h> #include <southbridge/intel/bd82x6x/pch.h> #include <southbridge/intel/common/gpio.h> -#include <cpu/x86/msr.h> -#include <halt.h> #include <superio/winbond/common/winbond.h> +#include <timestamp.h> +
void pch_enable_lpc(void) {
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31267 )
Change subject: mb/{asrock,intel,kontron}: Include missing <arch/acpi.h> ......................................................................
Patch Set 1: Code-Review+1
I’d still prefer two commits, but this is fine too.
Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31267 )
Change subject: mb/{asrock,intel,kontron}: Include missing <arch/acpi.h> ......................................................................
Patch Set 1:
(1 comment)
can you check if a timeless build produces the same binary with the patch applied? I'm not 100% sure if changing the order of the includes will change things; since the code quality of x4x and sandybridge is rather good, I don't really expect breakage from this though
https://review.coreboot.org/#/c/31267/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/#/c/31267/1//COMMIT_MSG@7 PS1, Line 7: <arch/acpi.h> <arch/io.h>
Hello Felix Held, Angel Pons, Paul Menzel, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/31267
to look at the new patch set (#2).
Change subject: mb/{asrock,intel,kontron}: Include missing <arch/io.h> ......................................................................
mb/{asrock,intel,kontron}: Include missing <arch/io.h>
Also includes lines sorted
Change-Id: Idf2b41f471f531b2a9c3e620563e3c658dea4729 Signed-off-by: Elyes HAOUAS ehaouas@noos.fr --- M src/mainboard/asrock/g41c-gs/romstage.c M src/mainboard/asus/p5qpl-am/romstage.c M src/mainboard/intel/dg41wv/romstage.c M src/mainboard/kontron/ktqm77/romstage.c 4 files changed, 34 insertions(+), 29 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/67/31267/2
HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31267 )
Change subject: mb/{asrock,intel,kontron}: Include missing <arch/io.h> ......................................................................
Patch Set 2:
(1 comment)
can you check if a timeless build produces the same binary with the patch applied? I'm not 100% sure if changing the order of the includes will change things; since the code quality of x4x and sandybridge is rather good, I don't really expect breakage from this though
sure, I will
HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31267 )
Change subject: mb/{asrock,intel,kontron}: Include missing <arch/io.h> ......................................................................
Patch Set 2:
(1 comment)
can you check if a timeless build produces the same binary with
the
patch applied? I'm not 100% sure if changing the order of the includes will
change
things; since the code quality of x4x and sandybridge is rather good, I don't really expect breakage from this though
sure, I will
Test with "make BUILD_TIMELESS=1" done on kontron/ktqm77 and asus/p5qpl-am. got the same md5sum build/coreboot.rom.
13d588fefd474a5e0ca3ce5439ea5edf for ktqm77 and 986cb89f71e1667b723d045c98d10b4c for p5qpl-am
Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31267 )
Change subject: mb/{asrock,intel,kontron}: Include missing <arch/io.h> ......................................................................
Patch Set 2: Code-Review+2
Patrick Georgi has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/31267 )
Change subject: mb/{asrock,intel,kontron}: Include missing <arch/io.h> ......................................................................
mb/{asrock,intel,kontron}: Include missing <arch/io.h>
Also includes lines sorted
Change-Id: Idf2b41f471f531b2a9c3e620563e3c658dea4729 Signed-off-by: Elyes HAOUAS ehaouas@noos.fr Reviewed-on: https://review.coreboot.org/c/31267 Reviewed-by: Felix Held felix-coreboot@felixheld.de Reviewed-by: Paul Menzel paulepanter@users.sourceforge.net Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/mainboard/asrock/g41c-gs/romstage.c M src/mainboard/asus/p5qpl-am/romstage.c M src/mainboard/intel/dg41wv/romstage.c M src/mainboard/kontron/ktqm77/romstage.c 4 files changed, 34 insertions(+), 29 deletions(-)
Approvals: build bot (Jenkins): Verified Paul Menzel: Looks good to me, but someone else must approve Felix Held: Looks good to me, approved
diff --git a/src/mainboard/asrock/g41c-gs/romstage.c b/src/mainboard/asrock/g41c-gs/romstage.c index 0605ef1..ab6b1d8 100644 --- a/src/mainboard/asrock/g41c-gs/romstage.c +++ b/src/mainboard/asrock/g41c-gs/romstage.c @@ -15,17 +15,18 @@ * GNU General Public License for more details. */
+#include <arch/io.h> #include <console/console.h> -#include <southbridge/intel/i82801gx/i82801gx.h> -#include <southbridge/intel/common/gpio.h> -#include <northbridge/intel/x4x/x4x.h> -#include <cpu/x86/bist.h> #include <cpu/intel/romstage.h> -#include <superio/nuvoton/nct6776/nct6776.h> -#include <superio/winbond/w83627dhg/w83627dhg.h> -#include <superio/winbond/common/winbond.h> -#include <superio/nuvoton/common/nuvoton.h> +#include <cpu/x86/bist.h> #include <northbridge/intel/x4x/iomap.h> +#include <northbridge/intel/x4x/x4x.h> +#include <southbridge/intel/common/gpio.h> +#include <southbridge/intel/i82801gx/i82801gx.h> +#include <superio/nuvoton/common/nuvoton.h> +#include <superio/nuvoton/nct6776/nct6776.h> +#include <superio/winbond/common/winbond.h> +#include <superio/winbond/w83627dhg/w83627dhg.h>
#define SERIAL_DEV_R2 PNP_DEV(0x2e, NCT6776_SP1) #define SERIAL_DEV_R1 PNP_DEV(0x2e, W83627DHG_SP1) diff --git a/src/mainboard/asus/p5qpl-am/romstage.c b/src/mainboard/asus/p5qpl-am/romstage.c index abe3b67..7f84b6a 100644 --- a/src/mainboard/asus/p5qpl-am/romstage.c +++ b/src/mainboard/asus/p5qpl-am/romstage.c @@ -15,18 +15,19 @@ * GNU General Public License for more details. */
+#include <arch/io.h> #include <console/console.h> -#include <southbridge/intel/i82801gx/i82801gx.h> -#include <southbridge/intel/common/gpio.h> -#include <northbridge/intel/x4x/x4x.h> -#include <cpu/x86/bist.h> #include <cpu/intel/romstage.h> -#include <superio/winbond/w83627dhg/w83627dhg.h> -#include <superio/winbond/common/winbond.h> -#include <northbridge/intel/x4x/iomap.h> -#include <halt.h> #include <cpu/intel/speedstep.h> +#include <cpu/x86/bist.h> #include <cpu/x86/msr.h> +#include <halt.h> +#include <northbridge/intel/x4x/iomap.h> +#include <northbridge/intel/x4x/x4x.h> +#include <southbridge/intel/common/gpio.h> +#include <southbridge/intel/i82801gx/i82801gx.h> +#include <superio/winbond/common/winbond.h> +#include <superio/winbond/w83627dhg/w83627dhg.h>
#define SERIAL_DEV PNP_DEV(0x2e, W83627DHG_SP1) #define GPIO_DEV PNP_DEV(0x2e, W83627DHG_GPIO2345_V) diff --git a/src/mainboard/intel/dg41wv/romstage.c b/src/mainboard/intel/dg41wv/romstage.c index 7038538..2caf5ec 100644 --- a/src/mainboard/intel/dg41wv/romstage.c +++ b/src/mainboard/intel/dg41wv/romstage.c @@ -15,15 +15,16 @@ * GNU General Public License for more details. */
+#include <arch/io.h> #include <console/console.h> -#include <southbridge/intel/i82801gx/i82801gx.h> -#include <southbridge/intel/common/gpio.h> -#include <northbridge/intel/x4x/x4x.h> -#include <cpu/x86/bist.h> #include <cpu/intel/romstage.h> -#include <superio/winbond/w83627dhg/w83627dhg.h> -#include <superio/winbond/common/winbond.h> +#include <cpu/x86/bist.h> #include <northbridge/intel/x4x/iomap.h> +#include <northbridge/intel/x4x/x4x.h> +#include <southbridge/intel/common/gpio.h> +#include <southbridge/intel/i82801gx/i82801gx.h> +#include <superio/winbond/common/winbond.h> +#include <superio/winbond/w83627dhg/w83627dhg.h>
#define SERIAL_DEV PNP_DEV(0x2e, W83627DHG_SP1) #define LPC_DEV PCI_DEV(0, 0x1f, 0) diff --git a/src/mainboard/kontron/ktqm77/romstage.c b/src/mainboard/kontron/ktqm77/romstage.c index 9083764..ea93e05 100644 --- a/src/mainboard/kontron/ktqm77/romstage.c +++ b/src/mainboard/kontron/ktqm77/romstage.c @@ -16,18 +16,20 @@
#include <stdint.h> #include <string.h> -#include <timestamp.h> -#include <device/pci_def.h> -#include <cpu/x86/lapic.h> #include <arch/acpi.h> -#include <northbridge/intel/sandybridge/sandybridge.h> -#include <northbridge/intel/sandybridge/raminit.h> +#include <arch/io.h> +#include <cpu/x86/lapic.h> +#include <cpu/x86/msr.h> +#include <device/pci_def.h> +#include <halt.h> #include <northbridge/intel/sandybridge/raminit_native.h> +#include <northbridge/intel/sandybridge/raminit.h> +#include <northbridge/intel/sandybridge/sandybridge.h> #include <southbridge/intel/bd82x6x/pch.h> #include <southbridge/intel/common/gpio.h> -#include <cpu/x86/msr.h> -#include <halt.h> #include <superio/winbond/common/winbond.h> +#include <timestamp.h> +
void pch_enable_lpc(void) {
-
Felix Held (Code Review) -
HAOUAS Elyes (Code Review)
-
Patrick Georgi (Code Review)
-
Paul Menzel (Code Review)