Kyösti Mälkki has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/35012 )
Change subject: samsung/exynos,qualcomm/ipq: Avoid conflicting DRAM_START ......................................................................
samsung/exynos,qualcomm/ipq: Avoid conflicting DRAM_START
There is duplicate definition in <memlayout.h> that gets indirectly included with followup work.
Change-Id: I1c41052542b215de8af3be858ca7a229e4206ecf Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com --- M src/mainboard/google/daisy/mainboard.c M src/mainboard/google/gale/mmu.c M src/mainboard/google/peach_pit/mainboard.c M src/mainboard/google/storm/mmu.c 4 files changed, 12 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/12/35012/1
diff --git a/src/mainboard/google/daisy/mainboard.c b/src/mainboard/google/daisy/mainboard.c index 46939b7..0a1701c 100644 --- a/src/mainboard/google/daisy/mainboard.c +++ b/src/mainboard/google/daisy/mainboard.c @@ -37,6 +37,9 @@
#define MMC0_GPIO_PIN (58)
+/* Conflicting definition from <memlayout.h> we do not need here. */ +#undef DRAM_START + /* convenient shorthand (in MB) */ #define DRAM_START ((uintptr_t)_dram/MiB) #define DRAM_SIZE CONFIG_DRAM_SIZE_MB diff --git a/src/mainboard/google/gale/mmu.c b/src/mainboard/google/gale/mmu.c index bf46f7a..d178295 100644 --- a/src/mainboard/google/gale/mmu.c +++ b/src/mainboard/google/gale/mmu.c @@ -15,6 +15,9 @@ #include <soc/soc_services.h> #include "mmu.h"
+/* Conflicting definition from <memlayout.h> we do not need here. */ +#undef DRAM_START + #define WIFI_IMEM_0_START ((uintptr_t)_wifi_imem_0 / KiB) #define WIFI_IMEM_0_END ((uintptr_t)_ewifi_imem_0 / KiB) #define WIFI_IMEM_1_START ((uintptr_t)_wifi_imem_1 / KiB) diff --git a/src/mainboard/google/peach_pit/mainboard.c b/src/mainboard/google/peach_pit/mainboard.c index ecd2260..86d18a0 100644 --- a/src/mainboard/google/peach_pit/mainboard.c +++ b/src/mainboard/google/peach_pit/mainboard.c @@ -38,6 +38,9 @@ #include <symbols.h> #include <vbe.h>
+/* Conflicting definition from <memlayout.h> we do not need here. */ +#undef DRAM_START + /* convenient shorthand (in MB) */ #define DRAM_START ((uintptr_t)_dram/MiB) #define DRAM_SIZE CONFIG_DRAM_SIZE_MB diff --git a/src/mainboard/google/storm/mmu.c b/src/mainboard/google/storm/mmu.c index 9750cc1..91976e7 100644 --- a/src/mainboard/google/storm/mmu.c +++ b/src/mainboard/google/storm/mmu.c @@ -15,6 +15,9 @@ #include <symbols.h> #include "mmu.h"
+/* Conflicting definition from <memlayout.h> we do not need here. */ +#undef DRAM_START + /* convenient shorthand (in MB) */ #define RPM_START ((uintptr_t)_rpm / KiB) #define RPM_END ((uintptr_t)_erpm / KiB)
Kyösti Mälkki has uploaded a new patch set (#2). ( https://review.coreboot.org/c/coreboot/+/35012 )
Change subject: samsung/exynos,qualcomm/ipq: Avoid conflicting DRAM_START ......................................................................
samsung/exynos,qualcomm/ipq: Avoid conflicting DRAM_START
There is duplicate definition in <memlayout.h> that gets indirectly included with followup work.
Change-Id: I1c41052542b215de8af3be858ca7a229e4206ecf Signed-off-by: Kyösti Mälkki kyosti.malkki@gmail.com --- M src/mainboard/google/daisy/mainboard.c M src/mainboard/google/gale/mmu.c M src/mainboard/google/peach_pit/mainboard.c M src/mainboard/google/storm/mmu.c M src/soc/samsung/exynos5420/bootblock.c 5 files changed, 18 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/12/35012/2
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35012 )
Change subject: samsung/exynos,qualcomm/ipq: Avoid conflicting DRAM_START ......................................................................
Patch Set 2:
Is this just because you include stddef.h from memlayout.h? I don't think we should do that, I think we should rather move those ARCH_STAGE_xxx definitions from memlayout.h into rules.h. memlayout.h was only ever meant to be the header included directly by memlayout.ld files and defining the memlayout "language", not a general collection of program state macros (that should be rules.h).
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35012 )
Change subject: samsung/exynos,qualcomm/ipq: Avoid conflicting DRAM_START ......................................................................
Patch Set 2:
Patch Set 2:
Is this just because you include stddef.h from memlayout.h? I don't think we should do that, I think we should rather move those ARCH_STAGE_xxx definitions from memlayout.h into rules.h. memlayout.h was only ever meant to be the header included directly by memlayout.ld files and defining the memlayout "language", not a general collection of program state macros (that should be rules.h).
I think that would mean I have to include <arch/memlayout.h> in the middle of rules.h to make it work. Is that the preferred route to take?
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35012 )
Change subject: samsung/exynos,qualcomm/ipq: Avoid conflicting DRAM_START ......................................................................
Patch Set 2:
I think that would mean I have to include <arch/memlayout.h> in the middle of rules.h to make it work. Is that the preferred route to take?
No, I think we should decouple this fully from memlayout. Either rules.h should directly check for CONFIG(ARCH_X86) (since I think that's the only arch-specific difference we have, that may be easiest for now), or we should add <arch/rules.h> files.
Kyösti Mälkki has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/35012 )
Change subject: samsung/exynos,qualcomm/ipq: Avoid conflicting DRAM_START ......................................................................
Abandoned
Took better approach with CB:35014
-
Julius Werner (Code Review) -
Kyösti Mälkki (Code Review)