[PATCH 01/10] board_f: Introduce arch_setup_bdinfo initcall
Certain architectures (ppc, mips, sh, m68k) use setup board_part1 and setup_board_part2 calls during pre-relocation init to populate gd->bd boardinfo fields. This makes the generic init sequence cluttered with arch-specific ifdefs.
In order to clean these arch-specific sequences from generic init, introduce arch_setup_bdinfo weak initcall so that everyone can define their own bdinfo setup routines.
Signed-off-by: Ovidiu Panait ovidiu.panait@windriver.com ---
common/board_f.c | 6 ++++++ include/init.h | 12 ++++++++++++ 2 files changed, 18 insertions(+)
diff --git a/common/board_f.c b/common/board_f.c index dcad551ae4..e597749d2f 100644 --- a/common/board_f.c +++ b/common/board_f.c @@ -597,6 +597,11 @@ static int display_new_sp(void) return 0; }
+__weak int arch_setup_bdinfo(void) +{ + return 0; +} + #if defined(CONFIG_M68K) || defined(CONFIG_MIPS) || defined(CONFIG_PPC) || \ defined(CONFIG_SH) static int setup_board_part1(void) @@ -974,6 +979,7 @@ static const init_fnc_t init_sequence_f[] = { reserve_stacks, dram_init_banksize, show_dram_config, + arch_setup_bdinfo, #if defined(CONFIG_M68K) || defined(CONFIG_MIPS) || defined(CONFIG_PPC) || \ defined(CONFIG_SH) setup_board_part1, diff --git a/include/init.h b/include/init.h index e727031514..ccb766a348 100644 --- a/include/init.h +++ b/include/init.h @@ -141,6 +141,18 @@ int arch_reserve_stacks(void); */ int arch_reserve_mmu(void);
+/** + * arch_setup_bdinfo() - Architecture dependent boardinfo setup + * + * Architecture-specific routine for populating various boardinfo fields of + * gd->bd. It is called during the generic board init sequence. + * + * If an implementation is not provided, it will just be a nop stub. + * + * Return: 0 if OK + */ +int arch_setup_bdinfo(void); + /** * init_cache_f_r() - Turn on the cache in preparation for relocation *
Factor out m68k-specific bdinfo setup to arch_setup_bdinfo in arch/m68k/lib/bdinfo.c. Also, use if(IS_ENABLED()) instead of #ifdef where possible.
Signed-off-by: Ovidiu Panait ovidiu.panait@windriver.com ---
arch/m68k/lib/bdinfo.c | 32 ++++++++++++++++++++++++++++++++ common/board_f.c | 14 ++++---------- 2 files changed, 36 insertions(+), 10 deletions(-)
diff --git a/arch/m68k/lib/bdinfo.c b/arch/m68k/lib/bdinfo.c index 971c47c306..83e367b502 100644 --- a/arch/m68k/lib/bdinfo.c +++ b/arch/m68k/lib/bdinfo.c @@ -11,6 +11,38 @@
DECLARE_GLOBAL_DATA_PTR;
+int arch_setup_bdinfo(void) +{ + bd_t *bd = gd->bd; + + /* + * Save local variables to board info struct + */ + bd->bi_memstart = CONFIG_SYS_SDRAM_BASE; /* start of memory */ + bd->bi_memsize = gd->ram_size; /* size in bytes */ + +#ifdef CONFIG_SYS_SRAM_BASE + bd->bi_sramstart = CONFIG_SYS_SRAM_BASE; /* start of SRAM */ + bd->bi_sramsize = CONFIG_SYS_SRAM_SIZE; /* size of SRAM */ +#endif + + bd->bi_mbar_base = CONFIG_SYS_MBAR; /* base of internal registers */ + + bd->bi_intfreq = gd->cpu_clk; /* Internal Freq, in Hz */ + bd->bi_busfreq = gd->bus_clk; /* Bus Freq, in Hz */ + + if (IS_ENABLED(CONFIG_PCI)) + bd->bi_pcifreq = gd->pci_clk; + +#if defined(CONFIG_EXTRA_CLOCK) + bd->bi_inpfreq = gd->arch.inp_clk; /* input Freq in Hz */ + bd->bi_vcofreq = gd->arch.vco_clk; /* vco Freq in Hz */ + bd->bi_flbfreq = gd->arch.flb_clk; /* flexbus Freq in Hz */ +#endif + + return 0; +} + void arch_print_bdinfo(void) { bd_t *bd = gd->bd; diff --git a/common/board_f.c b/common/board_f.c index e597749d2f..7d65879b93 100644 --- a/common/board_f.c +++ b/common/board_f.c @@ -602,7 +602,7 @@ __weak int arch_setup_bdinfo(void) return 0; }
-#if defined(CONFIG_M68K) || defined(CONFIG_MIPS) || defined(CONFIG_PPC) || \ +#if defined(CONFIG_MIPS) || defined(CONFIG_PPC) || \ defined(CONFIG_SH) static int setup_board_part1(void) { @@ -622,9 +622,6 @@ static int setup_board_part1(void) #if defined(CONFIG_E500) || defined(CONFIG_MPC86xx) bd->bi_immr_base = CONFIG_SYS_IMMR; /* base of IMMR register */ #endif -#if defined(CONFIG_M68K) - bd->bi_mbar_base = CONFIG_SYS_MBAR; /* base of internal registers */ -#endif #if defined(CONFIG_MPC83xx) bd->bi_immrbar = CONFIG_SYS_IMMR; #endif @@ -633,7 +630,7 @@ static int setup_board_part1(void) } #endif
-#if defined(CONFIG_PPC) || defined(CONFIG_M68K) +#if defined(CONFIG_PPC) static int setup_board_part2(void) { bd_t *bd = gd->bd; @@ -646,9 +643,6 @@ static int setup_board_part2(void) bd->bi_sccfreq = gd->arch.scc_clk; bd->bi_vco = gd->arch.vco_out; #endif /* CONFIG_CPM2 */ -#if defined(CONFIG_M68K) && defined(CONFIG_PCI) - bd->bi_pcifreq = gd->pci_clk; -#endif #if defined(CONFIG_EXTRA_CLOCK) bd->bi_inpfreq = gd->arch.inp_clk; /* input Freq in Hz */ bd->bi_vcofreq = gd->arch.vco_clk; /* vco Freq in Hz */ @@ -980,11 +974,11 @@ static const init_fnc_t init_sequence_f[] = { dram_init_banksize, show_dram_config, arch_setup_bdinfo, -#if defined(CONFIG_M68K) || defined(CONFIG_MIPS) || defined(CONFIG_PPC) || \ +#if defined(CONFIG_MIPS) || defined(CONFIG_PPC) || \ defined(CONFIG_SH) setup_board_part1, #endif -#if defined(CONFIG_PPC) || defined(CONFIG_M68K) +#if defined(CONFIG_PPC) INIT_FUNC_WATCHDOG_RESET setup_board_part2, #endif
On Thu, 9 Jul 2020 at 02:09, Ovidiu Panait ovidiu.panait@windriver.com wrote:
Factor out m68k-specific bdinfo setup to arch_setup_bdinfo in arch/m68k/lib/bdinfo.c. Also, use if(IS_ENABLED()) instead of #ifdef where possible.
Signed-off-by: Ovidiu Panait ovidiu.panait@windriver.com
arch/m68k/lib/bdinfo.c | 32 ++++++++++++++++++++++++++++++++ common/board_f.c | 14 ++++---------- 2 files changed, 36 insertions(+), 10 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
Factor out ppc-specific bdinfo setup from generic init sequence to arch_setup_bdinfo in arch/powerpc/lib/bdinfo.c.
Signed-off-by: Ovidiu Panait ovidiu.panait@windriver.com ---
arch/powerpc/lib/bdinfo.c | 42 +++++++++++++++++++++++++++++++++++++++ common/board_f.c | 39 ++---------------------------------- 2 files changed, 44 insertions(+), 37 deletions(-)
diff --git a/arch/powerpc/lib/bdinfo.c b/arch/powerpc/lib/bdinfo.c index d8c64155f0..49635874cf 100644 --- a/arch/powerpc/lib/bdinfo.c +++ b/arch/powerpc/lib/bdinfo.c @@ -11,6 +11,48 @@
DECLARE_GLOBAL_DATA_PTR;
+int arch_setup_bdinfo(void) +{ + bd_t *bd = gd->bd; + + /* + * Save local variables to board info struct + */ + bd->bi_memstart = CONFIG_SYS_SDRAM_BASE; /* start of memory */ + bd->bi_memsize = gd->ram_size; /* size in bytes */ + +#ifdef CONFIG_SYS_SRAM_BASE + bd->bi_sramstart = CONFIG_SYS_SRAM_BASE; /* start of SRAM */ + bd->bi_sramsize = CONFIG_SYS_SRAM_SIZE; /* size of SRAM */ +#endif + +#if defined(CONFIG_E500) || defined(CONFIG_MPC86xx) + bd->bi_immr_base = CONFIG_SYS_IMMR; /* base of IMMR register */ +#endif + +#if defined(CONFIG_MPC83xx) + bd->bi_immrbar = CONFIG_SYS_IMMR; +#endif + + bd->bi_intfreq = gd->cpu_clk; /* Internal Freq, in Hz */ + bd->bi_busfreq = gd->bus_clk; /* Bus Freq, in Hz */ + +#if defined(CONFIG_CPM2) + bd->bi_cpmfreq = gd->arch.cpm_clk; + bd->bi_brgfreq = gd->arch.brg_clk; + bd->bi_sccfreq = gd->arch.scc_clk; + bd->bi_vco = gd->arch.vco_out; +#endif /* CONFIG_CPM2 */ + +#if defined(CONFIG_EXTRA_CLOCK) + bd->bi_inpfreq = gd->arch.inp_clk; /* input Freq in Hz */ + bd->bi_vcofreq = gd->arch.vco_clk; /* vco Freq in Hz */ + bd->bi_flbfreq = gd->arch.flb_clk; /* flexbus Freq in Hz */ +#endif + + return 0; +} + void __weak board_detail(void) { /* Please define board_detail() for your PPC platform */ diff --git a/common/board_f.c b/common/board_f.c index 7d65879b93..960a9c83db 100644 --- a/common/board_f.c +++ b/common/board_f.c @@ -602,8 +602,7 @@ __weak int arch_setup_bdinfo(void) return 0; }
-#if defined(CONFIG_MIPS) || defined(CONFIG_PPC) || \ - defined(CONFIG_SH) +#if defined(CONFIG_MIPS) || defined(CONFIG_SH) static int setup_board_part1(void) { bd_t *bd = gd->bd; @@ -619,36 +618,6 @@ static int setup_board_part1(void) bd->bi_sramsize = CONFIG_SYS_SRAM_SIZE; /* size of SRAM */ #endif
-#if defined(CONFIG_E500) || defined(CONFIG_MPC86xx) - bd->bi_immr_base = CONFIG_SYS_IMMR; /* base of IMMR register */ -#endif -#if defined(CONFIG_MPC83xx) - bd->bi_immrbar = CONFIG_SYS_IMMR; -#endif - - return 0; -} -#endif - -#if defined(CONFIG_PPC) -static int setup_board_part2(void) -{ - bd_t *bd = gd->bd; - - bd->bi_intfreq = gd->cpu_clk; /* Internal Freq, in Hz */ - bd->bi_busfreq = gd->bus_clk; /* Bus Freq, in Hz */ -#if defined(CONFIG_CPM2) - bd->bi_cpmfreq = gd->arch.cpm_clk; - bd->bi_brgfreq = gd->arch.brg_clk; - bd->bi_sccfreq = gd->arch.scc_clk; - bd->bi_vco = gd->arch.vco_out; -#endif /* CONFIG_CPM2 */ -#if defined(CONFIG_EXTRA_CLOCK) - bd->bi_inpfreq = gd->arch.inp_clk; /* input Freq in Hz */ - bd->bi_vcofreq = gd->arch.vco_clk; /* vco Freq in Hz */ - bd->bi_flbfreq = gd->arch.flb_clk; /* flexbus Freq in Hz */ -#endif - return 0; } #endif @@ -974,13 +943,9 @@ static const init_fnc_t init_sequence_f[] = { dram_init_banksize, show_dram_config, arch_setup_bdinfo, -#if defined(CONFIG_MIPS) || defined(CONFIG_PPC) || \ - defined(CONFIG_SH) +#if defined(CONFIG_MIPS) || defined(CONFIG_SH) setup_board_part1, -#endif -#if defined(CONFIG_PPC) INIT_FUNC_WATCHDOG_RESET - setup_board_part2, #endif display_new_sp, #ifdef CONFIG_OF_BOARD_FIXUP
On Thu, 9 Jul 2020 at 02:09, Ovidiu Panait ovidiu.panait@windriver.com wrote:
Factor out ppc-specific bdinfo setup from generic init sequence to arch_setup_bdinfo in arch/powerpc/lib/bdinfo.c.
Signed-off-by: Ovidiu Panait ovidiu.panait@windriver.com
arch/powerpc/lib/bdinfo.c | 42 +++++++++++++++++++++++++++++++++++++++ common/board_f.c | 39 ++---------------------------------- 2 files changed, 44 insertions(+), 37 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
Factor out sh-specific bdinfo setup from generic init sequence to arch_setup_bdinfo in arch/sh/lib/board.c.
Signed-off-by: Ovidiu Panait ovidiu.panait@windriver.com ---
arch/sh/lib/board.c | 18 ++++++++++++++++++ common/board_f.c | 4 ++-- 2 files changed, 20 insertions(+), 2 deletions(-)
diff --git a/arch/sh/lib/board.c b/arch/sh/lib/board.c index a6a8f07e6f..32e87e742b 100644 --- a/arch/sh/lib/board.c +++ b/arch/sh/lib/board.c @@ -16,6 +16,24 @@ int dram_init(void) return 0; }
+int arch_setup_bdinfo(void) +{ + bd_t *bd = gd->bd; + + /* + * Save local variables to board info struct + */ + bd->bi_memstart = CONFIG_SYS_SDRAM_BASE; /* start of memory */ + bd->bi_memsize = gd->ram_size; /* size in bytes */ + +#ifdef CONFIG_SYS_SRAM_BASE + bd->bi_sramstart = CONFIG_SYS_SRAM_BASE; /* start of SRAM */ + bd->bi_sramsize = CONFIG_SYS_SRAM_SIZE; /* size of SRAM */ +#endif + + return 0; +} + void relocate_code(ulong start_addr_sp, gd_t *new_gd, ulong relocaddr) { void (*reloc_board_init_r)(gd_t *gd, ulong dest) = board_init_r; diff --git a/common/board_f.c b/common/board_f.c index 960a9c83db..9bfcd6b236 100644 --- a/common/board_f.c +++ b/common/board_f.c @@ -602,7 +602,7 @@ __weak int arch_setup_bdinfo(void) return 0; }
-#if defined(CONFIG_MIPS) || defined(CONFIG_SH) +#if defined(CONFIG_MIPS) static int setup_board_part1(void) { bd_t *bd = gd->bd; @@ -943,7 +943,7 @@ static const init_fnc_t init_sequence_f[] = { dram_init_banksize, show_dram_config, arch_setup_bdinfo, -#if defined(CONFIG_MIPS) || defined(CONFIG_SH) +#if defined(CONFIG_MIPS) setup_board_part1, INIT_FUNC_WATCHDOG_RESET #endif
On Thu, 9 Jul 2020 at 02:08, Ovidiu Panait ovidiu.panait@windriver.com wrote:
Factor out sh-specific bdinfo setup from generic init sequence to arch_setup_bdinfo in arch/sh/lib/board.c.
Signed-off-by: Ovidiu Panait ovidiu.panait@windriver.com
arch/sh/lib/board.c | 18 ++++++++++++++++++ common/board_f.c | 4 ++-- 2 files changed, 20 insertions(+), 2 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
Factor out mips-specific bdinfo setup from generic init sequence to arch_setup_bdinfo in arch/mips/lib/boot.c.
Signed-off-by: Ovidiu Panait ovidiu.panait@windriver.com ---
arch/mips/lib/boot.c | 18 ++++++++++++++++++ common/board_f.c | 25 +------------------------ 2 files changed, 19 insertions(+), 24 deletions(-)
diff --git a/arch/mips/lib/boot.c b/arch/mips/lib/boot.c index db862f6379..b3a48ce10f 100644 --- a/arch/mips/lib/boot.c +++ b/arch/mips/lib/boot.c @@ -9,6 +9,24 @@
DECLARE_GLOBAL_DATA_PTR;
+int arch_setup_bdinfo(void) +{ + bd_t *bd = gd->bd; + + /* + * Save local variables to board info struct + */ + bd->bi_memstart = CONFIG_SYS_SDRAM_BASE; /* start of memory */ + bd->bi_memsize = gd->ram_size; /* size in bytes */ + +#ifdef CONFIG_SYS_SRAM_BASE + bd->bi_sramstart = CONFIG_SYS_SRAM_BASE; /* start of SRAM */ + bd->bi_sramsize = CONFIG_SYS_SRAM_SIZE; /* size of SRAM */ +#endif + + return 0; +} + unsigned long do_go_exec(ulong (*entry)(int, char * const []), int argc, char * const argv[]) { diff --git a/common/board_f.c b/common/board_f.c index 9bfcd6b236..fd7e6a17ad 100644 --- a/common/board_f.c +++ b/common/board_f.c @@ -602,26 +602,6 @@ __weak int arch_setup_bdinfo(void) return 0; }
-#if defined(CONFIG_MIPS) -static int setup_board_part1(void) -{ - bd_t *bd = gd->bd; - - /* - * Save local variables to board info struct - */ - bd->bi_memstart = CONFIG_SYS_SDRAM_BASE; /* start of memory */ - bd->bi_memsize = gd->ram_size; /* size in bytes */ - -#ifdef CONFIG_SYS_SRAM_BASE - bd->bi_sramstart = CONFIG_SYS_SRAM_BASE; /* start of SRAM */ - bd->bi_sramsize = CONFIG_SYS_SRAM_SIZE; /* size of SRAM */ -#endif - - return 0; -} -#endif - #ifdef CONFIG_POST static int init_post(void) { @@ -942,11 +922,8 @@ static const init_fnc_t init_sequence_f[] = { reserve_stacks, dram_init_banksize, show_dram_config, - arch_setup_bdinfo, -#if defined(CONFIG_MIPS) - setup_board_part1, INIT_FUNC_WATCHDOG_RESET -#endif + arch_setup_bdinfo, display_new_sp, #ifdef CONFIG_OF_BOARD_FIXUP fix_fdt,
On 09.07.20 10:04, Ovidiu Panait wrote:
Factor out mips-specific bdinfo setup from generic init sequence to arch_setup_bdinfo in arch/mips/lib/boot.c.
Signed-off-by: Ovidiu Panait ovidiu.panait@windriver.com
arch/mips/lib/boot.c | 18 ++++++++++++++++++ common/board_f.c | 25 +------------------------ 2 files changed, 19 insertions(+), 24 deletions(-)
diff --git a/arch/mips/lib/boot.c b/arch/mips/lib/boot.c index db862f6379..b3a48ce10f 100644 --- a/arch/mips/lib/boot.c +++ b/arch/mips/lib/boot.c @@ -9,6 +9,24 @@
DECLARE_GLOBAL_DATA_PTR;
+int arch_setup_bdinfo(void) +{
- bd_t *bd = gd->bd;
- /*
* Save local variables to board info struct*/- bd->bi_memstart = CONFIG_SYS_SDRAM_BASE; /* start of memory */
- bd->bi_memsize = gd->ram_size; /* size in bytes */
+#ifdef CONFIG_SYS_SRAM_BASE
We want to get rid of #ifdef where possible. So it is preferable to write:
if IS_ENABLED(CONFIG_SYS_SRAM_BASE) {
One benefit is that static code analysis will consider the code.
Best regards
Heinrich
- bd->bi_sramstart = CONFIG_SYS_SRAM_BASE; /* start of SRAM */
- bd->bi_sramsize = CONFIG_SYS_SRAM_SIZE; /* size of SRAM */
+#endif
- return 0;
+}
unsigned long do_go_exec(ulong (*entry)(int, char * const []), int argc, char * const argv[]) { diff --git a/common/board_f.c b/common/board_f.c index 9bfcd6b236..fd7e6a17ad 100644 --- a/common/board_f.c +++ b/common/board_f.c @@ -602,26 +602,6 @@ __weak int arch_setup_bdinfo(void) return 0; }
-#if defined(CONFIG_MIPS) -static int setup_board_part1(void) -{
- bd_t *bd = gd->bd;
- /*
* Save local variables to board info struct*/- bd->bi_memstart = CONFIG_SYS_SDRAM_BASE; /* start of memory */
- bd->bi_memsize = gd->ram_size; /* size in bytes */
-#ifdef CONFIG_SYS_SRAM_BASE
- bd->bi_sramstart = CONFIG_SYS_SRAM_BASE; /* start of SRAM */
- bd->bi_sramsize = CONFIG_SYS_SRAM_SIZE; /* size of SRAM */
-#endif
- return 0;
-} -#endif
#ifdef CONFIG_POST static int init_post(void) { @@ -942,11 +922,8 @@ static const init_fnc_t init_sequence_f[] = { reserve_stacks, dram_init_banksize, show_dram_config,
- arch_setup_bdinfo,
-#if defined(CONFIG_MIPS)
- setup_board_part1, INIT_FUNC_WATCHDOG_RESET
-#endif
- arch_setup_bdinfo, display_new_sp,
#ifdef CONFIG_OF_BOARD_FIXUP fix_fdt,
Hi,
On 09.07.2020 12:15, Heinrich Schuchardt wrote:
On 09.07.20 10:04, Ovidiu Panait wrote:
Factor out mips-specific bdinfo setup from generic init sequence to arch_setup_bdinfo in arch/mips/lib/boot.c.
Signed-off-by: Ovidiu Panait ovidiu.panait@windriver.com
arch/mips/lib/boot.c | 18 ++++++++++++++++++ common/board_f.c | 25 +------------------------ 2 files changed, 19 insertions(+), 24 deletions(-)
diff --git a/arch/mips/lib/boot.c b/arch/mips/lib/boot.c index db862f6379..b3a48ce10f 100644 --- a/arch/mips/lib/boot.c +++ b/arch/mips/lib/boot.c @@ -9,6 +9,24 @@
DECLARE_GLOBAL_DATA_PTR;
+int arch_setup_bdinfo(void) +{
- bd_t *bd = gd->bd;
- /*
* Save local variables to board info struct*/- bd->bi_memstart = CONFIG_SYS_SDRAM_BASE; /* start of memory */
- bd->bi_memsize = gd->ram_size; /* size in bytes */
+#ifdef CONFIG_SYS_SRAM_BASE
We want to get rid of #ifdef where possible. So it is preferable to write:
if IS_ENABLED(CONFIG_SYS_SRAM_BASE) {
One benefit is that static code analysis will consider the code.
Best regards
Heinrich
My understanding is that IS_ENABLED() only works with with boolean and tristate options.
In this case, CONFIG_SYS_SRAM_BASE is a hex value:
include/configs/pic32mzdask.h:22:#define CONFIG_SYS_SRAM_BASE 0x80000000
Switching to IS_ENABLED() produces the following build errors for qemu mips:
$ git diff diff --git a/arch/mips/lib/boot.c b/arch/mips/lib/boot.c index b3a48ce10f..aa047335ec 100644 --- a/arch/mips/lib/boot.c +++ b/arch/mips/lib/boot.c @@ -19,10 +19,10 @@ int arch_setup_bdinfo(void) bd->bi_memstart = CONFIG_SYS_SDRAM_BASE; /* start of memory */ bd->bi_memsize = gd->ram_size; /* size in bytes */
-#ifdef CONFIG_SYS_SRAM_BASE - bd->bi_sramstart = CONFIG_SYS_SRAM_BASE; /* start of SRAM */ - bd->bi_sramsize = CONFIG_SYS_SRAM_SIZE; /* size of SRAM */ -#endif + if (IS_ENABLED(CONFIG_SYS_SRAM_BASE)) { + bd->bi_sramstart = CONFIG_SYS_SRAM_BASE; /* start of SRAM */ + bd->bi_sramsize = CONFIG_SYS_SRAM_SIZE; /* size of SRAM */ + }
return 0; }
$ make
...
arch/mips/lib/boot.c: In function 'arch_setup_bdinfo': CC common/init/board_init.o arch/mips/lib/boot.c:23:22: error: 'CONFIG_SYS_SRAM_BASE' undeclared (first use in this function); did you mean 'CONFIG_SYS_SDRAM_BASE'? 23 | bd->bi_sramstart = CONFIG_SYS_SRAM_BASE; /* start of SRAM */ | ^~~~~~~~~~~~~~~~~~~~ | CONFIG_SYS_SDRAM_BASE arch/mips/lib/boot.c:23:22: note: each undeclared identifier is reported only once for each function it appears in arch/mips/lib/boot.c:24:21: error: 'CONFIG_SYS_SRAM_SIZE' undeclared (first use in this function); did you mean 'CONFIG_SYS_SDRAM_BASE'? 24 | bd->bi_sramsize = CONFIG_SYS_SRAM_SIZE; /* size of SRAM */ | ^~~~~~~~~~~~~~~~~~~~ | CONFIG_SYS_SDRAM_BASE
Thanks,
Ovidiu
- bd->bi_sramstart = CONFIG_SYS_SRAM_BASE; /* start of SRAM */
- bd->bi_sramsize = CONFIG_SYS_SRAM_SIZE; /* size of SRAM */
+#endif
- return 0;
+}
- unsigned long do_go_exec(ulong (*entry)(int, char * const []), int argc, char * const argv[]) {
diff --git a/common/board_f.c b/common/board_f.c index 9bfcd6b236..fd7e6a17ad 100644 --- a/common/board_f.c +++ b/common/board_f.c @@ -602,26 +602,6 @@ __weak int arch_setup_bdinfo(void) return 0; }
-#if defined(CONFIG_MIPS) -static int setup_board_part1(void) -{
- bd_t *bd = gd->bd;
- /*
* Save local variables to board info struct*/- bd->bi_memstart = CONFIG_SYS_SDRAM_BASE; /* start of memory */
- bd->bi_memsize = gd->ram_size; /* size in bytes */
-#ifdef CONFIG_SYS_SRAM_BASE
- bd->bi_sramstart = CONFIG_SYS_SRAM_BASE; /* start of SRAM */
- bd->bi_sramsize = CONFIG_SYS_SRAM_SIZE; /* size of SRAM */
-#endif
- return 0;
-} -#endif
- #ifdef CONFIG_POST static int init_post(void) {
@@ -942,11 +922,8 @@ static const init_fnc_t init_sequence_f[] = { reserve_stacks, dram_init_banksize, show_dram_config,
- arch_setup_bdinfo,
-#if defined(CONFIG_MIPS)
- setup_board_part1, INIT_FUNC_WATCHDOG_RESET
-#endif
- arch_setup_bdinfo, display_new_sp, #ifdef CONFIG_OF_BOARD_FIXUP fix_fdt,
On 09.07.20 12:27, Ovidiu Panait wrote:
Hi,
On 09.07.2020 12:15, Heinrich Schuchardt wrote:
On 09.07.20 10:04, Ovidiu Panait wrote:
Factor out mips-specific bdinfo setup from generic init sequence to arch_setup_bdinfo in arch/mips/lib/boot.c.
Signed-off-by: Ovidiu Panait ovidiu.panait@windriver.com
arch/mips/lib/boot.c | 18 ++++++++++++++++++ common/board_f.c | 25 +------------------------ 2 files changed, 19 insertions(+), 24 deletions(-)
diff --git a/arch/mips/lib/boot.c b/arch/mips/lib/boot.c index db862f6379..b3a48ce10f 100644 --- a/arch/mips/lib/boot.c +++ b/arch/mips/lib/boot.c @@ -9,6 +9,24 @@
DECLARE_GLOBAL_DATA_PTR;
+int arch_setup_bdinfo(void) +{ + bd_t *bd = gd->bd;
+ /* + * Save local variables to board info struct + */ + bd->bi_memstart = CONFIG_SYS_SDRAM_BASE; /* start of memory */ + bd->bi_memsize = gd->ram_size; /* size in bytes */
+#ifdef CONFIG_SYS_SRAM_BASE
We want to get rid of #ifdef where possible. So it is preferable to write:
if IS_ENABLED(CONFIG_SYS_SRAM_BASE) {
One benefit is that static code analysis will consider the code.
Best regards
Heinrich
My understanding is that IS_ENABLED() only works with with boolean and tristate options.
In this case, CONFIG_SYS_SRAM_BASE is a hex value:
include/configs/pic32mzdask.h:22:#define CONFIG_SYS_SRAM_BASE 0x80000000
Switching to IS_ENABLED() produces the following build errors for qemu mips:
You could add the following helper macro to include/linux/kconfig.h
#define config_defined(cfg) _config_defined(cfg, #cfg) #define _config_defined(a1, a2) !!__builtin_strcmp(#a1, a2)
config_defined(cfg) evaluates to true if: * cfg is defined and * cfg is not defined as cfg.
As long as optimization is switch on __builtin_strcmp() is evaluated at compile time.
Best regards
Heinrich
$ git diff diff --git a/arch/mips/lib/boot.c b/arch/mips/lib/boot.c index b3a48ce10f..aa047335ec 100644 --- a/arch/mips/lib/boot.c +++ b/arch/mips/lib/boot.c @@ -19,10 +19,10 @@ int arch_setup_bdinfo(void) bd->bi_memstart = CONFIG_SYS_SDRAM_BASE; /* start of memory */ bd->bi_memsize = gd->ram_size; /* size in bytes */
-#ifdef CONFIG_SYS_SRAM_BASE - bd->bi_sramstart = CONFIG_SYS_SRAM_BASE; /* start of SRAM */ - bd->bi_sramsize = CONFIG_SYS_SRAM_SIZE; /* size of SRAM */ -#endif + if (IS_ENABLED(CONFIG_SYS_SRAM_BASE)) { + bd->bi_sramstart = CONFIG_SYS_SRAM_BASE; /* start of SRAM */ + bd->bi_sramsize = CONFIG_SYS_SRAM_SIZE; /* size of SRAM */ + }
return 0; }
$ make
...
arch/mips/lib/boot.c: In function 'arch_setup_bdinfo': CC common/init/board_init.o arch/mips/lib/boot.c:23:22: error: 'CONFIG_SYS_SRAM_BASE' undeclared (first use in this function); did you mean 'CONFIG_SYS_SDRAM_BASE'? 23 | bd->bi_sramstart = CONFIG_SYS_SRAM_BASE; /* start of SRAM */ | ^~~~~~~~~~~~~~~~~~~~ | CONFIG_SYS_SDRAM_BASE arch/mips/lib/boot.c:23:22: note: each undeclared identifier is reported only once for each function it appears in arch/mips/lib/boot.c:24:21: error: 'CONFIG_SYS_SRAM_SIZE' undeclared (first use in this function); did you mean 'CONFIG_SYS_SDRAM_BASE'? 24 | bd->bi_sramsize = CONFIG_SYS_SRAM_SIZE; /* size of SRAM */ | ^~~~~~~~~~~~~~~~~~~~ | CONFIG_SYS_SDRAM_BASE
Thanks,
Ovidiu
+ bd->bi_sramstart = CONFIG_SYS_SRAM_BASE; /* start of SRAM */ + bd->bi_sramsize = CONFIG_SYS_SRAM_SIZE; /* size of SRAM */ +#endif
+ return 0; +}
unsigned long do_go_exec(ulong (*entry)(int, char * const []), int argc, char * const argv[]) { diff --git a/common/board_f.c b/common/board_f.c index 9bfcd6b236..fd7e6a17ad 100644 --- a/common/board_f.c +++ b/common/board_f.c @@ -602,26 +602,6 @@ __weak int arch_setup_bdinfo(void) return 0; }
-#if defined(CONFIG_MIPS) -static int setup_board_part1(void) -{ - bd_t *bd = gd->bd;
- /* - * Save local variables to board info struct - */ - bd->bi_memstart = CONFIG_SYS_SDRAM_BASE; /* start of memory */ - bd->bi_memsize = gd->ram_size; /* size in bytes */
-#ifdef CONFIG_SYS_SRAM_BASE - bd->bi_sramstart = CONFIG_SYS_SRAM_BASE; /* start of SRAM */ - bd->bi_sramsize = CONFIG_SYS_SRAM_SIZE; /* size of SRAM */ -#endif
- return 0; -} -#endif
#ifdef CONFIG_POST static int init_post(void) { @@ -942,11 +922,8 @@ static const init_fnc_t init_sequence_f[] = { reserve_stacks, dram_init_banksize, show_dram_config, - arch_setup_bdinfo, -#if defined(CONFIG_MIPS) - setup_board_part1, INIT_FUNC_WATCHDOG_RESET -#endif + arch_setup_bdinfo, display_new_sp, #ifdef CONFIG_OF_BOARD_FIXUP fix_fdt,
Hi,
On Thu, 9 Jul 2020 at 06:28, Heinrich Schuchardt xypron.debian@gmx.de wrote:
On 09.07.20 12:27, Ovidiu Panait wrote:
Hi,
On 09.07.2020 12:15, Heinrich Schuchardt wrote:
On 09.07.20 10:04, Ovidiu Panait wrote:
Factor out mips-specific bdinfo setup from generic init sequence to arch_setup_bdinfo in arch/mips/lib/boot.c.
Signed-off-by: Ovidiu Panait ovidiu.panait@windriver.com
arch/mips/lib/boot.c | 18 ++++++++++++++++++ common/board_f.c | 25 +------------------------ 2 files changed, 19 insertions(+), 24 deletions(-)
diff --git a/arch/mips/lib/boot.c b/arch/mips/lib/boot.c index db862f6379..b3a48ce10f 100644 --- a/arch/mips/lib/boot.c +++ b/arch/mips/lib/boot.c @@ -9,6 +9,24 @@
DECLARE_GLOBAL_DATA_PTR;
+int arch_setup_bdinfo(void) +{
- bd_t *bd = gd->bd;
- /*
* Save local variables to board info struct*/- bd->bi_memstart = CONFIG_SYS_SDRAM_BASE; /* start of memory */
- bd->bi_memsize = gd->ram_size; /* size in bytes */
+#ifdef CONFIG_SYS_SRAM_BASE
We want to get rid of #ifdef where possible. So it is preferable to write:
if IS_ENABLED(CONFIG_SYS_SRAM_BASE) {
One benefit is that static code analysis will consider the code.
Best regards
Heinrich
My understanding is that IS_ENABLED() only works with with boolean and tristate options.
In this case, CONFIG_SYS_SRAM_BASE is a hex value:
include/configs/pic32mzdask.h:22:#define CONFIG_SYS_SRAM_BASE 0x80000000
Switching to IS_ENABLED() produces the following build errors for qemu mips:
You could add the following helper macro to include/linux/kconfig.h
#define config_defined(cfg) _config_defined(cfg, #cfg) #define _config_defined(a1, a2) !!__builtin_strcmp(#a1, a2)
config_defined(cfg) evaluates to true if:
- cfg is defined and
- cfg is not defined as cfg.
As long as optimization is switch on __builtin_strcmp() is evaluated at compile time.
I think we should have a new CONFIG_SYS_HAS_SRAM.
Regards, Simon
Use IS_ENABLED() instead of #ifdef in blk_post_probe function.
No functional change intended.
Signed-off-by: Ovidiu Panait ovidiu.panait@windriver.com ---
drivers/block/blk-uclass.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/block/blk-uclass.c b/drivers/block/blk-uclass.c index b19375cbc8..b2738f5717 100644 --- a/drivers/block/blk-uclass.c +++ b/drivers/block/blk-uclass.c @@ -644,11 +644,12 @@ int blk_unbind_all(int if_type)
static int blk_post_probe(struct udevice *dev) { -#if defined(CONFIG_PARTITIONS) && defined(CONFIG_HAVE_BLOCK_DEVICE) - struct blk_desc *desc = dev_get_uclass_platdata(dev); + if (IS_ENABLED(CONFIG_PARTITIONS) && + IS_ENABLED(CONFIG_HAVE_BLOCK_DEV)) { + struct blk_desc *desc = dev_get_uclass_platdata(dev);
- part_init(desc); -#endif + part_init(desc); + }
return 0; }
On Thu, 9 Jul 2020 at 02:09, Ovidiu Panait ovidiu.panait@windriver.com wrote:
Use IS_ENABLED() instead of #ifdef in blk_post_probe function.
No functional change intended.
Signed-off-by: Ovidiu Panait ovidiu.panait@windriver.com
drivers/block/blk-uclass.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
serial_initialize is called only during the common init sequence, after relocation (in common/board_r.c). Because it has a void return value, it has to wrapped in initr_serial. In order to be able to get rid of this indirection, make serial_initialize return int.
Remove extern from prototype in order to silence the following checkpatch warning: check: extern prototypes should be avoided in .h files
Signed-off-by: Ovidiu Panait ovidiu.panait@windriver.com ---
drivers/serial/serial-uclass.c | 4 ++-- drivers/serial/serial.c | 4 +++- include/serial.h | 2 +- 3 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/serial/serial-uclass.c b/drivers/serial/serial-uclass.c index a0af0e6bfd..0027625ebf 100644 --- a/drivers/serial/serial-uclass.c +++ b/drivers/serial/serial-uclass.c @@ -170,9 +170,9 @@ int serial_init(void) }
/* Called after relocation */ -void serial_initialize(void) +int serial_initialize(void) { - serial_init(); + return serial_init(); }
static void _serial_putc(struct udevice *dev, char ch) diff --git a/drivers/serial/serial.c b/drivers/serial/serial.c index da017dc5b3..53358acb81 100644 --- a/drivers/serial/serial.c +++ b/drivers/serial/serial.c @@ -170,7 +170,7 @@ void serial_register(struct serial_device *dev) * serial port to the serial core. That serial port is then used as a * default output. */ -void serial_initialize(void) +int serial_initialize(void) { atmel_serial_initialize(); mcf_serial_initialize(); @@ -183,6 +183,8 @@ void serial_initialize(void) mtk_serial_initialize();
serial_assign(default_serial_console()->name); + + return 0; }
static int serial_stub_start(struct stdio_dev *sdev) diff --git a/include/serial.h b/include/serial.h index c590637b1f..6d1e62c677 100644 --- a/include/serial.h +++ b/include/serial.h @@ -42,10 +42,10 @@ extern struct serial_device eserial5_device; extern struct serial_device eserial6_device;
extern void serial_register(struct serial_device *); -extern void serial_initialize(void); extern void serial_stdio_init(void); extern int serial_assign(const char *name); extern void serial_reinit_all(void); +int serial_initialize(void);
/* For usbtty */ #ifdef CONFIG_USB_TTY
On Thu, 9 Jul 2020 at 02:10, Ovidiu Panait ovidiu.panait@windriver.com wrote:
serial_initialize is called only during the common init sequence, after relocation (in common/board_r.c). Because it has a void return value, it has to wrapped in initr_serial. In order to be able to get rid of this indirection, make serial_initialize return int.
Remove extern from prototype in order to silence the following checkpatch warning: check: extern prototypes should be avoided in .h files
Signed-off-by: Ovidiu Panait ovidiu.panait@windriver.com
drivers/serial/serial-uclass.c | 4 ++-- drivers/serial/serial.c | 4 +++- include/serial.h | 2 +- 3 files changed, 6 insertions(+), 4 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
Has bothered me for a while :-)
Remove the initr_serial->serial_initialize indirection and call serial_initialize directly.
Signed-off-by: Ovidiu Panait ovidiu.panait@windriver.com ---
common/board_r.c | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-)
diff --git a/common/board_r.c b/common/board_r.c index 5e924322b2..522059c5a5 100644 --- a/common/board_r.c +++ b/common/board_r.c @@ -187,12 +187,6 @@ static int initr_reloc_global_data(void) return 0; }
-static int initr_serial(void) -{ - serial_initialize(); - return 0; -} - #if defined(CONFIG_PPC) || defined(CONFIG_M68K) || defined(CONFIG_MIPS) static int initr_trap(void) { @@ -705,7 +699,7 @@ static init_fnc_t init_sequence_r[] = { #endif initr_dm_devices, stdio_init_tables, - initr_serial, + serial_initialize, initr_announce, #if CONFIG_IS_ENABLED(WDT) initr_watchdog,
On Thu, 9 Jul 2020 at 02:09, Ovidiu Panait ovidiu.panait@windriver.com wrote:
Remove the initr_serial->serial_initialize indirection and call serial_initialize directly.
Signed-off-by: Ovidiu Panait ovidiu.panait@windriver.com
common/board_r.c | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
Extend manual relocation of block_cache list pointers to all platforms that enable CONFIG_NEEDS_MANUAL_RELOC. Remove m68k-specific checks and provide a single implementation that adds gd->reloc_off to the pre-relocation pointers.
Cc: Angelo Durgehello angelo.dureghello@timesys.com Signed-off-by: Ovidiu Panait ovidiu.panait@windriver.com ---
common/board_r.c | 2 +- drivers/block/blkcache.c | 13 +++++++------ 2 files changed, 8 insertions(+), 7 deletions(-)
diff --git a/common/board_r.c b/common/board_r.c index 522059c5a5..29d831d5eb 100644 --- a/common/board_r.c +++ b/common/board_r.c @@ -836,7 +836,7 @@ static init_fnc_t init_sequence_r[] = { #if defined(CONFIG_PRAM) initr_mem, #endif -#if defined(CONFIG_M68K) && defined(CONFIG_BLOCK_CACHE) +#if defined(CONFIG_NEEDS_MANUAL_RELOC) && defined(CONFIG_BLOCK_CACHE) blkcache_init, #endif run_main_loop, diff --git a/drivers/block/blkcache.c b/drivers/block/blkcache.c index b6fc72fe98..d97ee99cf4 100644 --- a/drivers/block/blkcache.c +++ b/drivers/block/blkcache.c @@ -12,6 +12,8 @@ #include <linux/ctype.h> #include <linux/list.h>
+DECLARE_GLOBAL_DATA_PTR; + struct block_cache_node { struct list_head lh; int iftype; @@ -22,21 +24,20 @@ struct block_cache_node { char *cache; };
-#ifndef CONFIG_M68K static LIST_HEAD(block_cache); -#else -static struct list_head block_cache; -#endif
static struct block_cache_stats _stats = { .max_blocks_per_entry = 8, .max_entries = 32 };
-#ifdef CONFIG_M68K +#ifdef CONFIG_NEEDS_MANUAL_RELOC int blkcache_init(void) { - INIT_LIST_HEAD(&block_cache); + struct list_head *head = &block_cache; + + head->next = (uintptr_t)head->next + gd->reloc_off; + head->prev = (uintptr_t)head->prev + gd->reloc_off;
return 0; }
On Thu, 9 Jul 2020 at 02:09, Ovidiu Panait ovidiu.panait@windriver.com wrote:
Extend manual relocation of block_cache list pointers to all platforms that enable CONFIG_NEEDS_MANUAL_RELOC. Remove m68k-specific checks and provide a single implementation that adds gd->reloc_off to the pre-relocation pointers.
Cc: Angelo Durgehello angelo.dureghello@timesys.com Signed-off-by: Ovidiu Panait ovidiu.panait@windriver.com
common/board_r.c | 2 +- drivers/block/blkcache.c | 13 +++++++------ 2 files changed, 8 insertions(+), 7 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
blkcache_init manually relocates blkcache list pointers when CONFIG_NEEDS_MANUAL_RELOC is enabled. However, it is called very late in the boot sequence, which could be a problem if previous boot calls execute blkcache operations with the non-relocated pointers. For example, mmc is initialized earlier and might call blkcache_invalidate (in mmc_select_hwpart()) when trying to load the environment from mmc via env_load().
To fix this issue, move blkcache_init boot call earlier, before mmc gets initialized.
Cc: Angelo Durgehello angelo.dureghello@timesys.com Signed-off-by: Ovidiu Panait ovidiu.panait@windriver.com ---
common/board_r.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/common/board_r.c b/common/board_r.c index 29d831d5eb..a3c26bb380 100644 --- a/common/board_r.c +++ b/common/board_r.c @@ -705,6 +705,9 @@ static init_fnc_t init_sequence_r[] = { initr_watchdog, #endif INIT_FUNC_WATCHDOG_RESET +#if defined(CONFIG_NEEDS_MANUAL_RELOC) && defined(CONFIG_BLOCK_CACHE) + blkcache_init, +#endif #ifdef CONFIG_NEEDS_MANUAL_RELOC initr_manual_reloc_cmdtable, #endif @@ -835,9 +838,6 @@ static init_fnc_t init_sequence_r[] = { #endif #if defined(CONFIG_PRAM) initr_mem, -#endif -#if defined(CONFIG_NEEDS_MANUAL_RELOC) && defined(CONFIG_BLOCK_CACHE) - blkcache_init, #endif run_main_loop, };
On Thu, 9 Jul 2020 at 02:11, Ovidiu Panait ovidiu.panait@windriver.com wrote:
blkcache_init manually relocates blkcache list pointers when CONFIG_NEEDS_MANUAL_RELOC is enabled. However, it is called very late in the boot sequence, which could be a problem if previous boot calls execute blkcache operations with the non-relocated pointers. For example, mmc is initialized earlier and might call blkcache_invalidate (in mmc_select_hwpart()) when trying to load the environment from mmc via env_load().
To fix this issue, move blkcache_init boot call earlier, before mmc gets initialized.
Cc: Angelo Durgehello angelo.dureghello@timesys.com Signed-off-by: Ovidiu Panait ovidiu.panait@windriver.com
common/board_r.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
On Thu, 9 Jul 2020 at 02:07, Ovidiu Panait ovidiu.panait@windriver.com wrote:
Certain architectures (ppc, mips, sh, m68k) use setup board_part1 and setup_board_part2 calls during pre-relocation init to populate gd->bd boardinfo fields. This makes the generic init sequence cluttered with arch-specific ifdefs.
In order to clean these arch-specific sequences from generic init, introduce arch_setup_bdinfo weak initcall so that everyone can define their own bdinfo setup routines.
Signed-off-by: Ovidiu Panait ovidiu.panait@windriver.com
common/board_f.c | 6 ++++++ include/init.h | 12 ++++++++++++ 2 files changed, 18 insertions(+)
Reviewed-by: Simon Glass sjg@chromium.org
participants (4)
-
Heinrich Schuchardt -
Heinrich Schuchardt -
Ovidiu Panait -
Simon Glass