Skip to contentSkip to conversation

ARM: lib: Remove CONFIG_ARCH_CPU_INIT dependency

PATCH New Fabio Estevam 7 participants 18 messages Mar 1, 2012 – Jul 19, 2012 Arm (32-bit)

lore
Share & tools
lore

Fabio Estevam

author
#0 lore raw

PATCH 1/3 ARM: lib: Remove CONFIG_ARCH_CPU_INIT dependency

Create a weak-aliased arch_cpu_init, so that we can get rid of CONFIG_ARCH_CPU_INIT and always call arch_cpu_init.

This way we do not need to define CONFIG_ARCH_CPU_INIT in every board file, since arch_cpu_init() is supposed to handle common CPU level code.

Signed-off-by Fabio Estevam <[email protected]>
---
 arch/arm/lib/board.c |   10 ++++++++--
 1 files changed, 8 insertions(+), 2 deletions(-)
arch/arm/lib/board.c +8 -2
index 500e216..6463db5 100644
@@ -224,10 +224,16 @@ void __dram_init_banksize(void)
 void dram_init_banksize(void)
 	__attribute__((weak, alias("__dram_init_banksize")));
 
+int __arch_cpu_init(void)
+{
+	return 0;
+}
+int arch_cpu_init(void)
+	__attribute__((weak, alias("__arch_cpu_init")));
+
 init_fnc_t *init_sequence[] = {
-#if defined(CONFIG_ARCH_CPU_INIT)
 	arch_cpu_init,		/* basic arch cpu dependent setup */
-#endif
+
 #if defined(CONFIG_BOARD_EARLY_INIT_F)
 	board_early_init_f,
 #endif
Signature · 1 line
1.7.1
author
#1 lore raw

PATCH 2/3 ARM: mx6: Remove CONFIG_ARCH_CPU_INIT

No need to define CONFIG_ARCH_CPU_INIT.

All mx6 based boards should use arch_cpu_init().

Signed-off-by Fabio Estevam <[email protected]>
---
 arch/arm/cpu/armv7/mx6/soc.c    |    2 --
 include/configs/mx6qarm2.h      |    1 -
 include/configs/mx6qsabrelite.h |    1 -
 3 files changed, 0 insertions(+), 4 deletions(-)
arch/arm/cpu/armv7/mx6/soc.c +0 -2
index 2ac74b5..2d07099 100644
@@ -37,7 +37,6 @@ u32 get_cpu_rev(void)
 	return system_rev;
 }
 
-#ifdef CONFIG_ARCH_CPU_INIT
 void init_aips(void)
 {
 	struct aipstz_regs *aips1, *aips2;

@@ -77,7 +76,6 @@ int arch_cpu_init(void)
 
 	return 0;
 }
-#endif
 
 #if defined(CONFIG_FEC_MXC)
 void imx_get_mac_from_fuse(int dev_id, unsigned char *mac)
include/configs/mx6qarm2.h +0 -1
index 0962d3c..87ff0a3 100644
@@ -37,7 +37,6 @@
 /* Size of malloc() pool */
 #define CONFIG_SYS_MALLOC_LEN		(CONFIG_ENV_SIZE + 2 * 1024 * 1024)
 
-#define CONFIG_ARCH_CPU_INIT
 #define CONFIG_BOARD_EARLY_INIT_F
 #define CONFIG_MXC_GPIO
include/configs/mx6qsabrelite.h +0 -1
index 93000f0..85e6570 100644
@@ -37,7 +37,6 @@
 /* Size of malloc() pool */
 #define CONFIG_SYS_MALLOC_LEN          (CONFIG_ENV_SIZE + 2 * 1024 * 1024)
 
-#define CONFIG_ARCH_CPU_INIT
 #define CONFIG_BOARD_EARLY_INIT_F
 #define CONFIG_MXC_GPIO
Signature · 1 line
1.7.1
#2 lore raw

PATCH 2/3 ARM: mx6: Remove CONFIG_ARCH_CPU_INIT

Quoted 4 lines

No need to define CONFIG_ARCH_CPU_INIT.

All mx6 based boards should use arch_cpu_init().

Acked-by Marek Vasut <[email protected]>
Quoted 32 lines
Signed-off-by Fabio Estevam <[email protected]>
---
 arch/arm/cpu/armv7/mx6/soc.c    |    2 --
 include/configs/mx6qarm2.h      |    1 -
 include/configs/mx6qsabrelite.h |    1 -
 3 files changed, 0 insertions(+), 4 deletions(-)
