libfdt: replace ARCH_FIXUP_FDT with ARCH_FIXUP_FDT_MEMORY

PATCH Superseded Masahiro Yamada 3 participants 7 messages Oct 20, 2016 – Nov 26, 2016

Subsystems (5)

Arm (32-bit) PowerPC MIPS Synopsys ARC architecture +1 more subsystems

lore

Share & tools

lore

Masahiro Yamada

author

2016-10-20 00:15 UTC #0 lore raw

PATCH v2 1/1 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 <[email protected]>

---

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(-)

Kconfig +2 -3

index 1263d0b..b7cb142 100644
@@ -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

arch/arc/lib/bootm.c +5 -0

index 04d9d9c..5798149 100644
@@ -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();

arch/arm/lib/bootm-fdt.c +0 -2

index a517550..4481f9e 100644
@@ -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

arch/arm/lib/bootm.c +0 -2

index 53c3141..0e890ce 100644
@@ -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

arch/microblaze/lib/bootm.c +5 -0

index 3eb3440..2732203 100644
@@ -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)
 {

arch/mips/lib/bootm.c +0 -2

index 0c6a4ab..aa0475a 100644
@@ -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)
 {

arch/powerpc/lib/bootm.c +5 -0

index ef15e7a..17c5ed1 100644
@@ -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,

common/fdt_support.c +2 -0

index 0609470..c9f7019 100644
@@ -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)
 {

common/image-fdt.c +0 -2

index 5454227..e7540be 100644
@@ -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) {

include/fdt_support.h +8 -0

index 506bc5a..955c121 100644
@@ -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,

Signature · 1 line

1.9.1

Alexey Brodkin

2016-10-20 11:55 UTC #1 lore raw

PATCH v2 1/1 libfdt: replace ARCH_FIXUP_FDT with ARCH_FIXUP_FDT_MEMORY

Hello Masahiro-san,

Quoted 16 lines · Masahiro Yamada

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 <[email protected]>
---
Changes in v2:
?- Add empty stub to ARC, PowerPC, Microblaze instead of ???a weak function common/image-fdt.c

[snip]

arch/arc/lib/bootm.c void arch_lmb_reserve(struct lmb *lmb) · line 44

Quoted 13 lines

arch/arc/lib/bootm.c +0 -0

index 04d9d9c..5798149 100644
@@ -37,6 +37,11 @@ void arch_lmb_reserve(struct lmb *lmb)
 ?	lmb_reserve(lmb, sp, (CONFIG_SYS_SDRAM_BASE + gd->ram_size - sp));
 ?}

arch/arc/lib/bootm.c +5 -0

+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.

Quoted 27 lines

common/image-fdt.c +0 -2

index 5454227..e7540be 100644
@@ -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) {

include/fdt_support.h +0 -0

index 506bc5a..955c121 100644
@@ -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);

include/fdt_support.h int fdt_fixup_memory(void *blob, u64 start, u64 size); · line 104

Quoted 7 lines

include/fdt_support.h +7 -0

+#else
+static inline int fdt_fixup_memory_banks(void *blob, u64 start[], u64 size[],
+					?int banks)
+{
+	return 0;
+}
+#endif

-Alexey

Masahiro Yamada

author

2016-10-21 01:58 UTC #2 lore raw

PATCH v2 1/1 libfdt: replace ARCH_FIXUP_FDT with ARCH_FIXUP_FDT_MEMORY

Hi Alexey,

2016-10-20 20:55 GMT+09:00 Alexey Brodkin <[email protected]>:

Quoted 30 lines

Hello Masahiro-san,
Quoted 16 lines · Masahiro Yamada
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 <[email protected]>
---
Changes in v2:
- Add empty stub to ARC, PowerPC, Microblaze instead of a weak function common/image-fdt.c
[snip]
Quoted 8 lines
arch/arc/lib/bootm.c +0 -0
index 04d9d9c..5798149 100644
@@ -37,6 +37,11 @@ void arch_lmb_reserve(struct lmb *lmb)
      lmb_reserve(lmb, sp, (CONFIG_SYS_SDRAM_BASE + gd->ram_size - sp));
 }
 

Quoted 9 lines

arch/arc/lib/bootm.c void arch_lmb_reserve(struct lmb *lmb) · line 44
Quoted 5 lines
arch/arc/lib/bootm.c +5 -0
+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.

Signature · 2 lines

Best Regards
Masahiro Yamada

Alexey Brodkin

2016-10-21 07:12 UTC #3 lore raw

PATCH v2 1/1 libfdt: replace ARCH_FIXUP_FDT with ARCH_FIXUP_FDT_MEMORY

