Change in coreboot[master]: AGESA,binaryPI: Remove early_all_cores()

[Kyösti Mälkki (Code Review)]

Kyösti Mälkki has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/37203 )

Change subject: AGESA,binaryPI: Remove early_all_cores()
......................................................................

AGESA,binaryPI: Remove early_all_cores()

This was implemented to make sure it gets called before
attempting any PCI MMIO access. Now that we have one
central romstage_main() implementation this extra precaution
is no longer useful.

Change-Id: I09b24da827e00d7a9ba0a51d5eef36f174b893a6
Signed-off-by: Kyösti Mälkki 
---
M src/drivers/amd/agesa/cache_as_ram.S
M src/drivers/amd/agesa/romstage.c
2 files changed, 3 insertions(+), 11 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/03/37203/1

diff --git a/src/drivers/amd/agesa/cache_as_ram.S b/src/drivers/amd/agesa/cache_as_ram.S
index 3f1358a..e3e5735 100644
--- a/src/drivers/amd/agesa/cache_as_ram.S
+++ b/src/drivers/amd/agesa/cache_as_ram.S
@@ -94,8 +94,6 @@

#endif

-	call early_all_cores
-
    /* Must maintain 16-byte stack alignment here. */
    pushl	$0x0
    pushl	$0x0
diff --git a/src/drivers/amd/agesa/romstage.c b/src/drivers/amd/agesa/romstage.c
index 72aac3e..0ecfeb2 100644
--- a/src/drivers/amd/agesa/romstage.c
+++ b/src/drivers/amd/agesa/romstage.c
@@ -27,15 +27,6 @@
 #include <northbridge/amd/agesa/agesa_helper.h>
 #include <northbridge/amd/agesa/state_machine.h>

-#if !CONFIG(POSTCAR_STAGE)
-#error "Only POSTCAR_STAGE is supported."
-#endif
-
-void asmlinkage early_all_cores(void)
-{
-	amd_initmmio();
-}
-
 void __weak platform_once(struct sysinfo *cb)
 {
    board_BeforeAgesa(cb);
@@ -57,6 +48,9 @@
    u8 initial_apic_id = (u8) (cpuid_ebx(1) >> 24);
    int cbmem_initted = 0;

+	/* Enable PCI MMIO configuration. */
+	amd_initmmio();
+
    fill_sysinfo(cb);

if ((initial_apic_id == 0) && boot_cpu()) {

[Michał Żygowski (Code Review)]

Michał Żygowski has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37203 )

Change subject: AGESA,binaryPI: Remove early_all_cores()
......................................................................

Patch Set 1:

Kyösti, I will rebase on this change instead

[Kyösti Mälkki (Code Review)]

Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37203 )

Change subject: AGESA,binaryPI: Remove early_all_cores()
......................................................................

Patch Set 1:
> Patch Set 1:
>
> Kyösti, I will rebase on this change instead

Excellent

[Kyösti Mälkki (Code Review)]

Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37203 )

Change subject: AGESA,binaryPI: Remove early_all_cores()
......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/c/coreboot/+/37203/1/src/drivers/amd/agesa/romst... 
File src/drivers/amd/agesa/romstage.c:

https://review.coreboot.org/c/coreboot/+/37203/1/src/drivers/amd/agesa/romst... 
PS1, Line 34: void asmlinkage early_all_cores(void)
I forgot the prototype in amd/car.h

[Michał Żygowski (Code Review)]

Michał Żygowski has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37203 )

Change subject: AGESA,binaryPI: Remove early_all_cores()
......................................................................

Patch Set 1: Code-Review+1

As there seem to be a merge conflict, +1 for now

[Michał Żygowski (Code Review)]

Michał Żygowski has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37203 )

Change subject: AGESA,binaryPI: Remove early_all_cores()
......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/c/coreboot/+/37203/1/src/drivers/amd/agesa/romst... 
File src/drivers/amd/agesa/romstage.c:

https://review.coreboot.org/c/coreboot/+/37203/1/src/drivers/amd/agesa/romst... 
PS1, Line 34: void asmlinkage early_all_cores(void)
> I forgot the prototype in amd/car. […]

Right, waiting with rebase few mins

[Kyösti Mälkki (Code Review)]

Hello build bot (Jenkins), Michał Żygowski,

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

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

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

Change subject: AGESA,binaryPI: Remove early_all_cores()
......................................................................

AGESA,binaryPI: Remove early_all_cores()

This was implemented to make sure it gets called before
attempting any PCI MMIO access. Now that we have one
central romstage_main() implementation this extra precaution
is no longer useful.

Change-Id: I09b24da827e00d7a9ba0a51d5eef36f174b893a6
Signed-off-by: Kyösti Mälkki 
---
M src/drivers/amd/agesa/cache_as_ram.S
M src/drivers/amd/agesa/romstage.c
M src/include/cpu/amd/car.h
3 files changed, 3 insertions(+), 13 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/03/37203/2