arch/arm/cpu/armv7/mx6/soc.c +0 -2
index 2ac74b5..2d07099 100644
@@ -37,7 +37,6 @@ u32 get_cpu_rev(void)
 	return system_rev;
 }
 
-#ifdef CONFIG_ARCH_CPU_INIT
 void init_aips(void)
 {
 	struct aipstz_regs *aips1, *aips2;

@@ -77,7 +76,6 @@ int arch_cpu_init(void)
 
 	return 0;
 }
-#endif
 
 #if defined(CONFIG_FEC_MXC)
 void imx_get_mac_from_fuse(int dev_id, unsigned char *mac)
include/configs/mx6qarm2.h +0 -0
index 0962d3c..87ff0a3 100644

include/configs/mx6qarm2.h line 38

Quoted 2 lines
include/configs/mx6qarm2.h +0 -0
@@ -37,7 +37,6 @@
 /* Size of malloc() pool */
 #define CONFIG_SYS_MALLOC_LEN		(CONFIG_ENV_SIZE + 2 * 1024 *

1024)

Quoted 16 lines
include/configs/mx6qarm2.h +0 -1
-#define CONFIG_ARCH_CPU_INIT
 #define CONFIG_BOARD_EARLY_INIT_F
 #define CONFIG_MXC_GPIO
(quoted diff) +0 -0

b/include/configs/mx6qsabrelite.h index 93000f0..85e6570 100644

include/configs/mx6qsabrelite.h +0 -1
@@ -37,7 +37,6 @@
 /* Size of malloc() pool */
 #define CONFIG_SYS_MALLOC_LEN          (CONFIG_ENV_SIZE + 2 * 1024 * 1024)
 
-#define CONFIG_ARCH_CPU_INIT
 #define CONFIG_BOARD_EARLY_INIT_F
 #define CONFIG_MXC_GPIO
author
#4 lore raw

PATCH 3/3 ARM: mx28: Remove CONFIG_ARCH_CPU_INIT

In reply to Fabio Estevam

No need to define CONFIG_ARCH_CPU_INIT.

All mx28 based boards should use arch_cpu_init().

Signed-off-by Fabio Estevam <[email protected]>
---
 arch/arm/cpu/arm926ejs/mx28/mx28.c |    2 --
 include/configs/m28evk.h           |    1 -
 include/configs/mx28evk.h          |    1 -
 3 files changed, 0 insertions(+), 4 deletions(-)
arch/arm/cpu/arm926ejs/mx28/mx28.c +0 -2
index b235091..f98e6a3 100644
@@ -135,7 +135,6 @@ int arch_misc_init(void)
 }
 #endif
 
-#ifdef	CONFIG_ARCH_CPU_INIT
 int arch_cpu_init(void)
 {
 	struct mx28_clkctrl_regs *clkctrl_regs =

@@ -164,7 +163,6 @@ int arch_cpu_init(void)
 
 	return 0;
 }
-#endif
 
 #if defined(CONFIG_DISPLAY_CPUINFO)
 int print_cpuinfo(void)
include/configs/m28evk.h +0 -1
index 4d59153..31aaf47 100644
@@ -40,7 +40,6 @@
 #define	CONFIG_SYS_ICACHE_OFF
 #define	CONFIG_SYS_DCACHE_OFF
 #define	CONFIG_BOARD_EARLY_INIT_F
