Kyösti Mälkki has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/37429 )
Change subject: bootblock: Provide some common prototypes ......................................................................
bootblock: Provide some common prototypes
The split of bootblock initialisation to cpu, northbridge and southbridge is not specific to intel at all.
Change-Id: I702cc6bad4afee4f61acf58b9155608b28eb417e Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com --- M src/cpu/intel/car/bootblock.c D src/cpu/intel/car/bootblock.h M src/cpu/intel/haswell/bootblock.c M src/cpu/intel/model_206ax/bootblock.c M src/include/bootblock_common.h M src/northbridge/intel/gm45/bootblock.c M src/northbridge/intel/haswell/bootblock.c M src/northbridge/intel/i945/bootblock.c M src/northbridge/intel/nehalem/bootblock.c M src/northbridge/intel/pineview/bootblock.c M src/northbridge/intel/sandybridge/bootblock.c M src/northbridge/intel/x4x/bootblock.c M src/soc/intel/baytrail/bootblock/bootblock.c M src/soc/intel/broadwell/bootblock/cpu.c M src/soc/intel/broadwell/bootblock/pch.c M src/soc/intel/broadwell/bootblock/systemagent.c M src/southbridge/intel/bd82x6x/bootblock.c M src/southbridge/intel/i82371eb/bootblock.c M src/southbridge/intel/i82801dx/bootblock.c M src/southbridge/intel/i82801gx/bootblock.c M src/southbridge/intel/i82801ix/bootblock.c M src/southbridge/intel/i82801jx/bootblock.c M src/southbridge/intel/ibexpeak/bootblock.c M src/southbridge/intel/lynxpoint/bootblock.c 24 files changed, 25 insertions(+), 43 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/29/37429/1
diff --git a/src/cpu/intel/car/bootblock.c b/src/cpu/intel/car/bootblock.c index 664c2b5..4435fa4 100644 --- a/src/cpu/intel/car/bootblock.c +++ b/src/cpu/intel/car/bootblock.c @@ -12,7 +12,6 @@ */
#include <bootblock_common.h> -#include <cpu/intel/car/bootblock.h> #include <cpu/x86/bist.h>
static uint32_t saved_bist; diff --git a/src/cpu/intel/car/bootblock.h b/src/cpu/intel/car/bootblock.h deleted file mode 100644 index 5adfd87..0000000 --- a/src/cpu/intel/car/bootblock.h +++ /dev/null @@ -1,21 +0,0 @@ -/* - * This file is part of the coreboot project. - * - * This program is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License as published by - * the Free Software Foundation; version 2 of the License. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - */ - -#ifndef _CPU_INTEL_CAR_BOOTBLOCK_H -#define _CPU_INTEL_CAR_BOOTBLOCK_H - -void bootblock_early_cpu_init(void); -void bootblock_early_northbridge_init(void); -void bootblock_early_southbridge_init(void); - -#endif diff --git a/src/cpu/intel/haswell/bootblock.c b/src/cpu/intel/haswell/bootblock.c index 94e5d36..3bc3922 100644 --- a/src/cpu/intel/haswell/bootblock.c +++ b/src/cpu/intel/haswell/bootblock.c @@ -13,6 +13,7 @@
#include <stdint.h> #include <arch/cpu.h> +#include <bootblock_common.h> #include <cpu/x86/msr.h> #include <cpu/x86/mtrr.h> #include <arch/io.h> @@ -21,7 +22,6 @@ #include "haswell.h"
#include <southbridge/intel/lynxpoint/pch.h> -#include <cpu/intel/car/bootblock.h>
static void set_flex_ratio_to_tdp_nominal(void) { diff --git a/src/cpu/intel/model_206ax/bootblock.c b/src/cpu/intel/model_206ax/bootblock.c index da0333f..ca679c1 100644 --- a/src/cpu/intel/model_206ax/bootblock.c +++ b/src/cpu/intel/model_206ax/bootblock.c @@ -13,10 +13,10 @@
#include <stdint.h> #include <arch/cpu.h> +#include <bootblock_common.h> #include <cpu/x86/msr.h> #include <arch/io.h> #include <halt.h> -#include <cpu/intel/car/bootblock.h>
#include "model_206ax.h"
diff --git a/src/include/bootblock_common.h b/src/include/bootblock_common.h index eb9c24c..09df3ba 100644 --- a/src/include/bootblock_common.h +++ b/src/include/bootblock_common.h @@ -32,6 +32,10 @@ void bootblock_soc_early_init(void); void bootblock_soc_init(void);
+void bootblock_early_cpu_init(void); +void bootblock_early_northbridge_init(void); +void bootblock_early_southbridge_init(void); + /* * C code entry point for the boot block. */ diff --git a/src/northbridge/intel/gm45/bootblock.c b/src/northbridge/intel/gm45/bootblock.c index d3aeb03..fda3aac 100644 --- a/src/northbridge/intel/gm45/bootblock.c +++ b/src/northbridge/intel/gm45/bootblock.c @@ -11,7 +11,7 @@ * GNU General Public License for more details. */
-#include <cpu/intel/car/bootblock.h> +#include <bootblock_common.h> #include <device/pci_ops.h>
/* Just re-define these instead of including gm45.h. It blows up romcc. */ diff --git a/src/northbridge/intel/haswell/bootblock.c b/src/northbridge/intel/haswell/bootblock.c index 2c1bd58..05dd306 100644 --- a/src/northbridge/intel/haswell/bootblock.c +++ b/src/northbridge/intel/haswell/bootblock.c @@ -11,8 +11,8 @@ * GNU General Public License for more details. */
+#include <bootblock_common.h> #include <device/pci_ops.h> -#include <cpu/intel/car/bootblock.h> #include "haswell.h"
void bootblock_early_northbridge_init(void) diff --git a/src/northbridge/intel/i945/bootblock.c b/src/northbridge/intel/i945/bootblock.c index e86abe5..5ca73fe 100644 --- a/src/northbridge/intel/i945/bootblock.c +++ b/src/northbridge/intel/i945/bootblock.c @@ -11,7 +11,7 @@ * GNU General Public License for more details. */
-#include <cpu/intel/car/bootblock.h> +#include <bootblock_common.h> #include <device/pci_ops.h> #include "i945.h"
diff --git a/src/northbridge/intel/nehalem/bootblock.c b/src/northbridge/intel/nehalem/bootblock.c index 46cdef0..3519349 100644 --- a/src/northbridge/intel/nehalem/bootblock.c +++ b/src/northbridge/intel/nehalem/bootblock.c @@ -11,8 +11,8 @@ * GNU General Public License for more details. */
+#include <bootblock_common.h> #include <device/pci_ops.h> -#include <cpu/intel/car/bootblock.h>
void bootblock_early_northbridge_init(void) { diff --git a/src/northbridge/intel/pineview/bootblock.c b/src/northbridge/intel/pineview/bootblock.c index bd510b0..b118521 100644 --- a/src/northbridge/intel/pineview/bootblock.c +++ b/src/northbridge/intel/pineview/bootblock.c @@ -11,8 +11,8 @@ * GNU General Public License for more details. */
+#include <bootblock_common.h> #include <device/pci_ops.h> -#include <cpu/intel/car/bootblock.h> #include "pineview.h"
#define MMCONF_256_BUSSES 16 diff --git a/src/northbridge/intel/sandybridge/bootblock.c b/src/northbridge/intel/sandybridge/bootblock.c index 40819bf..2d61f84 100644 --- a/src/northbridge/intel/sandybridge/bootblock.c +++ b/src/northbridge/intel/sandybridge/bootblock.c @@ -11,8 +11,8 @@ * GNU General Public License for more details. */
+#include <bootblock_common.h> #include <device/pci_ops.h> -#include <cpu/intel/car/bootblock.h> #include "sandybridge.h"
void bootblock_early_northbridge_init(void) diff --git a/src/northbridge/intel/x4x/bootblock.c b/src/northbridge/intel/x4x/bootblock.c index 64643dd..da48bf4 100644 --- a/src/northbridge/intel/x4x/bootblock.c +++ b/src/northbridge/intel/x4x/bootblock.c @@ -14,8 +14,8 @@ * GNU General Public License for more details. */
+#include <bootblock_common.h> #include <device/pci_ops.h> -#include <cpu/intel/car/bootblock.h> #include "x4x.h" #include "iomap.h"
diff --git a/src/soc/intel/baytrail/bootblock/bootblock.c b/src/soc/intel/baytrail/bootblock/bootblock.c index 1c5bfc5..5cdbaca 100644 --- a/src/soc/intel/baytrail/bootblock/bootblock.c +++ b/src/soc/intel/baytrail/bootblock/bootblock.c @@ -13,7 +13,7 @@ * GNU General Public License for more details. */
-#include <cpu/intel/car/bootblock.h> +#include <bootblock_common.h> #include <device/pci_ops.h> #include <soc/iosf.h> #include <soc/iomap.h> diff --git a/src/soc/intel/broadwell/bootblock/cpu.c b/src/soc/intel/broadwell/bootblock/cpu.c index f3c35f3..5ad5f37 100644 --- a/src/soc/intel/broadwell/bootblock/cpu.c +++ b/src/soc/intel/broadwell/bootblock/cpu.c @@ -18,11 +18,11 @@ #include <cpu/x86/msr.h> #include <cpu/x86/mtrr.h> #include <arch/io.h> +#include <bootblock_common.h> #include <halt.h> #include <soc/rcba.h> #include <soc/msr.h> #include <delay.h> -#include <cpu/intel/car/bootblock.h>
static void set_flex_ratio_to_tdp_nominal(void) { diff --git a/src/soc/intel/broadwell/bootblock/pch.c b/src/soc/intel/broadwell/bootblock/pch.c index 590961b..7d0c5be 100644 --- a/src/soc/intel/broadwell/bootblock/pch.c +++ b/src/soc/intel/broadwell/bootblock/pch.c @@ -22,7 +22,7 @@ #include <reg_script.h> #include <soc/pm.h> #include <soc/romstage.h> -#include <cpu/intel/car/bootblock.h> +#include <bootblock_common.h>
/* * Enable Prefetching and Caching. diff --git a/src/soc/intel/broadwell/bootblock/systemagent.c b/src/soc/intel/broadwell/bootblock/systemagent.c index 7aaed78..41b12d9 100644 --- a/src/soc/intel/broadwell/bootblock/systemagent.c +++ b/src/soc/intel/broadwell/bootblock/systemagent.c @@ -13,10 +13,10 @@ * GNU General Public License for more details. */
+#include <bootblock_common.h> #include <device/pci_ops.h> #include <soc/pci_devs.h> #include <soc/systemagent.h> -#include <cpu/intel/car/bootblock.h>
void bootblock_early_northbridge_init(void) { diff --git a/src/southbridge/intel/bd82x6x/bootblock.c b/src/southbridge/intel/bd82x6x/bootblock.c index 1a8242f..bfa5b42 100644 --- a/src/southbridge/intel/bd82x6x/bootblock.c +++ b/src/southbridge/intel/bd82x6x/bootblock.c @@ -13,7 +13,7 @@ * GNU General Public License for more details. */
-#include <cpu/intel/car/bootblock.h> +#include <bootblock_common.h> #include <device/pci_ops.h> #include "pch.h"
diff --git a/src/southbridge/intel/i82371eb/bootblock.c b/src/southbridge/intel/i82371eb/bootblock.c index a6d62e0..0cc6c64 100644 --- a/src/southbridge/intel/i82371eb/bootblock.c +++ b/src/southbridge/intel/i82371eb/bootblock.c @@ -15,10 +15,10 @@ */
#include <stdint.h> +#include <bootblock_common.h> #include <device/pci_ops.h> #include <device/pci_ids.h> #include <device/pci_type.h> -#include <cpu/intel/car/bootblock.h> #include "i82371eb.h"
#define PCI_ID(VENDOR_ID, DEVICE_ID) \ diff --git a/src/southbridge/intel/i82801dx/bootblock.c b/src/southbridge/intel/i82801dx/bootblock.c index ef34855..e427f4b 100644 --- a/src/southbridge/intel/i82801dx/bootblock.c +++ b/src/southbridge/intel/i82801dx/bootblock.c @@ -11,7 +11,7 @@ * GNU General Public License for more details. */
-#include <cpu/intel/car/bootblock.h> +#include <bootblock_common.h> #include <device/pci_ops.h>
void bootblock_early_southbridge_init(void) diff --git a/src/southbridge/intel/i82801gx/bootblock.c b/src/southbridge/intel/i82801gx/bootblock.c index 4c464ff..34c85ea 100644 --- a/src/southbridge/intel/i82801gx/bootblock.c +++ b/src/southbridge/intel/i82801gx/bootblock.c @@ -13,8 +13,8 @@ * GNU General Public License for more details. */
+#include <bootblock_common.h> #include <device/pci_ops.h> -#include <cpu/intel/car/bootblock.h> #include "i82801gx.h"
static void enable_spi_prefetch(void) diff --git a/src/southbridge/intel/i82801ix/bootblock.c b/src/southbridge/intel/i82801ix/bootblock.c index 0b50d61..6707766 100644 --- a/src/southbridge/intel/i82801ix/bootblock.c +++ b/src/southbridge/intel/i82801ix/bootblock.c @@ -13,8 +13,8 @@ * GNU General Public License for more details. */
+#include <bootblock_common.h> #include <device/pci_ops.h> -#include <cpu/intel/car/bootblock.h> #include "i82801ix.h"
diff --git a/src/southbridge/intel/i82801jx/bootblock.c b/src/southbridge/intel/i82801jx/bootblock.c index b601679..06b1a8a 100644 --- a/src/southbridge/intel/i82801jx/bootblock.c +++ b/src/southbridge/intel/i82801jx/bootblock.c @@ -13,8 +13,8 @@ * GNU General Public License for more details. */
+#include <bootblock_common.h> #include <device/pci_ops.h> -#include <cpu/intel/car/bootblock.h> #include "i82801jx.h"
static void enable_spi_prefetch(void) diff --git a/src/southbridge/intel/ibexpeak/bootblock.c b/src/southbridge/intel/ibexpeak/bootblock.c index c8b1d6e..12f1010 100644 --- a/src/southbridge/intel/ibexpeak/bootblock.c +++ b/src/southbridge/intel/ibexpeak/bootblock.c @@ -13,8 +13,8 @@ * GNU General Public License for more details. */
+#include <bootblock_common.h> #include <device/pci_ops.h> -#include <cpu/intel/car/bootblock.h> #include "pch.h" #include "chip.h"
diff --git a/src/southbridge/intel/lynxpoint/bootblock.c b/src/southbridge/intel/lynxpoint/bootblock.c index 39e6925..becf47f 100644 --- a/src/southbridge/intel/lynxpoint/bootblock.c +++ b/src/southbridge/intel/lynxpoint/bootblock.c @@ -13,8 +13,8 @@ * GNU General Public License for more details. */
+#include <bootblock_common.h> #include <device/pci_ops.h> -#include <cpu/intel/car/bootblock.h> #include "pch.h"
/*
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37429 )
Change subject: bootblock: Provide some common prototypes ......................................................................
Patch Set 1: Code-Review+1
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37429 )
Change subject: bootblock: Provide some common prototypes ......................................................................
Patch Set 1: Code-Review+1
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37429 )
Change subject: bootblock: Provide some common prototypes ......................................................................
Patch Set 1: Code-Review+2
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37429 )
Change subject: bootblock: Provide some common prototypes ......................................................................
Patch Set 1: Code-Review+2
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37429 )
Change subject: bootblock: Provide some common prototypes ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/37429/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/37429/1//COMMIT_MSG@10 PS1, Line 10: southbridge is not specific to intel at all. I think these are defined in intel/car.h because the code calling them is in intel/car.c? So I don't really see a point moving them to bootblock_common.h unless you move the whole bootblock_soc_init() from car.c somewhere else as well.
Northbridge/southbridge are still x86-specific, so if you want to use this on a non-Intel board, maybe move it into arch/x86 or cpu/x86?
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37429 )
Change subject: bootblock: Provide some common prototypes ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/37429/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/37429/1//COMMIT_MSG@10 PS1, Line 10: southbridge is not specific to intel at all.
I think these are defined in intel/car.h because the code calling them is in intel/car. […]
For AMD we could reuse bootblock_early_southbridge_init() name, not sure if we need the rest.
Looking at lib/bootblock.c, it already has parts some of which are specific to ARM-only (pre-bootbock timestamps stuff) and some x86-only (pc80/rtc and cmos).
There is also CB:36711 adding yet more prototypes and weak implementations for Intel side.
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37429 )
Change subject: bootblock: Provide some common prototypes ......................................................................
Patch Set 2:
Due the discussions, let's move this just before CB:37440, it's not needed until that.
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37429 )
Change subject: bootblock: Provide some common prototypes ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/37429/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/37429/1//COMMIT_MSG@10 PS1, Line 10: southbridge is not specific to intel at all.
For AMD we could reuse bootblock_early_southbridge_init() name, not sure if we need the rest. […]
I think stuff should go in lib/bootblock if it's "all but x86" code (e.g. decompressor support), but it can go in arch/x86 if it's x86-only code.
For the pc80/cmos stuff, I agree, I'd prefer to see that moved somewhere else too (we could create a bootblock_arch_early_init() or something...).
HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37429 )
Change subject: bootblock: Provide some common prototypes ......................................................................
Patch Set 9: Code-Review+1
Hello Patrick Rudolph, HAOUAS Elyes, Angel Pons, Arthur Heymans, Patrick Rudolph, Paul Menzel, build bot (Jenkins), Damien Zammit,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/37429
to look at the new patch set (#10).
Change subject: bootblock: Provide some common prototypes ......................................................................
bootblock: Provide some common prototypes
The split of bootblock initialisation to cpu, northbridge and southbridge is not specific to intel at all, create new header <arch/bootblock.h> as AMD will want some of these too.
Change-Id: I702cc6bad4afee4f61acf58b9155608b28eb417e Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com --- R src/arch/x86/include/arch/bootblock.h M src/cpu/intel/car/bootblock.c M src/cpu/intel/haswell/bootblock.c M src/cpu/intel/model_206ax/bootblock.c M src/northbridge/intel/gm45/bootblock.c M src/northbridge/intel/haswell/bootblock.c M src/northbridge/intel/i945/bootblock.c M src/northbridge/intel/nehalem/bootblock.c M src/northbridge/intel/pineview/bootblock.c M src/northbridge/intel/sandybridge/bootblock.c M src/northbridge/intel/x4x/bootblock.c M src/soc/intel/baytrail/bootblock/bootblock.c M src/soc/intel/broadwell/bootblock/cpu.c M src/soc/intel/broadwell/bootblock/pch.c M src/soc/intel/broadwell/bootblock/systemagent.c M src/southbridge/intel/bd82x6x/bootblock.c M src/southbridge/intel/i82371eb/bootblock.c M src/southbridge/intel/i82801dx/bootblock.c M src/southbridge/intel/i82801gx/bootblock.c M src/southbridge/intel/i82801ix/bootblock.c M src/southbridge/intel/i82801jx/bootblock.c M src/southbridge/intel/ibexpeak/bootblock.c M src/southbridge/intel/lynxpoint/bootblock.c 23 files changed, 25 insertions(+), 25 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/29/37429/10
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37429 )
Change subject: bootblock: Provide some common prototypes ......................................................................
Patch Set 10:
(1 comment)
https://review.coreboot.org/c/coreboot/+/37429/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/37429/1//COMMIT_MSG@10 PS1, Line 10: southbridge is not specific to intel at all.
I think stuff should go in lib/bootblock if it's "all but x86" code (e.g. […]
Done
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37429 )
Change subject: bootblock: Provide some common prototypes ......................................................................
Patch Set 10: Code-Review+2
-
Angel Pons (Code Review) -
Arthur Heymans (Code Review)
-
HAOUAS Elyes (Code Review)
-
Julius Werner (Code Review)
-
Kyösti Mälkki (Code Review)
-
Patrick Rudolph (Code Review)
-
Paul Menzel (Code Review)