[Kyösti Mälkki (Code Review)]

Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37203 )

Change subject: AGESA,binaryPI: Remove early_all_cores()
......................................................................

Patch Set 2:

(1 comment)

https://review.coreboot.org/c/coreboot/+/37203/1/src/drivers/amd/agesa/romst... 
File src/drivers/amd/agesa/romstage.c:

https://review.coreboot.org/c/coreboot/+/37203/1/src/drivers/amd/agesa/romst... 
PS1, Line 34: void asmlinkage early_all_cores(void)
> Right, waiting with rebase few mins

Done, I hope I did not mess it up now with CB:37235

[Arthur Heymans (Code Review)]

Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37203 )

Change subject: AGESA,binaryPI: Remove early_all_cores()
......................................................................

Patch Set 2: Code-Review+2

[Michał Żygowski (Code Review)]

Michał Żygowski has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37203 )

Change subject: AGESA,binaryPI: Remove early_all_cores()
......................................................................

Patch Set 2:

Now when CB:37235 is merged, a rebase should resolve the merge conflicts

[Kyösti Mälkki (Code Review)]

Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37203 )

Change subject: AGESA,binaryPI: Remove early_all_cores()
......................................................................

Patch Set 2:

(1 comment)

https://review.coreboot.org/c/coreboot/+/37203/1/src/drivers/amd/agesa/romst... 
File src/drivers/amd/agesa/romstage.c:

https://review.coreboot.org/c/coreboot/+/37203/1/src/drivers/amd/agesa/romst... 
PS1, Line 34: void asmlinkage early_all_cores(void)
> Done, I hope I did not mess it up now with CB:37235

Done

[Michał Żygowski (Code Review)]

Michał Żygowski has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37203 )

Change subject: AGESA,binaryPI: Remove early_all_cores()
......................................................................

Patch Set 2: Code-Review+2

[Kyösti Mälkki (Code Review)]

Kyösti Mälkki has submitted this change. ( https://review.coreboot.org/c/coreboot/+/37203 )

Change subject: AGESA,binaryPI: Remove early_all_cores()
......................................................................

AGESA,binaryPI: Remove early_all_cores()

This was implemented to make sure it gets called before
attempting any PCI MMIO access. Now that we have one
central romstage_main() implementation this extra precaution
is no longer useful.

Change-Id: I09b24da827e00d7a9ba0a51d5eef36f174b893a6
Signed-off-by: Kyösti Mälkki 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/37203
Tested-by: build bot (Jenkins) 
Reviewed-by: Arthur Heymans 
Reviewed-by: Michał Żygowski 
---
M src/drivers/amd/agesa/cache_as_ram.S
M src/drivers/amd/agesa/romstage.c
M src/include/cpu/amd/car.h
3 files changed, 3 insertions(+), 13 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Arthur Heymans: Looks good to me, approved
  Michał Żygowski: Looks good to me, approved

diff --git a/src/drivers/amd/agesa/cache_as_ram.S b/src/drivers/amd/agesa/cache_as_ram.S
index 3f1358a..e3e5735 100644
--- a/src/drivers/amd/agesa/cache_as_ram.S
+++ b/src/drivers/amd/agesa/cache_as_ram.S
@@ -94,8 +94,6 @@

#endif

-	call early_all_cores
-
    /* Must maintain 16-byte stack alignment here. */
    pushl	$0x0
    pushl	$0x0
diff --git a/src/drivers/amd/agesa/romstage.c b/src/drivers/amd/agesa/romstage.c
index 72aac3e..0ecfeb2 100644
--- a/src/drivers/amd/agesa/romstage.c
+++ b/src/drivers/amd/agesa/romstage.c
@@ -27,15 +27,6 @@
 #include <northbridge/amd/agesa/agesa_helper.h>
 #include <northbridge/amd/agesa/state_machine.h>

-#if !CONFIG(POSTCAR_STAGE)
-#error "Only POSTCAR_STAGE is supported."
-#endif
-
-void asmlinkage early_all_cores(void)
-{
-	amd_initmmio();
-}
-
 void __weak platform_once(struct sysinfo *cb)
 {
    board_BeforeAgesa(cb);
@@ -57,6 +48,9 @@
    u8 initial_apic_id = (u8) (cpuid_ebx(1) >> 24);
    int cbmem_initted = 0;

+	/* Enable PCI MMIO configuration. */
+	amd_initmmio();
+
    fill_sysinfo(cb);

if ((initial_apic_id == 0) && boot_cpu()) {
diff --git a/src/include/cpu/amd/car.h b/src/include/cpu/amd/car.h
index 59f5bb9..46f7e1d 100644
--- a/src/include/cpu/amd/car.h
+++ b/src/include/cpu/amd/car.h
@@ -5,8 +5,6 @@

void cache_as_ram_main(unsigned long bist, unsigned long cpu_init_detectedx);

-void asmlinkage early_all_cores(void);
-
 void *asmlinkage romstage_main(unsigned long bist);

#endif