-#define	CONFIG_ARCH_CPU_INIT
 #define	CONFIG_ARCH_MISC_INIT
 
 /*
include/configs/mx28evk.h +0 -1
index 04967d7..92903da 100644
@@ -34,7 +34,6 @@
 #define CONFIG_SYS_ICACHE_OFF
 #define CONFIG_SYS_DCACHE_OFF
 #define CONFIG_BOARD_EARLY_INIT_F
-#define CONFIG_ARCH_CPU_INIT
 #define CONFIG_ARCH_MISC_INIT
 
 /*
Signature · 1 line
1.7.1
#5 lore raw

PATCH 3/3 ARM: mx28: Remove CONFIG_ARCH_CPU_INIT

Quoted 4 lines

No need to define CONFIG_ARCH_CPU_INIT.

All mx28 based boards should use arch_cpu_init().

Acked-by Marek Vasut <[email protected]>
Quoted 51 lines
Signed-off-by Fabio Estevam <[email protected]>
---
 arch/arm/cpu/arm926ejs/mx28/mx28.c |    2 --
 include/configs/m28evk.h           |    1 -
 include/configs/mx28evk.h          |    1 -
 3 files changed, 0 insertions(+), 4 deletions(-)
(quoted diff) +0 -0

b/arch/arm/cpu/arm926ejs/mx28/mx28.c index b235091..f98e6a3 100644

arch/arm/cpu/arm926ejs/mx28/mx28.c +0 -2
@@ -135,7 +135,6 @@ int arch_misc_init(void)
 }
 #endif
 
-#ifdef	CONFIG_ARCH_CPU_INIT
 int arch_cpu_init(void)
 {
 	struct mx28_clkctrl_regs *clkctrl_regs =

@@ -164,7 +163,6 @@ int arch_cpu_init(void)
 
 	return 0;
 }
-#endif
 
 #if defined(CONFIG_DISPLAY_CPUINFO)
 int print_cpuinfo(void)
include/configs/m28evk.h +0 -1
index 4d59153..31aaf47 100644
@@ -40,7 +40,6 @@
 #define	CONFIG_SYS_ICACHE_OFF
 #define	CONFIG_SYS_DCACHE_OFF
 #define	CONFIG_BOARD_EARLY_INIT_F
-#define	CONFIG_ARCH_CPU_INIT
 #define	CONFIG_ARCH_MISC_INIT
 
 /*
include/configs/mx28evk.h +0 -1
index 04967d7..92903da 100644
@@ -34,7 +34,6 @@
 #define CONFIG_SYS_ICACHE_OFF
 #define CONFIG_SYS_DCACHE_OFF
 #define CONFIG_BOARD_EARLY_INIT_F
-#define CONFIG_ARCH_CPU_INIT
 #define CONFIG_ARCH_MISC_INIT
 
 /*
#7 lore raw

PATCH 3/3 ARM: mx28: Remove CONFIG_ARCH_CPU_INIT

Dear Albert ARIBAUD,

Quoted 5 lines

Hi Fabio,

Quoted 2 lines · Marek Vasut
Quoted 2 lines

No need to define CONFIG_ARCH_CPU_INIT.

Quoted 8 lines
Quoted 6 lines
Quoted 1 line

All mx28 based boards should use arch_cpu_init().

Acked-by Marek Vasut <[email protected]>
Quoted 2 lines
Signed-off-by Fabio Estevam <[email protected]>

---

Applied to ub-boot-arm/master, thanks.

Did you also apply 1/3 ? If not, everything will be broken now (at least mx28 doesn't boot without 1/3 applied as well).

Quoted 1 line

Amicalement,

Best regards,
Marek Vasut

#8 lore raw

PATCH 1/3 ARM: lib: Remove CONFIG_ARCH_CPU_INIT dependency

In reply to Fabio Estevam
Quoted 5 lines

Create a weak-aliased arch_cpu_init, so that we can get rid of CONFIG_ARCH_CPU_INIT and always call arch_cpu_init.

This way we do not need to define CONFIG_ARCH_CPU_INIT in every board file, since arch_cpu_init() is supposed to handle common CPU level code.

Acked-by Marek Vasut <[email protected]>
Quoted 29 lines
Signed-off-by Fabio Estevam <[email protected]>
---
 arch/arm/lib/board.c |   10 ++++++++--
 1 files changed, 8 insertions(+), 2 deletions(-)
arch/arm/lib/board.c +8 -2
index 500e216..6463db5 100644
@@ -224,10 +224,16 @@ void __dram_init_banksize(void)
 void dram_init_banksize(void)
 	__attribute__((weak, alias("__dram_init_banksize")));
 
+int __arch_cpu_init(void)
+{
+	return 0;
+}
+int arch_cpu_init(void)
+	__attribute__((weak, alias("__arch_cpu_init")));
+
 init_fnc_t *init_sequence[] = {
-#if defined(CONFIG_ARCH_CPU_INIT)
 	arch_cpu_init,		/* basic arch cpu dependent setup */
-#endif
+
 #if defined(CONFIG_BOARD_EARLY_INIT_F)
 	board_early_init_f,
 #endif
#9 lore raw

PATCH 1/3 ARM: lib: Remove CONFIG_ARCH_CPU_INIT dependency

In reply to Fabio Estevam

Le 01/03/2012 15:02, Fabio Estevam a ?crit :

Quoted 5 lines

Create a weak-aliased arch_cpu_init, so that we can get rid of CONFIG_ARCH_CPU_INIT and always call arch_cpu_init.

This way we do not need to define CONFIG_ARCH_CPU_INIT in every board file, since arch_cpu_init() is supposed to handle common CPU level code.

