[U-Boot] [PATCH] libfdt: replace ARCH_FIXUP_FDT with ARCH_FIXUP_FDT_MEMORY
Commit e2f88dfd2d96 ("libfdt: Introduce new ARCH_FIXUP_FDT option") allows us to skip memory setup of DTB, but a problem for ARM is that spin_table_update_dt() and psci_update_dt() are skipped as well if CONFIG_ARCH_FIXUP_FDT is disabled.
This commit allows us to skip only fdt_fixup_memory_banks() instead of the whole of arch_fixup_fdt(). It will be useful when we want to use a memory node from a kernel DTB as is, but need some fixups for Spin-Table/PSCI.
Signed-off-by: Masahiro Yamada yamada.masahiro@socionext.com ---
Kconfig | 5 ++--- arch/arm/lib/bootm-fdt.c | 2 -- arch/arm/lib/bootm.c | 2 -- arch/mips/lib/bootm.c | 2 -- common/fdt_support.c | 2 ++ common/image-fdt.c | 7 +++++-- include/fdt_support.h | 8 ++++++++ 7 files changed, 17 insertions(+), 11 deletions(-)
diff --git a/Kconfig b/Kconfig index 1263d0b..b7cb142 100644 --- a/Kconfig +++ b/Kconfig @@ -324,9 +324,8 @@ config SYS_CLK_FREQ help TODO: Move CONFIG_SYS_CLK_FREQ for all the architecture
-config ARCH_FIXUP_FDT - bool "Enable arch_fixup_fdt() call" - depends on ARM || MIPS +config ARCH_FIXUP_FDT_MEMORY + bool "Enable arch_fixup_memory_banks() call" default y help Enable FDT memory map syncup before OS boot. This feature can be diff --git a/arch/arm/lib/bootm-fdt.c b/arch/arm/lib/bootm-fdt.c index a517550..4481f9e 100644 --- a/arch/arm/lib/bootm-fdt.c +++ b/arch/arm/lib/bootm-fdt.c @@ -25,7 +25,6 @@
DECLARE_GLOBAL_DATA_PTR;
-#ifdef CONFIG_ARCH_FIXUP_FDT int arch_fixup_fdt(void *blob) { bd_t *bd = gd->bd; @@ -61,4 +60,3 @@ int arch_fixup_fdt(void *blob)
return 0; } -#endif diff --git a/arch/arm/lib/bootm.c b/arch/arm/lib/bootm.c index 53c3141..0e890ce 100644 --- a/arch/arm/lib/bootm.c +++ b/arch/arm/lib/bootm.c @@ -372,10 +372,8 @@ void boot_prep_vxworks(bootm_headers_t *images) if (images->ft_addr) { off = fdt_path_offset(images->ft_addr, "/memory"); if (off < 0) { -#ifdef CONFIG_ARCH_FIXUP_FDT if (arch_fixup_fdt(images->ft_addr)) puts("## WARNING: fixup memory failed!\n"); -#endif } } #endif diff --git a/arch/mips/lib/bootm.c b/arch/mips/lib/bootm.c index 0c6a4ab..aa0475a 100644 --- a/arch/mips/lib/bootm.c +++ b/arch/mips/lib/bootm.c @@ -253,7 +253,6 @@ static int boot_reloc_fdt(bootm_headers_t *images) #endif }
-#ifdef CONFIG_ARCH_FIXUP_FDT int arch_fixup_fdt(void *blob) { #if CONFIG_IS_ENABLED(MIPS_BOOT_FDT) && CONFIG_IS_ENABLED(OF_LIBFDT) @@ -265,7 +264,6 @@ int arch_fixup_fdt(void *blob) return 0; #endif } -#endif
static int boot_setup_fdt(bootm_headers_t *images) { diff --git a/common/fdt_support.c b/common/fdt_support.c index 2020586..c87031f 100644 --- a/common/fdt_support.c +++ b/common/fdt_support.c @@ -381,6 +381,7 @@ void do_fixup_by_compat_u32(void *fdt, const char *compat, do_fixup_by_compat(fdt, compat, prop, &tmp, 4, create); }
+#ifdef CONFIG_ARCH_FIXUP_FDT_MEMORY /* * fdt_pack_reg - pack address and size array into the "reg"-suitable stream */ @@ -459,6 +460,7 @@ int fdt_fixup_memory_banks(void *blob, u64 start[], u64 size[], int banks) } return 0; } +#endif
int fdt_fixup_memory(void *blob, u64 start, u64 size) { diff --git a/common/image-fdt.c b/common/image-fdt.c index 3d23608..91970d4 100644 --- a/common/image-fdt.c +++ b/common/image-fdt.c @@ -458,6 +458,11 @@ __weak int ft_verify_fdt(void *fdt) return 1; }
+__weak int arch_fixup_fdt(void *blob) +{ + return 0; +} + int image_setup_libfdt(bootm_headers_t *images, void *blob, int of_size, struct lmb *lmb) { @@ -474,12 +479,10 @@ int image_setup_libfdt(bootm_headers_t *images, void *blob, printf("ERROR: /chosen node create failed\n"); goto err; } -#ifdef CONFIG_ARCH_FIXUP_FDT if (arch_fixup_fdt(blob) < 0) { printf("ERROR: arch-specific fdt fixup failed\n"); goto err; } -#endif if (IMAGE_OF_BOARD_SETUP) { fdt_ret = ft_board_setup(blob, gd->bd); if (fdt_ret) { diff --git a/include/fdt_support.h b/include/fdt_support.h index 8f40231..7110061 100644 --- a/include/fdt_support.h +++ b/include/fdt_support.h @@ -93,7 +93,15 @@ int fdt_fixup_memory(void *blob, u64 start, u64 size); * property will be left untouched. * @return 0 if ok, or -1 or -FDT_ERR_... on error */ +#ifdef CONFIG_ARCH_FIXUP_FDT_MEMORY int fdt_fixup_memory_banks(void *blob, u64 start[], u64 size[], int banks); +#else +static inline int fdt_fixup_memory_banks(void *blob, u64 start[], u64 size[], + int banks) +{ + return 0; +} +#endif
void fdt_fixup_ethernet(void *fdt); int fdt_find_and_setprop(void *fdt, const char *node, const char *prop,
Hi Masahiro,
On 4 October 2016 at 06:03, Masahiro Yamada yamada.masahiro@socionext.com wrote:
Commit e2f88dfd2d96 ("libfdt: Introduce new ARCH_FIXUP_FDT option") allows us to skip memory setup of DTB, but a problem for ARM is that spin_table_update_dt() and psci_update_dt() are skipped as well if CONFIG_ARCH_FIXUP_FDT is disabled.
This commit allows us to skip only fdt_fixup_memory_banks() instead of the whole of arch_fixup_fdt(). It will be useful when we want to use a memory node from a kernel DTB as is, but need some fixups for Spin-Table/PSCI.
Signed-off-by: Masahiro Yamada yamada.masahiro@socionext.com
Kconfig | 5 ++--- arch/arm/lib/bootm-fdt.c | 2 -- arch/arm/lib/bootm.c | 2 -- arch/mips/lib/bootm.c | 2 -- common/fdt_support.c | 2 ++ common/image-fdt.c | 7 +++++-- include/fdt_support.h | 8 ++++++++ 7 files changed, 17 insertions(+), 11 deletions(-)
diff --git a/Kconfig b/Kconfig index 1263d0b..b7cb142 100644 --- a/Kconfig +++ b/Kconfig @@ -324,9 +324,8 @@ config SYS_CLK_FREQ help TODO: Move CONFIG_SYS_CLK_FREQ for all the architecture
-config ARCH_FIXUP_FDT
bool "Enable arch_fixup_fdt() call"depends on ARM || MIPS+config ARCH_FIXUP_FDT_MEMORY
bool "Enable arch_fixup_memory_banks() call" default y help Enable FDT memory map syncup before OS boot. This feature can bediff --git a/arch/arm/lib/bootm-fdt.c b/arch/arm/lib/bootm-fdt.c index a517550..4481f9e 100644 --- a/arch/arm/lib/bootm-fdt.c +++ b/arch/arm/lib/bootm-fdt.c @@ -25,7 +25,6 @@
DECLARE_GLOBAL_DATA_PTR;
-#ifdef CONFIG_ARCH_FIXUP_FDT int arch_fixup_fdt(void *blob) { bd_t *bd = gd->bd; @@ -61,4 +60,3 @@ int arch_fixup_fdt(void *blob)
return 0;} -#endif diff --git a/arch/arm/lib/bootm.c b/arch/arm/lib/bootm.c index 53c3141..0e890ce 100644 --- a/arch/arm/lib/bootm.c +++ b/arch/arm/lib/bootm.c @@ -372,10 +372,8 @@ void boot_prep_vxworks(bootm_headers_t *images) if (images->ft_addr) { off = fdt_path_offset(images->ft_addr, "/memory"); if (off < 0) { -#ifdef CONFIG_ARCH_FIXUP_FDT if (arch_fixup_fdt(images->ft_addr)) puts("## WARNING: fixup memory failed!\n"); -#endif } } #endif diff --git a/arch/mips/lib/bootm.c b/arch/mips/lib/bootm.c index 0c6a4ab..aa0475a 100644 --- a/arch/mips/lib/bootm.c +++ b/arch/mips/lib/bootm.c @@ -253,7 +253,6 @@ static int boot_reloc_fdt(bootm_headers_t *images) #endif }
-#ifdef CONFIG_ARCH_FIXUP_FDT int arch_fixup_fdt(void *blob) { #if CONFIG_IS_ENABLED(MIPS_BOOT_FDT) && CONFIG_IS_ENABLED(OF_LIBFDT) @@ -265,7 +264,6 @@ int arch_fixup_fdt(void *blob) return 0; #endif } -#endif
static int boot_setup_fdt(bootm_headers_t *images) { diff --git a/common/fdt_support.c b/common/fdt_support.c index 2020586..c87031f 100644 --- a/common/fdt_support.c +++ b/common/fdt_support.c @@ -381,6 +381,7 @@ void do_fixup_by_compat_u32(void *fdt, const char *compat, do_fixup_by_compat(fdt, compat, prop, &tmp, 4, create); }
+#ifdef CONFIG_ARCH_FIXUP_FDT_MEMORY /*
- fdt_pack_reg - pack address and size array into the "reg"-suitable stream
*/ @@ -459,6 +460,7 @@ int fdt_fixup_memory_banks(void *blob, u64 start[], u64 size[], int banks) } return 0; } +#endif
int fdt_fixup_memory(void *blob, u64 start, u64 size) { diff --git a/common/image-fdt.c b/common/image-fdt.c index 3d23608..91970d4 100644 --- a/common/image-fdt.c +++ b/common/image-fdt.c @@ -458,6 +458,11 @@ __weak int ft_verify_fdt(void *fdt) return 1; }
+__weak int arch_fixup_fdt(void *blob) +{
return 0;+}
Do we have to have a weak function? I was hoping we could avoid these since they make it hard to figure out at build time what code is executed.
int image_setup_libfdt(bootm_headers_t *images, void *blob, int of_size, struct lmb *lmb) { @@ -474,12 +479,10 @@ int image_setup_libfdt(bootm_headers_t *images, void *blob, printf("ERROR: /chosen node create failed\n"); goto err; } -#ifdef CONFIG_ARCH_FIXUP_FDT if (arch_fixup_fdt(blob) < 0) { printf("ERROR: arch-specific fdt fixup failed\n"); goto err; } -#endif if (IMAGE_OF_BOARD_SETUP) { fdt_ret = ft_board_setup(blob, gd->bd); if (fdt_ret) { diff --git a/include/fdt_support.h b/include/fdt_support.h index 8f40231..7110061 100644 --- a/include/fdt_support.h +++ b/include/fdt_support.h @@ -93,7 +93,15 @@ int fdt_fixup_memory(void *blob, u64 start, u64 size);
property will be left untouched.- @return 0 if ok, or -1 or -FDT_ERR_... on error
*/ +#ifdef CONFIG_ARCH_FIXUP_FDT_MEMORY int fdt_fixup_memory_banks(void *blob, u64 start[], u64 size[], int banks); +#else +static inline int fdt_fixup_memory_banks(void *blob, u64 start[], u64 size[],
int banks)+{
return 0;+} +#endif
void fdt_fixup_ethernet(void *fdt); int fdt_find_and_setprop(void *fdt, const char *node, const char *prop, -- 1.9.1
Regards, Simon
Hi Simon,
2016-10-05 0:37 GMT+09:00 Simon Glass sjg@chromium.org:
diff --git a/common/image-fdt.c b/common/image-fdt.c index 3d23608..91970d4 100644 --- a/common/image-fdt.c +++ b/common/image-fdt.c @@ -458,6 +458,11 @@ __weak int ft_verify_fdt(void *fdt) return 1; }
+__weak int arch_fixup_fdt(void *blob) +{
return 0;+}
Do we have to have a weak function? I was hoping we could avoid these since they make it hard to figure out at build time what code is executed.
This hunk is just reverting Michal's commit e2f88dfd2d9671.
Is it better to add an empty stub to every architecture that may call it?
Hi Masahiro,
On 4 October 2016 at 21:27, Masahiro Yamada yamada.masahiro@socionext.com wrote:
Hi Simon,
2016-10-05 0:37 GMT+09:00 Simon Glass sjg@chromium.org:
diff --git a/common/image-fdt.c b/common/image-fdt.c index 3d23608..91970d4 100644 --- a/common/image-fdt.c +++ b/common/image-fdt.c @@ -458,6 +458,11 @@ __weak int ft_verify_fdt(void *fdt) return 1; }
+__weak int arch_fixup_fdt(void *blob) +{
return 0;+}
Do we have to have a weak function? I was hoping we could avoid these since they make it hard to figure out at build time what code is executed.
This hunk is just reverting Michal's commit e2f88dfd2d9671.
Is it better to add an empty stub to every architecture that may call it?
IMO all the FDT fixups need work. Perhaps we need a linker list approach so we can declare these fixups more easily? Or perhaps that will just make things harder to figure out?
But in this case I'm keen to avoid going back to using a weak function. Can we do that?
Regards, Simon
Hi Simon,
2016-10-06 1:09 GMT+09:00 Simon Glass sjg@chromium.org:
Hi Masahiro,
On 4 October 2016 at 21:27, Masahiro Yamada yamada.masahiro@socionext.com wrote:
Hi Simon,
2016-10-05 0:37 GMT+09:00 Simon Glass sjg@chromium.org:
diff --git a/common/image-fdt.c b/common/image-fdt.c index 3d23608..91970d4 100644 --- a/common/image-fdt.c +++ b/common/image-fdt.c @@ -458,6 +458,11 @@ __weak int ft_verify_fdt(void *fdt) return 1; }
+__weak int arch_fixup_fdt(void *blob) +{
return 0;+}
Do we have to have a weak function? I was hoping we could avoid these since they make it hard to figure out at build time what code is executed.
This hunk is just reverting Michal's commit e2f88dfd2d9671.
Is it better to add an empty stub to every architecture that may call it?
IMO all the FDT fixups need work. Perhaps we need a linker list approach so we can declare these fixups more easily? Or perhaps that will just make things harder to figure out?
This is up to you.
My interest is to not touch memory node, but need other DT fixups.
I sent v2 with no-op stubs instead of the weak function.
participants (2)
-
Masahiro Yamada -
Simon Glass