Hello Masahiro-san,

Quoted 47 lines · Masahiro Yamada

Hi Alexey,
2016-10-20 20:55 GMT+09:00 Alexey Brodkin <[email protected]>:
Quoted 33 lines
Hello Masahiro-san,
Quoted 17 lines · Masahiro Yamada
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 <[email protected]>
---
Changes in v2:
?- Add empty stub to ARC, PowerPC, Microblaze instead of ???a weak function common/image-fdt.c
[snip]
Quoted 9 lines
arch/arc/lib/bootm.c +0 -0
index 04d9d9c..5798149 100644
@@ -37,6 +37,11 @@ void arch_lmb_reserve(struct lmb *lmb)
 ??????lmb_reserve(lmb, sp, (CONFIG_SYS_SDRAM_BASE + gd->ram_size - sp));
 ?}
 
Quoted 9 lines
arch/arc/lib/bootm.c void arch_lmb_reserve(struct lmb *lmb) · line 44
Quoted 5 lines
arch/arc/lib/bootm.c +5 -0
+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.

Quoted 7 lines

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 <[email protected]>

Simon Glass

2016-10-24 03:01 UTC #4 lore raw

PATCH v2 1/1 libfdt: replace ARCH_FIXUP_FDT with ARCH_FIXUP_FDT_MEMORY

Quoted 56 lines · Alexey Brodkin

Hello Masahiro-san,
Quoted 53 lines · Masahiro Yamada
Hi Alexey,
2016-10-20 20:55 GMT+09:00 Alexey Brodkin <[email protected]>:
Quoted 33 lines
Hello Masahiro-san,
Quoted 17 lines · Masahiro Yamada
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 <[email protected]>
---
Changes in v2:
- Add empty stub to ARC, PowerPC, Microblaze instead of a weak function common/image-fdt.c
[snip]
Quoted 9 lines
arch/arc/lib/bootm.c +0 -0
index 04d9d9c..5798149 100644
@@ -37,6 +37,11 @@ void arch_lmb_reserve(struct lmb *lmb)
      lmb_reserve(lmb, sp, (CONFIG_SYS_SDRAM_BASE + gd->ram_size - sp));
 }
 
Quoted 9 lines
arch/arc/lib/bootm.c void arch_lmb_reserve(struct lmb *lmb) · line 44
Quoted 5 lines
arch/arc/lib/bootm.c +5 -0
+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/

Quoted 8 lines

Quoted 1 line
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 <[email protected]>

Acked-by Simon Glass <[email protected]>

Simon Glass

2016-11-25 19:37 UTC #5 lore raw

PATCH v2 1/1 libfdt: replace ARCH_FIXUP_FDT with ARCH_FIXUP_FDT_MEMORY

Hi,

Quoted 59 lines · Simon Glass

Quoted 58 lines · Alexey Brodkin
Hello Masahiro-san,
Quoted 54 lines · Masahiro Yamada
Hi Alexey,
2016-10-20 20:55 GMT+09:00 Alexey Brodkin <[email protected]>:
Quoted 33 lines
Hello Masahiro-san,
Quoted 17 lines · Masahiro Yamada
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 <[email protected]>
---
Changes in v2:
- Add empty stub to ARC, PowerPC, Microblaze instead of a weak function common/image-fdt.c
[snip]
Quoted 9 lines
arch/arc/lib/bootm.c +0 -0
index 04d9d9c..5798149 100644
@@ -37,6 +37,11 @@ void arch_lmb_reserve(struct lmb *lmb)
      lmb_reserve(lmb, sp, (CONFIG_SYS_SDRAM_BASE + gd->ram_size - sp));
 }
 
Quoted 9 lines
arch/arc/lib/bootm.c void arch_lmb_reserve(struct lmb *lmb) · line 44
Quoted 5 lines
arch/arc/lib/bootm.c +5 -0
+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.

Quoted 8 lines

Quoted 6 lines
Ok, thanks for the reference.
Indeed Simon's comments make sense.
So for ARC changes
Acked-by Alexey Brodkin <[email protected]>
Acked-by Simon Glass <[email protected]>

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

Masahiro Yamada

author

2016-11-26 02:07 UTC #6 lore raw

PATCH v2 1/1 libfdt: replace ARCH_FIXUP_FDT with ARCH_FIXUP_FDT_MEMORY

Hi Simon,

2016-11-26 4:37 GMT+09:00 Simon Glass <[email protected]>:

Review

Quoted 9 lines

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
(quoted diff) +5 -0
+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.

Signature · 2 lines

Best Regards
Masahiro Yamada