This adds (some) (functionally dead) code to all boards that did not require arch_cpu_init().

Amicalement,

Signature · 1 line
Albert.
#10 lore raw

PATCH 1/3 ARM: lib: Remove CONFIG_ARCH_CPU_INIT dependency

Quoted 2 lines · Albert ARIBAUD

This adds (some) (functionally dead) code to all boards that did not require arch_cpu_init().

Yes, it adds a "return 0" only. Is this terribly bad?

Isn't it better than having to select CONFIG_ARCH_CPU_INIT for every board to select arch_cpu_init(), which is supposed to be CPU dependent?

#11 lore raw

PATCH 1/3 ARM: lib: Remove CONFIG_ARCH_CPU_INIT dependency

Le 01/03/2012 21:57, Fabio Estevam a ?crit :

Quoted 7 lines
Quoted 2 lines · Albert ARIBAUD

This adds (some) (functionally dead) code to all boards that did not require arch_cpu_init().

Yes, it adds a "return 0" only. Is this terribly bad?

Well...

Quoted 3 lines

Isn't it better than having to select CONFIG_ARCH_CPU_INIT for every board to select arch_cpu_init(), which is supposed to be CPU dependent?

It adds a "#define CONFIG_ARCH_CPU_INTI" only. Is this terribly bad ?

If what bothers you is that some board all repeat something that should be done only once, then it should bother you just as much that many boards repeat something that should not be done at all most of the time.

I understand that you see a repetition of CONFIG_ARCH_CPU_INIT that you suspect is highly redundant. But then, you should cure the repetition, not the existence of CONFIG_ARCH_CPU_INIT.

How about this approach: since the problem is the presence of CPU- (or Soc-)level elements in board-level config files, and since there are probably many other such items in board config files (CPU type in a SoC and HW IP configurations, for instance) then why not introduce some cpu-config and soc-config file which would group all those cpu- (resp soc-)level items, including any "#define CONFIG_ARCH_CPU_INIT", and replace these items in board config files with a single "#include "<cpu>-config.h" (or "<soc>-config.h")? This would significantly reduce board config file size.

One could even choose either of two courses depending on how many or how few boards require CONFIG_ARCH_CPU_INIT: if many boards require it, let the cpu- or soc-common config contain the define, and the few boards which don't will have to explicitly undefine it; if only a few boards need the define, then it won't be done in the cpu- or soc-common config but in each board that requires it.

And CPUs, SoCs and boards that don't need ARCH_CPU_INIT at all won't grow a single useless byte.

Amicalement,

Signature · 1 line
Albert.
#12 lore raw

PATCH 1/3 ARM: lib: Remove CONFIG_ARCH_CPU_INIT dependency

Quoted 9 lines · Albert ARIBAUD

How about this approach: since the problem is the presence of CPU- (or Soc-)level elements in board-level config files, and since there are probably many other such items in board config files (CPU type in a SoC and HW IP configurations, for instance) then why not introduce some cpu-config and soc-config file which would group all those cpu- (resp soc-)level items, including any "#define CONFIG_ARCH_CPU_INIT", and replace these items in board config files with a single "#include "<cpu>-config.h" (or "<soc>-config.h")? This would significantly reduce board config file size.

Note that omap4/5 do something like this already and my hope is to do this for omap3/similar and make some other common files as well. It's not the top priority for me atm however...

Signature · 1 line
Tom
#13 lore raw

PATCH 1/3 ARM: lib: Remove CONFIG_ARCH_CPU_INIT dependency

Hi Tom,

Le 05/03/2012 18:56, Tom Rini a ?crit :

Quoted 12 lines · Albert ARIBAUD
Quoted 10 lines

How about this approach: since the problem is the presence of CPU- (or Soc-)level elements in board-level config files, and since there are probably many other such items in board config files (CPU type in a SoC and HW IP configurations, for instance) then why not introduce some cpu-config and soc-config file which would group all those cpu- (resp soc-)level items, including any "#define CONFIG_ARCH_CPU_INIT", and replace these items in board config files with a single "#include "<cpu>-config.h" (or "<soc>-config.h")? This would significantly reduce board config file size.

Quoted 3 lines

Note that omap4/5 do something like this already and my hope is to do this for omap3/similar and make some other common files as well. It's not the top priority for me atm however...

Ok, so that's one more reason not to go the "everybody call (a possibly weak and empty) arch_cpu_init()" route.

Amicalement,

Signature · 1 line
Albert.
#14 lore raw

PATCH 1/3 ARM: lib: Remove CONFIG_ARCH_CPU_INIT dependency

