| Message ID | 1476922559-22084-1-git-send-email-yamada.masahiro@socionext.com |
|---|---|
| State | Superseded |
| Headers | show |
Hello Masahiro-san, On Thu, 2016-10-20 at 09:15 +0900, Masahiro Yamada 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> > --- > > Changes in v2: > - Add empty stub to ARC, PowerPC, Microblaze instead of > a weak function common/image-fdt.c [snip] > diff --git a/arch/arc/lib/bootm.c b/arch/arc/lib/bootm.c > index 04d9d9c..5798149 100644 > --- a/arch/arc/lib/bootm.c > +++ b/arch/arc/lib/bootm.c > @@ -37,6 +37,11 @@ void arch_lmb_reserve(struct lmb *lmb) > lmb_reserve(lmb, sp, (CONFIG_SYS_SDRAM_BASE + gd->ram_size - sp)); > } > > +int arch_fixup_fdt(void *blob) > +{ > + return 0; > +} > + I'm wondering why don't we add weak implementation of arch_fixup_fdt() right in say common/image-fdt.c? This will allow us to not add dummy stubs for those arches that don't really use it. > diff --git a/common/image-fdt.c b/common/image-fdt.c > index 5454227..e7540be 100644 > --- a/common/image-fdt.c > +++ b/common/image-fdt.c > @@ -474,12 +474,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 506bc5a..955c121 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 -Alexey
Hi Alexey, 2016-10-20 20:55 GMT+09:00 Alexey Brodkin <Alexey.Brodkin@synopsys.com>: > Hello Masahiro-san, > > On Thu, 2016-10-20 at 09:15 +0900, Masahiro Yamada 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> >> --- >> >> Changes in v2: >> - Add empty stub to ARC, PowerPC, Microblaze instead of >> a weak function common/image-fdt.c > > [snip] > >> diff --git a/arch/arc/lib/bootm.c b/arch/arc/lib/bootm.c >> index 04d9d9c..5798149 100644 >> --- a/arch/arc/lib/bootm.c >> +++ b/arch/arc/lib/bootm.c >> @@ -37,6 +37,11 @@ void arch_lmb_reserve(struct lmb *lmb) >> lmb_reserve(lmb, sp, (CONFIG_SYS_SDRAM_BASE + gd->ram_size - sp)); >> } >> >> +int arch_fixup_fdt(void *blob) >> +{ >> + return 0; >> +} >> + > > I'm wondering why don't we add weak implementation of arch_fixup_fdt() > right in say common/image-fdt.c? This will allow us to not add dummy stubs > for those arches that don't really use it. I fully agree with you. I used a weak function in v1: http://patchwork.ozlabs.org/patch/678049/ But, it was change-request'ed by Simon. -- Best Regards Masahiro Yamada _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Hello Masahiro-san, On Fri, 2016-10-21 at 10:58 +0900, Masahiro Yamada wrote: > Hi Alexey, > > 2016-10-20 20:55 GMT+09:00 Alexey Brodkin <Alexey.Brodkin@synopsys.com>: > > > > Hello Masahiro-san, > > > > On Thu, 2016-10-20 at 09:15 +0900, Masahiro Yamada 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> > > > --- > > > > > > Changes in v2: > > > - Add empty stub to ARC, PowerPC, Microblaze instead of > > > a weak function common/image-fdt.c > > > > [snip] > > > > > > > > diff --git a/arch/arc/lib/bootm.c b/arch/arc/lib/bootm.c > > > index 04d9d9c..5798149 100644 > > > --- a/arch/arc/lib/bootm.c > > > +++ b/arch/arc/lib/bootm.c > > > @@ -37,6 +37,11 @@ void arch_lmb_reserve(struct lmb *lmb) > > > lmb_reserve(lmb, sp, (CONFIG_SYS_SDRAM_BASE + gd->ram_size - sp)); > > > } > > > > > > +int arch_fixup_fdt(void *blob) > > > +{ > > > + return 0; > > > +} > > > + > > > > I'm wondering why don't we add weak implementation of arch_fixup_fdt() > > right in say common/image-fdt.c? This will allow us to not add dummy stubs > > for those arches that don't really use it. > > > I fully agree with you. > > > I used a weak function in v1: > http://patchwork.ozlabs.org/patch/678049/ > > But, it was change-request'ed by Simon. Ok, thanks for the reference. Indeed Simon's comments make sense. So for ARC changes Acked-by: Alexey Brodkin <abrodkin@synopsys.com>
On 21 October 2016 at 01:12, Alexey Brodkin <Alexey.Brodkin@synopsys.com> wrote: > Hello Masahiro-san, > > On Fri, 2016-10-21 at 10:58 +0900, Masahiro Yamada wrote: >> Hi Alexey, >> >> 2016-10-20 20:55 GMT+09:00 Alexey Brodkin <Alexey.Brodkin@synopsys.com>: >> > >> > Hello Masahiro-san, >> > >> > On Thu, 2016-10-20 at 09:15 +0900, Masahiro Yamada 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> >> > > --- >> > > >> > > Changes in v2: >> > > - Add empty stub to ARC, PowerPC, Microblaze instead of >> > > a weak function common/image-fdt.c >> > >> > [snip] >> > >> > > >> > > diff --git a/arch/arc/lib/bootm.c b/arch/arc/lib/bootm.c >> > > index 04d9d9c..5798149 100644 >> > > --- a/arch/arc/lib/bootm.c >> > > +++ b/arch/arc/lib/bootm.c >> > > @@ -37,6 +37,11 @@ void arch_lmb_reserve(struct lmb *lmb) >> > > lmb_reserve(lmb, sp, (CONFIG_SYS_SDRAM_BASE + gd->ram_size - sp)); >> > > } >> > > >> > > +int arch_fixup_fdt(void *blob) >> > > +{ >> > > + return 0; >> > > +} >> > > + >> > >> > I'm wondering why don't we add weak implementation of arch_fixup_fdt() >> > right in say common/image-fdt.c? This will allow us to not add dummy stubs >> > for those arches that don't really use it. >> >> >> I fully agree with you. >> >> >> I used a weak function in v1: >> http://patchwork.ozlabs.org/patch/678049/ >> >> But, it was change-request'ed by Simon. > > Ok, thanks for the reference. > Indeed Simon's comments make sense. > > So for ARC changes > > Acked-by: Alexey Brodkin <abrodkin@synopsys.com> Acked-by: Simon Glass <sjg@chromium.org> _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Hi, On 23 October 2016 at 21:01, Simon Glass <sjg@chromium.org> wrote: > On 21 October 2016 at 01:12, Alexey Brodkin <Alexey.Brodkin@synopsys.com> wrote: >> Hello Masahiro-san, >> >> On Fri, 2016-10-21 at 10:58 +0900, Masahiro Yamada wrote: >>> Hi Alexey, >>> >>> 2016-10-20 20:55 GMT+09:00 Alexey Brodkin <Alexey.Brodkin@synopsys.com>: >>> > >>> > Hello Masahiro-san, >>> > >>> > On Thu, 2016-10-20 at 09:15 +0900, Masahiro Yamada 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> >>> > > --- >>> > > >>> > > Changes in v2: >>> > > - Add empty stub to ARC, PowerPC, Microblaze instead of >>> > > a weak function common/image-fdt.c >>> > >>> > [snip] >>> > >>> > > >>> > > diff --git a/arch/arc/lib/bootm.c b/arch/arc/lib/bootm.c >>> > > index 04d9d9c..5798149 100644 >>> > > --- a/arch/arc/lib/bootm.c >>> > > +++ b/arch/arc/lib/bootm.c >>> > > @@ -37,6 +37,11 @@ void arch_lmb_reserve(struct lmb *lmb) >>> > > lmb_reserve(lmb, sp, (CONFIG_SYS_SDRAM_BASE + gd->ram_size - sp)); >>> > > } >>> > > >>> > > +int arch_fixup_fdt(void *blob) >>> > > +{ >>> > > + return 0; >>> > > +} >>> > > + >>> > >>> > I'm wondering why don't we add weak implementation of arch_fixup_fdt() >>> > right in say common/image-fdt.c? This will allow us to not add dummy stubs >>> > for those arches that don't really use it. >>> >>> >>> I fully agree with you. >>> >>> >>> I used a weak function in v1: >>> http://patchwork.ozlabs.org/patch/678049/ >>> >>> But, it was change-request'ed by Simon. >> >> Ok, thanks for the reference. >> Indeed Simon's comments make sense. >> >> So for ARC changes >> >> Acked-by: Alexey Brodkin <abrodkin@synopsys.com> > > Acked-by: Simon Glass <sjg@chromium.org> Unfortunately this breaks sandbox - can you please take a look? 02: libfdt: replace ARCH_FIXUP_FDT with ARCH_FIXUP_FDT_MEMORY sandbox: + sandbox sandbox_spl sandbox_noblk +common/built-in.o: In function `image_setup_libfdt': +build/../common/image-fdt.c:477: undefined reference to `arch_fixup_fdt' +collect2: error: ld returned 1 exit status +make[1]: *** [u-boot] Error 1 +make: *** [sub-make] Error 2 Regards, Simon _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Hi Simon, 2016-11-26 4:37 GMT+09:00 Simon Glass <sjg@chromium.org>: > Unfortunately this breaks sandbox - can you please take a look? > > 02: libfdt: replace ARCH_FIXUP_FDT with ARCH_FIXUP_FDT_MEMORY > sandbox: + sandbox sandbox_spl sandbox_noblk > +common/built-in.o: In function `image_setup_libfdt': > +build/../common/image-fdt.c:477: undefined reference to `arch_fixup_fdt' > +collect2: error: ld returned 1 exit status > +make[1]: *** [u-boot] Error 1 > +make: *** [sub-make] Error 2 Oops. I've posted v3. Thanks. -- Best Regards Masahiro Yamada _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
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/arc/lib/bootm.c b/arch/arc/lib/bootm.c index 04d9d9c..5798149 100644 --- a/arch/arc/lib/bootm.c +++ b/arch/arc/lib/bootm.c @@ -37,6 +37,11 @@ void arch_lmb_reserve(struct lmb *lmb) lmb_reserve(lmb, sp, (CONFIG_SYS_SDRAM_BASE + gd->ram_size - sp)); } +int arch_fixup_fdt(void *blob) +{ + return 0; +} + static int cleanup_before_linux(void) { disable_interrupts(); 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/microblaze/lib/bootm.c b/arch/microblaze/lib/bootm.c index 3eb3440..2732203 100644 --- a/arch/microblaze/lib/bootm.c +++ b/arch/microblaze/lib/bootm.c @@ -17,6 +17,11 @@ DECLARE_GLOBAL_DATA_PTR; +int arch_fixup_fdt(void *blob) +{ + return 0; +} + int do_bootm_linux(int flag, int argc, char * const argv[], bootm_headers_t *images) { 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/arch/powerpc/lib/bootm.c b/arch/powerpc/lib/bootm.c index ef15e7a..17c5ed1 100644 --- a/arch/powerpc/lib/bootm.c +++ b/arch/powerpc/lib/bootm.c @@ -38,6 +38,11 @@ static void set_clocks_in_mhz (bd_t *kbd); #define CONFIG_SYS_LINUX_LOWMEM_MAX_SIZE (768*1024*1024) #endif +int arch_fixup_fdt(void *blob) +{ + return 0; +} + static void boot_jump_linux(bootm_headers_t *images) { void (*kernel)(bd_t *, ulong r4, ulong r5, ulong r6, diff --git a/common/fdt_support.c b/common/fdt_support.c index 0609470..c9f7019 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 5454227..e7540be 100644 --- a/common/image-fdt.c +++ b/common/image-fdt.c @@ -474,12 +474,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 506bc5a..955c121 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,
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> --- Changes in v2: - Add empty stub to ARC, PowerPC, Microblaze instead of a weak function common/image-fdt.c Kconfig | 5 ++--- arch/arc/lib/bootm.c | 5 +++++ arch/arm/lib/bootm-fdt.c | 2 -- arch/arm/lib/bootm.c | 2 -- arch/microblaze/lib/bootm.c | 5 +++++ arch/mips/lib/bootm.c | 2 -- arch/powerpc/lib/bootm.c | 5 +++++ common/fdt_support.c | 2 ++ common/image-fdt.c | 2 -- include/fdt_support.h | 8 ++++++++ 10 files changed, 27 insertions(+), 11 deletions(-) -- 1.9.1 _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot