HAOUAS Elyes has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/44209 )
Change subject: soc/samsung: Add missing <commonlib/helpers.h> ......................................................................
soc/samsung: Add missing <commonlib/helpers.h>
Use of 'KiB' needs <commonlib/helpers.h> header.
Change-Id: If506786c80644e12549798ce5f7f6f5b8e4af348 Signed-off-by: Elyes HAOUAS ehaouas@noos.fr --- M src/soc/samsung/exynos5250/include/soc/cpu.h M src/soc/samsung/exynos5420/include/soc/cpu.h 2 files changed, 2 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/09/44209/1
diff --git a/src/soc/samsung/exynos5250/include/soc/cpu.h b/src/soc/samsung/exynos5250/include/soc/cpu.h index aa5a2e8..3c2cd77 100644 --- a/src/soc/samsung/exynos5250/include/soc/cpu.h +++ b/src/soc/samsung/exynos5250/include/soc/cpu.h @@ -3,6 +3,7 @@ #ifndef CPU_SAMSUNG_EXYNOS5250_CPU_H #define CPU_SAMSUNG_EXYNOS5250_CPU_H
+#include <commonlib/helpers.h> #include <symbols.h>
/* Base address registers */ diff --git a/src/soc/samsung/exynos5420/include/soc/cpu.h b/src/soc/samsung/exynos5420/include/soc/cpu.h index 6a22770..bddfd86 100644 --- a/src/soc/samsung/exynos5420/include/soc/cpu.h +++ b/src/soc/samsung/exynos5420/include/soc/cpu.h @@ -3,6 +3,7 @@ #ifndef CPU_SAMSUNG_EXYNOS5420_CPU_H #define CPU_SAMSUNG_EXYNOS5420_CPU_H
+#include <commonlib/helpers.h> #include <symbols.h>
#define EXYNOS5_SRAM_BASE 0x02020000
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44209 )
Change subject: soc/samsung: Add missing <commonlib/helpers.h> ......................................................................
Patch Set 1: Code-Review+2
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44209 )
Change subject: soc/samsung: Add missing <commonlib/helpers.h> ......................................................................
Patch Set 1:
I would prefer we always include <types.h> for this sort of stuff. commonlib is an implementation detail, we shouldn't have that complicated web of header directories reflected in every source file.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44209 )
Change subject: soc/samsung: Add missing <commonlib/helpers.h> ......................................................................
Patch Set 1:
Patch Set 1:
I would prefer we always include <types.h> for this sort of stuff. commonlib is an implementation detail, we shouldn't have that complicated web of header directories reflected in every source file.
Do you mean relocating definitions from commonlib/bsd/helpers.h or including commonlib/helpers.h through types.h ?
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44209 )
Change subject: soc/samsung: Add missing <commonlib/helpers.h> ......................................................................
Patch Set 1:
Do you mean relocating definitions from commonlib/bsd/helpers.h or including commonlib/helpers.h through types.h ?
No I mean just putting #include <types.h> here (which chain-includes that stuff). Those definitions are in commonlib for a reason, but those reasons are complicated (licensing and code sharing stuff) and I don't think every random source file in coreboot should be bothered by them (especially since they may change in the future). I think it would be much easier for all of us (and help keep attention focused on the important stuff) to just use types.h as a catchall header for all the random type and helper stuff.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44209 )
Change subject: soc/samsung: Add missing <commonlib/helpers.h> ......................................................................
Patch Set 1:
Patch Set 1:
Do you mean relocating definitions from commonlib/bsd/helpers.h or including commonlib/helpers.h through types.h ?
No I mean just putting #include <types.h> here (which chain-includes that stuff). Those definitions are in commonlib for a reason, but those reasons are complicated (licensing and code sharing stuff) and I don't think every random source file in coreboot should be bothered by them (especially since they may change in the future). I think it would be much easier for all of us (and help keep attention focused on the important stuff) to just use types.h as a catchall header for all the random type and helper stuff.
Hrm, I just checked and types.h doesn't chain-include that...
HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44209 )
Change subject: soc/samsung: Add missing <commonlib/helpers.h> ......................................................................
Patch Set 1:
Patch Set 1:
Do you mean relocating definitions from commonlib/bsd/helpers.h or including commonlib/helpers.h through types.h ?
No I mean just putting #include <types.h> here (which chain-includes that stuff). Those definitions are in commonlib for a reason, but those reasons are complicated (licensing and code sharing stuff) and I don't think every random source file in coreboot should be bothered by them (especially since they may change in the future). I think it would be much easier for all of us (and help keep attention focused on the important stuff) to just use types.h as a catchall header for all the random type and helper stuff.
Julius, I got a problem with <types.h> here : https://qa.coreboot.org/job/coreboot-gerrit/136626/ would you please have a look at it when you have time? Thx
Angel Pons has removed a vote from this change. ( https://review.coreboot.org/c/coreboot/+/44209 )
Change subject: soc/samsung: Add missing <commonlib/helpers.h> ......................................................................
Removed Code-Review+2 by Angel Pons th3fanbus@gmail.com
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44209 )
Change subject: soc/samsung: Add missing <commonlib/helpers.h> ......................................................................
Patch Set 1:
Hrm, I just checked and types.h doesn't chain-include that...
It chain-includes <stddef.h> which chain-includes <commonlib/helpers.h> which chain-includes <commonlib/bsd/helpers.h>.
Julius, I got a problem with <types.h> here : https://qa.coreboot.org/job/coreboot-gerrit/136626/ would you please have a look at it when you have time? Thx
Sorry, looks like those results are too old and Jenkins deleted them already? If you rebase to kick off a new run I can take a look (or just explain what the problem was?).
HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/44209 )
Change subject: soc/samsung: Add missing <commonlib/helpers.h> ......................................................................
Patch Set 1:
Patch Set 1:
Hrm, I just checked and types.h doesn't chain-include that...
It chain-includes <stddef.h> which chain-includes <commonlib/helpers.h> which chain-includes <commonlib/bsd/helpers.h>.
Julius, I got a problem with <types.h> here : https://qa.coreboot.org/job/coreboot-gerrit/136626/ would you please have a look at it when you have time? Thx
Sorry, looks like those results are too old and Jenkins deleted them already? If you rebase to kick off a new run I can take a look (or just explain what the problem was?).
Thank you. here we go: https://review.coreboot.org/c/coreboot/+/43967/3/src/drivers/intel/fsp2_0/in...
HAOUAS Elyes has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/44209 )
Change subject: soc/samsung: Add missing <commonlib/helpers.h> ......................................................................
Abandoned
-
Angel Pons (Code Review) -
HAOUAS Elyes (Code Review)
-
Julius Werner (Code Review)