In reply to Fabio Estevam
Quoted 24 lines · Fabio Estevam

Create a weak-aliased arch_cpu_init, so that we can get rid of CONFIG_ARCH_CPU_INIT and always call arch_cpu_init.

This way we do not need to define CONFIG_ARCH_CPU_INIT in every board file, since arch_cpu_init() is supposed to handle common CPU level code.

Signed-off-by Fabio Estevam <[email protected]>
---
 arch/arm/lib/board.c |   10 ++++++++--
 1 files changed, 8 insertions(+), 2 deletions(-)
arch/arm/lib/board.c +5 -0
index 500e216..6463db5 100644
@@ -224,10 +224,16 @@ void __dram_init_banksize(void)
 void dram_init_banksize(void)
 	__attribute__((weak, alias("__dram_init_banksize")));
 
+int __arch_cpu_init(void)
+{
+	return 0;
+}
+int arch_cpu_init(void)

arch/arm/lib/board.c void __dram_init_banksize(void) · line 239

Quoted 10 lines
arch/arm/lib/board.c +3 -2
+	__attribute__((weak, alias("__arch_cpu_init")));
+
 init_fnc_t *init_sequence[] = {
-#if defined(CONFIG_ARCH_CPU_INIT)
 	arch_cpu_init,		/* basic arch cpu dependent setup */
-#endif
+
 #if defined(CONFIG_BOARD_EARLY_INIT_F)
 	board_early_init_f,
 #endif
Acked-by Stefano Babic <[email protected]>

Best regards,
Stefano Babic

Signature · 5 lines
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================
#15 lore raw

PATCH 1/3 ARM: lib: Remove CONFIG_ARCH_CPU_INIT dependency

In reply to Fabio Estevam
Quoted 16 lines · Fabio Estevam

Create a weak-aliased arch_cpu_init, so that we can get rid of CONFIG_ARCH_CPU_INIT and always call arch_cpu_init.

This way we do not need to define CONFIG_ARCH_CPU_INIT in every board file, since arch_cpu_init() is supposed to handle common CPU level code.

Signed-off-by Fabio Estevam <[email protected]>
---
 arch/arm/lib/board.c |   10 ++++++++--
 1 files changed, 8 insertions(+), 2 deletions(-)
arch/arm/lib/board.c +0 -0
index 500e216..6463db5 100644

arch/arm/lib/board.c void __dram_init_banksize(void) · line 233

Quoted 10 lines
arch/arm/lib/board.c +7 -0
@@ -224,10 +224,16 @@ void __dram_init_banksize(void)
 void dram_init_banksize(void)
 	__attribute__((weak, alias("__dram_init_banksize")));
 
+int __arch_cpu_init(void)
+{
+	return 0;
+}
+int arch_cpu_init(void)
+	__attribute__((weak, alias("__arch_cpu_init")));
+

Please add <linux/compiler.h> and use __weak instead.

Signature · 1 line
Tom
#16 lore raw

PATCH 1/3 ARM: lib: Remove CONFIG_ARCH_CPU_INIT dependency

Quoted 9 lines · Tom Rini

arch/arm/lib/board.c void __dram_init_banksize(void) · line 233

Quoted 7 lines
arch/arm/lib/board.c +7 -0
+int __arch_cpu_init(void)
+{
+ ? ? return 0;
+}
+int arch_cpu_init(void)
+ ? ? __attribute__((weak, alias("__arch_cpu_init")));
+

Please add <linux/compiler.h> and use __weak instead.

Ok, before I rework this patch I would like to confirm with Albert if I should proceed with this patch series, or if he NACKs this idea.

Regards,

Fabio Estevam

#17 lore raw

PATCH 1/3 ARM: lib: Remove CONFIG_ARCH_CPU_INIT dependency

Quoted 12 lines · Fabio Estevam
Quoted 9 lines · Tom Rini

arch/arm/lib/board.c void __dram_init_banksize(void) · line 233

Quoted 7 lines
arch/arm/lib/board.c +7 -0
+int __arch_cpu_init(void)
+{
+ ? ? return 0;
+}
+int arch_cpu_init(void)
+ ? ? __attribute__((weak, alias("__arch_cpu_init")));
+

Please add <linux/compiler.h> and use __weak instead.

Quoted 2 lines

Ok, before I rework this patch I would like to confirm with Albert if I should proceed with this patch series, or if he NACKs this idea.

By "this idea" I mean the removal of CONFIG_ARCH_CPU_INIT.